-
Notifications
You must be signed in to change notification settings - Fork 6
feat(pass): add --os-keychain flag to pass run
#532
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import ( | |
| "github.com/spf13/cobra" | ||
|
|
||
| "github.com/docker/secrets-engine/client" | ||
| "github.com/docker/secrets-engine/store" | ||
| "github.com/docker/secrets-engine/x/api" | ||
| "github.com/docker/secrets-engine/x/secrets" | ||
| ) | ||
|
|
@@ -52,24 +53,20 @@ func (e *ExitCodeError) Error() string { | |
| //go:embed run_example.md | ||
| var runExample string | ||
|
|
||
| //go:embed run_long.md | ||
| var runLong string | ||
|
|
||
| type runOpts struct { | ||
| envFiles []string | ||
| envFiles []string | ||
| osKeychain bool | ||
| } | ||
|
|
||
| func RunCommand() *cobra.Command { | ||
| func RunCommand(s store.Store) *cobra.Command { | ||
| opts := runOpts{} | ||
| cmd := &cobra.Command{ | ||
| Use: "run -- CMD [ARGS...]", | ||
| Short: "Run a command with `se://` environment references resolved.", | ||
| Long: "Scans the current environment (plus any `--env-file` inputs) for variables\n" + | ||
| "whose value is exactly `se://<ID|pattern>`. Each reference is resolved through the\n" + | ||
| "secrets-engine daemon and the resolved value is passed to the child process.\n" + | ||
| "The child inherits stdin, stdout, and stderr.\n" + | ||
| "\n" + | ||
| "Requires the secrets-engine daemon (Docker Desktop) to be running.\n" + | ||
| "\n" + | ||
| "If any reference cannot be resolved, the command fails before the child is\n" + | ||
| "started and exits non-zero.", | ||
| Use: "run -- CMD [ARGS...]", | ||
| Short: "Run a command with `se://` environment references resolved.", | ||
| Long: strings.Trim(runLong, "\n"), | ||
| Example: strings.Trim(runExample, "\n"), | ||
| Args: cobra.MinimumNArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
|
|
@@ -78,12 +75,18 @@ func RunCommand() *cobra.Command { | |
| return err | ||
| } | ||
|
|
||
| c, err := client.New(client.WithSocketPath(api.DefaultSocketPath())) | ||
| if err != nil { | ||
| return err | ||
| var r secrets.Resolver | ||
| if opts.osKeychain { | ||
| r = &storeResolver{s: s} | ||
| } else { | ||
| c, err := client.New(client.WithSocketPath(api.DefaultSocketPath())) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r = c | ||
| } | ||
|
|
||
| env, err := resolveEnv(cmd.Context(), c, merged) | ||
| env, err := resolveEnv(cmd.Context(), r, merged) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -139,9 +142,35 @@ func RunCommand() *cobra.Command { | |
| } | ||
| cmd.Flags().StringArrayVar(&opts.envFiles, "env-file", nil, | ||
| "Read environment variables from a dotenv-formatted file. Repeatable; later files override earlier files and the process environment.") | ||
| cmd.Flags().BoolVar(&opts.osKeychain, "os-keychain", false, | ||
| "Resolve `se://` references directly from the local OS keychain instead of the secrets-engine daemon.") | ||
| return cmd | ||
| } | ||
|
|
||
| type storeResolver struct { | ||
| s store.Store | ||
| } | ||
|
|
||
| func (r *storeResolver) GetSecrets(ctx context.Context, p secrets.Pattern) ([]secrets.Envelope, error) { | ||
| m, err := r.s.Filter(ctx, p) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] In Both paths ultimately trigger Consider normalising in if errors.Is(err, store.ErrCredentialNotFound) {
return nil, nil // let the caller's empty-slice ErrNotFound path handle it
} |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| out := make([]secrets.Envelope, 0, len(m)) | ||
| for id, sec := range m { | ||
| val, err := sec.Marshal() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| out = append(out, secrets.Envelope{ | ||
| ID: id, | ||
| Value: val, | ||
| Metadata: sec.Metadata(), | ||
| }) | ||
| } | ||
| return out, nil | ||
| } | ||
|
|
||
| // mergeEnv folds the process environment and any --env-file inputs into a | ||
| // single deterministic KEY=VALUE slice. Precedence: process env first, then | ||
| // each file in order; later entries override earlier ones. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| Scans the current environment (plus any `--env-file` inputs) for variables | ||
| whose value is exactly `se://<ID|pattern>` and resolves each reference before | ||
| launching the child process. The child inherits stdin, stdout, and stderr. | ||
|
|
||
| By default, references are resolved through the secrets-engine daemon (Docker | ||
| Desktop must be running). Pass `--os-keychain` to resolve directly from the | ||
| local OS keychain instead, with no daemon required. | ||
|
|
||
| If any reference cannot be resolved, the command fails before the child is | ||
| started and exits non-zero. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Missing nil guard:
--os-keychainwith nil store panics at runtimeRunCommandnow accepts astore.Storeparameter. When--os-keychainis set,&storeResolver{s: s}is constructed andr.s.Filter(ctx, p)is immediately called (line 155). Ifsisnil, this is a nil-interface dereference → runtime panic.In the normal production path (
Root() → RunCommand(s)),sis always a real store fromNewPassPlugin, so this is not reachable there. However, the tests already callRunCommand(nil)— if any future test adds--os-keychainit will panic rather than fail gracefully. A defensive early return would harden this:Consider adding this guard at line 79, before constructing
storeResolver.