Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a1ee722
test: add Sign() unit tests exposing container-path bug (#645)
skevetter Apr 11, 2026
5f26abb
fix: write public key to temp file on host before signing (#645)
skevetter Apr 11, 2026
7f870f9
fix: include public key content in signature request (#645)
skevetter Apr 11, 2026
9b1797a
test: verify public key content flows through tunnel (#645)
skevetter Apr 11, 2026
831a800
test: add tunnelserver GitSSHSignature key resolution test (#645)
skevetter Apr 11, 2026
e042aaf
test(e2e): verify SSH signing works with file-path signingkey (#645)
skevetter Apr 11, 2026
98d077f
fix: pass public key path directly to ssh-keygen (#645)
skevetter Apr 11, 2026
a7d8427
fix(e2e): get public key from SSH agent for file-path signingkey test
skevetter Apr 11, 2026
e3f3dbf
fix(e2e): add generated signing key to SSH agent for forwarding
skevetter Apr 11, 2026
3aee8b9
fix(e2e): start SSH agent before adding signing key
skevetter Apr 11, 2026
49eab8f
fix(e2e): unset SSH_AUTH_SOCK after test to prevent stale socket
skevetter Apr 11, 2026
e482e4f
refactor(e2e): use GinkgoT().Setenv for automatic env restoration
skevetter Apr 11, 2026
7e80c36
fix: make SSH agent forwarding failure non-fatal
skevetter Apr 11, 2026
1fed45d
docs: document OpenSSH precedent for non-fatal agent forwarding
skevetter Apr 11, 2026
3c78c11
chore: clean up verbose inline comments
skevetter Apr 12, 2026
f36a6b8
chore: remove obvious inline comments
skevetter Apr 12, 2026
9c1fb23
fix: address review comments
skevetter Apr 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions e2e/tests/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,32 @@ 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()
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]
t.Setenv(key, val)
}
}
}
ginkgo.DeferCleanup(func(_ context.Context) {
if pid := os.Getenv("SSH_AGENT_PID"); pid != "" {
// #nosec G204 -- controlled pid from ssh-agent we started
_ = exec.Command("kill", pid).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)
Expand All @@ -162,6 +188,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
Expand Down
33 changes: 33 additions & 0 deletions pkg/agent/tunnelserver/tunnelserver_gitssh_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
43 changes: 43 additions & 0 deletions pkg/credentials/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,46 @@ 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, _ := json.Marshal(sig)
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)
}
14 changes: 11 additions & 3 deletions pkg/devcontainer/sshtunnel/sshtunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
13 changes: 11 additions & 2 deletions pkg/gitsshsigning/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/gitsshsigning/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"

"github.com/skevetter/log"
Expand Down Expand Up @@ -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)
}
64 changes: 57 additions & 7 deletions pkg/gitsshsigning/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
}
Expand All @@ -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
}
31 changes: 31 additions & 0 deletions pkg/gitsshsigning/server_test.go
Original file line number Diff line number Diff line change
@@ -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")
}