From 48a72564f6366fa957ff43af229ac01e37c0b837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Gro=C3=9Fmann?= Date: Wed, 14 Oct 2026 11:51:51 +0200 Subject: [PATCH] fix(pass): reject empty secrets before authorizing --force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store now returns store.ErrEmptySecret from Save and Upsert on every platform (#694). With --force, set asked the engine to authorize the overwrite before the store could reject the value, so the user approved an overwrite that then failed. Check the value right after reading it instead, before any engine round-trip. Move the value switch into a readSecret helper to keep SetCommand under the gocyclo limit. Signed-off-by: Johannes Großmann --- plugins/pass/commands/command_test.go | 21 +++++++++++++++++ plugins/pass/commands/set.go | 34 ++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index 66ebfafb..f384c8ca 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -93,6 +93,17 @@ func Test_SetCommand(t *testing.T) { _, err := execute(t, mustSetCommand(t), mock, "foo=bar", "--metadata", "invalid") assert.ErrorContains(t, err, "invalid metadata pair (expected key=value): invalid") }) + t.Run("rejects an empty inline value", func(t *testing.T) { + mock := teststore.NewMockStore(teststore.WithStoreSaveErr(errors.New("save should not be called for an empty value"))) + out, err := execute(t, mustSetCommand(t), mock, "foo=") + assert.ErrorIs(t, err, store.ErrEmptySecret) + assert.Equal(t, "Error: "+store.ErrEmptySecret.Error()+"\n", out) + }) + t.Run("rejects empty STDIN", func(t *testing.T) { + mock := teststore.NewMockStore(teststore.WithStoreSaveErr(errors.New("save should not be called for an empty value"))) + _, err := executeWithStdin(t, mustSetCommand(t), mock, "", "foo") + assert.ErrorIs(t, err, store.ErrEmptySecret) + }) t.Run("store error", func(t *testing.T) { errSave := errors.New("save error") mock := teststore.NewMockStore(teststore.WithStoreSaveErr(errSave)) @@ -170,6 +181,16 @@ func Test_SetCommand(t *testing.T) { assert.ErrorContains(t, err, "invalid metadata pair (expected key=value): invalid") assert.NotErrorIs(t, err, client.ErrSecretsEngineNotRunning) }) + t.Run("--force rejects an empty value before asking the engine", func(t *testing.T) { + mock := teststore.NewMockStore(teststore.WithStore(map[store.ID]store.Secret{ + store.MustParseID("foo"): pass.NewPassValue([]byte("old")), + })) + cmd := mustSetCommand(t, WithTimeout(time.Second), WithSocketPath(deadSocket(t))) + _, err := execute(t, cmd, mock, "foo=", "--force") + assert.ErrorIs(t, err, store.ErrEmptySecret) + assert.NotErrorIs(t, err, client.ErrSecretsEngineNotRunning) + assertStoredValue(t, mock, "old") + }) t.Run("--force surfaces upsert error", func(t *testing.T) { errUpsert := errors.New("upsert error") mock := teststore.NewMockStore(teststore.WithStoreUpsertErr(errUpsert)) diff --git a/plugins/pass/commands/set.go b/plugins/pass/commands/set.go index 753eaa0a..b7586852 100644 --- a/plugins/pass/commands/set.go +++ b/plugins/pass/commands/set.go @@ -71,17 +71,7 @@ func SetCommand(options ...ClientOption) (*cobra.Command, error) { return err } - var s *setPayload - switch in, isFile := prompt.UnwrapFile(cmd.InOrStdin()); { - case isNotImplicitReadFromStdinSyntax(args): - s = &setPayload{val: value} - case isFile && term.IsTerminal(in.Fd()): - var val string - val, err = prompt.ReadMasked(cmd.Context(), in, cmd.ErrOrStderr(), id) - s = &setPayload{val: val} - default: - s, err = secretMappingFromSTDIN(cmd.Context(), cmd.InOrStdin()) - } + s, err := readSecret(cmd, args, value, id) if err != nil { return err } @@ -131,6 +121,28 @@ func parseMetadataFlags(raw []string) (map[string]string, error) { return m, nil } +func readSecret(cmd *cobra.Command, args []string, value string, id secrets.ID) (*setPayload, error) { + var s *setPayload + var err error + switch in, isFile := prompt.UnwrapFile(cmd.InOrStdin()); { + case isNotImplicitReadFromStdinSyntax(args): + s = &setPayload{val: value} + case isFile && term.IsTerminal(in.Fd()): + var val string + val, err = prompt.ReadMasked(cmd.Context(), in, cmd.ErrOrStderr(), id) + s = &setPayload{val: val} + default: + s, err = secretMappingFromSTDIN(cmd.Context(), cmd.InOrStdin()) + } + if err != nil { + return nil, err + } + if s.val == "" { + return nil, secrets.ErrEmptySecret + } + return s, nil +} + func isNotImplicitReadFromStdinSyntax(args []string) bool { return strings.Contains(args[0], "=") || len(args) > 1 }