fix(agent/git_ssh_signature): ssh signature forwarding fails when signing (#648)

Author: skevetter

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
 }

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")
+}

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,21 +383,27 @@ 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,
 		cmd.DotfilesScriptEnv,
 		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
 }

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) {

e2e/tests/ssh/testdata/ssh-signing/devcontainer.json

@@ -0,0 +1,4 @@
+{
+  "name": "SSH Signing Test",
+  "image": "mcr.microsoft.com/devcontainers/go:1"
+}

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...)
+}