From bc7a73cbac54613693b1df115e337dd23040d3fc Mon Sep 17 00:00:00 2001 From: Samuel K <69881238+skevetter@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:52:24 -0600 Subject: [PATCH 1/5] fix(gpg): rebind failed agent forwards --- cmd/workspace/gpg_tunnel.go | 87 +++++++++++++++++++++++++------- cmd/workspace/gpg_tunnel_test.go | 70 +++++++++++++++++++++++++ cmd/workspace/port_forward.go | 61 +++++++++++++++++++--- 3 files changed, 193 insertions(+), 25 deletions(-) diff --git a/cmd/workspace/gpg_tunnel.go b/cmd/workspace/gpg_tunnel.go index 4902ae820a..817531228e 100644 --- a/cmd/workspace/gpg_tunnel.go +++ b/cmd/workspace/gpg_tunnel.go @@ -48,18 +48,20 @@ func writeGPGForwardFailedOSC(w io.Writer, reason string) { // another (owning) terminal's disconnect can re-establish it itself. const gpgTunnelHealthCheckInterval = 30 * time.Second +const gpgForwardStopTimeout = 2 * time.Second + // gpgTunnel owns the lifecycle of GPG-agent forwarding for one SSH session: -// deciding whether it's requested, binding the reverse-listen socket at most -// once, running the remote setup-gpg step, and periodically checking the -// tunnel is still alive for as long as the session runs. +// deciding whether it's requested, owning the reverse-forward lifecycle, +// running the remote setup-gpg step, and periodically checking the tunnel. type gpgTunnel struct { cmd *SSHCmd enabled bool - // forwardBound guards against re-binding the reverse-listen socket on a - // health-check retry: the listener stays open for the session, and the - // server rejects a second bind of the same path. - forwardBound bool + startForward func(context.Context, *ssh.Client, []string) (*managedReverseForward, error) + + // forward is nil unless this session currently owns a live forwarding loop. + // Its completion is reconciled before setup so an exited loop can be rebound. + forward *managedReverseForward // failureReported prevents a repeated OSC 9977 notification while the // tunnel stays down across health-check ticks. @@ -140,20 +142,32 @@ func (t *gpgTunnel) ensure(ctx context.Context, sshClient *ssh.Client) bool { log.Warnf("gpg agent forwarding failed (continuing without it): %v", err) t.failureReported = true // Emit OSC code for UI to detect. - writeGPGForwardFailedOSC(os.Stderr, "check logs for details") + writeGPGForwardFailedOSC(os.Stderr, gpgForwardFailureReason(err)) return false } +func gpgForwardFailureReason(err error) string { + switch { + case strings.Contains(err.Error(), "start gpg-agent reverse forward"): + return "GPG reverse forwarding failed" + case strings.Contains(err.Error(), "detect gpg-agent socket path"): + return "Host GPG agent socket unavailable" + case strings.Contains(err.Error(), "export local ownertrust from GPG"): + return "Unable to read host GPG configuration" + default: + return "GPG agent setup failed" + } +} + // setup runs the remote setup-gpg command, which imports the host's owner trust -// and signing key into the container's gpg-agent. It also ensures the -// reverse-forward socket is bound at most once per process, so concurrent -// terminals don't race to bind the same path. +// and signing key into the container's gpg-agent. It also ensures this session +// owns a live reverse-forward listener before configuring the remote socket. func (t *gpgTunnel) setup(ctx context.Context, containerClient *ssh.Client) error { log.Debugf("detecting gpg-agent socket path on host") // this socket gets forwarded to the remote and symlinked in multiple paths gpgExtraSocketPath, err := gpg.DetectAgentSocketPath() if err != nil { - return err + return fmt.Errorf("detect gpg-agent socket path: %w", err) } log.Debugf("detected gpg-agent socket path %s", gpgExtraSocketPath) @@ -222,14 +236,14 @@ func (t *gpgTunnel) buildSetupCommand(ctx context.Context) (string, error) { return command, nil } -// ensureForwardBound binds the reverse-listen socket at most once per -// process. +// ensureForwardBound keeps the GPG reverse-forward loop alive and starts a +// replacement after the previous loop exits. func (t *gpgTunnel) ensureForwardBound( ctx context.Context, containerClient *ssh.Client, gpgExtraSocketPath string, ) error { - if t.forwardBound { + if t.reconcileForward() { return nil } @@ -241,14 +255,35 @@ func (t *gpgTunnel) ensureForwardBound( []string{gpg.ContainerSocketPath + ":" + gpgExtraSocketPath}, t.cmd.ReverseForwardPorts..., ) - err := t.cmd.startReverseForwardsAndWait(ctx, containerClient, reverseForwardPorts) + startForward := t.startForward + if startForward == nil { + startForward = t.cmd.startReverseForwardsAndWait + } + forward, err := startForward(ctx, containerClient, reverseForwardPorts) if err != nil { return fmt.Errorf("start gpg-agent reverse forward: %w", err) } - t.forwardBound = true + t.forward = forward return nil } +// reconcileForward reports whether this tunnel still owns an active forward. +func (t *gpgTunnel) reconcileForward() bool { + if t.forward == nil { + return false + } + select { + case err, ok := <-t.forward.done: + t.forward = nil + if ok && err != nil { + log.Debugf("gpg agent reverse forward exited: %v", err) + } + return false + default: + return true + } +} + // signalReadyOnce signals gpg forward readiness at most once per gpgTunnel: // once the write actually succeeds, so a later health-check retry that also // succeeds does not write the readiness byte again. @@ -299,11 +334,11 @@ func runGPGTunnelInBackground( t *gpgTunnel, sshClient *ssh.Client, ) (wait func()) { - if t.enabled && t.ensure(ctx, sshClient) { + tunnelCtx, cancel := context.WithCancel(ctx) + if t.enabled && t.ensure(tunnelCtx, sshClient) { t.signalReadyOnce() } - tunnelCtx, cancel := context.WithCancel(ctx) done := make(chan struct{}) go func() { defer close(done) @@ -312,5 +347,19 @@ func runGPGTunnelInBackground( return func() { cancel() <-done + t.stopForward() + } +} + +func (t *gpgTunnel) stopForward() { + if t.forward == nil { + return + } + t.forward.cancel() + select { + case <-t.forward.done: + case <-time.After(gpgForwardStopTimeout): + log.Debugf("timed out waiting for gpg agent reverse forward to stop") } + t.forward = nil } diff --git a/cmd/workspace/gpg_tunnel_test.go b/cmd/workspace/gpg_tunnel_test.go index 209fb324d0..5e350a1ebf 100644 --- a/cmd/workspace/gpg_tunnel_test.go +++ b/cmd/workspace/gpg_tunnel_test.go @@ -2,11 +2,81 @@ package workspace import ( "bytes" + "context" + "errors" "fmt" "strings" "testing" + + "golang.org/x/crypto/ssh" ) +func TestGPGTunnelEnsureForwardBoundRebindsExitedForward(t *testing.T) { + firstDone := make(chan error, 1) + firstDone <- errors.New("forward exited") + close(firstDone) + + secondDone := make(chan error) + starts := 0 + tunnel := &gpgTunnel{ + cmd: &SSHCmd{}, + forward: &managedReverseForward{done: firstDone}, + startForward: func( + context.Context, + *ssh.Client, + []string, + ) (*managedReverseForward, error) { + starts++ + return &managedReverseForward{done: secondDone}, nil + }, + } + + if err := tunnel.ensureForwardBound( + context.Background(), nil, "/host/gpg-agent.sock", + ); err != nil { + t.Fatalf("ensureForwardBound() error = %v", err) + } + if starts != 1 { + t.Fatalf("startForward calls = %d, want 1", starts) + } + if tunnel.forward == nil || tunnel.forward.done != secondDone { + t.Fatal("ensureForwardBound() did not retain the replacement forward") + } +} + +func TestGPGTunnelEnsureForwardBoundKeepsActiveForward(t *testing.T) { + done := make(chan error) + starts := 0 + tunnel := &gpgTunnel{ + cmd: &SSHCmd{}, + forward: &managedReverseForward{done: done}, + startForward: func( + context.Context, + *ssh.Client, + []string, + ) (*managedReverseForward, error) { + starts++ + return nil, errors.New("unexpected rebind") + }, + } + + if err := tunnel.ensureForwardBound( + context.Background(), nil, "/host/gpg-agent.sock", + ); err != nil { + t.Fatalf("ensureForwardBound() error = %v", err) + } + if starts != 0 { + t.Fatalf("startForward calls = %d, want 0", starts) + } +} + +func TestGPGForwardFailureReasonUsesSafeCategory(t *testing.T) { + err := errors.New("start gpg-agent reverse forward: /private/path") + if got, want := gpgForwardFailureReason(err), "GPG reverse forwarding failed"; got != want { + t.Fatalf("gpgForwardFailureReason() = %q, want %q", got, want) + } +} + func TestWriteGPGForwardFailedOSC_WellFormedSequence(t *testing.T) { var buf bytes.Buffer writeGPGForwardFailedOSC(&buf, "socket did not appear") diff --git a/cmd/workspace/port_forward.go b/cmd/workspace/port_forward.go index 640f100ef1..c57f763062 100644 --- a/cmd/workspace/port_forward.go +++ b/cmd/workspace/port_forward.go @@ -194,21 +194,39 @@ type boundReverseForward struct { listener net.Listener } -// startReverseForwardsAndWait blocks until every forward's listener is bound, -// unlike reverseForwardPorts which blocks for the forward's lifetime. +// managedReverseForward owns the GPG socket's reverse-forward loop. done +// receives its terminal result after the listener has been closed. +type managedReverseForward struct { + cancel context.CancelFunc + done <-chan error +} + +type managedReverseForwardRun struct { + ctx context.Context + cancel context.CancelFunc + client *ssh.Client + forward boundReverseForward + timeout time.Duration + doneChan chan<- error +} + +// startReverseForwardsAndWait returns after every forward's listener is bound, +// unlike reverseForwardPorts which blocks for the forward's lifetime. The +// returned handle lets the GPG tunnel observe its first mapping's lifecycle; +// any additional mappings retain their independent lifetime. func (cmd *SSHCmd) startReverseForwardsAndWait( ctx context.Context, containerClient *ssh.Client, portMappings []string, -) error { +) (*managedReverseForward, error) { timeout, err := cmd.forwardTimeout() if err != nil { - return err + return nil, err } bound, err := bindReverseForwards(containerClient, portMappings) if err != nil { - return err + return nil, err } for _, b := range bound { @@ -219,10 +237,41 @@ func (cmd *SSHCmd) startReverseForwardsAndWait( b.mapping.Container.Protocol, b.mapping.Container.Address, ) + } + + forwardCtx, cancel := context.WithCancel(ctx) + done := make(chan error, 1) + go runManagedReverseForward(managedReverseForwardRun{ + ctx: forwardCtx, + cancel: cancel, + client: containerClient, + forward: bound[0], + timeout: timeout, + doneChan: done, + }) + for _, b := range bound[1:] { go runReverseForwardInBackground(ctx, containerClient, b, timeout) } - return nil + return &managedReverseForward{cancel: cancel, done: done}, nil +} + +func runManagedReverseForward(run managedReverseForwardRun) { + defer close(run.doneChan) + err := devssh.RunReverseForward(run.ctx, run.client, devssh.ReverseForwardOpts{ + Listener: run.forward.listener, + RemoteAddr: run.forward.mapping.Host.Address, + LocalNetwork: run.forward.mapping.Container.Protocol, + LocalAddr: run.forward.mapping.Container.Address, + ExitAfterTimeout: run.timeout, + }) + if err != nil && !errors.Is(err, devssh.ErrIdleTimeout) && !errors.Is(err, io.EOF) && + !errors.Is(err, context.Canceled) { + log.Errorf("error forwarding %s: %v", run.forward.portMapping, err) + err = fmt.Errorf("error forwarding %s: %w", run.forward.portMapping, err) + } + run.cancel() + run.doneChan <- err } func bindReverseForwards( From 3beedf7045c3f9a4e46b534cbd561c726b85d7f8 Mon Sep 17 00:00:00 2001 From: Samuel K <69881238+skevetter@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:03:51 -0600 Subject: [PATCH 2/5] fix(gpg): persist desktop forwarding diagnostics --- cmd/workspace/gpg_tunnel.go | 106 +++++++++++++++++- cmd/workspace/gpg_tunnel_test.go | 74 ++++++++++++ desktop/src/main/__tests__/log-store.test.ts | 18 +++ desktop/src/main/__tests__/pty.test.ts | 30 +++++ desktop/src/main/ipc.ts | 11 +- desktop/src/main/log-store.ts | 18 ++- desktop/src/main/pty.ts | 21 +++- .../src/pages/WorkspaceDetailPage.svelte | 2 +- 8 files changed, 269 insertions(+), 11 deletions(-) diff --git a/cmd/workspace/gpg_tunnel.go b/cmd/workspace/gpg_tunnel.go index 817531228e..89344351b6 100644 --- a/cmd/workspace/gpg_tunnel.go +++ b/cmd/workspace/gpg_tunnel.go @@ -3,9 +3,11 @@ package workspace import ( "context" "encoding/base64" + "encoding/json" "fmt" "io" "os" + "path/filepath" "strconv" "strings" "time" @@ -15,6 +17,7 @@ import ( "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/gpg" "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/pkg/secrets" devssh "github.com/devsy-org/devsy/pkg/ssh" "golang.org/x/crypto/ssh" ) @@ -27,6 +30,11 @@ const gpgForwardFailedOSC = 9977 // toast renders reason verbatim and it can originate from a remote error. const gpgForwardFailedReasonMaxLen = 256 +const ( + gpgForwardDiagnosticFileEnv = "DEVSY_GPG_FORWARD_DIAGNOSTIC_FILE" + gpgForwardSessionIDEnv = "DEVSY_GPG_FORWARD_SESSION_ID" +) + func writeGPGForwardFailedOSC(w io.Writer, reason string) { runes := []rune(reason) if len(runes) > gpgForwardFailedReasonMaxLen { @@ -141,11 +149,107 @@ func (t *gpgTunnel) ensure(ctx context.Context, sshClient *ssh.Client) bool { } log.Warnf("gpg agent forwarding failed (continuing without it): %v", err) t.failureReported = true + reason := gpgForwardFailureReason(err) + if writeGPGForwardDiagnostic(err) { + reason += ": details saved in workspace logs" + } // Emit OSC code for UI to detect. - writeGPGForwardFailedOSC(os.Stderr, gpgForwardFailureReason(err)) + writeGPGForwardFailedOSC(os.Stderr, reason) return false } +type gpgForwardDiagnostic struct { + Timestamp string `json:"timestamp"` + Component string `json:"component"` + Code string `json:"code"` + SessionID string `json:"sessionId,omitempty"` + Message string `json:"message"` +} + +func writeGPGForwardDiagnostic(err error) bool { + path, pathErr := desktopGPGDiagnosticPath(os.Getenv(gpgForwardDiagnosticFileEnv)) + if pathErr != nil { + log.Debugf("resolve gpg agent forwarding diagnostic path: %v", pathErr) + return false + } + + record, marshalErr := marshalGPGForwardDiagnostic(err) + if marshalErr != nil { + log.Debugf("encode gpg agent forwarding diagnostic: %v", marshalErr) + return false + } + // #nosec G304 G703 -- The path is validated to stay within Desktop's workspace log directory. + file, openErr := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + if openErr != nil { + log.Debugf("open gpg agent forwarding diagnostic: %v", openErr) + return false + } + if n, writeErr := file.Write(append(record, '\n')); writeErr != nil || n != len(record)+1 { + if writeErr == nil { + writeErr = io.ErrShortWrite + } + log.Debugf("write gpg agent forwarding diagnostic: %v", writeErr) + _ = file.Close() + return false + } + if closeErr := file.Close(); closeErr != nil { + log.Debugf("close gpg agent forwarding diagnostic: %v", closeErr) + return false + } + return true +} + +func marshalGPGForwardDiagnostic(err error) ([]byte, error) { + message := secrets.NewEnvironmentRedactor(os.Environ()).Redact(err.Error()) + if runes := []rune(message); len(runes) > 2048 { + message = string(runes[:2048]) + } + return json.Marshal(gpgForwardDiagnostic{ + Timestamp: time.Now().UTC().Format(time.RFC3339Nano), + Component: "gpg-forwarding", + Code: gpgForwardFailureCode(err), + SessionID: os.Getenv(gpgForwardSessionIDEnv), + Message: message, + }) +} + +func desktopGPGDiagnosticPath(path string) (string, error) { + if path == "" { + return "", fmt.Errorf("diagnostic file path is not configured") + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home directory: %w", err) + } + root, err := filepath.Abs(filepath.Join(home, ".devsy", "desktop", "logs", "workspaces")) + if err != nil { + return "", fmt.Errorf("resolve Desktop log directory: %w", err) + } + path, err = filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolve diagnostic file path: %w", err) + } + relative, err := filepath.Rel(root, path) + if err != nil || relative == ".." || + strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("diagnostic path is outside Desktop workspace logs") + } + return path, nil +} + +func gpgForwardFailureCode(err error) string { + switch { + case strings.Contains(err.Error(), "start gpg-agent reverse forward"): + return "reverse_forward_failed" + case strings.Contains(err.Error(), "detect gpg-agent socket path"): + return "host_agent_socket_unavailable" + case strings.Contains(err.Error(), "export local ownertrust from GPG"): + return "host_gpg_configuration_unavailable" + default: + return "setup_failed" + } +} + func gpgForwardFailureReason(err error) string { switch { case strings.Contains(err.Error(), "start gpg-agent reverse forward"): diff --git a/cmd/workspace/gpg_tunnel_test.go b/cmd/workspace/gpg_tunnel_test.go index 5e350a1ebf..156f4597cc 100644 --- a/cmd/workspace/gpg_tunnel_test.go +++ b/cmd/workspace/gpg_tunnel_test.go @@ -3,8 +3,11 @@ package workspace import ( "bytes" "context" + "encoding/json" "errors" "fmt" + "os" + "path/filepath" "strings" "testing" @@ -77,6 +80,77 @@ func TestGPGForwardFailureReasonUsesSafeCategory(t *testing.T) { } } +func TestWriteGPGForwardDiagnosticRedactsAndScopesDetails(t *testing.T) { + home := t.TempDir() + path := filepath.Join( + home, ".devsy", "desktop", "logs", "workspaces", "default", "ws-1", "ssh.log", + ) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("create diagnostic directory: %v", err) + } + t.Setenv("HOME", home) + t.Setenv(gpgForwardDiagnosticFileEnv, path) + t.Setenv(gpgForwardSessionIDEnv, "session-123") + t.Setenv("TOKEN", "diagnostic-secret") + + if !writeGPGForwardDiagnostic(errors.New( + "start gpg-agent reverse forward: token=diagnostic-secret", + )) { + t.Fatal("writeGPGForwardDiagnostic() = false, want true") + } + + // #nosec G304 -- Test path is under its temporary home directory. + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read diagnostic: %v", err) + } + var record gpgForwardDiagnostic + if err := json.Unmarshal(bytes.TrimSpace(data), &record); err != nil { + t.Fatalf("decode diagnostic: %v", err) + } + assertGPGForwardDiagnostic(t, record) + assertPrivateGPGDiagnosticFile(t, path) +} + +func TestDesktopGPGDiagnosticPathRejectsOutsideLogRoot(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + path := filepath.Join(home, "outside.log") + if _, err := desktopGPGDiagnosticPath(path); err == nil { + t.Fatalf("desktopGPGDiagnosticPath(%q) returned no error", path) + } +} + +func assertGPGForwardDiagnostic(t *testing.T, record gpgForwardDiagnostic) { + t.Helper() + if record.Component != "gpg-forwarding" || record.Code != "reverse_forward_failed" { + t.Fatalf("diagnostic component/code = %q/%q", record.Component, record.Code) + } + if record.SessionID != "session-123" { + t.Fatalf("diagnostic session id = %q, want session-123", record.SessionID) + } + if strings.Contains(record.Message, "diagnostic-secret") { + t.Fatalf("diagnostic message exposed the secret: %q", record.Message) + } + if !strings.Contains(record.Message, "***") { + t.Fatalf("diagnostic message was not redacted: %q", record.Message) + } + if record.Timestamp == "" { + t.Fatal("diagnostic timestamp is empty") + } +} + +func assertPrivateGPGDiagnosticFile(t *testing.T, path string) { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat diagnostic: %v", err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("diagnostic permissions = %o, want 600", info.Mode().Perm()) + } +} + func TestWriteGPGForwardFailedOSC_WellFormedSequence(t *testing.T) { var buf bytes.Buffer writeGPGForwardFailedOSC(&buf, "socket did not appear") diff --git a/desktop/src/main/__tests__/log-store.test.ts b/desktop/src/main/__tests__/log-store.test.ts index e767adc692..bc08f54a88 100644 --- a/desktop/src/main/__tests__/log-store.test.ts +++ b/desktop/src/main/__tests__/log-store.test.ts @@ -25,6 +25,24 @@ describe("LogStore", () => { expect(logPath).toMatch(/\.log$/) }) + it("reserves an SSH diagnostic log path without exposing raw PTY output", async () => { + const logPath = store.createDiagnosticLogPath(CTX, "ws-1") + expect(logPath).toContain(join("workspaces", CTX, "ws-1")) + expect(logPath).toMatch(/-ssh-diagnostics\.log$/) + + store.appendLog( + logPath, + '{"component":"gpg-forwarding","code":"setup_failed"}', + ) + await store.closeLog(logPath) + + const entries = store.listLogs(CTX, "ws-1") + expect(entries).toHaveLength(1) + expect(store.readLog(CTX, "ws-1", entries[0].filename)).toContain( + '"component":"gpg-forwarding"', + ) + }) + it("appends lines to a log file", async () => { const logPath = store.createLogFile(CTX, "ws-1") store.appendLog(logPath, "line 1") diff --git a/desktop/src/main/__tests__/pty.test.ts b/desktop/src/main/__tests__/pty.test.ts index e251032785..47488650cf 100644 --- a/desktop/src/main/__tests__/pty.test.ts +++ b/desktop/src/main/__tests__/pty.test.ts @@ -36,4 +36,34 @@ describe("PtyManager", () => { "workspace-id", ]) }) + + it("passes a session-scoped diagnostic log path to the SSH CLI", () => { + const proc = { + onData: vi.fn(), + onExit: vi.fn(), + } as unknown as IPty + const spawnPty = vi + .fn() + .mockReturnValue(proc) as unknown as typeof import("node-pty").spawn + const manager = new PtyManager({ + binaryPath: "/devsy", + getMainWindow: () => null, + spawnPty, + }) + + const sessionId = manager.createSshSession( + "workspace-id", + 80, + 24, + "/logs/ssh-diagnostics.log", + ) + + const options = spawnPty.mock.calls[0]?.[2] as { + env: Record + } + expect(options.env.DEVSY_GPG_FORWARD_DIAGNOSTIC_FILE).toBe( + "/logs/ssh-diagnostics.log", + ) + expect(options.env.DEVSY_GPG_FORWARD_SESSION_ID).toBe(sessionId) + }) }) diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 79147783f3..ce3ec0d65d 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -2413,7 +2413,16 @@ export function registerIpcHandlers(deps: IpcDependencies): { _event, args: { workspaceId: string; cols: number; rows: number }, ) => { - return deps.pty.createSshSession(args.workspaceId, args.cols, args.rows) + const diagnosticLogPath = logStore.createDiagnosticLogPath( + state.workspaceContext(args.workspaceId), + args.workspaceId, + ) + return deps.pty.createSshSession( + args.workspaceId, + args.cols, + args.rows, + diagnosticLogPath, + ) }, ) diff --git a/desktop/src/main/log-store.ts b/desktop/src/main/log-store.ts index bc5e3f7291..2adc01cece 100644 --- a/desktop/src/main/log-store.ts +++ b/desktop/src/main/log-store.ts @@ -63,14 +63,24 @@ export class LogStore { createLogFile(context: string, workspaceId: string): string { const dir = this.workspaceLogDir(context, workspaceId) mkdirSync(dir, { recursive: true }) - const timestamp = new Date().toISOString().replace(/[:.]/g, "-") - const suffix = String(counter++).padStart(4, "0") - const filename = `${timestamp}-${suffix}.log` + const filename = this.nextLogFilename() const filePath = join(dir, filename) - writeFileSync(filePath, "") + writeFileSync(filePath, "", { mode: 0o600 }) return filePath } + createDiagnosticLogPath(context: string, workspaceId: string): string { + const dir = this.workspaceLogDir(context, workspaceId) + mkdirSync(dir, { recursive: true }) + return join(dir, this.nextLogFilename("-ssh-diagnostics")) + } + + private nextLogFilename(suffix = ""): string { + const timestamp = new Date().toISOString().replace(/[:.]/g, "-") + const sequence = String(counter++).padStart(4, "0") + return `${timestamp}-${sequence}${suffix}.log` + } + appendLog(logPath: string, line: string): boolean { let stream = this.streams.get(logPath) if (!stream) { diff --git a/desktop/src/main/pty.ts b/desktop/src/main/pty.ts index aab5cc8937..78d1d736f1 100644 --- a/desktop/src/main/pty.ts +++ b/desktop/src/main/pty.ts @@ -82,14 +82,27 @@ export class PtyManager { return sessionId } - createSshSession(workspaceId: string, cols: number, rows: number): string { - const pty = requirePty() + createSshSession( + workspaceId: string, + cols: number, + rows: number, + diagnosticLogPath?: string, + ): string { + const spawn = this.deps.spawnPty ?? requirePty().spawn const sessionId = crypto.randomUUID() - const proc = pty.spawn(this.deps.binaryPath, sshTerminalArgs(workspaceId), { + const proc = spawn(this.deps.binaryPath, sshTerminalArgs(workspaceId), { name: "xterm-256color", cols, rows, - env: this.env, + env: { + ...this.env, + ...(diagnosticLogPath + ? { + DEVSY_GPG_FORWARD_DIAGNOSTIC_FILE: diagnosticLogPath, + DEVSY_GPG_FORWARD_SESSION_ID: sessionId, + } + : {}), + }, }) this.wire(sessionId, proc, workspaceId) diff --git a/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte b/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte index 8bfc02529e..6305c9fb94 100644 --- a/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte +++ b/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte @@ -435,7 +435,7 @@ function handleSshExit(exitCode?: number, _signal?: number) { function handleGpgForwardFailed(reason: string) { toasts.error( - `GPG agent forwarding failed for ${id}, continuing without it: ${reason}`, + `GPG agent forwarding is unavailable for ${id}: ${reason}. The terminal will continue without GPG signing.`, ) } From de7b1458b00edb03a1275e5a2f7647980863ff31 Mon Sep 17 00:00:00 2001 From: Samuel K <69881238+skevetter@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:08:08 -0600 Subject: [PATCH 3/5] test(gpg): cover failed forward rebinding --- cmd/workspace/gpg_tunnel_test.go | 82 ++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/cmd/workspace/gpg_tunnel_test.go b/cmd/workspace/gpg_tunnel_test.go index 156f4597cc..0fd866765d 100644 --- a/cmd/workspace/gpg_tunnel_test.go +++ b/cmd/workspace/gpg_tunnel_test.go @@ -6,11 +6,14 @@ import ( "encoding/json" "errors" "fmt" + "net" "os" "path/filepath" "strings" "testing" + "time" + "github.com/devsy-org/devsy/pkg/port" "golang.org/x/crypto/ssh" ) @@ -73,6 +76,85 @@ func TestGPGTunnelEnsureForwardBoundKeepsActiveForward(t *testing.T) { } } +func TestGPGTunnelRebindsWhenManagedReverseForwardExits(t *testing.T) { + ctx := context.Background() + var listeners []net.Listener + starts := 0 + tunnel := &gpgTunnel{cmd: &SSHCmd{}} + tunnel.startForward = func( + ctx context.Context, + client *ssh.Client, + _ []string, + ) (*managedReverseForward, error) { + forward, listener, err := startTestManagedReverseForward(ctx, client) + if err != nil { + return nil, err + } + listeners = append(listeners, listener) + starts++ + return forward, nil + } + t.Cleanup(tunnel.stopForward) + + if err := tunnel.ensureForwardBound(ctx, nil, "/host/gpg-agent.sock"); err != nil { + t.Fatalf("initial ensureForwardBound() error = %v", err) + } + if err := listeners[0].Close(); err != nil { + t.Fatalf("close first listener: %v", err) + } + waitForManagedReverseForward(t, tunnel.forward) + if err := tunnel.ensureForwardBound(ctx, nil, "/host/gpg-agent.sock"); err != nil { + t.Fatalf("replacement ensureForwardBound() error = %v", err) + } + if starts != 2 { + t.Fatalf("startForward calls = %d, want 2", starts) + } +} + +func startTestManagedReverseForward( + ctx context.Context, + client *ssh.Client, +) (*managedReverseForward, net.Listener, error) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return nil, nil, err + } + forwardCtx, cancel := context.WithCancel(ctx) + done := make(chan error, 1) + go runManagedReverseForward(managedReverseForwardRun{ + ctx: forwardCtx, + cancel: cancel, + client: client, + forward: boundReverseForward{ + portMapping: "gpg socket", + mapping: port.Mapping{ + Host: port.Address{Protocol: "tcp", Address: listener.Addr().String()}, + Container: port.Address{Protocol: "tcp", Address: "127.0.0.1:1"}, + }, + listener: listener, + }, + doneChan: done, + }) + return &managedReverseForward{cancel: cancel, done: done}, listener, nil +} + +func waitForManagedReverseForward(t *testing.T, forward *managedReverseForward) { + t.Helper() + select { + case <-forward.done: + case <-time.After(time.Second): + t.Fatal("managed forward did not report listener exit") + } + select { + case _, ok := <-forward.done: + if ok { + t.Fatal("managed forward reported more than one result") + } + case <-time.After(time.Second): + t.Fatal("managed forward did not close its completion channel") + } +} + func TestGPGForwardFailureReasonUsesSafeCategory(t *testing.T) { err := errors.New("start gpg-agent reverse forward: /private/path") if got, want := gpgForwardFailureReason(err), "GPG reverse forwarding failed"; got != want { From 2594b656b349be7bd6e258e48b3123c90759caf0 Mon Sep 17 00:00:00 2001 From: Samuel K <69881238+skevetter@users.noreply.github.com> Date: Tue, 6 Oct 2026 01:30:28 -0600 Subject: [PATCH 4/5] fix(gpg): preserve user forwards during recovery --- cmd/workspace/gpg_tunnel.go | 12 ++- cmd/workspace/gpg_tunnel_test.go | 128 +++++++++++++++++++++++++++---- 2 files changed, 120 insertions(+), 20 deletions(-) diff --git a/cmd/workspace/gpg_tunnel.go b/cmd/workspace/gpg_tunnel.go index 89344351b6..45281c413d 100644 --- a/cmd/workspace/gpg_tunnel.go +++ b/cmd/workspace/gpg_tunnel.go @@ -71,6 +71,9 @@ type gpgTunnel struct { // Its completion is reconciled before setup so an exited loop can be rebound. forward *managedReverseForward + // User mappings outlive the managed GPG loop and must only be bound once. + userReverseForwardsStarted bool + // failureReported prevents a repeated OSC 9977 notification while the // tunnel stays down across health-check ticks. failureReported bool @@ -355,10 +358,10 @@ func (t *gpgTunnel) ensureForwardBound( "start reverse forward of gpg-agent socket %s, keeping connection open", gpgExtraSocketPath, ) - reverseForwardPorts := append( - []string{gpg.ContainerSocketPath + ":" + gpgExtraSocketPath}, - t.cmd.ReverseForwardPorts..., - ) + reverseForwardPorts := []string{gpg.ContainerSocketPath + ":" + gpgExtraSocketPath} + if !t.userReverseForwardsStarted { + reverseForwardPorts = append(reverseForwardPorts, t.cmd.ReverseForwardPorts...) + } startForward := t.startForward if startForward == nil { startForward = t.cmd.startReverseForwardsAndWait @@ -368,6 +371,7 @@ func (t *gpgTunnel) ensureForwardBound( return fmt.Errorf("start gpg-agent reverse forward: %w", err) } t.forward = forward + t.userReverseForwardsStarted = true return nil } diff --git a/cmd/workspace/gpg_tunnel_test.go b/cmd/workspace/gpg_tunnel_test.go index 0fd866765d..66254ad433 100644 --- a/cmd/workspace/gpg_tunnel_test.go +++ b/cmd/workspace/gpg_tunnel_test.go @@ -9,10 +9,12 @@ import ( "net" "os" "path/filepath" + "slices" "strings" "testing" "time" + "github.com/devsy-org/devsy/pkg/gpg" "github.com/devsy-org/devsy/pkg/port" "golang.org/x/crypto/ssh" ) @@ -78,34 +80,128 @@ func TestGPGTunnelEnsureForwardBoundKeepsActiveForward(t *testing.T) { func TestGPGTunnelRebindsWhenManagedReverseForwardExits(t *testing.T) { ctx := context.Background() - var listeners []net.Listener - starts := 0 - tunnel := &gpgTunnel{cmd: &SSHCmd{}} - tunnel.startForward = func( - ctx context.Context, - client *ssh.Client, - _ []string, - ) (*managedReverseForward, error) { - forward, listener, err := startTestManagedReverseForward(ctx, client) - if err != nil { - return nil, err - } - listeners = append(listeners, listener) - starts++ - return forward, nil + forwards := &testGPGReverseForwards{t: t, userMapping: "127.0.0.1:9000:127.0.0.1:9001"} + tunnel := &gpgTunnel{ + cmd: &SSHCmd{ReverseForwardPorts: []string{forwards.userMapping}}, + startForward: forwards.start, } t.Cleanup(tunnel.stopForward) if err := tunnel.ensureForwardBound(ctx, nil, "/host/gpg-agent.sock"); err != nil { t.Fatalf("initial ensureForwardBound() error = %v", err) } - if err := listeners[0].Close(); err != nil { + if err := forwards.listeners[0].Close(); err != nil { t.Fatalf("close first listener: %v", err) } waitForManagedReverseForward(t, tunnel.forward) if err := tunnel.ensureForwardBound(ctx, nil, "/host/gpg-agent.sock"); err != nil { t.Fatalf("replacement ensureForwardBound() error = %v", err) } + if len(forwards.listeners) != 2 { + t.Fatalf("startForward calls = %d, want 2", len(forwards.listeners)) + } + assertUserReverseForwardActive(t, forwards.userForward, forwards.userListener) + replacement := tunnel.forward + tunnel.stopForward() + waitForManagedReverseForward(t, replacement) + assertReverseForwardListenerReleased(t, forwards.listeners[1]) +} + +type testGPGReverseForwards struct { + t *testing.T + userMapping string + userForward *managedReverseForward + userListener net.Listener + listeners []net.Listener +} + +func (f *testGPGReverseForwards) start( + ctx context.Context, + client *ssh.Client, + mappings []string, +) (*managedReverseForward, error) { + want := []string{gpg.ContainerSocketPath + ":/host/gpg-agent.sock"} + if len(f.listeners) == 0 { + want = append(want, f.userMapping) + } + if !slices.Equal(mappings, want) { + return nil, fmt.Errorf("forward mappings = %v, want %v", mappings, want) + } + if len(f.listeners) == 0 { + var err error + f.userForward, f.userListener, err = startTestManagedReverseForward(ctx, client) + if err != nil { + return nil, err + } + f.t.Cleanup(func() { + f.userForward.cancel() + waitForManagedReverseForward(f.t, f.userForward) + }) + } + forward, listener, err := startTestManagedReverseForward(ctx, client) + if err != nil { + return nil, err + } + f.listeners = append(f.listeners, listener) + return forward, nil +} + +func assertUserReverseForwardActive( + t *testing.T, + forward *managedReverseForward, + listener net.Listener, +) { + t.Helper() + select { + case <-forward.done: + t.Fatal("GPG replacement stopped the independent user forward") + default: + } + duplicate, err := net.Listen("tcp", listener.Addr().String()) + if err == nil { + _ = duplicate.Close() + t.Fatal("user forward listener was released during GPG replacement") + } +} + +func assertReverseForwardListenerReleased(t *testing.T, stopped net.Listener) { + t.Helper() + if listener, err := net.Listen("tcp", stopped.Addr().String()); err != nil { + t.Fatalf("replacement listener still bound after stop: %v", err) + } else { + _ = listener.Close() + } +} + +func TestGPGTunnelRetriesUserMappingsAfterInitialBindFailure(t *testing.T) { + userMapping := "127.0.0.1:9000:127.0.0.1:9001" + want := []string{gpg.ContainerSocketPath + ":/host/gpg-agent.sock", userMapping} + starts := 0 + tunnel := &gpgTunnel{ + cmd: &SSHCmd{ReverseForwardPorts: []string{userMapping}}, + startForward: func(_ context.Context, _ *ssh.Client, mappings []string) (*managedReverseForward, error) { + starts++ + if !slices.Equal(mappings, want) { + t.Fatalf("forward mappings = %v, want %v", mappings, want) + } + if starts == 1 { + return nil, errors.New("initial listener bind failed") + } + return &managedReverseForward{done: make(chan error)}, nil + }, + } + if err := tunnel.ensureForwardBound( + context.Background(), nil, "/host/gpg-agent.sock", + ); err == nil { + t.Fatal("initial listener bind failure was discarded") + } + if err := tunnel.ensureForwardBound( + context.Background(), + nil, + "/host/gpg-agent.sock", + ); err != nil { + t.Fatalf("retry ensureForwardBound() error = %v", err) + } if starts != 2 { t.Fatalf("startForward calls = %d, want 2", starts) } From 057945d35700f838d8d318626faa190002c8ea26 Mon Sep 17 00:00:00 2001 From: Samuel K <69881238+skevetter@users.noreply.github.com> Date: Tue, 6 Oct 2026 02:06:34 -0600 Subject: [PATCH 5/5] test(driver): stabilize info negotiation timeout --- pkg/driver/external/host_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/driver/external/host_test.go b/pkg/driver/external/host_test.go index 5d52071c84..80ec8dd4f1 100644 --- a/pkg/driver/external/host_test.go +++ b/pkg/driver/external/host_test.go @@ -116,8 +116,17 @@ func (s *HostSuite) TestStartupTimeout() { } func (s *HostSuite) TestInfoNegotiationTimeout() { - _, err := s.newHost(context.Background(), "block-info", 500*time.Millisecond) - s.ErrorIs(err, context.DeadlineExceeded) + started := time.Now() + host, err := s.newHost(context.Background(), "block-info", 500*time.Millisecond) + s.Nil(host) + s.Require().Error(err) + // The server's deadline can reset the stream before the host's timer fires. + s.True( + errors.Is(err, context.DeadlineExceeded) || + status.Code(err) == codes.DeadlineExceeded || status.Code(err) == codes.Canceled, + "expected deadline or stream cancellation, got %v", err, + ) + s.Less(time.Since(started), 5*time.Second) } func (s *HostSuite) TestStructuredFailureAndCapabilitiesCopy() {