Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions plugins/pass/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,33 @@ func Test_rootCommand(t *testing.T) {
assert.ErrorIs(t, err, errInvalidID)
assert.Equal(t, "Error: "+errInvalidID.Error()+"\n", out)
})
t.Run("--force overwrites existing secret", func(t *testing.T) {
// Make Save return an error so the test fails if --force does not
// route the call through Upsert.
mock := teststore.NewMockStore(
teststore.WithStore(map[store.ID]store.Secret{
store.MustParseID("foo"): pass.NewPassValue([]byte("old")),
}),
teststore.WithStoreSaveErr(errors.New("save should not be called when --force is set")),
)
out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=new", "--force")
assert.NoError(t, err)
assert.Empty(t, out)
s, err := mock.Get(t.Context(), secrets.MustParseID("foo"))
require.NoError(t, err)
impl, ok := s.(*pass.PassValue)
require.True(t, ok)
v, err := impl.Marshal()
require.NoError(t, err)
assert.Equal(t, "new", string(v))
})
t.Run("--force surfaces upsert error", func(t *testing.T) {
errUpsert := errors.New("upsert error")
mock := teststore.NewMockStore(teststore.WithStoreUpsertErr(errUpsert))
out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=bar", "--force")
assert.ErrorIs(t, errUpsert, err)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — Swapped assert.ErrorIs arguments in '--force surfaces upsert error' test

assert.ErrorIs(t, errUpsert, err) has its arguments reversed. The testify signature is assert.ErrorIs(t, actual, target), mirroring errors.Is(actual, target). As written, the test checks whether the sentinel errUpsert wraps the command's returned error — not the other way around.

The test passes today because cobra's RunE forwards the error without wrapping, making errors.Is(errUpsert, errUpsert) trivially true. If any future middleware wraps the error (e.g. fmt.Errorf("set: %w", errUpsert)), this assertion becomes a silent false-negative rather than a test failure.

Fix: reverse the arguments:

assert.ErrorIs(t, err, errUpsert)

This is consistent with the existing pattern used in the pre-existing "get" > "store error" and "rm" > "store error" subtests.

assert.Equal(t, "Error: "+errUpsert.Error()+"\n", out)
})
})
t.Run("list", func(t *testing.T) {
t.Run("ok", func(t *testing.T) {
Expand Down
20 changes: 18 additions & 2 deletions plugins/pass/commands/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,14 @@ docker pass set POSTGRES_PASSWORD=my-secret-password --metadata owner=alice --me

### Or pass a JSON payload with secret and metadata via STDIN:
echo '{"secret":"my-secret-password","metadata":{"owner":"alice"}}' | docker pass set POSTGRES_PASSWORD

### Overwrite an existing secret:
docker pass set POSTGRES_PASSWORD=new-secret-password --force
`

type setOpts struct {
metadata []string // raw "key=value" strings from --metadata flag
force bool // if true, overwrite existing secret instead of erroring
}

type stdinPayload struct {
Expand All @@ -59,8 +63,16 @@ func SetCommand(kc store.Store) *cobra.Command {
Use: "set id[=value]",
Aliases: []string{"store", "save"},
Short: "Set a secret",
Long: `Stores a secret in the local OS keychain. The secret value can be
provided inline (NAME=VALUE) or piped via STDIN.`,
Long: `Stores a secret in the local OS keychain. The secret value can be provided inline (NAME=VALUE) or piped via STDIN.

Behavior when a secret with the same id already exists is platform-dependent:
- macOS (Keychain): the command fails with a duplicate-item error.
- Linux (Secret Service) and Windows (Credential Manager): the existing
value is silently overwritten.

Pass --force to overwrite an existing secret. On Linux and Windows the
replacement is performed atomically. On macOS the Keychain API requires
a delete-then-add sequence.`,
Example: strings.Trim(setExample, "\n"),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -106,11 +118,15 @@ provided inline (NAME=VALUE) or piped via STDIN.`,
return err
}
}
if opts.force {
return kc.Upsert(cmd.Context(), id, pv)
}
return kc.Save(cmd.Context(), id, pv)
},
}
flags := cmd.Flags()
flags.StringArrayVar(&opts.metadata, "metadata", nil, "Non-sensitive key=value metadata (repeatable)")
flags.BoolVarP(&opts.force, "force", "f", false, "Overwrite existing secret if it already exists")
return cmd
}

Expand Down
Loading