fix(gpg): recover failed agent forwarding

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 {
@@ -48,18 +56,23 @@ 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
+
+	// 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.
@@ -139,21 +152,129 @@ 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, "check logs for details")
+	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"):
+		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,33 +343,55 @@ 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
 	}
 
 	log.Debugf(
 		"start reverse forward of gpg-agent socket %s, keeping connection open",
 		gpgExtraSocketPath,
 	)
-	reverseForwardPorts := append(
-		[]string{gpg.ContainerSocketPath + ":" + gpgExtraSocketPath},
-		t.cmd.ReverseForwardPorts...,
-	)
-	err := t.cmd.startReverseForwardsAndWait(ctx, containerClient, 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
+	}
+	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
+	t.userReverseForwardsStarted = true
 	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 +442,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 +455,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
 }