From 4d604e3f223ea009160e8a82d44d8e800ff0feb8 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:29:37 +0200 Subject: [PATCH 1/5] feat(pass): hint how to fix a failed connection to the secrets engine get --reveal, rm and run now append a hint to a client.ConnectError, based on its reason: start the engine when it is not running, restart it when it does not respond, and check access to the socket when permission is denied. The not-running and timeout hints name Docker Desktop only when the socket is Docker Desktop's; for any other socket, such as a standalone engine, they name the socket instead. Unknown reasons and other errors pass through unchanged, as with the keychain hints. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- plugins/pass/commands/client.go | 53 +++++++++++++++++++++++++++ plugins/pass/commands/command_test.go | 45 ++++++++++++++++++++++- plugins/pass/commands/get.go | 2 +- plugins/pass/commands/rm.go | 2 +- plugins/pass/commands/run.go | 2 +- 5 files changed, 99 insertions(+), 5 deletions(-) diff --git a/plugins/pass/commands/client.go b/plugins/pass/commands/client.go index 4e66918b..c3f12490 100644 --- a/plugins/pass/commands/client.go +++ b/plugins/pass/commands/client.go @@ -18,8 +18,11 @@ import ( "context" "errors" "fmt" + "path/filepath" "time" + "github.com/spf13/cobra" + "github.com/docker/secrets-engine/client" "github.com/docker/secrets-engine/x/api" "github.com/docker/secrets-engine/x/secrets" @@ -27,6 +30,56 @@ import ( const defaultPreflightPingTimeout = 3 * time.Second +const ( + desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." + desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." +) + +func wrapEngineErrors(cmd *cobra.Command) *cobra.Command { + if run := cmd.RunE; run != nil { + cmd.RunE = func(c *cobra.Command, args []string) error { + return withEngineHint(run(c, args)) + } + } + return cmd +} + +func withEngineHint(err error) error { + ce, ok := errors.AsType[*client.ConnectError](err) + if !ok { + return err + } + socket := "the secrets engine socket" + if ce.SocketPath != "" { + socket = ce.SocketPath + } + desktop := isDesktopSocket(ce.SocketPath) + var hint string + switch ce.Reason { + case client.ReasonNotRunning: + hint = "Start the secrets engine and retry: nothing is listening on " + socket + "." + if desktop { + hint = desktopNotRunningHint + } + case client.ReasonTimeout: + hint = "The secrets engine on " + socket + " did not respond. Restart it and retry." + if desktop { + hint = desktopTimeoutHint + } + case client.ReasonPermissionDenied: + hint = "Check that your user may open " + socket + "." + default: + return err + } + return fmt.Errorf("%w\n\n%s", err, hint) +} + +// isDesktopSocket reports whether path is the socket Docker Desktop's +// secrets engine listens on. A standalone engine uses a different socket. +func isDesktopSocket(path string) bool { + return path != "" && filepath.Clean(path) == filepath.Clean(api.DesktopSocketPath()) +} + type clientOpts struct { timeout time.Duration responseTimeout time.Duration diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index d35ee86f..ae14f56a 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -17,6 +17,7 @@ package commands import ( "bytes" "errors" + "fmt" "testing" "time" @@ -29,6 +30,7 @@ import ( "github.com/docker/secrets-engine/plugins/pass/teststore" "github.com/docker/secrets-engine/store" "github.com/docker/secrets-engine/store/keychain" + "github.com/docker/secrets-engine/x/api" "github.com/docker/secrets-engine/x/secrets" "github.com/docker/secrets-engine/x/testhelper" ) @@ -210,9 +212,11 @@ func Test_RmCommand(t *testing.T) { }) t.Run("unreachable engine removes nothing", func(t *testing.T) { mock := twoSecrets() - cmd := mustRmCommand(t, WithTimeout(time.Second), WithSocketPath(deadSocket(t))) + socket := deadSocket(t) + cmd := mustRmCommand(t, WithTimeout(time.Second), WithSocketPath(socket)) out, err := execute(t, cmd, mock, "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) + assert.Contains(t, out, "Start the secrets engine and retry: nothing is listening on "+socket+".") assert.NotContains(t, out, "RM:") assert.Equal(t, 2, remaining(t, mock)) }) @@ -342,10 +346,12 @@ func Test_GetCommand(t *testing.T) { assert.NotContains(t, out, "bar") }) t.Run("--reveal pings the engine first when requests are unbounded", func(t *testing.T) { - cmd := mustGetCommand(t, WithSocketPath(deadSocket(t))) + socket := deadSocket(t) + cmd := mustGetCommand(t, WithSocketPath(socket)) out, err := execute(t, cmd, fooStore(), "--reveal", "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) assert.ErrorContains(t, err, "preflight ping") + assert.Contains(t, out, "\n\nStart the secrets engine and retry: nothing is listening on "+socket+".\n") assert.NotContains(t, out, "bar") }) t.Run("rejects an invalid option", func(t *testing.T) { @@ -369,6 +375,41 @@ func mustRmCommand(t *testing.T, options ...ClientOption) *cobra.Command { return cmd } +func TestWithEngineHint(t *testing.T) { + t.Parallel() + connErr := func(reason client.ConnectReason, socket string) error { + return fmt.Errorf("authorizing: %w", &client.ConnectError{Reason: reason, SocketPath: socket, Err: errors.New("dial")}) + } + desktop := api.DesktopSocketPath() + tests := []struct { + name string + err error + hint string + }{ + {name: "desktop not running", err: connErr(client.ReasonNotRunning, desktop), hint: desktopNotRunningHint}, + {name: "desktop timeout", err: connErr(client.ReasonTimeout, desktop), hint: desktopTimeoutHint}, + {name: "standalone not running", err: connErr(client.ReasonNotRunning, "/s.sock"), hint: "Start the secrets engine and retry: nothing is listening on /s.sock."}, + {name: "standalone timeout", err: connErr(client.ReasonTimeout, "/s.sock"), hint: "The secrets engine on /s.sock did not respond. Restart it and retry."}, + {name: "permission denied", err: connErr(client.ReasonPermissionDenied, "/s.sock"), hint: "Check that your user may open /s.sock."}, + {name: "permission denied on desktop socket", err: connErr(client.ReasonPermissionDenied, desktop), hint: "Check that your user may open " + desktop + "."}, + {name: "permission denied without path", err: connErr(client.ReasonPermissionDenied, ""), hint: "Check that your user may open the secrets engine socket."}, + {name: "not running without path", err: connErr(client.ReasonNotRunning, ""), hint: "Start the secrets engine and retry: nothing is listening on the secrets engine socket."}, + {name: "joined with other errors", err: errors.Join(store.ErrCredentialNotFound, connErr(client.ReasonNotRunning, desktop)), hint: desktopNotRunningHint}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := withEngineHint(tc.err) + assert.Equal(t, tc.err.Error()+"\n\n"+tc.hint, got.Error()) + assert.ErrorIs(t, got, tc.err) + }) + } + t.Run("unknown reason and other errors pass through", func(t *testing.T) { + for _, err := range []error{nil, connErr(client.ReasonUnknown, "/s.sock"), client.ErrAccessDenied} { + assert.Equal(t, err, withEngineHint(err)) + } + }) +} + func deadSocket(t *testing.T) string { t.Helper() return testhelper.RandomShortSocketName() diff --git a/plugins/pass/commands/get.go b/plugins/pass/commands/get.go index 92e80d70..d9edc3cd 100644 --- a/plugins/pass/commands/get.go +++ b/plugins/pass/commands/get.go @@ -79,7 +79,7 @@ func GetCommand(options ...ClientOption) (*cobra.Command, error) { }, } cmd.Flags().BoolVar(&reveal, "reveal", false, "Show the secret value in plaintext") - return wrapKeychainErrors(cmd), nil + return wrapEngineErrors(wrapKeychainErrors(cmd)), nil } func printSecret(w io.Writer, id store.ID, value []byte) error { diff --git a/plugins/pass/commands/rm.go b/plugins/pass/commands/rm.go index fd91119a..af103815 100644 --- a/plugins/pass/commands/rm.go +++ b/plugins/pass/commands/rm.go @@ -65,7 +65,7 @@ func RmCommand(options ...ClientOption) (*cobra.Command, error) { } flags := cmd.Flags() flags.BoolVar(&opts.all, "all", false, "Remove all secrets") - return wrapKeychainErrors(cmd), nil + return wrapEngineErrors(wrapKeychainErrors(cmd)), nil } func validateArgs(args []string, opts rmOpts) ([]store.ID, error) { diff --git a/plugins/pass/commands/run.go b/plugins/pass/commands/run.go index 23fcefda..85474dfc 100644 --- a/plugins/pass/commands/run.go +++ b/plugins/pass/commands/run.go @@ -61,7 +61,7 @@ func RunCommand(options ...ClientOption) (*cobra.Command, error) { if err != nil { return nil, err } - return newRunCommand(runOpts{clientOpts: copts}), nil + return wrapEngineErrors(newRunCommand(runOpts{clientOpts: copts})), nil } func newRunCommand(opts runOpts) *cobra.Command { From 60735231c26c96df701f293b5436ca05470e6a1a Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:07:56 +0200 Subject: [PATCH 2/5] feat(pass): name the standalone engine in connection hints Tell the Docker Desktop, standalone and custom sockets apart: the not-running and timeout hints now name the standalone secrets engine when the socket is api.StandaloneSocketPath(), and keep the generic wording for any other socket. Address review feedback: reword the permission hint to say the user lacks permission to connect and needs read and write access to the socket, quote the socket path so control characters cannot corrupt the output, and wrap PreRunE as well as RunE, as the keychain hints do. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- plugins/pass/commands/client.go | 63 +++++++++++++++++++++------ plugins/pass/commands/command_test.go | 26 ++++++++--- 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/plugins/pass/commands/client.go b/plugins/pass/commands/client.go index c3f12490..c1ed32b0 100644 --- a/plugins/pass/commands/client.go +++ b/plugins/pass/commands/client.go @@ -31,11 +31,29 @@ import ( const defaultPreflightPingTimeout = 3 * time.Second const ( - desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." - desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." + desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." + desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." + standaloneNotRunningHint = "Start the standalone secrets engine and retry: nothing is listening on %q." + standaloneTimeoutHint = "The standalone secrets engine on %q did not respond. Restart it and retry." + customNotRunningHint = "Start the secrets engine and retry: nothing is listening on %s." + customTimeoutHint = "The secrets engine on %s did not respond. Restart it and retry." + permissionDeniedHint = "Your user lacks permission to connect to %s. Grant it read and write access to the socket and retry." +) + +type engineKind int + +const ( + engineCustom engineKind = iota + engineDesktop + engineStandalone ) func wrapEngineErrors(cmd *cobra.Command) *cobra.Command { + if pre := cmd.PreRunE; pre != nil { + cmd.PreRunE = func(c *cobra.Command, args []string) error { + return withEngineHint(pre(c, args)) + } + } if run := cmd.RunE; run != nil { cmd.RunE = func(c *cobra.Command, args []string) error { return withEngineHint(run(c, args)) @@ -49,35 +67,54 @@ func withEngineHint(err error) error { if !ok { return err } + // Quote the path: it is user-controlled and may hold control characters. socket := "the secrets engine socket" if ce.SocketPath != "" { - socket = ce.SocketPath + socket = fmt.Sprintf("%q", ce.SocketPath) } - desktop := isDesktopSocket(ce.SocketPath) + kind := classifySocket(ce.SocketPath) var hint string switch ce.Reason { case client.ReasonNotRunning: - hint = "Start the secrets engine and retry: nothing is listening on " + socket + "." - if desktop { + switch kind { + case engineDesktop: hint = desktopNotRunningHint + case engineStandalone: + hint = fmt.Sprintf(standaloneNotRunningHint, ce.SocketPath) + default: + hint = fmt.Sprintf(customNotRunningHint, socket) } case client.ReasonTimeout: - hint = "The secrets engine on " + socket + " did not respond. Restart it and retry." - if desktop { + switch kind { + case engineDesktop: hint = desktopTimeoutHint + case engineStandalone: + hint = fmt.Sprintf(standaloneTimeoutHint, ce.SocketPath) + default: + hint = fmt.Sprintf(customTimeoutHint, socket) } case client.ReasonPermissionDenied: - hint = "Check that your user may open " + socket + "." + hint = fmt.Sprintf(permissionDeniedHint, socket) default: return err } return fmt.Errorf("%w\n\n%s", err, hint) } -// isDesktopSocket reports whether path is the socket Docker Desktop's -// secrets engine listens on. A standalone engine uses a different socket. -func isDesktopSocket(path string) bool { - return path != "" && filepath.Clean(path) == filepath.Clean(api.DesktopSocketPath()) +// classifySocket reports which engine listens on path by default: Docker +// Desktop's, the standalone engine's, or neither (a custom socket). +func classifySocket(path string) engineKind { + if path == "" { + return engineCustom + } + switch filepath.Clean(path) { + case filepath.Clean(api.DesktopSocketPath()): + return engineDesktop + case filepath.Clean(api.StandaloneSocketPath()): + return engineStandalone + default: + return engineCustom + } } type clientOpts struct { diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index ae14f56a..ec7e4cc8 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -18,6 +18,7 @@ import ( "bytes" "errors" "fmt" + "strconv" "testing" "time" @@ -216,7 +217,7 @@ func Test_RmCommand(t *testing.T) { cmd := mustRmCommand(t, WithTimeout(time.Second), WithSocketPath(socket)) out, err := execute(t, cmd, mock, "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) - assert.Contains(t, out, "Start the secrets engine and retry: nothing is listening on "+socket+".") + assert.Contains(t, out, "Start the secrets engine and retry: nothing is listening on "+strconv.Quote(socket)+".") assert.NotContains(t, out, "RM:") assert.Equal(t, 2, remaining(t, mock)) }) @@ -351,7 +352,7 @@ func Test_GetCommand(t *testing.T) { out, err := execute(t, cmd, fooStore(), "--reveal", "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) assert.ErrorContains(t, err, "preflight ping") - assert.Contains(t, out, "\n\nStart the secrets engine and retry: nothing is listening on "+socket+".\n") + assert.Contains(t, out, "\n\nStart the secrets engine and retry: nothing is listening on "+strconv.Quote(socket)+".\n") assert.NotContains(t, out, "bar") }) t.Run("rejects an invalid option", func(t *testing.T) { @@ -381,6 +382,10 @@ func TestWithEngineHint(t *testing.T) { return fmt.Errorf("authorizing: %w", &client.ConnectError{Reason: reason, SocketPath: socket, Err: errors.New("dial")}) } desktop := api.DesktopSocketPath() + standalone := api.StandaloneSocketPath() + denied := func(socket string) string { + return "Your user lacks permission to connect to " + socket + ". Grant it read and write access to the socket and retry." + } tests := []struct { name string err error @@ -388,12 +393,19 @@ func TestWithEngineHint(t *testing.T) { }{ {name: "desktop not running", err: connErr(client.ReasonNotRunning, desktop), hint: desktopNotRunningHint}, {name: "desktop timeout", err: connErr(client.ReasonTimeout, desktop), hint: desktopTimeoutHint}, - {name: "standalone not running", err: connErr(client.ReasonNotRunning, "/s.sock"), hint: "Start the secrets engine and retry: nothing is listening on /s.sock."}, - {name: "standalone timeout", err: connErr(client.ReasonTimeout, "/s.sock"), hint: "The secrets engine on /s.sock did not respond. Restart it and retry."}, - {name: "permission denied", err: connErr(client.ReasonPermissionDenied, "/s.sock"), hint: "Check that your user may open /s.sock."}, - {name: "permission denied on desktop socket", err: connErr(client.ReasonPermissionDenied, desktop), hint: "Check that your user may open " + desktop + "."}, - {name: "permission denied without path", err: connErr(client.ReasonPermissionDenied, ""), hint: "Check that your user may open the secrets engine socket."}, + { + name: "standalone not running", + err: connErr(client.ReasonNotRunning, standalone), + hint: "Start the standalone secrets engine and retry: nothing is listening on " + strconv.Quote(standalone) + ".", + }, + {name: "standalone timeout", err: connErr(client.ReasonTimeout, standalone), hint: "The standalone secrets engine on " + strconv.Quote(standalone) + " did not respond. Restart it and retry."}, + {name: "custom socket not running", err: connErr(client.ReasonNotRunning, "/s.sock"), hint: `Start the secrets engine and retry: nothing is listening on "/s.sock".`}, + {name: "custom socket timeout", err: connErr(client.ReasonTimeout, "/s.sock"), hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`}, + {name: "permission denied", err: connErr(client.ReasonPermissionDenied, "/s.sock"), hint: denied(`"/s.sock"`)}, + {name: "permission denied on desktop socket", err: connErr(client.ReasonPermissionDenied, desktop), hint: denied(strconv.Quote(desktop))}, + {name: "permission denied without path", err: connErr(client.ReasonPermissionDenied, ""), hint: denied("the secrets engine socket")}, {name: "not running without path", err: connErr(client.ReasonNotRunning, ""), hint: "Start the secrets engine and retry: nothing is listening on the secrets engine socket."}, + {name: "control characters in path are quoted", err: connErr(client.ReasonNotRunning, "/s\n.sock"), hint: `Start the secrets engine and retry: nothing is listening on "/s\n.sock".`}, {name: "joined with other errors", err: errors.Join(store.ErrCredentialNotFound, connErr(client.ReasonNotRunning, desktop)), hint: desktopNotRunningHint}, } for _, tc := range tests { From 2ab7e503657e7fb920f64886983dc1af9d5229f2 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:19:41 +0200 Subject: [PATCH 3/5] feat(client): move connection hints into ConnectError.Hint Add ConnectError.Hint, which returns a one-line suggestion for fixing a failed connection based on its reason and socket: Docker Desktop's socket, the standalone engine's socket, or a custom one. Every SDK consumer can now show the same hints, and Error() is unchanged, so callers choose whether to print them. pass's withEngineHint now only finds the ConnectError and appends its hint. The per-reason wording tests move to the client package. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- client/client_test.go | 44 ++++++++++++++++ client/errors.go | 72 +++++++++++++++++++++++++++ plugins/pass/commands/client.go | 66 +----------------------- plugins/pass/commands/command_test.go | 28 ++++------- 4 files changed, 128 insertions(+), 82 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 0cd9c19b..881ee920 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "syscall" "testing" "time" @@ -422,6 +423,49 @@ func TestConnectError(t *testing.T) { }) } +func TestConnectErrorHint(t *testing.T) { + desktop := api.DesktopSocketPath() + standalone := api.StandaloneSocketPath() + denied := func(socket string) string { + return "Your user lacks permission to connect to " + socket + ". Grant it read and write access to the socket and retry." + } + tests := []struct { + name string + reason ConnectReason + socket string + hint string + }{ + {name: "desktop not running", reason: ReasonNotRunning, socket: desktop, hint: desktopNotRunningHint}, + {name: "desktop timeout", reason: ReasonTimeout, socket: desktop, hint: desktopTimeoutHint}, + { + name: "standalone not running", + reason: ReasonNotRunning, + socket: standalone, + hint: "Start the standalone secrets engine and retry: nothing is listening on " + strconv.Quote(standalone) + ".", + }, + { + name: "standalone timeout", + reason: ReasonTimeout, + socket: standalone, + hint: "The standalone secrets engine on " + strconv.Quote(standalone) + " did not respond. Restart it and retry.", + }, + {name: "custom socket not running", reason: ReasonNotRunning, socket: "/s.sock", hint: `Start the secrets engine and retry: nothing is listening on "/s.sock".`}, + {name: "custom socket timeout", reason: ReasonTimeout, socket: "/s.sock", hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`}, + {name: "permission denied", reason: ReasonPermissionDenied, socket: "/s.sock", hint: denied(`"/s.sock"`)}, + {name: "permission denied on desktop socket", reason: ReasonPermissionDenied, socket: desktop, hint: denied(strconv.Quote(desktop))}, + {name: "permission denied without path", reason: ReasonPermissionDenied, hint: denied("the secrets engine socket")}, + {name: "not running without path", reason: ReasonNotRunning, hint: "Start the secrets engine and retry: nothing is listening on the secrets engine socket."}, + {name: "control characters in path are quoted", reason: ReasonNotRunning, socket: "/s\n.sock", hint: `Start the secrets engine and retry: nothing is listening on "/s\n.sock".`}, + {name: "unknown reason has no hint", reason: ReasonUnknown, socket: "/s.sock", hint: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ce := &ConnectError{Reason: tc.reason, SocketPath: tc.socket, Err: errors.New("dial")} + assert.Equal(t, tc.hint, ce.Hint()) + }) + } +} + func TestConnectErrorFromSocket(t *testing.T) { getSecrets := func(t *testing.T, socketPath string) error { t.Helper() diff --git a/client/errors.go b/client/errors.go index 6aac009b..e37dd13a 100644 --- a/client/errors.go +++ b/client/errors.go @@ -17,8 +17,12 @@ package client import ( "context" "errors" + "fmt" "io/fs" "net" + "path/filepath" + + "github.com/docker/secrets-engine/x/api" ) var ( @@ -89,6 +93,74 @@ func (e *ConnectError) Is(target error) bool { return s != nil && target == s } +const ( + desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." + desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." + standaloneNotRunningHint = "Start the standalone secrets engine and retry: nothing is listening on %s." + standaloneTimeoutHint = "The standalone secrets engine on %s did not respond. Restart it and retry." + customNotRunningHint = "Start the secrets engine and retry: nothing is listening on %s." + customTimeoutHint = "The secrets engine on %s did not respond. Restart it and retry." + permissionDeniedHint = "Your user lacks permission to connect to %s. Grant it read and write access to the socket and retry." +) + +// Hint returns a one-line suggestion for how a user can fix the failed +// connection, or "" when the reason is unknown. It names Docker Desktop or the +// standalone engine when SocketPath is that engine's default socket. The socket +// path is quoted so control characters in it cannot corrupt terminal output. +func (e *ConnectError) Hint() string { + socket := "the secrets engine socket" + if e.SocketPath != "" { + socket = fmt.Sprintf("%q", e.SocketPath) + } + kind := classifySocket(e.SocketPath) + switch e.Reason { + case ReasonNotRunning: + switch kind { + case engineDesktop: + return desktopNotRunningHint + case engineStandalone: + return fmt.Sprintf(standaloneNotRunningHint, socket) + default: + return fmt.Sprintf(customNotRunningHint, socket) + } + case ReasonTimeout: + switch kind { + case engineDesktop: + return desktopTimeoutHint + case engineStandalone: + return fmt.Sprintf(standaloneTimeoutHint, socket) + default: + return fmt.Sprintf(customTimeoutHint, socket) + } + case ReasonPermissionDenied: + return fmt.Sprintf(permissionDeniedHint, socket) + default: + return "" + } +} + +type engineKind int + +const ( + engineCustom engineKind = iota + engineDesktop + engineStandalone +) + +func classifySocket(path string) engineKind { + if path == "" { + return engineCustom + } + switch filepath.Clean(path) { + case filepath.Clean(api.DesktopSocketPath()): + return engineDesktop + case filepath.Clean(api.StandaloneSocketPath()): + return engineStandalone + default: + return engineCustom + } +} + func connectReason(err error) ConnectReason { var ne net.Error switch { diff --git a/plugins/pass/commands/client.go b/plugins/pass/commands/client.go index c1ed32b0..2bc7de90 100644 --- a/plugins/pass/commands/client.go +++ b/plugins/pass/commands/client.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "path/filepath" "time" "github.com/spf13/cobra" @@ -30,24 +29,6 @@ import ( const defaultPreflightPingTimeout = 3 * time.Second -const ( - desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." - desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." - standaloneNotRunningHint = "Start the standalone secrets engine and retry: nothing is listening on %q." - standaloneTimeoutHint = "The standalone secrets engine on %q did not respond. Restart it and retry." - customNotRunningHint = "Start the secrets engine and retry: nothing is listening on %s." - customTimeoutHint = "The secrets engine on %s did not respond. Restart it and retry." - permissionDeniedHint = "Your user lacks permission to connect to %s. Grant it read and write access to the socket and retry." -) - -type engineKind int - -const ( - engineCustom engineKind = iota - engineDesktop - engineStandalone -) - func wrapEngineErrors(cmd *cobra.Command) *cobra.Command { if pre := cmd.PreRunE; pre != nil { cmd.PreRunE = func(c *cobra.Command, args []string) error { @@ -67,56 +48,13 @@ func withEngineHint(err error) error { if !ok { return err } - // Quote the path: it is user-controlled and may hold control characters. - socket := "the secrets engine socket" - if ce.SocketPath != "" { - socket = fmt.Sprintf("%q", ce.SocketPath) - } - kind := classifySocket(ce.SocketPath) - var hint string - switch ce.Reason { - case client.ReasonNotRunning: - switch kind { - case engineDesktop: - hint = desktopNotRunningHint - case engineStandalone: - hint = fmt.Sprintf(standaloneNotRunningHint, ce.SocketPath) - default: - hint = fmt.Sprintf(customNotRunningHint, socket) - } - case client.ReasonTimeout: - switch kind { - case engineDesktop: - hint = desktopTimeoutHint - case engineStandalone: - hint = fmt.Sprintf(standaloneTimeoutHint, ce.SocketPath) - default: - hint = fmt.Sprintf(customTimeoutHint, socket) - } - case client.ReasonPermissionDenied: - hint = fmt.Sprintf(permissionDeniedHint, socket) - default: + hint := ce.Hint() + if hint == "" { return err } return fmt.Errorf("%w\n\n%s", err, hint) } -// classifySocket reports which engine listens on path by default: Docker -// Desktop's, the standalone engine's, or neither (a custom socket). -func classifySocket(path string) engineKind { - if path == "" { - return engineCustom - } - switch filepath.Clean(path) { - case filepath.Clean(api.DesktopSocketPath()): - return engineDesktop - case filepath.Clean(api.StandaloneSocketPath()): - return engineStandalone - default: - return engineCustom - } -} - type clientOpts struct { timeout time.Duration responseTimeout time.Duration diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index ec7e4cc8..5a19e044 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -381,32 +381,24 @@ func TestWithEngineHint(t *testing.T) { connErr := func(reason client.ConnectReason, socket string) error { return fmt.Errorf("authorizing: %w", &client.ConnectError{Reason: reason, SocketPath: socket, Err: errors.New("dial")}) } + // The wording of each hint is tested in the client package, next to + // ConnectError.Hint; here we check that pass finds and appends it. desktop := api.DesktopSocketPath() - standalone := api.StandaloneSocketPath() - denied := func(socket string) string { - return "Your user lacks permission to connect to " + socket + ". Grant it read and write access to the socket and retry." - } tests := []struct { name string err error hint string }{ - {name: "desktop not running", err: connErr(client.ReasonNotRunning, desktop), hint: desktopNotRunningHint}, - {name: "desktop timeout", err: connErr(client.ReasonTimeout, desktop), hint: desktopTimeoutHint}, { - name: "standalone not running", - err: connErr(client.ReasonNotRunning, standalone), - hint: "Start the standalone secrets engine and retry: nothing is listening on " + strconv.Quote(standalone) + ".", + name: "wrapped connect error", + err: connErr(client.ReasonNotRunning, desktop), + hint: "Start Docker Desktop and retry: the secrets engine runs as part of it.", + }, + { + name: "joined with other errors", + err: errors.Join(store.ErrCredentialNotFound, connErr(client.ReasonTimeout, "/s.sock")), + hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`, }, - {name: "standalone timeout", err: connErr(client.ReasonTimeout, standalone), hint: "The standalone secrets engine on " + strconv.Quote(standalone) + " did not respond. Restart it and retry."}, - {name: "custom socket not running", err: connErr(client.ReasonNotRunning, "/s.sock"), hint: `Start the secrets engine and retry: nothing is listening on "/s.sock".`}, - {name: "custom socket timeout", err: connErr(client.ReasonTimeout, "/s.sock"), hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`}, - {name: "permission denied", err: connErr(client.ReasonPermissionDenied, "/s.sock"), hint: denied(`"/s.sock"`)}, - {name: "permission denied on desktop socket", err: connErr(client.ReasonPermissionDenied, desktop), hint: denied(strconv.Quote(desktop))}, - {name: "permission denied without path", err: connErr(client.ReasonPermissionDenied, ""), hint: denied("the secrets engine socket")}, - {name: "not running without path", err: connErr(client.ReasonNotRunning, ""), hint: "Start the secrets engine and retry: nothing is listening on the secrets engine socket."}, - {name: "control characters in path are quoted", err: connErr(client.ReasonNotRunning, "/s\n.sock"), hint: `Start the secrets engine and retry: nothing is listening on "/s\n.sock".`}, - {name: "joined with other errors", err: errors.Join(store.ErrCredentialNotFound, connErr(client.ReasonNotRunning, desktop)), hint: desktopNotRunningHint}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { From f2b71923ba38cd7be92c28e715b63e7a46b94515 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:22:34 +0200 Subject: [PATCH 4/5] docs(client): shorten ConnectError.Hint godoc Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- client/errors.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/client/errors.go b/client/errors.go index e37dd13a..3adf788f 100644 --- a/client/errors.go +++ b/client/errors.go @@ -103,10 +103,7 @@ const ( permissionDeniedHint = "Your user lacks permission to connect to %s. Grant it read and write access to the socket and retry." ) -// Hint returns a one-line suggestion for how a user can fix the failed -// connection, or "" when the reason is unknown. It names Docker Desktop or the -// standalone engine when SocketPath is that engine's default socket. The socket -// path is quoted so control characters in it cannot corrupt terminal output. +// Hint returns a user friendly message to help recover from the error. func (e *ConnectError) Hint() string { socket := "the secrets engine socket" if e.SocketPath != "" { From b21661f87772349877a107c6cd6c1ca1fd5b3bbd Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:24:40 +0200 Subject: [PATCH 5/5] chore(pass): drop comment from TestWithEngineHint Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- plugins/pass/commands/command_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index 5a19e044..a503e839 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -381,8 +381,6 @@ func TestWithEngineHint(t *testing.T) { connErr := func(reason client.ConnectReason, socket string) error { return fmt.Errorf("authorizing: %w", &client.ConnectError{Reason: reason, SocketPath: socket, Err: errors.New("dial")}) } - // The wording of each hint is tested in the client package, next to - // ConnectError.Hint; here we check that pass finds and appends it. desktop := api.DesktopSocketPath() tests := []struct { name string