From a8282ccd8b869dc8c787610c0c53b4ca05cefb84 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 11:41:19 +0000 Subject: [PATCH] Make `secret unlocker add pgp` work on Linux (closes #88) CreatePGPUnlocker got the vault's long-term key from the keychain unlocker's helper, which on every platform but macOS is a stub that always fails. It now calls the vault's GetOrDeriveLongTermKey, as adding a passphrase unlocker does: from the mnemonic, checked against the vault, or else from the current unlocker. That method joins VaultInterface. The test GPG key gains an encryption subkey, and a new test adds a PGP unlocker with the long-term key from the mnemonic and from a passphrase unlocker, then reads a secret through it. Model: opus-5-5 --- TODO.md | 9 +++ internal/cli/unlockers_add_test.go | 74 +++++++++++++++++++++++- internal/cli/unreadable_dir_test.go | 11 +++- internal/secret/derivation_index_test.go | 2 + internal/secret/keychainunlocker_stub.go | 8 --- internal/secret/pgpunlocker.go | 13 +++-- internal/secret/pgpunlocker_test.go | 4 +- internal/secret/secret.go | 1 + internal/secret/secret_test.go | 4 ++ internal/secret/version_test.go | 4 ++ 10 files changed, 110 insertions(+), 20 deletions(-) diff --git a/TODO.md b/TODO.md index 46c18f7..7ba5841 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ Bring the repo into policy compliance in one commit: # Completed Steps +- 2026-10-04: `secret unlocker add pgp` works on Linux + (https://git.eeqj.de/sneak/secret/issues/88). `CreatePGPUnlocker` gets + the vault's long-term key as adding a passphrase unlocker does, with the + vault's `GetOrDeriveLongTermKey`, now part of `VaultInterface`: from the + mnemonic, checked against the vault, or else from the current unlocker. + Before, it used the keychain unlocker's helper, which on every platform + but macOS always failed. A test adds a PGP unlocker for a throwaway GPG + key, getting the long-term key once from the mnemonic and once from a + passphrase unlocker, and reads a secret through the new unlocker. - 2026-10-04: A vault name may use only lowercase ASCII letters, digits, `.`, `-` and `_`, and must not be empty, `.` or `..` (https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md` diff --git a/internal/cli/unlockers_add_test.go b/internal/cli/unlockers_add_test.go index be538ba..cc96960 100644 --- a/internal/cli/unlockers_add_test.go +++ b/internal/cli/unlockers_add_test.go @@ -5,18 +5,88 @@ import ( "path/filepath" "testing" + "git.eeqj.de/sneak/secret/internal/secret" + "git.eeqj.de/sneak/secret/internal/vault" + "github.com/awnumar/memguard" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) // unknownTestGPGUserID is a GPG user ID that no key in the test keyring has. const unknownTestGPGUserID = "not-in-keyring@example.com" +// The secret TestAddPGPUnlocker stores, then reads through the new unlocker. +const ( + addTestSecretName = "api-key" + addTestSecretValue = "value" +) + +// TestAddPGPUnlocker adds a PGP unlocker for a throwaway GPG key to a vault +// with a passphrase unlocker, getting the vault's long-term key from the +// mnemonic or, with the mnemonic unset, from the passphrase unlocker. It +// then reads a secret with neither the mnemonic nor the passphrase set, so +// through the new unlocker, which the add selects. +func TestAddPGPUnlocker(t *testing.T) { + newTestGPGKey(t) + + tests := []struct { + name string + // mnemonic is the mnemonic set while the unlocker is added. + mnemonic string + }{ + {"long-term key from the mnemonic", testMnemonic}, + {"long-term key from the current unlocker", ""}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Setenv(secret.EnvMnemonic, testMnemonic) + t.Setenv(secret.EnvUnlockPassphrase, testPassphrase) + + fs := afero.NewMemMapFs() + vlt, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName) + require.NoError(t, err) + + err = vlt.AddSecret(addTestSecretName, + memguard.NewBufferFromBytes([]byte(addTestSecretValue)), false) + require.NoError(t, err) + + _, err = vlt.CreatePassphraseUnlocker( + memguard.NewBufferFromBytes([]byte(testPassphrase))) + require.NoError(t, err) + + t.Setenv(secret.EnvMnemonic, test.mnemonic) + + instance, cmd := newTestInstance(fs) + cmd.Flags().String("keyid", unreadableTestGPGUserID, "") + require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd)) + + t.Setenv(secret.EnvMnemonic, "") + t.Setenv(secret.EnvUnlockPassphrase, "") + + reopened := vault.NewVault(fs, listTestStateDir, listTestVaultName) + + current, err := reopened.GetCurrentUnlocker() + require.NoError(t, err) + assert.Equal(t, unlockerTypePGP, current.GetType()) + + value, err := reopened.GetSecret(addTestSecretName) + require.NoError(t, err) + + defer value.Destroy() + + assert.Equal(t, addTestSecretValue, value.String()) + }) + } +} + // 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 // lookup moved after anything is written would also come after getting the -// vault's long-term key, which fails first on every platform but macOS -// (https://git.eeqj.de/sneak/secret/issues/88). +// vault's long-term key, which fails first here: this vault's unlockers hold +// no keys. // //nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests func TestAddPGPUnlockerUnknownKey(t *testing.T) { diff --git a/internal/cli/unreadable_dir_test.go b/internal/cli/unreadable_dir_test.go index 7143c9a..eda4486 100644 --- a/internal/cli/unreadable_dir_test.go +++ b/internal/cli/unreadable_dir_test.go @@ -122,7 +122,8 @@ func assertDirEntries(t *testing.T, fs afero.Fs, dir string, want ...string) { } // newTestGPGKey points GNUPGHOME at a fresh directory, generates a GPG key -// without a passphrase there, and returns the key's fingerprint. +// without a passphrase there, with a subkey for encryption, and returns the +// key's fingerprint. func newTestGPGKey(t *testing.T) string { t.Helper() @@ -151,6 +152,14 @@ func newTestGPGKey(t *testing.T) string { fingerprint, err := secret.ResolveGPGKeyFingerprint(unreadableTestGPGUserID) require.NoError(t, err) + //nolint:gosec // G204: fingerprint is the test key's, as gpg printed it + output, err = exec.CommandContext(t.Context(), "gpg", "--batch", + "--pinentry-mode", "loopback", "--passphrase", "", + "--quick-add-key", fingerprint, "cv25519", "encr", "never", + ).CombinedOutput() + require.NoError(t, err, "adding the test GPG key's encryption subkey: %s", + output) + return fingerprint } diff --git a/internal/secret/derivation_index_test.go b/internal/secret/derivation_index_test.go index ccbac13..dc6a542 100644 --- a/internal/secret/derivation_index_test.go +++ b/internal/secret/derivation_index_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "filippo.io/age" "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" "github.com/spf13/afero" @@ -32,6 +33,7 @@ func (v *realVault) GetFilesystem() afero.Fs { return v.fs } // Unused by getLongTermPrivateKey — these satisfy VaultInterface. func (v *realVault) AddSecret(string, *memguard.LockedBuffer, bool) error { panic("not used") } func (v *realVault) GetCurrentUnlocker() (Unlocker, error) { panic("not used") } +func (v *realVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { panic("not used") } func (v *realVault) CreatePassphraseUnlocker(*memguard.LockedBuffer) (*PassphraseUnlocker, error) { panic("not used") } diff --git a/internal/secret/keychainunlocker_stub.go b/internal/secret/keychainunlocker_stub.go index c6de515..2008079 100644 --- a/internal/secret/keychainunlocker_stub.go +++ b/internal/secret/keychainunlocker_stub.go @@ -6,7 +6,6 @@ import ( "errors" "filippo.io/age" - "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -79,10 +78,3 @@ func (k *KeychainUnlocker) Remove() error { func CreateKeychainUnlocker(_ afero.Fs, _ string) (*KeychainUnlocker, error) { return nil, errKeychainNotSupported } - -// getLongTermPrivateKey returns an error on non-Darwin platforms -func getLongTermPrivateKey( - _ afero.Fs, _ VaultInterface, -) (*memguard.LockedBuffer, error) { - return nil, errKeychainNotSupported -} diff --git a/internal/secret/pgpunlocker.go b/internal/secret/pgpunlocker.go index 4eafaa6..46b8211 100644 --- a/internal/secret/pgpunlocker.go +++ b/internal/secret/pgpunlocker.go @@ -277,7 +277,7 @@ func CreatePGPUnlocker( // Step 2: Encrypt the long-term private key to the new keypair, and the // keypair's private key to the GPG key encryptedLtPrivKey, encryptedAgePrivKey, err := encryptPGPUnlockerKeys( - fs, vault, ageIdentity, gpgKeyID) + vault, ageIdentity, gpgKeyID) if err != nil { return nil, err } @@ -316,14 +316,15 @@ func CreatePGPUnlocker( // to the new PGP unlocker's age keypair, and that keypair's private key // encrypted to the GPG key gpgKeyID. func encryptPGPUnlockerKeys( - fs afero.Fs, vault VaultInterface, - ageIdentity *age.X25519Identity, gpgKeyID string, + vault VaultInterface, ageIdentity *age.X25519Identity, gpgKeyID string, ) ([]byte, []byte, error) { - // Get or derive the long-term private key - ltPrivKeyData, err := getLongTermPrivateKey(fs, vault) + // From the mnemonic or the current unlocker, as for a passphrase unlocker + ltIdentity, err := vault.GetOrDeriveLongTermKey() if err != nil { - return nil, nil, err + return nil, nil, fmt.Errorf("failed to get long-term key: %w", err) } + + ltPrivKeyData := memguard.NewBufferFromBytes([]byte(ltIdentity.String())) defer ltPrivKeyData.Destroy() encryptedLtPrivKey, err := EncryptToRecipient( diff --git a/internal/secret/pgpunlocker_test.go b/internal/secret/pgpunlocker_test.go index 7d95a32..435a9cd 100644 --- a/internal/secret/pgpunlocker_test.go +++ b/internal/secret/pgpunlocker_test.go @@ -40,9 +40,7 @@ func installFakeGPG(t *testing.T) { // TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at // getting the vault's long-term key, which used to come after part of the // unlocker was written, and asserts that nothing is written. Getting the key -// fails because on macOS there is no mnemonic and no current unlocker, and -// on every other platform it always fails -// (https://git.eeqj.de/sneak/secret/issues/88). +// fails because there is no mnemonic and no current unlocker. func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) { installFakeGPG(t) t.Setenv(secret.EnvMnemonic, "") diff --git a/internal/secret/secret.go b/internal/secret/secret.go index b6aa789..cbeaa0f 100644 --- a/internal/secret/secret.go +++ b/internal/secret/secret.go @@ -35,6 +35,7 @@ type VaultInterface interface { GetName() string GetFilesystem() afero.Fs GetCurrentUnlocker() (Unlocker, error) + GetOrDeriveLongTermKey() (*age.X25519Identity, error) CreatePassphraseUnlocker( passphrase *memguard.LockedBuffer) (*PassphraseUnlocker, error) } diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go index e76dd63..54496be 100644 --- a/internal/secret/secret_test.go +++ b/internal/secret/secret_test.go @@ -107,6 +107,10 @@ func (m *MockVault) GetCurrentUnlocker() (Unlocker, error) { return nil, errNotImplementedInMock } +func (m *MockVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { + return nil, errNotImplementedInMock +} + func (m *MockVault) CreatePassphraseUnlocker( _ *memguard.LockedBuffer, ) (*PassphraseUnlocker, error) { diff --git a/internal/secret/version_test.go b/internal/secret/version_test.go index a1f2cda..10bc5a2 100644 --- a/internal/secret/version_test.go +++ b/internal/secret/version_test.go @@ -87,6 +87,10 @@ func (m *MockVersionVault) GetCurrentUnlocker() (secret.Unlocker, error) { return nil, errNotImplementedInMock } +func (m *MockVersionVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { + return nil, errNotImplementedInMock +} + func (m *MockVersionVault) CreatePassphraseUnlocker( _ *memguard.LockedBuffer, ) (*secret.PassphraseUnlocker, error) { -- 2.54.0