diff --git a/plugins/pass/command.go b/plugins/pass/command.go index 2521555d..e9c33180 100644 --- a/plugins/pass/command.go +++ b/plugins/pass/command.go @@ -34,6 +34,12 @@ import ( "github.com/docker/secrets-engine/store" ) +// StoreFactory opens the backing keychain store on demand. Subcommands invoke +// it from their RunE so that commands which never touch the store (version, +// help, completion) do not pay the keychain-init cost — and do not hang in +// headless environments where no D-Bus session bus is reachable. +type StoreFactory func() (store.Store, error) + // Note: We use a custom help template to make it more brief. const helpTemplate = `Docker Pass CLI - Manage your local secrets. {{if .UseLine}} @@ -56,8 +62,13 @@ var rootExample string //go:embed long.md var rootLong string -// Root returns the root command for the docker-pass CLI plugin -func Root(ctx context.Context, s store.Store, info commands.VersionInfo) *cobra.Command { +// Root returns the root command for the docker-pass CLI plugin. +// +// newStore is invoked lazily by subcommands that need keychain access. Commands +// that do not touch the store (version, help, completion) never call it, so the +// binary stays usable in headless environments where opening the keychain would +// hang on a missing D-Bus session bus. +func Root(ctx context.Context, newStore StoreFactory, info commands.VersionInfo) *cobra.Command { cmd := &cobra.Command{ Use: "pass set|get|ls|rm|run", Short: "Manage your local OS keychain secrets.", @@ -80,10 +91,10 @@ func Root(ctx context.Context, s store.Store, info commands.VersionInfo) *cobra. return []string{"--help"}, cobra.ShellCompDirectiveNoFileComp }) - cmd.AddCommand(wrapRunEWithSpan(commands.SetCommand(s))) - cmd.AddCommand(wrapRunEWithSpan(commands.ListCommand(s))) - cmd.AddCommand(wrapRunEWithSpan(commands.RmCommand(s))) - cmd.AddCommand(wrapRunEWithSpan(commands.GetCommand(s))) + cmd.AddCommand(wrapRunEWithSpan(commands.SetCommand(newStore))) + cmd.AddCommand(wrapRunEWithSpan(commands.ListCommand(newStore))) + cmd.AddCommand(wrapRunEWithSpan(commands.RmCommand(newStore))) + cmd.AddCommand(wrapRunEWithSpan(commands.GetCommand(newStore))) cmd.AddCommand(wrapRunEWithSpan(commands.RunCommand())) cmd.AddCommand(commands.VersionCommand(info)) diff --git a/plugins/pass/command_test.go b/plugins/pass/command_test.go index a29592e2..dc09e7c8 100644 --- a/plugins/pass/command_test.go +++ b/plugins/pass/command_test.go @@ -38,18 +38,22 @@ var mockInfo = commands.VersionInfo{ Commit: "abc", } +func staticFactory(s store.Store) StoreFactory { + return func() (store.Store, error) { return s, nil } +} + func Test_rootCommand(t *testing.T) { t.Parallel() t.Run("version", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "version") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "version") assert.NoError(t, err) assert.Equal(t, "Version: v88\nCommit: abc\n", out) }) t.Run("set", func(t *testing.T) { t.Run("ok", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=bar=bar=bar") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=bar=bar=bar") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -62,7 +66,7 @@ func Test_rootCommand(t *testing.T) { }) t.Run("from STDIN", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommandWithStdin(Root(t.Context(), mock, mockInfo), "my\nmultiline\nvalue", "set", "foo") + out, err := executeCommandWithStdin(Root(t.Context(), staticFactory(mock), mockInfo), "my\nmultiline\nvalue", "set", "foo") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -75,7 +79,7 @@ func Test_rootCommand(t *testing.T) { }) t.Run("with --metadata flag", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=bar", "--metadata", "name=bob", "--metadata", "expiry=2027-03-01") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=bar", "--metadata", "name=bob", "--metadata", "expiry=2027-03-01") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -89,7 +93,7 @@ func Test_rootCommand(t *testing.T) { }) t.Run("from STDIN JSON with value and metadata", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommandWithStdin(Root(t.Context(), mock, mockInfo), `{"secret":"bar","metadata":{"name":"bob"}}`, "set", "foo") + out, err := executeCommandWithStdin(Root(t.Context(), staticFactory(mock), mockInfo), `{"secret":"bar","metadata":{"name":"bob"}}`, "set", "foo") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -103,7 +107,7 @@ func Test_rootCommand(t *testing.T) { }) t.Run("from STDIN JSON merged with --metadata flag wins on collision", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommandWithStdin(Root(t.Context(), mock, mockInfo), `{"secret":"bar","metadata":{"name":"bob","extra":"thing"}}`, "set", "foo", "--metadata", "name=alice") + out, err := executeCommandWithStdin(Root(t.Context(), staticFactory(mock), mockInfo), `{"secret":"bar","metadata":{"name":"bob","extra":"thing"}}`, "set", "foo", "--metadata", "name=alice") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -117,20 +121,20 @@ func Test_rootCommand(t *testing.T) { }) t.Run("invalid --metadata flag (no =)", func(t *testing.T) { mock := teststore.NewMockStore() - _, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=bar", "--metadata", "invalid") + _, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=bar", "--metadata", "invalid") assert.ErrorContains(t, err, "invalid metadata pair (expected key=value): invalid") }) t.Run("store error", func(t *testing.T) { errSave := errors.New("save error") mock := teststore.NewMockStore(teststore.WithStoreSaveErr(errSave)) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "foo=bar") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=bar") assert.ErrorIs(t, errSave, err) assert.Equal(t, "Error: "+errSave.Error()+"\n", out) }) t.Run("invalid id", func(t *testing.T) { errSave := errors.New("save error") mock := teststore.NewMockStore(teststore.WithStoreSaveErr(errSave)) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "set", "/foo=bar") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "/foo=bar") errInvalidID := secrets.ErrInvalidID{ID: "/foo"} assert.ErrorIs(t, err, errInvalidID) assert.Equal(t, "Error: "+errInvalidID.Error()+"\n", out) @@ -144,7 +148,7 @@ func Test_rootCommand(t *testing.T) { }), 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") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=new", "--force") assert.NoError(t, err) assert.Empty(t, out) s, err := mock.Get(t.Context(), secrets.MustParseID("foo")) @@ -158,7 +162,7 @@ func Test_rootCommand(t *testing.T) { 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") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "set", "foo=bar", "--force") assert.ErrorIs(t, errUpsert, err) assert.Equal(t, "Error: "+errUpsert.Error()+"\n", out) }) @@ -169,14 +173,14 @@ func Test_rootCommand(t *testing.T) { store.MustParseID("foo"): pass.NewPassValue([]byte("bar")), store.MustParseID("baz"): pass.NewPassValue([]byte("0")), })) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "list") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "list") assert.NoError(t, err) assert.Equal(t, "baz\nfoo\n", out) }) t.Run("store error", func(t *testing.T) { errGetAll := errors.New("get error") mock := teststore.NewMockStore(teststore.WithStoreGetAllErr(errGetAll)) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "list") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "list") assert.ErrorIs(t, errGetAll, err) assert.Equal(t, "Error: "+errGetAll.Error()+"\n", out) }) @@ -187,7 +191,7 @@ func Test_rootCommand(t *testing.T) { store.MustParseID("foo"): pass.NewPassValue([]byte("bar")), store.MustParseID("baz"): pass.NewPassValue([]byte("0")), })) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm", "foo", "baz") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm", "foo", "baz") assert.NoError(t, err) assert.Equal(t, "RM: baz\nRM: foo\n", out) l, err := mock.GetAllMetadata(t.Context()) @@ -199,7 +203,7 @@ func Test_rootCommand(t *testing.T) { store.MustParseID("foo"): pass.NewPassValue([]byte("bar")), store.MustParseID("baz"): pass.NewPassValue([]byte("0")), })) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm", "--all") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm", "--all") assert.NoError(t, err) assert.Equal(t, "RM: baz\nRM: foo\n", out) l, err := mock.GetAllMetadata(t.Context()) @@ -209,26 +213,26 @@ func Test_rootCommand(t *testing.T) { t.Run("store error", func(t *testing.T) { errRemove := errors.New("remove error") mock := teststore.NewMockStore(teststore.WithStoreDeleteErr(errRemove)) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm", "foo") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm", "foo") assert.ErrorIs(t, err, errRemove) assert.Equal(t, "ERR: foo: remove error\nError: "+errRemove.Error()+"\n", out) }) t.Run("invalid id", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm", "/foo") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm", "/foo") errInvalidID := secrets.ErrInvalidID{ID: "/foo"} assert.ErrorIs(t, err, errInvalidID) assert.Equal(t, "Error: "+errInvalidID.Error()+"\n", out) }) t.Run("cannot mix --all with explicit list", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm", "--all", "foo") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm", "--all", "foo") assert.ErrorContains(t, err, "either provide a secret name or use --all to remove all secrets") assert.Equal(t, "Error: either provide a secret name or use --all to remove all secrets\n", out) }) t.Run("no args or --all", func(t *testing.T) { mock := teststore.NewMockStore() - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "rm") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "rm") assert.ErrorContains(t, err, "either provide a secret name or use --all to remove all secrets") assert.Equal(t, "Error: either provide a secret name or use --all to remove all secrets\n", out) }) @@ -238,14 +242,14 @@ func Test_rootCommand(t *testing.T) { mock := teststore.NewMockStore(teststore.WithStore(map[store.ID]store.Secret{ store.MustParseID("foo"): pass.NewPassValue([]byte("bar")), })) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "get", "foo") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "get", "foo") assert.NoError(t, err) assert.Equal(t, "ID: foo\nValue: **********\n", out) }) t.Run("store error", func(t *testing.T) { errGet := errors.New("get error") mock := teststore.NewMockStore(teststore.WithStoreGetErr(errGet)) - out, err := executeCommand(Root(t.Context(), mock, mockInfo), "get", "foo") + out, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), "get", "foo") assert.ErrorIs(t, err, errGet) assert.Equal(t, "Error: "+errGet.Error()+"\n", out) }) @@ -280,7 +284,7 @@ func Test_rootCommandTelemetry(t *testing.T) { mock := teststore.NewMockStore(teststore.WithStore(map[store.ID]store.Secret{ store.MustParseID("baz"): pass.NewPassValue([]byte("bar")), })) - _, err := executeCommand(Root(t.Context(), mock, mockInfo), tc.args...) + _, err := executeCommand(Root(t.Context(), staticFactory(mock), mockInfo), tc.args...) assert.NoError(t, err) var rm metricdata.ResourceMetrics diff --git a/plugins/pass/commands/get.go b/plugins/pass/commands/get.go index 6ae1bac4..bb780694 100644 --- a/plugins/pass/commands/get.go +++ b/plugins/pass/commands/get.go @@ -23,7 +23,7 @@ import ( "github.com/docker/secrets-engine/store" ) -func GetCommand(kc store.Store) *cobra.Command { +func GetCommand(newStore func() (store.Store, error)) *cobra.Command { cmd := &cobra.Command{ Use: "get NAME", Args: cobra.ExactArgs(1), @@ -34,6 +34,10 @@ func GetCommand(kc store.Store) *cobra.Command { if err != nil { return err } + kc, err := newStore() + if err != nil { + return err + } s, err := kc.Get(cmd.Context(), id) if err != nil { return err diff --git a/plugins/pass/commands/list.go b/plugins/pass/commands/list.go index a50922fe..38befc60 100644 --- a/plugins/pass/commands/list.go +++ b/plugins/pass/commands/list.go @@ -22,7 +22,7 @@ import ( "github.com/docker/secrets-engine/store" ) -func ListCommand(kc store.Store) *cobra.Command { +func ListCommand(newStore func() (store.Store, error)) *cobra.Command { cmd := &cobra.Command{ Use: "ls", Aliases: []string{"list"}, @@ -30,6 +30,10 @@ func ListCommand(kc store.Store) *cobra.Command { Long: "Lists the names of all secrets stored in the local OS keychain.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + kc, err := newStore() + if err != nil { + return err + } l, err := kc.GetAllMetadata(cmd.Context()) if err != nil { return err diff --git a/plugins/pass/commands/rm.go b/plugins/pass/commands/rm.go index f4ecdcc1..778fe330 100644 --- a/plugins/pass/commands/rm.go +++ b/plugins/pass/commands/rm.go @@ -35,7 +35,7 @@ type rmOpts struct { All bool } -func RmCommand(kc store.Store) *cobra.Command { +func RmCommand(newStore func() (store.Store, error)) *cobra.Command { opts := rmOpts{} cmd := &cobra.Command{ Use: "rm name1 name2 ...", @@ -48,6 +48,10 @@ func RmCommand(kc store.Store) *cobra.Command { if err != nil { return err } + kc, err := newStore() + if err != nil { + return err + } return runRm(cmd.Context(), cmd.OutOrStdout(), kc, idList, opts) }, } diff --git a/plugins/pass/commands/set.go b/plugins/pass/commands/set.go index fc2b0820..8b7fc380 100644 --- a/plugins/pass/commands/set.go +++ b/plugins/pass/commands/set.go @@ -46,7 +46,7 @@ type stdinPayload struct { Metadata map[string]string `json:"metadata,omitempty"` } -func SetCommand(kc store.Store) *cobra.Command { +func SetCommand(newStore func() (store.Store, error)) *cobra.Command { opts := setOpts{} cmd := &cobra.Command{ Use: "set id[=value]", @@ -98,6 +98,11 @@ func SetCommand(kc store.Store) *cobra.Command { return err } } + + kc, err := newStore() + if err != nil { + return err + } if opts.force { return kc.Upsert(cmd.Context(), id, pv) }