diff --git a/cmd/agent/git_ssh_signature.go b/cmd/agent/git_ssh_signature.go index cbed6c1e4..3eb34ca36 100644 --- a/cmd/agent/git_ssh_signature.go +++ b/cmd/agent/git_ssh_signature.go @@ -32,8 +32,13 @@ func NewGitSSHSignatureCmd(flags *flags.GlobalFlags) *cobra.Command { GlobalFlags: flags, } - gitSshSignatureCmd := &cobra.Command{ + gitSSHSignatureCmd := &cobra.Command{ Use: "git-ssh-signature", + // Allow unknown flags so that git can pass any ssh-keygen flags + // (e.g. -U for stdin input) without cobra rejecting them. + FParseErrWhitelist: cobra.FParseErrWhitelist{ + UnknownFlags: true, + }, RunE: func(_ *cobra.Command, args []string) error { logger := log.GetInstance() @@ -54,10 +59,10 @@ func NewGitSSHSignatureCmd(flags *flags.GlobalFlags) *cobra.Command { }, } - gitSshSignatureCmd.Flags().StringVarP(&cmd.CertPath, "file", "f", "", "Path to the private key") - gitSshSignatureCmd.Flags().StringVarP(&cmd.Namespace, "namespace", "n", "", "Namespace") - gitSshSignatureCmd.Flags(). + gitSSHSignatureCmd.Flags().StringVarP(&cmd.CertPath, "file", "f", "", "Path to the private key") + gitSSHSignatureCmd.Flags().StringVarP(&cmd.Namespace, "namespace", "n", "", "Namespace") + gitSSHSignatureCmd.Flags(). StringVarP(&cmd.Command, "command", "Y", "sign", "Command - should be 'sign'") - return gitSshSignatureCmd + return gitSSHSignatureCmd } diff --git a/cmd/agent/git_ssh_signature_test.go b/cmd/agent/git_ssh_signature_test.go new file mode 100644 index 000000000..39e982b8d --- /dev/null +++ b/cmd/agent/git_ssh_signature_test.go @@ -0,0 +1,76 @@ +package agent + +import ( + "testing" + + "github.com/skevetter/devpod/cmd/flags" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" +) + +type GitSSHSignatureTestSuite struct { + suite.Suite +} + +func TestGitSSHSignatureSuite(t *testing.T) { + suite.Run(t, new(GitSSHSignatureTestSuite)) +} + +func (s *GitSSHSignatureTestSuite) TestAcceptsUnknownFlags() { + cmd := NewGitSSHSignatureCmd(&flags.GlobalFlags{}) + + // Git passes: -Y sign -n git -f /path/to/key -U /dev/stdin /tmp/buffer + // -U is an unknown flag that consumes /dev/stdin as its value. + // /tmp/buffer remains as a positional argument. + err := cmd.ParseFlags( + []string{ + "-Y", + "sign", + "-n", + "git", + "-f", + "/path/to/key", + "-U", + "/dev/stdin", + "/tmp/buffer", + }, + ) + assert.NoError(s.T(), err, "flag parsing should succeed with unknown flag -U") + + args := cmd.Flags().Args() + s.Require().NotEmpty(args, "should have positional args") + assert.Equal(s.T(), "/tmp/buffer", args[len(args)-1], + "buffer file should be preserved as last positional arg") +} + +func (s *GitSSHSignatureTestSuite) TestBufferFileAsPositionalArg() { + cmd := NewGitSSHSignatureCmd(&flags.GlobalFlags{}) + + err := cmd.ParseFlags( + []string{"-Y", "sign", "-n", "git", "-f", "/path/to/key", "/tmp/buffer"}, + ) + assert.NoError(s.T(), err) + + args := cmd.Flags().Args() + s.Require().NotEmpty(args, "should have positional args") + assert.Equal(s.T(), "/tmp/buffer", args[len(args)-1], + "last positional arg should be the buffer file") +} + +func (s *GitSSHSignatureTestSuite) TestKnownFlagsParsed() { + cmd := NewGitSSHSignatureCmd(&flags.GlobalFlags{}) + + err := cmd.ParseFlags( + []string{"-Y", "sign", "-n", "git", "-f", "/path/to/key", "/tmp/buffer"}, + ) + assert.NoError(s.T(), err) + + val, err := cmd.Flags().GetString("command") + assert.NoError(s.T(), err) + assert.Equal(s.T(), "sign", val, "command flag should be 'sign'") + + args := cmd.Flags().Args() + s.Require().NotEmpty(args, "should have positional args") + assert.Equal(s.T(), "/tmp/buffer", args[len(args)-1], + "last positional arg should be the buffer file") +} diff --git a/cmd/up.go b/cmd/up.go index 43dd465ed..deb7d9c90 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -15,6 +15,7 @@ import ( "strings" "syscall" + "al.essio.dev/pkg/shellescape" "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/skevetter/devpod/cmd/flags" @@ -382,13 +383,7 @@ func (cmd *UpCmd) configureWorkspace( log.Info("SSH configuration completed in workspace") } - if cmd.GitSSHSigningKey != "" { - if err := setupGitSSHSignature(cmd.GitSSHSigningKey, client, log); err != nil { - return err - } - } - - return setupDotfiles( + if err := setupDotfiles( cmd.DotfilesSource, cmd.DotfilesScript, cmd.DotfilesScriptEnvFile, @@ -396,7 +391,19 @@ func (cmd *UpCmd) configureWorkspace( client, devPodConfig, log, - ) + ); err != nil { + return err + } + + // Run after dotfiles so the signing config isn't overwritten by a + // dotfiles installer that replaces .gitconfig. + if cmd.GitSSHSigningKey != "" { + if err := setupGitSSHSignature(cmd.GitSSHSigningKey, client); err != nil { + return err + } + } + + return nil } // openIDE opens the configured IDE. @@ -1539,7 +1546,6 @@ func collectDotfilesScriptEnvKeyvaluePairs(envFiles []string) ([]string, error) func setupGitSSHSignature( signingKey string, client client2.BaseWorkspaceClient, - log log.Logger, ) error { execPath, err := os.Executable() if err != nil { @@ -1555,7 +1561,8 @@ func setupGitSSHSignature( remoteUser = "root" } - err = exec.Command( + // #nosec G204 -- execPath is from os.Executable(), not user input + out, err := exec.Command( execPath, "ssh", "--agent-forwarding=true", @@ -1565,10 +1572,13 @@ func setupGitSSHSignature( "--context", client.Context(), client.Workspace(), - "--command", fmt.Sprintf("devpod agent git-ssh-signature-helper %s", signingKey), - ).Run() + "--command", + shellescape.QuoteCommand( + []string{"devpod", "agent", "git-ssh-signature-helper", signingKey}, + ), + ).CombinedOutput() if err != nil { - log.Error("failure in setting up git ssh signature helper") + return fmt.Errorf("setup git ssh signature helper: %w, output: %s", err, string(out)) } return nil } diff --git a/e2e/tests/ssh/ssh.go b/e2e/tests/ssh/ssh.go index 0c15976f8..c39995fda 100644 --- a/e2e/tests/ssh/ssh.go +++ b/e2e/tests/ssh/ssh.go @@ -9,6 +9,7 @@ import ( "path/filepath" "runtime" "strconv" + "strings" "time" "github.com/onsi/ginkgo/v2" @@ -104,6 +105,81 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord // framework.ExpectNoError(err) // }) + ginkgo.It( + "should set up git SSH signature helper in workspace", + func(ctx context.Context) { + if runtime.GOOS == "windows" { + ginkgo.Skip("skipping on windows") + } + + tempDir, err := framework.CopyToTempDir("tests/ssh/testdata/ssh-signing") + framework.ExpectNoError(err) + + f := framework.NewDefaultFramework(initialDir + "/bin") + _ = f.DevPodProviderAdd(ctx, "docker") + err = f.DevPodProviderUse(ctx, "docker") + framework.ExpectNoError(err) + + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + _ = f.DevPodWorkspaceDelete(cleanupCtx, tempDir) + framework.CleanupTempDir(initialDir, tempDir) + }) + + // Generate a temporary SSH key for signing + sshKeyDir, err := os.MkdirTemp("", "devpod-ssh-signing-test") + framework.ExpectNoError(err) + defer func() { _ = os.RemoveAll(sshKeyDir) }() + + keyPath := filepath.Join(sshKeyDir, "id_ed25519") + // #nosec G204 -- test command with controlled arguments + err = exec.Command( + "ssh-keygen", "-t", "ed25519", "-f", keyPath, "-N", "", "-q", + ).Run() + framework.ExpectNoError(err) + + // Start workspace with git-ssh-signing-key flag + devpodUpCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + err = f.DevPodUp(devpodUpCtx, tempDir, + "--git-ssh-signing-key", keyPath+".pub", + ) + framework.ExpectNoError(err) + + sshCtx, cancelSSH := context.WithTimeout(ctx, 20*time.Second) + defer cancelSSH() + + // Verify the helper script was installed + out, err := f.DevPodSSH(sshCtx, 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", + ) + + // Verify git config has the SSH signing program set + out, err = f.DevPodSSH(sshCtx, tempDir, + "git config --global gpg.ssh.program", + ) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To( + gomega.Equal("devpod-ssh-signature"), + "git gpg.ssh.program should be set to devpod-ssh-signature", + ) + + // Verify git config has gpg format set to ssh + out, err = f.DevPodSSH(sshCtx, tempDir, + "git config --global gpg.format", + ) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To( + gomega.Equal("ssh"), + "git gpg.format should be set to ssh", + ) + }, + ) + ginkgo.It( "should start a new workspace with a docker provider (default) and forward a port into it", func(ctx context.Context) { diff --git a/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json b/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json new file mode 100644 index 000000000..278d342d0 --- /dev/null +++ b/e2e/tests/ssh/testdata/ssh-signing/devcontainer.json @@ -0,0 +1,4 @@ +{ + "name": "SSH Signing Test", + "image": "mcr.microsoft.com/devcontainers/go:1" +} diff --git a/pkg/gitsshsigning/helper.go b/pkg/gitsshsigning/helper.go index 2abee4e29..2d7f1962a 100644 --- a/pkg/gitsshsigning/helper.go +++ b/pkg/gitsshsigning/helper.go @@ -10,7 +10,6 @@ import ( "github.com/skevetter/devpod/pkg/command" "github.com/skevetter/devpod/pkg/file" "github.com/skevetter/log" - "github.com/skevetter/log/scanner" ) const ( @@ -150,27 +149,71 @@ func removeGitConfigHelper(gitConfigPath, userName string) error { } func removeSignatureHelper(content string) string { - scan := scanner.NewScanner(strings.NewReader(content)) - isGpgSetup := false - out := []string{} - - for scan.Scan() { - line := scan.Text() - if strings.TrimSpace(line) == "[gpg \"ssh\"]" { - isGpgSetup = true - continue - } else if strings.TrimSpace(line) == "[gpg]" { - isGpgSetup = true - } else if isGpgSetup { - trimmed := strings.TrimSpace(line) - if len(trimmed) > 0 && trimmed[0] == '[' { - isGpgSetup = false - } else { + inGpgSSHSection := false + inGpgSection := false + var gpgSSHBuffer []string + var out []string + + 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 + } + inGpgSSHSection = trimmed == `[gpg "ssh"]` + inGpgSection = trimmed == "[gpg]" + if inGpgSSHSection { + gpgSSHBuffer = append(gpgSSHBuffer, line) continue } } - out = append(out, line) + + if inGpgSSHSection { + gpgSSHBuffer = append(gpgSSHBuffer, line) + continue + } + + if !isDevpodManagedGpgKey(inGpgSection, trimmed) { + out = append(out, line) + } + } + + if inGpgSSHSection { + out = append(out, filterGpgSSHSection(gpgSSHBuffer)...) } return strings.Join(out, "\n") } + +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 + } + return strings.HasPrefix(trimmed, "format = ssh") +} + +// 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 { + if len(lines) == 0 { + return nil + } + var kept []string + for _, line := range lines[1:] { + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, "program = devpod-ssh-signature") { + kept = append(kept, line) + } + } + if len(kept) == 0 { + return nil + } + return append([]string{lines[0]}, kept...) +} diff --git a/pkg/gitsshsigning/helper_test.go b/pkg/gitsshsigning/helper_test.go new file mode 100644 index 000000000..e76e8bb1e --- /dev/null +++ b/pkg/gitsshsigning/helper_test.go @@ -0,0 +1,91 @@ +package gitsshsigning + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" +) + +type HelperTestSuite struct { + suite.Suite +} + +func TestHelperSuite(t *testing.T) { + suite.Run(t, new(HelperTestSuite)) +} + +func (s *HelperTestSuite) TestRemoveSignatureHelper_PreservesUnrelatedGpgConfig() { + input := strings.Join([]string{ + "[user]", "\tname = Test User", "\temail = test@example.com", + `[gpg "ssh"]`, "\tprogram = devpod-ssh-signature", + "[gpg]", "\tformat = ssh", "\tprogram = /usr/bin/gpg2", + "[commit]", "\tgpgsign = true", + "[user]", "\tsigningkey = /path/to/key", + }, "\n") + + result := removeSignatureHelper(input) + + assert.NotContains(s.T(), result, "devpod-ssh-signature") + assert.Contains(s.T(), result, "[user]") + assert.Contains(s.T(), result, "[commit]") + assert.Contains(s.T(), result, "program = /usr/bin/gpg2") + assert.NotContains(s.T(), result, "format = ssh") +} + +func (s *HelperTestSuite) TestRemoveSignatureHelper_RemovesDevpodSections() { + input := strings.Join([]string{ + "[user]", "\tname = Test User", + `[gpg "ssh"]`, "\tprogram = devpod-ssh-signature", + "[gpg]", "\tformat = ssh", + "[user]", "\tsigningkey = /path/to/key", + }, "\n") + + result := removeSignatureHelper(input) + + assert.NotContains(s.T(), result, "devpod-ssh-signature") + assert.NotContains(s.T(), result, "format = ssh") + assert.Contains(s.T(), result, "Test User") +} + +func (s *HelperTestSuite) TestRemoveSignatureHelper_NoGpgSections() { + input := "[user]\n\tname = Test User\n\temail = test@example.com" + + result := removeSignatureHelper(input) + + assert.Equal(s.T(), input, result) +} + +func (s *HelperTestSuite) TestRemoveSignatureHelper_PreservesUserOwnedGpgSSHKeys() { + input := strings.Join([]string{ + "[user]", "\tname = Test User", + `[gpg "ssh"]`, "\tprogram = devpod-ssh-signature", + "\tallowedSignersFile = ~/.ssh/allowed_signers", + "[commit]", "\tgpgsign = true", + }, "\n") + + result := removeSignatureHelper(input) + + assert.NotContains(s.T(), result, "devpod-ssh-signature") + assert.Contains(s.T(), result, `[gpg "ssh"]`, + "section header should be preserved when user keys remain") + assert.Contains(s.T(), result, "allowedSignersFile", + "user-owned key should be preserved") + assert.Contains(s.T(), result, "[commit]") +} + +func (s *HelperTestSuite) TestRemoveSignatureHelper_DropsEmptyGpgSSHSection() { + input := strings.Join([]string{ + "[user]", "\tname = Test User", + `[gpg "ssh"]`, "\tprogram = devpod-ssh-signature", + "[commit]", "\tgpgsign = true", + }, "\n") + + result := removeSignatureHelper(input) + + assert.NotContains(s.T(), result, "devpod-ssh-signature") + assert.NotContains(s.T(), result, `[gpg "ssh"]`, + "empty section should be dropped entirely") + assert.Contains(s.T(), result, "[commit]") +}