Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand secrets env command formatting options #144

Closed
4 tasks done
tegefaulkes opened this issue Feb 28, 2024 · 14 comments · Fixed by #147
Closed
4 tasks done

Expand secrets env command formatting options #144

tegefaulkes opened this issue Feb 28, 2024 · 14 comments · Fixed by #147
Assignees
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Feb 28, 2024

Specification

There are a few parts to this.

First, we need to expand the range of formats available. While the basic linux ones are are supported we're missing formats useful for windows CMD and Powershell.

Second, while I appended the dotenv and prepend format to the --format choices. It seems this added the choices to all of the commands. This was unintended and needs to be fixed. This means the hacky implementation needs to be replaced by removing the format option from the CommandPolykey constructor and making each command add the format option themselves. This means all commands need to be updated. Then select commands can add their own custom --format if needed.

Thirdly, all of the format options need to be clearly explained as part of the secrets env --help help output.

Lastly, the --format option should default in such a way that the most appropriate platform specific format is used. This means we need to auto switch based on the platform.

Additional context

Tasks

  • 1. expand the format options to include windows specific env formats.
  • 2. Fix how the --format option is modified. It's currently changed globally but that is unintended.
  • 3. The help output needs to be expanded to explain all of the different format outputs.
  • 4. the --format option should default to the most appropriate env format for the platform/terminal it is run in. We can switch based on platform but it would be advantageous to detect CMD or Powershell on windows.
@tegefaulkes tegefaulkes added the development Standard development label Feb 28, 2024
@tegefaulkes
Copy link
Contributor Author

Related commentary: #129 (comment)

@tegefaulkes
Copy link
Contributor Author

Going over the formats we may want to support.

For linux

Linux has a few ways to set envs. Right now we support prepend which is a name I've given to what you provide when running a command. And dotenv which is the .env format.

The dotenv is the format when specifying a .env file. It will be used to write directly into a file. I think one way we can improve this is to add comments outlining where each secret came from.

# vault1:path1
key1="value1"
# vault1:dir1/path1
KEY2="value2"
# vault2:path3
KEY_3="value 3"

prepend is something I don't have a good name for. It's how you'd write out envs just before running a command. for example key=value command. This format will only have the env set for the duration of the command and will not be passed to children AFAIK. If you want to take the output of secrets env in this format and directly use it then there are some extra steps such as using eval or source.

Note that for this format doing test=value echo $test will not output the value of test. The test is set whole echo is running, but not when $test is substituted.

key1="value1 key2="value2" key3=value3

If you want these to be added to the current environment and children processes then you'd need to use export.

To be continued.

@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Feb 28, 2024

For windows we have to 'terminals' we need to support, cmd and Powershell. Each has it's own ways of setting the envrioment.

For cmd

As far as I can tell, there isn't really a .env equivalent in cmd. the format is similar key=value but is set with the set keyword. You can store envs in a vars.bat file and run that to set the enviroment. That would take the following format. Comments can be set with 'REM' or '::'

:: var.bat file 

REM vault1:path1
set "key1=value1"
:: vault1:dir1/path1
set "KEY2=value2"
REM vault2:path3
set "KEY_3=value 3"

Likewise, the equivalent of setting this on a single line prepend format would be set "key1=value1" & set "key2=value2" & set "key3=value 3". From what I can tell, this should work of copied and pasted but I don't thing cmd has a way to directly eval this. However we can take the above .bat format and run the command output using for /f. for /f "tokens=*" %i in ('polykey secrets env -e vault:secret --format dotbat') do @%i

@tegefaulkes
Copy link
Contributor Author

I'll write up the powershell formats once I've looked into it more.

In the mean time, removing --format option from the CommandPolykey is a no-go. It's being used to change the formatting of the logger output. So we can't replace it with a fully custom format options.

So it means that we may just need a 2nd format option for the secrets env command. using the --env-format option I had before removing it...

@tegefaulkes
Copy link
Contributor Author

For powershell

In the simplest form, the env is set in Powershell with $env:key = 'value'

For multiple lines It will be formatted much the same way.

# vault1:SECRET1
$env:SECRET1 = 'this is the secret1'
# vault1:dir1/SECRET3
$env:SECRET3 = 'this is the secret3'
# vault2:SECRET2
$env:SECRET2 = 'this is the secret2'
# vault2:dir1/SECRET4
$env:SECRET4 = 'this is the secret4'

I don't think there's an equivalent to the prepend option where everything is set on one line.

@CMCDragonkai
Copy link
Member

By default so not export unless the export flag is set.

@CMCDragonkai
Copy link
Member

Do not export* - default behaviour should be to set shell session variables only!

@tegefaulkes
Copy link
Contributor Author

Here are all of the --env-format options I've made.

  1. platform - Auto selects the format based on the platform. Right now linux and mac defaults to dotenv and win32 defaults to dotbat.
  2. human - Basically an alias for dotenv since I consider that one the most readable and informative.
  3. json - Outputs as a json object in the form of {key: 'value'};
  4. dotenv - In the .env format, it includes comments before each env showing the secret path the env was taken from.
  5. dotbat - In a format that can be run as a .bat file in windows cmd. it includes comments before each env showing the secret path the env was taken from.
  6. dotps - In a format that can be run as a .ps1 file in windows Powershell. it includes comments before each env showing the secret path the env was taken from.
  7. prepend - In a format that can be added just before running a command to set the env varibles for that command.
  8. prependCmd - Similar to prepend but formatted to set env variables in CMD in a single line.

@tegefaulkes
Copy link
Contributor Author

Do not export* - default behaviour should be to set shell session variables only!

That is currently the case, Right now there is no functionality to export envs unless you take one of the formatted outputs and do that manually. --export will do that when implemented. #145

@CMCDragonkai
Copy link
Member

Oh cool - but I think it would be better to use unix, cmd and powershell. Dotenv and dotps and dotcmd aren't really standard. Also I'm not sure prepend is necessary. People can just use pk env cmd. We would want to direct them to the right way to do things.

@tegefaulkes
Copy link
Contributor Author

I had named them for the files you're put them in by standard. For unix it's the .env which is basically formatted like a bash script. For cmd it's a .bat script and for powershell it's a .ps script. But I do agree the naming isn't very good. So maybe unix, cmd and powershell is better.

For the prepend and prependcmd, I think yeah, we can just cut that out. Actually using it is clunky and yeah, something we should steer away from.

@CMCDragonkai
Copy link
Member

I mean the "dotfiles" is not a standard... at least for CMD and powershell. The unix, powershell, cmd is going to be alot more obvious.

I still think human which is the default format should be using platform detection, it's all about UX here.

And as long as the variables are locally defined by default, all 3 should be perfectly readable (maybe not CMD, but that's their fault for using CMD).

@tegefaulkes
Copy link
Contributor Author

I still think human which is the default format should be using platform detection, it's all about UX here.

I'm defaulting to platform which is doing the platform detection. So that's functionality the same as you're describing. I can just rename it to human but I think human should be formatted better to be more human readable and not switch based on platform.

@CMCDragonkai
Copy link
Member

Can we use auto instead? I think defaulting --env-format is auto is better! And human does not make sense in this context, as the output of pk secrets env is not intended for human consumption anyway.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Development

Successfully merging a pull request may close this issue.

2 participants