Use gpg.conf's default-key when unlocker add pgp has no --keyid (closes #141)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit is contained in:
@@ -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: Removing the current unlocker no longer leaves the vault unable to
|
||||
open through the unlockers it still has
|
||||
(https://git.eeqj.de/sneak/secret/issues/137). `current-unlocker` went on
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 <alice@example.com>`,
|
||||
"Doe, Alice <alice@example.com>",
|
||||
},
|
||||
{"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
|
||||
|
||||
Reference in New Issue
Block a user