Skip to content

Commit 3cc1d02

Browse files
skevetterskevetter
authored
fix: resolve Git SSH signature forwarding failures (#704)
The `setupGitSSHSignature` function in `cmd/up.go` opened an ephemeral SSH session to configure the git signing helper inside the container. This raced with the main tunnel's port forwarding, causing "use of closed network connection" errors that blocked the entire workspace startup. The credentials server tunnel (`credentials_server.go`) already calls `gitsshsigning.ConfigureHelper` with the base64-decoded signing key, making `setupGitSSHSignature` redundant. Removing it eliminates the race condition and the cascade of failures users reported: - Port forwarding errors on first start - "No such file" for the signing key (key never written due to setup failure) - JSON decode errors (non-JSON error responses from the signature endpoint) Also fixes the `/git-ssh-signature` HTTP handler to always return JSON error responses, even when the request body cannot be read.
1 parent b3b2fe6 commit 3cc1d02

File tree

12 files changed

+356
-149
lines changed

12 files changed

+356
-149
lines changed

‎cmd/agent/container/credentials_server.go‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,21 +135,26 @@ func (cmd *CredentialsServerCmd) Run(ctx context.Context, port int) error {
135135
}(cmd.User)
136136
}
137137

138-
// configure git ssh signature helper
138+
// configure git ssh signature helper — non-fatal so that a signing
139+
// setup failure does not take down the entire credentials server
140+
// (git/docker credential forwarding, port forwarding, etc.)
139141
if cmd.GitUserSigningKey != "" {
140142
decodedKey, err := base64.StdEncoding.DecodeString(cmd.GitUserSigningKey)
141143
if err != nil {
142-
return fmt.Errorf("decode git ssh signature key: %w", err)
143-
}
144-
err = gitsshsigning.ConfigureHelper(cmd.User, string(decodedKey), log)
145-
if err != nil {
146-
return fmt.Errorf("configure git ssh signature helper: %w", err)
144+
log.Errorf("Failed to decode git SSH signing key, signing will be unavailable: %v", err)
145+
} else {
146+
err = gitsshsigning.ConfigureHelper(cmd.User, string(decodedKey), log)
147+
if err != nil {
148+
log.Errorf(
149+
"Failed to configure git SSH signature helper, signing will be unavailable: %v",
150+
err,
151+
)
152+
} else {
153+
defer func(userName string) {
154+
_ = gitsshsigning.RemoveHelper(userName)
155+
}(cmd.User)
156+
}
147157
}
148-
149-
// cleanup when we are done
150-
defer func(userName string) {
151-
_ = gitsshsigning.RemoveHelper(userName)
152-
}(cmd.User)
153158
}
154159

155160
return credentials.RunCredentialsServer(ctx, port, tunnelClient, log)

‎cmd/agent/git_ssh_signature_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,26 @@ func (s *GitSSHSignatureTestSuite) TestParseDefaultsToSign() {
5555
assert.Equal(s.T(), "sign", result.command)
5656
assert.Equal(s.T(), "/tmp/buffer", result.bufferFile)
5757
}
58+
59+
func (s *GitSSHSignatureTestSuite) TestParseEmptyArgs() {
60+
result := parseSSHKeygenArgs([]string{})
61+
assert.Equal(s.T(), "sign", result.command)
62+
assert.Equal(s.T(), "", result.bufferFile)
63+
assert.Equal(s.T(), "", result.certPath)
64+
assert.Equal(s.T(), "", result.namespace)
65+
}
66+
67+
func (s *GitSSHSignatureTestSuite) TestParseWithUFlag() {
68+
// Git passes -U when using a literal SSH key value. The parser must
69+
// still identify certPath and bufferFile with -U present.
70+
args := []string{"-Y", "sign", "-n", "git", "-f", "/key.pub", "-U", "/tmp/buf"}
71+
result := parseSSHKeygenArgs(args)
72+
assert.Equal(s.T(), "/key.pub", result.certPath)
73+
assert.Equal(s.T(), "/tmp/buf", result.bufferFile)
74+
}
75+
76+
func (s *GitSSHSignatureTestSuite) TestParseBufferFileWithSpaces() {
77+
args := []string{"-Y", "sign", "-n", "git", "-f", "/key.pub", "/tmp/my buffer file"}
78+
result := parseSSHKeygenArgs(args)
79+
assert.Equal(s.T(), "/tmp/my buffer file", result.bufferFile)
80+
}

‎cmd/ssh.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ type SSHCmd struct {
5252
AgentForwarding bool
5353
GPGAgentForwarding bool
5454
GitSSHSignatureForwarding bool
55+
GitSSHSigningKey string
5556

5657
// ssh keepalive options
5758
SSHKeepAliveInterval time.Duration `json:"sshKeepAliveInterval,omitempty"`
@@ -147,6 +148,9 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command {
147148
sshCmd.Flags().
148149
DurationVar(&cmd.SSHKeepAliveInterval, "ssh-keepalive-interval", 55*time.Second,
149150
"How often should keepalive request be made (55s)")
151+
sshCmd.Flags().
152+
StringVar(&cmd.GitSSHSigningKey, "git-ssh-signing-key", "",
153+
"The SSH signing key to use for git commit signing inside the workspace")
150154
sshCmd.Flags().StringVar(
151155
&cmd.TermMode,
152156
"term-mode",
@@ -517,6 +521,7 @@ func (cmd *SSHCmd) startTunnel(
517521
configureDockerCredentials,
518522
configureGitCredentials,
519523
configureGitSSHSignatureHelper,
524+
cmd.GitSSHSigningKey,
520525
log,
521526
)
522527
}
@@ -652,6 +657,7 @@ func (cmd *SSHCmd) startServices(
652657
containerClient *ssh.Client,
653658
workspace *provider.Workspace,
654659
configureDockerCredentials, configureGitCredentials, configureGitSSHSignatureHelper bool,
660+
gitSSHSigningKey string,
655661
log log.Logger,
656662
) {
657663
if cmd.User != "" {
@@ -668,6 +674,7 @@ func (cmd *SSHCmd) startServices(
668674
ConfigureDockerCredentials: configureDockerCredentials,
669675
ConfigureGitCredentials: configureGitCredentials,
670676
ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper,
677+
GitSSHSigningKey: gitSSHSigningKey,
671678
Log: log,
672679
},
673680
)

‎cmd/up.go‎

Lines changed: 11 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"strings"
1616
"syscall"
1717

18-
"al.essio.dev/pkg/shellescape"
1918
"github.com/blang/semver/v4"
2019
"github.com/sirupsen/logrus"
2120
"github.com/skevetter/devpod/cmd/flags"
@@ -395,17 +394,6 @@ func (cmd *UpCmd) configureWorkspace(
395394
return err
396395
}
397396

398-
// Run after dotfiles so the signing config isn't overwritten by a
399-
// dotfiles installer that replaces .gitconfig.
400-
gitSSHSignatureEnabled := devPodConfig.ContextOption(
401-
config.ContextOptionGitSSHSignatureForwarding,
402-
) == "true"
403-
if cmd.GitSSHSigningKey != "" && gitSSHSignatureEnabled {
404-
if err := setupGitSSHSignature(cmd.GitSSHSigningKey, client); err != nil {
405-
return err
406-
}
407-
}
408-
409397
return nil
410398
}
411399

@@ -481,6 +469,7 @@ func (o *ideOpener) open(
481469
user,
482470
ideOptions,
483471
o.cmd.SSHAuthSockID,
472+
o.cmd.GitSSHSigningKey,
484473
o.log,
485474
)
486475

@@ -499,6 +488,7 @@ func (o *ideOpener) open(
499488
user,
500489
ideOptions,
501490
o.cmd.SSHAuthSockID,
491+
o.cmd.GitSSHSigningKey,
502492
o.log,
503493
)
504494

@@ -511,6 +501,7 @@ func (o *ideOpener) open(
511501
user,
512502
ideOptions,
513503
o.cmd.SSHAuthSockID,
504+
o.cmd.GitSSHSigningKey,
514505
o.log,
515506
)
516507

@@ -854,6 +845,7 @@ func startJupyterNotebookInBrowser(
854845
user string,
855846
ideOptions map[string]config.OptionValue,
856847
authSockID string,
848+
gitSSHSigningKey string,
857849
logger log.Logger,
858850
) error {
859851
if forwardGpg {
@@ -900,6 +892,7 @@ func startJupyterNotebookInBrowser(
900892
false,
901893
extraPorts,
902894
authSockID,
895+
gitSSHSigningKey,
903896
logger,
904897
)
905898
}
@@ -912,6 +905,7 @@ func startRStudioInBrowser(
912905
user string,
913906
ideOptions map[string]config.OptionValue,
914907
authSockID string,
908+
gitSSHSigningKey string,
915909
logger log.Logger,
916910
) error {
917911
if forwardGpg {
@@ -957,6 +951,7 @@ func startRStudioInBrowser(
957951
false,
958952
extraPorts,
959953
authSockID,
954+
gitSSHSigningKey,
960955
logger,
961956
)
962957
}
@@ -1005,6 +1000,7 @@ func startVSCodeInBrowser(
10051000
workspaceFolder, user string,
10061001
ideOptions map[string]config.OptionValue,
10071002
authSockID string,
1003+
gitSSHSigningKey string,
10081004
logger log.Logger,
10091005
) error {
10101006
if forwardGpg {
@@ -1054,6 +1050,7 @@ func startVSCodeInBrowser(
10541050
forwardPorts,
10551051
extraPorts,
10561052
authSockID,
1053+
gitSSHSigningKey,
10571054
logger,
10581055
)
10591056
}
@@ -1150,6 +1147,7 @@ func startBrowserTunnel(
11501147
forwardPorts bool,
11511148
extraPorts []string,
11521149
authSockID string,
1150+
gitSSHSigningKey string,
11531151
logger log.Logger,
11541152
) error {
11551153
// Setup a backhaul SSH connection using the remote user so there is an AUTH SOCK to use
@@ -1245,6 +1243,7 @@ func startBrowserTunnel(
12451243
ConfigureDockerCredentials: configureDockerCredentials,
12461244
ConfigureGitCredentials: configureGitCredentials,
12471245
ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper,
1246+
GitSSHSigningKey: gitSSHSigningKey,
12481247
Log: logger,
12491248
},
12501249
)
@@ -1553,44 +1552,6 @@ func collectDotfilesScriptEnvKeyvaluePairs(envFiles []string) ([]string, error)
15531552
return keyValues, nil
15541553
}
15551554

1556-
func setupGitSSHSignature(
1557-
signingKey string,
1558-
client client2.BaseWorkspaceClient,
1559-
) error {
1560-
execPath, err := os.Executable()
1561-
if err != nil {
1562-
return err
1563-
}
1564-
1565-
remoteUser, err := devssh.GetUser(
1566-
client.WorkspaceConfig().ID,
1567-
client.WorkspaceConfig().SSHConfigPath,
1568-
client.WorkspaceConfig().SSHConfigIncludePath,
1569-
)
1570-
if err != nil {
1571-
remoteUser = "root"
1572-
}
1573-
1574-
// #nosec G204 -- execPath is from os.Executable(), not user input
1575-
out, err := exec.Command(
1576-
execPath,
1577-
"ssh",
1578-
"--user",
1579-
remoteUser,
1580-
"--context",
1581-
client.Context(),
1582-
client.Workspace(),
1583-
"--command",
1584-
shellescape.QuoteCommand(
1585-
[]string{config.BinaryName, "agent", "git-ssh-signature-helper", signingKey},
1586-
),
1587-
).CombinedOutput()
1588-
if err != nil {
1589-
return fmt.Errorf("setup git ssh signature helper: %w, output: %s", err, string(out))
1590-
}
1591-
return nil
1592-
}
1593-
15941555
func performGpgForwarding(
15951556
client client2.BaseWorkspaceClient,
15961557
log log.Logger,

‎e2e/tests/ssh/ssh.go‎

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -144,29 +144,15 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
144144
err = f.DevPodUp(ctx, tempDir, "--git-ssh-signing-key", keyPath+".pub")
145145
framework.ExpectNoError(err)
146146

147-
// Step 1: Verify the helper script was installed and executable
148-
out, err := f.DevPodSSH(ctx, tempDir,
149-
"test -x /usr/local/bin/devpod-ssh-signature && echo EXISTS",
150-
)
151-
framework.ExpectNoError(err)
152-
gomega.Expect(strings.TrimSpace(out)).To(
153-
gomega.Equal("EXISTS"),
154-
"devpod-ssh-signature helper script should be installed and executable",
155-
)
156-
157-
// Step 2: Verify git config was written correctly
158-
out, err = f.DevPodSSH(ctx, tempDir, "git config --global gpg.ssh.program")
159-
framework.ExpectNoError(err)
160-
gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("devpod-ssh-signature"))
161-
162-
out, err = f.DevPodSSH(ctx, tempDir, "git config --global gpg.format")
163-
framework.ExpectNoError(err)
164-
gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("ssh"))
165-
166-
// Step 3: Attempt a signed commit with the credentials server
167-
// tunnel active. The signing request is forwarded over the tunnel
168-
// to the host where ssh-keygen performs the actual signing.
147+
// Verify helper installation, git config, and a signed commit
148+
// in a single SSH session with --start-services so the
149+
// credentials server tunnel is active. The helper is installed
150+
// asynchronously by the credentials server, so retry briefly.
169151
commitCmd := strings.Join([]string{
152+
"for i in $(seq 1 30); do test -x /usr/local/bin/devpod-ssh-signature && break; sleep 1; done",
153+
"test -x /usr/local/bin/devpod-ssh-signature",
154+
"test \"$(git config --global gpg.ssh.program)\" = devpod-ssh-signature",
155+
"test \"$(git config --global gpg.format)\" = ssh",
170156
"cd /tmp",
171157
"git init test-sign-repo",
172158
"cd test-sign-repo",
@@ -178,13 +164,19 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
178164
"git commit -m 'signed test commit' 2>&1",
179165
}, " && ")
180166

181-
stdout, stderr, err := f.ExecCommandCapture(ctx, []string{
167+
// The signing key must be passed on each SSH invocation so the
168+
// credentials server can configure the helper inside the container.
169+
sshBase := []string{
182170
"ssh",
183171
"--agent-forwarding",
184172
"--start-services",
173+
"--git-ssh-signing-key", keyPath + ".pub",
185174
tempDir,
186-
"--command", commitCmd,
187-
})
175+
}
176+
177+
stdout, stderr, err := f.ExecCommandCapture(ctx,
178+
append(sshBase, "--command", commitCmd),
179+
)
188180
ginkgo.GinkgoWriter.Printf("commit stdout: %s\n", stdout)
189181
ginkgo.GinkgoWriter.Printf("commit stderr: %s\n", stderr)
190182
framework.ExpectNoError(err)
@@ -194,9 +186,7 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
194186
"git commit should succeed with the signed test commit message",
195187
)
196188

197-
// Step 4: Verify the commit is actually signed with a valid SSH signature.
198-
// Read the public key that was used for signing so we can build
199-
// an allowed-signers file inside the workspace for verification.
189+
// Verify the commit is signed with a valid SSH signature.
200190
pubKeyBytes, err := os.ReadFile(
201191
keyPath + ".pub",
202192
) // #nosec G304 -- test file with controlled path
@@ -205,20 +195,14 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
205195

206196
verifyCmd := strings.Join([]string{
207197
"cd /tmp/test-sign-repo",
208-
// Create allowed signers file mapping the test email to our public key
209198
"echo 'test@example.com " + pubKey + "' > /tmp/allowed_signers",
210199
"git config gpg.ssh.allowedSignersFile /tmp/allowed_signers",
211-
// Verify the commit signature is valid
212200
"git verify-commit HEAD 2>&1",
213201
}, " && ")
214202

215-
stdout, stderr, err = f.ExecCommandCapture(ctx, []string{
216-
"ssh",
217-
"--agent-forwarding",
218-
"--start-services",
219-
tempDir,
220-
"--command", verifyCmd,
221-
})
203+
stdout, stderr, err = f.ExecCommandCapture(ctx,
204+
append(sshBase, "--command", verifyCmd),
205+
)
222206
ginkgo.GinkgoWriter.Printf("verify stdout: %s\n", stdout)
223207
ginkgo.GinkgoWriter.Printf("verify stderr: %s\n", stderr)
224208
framework.ExpectNoError(err)
@@ -232,13 +216,9 @@ var _ = ginkgo.Describe("devpod ssh test suite", ginkgo.Label("ssh"), ginkgo.Ord
232216

233217
// And confirm the signature log shows the correct principal
234218
logCmd := "cd /tmp/test-sign-repo && git log --show-signature -1 2>&1"
235-
stdout, stderr, err = f.ExecCommandCapture(ctx, []string{
236-
"ssh",
237-
"--agent-forwarding",
238-
"--start-services",
239-
tempDir,
240-
"--command", logCmd,
241-
})
219+
stdout, stderr, err = f.ExecCommandCapture(ctx,
220+
append(sshBase, "--command", logCmd),
221+
)
242222
ginkgo.GinkgoWriter.Printf("log stdout: %s\n", stdout)
243223
ginkgo.GinkgoWriter.Printf("log stderr: %s\n", stderr)
244224
framework.ExpectNoError(err)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{
22
"name": "SSH Signing Test",
3-
"image": "mcr.microsoft.com/devcontainers/go:1"
3+
"image": "mcr.microsoft.com/devcontainers/go:1",
4+
"forwardPorts": [8300, 7080]
45
}

0 commit comments

Comments
 (0)