Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 10 additions & 5 deletions cmd/agent/git_ssh_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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
}
76 changes: 76 additions & 0 deletions cmd/agent/git_ssh_signature_test.go
Original file line number Diff line number Diff line change
@@ -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")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

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")
}
36 changes: 23 additions & 13 deletions cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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",
Expand All @@ -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},
),
Comment on lines +1575 to +1578
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

This calls the runtime helper with the wrong argv shape.

cmd/agent/git_ssh_signature.go treats the last positional argument as the buffer file for a Git-invoked sign operation; it never reads a setup-time signing key from args. With signingKey appended here, devpod up --git-ssh-signing-key=... will try to sign signingKey as if it were the buffer file, so the helper setup fails before any Git config is installed. Please route the key through the installer path/flag that persists user.signingkey, or extend the helper command to accept an explicit signing-key argument for setup.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/up.go` around lines 1575 - 1578, The helper is being invoked with the
signingKey as the last positional argv which git-ssh-signature.go expects to be
the buffer file; remove the appended signingKey from the
shellescape.QuoteCommand call that builds the "devpod agent
git-ssh-signature-helper" invocation in cmd/up.go, and instead persist the key
via the installer path/flag that sets user.signingkey (or add an explicit setup
flag to the helper). Concretely: stop passing signingKey as the final positional
argument in the argv construction in cmd/up.go, and either (A) call the
installer code/path that writes user.signingkey into config, or (B) extend the
helper command (cmd/agent/git_ssh_signature.go) to accept and parse an explicit
--setup-signing-key flag and use that for setup; update the caller accordingly
so the helper never treats the key as the Git buffer file.

).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
}
Expand Down
76 changes: 76 additions & 0 deletions e2e/tests/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"runtime"
"strconv"
"strings"
"time"

"github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions e2e/tests/ssh/testdata/ssh-signing/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "SSH Signing Test",
"image": "mcr.microsoft.com/devcontainers/go:1"
}
79 changes: 61 additions & 18 deletions pkg/gitsshsigning/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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...)
}
Loading
Loading