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

Polykey-Docs:polykey secrets env command doesn't align with Development Environment Secrets how-to-guide #176

Closed
tegefaulkes opened this issue Apr 24, 2024 · 6 comments · Fixed by #212
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 Apr 24, 2024

Specification

I ran into a slight problem when testing the polykey secrets env command when executing an AWS CLI command to check credentials in a demonstration. In this case env variables are added to a vault as separate files. When creating these files it's very common for editors to add a newline the end of the file automatically. https://stackoverflow.com/questions/16222530/why-do-i-need-vim-in-binary-mode-for-noeol-to-work.

This means the newline is added to the env variable when executing a command or printing out in a certain format. This is not the intended way to format the env variable and in the case of my demonstration actually caused an error. To solve this we need to trim the end of file newline from the variables when using them.

Superficially it's a very simple fix. But in some cases trailing whitespace may be intended or at the very least should be allowed. So applying trimEnd() string method might break in some cases. So we need to explore use cases and see what the best solution is.

So far I fixed it in a monkey patch with the following code

                const { exec } = await import('@matrixai/exec');
                for (let envpKey in envp) {
                  envp[envpKey] = envp[envpKey].trim();
                }
                exec.execvp(cmd, argv, envp);

The old spec is incorrect. Running unix env produces a newline at the end. All files must have a EOL as their EOF character. This is according to unix convention.

Instead the problem is that we only have 1 command that puts a secret into vaults as of now. That is polykey secrets create ....

This is lacking alot of nuance, and we want to change this with #32.

Instead usability issues were discovered during a recent demo with @CryptoTotalWar in #176 (comment).

Refer to #176 (comment) and #176 (comment) and https://polykey.com/docs/how-to-guides/developers/development-environment-secrets for spec on how this needs to work.

Tasks

  1. Flesh out spec according to https://polykey.com/docs/how-to-guides/developers/development-environment-secrets.
  2. Resolve ambiguities by creating a manual parser for all variadic arguments
  3. Ensure conformance similar to the unix env command
@tegefaulkes tegefaulkes added the development Standard development label Apr 24, 2024
Copy link

linear bot commented Apr 24, 2024

@CMCDragonkai
Copy link
Member

Several issues with the secrets env command that was discovered during a recent demo review with @CryptoTotalWar in relation to a blog post and also https://polykey.com/docs/how-to-guides/developers/development-environment-secrets.

@CryptoTotalWar can write his own comments here too.

  1. For unix systems, outputting the result to stdout should be using ' single quotes, not " double quotes.
  2. You should not bother with comments like # my-software-project:AWS_SECRET_ACCESS_KEY. We don't really want to show this information.
  3. The EOL at the end of showing secrets env IS CORRECT. It is not a bug. That's how env regularly works. You should have tests that ensure no regression on this.
  4. The problem is that secrets create is the only command to put secrets into a vault as of now. And we need to adopt the other Unix CLI commands to allow more easier ways to put secrets into the vaults see: Incorporate Unix Commands to Secrets Subcommand - pk secrets * #32.
  5. The help text IS TOO LONG when you do polykey secrets --help. It should be a single line in that scenario.
  6. The usage of --env to specify the secrets you want to put into the vault is not the requirement. See the original design here: https://polykey.com/docs/how-to-guides/developers/development-environment-secrets.
  7. The command and arguments help is wrong it shows up as when running `polykey secrets env --help:
    cmd] [argv                          command and arguments
    

@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented May 15, 2024

I used the --env option to have a clear separation form selecting secrets and the command you'd want to exec. I suppose it's possible to have the secrets as part of the args. But does commander support multiple varidic arguments? E.G. [...secrets], (...command)? I'll have to look into it.

@CMCDragonkai
Copy link
Member

Another thing is that when running the env subcommand, any arguments afterwards should be considered arguments of the subcommand without any -- needed.

This must work:

polykey secrets env my-software-project:AWS_SECRET_ACCESS_KEY zsh -d -f -i

@CMCDragonkai
Copy link
Member

I used the --env option to have a clear separation form selecting secrets and the command you'd want to exec. I suppose it's possible to have the secrets as part of the args. But does commander support multiple varidic arguments? E.G. [...secrets], (...command)? I'll have to look into it.

No it may not, in which case you take 1 single variadic arguments, and you do a manual parse over it.

In fact thinking about it, that would be the correct way to implement this.

@CMCDragonkai
Copy link
Member

I think the issues I raised above need to be part of the 1.0.0 - if not unix cli commands wise too. Usability is a big pain right now.

@CMCDragonkai CMCDragonkai changed the title Remove trailing newlines from env variables in secrets env command polykey secrets env command doesn't align with Development Environment Secrets how-to-guide May 15, 2024
@CryptoTotalWar CryptoTotalWar self-assigned this May 22, 2024
@CryptoTotalWar CryptoTotalWar changed the title polykey secrets env command doesn't align with Development Environment Secrets how-to-guide Polykey-Docs:polykey secrets env command doesn't align with Development Environment Secrets how-to-guide May 22, 2024
@tegefaulkes tegefaulkes reopened this Jun 28, 2024
@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Aug 15, 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.

3 participants