From 9ae4b74ee7e4fa32647877af296750f65fd48b78 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 19:25:58 +0000 Subject: [PATCH] Use gpg.conf's default-key when unlocker add pgp has no --keyid (closes #141) getDefaultGPGKey took the default-key value from `gpgconf --list-options gpg` as printed. gpgconf prints a string value with a leading `"` and percent-escaped, so gpg rejected the key ID. The parsing moved into parseDefaultGPGKey, which drops the `"` and percent-decodes the rest with url.PathUnescape, which leaves `+` alone. A value that does not decode is an error, not a fallback to the first secret key. Rule suppressed: gosec G703 on the test's write of gpg.conf into the GNUPGHOME directory newTestGPGKey made. Model: opus-5-5 --- TODO.md | 6 +++ internal/cli/unlockers.go | 35 +++++++++++--- internal/cli/unlockers_add_test.go | 73 ++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/TODO.md b/TODO.md index 857a05f..175edf8 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,12 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-14: `secret unlocker add pgp` without `--keyid` uses the `default-key` + set in `gpg.conf` (https://git.eeqj.de/sneak/secret/issues/141). The key ID + came from `gpgconf --list-options gpg`, which prints a string value with a + leading `"` and percent-escaped, and was passed to gpg undecoded, so gpg + rejected it. The value is now decoded first. With no `default-key` set, the + first secret key is still used. - 2026-10-14: `secret generate mnemonic`, `secret vault list` and `secret unlocker list` print their output to stdout, as `secret list` does, so it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138). diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 0a03189..87b3d7e 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -7,6 +7,7 @@ import ( "fmt" "log" "maps" + "net/url" "os" "os/exec" "path/filepath" @@ -66,12 +67,13 @@ func getDefaultGPGKey() (string, error) { output, err := cmd.Output() if err == nil { - for line := range strings.SplitSeq(string(output), "\n") { - fields := strings.Split(line, ":") - if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" { - // The default key is in field 10 (index 9) - return fields[9], nil - } + defaultKey, parseErr := parseDefaultGPGKey(string(output)) + if parseErr != nil { + return "", parseErr + } + + if defaultKey != "" { + return defaultKey, nil } } @@ -98,6 +100,27 @@ func getDefaultGPGKey() (string, error) { return "", errNoGPGSecretKeys } +// parseDefaultGPGKey returns the default-key value in the output of +// `gpgconf --list-options gpg`, or "" when gpg.conf sets none. gpgconf(1) +// prints a string value with a leading `"` and percent-escaped. +func parseDefaultGPGKey(gpgconfOutput string) (string, error) { + for line := range strings.SplitSeq(gpgconfOutput, "\n") { + fields := strings.Split(line, ":") + if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" { + // The default key is in field 10 (index 9) + keyID, err := url.PathUnescape(strings.TrimPrefix(fields[9], `"`)) + if err != nil { + return "", fmt.Errorf("failed to decode gpgconf default-key %s: %w", + fields[9], err) + } + + return keyID, nil + } + } + + return "", nil +} + func newUnlockerCmd() *cobra.Command { cmd := &cobra.Command{ Use: "unlocker", diff --git a/internal/cli/unlockers_add_test.go b/internal/cli/unlockers_add_test.go index 09ef065..742e5c1 100644 --- a/internal/cli/unlockers_add_test.go +++ b/internal/cli/unlockers_add_test.go @@ -2,6 +2,8 @@ package cli import ( + "bytes" + "os" "path/filepath" "strings" "testing" @@ -83,6 +85,77 @@ func TestAddPGPUnlocker(t *testing.T) { } } +// TestAddPGPUnlockerDefaultKey is a regression test for +// https://git.eeqj.de/sneak/secret/issues/141: with default-key set in +// gpg.conf and no --keyid, `secret unlocker add pgp` uses that key. +// gpg.conf names the key by its user ID, and the printed key ID must be that +// user ID. The first secret key, which is used when gpg.conf sets no +// default-key, prints as a hex key ID. +func TestAddPGPUnlockerDefaultKey(t *testing.T) { + newTestGPGKey(t) + // The key comes from neither --keyid nor the environment. + t.Setenv(secret.EnvGPGKeyID, "") + + gpgConf := filepath.Join(os.Getenv("GNUPGHOME"), "gpg.conf") + //nolint:gosec // G703: GNUPGHOME is the directory newTestGPGKey made + require.NoError(t, os.WriteFile(gpgConf, + []byte("default-key "+unreadableTestGPGUserID+"\n"), 0o600)) + + fs := afero.NewMemMapFs() + _, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName, + testMnemonicBuffer(t), nil) + require.NoError(t, err) + + instance, cmd := newTestInstance(fs) + instance.Mnemonic = testMnemonicBuffer(t) + + var output bytes.Buffer + cmd.SetOut(&output) + + require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd)) + assert.Contains(t, output.String(), + "Using default GPG key: "+unreadableTestGPGUserID+"\n") +} + +// TestParseDefaultGPGKey is a regression test for +// https://git.eeqj.de/sneak/secret/issues/141: gpgconf prints the +// default-key value with a leading `"` and percent-escaped, and the key ID +// is that value decoded. +func TestParseDefaultGPGKey(t *testing.T) { + t.Parallel() + + // gpgconfLine is the default-key line of `gpgconf --list-options gpg` + // up to the value, its last field. + const gpgconfLine = "default-key:0:0:use NAME as default secret key:" + + "1:1:NAME:::" + + tests := []struct { + name string + value string + want string + }{ + { + "fingerprint", `"56163D7F3FAFF709608855BEC1A360216D007F41`, + "56163D7F3FAFF709608855BEC1A360216D007F41", + }, + { + "user ID with a comma", `"Doe%2c Alice `, + "Doe, Alice ", + }, + {"no default-key in gpg.conf", "", ""}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + keyID, err := parseDefaultGPGKey(gpgconfLine + test.value + "\n") + require.NoError(t, err) + assert.Equal(t, test.want, keyID) + }) + } +} + // TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key // the keyring does not hold fails at looking up the key's fingerprint and // leaves no new unlocker directory. The error must come from the lookup: a