diff --git a/cmd/agent/container/credentials_server.go b/cmd/agent/container/credentials_server.go index c7aad53ef..a2123b9ef 100644 --- a/cmd/agent/container/credentials_server.go +++ b/cmd/agent/container/credentials_server.go @@ -135,21 +135,26 @@ func (cmd *CredentialsServerCmd) Run(ctx context.Context, port int) error { }(cmd.User) } - // configure git ssh signature helper + // configure git ssh signature helper — non-fatal so that a signing + // setup failure does not take down the entire credentials server + // (git/docker credential forwarding, port forwarding, etc.) if cmd.GitUserSigningKey != "" { decodedKey, err := base64.StdEncoding.DecodeString(cmd.GitUserSigningKey) if err != nil { - return fmt.Errorf("decode git ssh signature key: %w", err) - } - err = gitsshsigning.ConfigureHelper(cmd.User, string(decodedKey), log) - if err != nil { - return fmt.Errorf("configure git ssh signature helper: %w", err) + log.Errorf("Failed to decode git SSH signing key, signing will be unavailable: %v", err) + } else { + err = gitsshsigning.ConfigureHelper(cmd.User, string(decodedKey), log) + if err != nil { + log.Errorf( + "Failed to configure git SSH signature helper, signing will be unavailable: %v", + err, + ) + } else { + defer func(userName string) { + _ = gitsshsigning.RemoveHelper(userName) + }(cmd.User) + } } - - // cleanup when we are done - defer func(userName string) { - _ = gitsshsigning.RemoveHelper(userName) - }(cmd.User) } return credentials.RunCredentialsServer(ctx, port, tunnelClient, log) diff --git a/cmd/agent/git_ssh_signature_test.go b/cmd/agent/git_ssh_signature_test.go index 59a06bef0..6504c07e6 100644 --- a/cmd/agent/git_ssh_signature_test.go +++ b/cmd/agent/git_ssh_signature_test.go @@ -55,3 +55,26 @@ func (s *GitSSHSignatureTestSuite) TestParseDefaultsToSign() { assert.Equal(s.T(), "sign", result.command) assert.Equal(s.T(), "/tmp/buffer", result.bufferFile) } + +func (s *GitSSHSignatureTestSuite) TestParseEmptyArgs() { + result := parseSSHKeygenArgs([]string{}) + assert.Equal(s.T(), "sign", result.command) + assert.Equal(s.T(), "", result.bufferFile) + assert.Equal(s.T(), "", result.certPath) + assert.Equal(s.T(), "", result.namespace) +} + +func (s *GitSSHSignatureTestSuite) TestParseWithUFlag() { + // Git passes -U when using a literal SSH key value. The parser must + // still identify certPath and bufferFile with -U present. + args := []string{"-Y", "sign", "-n", "git", "-f", "/key.pub", "-U", "/tmp/buf"} + result := parseSSHKeygenArgs(args) + assert.Equal(s.T(), "/key.pub", result.certPath) + assert.Equal(s.T(), "/tmp/buf", result.bufferFile) +} + +func (s *GitSSHSignatureTestSuite) TestParseBufferFileWithSpaces() { + args := []string{"-Y", "sign", "-n", "git", "-f", "/key.pub", "/tmp/my buffer file"} + result := parseSSHKeygenArgs(args) + assert.Equal(s.T(), "/tmp/my buffer file", result.bufferFile) +} diff --git a/cmd/ssh.go b/cmd/ssh.go index e6371a84d..453473871 100644 --- a/cmd/ssh.go +++ b/cmd/ssh.go @@ -52,6 +52,7 @@ type SSHCmd struct { AgentForwarding bool GPGAgentForwarding bool GitSSHSignatureForwarding bool + GitSSHSigningKey string // ssh keepalive options SSHKeepAliveInterval time.Duration `json:"sshKeepAliveInterval,omitempty"` @@ -147,6 +148,9 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command { sshCmd.Flags(). DurationVar(&cmd.SSHKeepAliveInterval, "ssh-keepalive-interval", 55*time.Second, "How often should keepalive request be made (55s)") + sshCmd.Flags(). + StringVar(&cmd.GitSSHSigningKey, "git-ssh-signing-key", "", + "The SSH signing key to use for git commit signing inside the workspace") sshCmd.Flags().StringVar( &cmd.TermMode, "term-mode", @@ -517,6 +521,7 @@ func (cmd *SSHCmd) startTunnel( configureDockerCredentials, configureGitCredentials, configureGitSSHSignatureHelper, + cmd.GitSSHSigningKey, log, ) } @@ -652,6 +657,7 @@ func (cmd *SSHCmd) startServices( containerClient *ssh.Client, workspace *provider.Workspace, configureDockerCredentials, configureGitCredentials, configureGitSSHSignatureHelper bool, + gitSSHSigningKey string, log log.Logger, ) { if cmd.User != "" { @@ -668,6 +674,7 @@ func (cmd *SSHCmd) startServices( ConfigureDockerCredentials: configureDockerCredentials, ConfigureGitCredentials: configureGitCredentials, ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper, + GitSSHSigningKey: gitSSHSigningKey, Log: log, }, ) diff --git a/cmd/up.go b/cmd/up.go index b30f248fc..53edc923a 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -15,7 +15,6 @@ import ( "strings" "syscall" - "al.essio.dev/pkg/shellescape" "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/skevetter/devpod/cmd/flags" @@ -395,17 +394,6 @@ func (cmd *UpCmd) configureWorkspace( return err } - // Run after dotfiles so the signing config isn't overwritten by a - // dotfiles installer that replaces .gitconfig. - gitSSHSignatureEnabled := devPodConfig.ContextOption( - config.ContextOptionGitSSHSignatureForwarding, - ) == "true" - if cmd.GitSSHSigningKey != "" && gitSSHSignatureEnabled { - if err := setupGitSSHSignature(cmd.GitSSHSigningKey, client); err != nil { - return err - } - } - return nil } @@ -481,6 +469,7 @@ func (o *ideOpener) open( user, ideOptions, o.cmd.SSHAuthSockID, + o.cmd.GitSSHSigningKey, o.log, ) @@ -499,6 +488,7 @@ func (o *ideOpener) open( user, ideOptions, o.cmd.SSHAuthSockID, + o.cmd.GitSSHSigningKey, o.log, ) @@ -511,6 +501,7 @@ func (o *ideOpener) open( user, ideOptions, o.cmd.SSHAuthSockID, + o.cmd.GitSSHSigningKey, o.log, ) @@ -854,6 +845,7 @@ func startJupyterNotebookInBrowser( user string, ideOptions map[string]config.OptionValue, authSockID string, + gitSSHSigningKey string, logger log.Logger, ) error { if forwardGpg { @@ -900,6 +892,7 @@ func startJupyterNotebookInBrowser( false, extraPorts, authSockID, + gitSSHSigningKey, logger, ) } @@ -912,6 +905,7 @@ func startRStudioInBrowser( user string, ideOptions map[string]config.OptionValue, authSockID string, + gitSSHSigningKey string, logger log.Logger, ) error { if forwardGpg { @@ -957,6 +951,7 @@ func startRStudioInBrowser( false, extraPorts, authSockID, + gitSSHSigningKey, logger, ) } @@ -1005,6 +1000,7 @@ func startVSCodeInBrowser( workspaceFolder, user string, ideOptions map[string]config.OptionValue, authSockID string, + gitSSHSigningKey string, logger log.Logger, ) error { if forwardGpg { @@ -1054,6 +1050,7 @@ func startVSCodeInBrowser( forwardPorts, extraPorts, authSockID, + gitSSHSigningKey, logger, ) } @@ -1150,6 +1147,7 @@ func startBrowserTunnel( forwardPorts bool, extraPorts []string, authSockID string, + gitSSHSigningKey string, logger log.Logger, ) error { // Setup a backhaul SSH connection using the remote user so there is an AUTH SOCK to use @@ -1245,6 +1243,7 @@ func startBrowserTunnel( ConfigureDockerCredentials: configureDockerCredentials, ConfigureGitCredentials: configureGitCredentials, ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper, + GitSSHSigningKey: gitSSHSigningKey, Log: logger, }, ) @@ -1553,44 +1552,6 @@ func collectDotfilesScriptEnvKeyvaluePairs(envFiles []string) ([]string, error) return keyValues, nil } -func setupGitSSHSignature( - signingKey string, - client client2.BaseWorkspaceClient, -) error { - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - // #nosec G204 -- execPath is from os.Executable(), not user input - out, err := exec.Command( - execPath, - "ssh", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--command", - shellescape.QuoteCommand( - []string{config.BinaryName, "agent", "git-ssh-signature-helper", signingKey}, - ), - ).CombinedOutput() - if err != nil { - return fmt.Errorf("setup git ssh signature helper: %w, output: %s", err, string(out)) - } - return nil -} - func performGpgForwarding( client client2.BaseWorkspaceClient, log log.Logger, diff --git a/e2e/tests/ssh/ssh.go b/e2e/tests/ssh/ssh.go index 6fd728cf7..af03d2421 100644 --- a/e2e/tests/ssh/ssh.go +++ b/e2e/tests/ssh/ssh.go @@ -144,29 +144,15 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord err = f.DevPodUp(ctx, tempDir, "--git-ssh-signing-key", keyPath+".pub") framework.ExpectNoError(err) - // Step 1: Verify the helper script was installed and executable - out, err := f.DevPodSSH(ctx, tempDir, - "test -x /usr/local/bin/devpod-ssh-signature && echo EXISTS", - ) - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To( - gomega.Equal("EXISTS"), - "devpod-ssh-signature helper script should be installed and executable", - ) - - // Step 2: Verify git config was written correctly - out, err = f.DevPodSSH(ctx, tempDir, "git config --global gpg.ssh.program") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("devpod-ssh-signature")) - - out, err = f.DevPodSSH(ctx, tempDir, "git config --global gpg.format") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("ssh")) - - // Step 3: Attempt a signed commit with the credentials server - // tunnel active. The signing request is forwarded over the tunnel - // to the host where ssh-keygen performs the actual signing. + // Verify helper installation, git config, and a signed commit + // in a single SSH session with --start-services so the + // credentials server tunnel is active. The helper is installed + // asynchronously by the credentials server, so retry briefly. commitCmd := strings.Join([]string{ + "for i in $(seq 1 30); do test -x /usr/local/bin/devpod-ssh-signature && break; sleep 1; done", + "test -x /usr/local/bin/devpod-ssh-signature", + "test \"$(git config --global gpg.ssh.program)\" = devpod-ssh-signature", + "test \"$(git config --global gpg.format)\" = ssh", "cd /tmp", "git init test-sign-repo", "cd test-sign-repo", @@ -178,13 +164,19 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord "git commit -m 'signed test commit' 2>&1", }, " && ") - stdout, stderr, err := f.ExecCommandCapture(ctx, []string{ + // The signing key must be passed on each SSH invocation so the + // credentials server can configure the helper inside the container. + sshBase := []string{ "ssh", "--agent-forwarding", "--start-services", + "--git-ssh-signing-key", keyPath + ".pub", tempDir, - "--command", commitCmd, - }) + } + + stdout, stderr, err := f.ExecCommandCapture(ctx, + append(sshBase, "--command", commitCmd), + ) ginkgo.GinkgoWriter.Printf("commit stdout: %s\n", stdout) ginkgo.GinkgoWriter.Printf("commit stderr: %s\n", stderr) framework.ExpectNoError(err) @@ -194,9 +186,7 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord "git commit should succeed with the signed test commit message", ) - // Step 4: Verify the commit is actually signed with a valid SSH signature. - // Read the public key that was used for signing so we can build - // an allowed-signers file inside the workspace for verification. + // Verify the commit is signed with a valid SSH signature. pubKeyBytes, err := os.ReadFile( keyPath + ".pub", ) // #nosec G304 -- test file with controlled path @@ -205,20 +195,14 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord verifyCmd := strings.Join([]string{ "cd /tmp/test-sign-repo", - // Create allowed signers file mapping the test email to our public key "echo 'test@example.com " + pubKey + "' > /tmp/allowed_signers", "git config gpg.ssh.allowedSignersFile /tmp/allowed_signers", - // Verify the commit signature is valid "git verify-commit HEAD 2>&1", }, " && ") - stdout, stderr, err = f.ExecCommandCapture(ctx, []string{ - "ssh", - "--agent-forwarding", - "--start-services", - tempDir, - "--command", verifyCmd, - }) + stdout, stderr, err = f.ExecCommandCapture(ctx, + append(sshBase, "--command", verifyCmd), + ) ginkgo.GinkgoWriter.Printf("verify stdout: %s\n", stdout) ginkgo.GinkgoWriter.Printf("verify stderr: %s\n", stderr) framework.ExpectNoError(err) @@ -232,13 +216,9 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord // And confirm the signature log shows the correct principal logCmd := "cd /tmp/test-sign-repo && git log --show-signature -1 2>&1" - stdout, stderr, err = f.ExecCommandCapture(ctx, []string{ - "ssh", - "--agent-forwarding", - "--start-services", - tempDir, - "--command", logCmd, - }) + stdout, stderr, err = f.ExecCommandCapture(ctx, + append(sshBase, "--command", logCmd), + ) ginkgo.GinkgoWriter.Printf("log stdout: %s\n", stdout) ginkgo.GinkgoWriter.Printf("log stderr: %s\n", stderr) framework.ExpectNoError(err) diff --git a/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json b/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json index 278d342d0..0224b074c 100644 --- a/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json +++ b/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json @@ -1,4 +1,5 @@ { "name": "SSH Signing Test", - "image": "mcr.microsoft.com/devcontainers/go:1" + "image": "mcr.microsoft.com/devcontainers/go:1", + "forwardPorts": [8300, 7080] } diff --git a/pkg/credentials/server.go b/pkg/credentials/server.go index 2dfb17d8b..2c932d038 100644 --- a/pkg/credentials/server.go +++ b/pkg/credentials/server.go @@ -156,7 +156,14 @@ func handleGitSSHSignatureRequest( ) error { out, err := io.ReadAll(request.Body) if err != nil { - return fmt.Errorf("read request body: %w", err) + log.Errorf("error reading git SSH signature request body: %v", err) + writer.Header().Set("Content-Type", "application/json") + writer.WriteHeader(http.StatusInternalServerError) + errJSON, _ := json.Marshal( + map[string]string{"error": fmt.Sprintf("read request body: %v", err)}, + ) + _, _ = writer.Write(errJSON) + return nil } log.WithFields(logrus.Fields{"data": string(out)}).Debug("received git SSH signature post data") diff --git a/pkg/credentials/server_test.go b/pkg/credentials/server_test.go index 70beb8cdf..345626ffa 100644 --- a/pkg/credentials/server_test.go +++ b/pkg/credentials/server_test.go @@ -15,6 +15,11 @@ import ( "github.com/stretchr/testify/require" ) +// errReader is an io.Reader that always returns an error. +type errReader struct{ err error } + +func (e *errReader) Read([]byte) (int, error) { return 0, e.err } + func TestHandleGitSSHSignature_GRPCError_ReturnsJSON500(t *testing.T) { mock := &mockTunnelClient{ gitSSHSignatureFunc: func(ctx context.Context, msg *tunnel.Message) (*tunnel.Message, error) { @@ -42,6 +47,34 @@ func TestHandleGitSSHSignature_GRPCError_ReturnsJSON500(t *testing.T) { assert.Contains(t, body["error"], "Permission denied") } +func TestHandleGitSSHSignature_BodyReadError_ReturnsJSON500(t *testing.T) { + mock := &mockTunnelClient{ + gitSSHSignatureFunc: func(ctx context.Context, msg *tunnel.Message) (*tunnel.Message, error) { + t.Fatal("gRPC should not be called when body read fails") + return nil, nil + }, + } + + req := httptest.NewRequest( + http.MethodPost, + "/git-ssh-signature", + &errReader{err: fmt.Errorf("connection reset")}, + ) + w := httptest.NewRecorder() + + err := handleGitSSHSignatureRequest(context.Background(), w, req, mock, log.Discard) + require.NoError(t, err, "error should be written to response, not returned") + + resp := w.Result() + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) + + var body map[string]string + err = json.NewDecoder(resp.Body).Decode(&body) + require.NoError(t, err, "response body must be valid JSON") + assert.Contains(t, body["error"], "connection reset") +} + func TestHandleGitSSHSignature_GRPCSuccess_ReturnsJSON200(t *testing.T) { expectedMessage := `{"signature":"abc123"}` mock := &mockTunnelClient{ diff --git a/pkg/gitsshsigning/helper.go b/pkg/gitsshsigning/helper.go index 2267cb4c2..58b102e22 100644 --- a/pkg/gitsshsigning/helper.go +++ b/pkg/gitsshsigning/helper.go @@ -52,7 +52,7 @@ func ConfigureHelper(userName, gitSigningKey string, log log.Logger) error { } log.Debugf("Got config path: %v", gitConfigPath) if err := updateGitConfig(gitConfigPath, userName, gitSigningKey); err != nil { - log.Errorf("Failed updating git configuration: %w", err) + log.Errorf("Failed updating git configuration: %v", err) return err } @@ -110,12 +110,14 @@ func updateGitConfig(gitConfigPath, userName, gitSigningKey string) error { return err } - if !strings.Contains(configContent, "program = "+pkgconfig.SSHSignatureHelperName) { - newConfig := fmt.Sprintf(GitConfigTemplate, gitSigningKey) - newContent := removeSignatureHelper(configContent) + newConfig - if err := writeGitConfig(gitConfigPath, newContent, userName); err != nil { - return err - } + // Always remove any existing devpod-managed signing config and rewrite + // with the current key. The previous guard (checking whether the program + // line already existed) would silently skip key updates after unclean + // shutdowns or key rotations. + newConfig := fmt.Sprintf(GitConfigTemplate, gitSigningKey) + newContent := removeSignatureHelper(configContent) + newConfig + if err := writeGitConfig(gitConfigPath, newContent, userName); err != nil { + return err } return nil @@ -151,40 +153,76 @@ func removeGitConfigHelper(gitConfigPath, userName string) error { } func removeSignatureHelper(content string) string { - inGpgSSHSection := false - inGpgSection := false - var gpgSSHBuffer []string + type sectionKind int + const ( + sectionNone sectionKind = iota + sectionGpgSSH + sectionGpg + sectionUser + ) + + current := sectionNone + var buf []string var out []string + flush := func() { + switch current { + case sectionGpgSSH: + out = append(out, filterSection(buf, func(trimmed string) bool { + return strings.HasPrefix(trimmed, "program = "+pkgconfig.SSHSignatureHelperName) + })...) + case sectionGpg: + out = append(out, filterSection(buf, func(trimmed string) bool { + return strings.HasPrefix(trimmed, "format = ssh") + })...) + case sectionUser: + // Only strip [user] sections that contain nothing but signingkey + // entries — these are the ones appended by GitConfigTemplate. + // Sections with other entries (name, email, etc.) are user-owned + // and must be preserved intact to avoid data loss. + if isDevpodOnlyUserSection(buf) { + // Drop the entire section — it was appended by devpod. + } else { + out = append(out, buf...) + } + } + buf = nil + } + for line := range strings.Lines(content) { line = strings.TrimRight(line, "\n") trimmed := strings.TrimSpace(line) if isSectionHeader(trimmed) { - if inGpgSSHSection { - out = append(out, filterGpgSSHSection(gpgSSHBuffer)...) - gpgSSHBuffer = nil + if current != sectionNone { + flush() + } + switch trimmed { + case `[gpg "ssh"]`: + current = sectionGpgSSH + case "[gpg]": + current = sectionGpg + case "[user]": + current = sectionUser + default: + current = sectionNone } - inGpgSSHSection = trimmed == `[gpg "ssh"]` - inGpgSection = trimmed == "[gpg]" - if inGpgSSHSection { - gpgSSHBuffer = append(gpgSSHBuffer, line) + if current != sectionNone { + buf = append(buf, line) continue } } - if inGpgSSHSection { - gpgSSHBuffer = append(gpgSSHBuffer, line) + if current != sectionNone { + buf = append(buf, line) continue } - if !isDevpodManagedGpgKey(inGpgSection, trimmed) { - out = append(out, line) - } + out = append(out, line) } - if inGpgSSHSection { - out = append(out, filterGpgSSHSection(gpgSSHBuffer)...) + if current != sectionNone { + flush() } return strings.Join(out, "\n") @@ -194,23 +232,33 @@ func isSectionHeader(trimmed string) bool { return len(trimmed) > 0 && trimmed[0] == '[' } -func isDevpodManagedGpgKey(inGpgSection bool, trimmed string) bool { - if !inGpgSection || len(trimmed) == 0 || trimmed[0] == '[' { - return false +// isDevpodOnlyUserSection returns true when a buffered [user] section contains +// nothing but signingkey entries. Such sections are appended by GitConfigTemplate +// and are safe to remove. Sections with other entries (name, email, etc.) belong +// to the user and must be preserved. +func isDevpodOnlyUserSection(lines []string) bool { + for _, line := range lines[1:] { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + if !strings.HasPrefix(trimmed, "signingkey = ") { + return false + } } - return strings.HasPrefix(trimmed, "format = ssh") + return true } -// filterGpgSSHSection removes devpod-managed keys from a buffered [gpg "ssh"] -// section. Returns the header + remaining user keys, or nil if no user keys remain. -func filterGpgSSHSection(lines []string) []string { +// filterSection removes lines matching the predicate from a buffered section. +// Returns the header + remaining lines, or nil if no lines remain after filtering. +func filterSection(lines []string, isManaged func(string) bool) []string { if len(lines) == 0 { return nil } var kept []string for _, line := range lines[1:] { trimmed := strings.TrimSpace(line) - if !strings.HasPrefix(trimmed, "program = "+pkgconfig.SSHSignatureHelperName) { + if !isManaged(trimmed) { kept = append(kept, line) } } diff --git a/pkg/gitsshsigning/helper_test.go b/pkg/gitsshsigning/helper_test.go index e76e8bb1e..565890dc6 100644 --- a/pkg/gitsshsigning/helper_test.go +++ b/pkg/gitsshsigning/helper_test.go @@ -1,10 +1,13 @@ package gitsshsigning import ( + "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -75,6 +78,68 @@ func (s *HelperTestSuite) TestRemoveSignatureHelper_PreservesUserOwnedGpgSSHKeys assert.Contains(s.T(), result, "[commit]") } +func (s *HelperTestSuite) TestRemoveSignatureHelper_PreservesUserOwnedSigningKey() { + // A [user] section with name + signingkey is user-owned; only the + // devpod-appended [user] section (signingkey-only) should be dropped. + input := strings.Join([]string{ + "[user]", "\tname = Test User", "\tsigningkey = /my/gpg-key", + `[gpg "ssh"]`, "\tprogram = devpod-ssh-signature", + "[gpg]", "\tformat = ssh", + "[user]", "\tsigningkey = /devpod/injected-key", + }, "\n") + + result := removeSignatureHelper(input) + + assert.Contains(s.T(), result, "signingkey = /my/gpg-key", + "user-owned signingkey must be preserved") + assert.NotContains(s.T(), result, "/devpod/injected-key", + "devpod-only [user] section should be removed") + assert.Contains(s.T(), result, "Test User") +} + +func TestUpdateGitConfig_Idempotent(t *testing.T) { + dir := t.TempDir() + gitConfigPath := filepath.Join(dir, ".gitconfig") + + // First call: writes signing config + err := updateGitConfig(gitConfigPath, "", "/path/to/key.pub") + require.NoError(t, err) + + content1, err := os.ReadFile(gitConfigPath) // #nosec G304 -- test path from t.TempDir + require.NoError(t, err) + assert.Contains(t, string(content1), "program = devpod-ssh-signature") + assert.Contains(t, string(content1), "signingkey = /path/to/key.pub") + + // Second call with same config: should be a no-op + err = updateGitConfig(gitConfigPath, "", "/path/to/key.pub") + require.NoError(t, err) + + content2, err := os.ReadFile(gitConfigPath) // #nosec G304 -- test path from t.TempDir + require.NoError(t, err) + assert.Equal(t, string(content1), string(content2), "second call should not duplicate config") +} + +func TestUpdateGitConfig_DifferentKey(t *testing.T) { + dir := t.TempDir() + gitConfigPath := filepath.Join(dir, ".gitconfig") + + // First call with key A + err := updateGitConfig(gitConfigPath, "", "/path/to/keyA.pub") + require.NoError(t, err) + + // Second call with key B: should update to the new key + err = updateGitConfig(gitConfigPath, "", "/path/to/keyB.pub") + require.NoError(t, err) + + content, err := os.ReadFile(gitConfigPath) // #nosec G304 -- test path from t.TempDir + require.NoError(t, err) + assert.Contains(t, string(content), "program = devpod-ssh-signature") + assert.Contains(t, string(content), "signingkey = /path/to/keyB.pub", + "key should be updated to the new value") + assert.NotContains(t, string(content), "keyA", + "old key should be replaced") +} + func (s *HelperTestSuite) TestRemoveSignatureHelper_DropsEmptyGpgSSHSection() { input := strings.Join([]string{ "[user]", "\tname = Test User", diff --git a/pkg/tunnel/services.go b/pkg/tunnel/services.go index 5f33519f4..0f9a21da3 100644 --- a/pkg/tunnel/services.go +++ b/pkg/tunnel/services.go @@ -51,6 +51,7 @@ type RunServicesOptions struct { ConfigureDockerCredentials bool ConfigureGitCredentials bool ConfigureGitSSHSignatureHelper bool + GitSSHSigningKey string Log log.Logger } @@ -104,16 +105,24 @@ func runTunnelServer(ctx context.Context, cancel context.CancelFunc, p tunnelSer } // addGitSSHSigningKey adds SSH signing key to command if configured. -func addGitSSHSigningKey(command string, log log.Logger) string { - format, userSigningKey, err := gitsshsigning.ExtractGitConfiguration() - if err != nil { - log.Debugf("failed to extract git configuration: %v", err) - return command - } - if format == gitsshsigning.GPGFormatSSH && userSigningKey != "" { - encodedKey := base64.StdEncoding.EncodeToString([]byte(userSigningKey)) - command += fmt.Sprintf(" --git-user-signing-key %s", encodedKey) +// When explicitKey is set (from --git-ssh-signing-key flag), it takes +// precedence over the host's .gitconfig. This ensures signing works +// even when user.signingkey is not in the host git configuration. +func addGitSSHSigningKey(command string, explicitKey string, log log.Logger) string { + userSigningKey := explicitKey + if userSigningKey == "" { + format, extracted, err := gitsshsigning.ExtractGitConfiguration() + if err != nil { + log.Debugf("failed to extract git configuration: %v", err) + return command + } + if format != gitsshsigning.GPGFormatSSH || extracted == "" { + return command + } + userSigningKey = extracted } + encodedKey := base64.StdEncoding.EncodeToString([]byte(userSigningKey)) + command += fmt.Sprintf(" --git-user-signing-key %s", encodedKey) return command } @@ -128,7 +137,7 @@ func buildCredentialsCommand(opts RunServicesOptions) string { command += " --configure-git-helper" } if opts.ConfigureGitSSHSignatureHelper { - command = addGitSSHSigningKey(command, opts.Log) + command = addGitSSHSigningKey(command, opts.GitSSHSigningKey, opts.Log) } if opts.ConfigureDockerCredentials { command += " --configure-docker-helper" diff --git a/pkg/tunnel/services_test.go b/pkg/tunnel/services_test.go new file mode 100644 index 000000000..ee06a5af8 --- /dev/null +++ b/pkg/tunnel/services_test.go @@ -0,0 +1,68 @@ +package tunnel + +import ( + "encoding/base64" + "testing" + + "github.com/skevetter/log" + "github.com/stretchr/testify/assert" +) + +const testBaseCommand = "devpod agent container credentials-server --user root" + +func TestAddGitSSHSigningKey_ExplicitKey(t *testing.T) { + command := testBaseCommand + result := addGitSSHSigningKey(command, "/path/to/key.pub", log.Discard) + + encoded := base64.StdEncoding.EncodeToString([]byte("/path/to/key.pub")) + assert.Equal(t, command+" --git-user-signing-key "+encoded, result) +} + +func TestAddGitSSHSigningKey_ExplicitKeyTakesPrecedence(t *testing.T) { + // When an explicit key is provided, it should be used regardless + // of what ExtractGitConfiguration might return from host .gitconfig. + command := testBaseCommand + explicitKey := "/explicit/key.pub" + result := addGitSSHSigningKey(command, explicitKey, log.Discard) + + encoded := base64.StdEncoding.EncodeToString([]byte(explicitKey)) + assert.Equal(t, command+" --git-user-signing-key "+encoded, result) +} + +func TestAddGitSSHSigningKey_EmptyExplicitKey_FallsBackToHostConfig(t *testing.T) { + // Ensure deterministic environment with no host git signing config. + command := testBaseCommand + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("XDG_CONFIG_HOME", tmpHome) + + result := addGitSSHSigningKey(command, "", log.Discard) + + assert.Equal(t, command, result) + assert.NotContains(t, result, "--git-user-signing-key") +} + +func TestBuildCredentialsCommand_IncludesSigningKey(t *testing.T) { + opts := RunServicesOptions{ + User: "testuser", + ConfigureGitSSHSignatureHelper: true, + GitSSHSigningKey: "/my/key.pub", + Log: log.Discard, + } + command := buildCredentialsCommand(opts) + + encoded := base64.StdEncoding.EncodeToString([]byte("/my/key.pub")) + assert.Contains(t, command, "--git-user-signing-key "+encoded) + assert.Contains(t, command, "--user testuser") +} + +func TestBuildCredentialsCommand_NoSigningKey(t *testing.T) { + opts := RunServicesOptions{ + User: "testuser", + ConfigureGitSSHSignatureHelper: false, + Log: log.Discard, + } + command := buildCredentialsCommand(opts) + + assert.NotContains(t, command, "--git-user-signing-key") +}