fix: resolve SSH signature key path for container-to-host forwarding (#714)

Author: skevetter

e2e/tests/ssh/ssh.go

@@ -140,6 +140,36 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
 			).Run()
 			framework.ExpectNoError(err)
 
+			agentOut, err := exec.Command("ssh-agent", "-s").Output()
+			framework.ExpectNoError(err)
+			t := ginkgo.GinkgoT()
+			var agentPID string
+			for line := range strings.SplitSeq(string(agentOut), "\n") {
+				for _, prefix := range []string{"SSH_AUTH_SOCK=", "SSH_AGENT_PID="} {
+					if _, after, ok := strings.Cut(line, prefix); ok {
+						val := after
+						if semi := strings.Index(val, ";"); semi >= 0 {
+							val = val[:semi]
+						}
+						key := prefix[:len(prefix)-1]
+						if key == "SSH_AGENT_PID" {
+							agentPID = val
+						}
+						t.Setenv(key, val)
+					}
+				}
+			}
+			ginkgo.DeferCleanup(func(_ context.Context) {
+				if agentPID != "" {
+					// #nosec G204 -- controlled pid from ssh-agent we started
+					_ = exec.Command("kill", agentPID).Run()
+				}
+			})
+
+			// #nosec G204 -- test command with controlled arguments
+			err = exec.Command("ssh-add", keyPath).Run()
+			framework.ExpectNoError(err)
+
 			// Start workspace with git-ssh-signing-key flag
 			err = f.DevPodUp(ctx, tempDir, "--git-ssh-signing-key", keyPath+".pub")
 			framework.ExpectNoError(err)
@@ -162,6 +192,11 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
 				"echo test > testfile",
 				"git add testfile",
 				"git commit -m 'signed test commit' 2>&1",
+				"ssh-add -L | head -1 > /tmp/test_signing_key.pub",
+				"git config --global user.signingkey /tmp/test_signing_key.pub",
+				"echo world >> file.txt",
+				"git add file.txt",
+				"git commit -m 'signed commit with file path key' 2>&1",
 			}, " && ")
 
 			// The signing key must be passed on each SSH invocation so the

pkg/agent/tunnelserver/tunnelserver_gitssh_test.go

@@ -0,0 +1,33 @@
+package tunnelserver
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/skevetter/devpod/pkg/agent/tunnel"
+	"github.com/skevetter/devpod/pkg/gitsshsigning"
+	"github.com/skevetter/log"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGitSSHSignature_DeserializesPublicKey(t *testing.T) {
+	req := gitsshsigning.GitSSHSignatureRequest{
+		Content:   "tree abc\nauthor Test\n\ncommit",
+		KeyPath:   "/container/tmp/.git_signing_key_tmpXXX",
+		PublicKey: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFake test@test.com",
+	}
+	reqJSON, err := json.Marshal(req)
+	require.NoError(t, err)
+
+	ts := New(log.Discard)
+
+	_, err = ts.GitSSHSignature(context.Background(), &tunnel.Message{
+		Message: string(reqJSON),
+	})
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), "git ssh sign request")
+	assert.NotContains(t, err.Error(), "resolve signing key")
+	assert.Contains(t, err.Error(), "failed to sign commit")
+}

pkg/credentials/integration_test.go

@@ -92,3 +92,49 @@ func TestIntegration_SigningSuccess_WritesSigFile(t *testing.T) {
 	require.NoError(t, readErr)
 	assert.Equal(t, expectedSig, sigContent)
 }
+
+func TestIntegration_SignatureRequest_IncludesPublicKeyContent(t *testing.T) {
+	var receivedMessage string
+	mock := &mockTunnelClient{
+		gitSSHSignatureFunc: func(ctx context.Context, msg *tunnel.Message) (*tunnel.Message, error) {
+			receivedMessage = msg.Message
+			sig := gitsshsigning.GitSSHSignatureResponse{
+				Signature: []byte(
+					"-----BEGIN SSH SIGNATURE-----\ntest\n-----END SSH SIGNATURE-----\n",
+				),
+			}
+			jsonBytes, err := json.Marshal(sig)
+			if err != nil {
+				return nil, fmt.Errorf("marshal sig: %w", err)
+			}
+			return &tunnel.Message{Message: string(jsonBytes)}, nil
+		},
+	}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		err := handleGitSSHSignatureRequest(context.Background(), w, r, mock, log.Discard)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+		}
+	}))
+	defer server.Close()
+
+	gitsshsigning.SetSignatureServerURL(server.URL + "/git-ssh-signature")
+	t.Cleanup(func() { gitsshsigning.SetSignatureServerURL("") })
+
+	tmpDir := t.TempDir()
+	certFile := filepath.Join(tmpDir, "test_key.pub")
+	pubKeyContent := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAITest test@example.com"
+	require.NoError(t, os.WriteFile(certFile, []byte(pubKeyContent), 0o600))
+
+	bufferFile := filepath.Join(tmpDir, "buffer")
+	require.NoError(t, os.WriteFile(bufferFile, []byte("commit content"), 0o600))
+
+	err := gitsshsigning.HandleGitSSHProgramCall(certFile, "git", bufferFile, log.Discard)
+	require.NoError(t, err)
+
+	var req gitsshsigning.GitSSHSignatureRequest
+	require.NoError(t, json.Unmarshal([]byte(receivedMessage), &req))
+	assert.Equal(t, pubKeyContent, req.PublicKey)
+	assert.Equal(t, "commit content", req.Content)
+}

pkg/devcontainer/sshtunnel/sshtunnel.go

@@ -349,7 +349,15 @@ func establishSSHSession(
 }
 
 // setupSSHAgentForwarding configures SSH agent forwarding on the session.
-// Errors are returned to the caller rather than sent to a channel directly.
+//
+// Failures are logged but never fatal. This matches OpenSSH's behavior:
+//   - clientloop.c: client_request_agent() returns NULL on failure,
+//     sending SSH2_MSG_CHANNEL_OPEN_FAILURE without terminating the session.
+//   - ssh_config(5) ExitOnForwardFailure only covers "dynamic, tunnel,
+//     local, and remote port forwardings" — agent forwarding is excluded.
+//
+// Stale SSH_AUTH_SOCK is common in practice (tmux, screen, reconnected
+// terminals), so a fatal error here would break devpod up for many users.
 func setupSSHAgentForwarding(
 	ts *sshSessionTunnel,
 	sshClient *ssh.Client,
@@ -368,9 +376,9 @@ func setupSSHAgentForwarding(
 	}
 
 	if err != nil {
-		ts.opts.Log.Warnf("SSH agent forwarding failed: %v", err)
+		ts.opts.Log.Warnf("SSH agent forwarding failed (continuing without agent): %v", err)
 	}
-	return err
+	return nil
 }
 
 // runCommandInSSHTunnel runs the agent command over the SSH tunnel and returns

pkg/gitsshsigning/client.go

@@ -80,9 +80,18 @@ func writeSignatureToFile(signature []byte, bufferFile string, log log.Logger) e
 }
 
 func createSignatureRequestBody(content []byte, certPath string) ([]byte, error) {
+	var publicKey string
+	pubKeyData, readErr := os.ReadFile(
+		certPath,
+	) // #nosec G304 -- certPath comes from git config, not user input
+	if readErr == nil {
+		publicKey = strings.TrimSpace(string(pubKeyData))
+	}
+
 	request := &GitSSHSignatureRequest{
-		Content: string(content),
-		KeyPath: certPath,
+		Content:   string(content),
+		KeyPath:   certPath,
+		PublicKey: publicKey,
 	}
 	return json.Marshal(request)
 }

pkg/gitsshsigning/client_test.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/skevetter/log"
@@ -96,3 +98,27 @@ func TestRequestContentSignature_InvalidJSON(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "not json at all")
 }
+
+func TestCreateSignatureRequestBody_IncludesPublicKeyContent(t *testing.T) {
+	certPath := filepath.Join(t.TempDir(), "test_key.pub")
+	pubKeyContent := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAITest test@example.com"
+	require.NoError(t, os.WriteFile(certPath, []byte(pubKeyContent), 0o600))
+
+	body, err := createSignatureRequestBody([]byte("commit content"), certPath)
+	require.NoError(t, err)
+
+	var req GitSSHSignatureRequest
+	require.NoError(t, json.Unmarshal(body, &req))
+	assert.Equal(t, "commit content", req.Content)
+	assert.Equal(t, certPath, req.KeyPath)
+	assert.Equal(t, pubKeyContent, req.PublicKey)
+}
+
+func TestCreateSignatureRequestBody_MissingCertFile_OmitsPublicKey(t *testing.T) {
+	body, err := createSignatureRequestBody([]byte("commit content"), "/nonexistent/key.pub")
+	require.NoError(t, err)
+
+	var req GitSSHSignatureRequest
+	require.NoError(t, json.Unmarshal(body, &req))
+	assert.Empty(t, req.PublicKey)
+}

pkg/gitsshsigning/server.go

@@ -3,12 +3,14 @@ package gitsshsigning
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"os/exec"
 )
 
 type GitSSHSignatureRequest struct {
-	Content string
-	KeyPath string
+	Content   string
+	KeyPath   string
+	PublicKey string // Public key content; when set, written to a temp file for ssh-keygen
 }
 
 type GitSSHSignatureResponse struct {
@@ -18,22 +20,38 @@ type GitSSHSignatureResponse struct {
 // Sign signs the content using the private key and returns the signature.
 // This is intended to be a drop-in replacement for gpg.ssh.program for git,
 // so we simply execute ssh-keygen in the same way as git would do locally.
+//
+// When PublicKey is set, it is written to a temporary file that ssh-keygen
+// can read. This is necessary because the original KeyPath comes from
+// inside the container and does not exist on the host where Sign() runs.
 func (req *GitSSHSignatureRequest) Sign() (*GitSSHSignatureResponse, error) {
-	// Create a buffer to store the commit content
+	keyFile, cleanup, err := req.resolveKeyFile()
+	if err != nil {
+		return nil, fmt.Errorf("resolve signing key: %w", err)
+	}
+	defer cleanup()
+
 	var commitBuffer bytes.Buffer
 	commitBuffer.WriteString(req.Content)
 
-	// Create the command to run ssh-keygen
-	cmd := exec.Command("ssh-keygen", "-Y", "sign", "-f", req.KeyPath, "-n", "git")
+	//nolint:gosec // keyFile is a controlled temp path or validated KeyPath
+	cmd := exec.Command(
+		"ssh-keygen",
+		"-Y",
+		"sign",
+		"-f",
+		keyFile,
+		"-n",
+		"git",
+	)
 	cmd.Stdin = &commitBuffer
 
-	// Capture the output of the command
 	var out bytes.Buffer
 	var stderr bytes.Buffer
 	cmd.Stdout = &out
 	cmd.Stderr = &stderr
 
-	err := cmd.Run()
+	err = cmd.Run()
 	if err != nil {
 		return nil, fmt.Errorf("failed to sign commit: %w, stderr: %s", err, stderr.String())
 	}
@@ -42,3 +60,35 @@ func (req *GitSSHSignatureRequest) Sign() (*GitSSHSignatureResponse, error) {
 		Signature: out.Bytes(),
 	}, nil
 }
+
+// resolveKeyFile returns the path to use for ssh-keygen -f and a cleanup function.
+// When PublicKey content is available, it writes a temp file. Otherwise falls back to KeyPath.
+func (req *GitSSHSignatureRequest) resolveKeyFile() (string, func(), error) {
+	noop := func() {}
+
+	if req.PublicKey == "" {
+		return req.KeyPath, noop, nil
+	}
+
+	// ssh-keygen -Y sign -f <path> reads the public key directly from <path>
+	// to identify which SSH agent key to use for signing. We write the public
+	// key content to a temp file and pass that path to -f.
+	tmpFile, err := os.CreateTemp("", ".git_signing_key_*")
+	if err != nil {
+		return "", noop, fmt.Errorf("create temp key file: %w", err)
+	}
+	keyPath := tmpFile.Name()
+
+	if _, err := tmpFile.WriteString(req.PublicKey); err != nil {
+		_ = tmpFile.Close()
+		_ = os.Remove(keyPath)
+		return "", noop, fmt.Errorf("write public key: %w", err)
+	}
+	_ = tmpFile.Close()
+
+	cleanup := func() {
+		_ = os.Remove(keyPath)
+	}
+
+	return keyPath, cleanup, nil
+}

pkg/gitsshsigning/server_test.go

@@ -0,0 +1,31 @@
+package gitsshsigning
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSign_WithPublicKeyContent_WritesToTempFile(t *testing.T) {
+	req := &GitSSHSignatureRequest{
+		Content:   "tree abc123\nauthor Test <test@example.com>\n\ntest commit",
+		KeyPath:   "/tmp/.git_signing_key_does_not_exist",
+		PublicKey: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFakeKeyForTesting test@example.com",
+	}
+
+	_, err := req.Sign()
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), "/tmp/.git_signing_key_does_not_exist")
+}
+
+func TestSign_NonExistentKeyPath_ReturnsError(t *testing.T) {
+	req := &GitSSHSignatureRequest{
+		Content: "tree abc123\nauthor Test <test@example.com>\n\ntest commit",
+		KeyPath: "/tmp/.git_signing_key_does_not_exist",
+	}
+
+	_, err := req.Sign()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to sign commit")
+}