From 22dcb89847551449768107f0d26d5ae7b49be6ab Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:10:00 -0500 Subject: [PATCH 01/10] fix(gpg): set no-autostart so agent forwarding survives gpg calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GPG agent forwarding into the container points gpg at the forwarded host socket via a symlink (~/.gnupg/S.gpg-agent). Setup wrote only "use-agent" to gpg.conf, never "no-autostart". Whenever gpg ran while the forwarded socket was momentarily unavailable, it autostarted a local gpg-agent, which replaced the symlink with an empty local socket and permanently broke forwarding for the session — gpg -K showed nothing and signing failed with "No secret key". Write no-autostart (the GnuPG-recommended setting for agent forwarding) so gpg only ever uses the forwarded agent. Verified end to end: with the forwarded socket and no-autostart, gpg -K lists the secret keys and a commit is GPG-signed and verifies. --- pkg/gpg/gpg_forwarding.go | 43 ++++++++++++++++------- pkg/gpg/gpg_forwarding_test.go | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 pkg/gpg/gpg_forwarding_test.go diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index baffb9afa..04135ccea 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -96,11 +96,16 @@ func (g *GPGConf) ImportOwnerTrust() error { return gpgOwnerTrustCmd.Run() } +// gpgConfDirectives are the gpg.conf options required for agent forwarding. +// no-autostart is essential: without it, any gpg invocation made while the +// forwarded socket is momentarily unavailable starts a local gpg-agent, which +// replaces the forwarded-socket symlink with an empty local socket and +// permanently breaks forwarding for the session. +var gpgConfDirectives = []string{"use-agent", "no-autostart"} + func (g *GPGConf) SetupGpgConf() error { - _, err := os.Stat(g.getConfigPath()) - if err != nil { - _, err = os.Create(g.getConfigPath()) - if err != nil { + if _, err := os.Stat(g.getConfigPath()); err != nil { + if _, err = os.Create(g.getConfigPath()); err != nil { return err } } @@ -110,15 +115,17 @@ func (g *GPGConf) SetupGpgConf() error { return err } - if !strings.Contains(string(gpgConfig), "use-agent") { - f, err := os.OpenFile(g.getConfigPath(), - os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) - if err != nil { - return err - } - defer func() { _ = f.Close() }() + f, err := os.OpenFile(g.getConfigPath(), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return err + } + defer func() { _ = f.Close() }() - if _, err := f.WriteString("use-agent\n"); err != nil { + for _, directive := range gpgConfDirectives { + if containsDirective(string(gpgConfig), directive) { + continue + } + if _, err := f.WriteString(directive + "\n"); err != nil { return err } } @@ -126,6 +133,18 @@ func (g *GPGConf) SetupGpgConf() error { return nil } +// containsDirective reports whether the gpg.conf contents already enable the +// given directive on its own line (ignoring surrounding whitespace), so we do +// not append duplicates. +func containsDirective(config, directive string) bool { + for _, line := range strings.Split(config, "\n") { + if strings.TrimSpace(line) == directive { + return true + } + } + return false +} + func (g *GPGConf) SetupRemoteSocketDirTree() error { err := exec.Command("sudo", "mkdir", "-p", "/run/user", filepath.Dir(g.SocketPath)).Run() if err != nil { diff --git a/pkg/gpg/gpg_forwarding_test.go b/pkg/gpg/gpg_forwarding_test.go new file mode 100644 index 000000000..f8344cd31 --- /dev/null +++ b/pkg/gpg/gpg_forwarding_test.go @@ -0,0 +1,63 @@ +package gpg + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetupGpgConf_WritesRequiredDirectives(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) + + g := &GPGConf{} + require.NoError(t, g.SetupGpgConf()) + + got := readConf(t, g.getConfigPath()) + for _, d := range gpgConfDirectives { + assert.Contains(t, got, d, "gpg.conf must enable %q for forwarding", d) + } + assert.Contains(t, gpgConfDirectives, "no-autostart", + "no-autostart is required to stop gpg spawning a local agent that clobbers the forwarded socket") +} + +func TestSetupGpgConf_Idempotent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) + + g := &GPGConf{} + require.NoError(t, g.SetupGpgConf()) + require.NoError(t, g.SetupGpgConf()) + + for _, d := range gpgConfDirectives { + assert.Equal(t, 1, strings.Count(readConf(t, g.getConfigPath()), d+"\n"), + "directive %q should not be duplicated on repeated setup", d) + } +} + +func TestSetupGpgConf_PreservesExistingDirectives(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) + + g := &GPGConf{} + require.NoError(t, os.WriteFile(g.getConfigPath(), []byte("use-agent\n"), 0o600)) + require.NoError(t, g.SetupGpgConf()) + + got := readConf(t, g.getConfigPath()) + assert.Equal(t, 1, strings.Count(got, "use-agent\n")) + assert.Contains(t, got, "no-autostart") +} + +func readConf(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + require.NoError(t, err) + return string(b) +} From 7b56cc1e3e2632c57bcaf2bb4b890a12b3937a02 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:13:07 -0500 Subject: [PATCH 02/10] fix(gpg): avoid concatenating directives onto a newline-less gpg.conf --- pkg/gpg/gpg_forwarding.go | 10 +++++++++- pkg/gpg/gpg_forwarding_test.go | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index 04135ccea..c2ac33107 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -121,11 +121,19 @@ func (g *GPGConf) SetupGpgConf() error { } defer func() { _ = f.Close() }() + // Avoid concatenating onto a final line that lacks a trailing newline. + needsLeadingNewline := len(gpgConfig) > 0 && !strings.HasSuffix(string(gpgConfig), "\n") + for _, directive := range gpgConfDirectives { if containsDirective(string(gpgConfig), directive) { continue } - if _, err := f.WriteString(directive + "\n"); err != nil { + line := directive + "\n" + if needsLeadingNewline { + line = "\n" + line + needsLeadingNewline = false + } + if _, err := f.WriteString(line); err != nil { return err } } diff --git a/pkg/gpg/gpg_forwarding_test.go b/pkg/gpg/gpg_forwarding_test.go index f8344cd31..fc98650b2 100644 --- a/pkg/gpg/gpg_forwarding_test.go +++ b/pkg/gpg/gpg_forwarding_test.go @@ -55,6 +55,23 @@ func TestSetupGpgConf_PreservesExistingDirectives(t *testing.T) { assert.Contains(t, got, "no-autostart") } +func TestSetupGpgConf_ExistingFileWithoutTrailingNewline(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) + + g := &GPGConf{} + // no trailing newline on the existing directive + require.NoError(t, os.WriteFile(g.getConfigPath(), []byte("use-agent"), 0o600)) + require.NoError(t, g.SetupGpgConf()) + + got := readConf(t, g.getConfigPath()) + assert.NotContains(t, got, "use-agentno-autostart", "directives must not be concatenated") + for _, line := range []string{"use-agent", "no-autostart"} { + assert.True(t, containsDirective(got, line), "expected %q on its own line, got:\n%s", line, got) + } +} + func readConf(t *testing.T, path string) string { t.Helper() b, err := os.ReadFile(path) From c73c7dd97b43f028acc6588383b9cddc076b9b06 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:24:25 -0500 Subject: [PATCH 03/10] fix(gpg): supervise agent forward so it survives connection drops ForwardAgent launched the background gpg-forwarding SSH process once in a detached goroutine with no restart. A single dropped connection (e.g. a laptop roaming between networks) killed the forward permanently until the workspace was reopened, and interactive sessions that reuse the forwarded socket then broke too. Supervise the process: restart it whenever it exits, with exponential backoff (reset after a stable run), tied to the caller's context so it stops cleanly when the workspace is closed. Combined with no-autostart, gpg forwarding now self-heals after transient connectivity loss. --- pkg/gpg/forward.go | 54 ++++++++++++++++++++++++++++++++++------ pkg/gpg/forward_test.go | 53 +++++++++++++++++++++++++++++++++++++++ pkg/ide/opener/opener.go | 2 +- 3 files changed, 101 insertions(+), 8 deletions(-) diff --git a/pkg/gpg/forward.go b/pkg/gpg/forward.go index 2d6d30e6c..da98983d7 100644 --- a/pkg/gpg/forward.go +++ b/pkg/gpg/forward.go @@ -1,8 +1,10 @@ package gpg import ( + "context" "os" "os/exec" + "time" client2 "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/flags/names" @@ -10,8 +12,19 @@ import ( devssh "github.com/devsy-org/devsy/pkg/ssh" ) -// ForwardAgent starts a background SSH connection that forwards the local GPG agent. -func ForwardAgent(client client2.BaseWorkspaceClient) error { +// Backoff bounds for restarting the forwarding process; vars so tests can +// shrink them. +var ( + forwardRestartMinBackoff = time.Second + forwardRestartMaxBackoff = 30 * time.Second +) + +// ForwardAgent starts a background SSH connection that forwards the local GPG +// agent and keeps it alive for the lifetime of ctx. The forwarding process is +// supervised: if it exits (e.g. the connection drops while roaming), it is +// restarted with exponential backoff so gpg forwarding self-heals without +// reopening the workspace. It stops when ctx is cancelled. +func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error { log.Debug("gpg forwarding enabled, performing immediately") execPath, err := os.Executable() @@ -32,14 +45,41 @@ func ForwardAgent(client client2.BaseWorkspaceClient) error { args := buildForwardArgs(remoteUser, client.Context(), client.Workspace()) - go func() { + go superviseForward(ctx, execPath, args) + + return nil +} + +// superviseForward runs the forwarding command and restarts it whenever it +// exits, until ctx is cancelled. Backoff grows exponentially on repeated +// failures and resets once a run stays up longer than the maximum backoff, so +// a flapping connection is retried gently while a stable one recovers quickly. +func superviseForward(ctx context.Context, execPath string, args []string) { + backoff := forwardRestartMinBackoff + for { + if ctx.Err() != nil { + return + } + + start := time.Now() //nolint:gosec // execPath comes from os.Executable() - if runErr := exec.Command(execPath, args...).Run(); runErr != nil { - log.Errorf("failure in forwarding gpg-agent: %v", runErr) + runErr := exec.CommandContext(ctx, execPath, args...).Run() + if ctx.Err() != nil { + return } - }() - return nil + if time.Since(start) >= forwardRestartMaxBackoff { + backoff = forwardRestartMinBackoff + } + log.Errorf("gpg-agent forward exited (%v); restarting in %s", runErr, backoff) + + select { + case <-ctx.Done(): + return + case <-time.After(backoff): + } + backoff = min(2*backoff, forwardRestartMaxBackoff) + } } func buildForwardArgs(user, context, workspace string) []string { diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go index 0c534e8ee..67de40657 100644 --- a/pkg/gpg/forward_test.go +++ b/pkg/gpg/forward_test.go @@ -1,9 +1,15 @@ package gpg import ( + "context" + "os" + "path/filepath" + "strings" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBuildForwardArgs(t *testing.T) { @@ -22,3 +28,50 @@ func TestBuildForwardArgs(t *testing.T) { } assert.Equal(t, expected, got) } + +func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) { + oldMin, oldMax := forwardRestartMinBackoff, forwardRestartMaxBackoff + forwardRestartMinBackoff, forwardRestartMaxBackoff = 10*time.Millisecond, 20*time.Millisecond + t.Cleanup(func() { forwardRestartMinBackoff, forwardRestartMaxBackoff = oldMin, oldMax }) + + runs := filepath.Join(t.TempDir(), "runs") + ctx, cancel := context.WithCancel(context.Background()) + + done := make(chan struct{}) + go func() { + // each "run" exits immediately, forcing the supervisor to restart it + superviseForward(ctx, "/bin/sh", []string{"-c", "printf x >> " + runs}) + close(done) + }() + + time.Sleep(120 * time.Millisecond) + cancel() + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("superviseForward did not stop after ctx cancel") + } + + data, err := os.ReadFile(runs) + require.NoError(t, err) + assert.GreaterOrEqual(t, strings.Count(string(data), "x"), 2, + "forward should have been restarted after exiting") +} + +func TestSuperviseForward_StopsImmediatelyIfCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + done := make(chan struct{}) + go func() { + superviseForward(ctx, "/bin/sh", []string{"-c", "exit 0"}) + close(done) + }() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("superviseForward should return promptly when ctx is already cancelled") + } +} diff --git a/pkg/ide/opener/opener.go b/pkg/ide/opener/opener.go index e7a7ab3cd..ff3a26333 100644 --- a/pkg/ide/opener/opener.go +++ b/pkg/ide/opener/opener.go @@ -324,7 +324,7 @@ type browserIDESpec struct { func openBrowserIDE(ctx context.Context, params IDEParams, spec browserIDESpec) (string, error) { if params.GPGAgentForwarding { - if err := gpg.ForwardAgent(params.Client); err != nil { + if err := gpg.ForwardAgent(ctx, params.Client); err != nil { return "", err } } From bcdba35342c9db433e70c2077dd7e0d9292ee060 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:28:26 -0500 Subject: [PATCH 04/10] style(gpg): satisfy golines/gosec/modernize lint on new code --- pkg/gpg/forward_test.go | 2 +- pkg/gpg/gpg_forwarding.go | 2 +- pkg/gpg/gpg_forwarding_test.go | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go index 67de40657..c682003f5 100644 --- a/pkg/gpg/forward_test.go +++ b/pkg/gpg/forward_test.go @@ -53,7 +53,7 @@ func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) { t.Fatal("superviseForward did not stop after ctx cancel") } - data, err := os.ReadFile(runs) + data, err := os.ReadFile(runs) //nolint:gosec // test path is created by the test require.NoError(t, err) assert.GreaterOrEqual(t, strings.Count(string(data), "x"), 2, "forward should have been restarted after exiting") diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index c2ac33107..aa363a688 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -145,7 +145,7 @@ func (g *GPGConf) SetupGpgConf() error { // given directive on its own line (ignoring surrounding whitespace), so we do // not append duplicates. func containsDirective(config, directive string) bool { - for _, line := range strings.Split(config, "\n") { + for line := range strings.SplitSeq(config, "\n") { if strings.TrimSpace(line) == directive { return true } diff --git a/pkg/gpg/gpg_forwarding_test.go b/pkg/gpg/gpg_forwarding_test.go index fc98650b2..72fcb79cd 100644 --- a/pkg/gpg/gpg_forwarding_test.go +++ b/pkg/gpg/gpg_forwarding_test.go @@ -22,8 +22,9 @@ func TestSetupGpgConf_WritesRequiredDirectives(t *testing.T) { for _, d := range gpgConfDirectives { assert.Contains(t, got, d, "gpg.conf must enable %q for forwarding", d) } - assert.Contains(t, gpgConfDirectives, "no-autostart", - "no-autostart is required to stop gpg spawning a local agent that clobbers the forwarded socket") + // no-autostart is the directive that stops gpg spawning a local agent + // that would clobber the forwarded socket. + assert.Contains(t, gpgConfDirectives, "no-autostart") } func TestSetupGpgConf_Idempotent(t *testing.T) { @@ -68,13 +69,13 @@ func TestSetupGpgConf_ExistingFileWithoutTrailingNewline(t *testing.T) { got := readConf(t, g.getConfigPath()) assert.NotContains(t, got, "use-agentno-autostart", "directives must not be concatenated") for _, line := range []string{"use-agent", "no-autostart"} { - assert.True(t, containsDirective(got, line), "expected %q on its own line, got:\n%s", line, got) + assert.True(t, containsDirective(got, line), "missing directive %q in %q", line, got) } } func readConf(t *testing.T, path string) string { t.Helper() - b, err := os.ReadFile(path) + b, err := os.ReadFile(path) //nolint:gosec // test path is created by the test require.NoError(t, err) return string(b) } From 3de4fb5d4fef56b019c62203455e54a2d97fb6f4 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:40:17 -0500 Subject: [PATCH 05/10] refactor(gpg): drop explanatory comments for self-documenting code --- pkg/gpg/forward.go | 13 ++----------- pkg/gpg/forward_test.go | 12 +++++------- pkg/gpg/gpg_forwarding.go | 9 --------- pkg/gpg/gpg_forwarding_test.go | 3 --- 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/pkg/gpg/forward.go b/pkg/gpg/forward.go index da98983d7..83bba0f49 100644 --- a/pkg/gpg/forward.go +++ b/pkg/gpg/forward.go @@ -12,18 +12,13 @@ import ( devssh "github.com/devsy-org/devsy/pkg/ssh" ) -// Backoff bounds for restarting the forwarding process; vars so tests can -// shrink them. var ( forwardRestartMinBackoff = time.Second forwardRestartMaxBackoff = 30 * time.Second ) -// ForwardAgent starts a background SSH connection that forwards the local GPG -// agent and keeps it alive for the lifetime of ctx. The forwarding process is -// supervised: if it exits (e.g. the connection drops while roaming), it is -// restarted with exponential backoff so gpg forwarding self-heals without -// reopening the workspace. It stops when ctx is cancelled. +// ForwardAgent starts a supervised background SSH connection that forwards the +// local GPG agent, restarting it until ctx is cancelled. func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error { log.Debug("gpg forwarding enabled, performing immediately") @@ -50,10 +45,6 @@ func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error return nil } -// superviseForward runs the forwarding command and restarts it whenever it -// exits, until ctx is cancelled. Backoff grows exponentially on repeated -// failures and resets once a run stays up longer than the maximum backoff, so -// a flapping connection is retried gently while a stable one recovers quickly. func superviseForward(ctx context.Context, execPath string, args []string) { backoff := forwardRestartMinBackoff for { diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go index c682003f5..72362e910 100644 --- a/pkg/gpg/forward_test.go +++ b/pkg/gpg/forward_test.go @@ -39,12 +39,15 @@ func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) { done := make(chan struct{}) go func() { - // each "run" exits immediately, forcing the supervisor to restart it superviseForward(ctx, "/bin/sh", []string{"-c", "printf x >> " + runs}) close(done) }() - time.Sleep(120 * time.Millisecond) + require.Eventually(t, func() bool { + data, _ := os.ReadFile(runs) //nolint:gosec // test path is created by the test + return strings.Count(string(data), "x") >= 2 + }, 5*time.Second, 10*time.Millisecond, "forward should be restarted after it exits") + cancel() select { @@ -52,11 +55,6 @@ func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) { case <-time.After(2 * time.Second): t.Fatal("superviseForward did not stop after ctx cancel") } - - data, err := os.ReadFile(runs) //nolint:gosec // test path is created by the test - require.NoError(t, err) - assert.GreaterOrEqual(t, strings.Count(string(data), "x"), 2, - "forward should have been restarted after exiting") } func TestSuperviseForward_StopsImmediatelyIfCancelled(t *testing.T) { diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index aa363a688..cd459dce8 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -96,11 +96,6 @@ func (g *GPGConf) ImportOwnerTrust() error { return gpgOwnerTrustCmd.Run() } -// gpgConfDirectives are the gpg.conf options required for agent forwarding. -// no-autostart is essential: without it, any gpg invocation made while the -// forwarded socket is momentarily unavailable starts a local gpg-agent, which -// replaces the forwarded-socket symlink with an empty local socket and -// permanently breaks forwarding for the session. var gpgConfDirectives = []string{"use-agent", "no-autostart"} func (g *GPGConf) SetupGpgConf() error { @@ -121,7 +116,6 @@ func (g *GPGConf) SetupGpgConf() error { } defer func() { _ = f.Close() }() - // Avoid concatenating onto a final line that lacks a trailing newline. needsLeadingNewline := len(gpgConfig) > 0 && !strings.HasSuffix(string(gpgConfig), "\n") for _, directive := range gpgConfDirectives { @@ -141,9 +135,6 @@ func (g *GPGConf) SetupGpgConf() error { return nil } -// containsDirective reports whether the gpg.conf contents already enable the -// given directive on its own line (ignoring surrounding whitespace), so we do -// not append duplicates. func containsDirective(config, directive string) bool { for line := range strings.SplitSeq(config, "\n") { if strings.TrimSpace(line) == directive { diff --git a/pkg/gpg/gpg_forwarding_test.go b/pkg/gpg/gpg_forwarding_test.go index 72fcb79cd..dd2436985 100644 --- a/pkg/gpg/gpg_forwarding_test.go +++ b/pkg/gpg/gpg_forwarding_test.go @@ -22,8 +22,6 @@ func TestSetupGpgConf_WritesRequiredDirectives(t *testing.T) { for _, d := range gpgConfDirectives { assert.Contains(t, got, d, "gpg.conf must enable %q for forwarding", d) } - // no-autostart is the directive that stops gpg spawning a local agent - // that would clobber the forwarded socket. assert.Contains(t, gpgConfDirectives, "no-autostart") } @@ -62,7 +60,6 @@ func TestSetupGpgConf_ExistingFileWithoutTrailingNewline(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) g := &GPGConf{} - // no trailing newline on the existing directive require.NoError(t, os.WriteFile(g.getConfigPath(), []byte("use-agent"), 0o600)) require.NoError(t, g.SetupGpgConf()) From 65136f69861f3a134405a129f429fe7a523e1fba Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:40:18 -0500 Subject: [PATCH 06/10] test(e2e): verify GPG secret key is reachable via agent forwarding The existing gpg-forwarding e2e only asserted the workspace boots with the flag set; it never ran gpg, so a broken forward passed. Add a functional check: import the test key on the host, up with --ssh-gpg-forwarding, then assert the container's forwarded agent exposes the secret key (the exact symptom that regressed). Replaces the unused, incorrect DevsySSHGpgTestKey. --- e2e/framework/command.go | 31 +++++++++++++++++++------------ e2e/tests/ssh/ssh.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/e2e/framework/command.go b/e2e/framework/command.go index 8952fdab8..499c35aee 100644 --- a/e2e/framework/command.go +++ b/e2e/framework/command.go @@ -468,34 +468,41 @@ func (f *Framework) SetupGPG(tmpDir string) error { return exec.Command("gpg", "-k").Run() } -func (f *Framework) DevsySSHGpgTestKey(ctx context.Context, workspace string) error { - pubKeyB, err := exec.Command("sh", "-c", "gpg -k --with-colons 2>/dev/null | grep sec | base64 -w0"). - Output() +func ImportGpgKey(privateKeyPath string) error { + // #nosec G204 -- test imports a controlled key path + out, err := exec.Command("gpg", "--batch", "--import", privateKeyPath).CombinedOutput() if err != nil { - return err + return fmt.Errorf("import gpg private key: %s: %w", out, err) + } + if out, err := exec.Command("gpg-connect-agent", "/bye").CombinedOutput(); err != nil { + return fmt.Errorf("start gpg-agent: %s: %w", out, err) } + return nil +} - // First run to trigger the first forwarding +func (f *Framework) DevsySSHGpgSecretKeyForwarded( + ctx context.Context, + workspace, expectedFingerprint string, +) error { stdout, _, err := f.ExecCommandCapture(ctx, []string{ cmdWorkspace, cmdSSH, names.Flag(names.AgentForwarding), names.Flag(names.SSHGPGForwarding), flagCommand, - "gpg -k --with-colons 2>/dev/null |grep sec | base64 -w0", workspace, + "gpg --list-secret-keys --with-colons 2>/dev/null", + workspace, }) if err != nil { - return err + return fmt.Errorf("list secret keys in container: %w", err) } - - if stdout != string(pubKeyB) { + if !strings.Contains(stdout, expectedFingerprint) { return fmt.Errorf( - "devsy gpg public key forwarding failed, expected %s, got %s", - string(pubKeyB), + "gpg agent forwarding failed: secret key %s not reachable in container, got: %q", + expectedFingerprint, stdout, ) } - return nil } diff --git a/e2e/tests/ssh/ssh.go b/e2e/tests/ssh/ssh.go index a5823330b..ae081679e 100644 --- a/e2e/tests/ssh/ssh.go +++ b/e2e/tests/ssh/ssh.go @@ -22,6 +22,8 @@ import ( const ( osWindows = "windows" + gpgTestKeyFingerprint = "07F681B9FD6C3411F679BFD1F51769DB572DDD3F" + envSSHAuthSock = "SSH_AUTH_SOCK" sshOptStrictHostKeyCheckingNo = "StrictHostKeyChecking=no" @@ -120,6 +122,43 @@ var _ = ginkgo.Describe("devsy ssh test suite", ginkgo.Label("ssh"), ginkgo.Orde }, ) + ginkgo.It( + "should expose the host GPG secret key in the container via agent forwarding", + ginkgo.Label("gpg"), + ginkgo.SpecTimeout(framework.TimeoutModerate()), + func(ctx ginkgo.SpecContext) { + if runtime.GOOS == osWindows { + ginkgo.Skip("skipping on windows") + } + + tempDir, err := framework.CopyToTempDir("tests/ssh/testdata/gpg-forwarding") + framework.ExpectNoError(err) + + f := framework.NewDefaultFramework(initialDir + "/bin") + _ = f.DevsyProviderAdd(ctx, "docker") + err = f.DevsyProviderUse(ctx, "docker") + framework.ExpectNoError(err) + + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + _ = f.DevsyWorkspaceDelete(cleanupCtx, tempDir) + framework.CleanupTempDir(initialDir, tempDir) + }) + + ginkgo.GinkgoT().Setenv("GNUPGHOME", ginkgo.GinkgoT().TempDir()) + framework.ExpectNoError( + framework.ImportGpgKey(filepath.Join(tempDir, "gpg-private.key")), + ) + + err = f.DevsyUp(ctx, tempDir, names.Flag(names.SSHGPGForwarding)) + framework.ExpectNoError(err) + + sshCtx, cancelSSH := context.WithDeadline(ctx, time.Now().Add(30*time.Second)) + defer cancelSSH() + err = f.DevsySSHGpgSecretKeyForwarded(sshCtx, tempDir, gpgTestKeyFingerprint) + framework.ExpectNoError(err) + }, + ) + ginkgo.It( "should set up git SSH signature helper and sign a commit", ginkgo.SpecTimeout(framework.TimeoutLong()), From d15fb03dcd19cf0df8714940827fca76cd4f4424 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:46:47 -0500 Subject: [PATCH 07/10] refactor(gpg): tidy forwarding code for idiomatic Go - pass backoff bounds into superviseForward via a struct instead of test-mutable package globals - dedupe the gpg stdin-import logic (runGpgWithStdin) and wrap both import errors consistently - simplify SetupGpgConf (rely on O_CREATE instead of stat+create) - clarify IsGpgTunnelRunning (TrimSpace != "" instead of len > 1) - drop the pointless slice-spread on gpgconf --kill - correct the stale "hardlinks" comment (symlinks) --- pkg/gpg/forward.go | 27 +++++++++--------- pkg/gpg/forward_test.go | 10 +++---- pkg/gpg/gpg_forwarding.go | 59 ++++++++++++++------------------------- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/pkg/gpg/forward.go b/pkg/gpg/forward.go index 83bba0f49..24d676968 100644 --- a/pkg/gpg/forward.go +++ b/pkg/gpg/forward.go @@ -12,16 +12,15 @@ import ( devssh "github.com/devsy-org/devsy/pkg/ssh" ) -var ( - forwardRestartMinBackoff = time.Second - forwardRestartMaxBackoff = 30 * time.Second -) +type backoff struct { + min, max time.Duration +} + +var forwardRestartBackoff = backoff{min: time.Second, max: 30 * time.Second} // ForwardAgent starts a supervised background SSH connection that forwards the // local GPG agent, restarting it until ctx is cancelled. func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error { - log.Debug("gpg forwarding enabled, performing immediately") - execPath, err := os.Executable() if err != nil { return err @@ -40,13 +39,13 @@ func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error args := buildForwardArgs(remoteUser, client.Context(), client.Workspace()) - go superviseForward(ctx, execPath, args) + go superviseForward(ctx, execPath, args, forwardRestartBackoff) return nil } -func superviseForward(ctx context.Context, execPath string, args []string) { - backoff := forwardRestartMinBackoff +func superviseForward(ctx context.Context, execPath string, args []string, b backoff) { + delay := b.min for { if ctx.Err() != nil { return @@ -59,17 +58,17 @@ func superviseForward(ctx context.Context, execPath string, args []string) { return } - if time.Since(start) >= forwardRestartMaxBackoff { - backoff = forwardRestartMinBackoff + if time.Since(start) >= b.max { + delay = b.min } - log.Errorf("gpg-agent forward exited (%v); restarting in %s", runErr, backoff) + log.Errorf("gpg-agent forward exited (%v); restarting in %s", runErr, delay) select { case <-ctx.Done(): return - case <-time.After(backoff): + case <-time.After(delay): } - backoff = min(2*backoff, forwardRestartMaxBackoff) + delay = min(2*delay, b.max) } } diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go index 72362e910..7786c9ba9 100644 --- a/pkg/gpg/forward_test.go +++ b/pkg/gpg/forward_test.go @@ -30,16 +30,13 @@ func TestBuildForwardArgs(t *testing.T) { } func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) { - oldMin, oldMax := forwardRestartMinBackoff, forwardRestartMaxBackoff - forwardRestartMinBackoff, forwardRestartMaxBackoff = 10*time.Millisecond, 20*time.Millisecond - t.Cleanup(func() { forwardRestartMinBackoff, forwardRestartMaxBackoff = oldMin, oldMax }) - runs := filepath.Join(t.TempDir(), "runs") ctx, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) go func() { - superviseForward(ctx, "/bin/sh", []string{"-c", "printf x >> " + runs}) + superviseForward(ctx, "/bin/sh", []string{"-c", "printf x >> " + runs}, + backoff{min: 10 * time.Millisecond, max: 20 * time.Millisecond}) close(done) }() @@ -63,7 +60,8 @@ func TestSuperviseForward_StopsImmediatelyIfCancelled(t *testing.T) { done := make(chan struct{}) go func() { - superviseForward(ctx, "/bin/sh", []string{"-c", "exit 0"}) + superviseForward(ctx, "/bin/sh", []string{"-c", "exit 0"}, + backoff{min: 10 * time.Millisecond, max: 20 * time.Millisecond}) close(done) }() diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index cd459dce8..432366b5f 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -35,7 +35,7 @@ func IsGpgTunnelRunning( command = fmt.Sprintf("su -c \"%s\" '%s'", command, user) } - // capture the output, if it's empty it means we don't have gpg-forwarding + // empty output means the forwarded agent exposes no secret keys var out bytes.Buffer err := devssh.Run(ctx, devssh.RunOptions{ Client: client, @@ -44,7 +44,7 @@ func IsGpgTunnelRunning( Stderr: writer, }) - return err == nil && len(out.Bytes()) > 1 + return err == nil && strings.TrimSpace(out.String()) != "" } func GetHostPubKey() ([]byte, error) { @@ -56,57 +56,42 @@ func GetHostOwnerTrust() ([]byte, error) { } func (g *GPGConf) StopGpgAgent() error { - return exec.Command("gpgconf", []string{"--kill", "gpg-agent"}...).Run() + return exec.Command("gpgconf", "--kill", "gpg-agent").Run() } func (g *GPGConf) ImportGpgKey() error { - gpgImportCmd := exec.Command("gpg", "--import") - - stdin, err := gpgImportCmd.StdinPipe() - if err != nil { - return err - } - - go func() { - defer func() { _ = stdin.Close() }() - _, _ = stdin.Write(g.PublicKey) - }() - - out, err := gpgImportCmd.CombinedOutput() - if err != nil { - return fmt.Errorf("import gpg public key: %s: %w", out, err) - } - - return nil + return runGpgWithStdin(g.PublicKey, "--import") } func (g *GPGConf) ImportOwnerTrust() error { - gpgOwnerTrustCmd := exec.Command("gpg", "--import-ownertrust") + return runGpgWithStdin(g.OwnerTrust, "--import-ownertrust") +} + +func runGpgWithStdin(input []byte, args ...string) error { + //nolint:gosec // args are internal gpg directive literals + cmd := exec.Command("gpg", args...) - stdin, err := gpgOwnerTrustCmd.StdinPipe() + stdin, err := cmd.StdinPipe() if err != nil { return err } - go func() { defer func() { _ = stdin.Close() }() - _, _ = stdin.Write(g.OwnerTrust) + _, _ = stdin.Write(input) }() - return gpgOwnerTrustCmd.Run() + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("gpg %s: %s: %w", strings.Join(args, " "), out, err) + } + return nil } var gpgConfDirectives = []string{"use-agent", "no-autostart"} func (g *GPGConf) SetupGpgConf() error { - if _, err := os.Stat(g.getConfigPath()); err != nil { - if _, err = os.Create(g.getConfigPath()); err != nil { - return err - } - } - gpgConfig, err := os.ReadFile(g.getConfigPath()) - if err != nil { + if err != nil && !os.IsNotExist(err) { return err } @@ -160,11 +145,9 @@ func (g *GPGConf) SetupRemoteSocketDirTree() error { ).Run() } -// This function will normalize the location of the forwarded socket. -// the forwarding that happens in pkg/ssh/forward.go will forward the socket in -// the same path (eg. /Users/foo/.gnupg/S.gpg-agent) -// This function will use hardlinks to normalize it to where linux usually -// expects the socket to be. +// SetupRemoteSocketLink symlinks the well-known gpg-agent socket paths to the +// forwarded socket, which pkg/ssh/forward.go binds at g.SocketPath (the host's +// path, e.g. /Users/foo/.gnupg/S.gpg-agent). func (g *GPGConf) SetupRemoteSocketLink() error { err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".gnupg"), 0o700) if err != nil { From 806cc8996a965bf9c6705c2b9807731131d45717 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:53:01 -0500 Subject: [PATCH 08/10] refactor(gpg): drop unused receivers and escape the su command - convert StopGpgAgent/SetupGpgConf/gpgConfigPath (which never used the GPGConf receiver) to package functions; update call sites - build the su -c wrapper with shellescape.QuoteCommand instead of hand-rolled fmt.Sprintf quoting - extract appendMissingDirectives to keep SetupGpgConf within complexity --- cmd/internal/agentworkspace/setup_gpg.go | 6 +++--- pkg/gpg/gpg_forwarding.go | 21 +++++++++++-------- pkg/gpg/gpg_forwarding_test.go | 26 ++++++++++-------------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/cmd/internal/agentworkspace/setup_gpg.go b/cmd/internal/agentworkspace/setup_gpg.go index f8c3808ea..a8bb279c7 100644 --- a/cmd/internal/agentworkspace/setup_gpg.go +++ b/cmd/internal/agentworkspace/setup_gpg.go @@ -113,7 +113,7 @@ func fetchAndDecodeKeys(ownerTrustB64 string) ([]byte, []byte, error) { func configureGPGAgent(gpgConf *gpg.GPGConf) error { log.Debugf("Stopping container gpg-agent") - if err := gpgConf.StopGpgAgent(); err != nil { + if err := gpg.StopGpgAgent(); err != nil { return fmt.Errorf("stop container gpg-agent: %w", err) } @@ -135,7 +135,7 @@ func configureGPGAgent(gpgConf *gpg.GPGConf) error { // Now we again kill the agent and remove the socket to really be sure every // thing is clean log.Debugf("Ensure stopping container gpg-agent") - if err := gpgConf.StopGpgAgent(); err != nil { + if err := gpg.StopGpgAgent(); err != nil { return fmt.Errorf("ensure stopping container gpg-agent: %w", err) } @@ -145,7 +145,7 @@ func configureGPGAgent(gpgConf *gpg.GPGConf) error { } log.Debugf("Setup gpg.conf") - if err := gpgConf.SetupGpgConf(); err != nil { + if err := gpg.SetupGpgConf(); err != nil { return fmt.Errorf("setup gpg.conf: %w", err) } diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index 432366b5f..fd627d6d8 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + "al.essio.dev/pkg/shellescape" "github.com/devsy-org/devsy/pkg/log" devssh "github.com/devsy-org/devsy/pkg/ssh" "golang.org/x/crypto/ssh" @@ -32,7 +33,7 @@ func IsGpgTunnelRunning( command := "gpg -K" if user != "" && user != "root" { - command = fmt.Sprintf("su -c \"%s\" '%s'", command, user) + command = shellescape.QuoteCommand([]string{"su", "-c", command, user}) } // empty output means the forwarded agent exposes no secret keys @@ -55,7 +56,7 @@ func GetHostOwnerTrust() ([]byte, error) { return exec.Command("gpg", "--export-ownertrust").Output() } -func (g *GPGConf) StopGpgAgent() error { +func StopGpgAgent() error { return exec.Command("gpgconf", "--kill", "gpg-agent").Run() } @@ -89,22 +90,26 @@ func runGpgWithStdin(input []byte, args ...string) error { var gpgConfDirectives = []string{"use-agent", "no-autostart"} -func (g *GPGConf) SetupGpgConf() error { - gpgConfig, err := os.ReadFile(g.getConfigPath()) +func SetupGpgConf() error { + existing, err := os.ReadFile(gpgConfigPath()) if err != nil && !os.IsNotExist(err) { return err } - f, err := os.OpenFile(g.getConfigPath(), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + f, err := os.OpenFile(gpgConfigPath(), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) if err != nil { return err } defer func() { _ = f.Close() }() - needsLeadingNewline := len(gpgConfig) > 0 && !strings.HasSuffix(string(gpgConfig), "\n") + return appendMissingDirectives(f, string(existing)) +} + +func appendMissingDirectives(f *os.File, existing string) error { + needsLeadingNewline := existing != "" && !strings.HasSuffix(existing, "\n") for _, directive := range gpgConfDirectives { - if containsDirective(string(gpgConfig), directive) { + if containsDirective(existing, directive) { continue } line := directive + "\n" @@ -177,6 +182,6 @@ func (g *GPGConf) SetupRemoteSocketLink() error { return nil } -func (g *GPGConf) getConfigPath() string { +func gpgConfigPath() string { return filepath.Join(os.Getenv("HOME"), ".gnupg", "gpg.conf") } diff --git a/pkg/gpg/gpg_forwarding_test.go b/pkg/gpg/gpg_forwarding_test.go index dd2436985..f3f485291 100644 --- a/pkg/gpg/gpg_forwarding_test.go +++ b/pkg/gpg/gpg_forwarding_test.go @@ -15,10 +15,9 @@ func TestSetupGpgConf_WritesRequiredDirectives(t *testing.T) { t.Setenv("HOME", home) require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) - g := &GPGConf{} - require.NoError(t, g.SetupGpgConf()) + require.NoError(t, SetupGpgConf()) - got := readConf(t, g.getConfigPath()) + got := readConf(t, gpgConfigPath()) for _, d := range gpgConfDirectives { assert.Contains(t, got, d, "gpg.conf must enable %q for forwarding", d) } @@ -30,12 +29,11 @@ func TestSetupGpgConf_Idempotent(t *testing.T) { t.Setenv("HOME", home) require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) - g := &GPGConf{} - require.NoError(t, g.SetupGpgConf()) - require.NoError(t, g.SetupGpgConf()) + require.NoError(t, SetupGpgConf()) + require.NoError(t, SetupGpgConf()) for _, d := range gpgConfDirectives { - assert.Equal(t, 1, strings.Count(readConf(t, g.getConfigPath()), d+"\n"), + assert.Equal(t, 1, strings.Count(readConf(t, gpgConfigPath()), d+"\n"), "directive %q should not be duplicated on repeated setup", d) } } @@ -45,11 +43,10 @@ func TestSetupGpgConf_PreservesExistingDirectives(t *testing.T) { t.Setenv("HOME", home) require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) - g := &GPGConf{} - require.NoError(t, os.WriteFile(g.getConfigPath(), []byte("use-agent\n"), 0o600)) - require.NoError(t, g.SetupGpgConf()) + require.NoError(t, os.WriteFile(gpgConfigPath(), []byte("use-agent\n"), 0o600)) + require.NoError(t, SetupGpgConf()) - got := readConf(t, g.getConfigPath()) + got := readConf(t, gpgConfigPath()) assert.Equal(t, 1, strings.Count(got, "use-agent\n")) assert.Contains(t, got, "no-autostart") } @@ -59,11 +56,10 @@ func TestSetupGpgConf_ExistingFileWithoutTrailingNewline(t *testing.T) { t.Setenv("HOME", home) require.NoError(t, os.MkdirAll(filepath.Join(home, ".gnupg"), 0o700)) - g := &GPGConf{} - require.NoError(t, os.WriteFile(g.getConfigPath(), []byte("use-agent"), 0o600)) - require.NoError(t, g.SetupGpgConf()) + require.NoError(t, os.WriteFile(gpgConfigPath(), []byte("use-agent"), 0o600)) + require.NoError(t, SetupGpgConf()) - got := readConf(t, g.getConfigPath()) + got := readConf(t, gpgConfigPath()) assert.NotContains(t, got, "use-agentno-autostart", "directives must not be concatenated") for _, line := range []string{"use-agent", "no-autostart"} { assert.True(t, containsDirective(got, line), "missing directive %q in %q", line, got) From 298c6f29a6fb7c21fd2f947b11e9df4598a97240 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 20:10:01 -0500 Subject: [PATCH 09/10] fix(gpg): forward agent socket to a container-reachable path Forwarding mirrored the host's socket path inside the container (e.g. /Users//.gnupg/... on a macOS host) and then symlinked to it. Those mirrored ancestor dirs are created root-owned, so a non-root workspace user cannot traverse them to reach the socket, and GPG forwarding silently failed (gpg -K empty, "No secret key"). The previous attempt to chmod the mirrored host dirs treated the symptom. Do what standard GPG agent forwarding does instead: forward the host's extra socket to a fixed container-local path the workspace user can reach, symlink gpg's own socket locations (~/.gnupg, /run/user//gnupg) to it, and take ownership of the socket (the ssh server binds it as root). No host-path mirroring, no permission changes to OS directories. Run the gpg-forwarding e2e as a non-root user so this failure mode is covered (the container's home is likewise root-owned, reproducing it). --- cmd/workspace/ssh.go | 9 ++- .../gpg-forwarding/.devcontainer.json | 2 +- .../testdata/gpg-forwarding/devcontainer.json | 3 +- pkg/gpg/gpg_forwarding.go | 70 +++++++++++-------- 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/cmd/workspace/ssh.go b/cmd/workspace/ssh.go index 08f30ac13..524038505 100644 --- a/cmd/workspace/ssh.go +++ b/cmd/workspace/ssh.go @@ -745,7 +745,12 @@ func (cmd *SSHCmd) setupGPGAgent( gitKey := gpg.SigningKey(ctx) - cmd.ReverseForwardPorts = append(cmd.ReverseForwardPorts, gpgExtraSocketPath) + // Forward the host's extra socket to a container-local path the workspace + // user can reach, rather than mirroring the host's path into the container. + cmd.ReverseForwardPorts = append( + cmd.ReverseForwardPorts, + gpg.ContainerSocketPath+":"+gpgExtraSocketPath, + ) // Now we forward the agent socket to the remote, and setup remote gpg to use it forwardAgent := []string{ @@ -757,7 +762,7 @@ func (cmd *SSHCmd) setupGPGAgent( names.Flag(names.OwnerTrust), ownerTrustArgument, names.Flag(names.SocketPath), - gpgExtraSocketPath, + gpg.ContainerSocketPath, } if log.DebugEnabled() { diff --git a/e2e/tests/ssh/testdata/gpg-forwarding/.devcontainer.json b/e2e/tests/ssh/testdata/gpg-forwarding/.devcontainer.json index b86c3547f..3b59e8273 100644 --- a/e2e/tests/ssh/testdata/gpg-forwarding/.devcontainer.json +++ b/e2e/tests/ssh/testdata/gpg-forwarding/.devcontainer.json @@ -1 +1 @@ -{ "image": "ghcr.io/devsy-org/test-images/base:ubuntu" } +{ "image": "ghcr.io/devsy-org/test-images/base:ubuntu", "remoteUser": "vscode" } diff --git a/e2e/tests/ssh/testdata/gpg-forwarding/devcontainer.json b/e2e/tests/ssh/testdata/gpg-forwarding/devcontainer.json index 25e8ab286..77ba88e1c 100755 --- a/e2e/tests/ssh/testdata/gpg-forwarding/devcontainer.json +++ b/e2e/tests/ssh/testdata/gpg-forwarding/devcontainer.json @@ -1,4 +1,5 @@ { "name": "Go", - "image": "ghcr.io/devsy-org/test-images/go:1" + "image": "ghcr.io/devsy-org/test-images/go:1", + "remoteUser": "vscode" } diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index fd627d6d8..da85c48d9 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "al.essio.dev/pkg/shellescape" "github.com/devsy-org/devsy/pkg/log" @@ -134,52 +135,65 @@ func containsDirective(config, directive string) bool { return false } +// ContainerSocketPath is where the host gpg-agent socket is forwarded inside +// the workspace container. It lives in a location the workspace user can always +// reach, so no host-path mirroring or permission changes are required. +const ContainerSocketPath = "/tmp/S.gpg-agent" + +// SetupRemoteSocketDirTree ensures the per-user runtime directory exists and is +// owned by the workspace user, so the forwarded-socket symlink can be created +// under it. func (g *GPGConf) SetupRemoteSocketDirTree() error { - err := exec.Command("sudo", "mkdir", "-p", "/run/user", filepath.Dir(g.SocketPath)).Run() - if err != nil { + runUserDir := filepath.Join("/run/user", strconv.Itoa(os.Getuid())) + + //nolint:gosec // runUserDir is a fixed per-uid runtime path, not user input + if err := exec.Command("sudo", "mkdir", "-p", runUserDir).Run(); err != nil { return err } + //nolint:gosec // runUserDir is a fixed per-uid runtime path, not user input return exec.Command("sudo", - "chown", - "-R", + "chown", "-R", strconv.Itoa(os.Getuid())+":"+strconv.Itoa(os.Getgid()), - "/run/user", - filepath.Dir(g.SocketPath), - g.SocketPath, + runUserDir, ).Run() } -// SetupRemoteSocketLink symlinks the well-known gpg-agent socket paths to the -// forwarded socket, which pkg/ssh/forward.go binds at g.SocketPath (the host's -// path, e.g. /Users/foo/.gnupg/S.gpg-agent). +// SetupRemoteSocketLink points gpg's well-known agent socket locations at the +// forwarded socket (g.SocketPath) so gpg talks to the host agent. func (g *GPGConf) SetupRemoteSocketLink() error { - err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".gnupg"), 0o700) - if err != nil { - return err - } - - err = exec.Command("sudo", "ln", "-s", "-f", g.SocketPath, "/tmp/S.gpg-agent").Run() - if err != nil { - return err - } - - symlinks := []string{ + links := []string{ filepath.Join(os.Getenv("HOME"), ".gnupg", "S.gpg-agent"), - "/run/user/" + strconv.Itoa(os.Getuid()) + "/gnupg/S.gpg-agent", + filepath.Join("/run/user", strconv.Itoa(os.Getuid()), "gnupg", "S.gpg-agent"), } - for _, link := range symlinks { + for _, link := range links { + //nolint:gosec // link derives from $HOME and the current uid, not user input + if err := os.MkdirAll(filepath.Dir(link), 0o700); err != nil { + return err + } _ = os.Remove(link) - _ = os.MkdirAll(filepath.Dir(link), 0o755) - - err = os.Symlink("/tmp/S.gpg-agent", link) - if err != nil { + if err := os.Symlink(g.SocketPath, link); err != nil { return err } } - return nil + return g.claimForwardedSocket() +} + +// claimForwardedSocket takes ownership of the forwarded socket. The ssh server +// binds it as root, but a non-root workspace user needs write access to connect +// to it. The socket is bound asynchronously, so wait briefly for it to appear. +func (g *GPGConf) claimForwardedSocket() error { + owner := strconv.Itoa(os.Getuid()) + ":" + strconv.Itoa(os.Getgid()) + for range 30 { + if _, err := os.Stat(g.SocketPath); err == nil { + //nolint:gosec // g.SocketPath is the fixed forwarded socket path + return exec.Command("sudo", "chown", owner, g.SocketPath).Run() + } + time.Sleep(100 * time.Millisecond) + } + return fmt.Errorf("forwarded gpg socket %q did not appear", g.SocketPath) } func gpgConfigPath() string { From 39c032aed91d5d6c56ebfb68fda7987743bf5497 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 20:24:21 -0500 Subject: [PATCH 10/10] docs(gpg): trim comments to self-documenting minimum --- cmd/workspace/ssh.go | 2 -- pkg/gpg/gpg_forwarding.go | 16 +++++----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/cmd/workspace/ssh.go b/cmd/workspace/ssh.go index 524038505..447e62874 100644 --- a/cmd/workspace/ssh.go +++ b/cmd/workspace/ssh.go @@ -745,8 +745,6 @@ func (cmd *SSHCmd) setupGPGAgent( gitKey := gpg.SigningKey(ctx) - // Forward the host's extra socket to a container-local path the workspace - // user can reach, rather than mirroring the host's path into the container. cmd.ReverseForwardPorts = append( cmd.ReverseForwardPorts, gpg.ContainerSocketPath+":"+gpgExtraSocketPath, diff --git a/pkg/gpg/gpg_forwarding.go b/pkg/gpg/gpg_forwarding.go index da85c48d9..58174b2c9 100644 --- a/pkg/gpg/gpg_forwarding.go +++ b/pkg/gpg/gpg_forwarding.go @@ -135,14 +135,10 @@ func containsDirective(config, directive string) bool { return false } -// ContainerSocketPath is where the host gpg-agent socket is forwarded inside -// the workspace container. It lives in a location the workspace user can always -// reach, so no host-path mirroring or permission changes are required. +// ContainerSocketPath is the container-local path the host gpg-agent socket is +// forwarded to; any workspace user can reach it. const ContainerSocketPath = "/tmp/S.gpg-agent" -// SetupRemoteSocketDirTree ensures the per-user runtime directory exists and is -// owned by the workspace user, so the forwarded-socket symlink can be created -// under it. func (g *GPGConf) SetupRemoteSocketDirTree() error { runUserDir := filepath.Join("/run/user", strconv.Itoa(os.Getuid())) @@ -159,8 +155,6 @@ func (g *GPGConf) SetupRemoteSocketDirTree() error { ).Run() } -// SetupRemoteSocketLink points gpg's well-known agent socket locations at the -// forwarded socket (g.SocketPath) so gpg talks to the host agent. func (g *GPGConf) SetupRemoteSocketLink() error { links := []string{ filepath.Join(os.Getenv("HOME"), ".gnupg", "S.gpg-agent"), @@ -181,9 +175,9 @@ func (g *GPGConf) SetupRemoteSocketLink() error { return g.claimForwardedSocket() } -// claimForwardedSocket takes ownership of the forwarded socket. The ssh server -// binds it as root, but a non-root workspace user needs write access to connect -// to it. The socket is bound asynchronously, so wait briefly for it to appear. +// claimForwardedSocket takes ownership of the socket, which the ssh server +// binds as root; a non-root user needs write access to connect. It is bound +// asynchronously, so wait briefly for it to appear. func (g *GPGConf) claimForwardedSocket() error { owner := strconv.Itoa(os.Getuid()) + ":" + strconv.Itoa(os.Getgid()) for range 30 {