Skip to content

Conversation

@aryanjassal
Copy link
Contributor

@aryanjassal aryanjassal commented Nov 21, 2024

Description

Previously, we needed to use two -- to separate commands from arguments. Now, that is no longer needed, and we can use a single -- to parse arguments.

polykey secrets env vault:SECRET -- cmd arg1 arg2

Note that you still can't do this, as it would be parsed by the shell instead of the actual command.

$ polykey secrets env vault:SECRET -- cmd arg1 arg2 && cmd2
'cmd' run by polykey
'cmd2' run by shell

Issues Fixed

Tasks

  • 1. Move parsing to action and emulate what it does
  • 2. Remove old parser
  • 3. Update tests

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@aryanjassal aryanjassal self-assigned this Nov 21, 2024
@linear
Copy link

linear bot commented Nov 21, 2024

@aryanjassal aryanjassal marked this pull request as ready for review November 21, 2024 00:59
@aryanjassal
Copy link
Contributor Author

aryanjassal commented Nov 21, 2024

I ran a couple of manual sanity checks to check this behaviour.

$ polykey secrets env testvault:SECRET1 -- bash -c 'echo $SECRET1'
testing

$ polykey secrets env testvault --preserve-newline testvault -- bash -c echo "$SECRET1"
testing


$ polykey secrets env testvault -- node -e 'console.log(JSON.stringify(process.env["SECRET1"]))'
"testing"

$ polykey secrets env testvault -- -- echo 'hi'
ErrorPolykeyCLIChildProcessFailure: A child process failed to exit gracefully - Command failed with error Error: 2
  data: {"command":["--","echo","hi"]}
  cause: Error: 2

$ polykey secrets env -- echo 'hi'
You must provide at least 1 secret path
Usage: polykey secrets env [options] <args...>

Run a command with the given secrets and env variables. If no command is specified then the variables are printed to stdout in the format specified by env-format.

Arguments:
  args                                command and arguments formatted as <envPaths...> [-- cmd [cmdArgs...]]

Options:
  -np, --node-path <path>             Path to Node State (default: "/home/aryanj/.local/share/polykey", env: PK_NODE_PATH)
  -pf, --password-file <path>         Path to Password
  -f, --format <format>               Output Format (choices: "human", "json", default: "human")
  -v, --verbose                       Log Verbose Messages (default: 0)
  -ni, --node-id <id>                  (env: PK_NODE_ID)
  -ch, --client-host <host>           Client Host Address (env: PK_CLIENT_HOST)
  -cp, --client-port <port>           Client Port (env: PK_CLIENT_PORT)
  -ef --env-format <envFormat>        Select how the env variables are formatted on stdout if no command is specified (choices: "auto", "json", "unix", "cmd", "powershell", default: "auto")
  -ei --env-invalid <envInvalid>      How invalid env variable names are handled when retrieving secrets. `error` will throw, `warn` will log a warning and drop and `ignore` will silently drop. (choices: "error", "warn", "ignore", default: "error")
  -ed --env-duplicate <envDuplicate>  How duplicate env variable names are handled. `keep` will keep the exising secret, `overwrite` will overwrite existing with the new secret, `warn` will log a warning and overwrite and `error` will throw. (choices: "keep", "overwrite", "warn", "error", default: "overwrite")
  --preserve-newline <path>           Preserve the last trailing newline for the secret content (default: [])
  -h, --help                          display help for command

So, I think this is ready for merge now.

fix: updated tests

chore: removed redundant tests
@aryanjassal aryanjassal force-pushed the feature-env-command-parsing branch from eef0cae to 58f4752 Compare November 21, 2024 02:55
@aryanjassal
Copy link
Contributor Author

Everything seems to be done for this PR. The CI is also passing. Merging.

@aryanjassal aryanjassal merged commit b1e53da into staging Nov 21, 2024
22 checks passed
@CMCDragonkai
Copy link
Member

This will require some UX testing. @brynblack can you start integrating this.

@CMCDragonkai
Copy link
Member

$ polykey secrets env vault:SECRET -- cmd arg1 arg2 && cmd2

This is generally correct, since the subprogram isn't a shell. People who want to do this usually have to make cmd the shell and the quote the script they want to run, but that's less common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

secrets env needs -- to be passed twice to parse the command

4 participants