From de503646b1901e6ee0d9c469a36551eceb51d26e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:10:00 -0500 Subject: [PATCH 1/2] 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 cbb5de2e53fccd0b6c93355c730c5104e9fe3700 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 27 Jul 2026 18:13:07 -0500 Subject: [PATCH 2/2] 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)