Compare commits

..
2 Commits
Author SHA1 Message Date
sneak c7d2e28f48 Say the mnemonic still opens a vault its unlocker cannot (closes #47)
check / check (push) Failing after 2s
When a vault cannot be opened through its current unlocker, the error now
ends by naming the vault, saying that it still opens with its mnemonic,
and that 'secret unlocker add passphrase' run with SB_SECRET_MNEMONIC set
gives it a new unlocker, after 'secret vault select' when it is not the
current vault. Only when the vault metadata records the key the mnemonic
derives, and not when the passphrase could not be read. 'secret encrypt'
and 'secret decrypt' read the key secret through vault.GetSecret, and
Secret.GetValue with its helpers is removed. An unreadable 'current'
file's error names 'secret version list' and 'secret version promote'.
Causes stay wrapped.

Model: opus-5-5
2026-10-04 19:33:38 +00:00
clawbot 0e6a4afb71 Make an unlocker's ID the name of its directory (closes #98)
check / check (push) Failing after 2s
Keychain and Secure Enclave unlocker IDs were the creation time to the
minute plus the host name, and passphrase unlocker IDs the time to the
minute, so two created within one minute shared an ID, and `unlocker
select`, `unlocker remove` and the selection after `unlocker add` acted on
the older one. Every unlocker's ID is now its directory name, unique in
its vault. `vault.ListUnlockers` returns each unlocker's metadata keyed by
that name, so `unlocker list` and shell completion no longer find IDs by
matching metadata. PGP unlocker IDs were `pgp-<fingerprint>`; a second
PGP unlocker for one key is refused by comparing fingerprints in metadata.

Model: opus-5-5
2026-10-04 21:25:00 +02:00
24 changed files with 334 additions and 668 deletions
+3 -1
View File
@@ -211,7 +211,9 @@ Generates and stores a random secret.
#### `secret unlocker list [--json]` / `secret unlocker ls` #### `secret unlocker list [--json]` / `secret unlocker ls`
Lists all unlockers in the current vault with their metadata. Lists all unlockers in the current vault with their metadata. An unlocker's ID,
which `secret unlocker select` and `secret unlocker remove` take, is the name of
its directory in `unlockers.d`.
#### `secret unlocker add <type> [options]` #### `secret unlocker add <type> [options]`
+30 -9
View File
@@ -18,21 +18,42 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps # Completed Steps
- 2026-10-04: When the vault cannot be opened through its current unlocker, - 2026-10-04: When a vault cannot be opened through its current unlocker,
because a file the unlocker needs is missing or damaged, its keychain item because a file the unlocker needs is missing or damaged, its keychain item
or Secure Enclave key is gone, or the passphrase is wrong, the error now or Secure Enclave key is gone, or the passphrase is wrong, the error now
ends by saying that the vault still opens with its mnemonic, and that ends by naming the vault, saying that it still opens with its mnemonic,
`secret unlocker add passphrase`, run with `SB_SECRET_MNEMONIC` set to it, and that `secret unlocker add passphrase`, run with `SB_SECRET_MNEMONIC`
gives the vault a new unlocker set to it, gives the vault a new unlocker; for a vault that is not the
(https://git.eeqj.de/sneak/secret/issues/47). Before, it ended with the current one, as in `secret move` between vaults, it says to run
bare cause. The advice is given only when the vault metadata records the `secret vault select` first (https://git.eeqj.de/sneak/secret/issues/47).
key the mnemonic derives, so not for a vault created without a mnemonic. Before, it ended with the bare cause. The advice is given only when the
`secret vault import` is not named: it refuses a vault that has a vault metadata records the key the mnemonic derives, so not for a vault
created without a mnemonic, and not when the passphrase could not be read
at all. `secret vault import` is not named: it refuses a vault that has a
long-term key. `secret encrypt` and `secret decrypt` now read the key long-term key. `secret encrypt` and `secret decrypt` now read the key
secret through `vault.GetSecret`, as `secret get` does, so they give the secret through `vault.GetSecret`, as `secret get` does, so they give the
same advice. When a secret's `current` file cannot be read, the error says same advice; `Secret.GetValue`, the other way to get the long-term key, is
removed. When a secret's `current` file cannot be read, the error says
that `secret version list` lists its versions and `secret version promote` that `secret version list` lists its versions and `secret version promote`
makes one current. The causes stay wrapped. makes one current. The causes stay wrapped.
- 2026-10-04: An unlocker's ID is the name of its directory in `unlockers.d`,
so no two unlockers of a vault share one
(https://git.eeqj.de/sneak/secret/issues/98). Before, a keychain or Secure
Enclave unlocker's ID was its creation time to the minute and the host name,
and a passphrase unlocker's the time to the minute, so two created within a
minute shared an ID, and `unlocker select`, `unlocker remove` and the
selection `unlocker add` makes acted on the older one. A PGP unlocker's ID
was `pgp-` and its key's fingerprint; a second PGP unlocker for a key is
still refused, now by comparing the fingerprint in the other unlockers'
metadata. `unlocker list` and the shell completion of `unlocker select` and
`unlocker remove` take each ID from the directory the unlocker was read
from, no longer by matching metadata, so two unlockers with the same
metadata are listed apart; an unlocker of an unknown type is listed under
its directory name, and completion now offers Secure Enclave unlockers too.
The keychain and Secure Enclave code was type-checked by
`script/lint-darwin`, never run; a test on Linux lists, completes, selects
and removes each of two passphrase unlockers with the same metadata by its
own ID.
- 2026-10-04: README's Storage Architecture, `secret version promote`, - 2026-10-04: README's Storage Architecture, `secret version promote`,
Technical Details and Testing text matches the code Technical Details and Testing text matches the code
(https://git.eeqj.de/sneak/secret/issues/102). `current` and (https://git.eeqj.de/sneak/secret/issues/102). `current` and
+6 -29
View File
@@ -1,10 +1,10 @@
package cli package cli
import ( import (
"path/filepath" "maps"
"slices"
"strings" "strings"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/internal/vault"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/spf13/cobra" "github.com/spf13/cobra"
@@ -44,7 +44,7 @@ func getSecretNamesCompletionFunc(fs afero.Fs, stateDir string) func(
} }
// getUnlockerIDsCompletionFunc returns a completion function that provides // getUnlockerIDsCompletionFunc returns a completion function that provides
// unlocker IDs // unlocker IDs, the names of the unlockers' directories in unlockers.d
func getUnlockerIDsCompletionFunc(fs afero.Fs, stateDir string) func( func getUnlockerIDsCompletionFunc(fs afero.Fs, stateDir string) func(
cmd *cobra.Command, args []string, toComplete string, cmd *cobra.Command, args []string, toComplete string,
) ([]string, cobra.ShellCompDirective) { ) ([]string, cobra.ShellCompDirective) {
@@ -57,38 +57,15 @@ func getUnlockerIDsCompletionFunc(fs afero.Fs, stateDir string) func(
return nil, cobra.ShellCompDirectiveNoFileComp return nil, cobra.ShellCompDirectiveNoFileComp
} }
// Get unlocker metadata list unlockerMetadata, err := vlt.ListUnlockers()
unlockerMetadataList, err := vlt.ListUnlockers()
if err != nil { if err != nil {
return nil, cobra.ShellCompDirectiveNoFileComp return nil, cobra.ShellCompDirectiveNoFileComp
} }
// Get vault directory
vaultDir, err := vlt.GetDirectory()
if err != nil {
return nil, cobra.ShellCompDirectiveNoFileComp
}
// Collect unlocker IDs
var completions []string var completions []string
unlockersDir := filepath.Join(vaultDir, "unlockers.d") for _, id := range slices.Sorted(maps.Keys(unlockerMetadata)) {
if strings.HasPrefix(id, toComplete) {
for _, metadata := range unlockerMetadataList {
// Get the actual unlocker ID by creating the unlocker instance
id, err := findUnlockerIDByMetadata(
fs, unlockersDir, metadata, false,
)
if err != nil {
secret.Warn(
"Could not read unlockers directory during completion, "+
"skipping unlocker",
"unlockers_dir", unlockersDir, "error", err)
continue
}
if id != "" && strings.HasPrefix(id, toComplete) {
completions = append(completions, id) completions = append(completions, id)
} }
} }
+4 -3
View File
@@ -101,7 +101,8 @@ func newRemoval(t *testing.T, command string) removal {
} }
fs, workDir, older := newConfirmTestVaults(t, unlockers) fs, workDir, older := newConfirmTestVaults(t, unlockers)
unlockerID := "pgp-" + listTestGPGKeyID + "A" // The first unlocker's directory name, written by newConfirmTestVaults
unlockerID := "pgp-0"
removeFirstUnlocker := func(cli *Instance, cmd *cobra.Command, force bool) error { removeFirstUnlocker := func(cli *Instance, cmd *cobra.Command, force bool) error {
return cli.UnlockersRemove(unlockerID, force, cmd) return cli.UnlockersRemove(unlockerID, force, cmd)
@@ -142,7 +143,7 @@ func newRemoval(t *testing.T, command string) removal {
return removal{ return removal{
fs: fs, fs: fs,
run: removeFirstUnlocker, run: removeFirstUnlocker,
removed: filepath.Join(workDir, "unlockers.d", "pgp-0"), removed: filepath.Join(workDir, "unlockers.d", unlockerID),
question: "Permanently remove unlocker '" + unlockerID + question: "Permanently remove unlocker '" + unlockerID +
"' from vault 'work'? It is not the vault's last unlocker.", "' from vault 'work'? It is not the vault's last unlocker.",
} }
@@ -150,7 +151,7 @@ func newRemoval(t *testing.T, command string) removal {
return removal{ return removal{
fs: fs, fs: fs,
run: removeFirstUnlocker, run: removeFirstUnlocker,
removed: filepath.Join(workDir, "unlockers.d", "pgp-0"), removed: filepath.Join(workDir, "unlockers.d", unlockerID),
question: "Permanently remove unlocker '" + unlockerID + question: "Permanently remove unlocker '" + unlockerID +
"', the last unlocker of vault 'work', which holds 1 " + "', the last unlocker of vault 'work', which holds 1 " +
"secret(s)? Without an unlocker the vault opens only " + "secret(s)? Without an unlocker the vault opens only " +
+107 -15
View File
@@ -1,11 +1,12 @@
// Unlock Failure Tests // Unlock Failure Tests
// //
// When the vault cannot be opened through its current unlocker, because a // When a vault cannot be opened through its current unlocker, because a
// file the unlocker needs is missing or the passphrase is wrong, the error // file the unlocker needs is missing or the passphrase is wrong, the error
// keeps its cause and ends by saying that the mnemonic still opens the // keeps its cause and ends by saying that the mnemonic still opens that
// vault, but only for a vault that the mnemonic does open. When a secret's // vault, but only for a vault that the mnemonic does open, and not when the
// current file is missing, the error says how to make a version current // passphrase could not be read at all. When a secret's current file is
// again. Each test that pins such advice also follows it. // missing, the error says how to make a version current again. Each test
// that pins such advice also follows it.
package cli_test package cli_test
@@ -13,11 +14,13 @@ import (
"bytes" "bytes"
"io" "io"
"os" "os"
"os/exec"
"path/filepath" "path/filepath"
"testing" "testing"
"filippo.io/age" "filippo.io/age"
"git.eeqj.de/sneak/secret/internal/cli" "git.eeqj.de/sneak/secret/internal/cli"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/internal/vault"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
"github.com/spf13/afero" "github.com/spf13/afero"
@@ -27,11 +30,11 @@ import (
) )
const ( const (
// mnemonicAdvice ends the error when a vault that its mnemonic opens // mnemonicAdvice ends the error when the current vault "default", which
// cannot be opened through its current unlocker. // its mnemonic opens, cannot be opened through its current unlocker.
mnemonicAdvice = "; the vault still opens with its mnemonic: run " + mnemonicAdvice = "; the vault 'default' still opens with its mnemonic: " +
"'secret unlocker add passphrase' with SB_SECRET_MNEMONIC set to " + "run 'secret unlocker add passphrase' with SB_SECRET_MNEMONIC set " +
"the mnemonic to give it a new unlocker" "to the mnemonic to give it a new unlocker"
// versionAdvice ends the error when a secret's current file cannot be // versionAdvice ends the error when a secret's current file cannot be
// read. // read.
@@ -44,6 +47,18 @@ const (
unlockTestVaultDir = testStateDir + "/vaults.d/default" unlockTestVaultDir = testStateDir + "/vaults.d/default"
) )
// currentUnlockerDir returns the directory of the current unlocker of the
// vault in vaultDir on fs.
func currentUnlockerDir(t *testing.T, fs afero.Fs, vaultDir string) string {
t.Helper()
unlockerName, err := afero.ReadFile(fs,
filepath.Join(vaultDir, "current-unlocker"))
require.NoError(t, err)
return filepath.Join(vaultDir, "unlockers.d", string(unlockerName))
}
// newUnlockTestCLI returns the directory of the current unlocker of the // newUnlockTestCLI returns the directory of the current unlocker of the
// vault "default" on fs, a copy of the vaults of newTwoVaultFs, and a CLI // vault "default" on fs, a copy of the vaults of newTwoVaultFs, and a CLI
// instance on fs that has the unlock passphrase, as from the environment, // instance on fs that has the unlock passphrase, as from the environment,
@@ -51,15 +66,11 @@ const (
func newUnlockTestCLI(t *testing.T, fs afero.Fs) (string, *cli.Instance) { func newUnlockTestCLI(t *testing.T, fs afero.Fs) (string, *cli.Instance) {
t.Helper() t.Helper()
unlockerName, err := afero.ReadFile(fs,
filepath.Join(unlockTestVaultDir, "current-unlocker"))
require.NoError(t, err)
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir) c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase)) c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
t.Cleanup(c.UnlockPassphrase.Destroy) t.Cleanup(c.UnlockPassphrase.Destroy)
return filepath.Join(unlockTestVaultDir, "unlockers.d", string(unlockerName)), c return currentUnlockerDir(t, fs, unlockTestVaultDir), c
} }
// discardCmd returns a command whose output is discarded. // discardCmd returns a command whose output is discarded.
@@ -173,6 +184,87 @@ func TestWrongPassphraseNamesMnemonic(t *testing.T) {
noMatch.Error()+mnemonicAdvice) noMatch.Error()+mnemonicAdvice)
} }
// TestMoveUnlockFailureNamesVault checks the error of `secret move` into
// the vault "work", which is not the current vault, when "work" cannot be
// opened through its current unlocker: the advice names "work" and says to
// select it first, since `secret unlocker add` acts on the current vault.
// The test then follows that advice, and the move succeeds.
func TestMoveUnlockFailureNamesVault(t *testing.T) {
t.Parallel()
fs := newTwoVaultFs(t)
_, c := newUnlockTestCLI(t, fs)
path := filepath.Join(
currentUnlockerDir(t, fs, testStateDir+"/vaults.d/work"), "priv.age")
require.NoError(t, fs.Remove(path))
err := c.MoveSecret(discardCmd(), "default:x", "work:y", false)
var cause *os.PathError
require.ErrorAs(t, err, &cause)
assert.Equal(t, path, cause.Path)
require.EqualError(t, err, "failed to unlock destination vault 'work': "+
"failed to get unlocker identity: failed to read unlocker private key: "+
cause.Error()+"; the vault 'work' still opens with its mnemonic: "+
"run 'secret vault select work', then 'secret unlocker add passphrase' "+
"with SB_SECRET_MNEMONIC set to the mnemonic to give it a new unlocker")
require.NoError(t, c.SelectVault(discardCmd(), "work"))
c.Mnemonic = testMnemonicBuffer(t)
require.NoError(t, c.UnlockersAdd("passphrase", discardCmd()))
c.Mnemonic = nil
require.NoError(t, c.MoveSecret(discardCmd(), "default:x", "work:y", false))
assert.Equal(t, "value", getSecret(t, c, "y"))
}
// TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
// binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a
// terminal, so the passphrase cannot be read. The unlocker was not tried,
// and adding one would need a passphrase read the same way, so the error
// is the cause alone, without the advice to use the mnemonic.
func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
t.Parallel()
stateDir := t.TempDir()
mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
defer mnemonic.Destroy()
passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
defer passphrase.Destroy()
vlt, err := vault.CreateVault(
afero.NewOsFs(), stateDir, "default", mnemonic, passphrase)
require.NoError(t, err)
value := memguard.NewBufferFromBytes([]byte("value"))
defer value.Destroy()
require.NoError(t, vlt.AddSecret("x", value, false))
//nolint:gosec // G204: test executes the freshly built secret binary
cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), "get", "x")
cmd.Env = []string{
secret.EnvStateDir + "=" + stateDir,
"PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"),
}
output, err := cmd.CombinedOutput()
require.Error(t, err)
assert.Equal(t, "Error: failed to unlock vault: "+
"failed to get long-term key: failed to get unlocker identity: "+
"failed to read passphrase: cannot read passphrase from non-terminal "+
"stdin (piped input or script). Please set the SB_UNLOCK_PASSPHRASE "+
"environment variable or run interactively\n", string(output))
}
// TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and
// `secret decrypt`, reading the key secret, end with the same advice as // `secret decrypt`, reading the key secret, end with the same advice as
// `secret get` when the vault cannot be opened through its current // `secret get` when the vault cannot be opened through its current
+21 -146
View File
@@ -6,6 +6,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"log" "log"
"maps"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
@@ -313,91 +314,8 @@ func newUnlockerSelectCmd() *cobra.Command {
} }
} }
// unlockerIDFromDir constructs an unlocker of the given metadata type // UnlockersList lists unlockers in the current vault, each under its ID,
// rooted at unlockerDir and returns its ID. Returns "" for unknown types // the name of its directory in unlockers.d
// and, when includeSecureEnclave is false, for secure enclave unlockers.
func unlockerIDFromDir(
fs afero.Fs, unlockerDir string, metadata secret.UnlockerMetadata,
includeSecureEnclave bool,
) string {
// Create the appropriate unlocker instance
var unlocker secret.Unlocker
switch metadata.Type {
case unlockerTypePassphrase:
unlocker = secret.NewPassphraseUnlocker(fs, unlockerDir, metadata)
case unlockerTypeKeychain:
unlocker = secret.NewKeychainUnlocker(fs, unlockerDir, metadata)
case unlockerTypePGP:
unlocker = secret.NewPGPUnlocker(fs, unlockerDir, metadata)
case unlockerTypeSecureEnclave:
if includeSecureEnclave {
unlocker = secret.NewSecureEnclaveUnlocker(fs, unlockerDir, metadata)
}
}
if unlocker == nil {
return ""
}
return unlocker.GetID()
}
// findUnlockerIDByMetadata scans unlockersDir for the directory whose
// stored metadata matches the given type and creation time and returns
// the matching unlocker's ID. It returns ("", nil) when the directory is
// readable but holds no match, and a non-nil error when the directory
// itself cannot be read. Callers must distinguish the two: an unreadable
// directory means the unlocker's real ID is unknowable, so the entry has
// to be skipped rather than reported under a synthesized ID.
//
// A metadata file that cannot be read or parsed is skipped without a
// warning: every caller gets metadata from vault.ListUnlockers first,
// which has already warned about that directory.
func findUnlockerIDByMetadata(
fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
includeSecureEnclave bool,
) (string, error) {
files, err := afero.ReadDir(fs, unlockersDir)
if err != nil {
return "", fmt.Errorf(
"failed to read unlockers directory %s: %w", unlockersDir, err,
)
}
for _, file := range files {
if !file.IsDir() {
continue
}
unlockerDir := filepath.Join(unlockersDir, file.Name())
metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
// Check if this is the right unlocker by comparing metadata
metadataBytes, err := afero.ReadFile(fs, metadataPath)
if err != nil {
continue
}
var diskMetadata secret.UnlockerMetadata
err = json.Unmarshal(metadataBytes, &diskMetadata)
if err != nil {
continue
}
// Match by type and creation time
if diskMetadata.Type == metadata.Type &&
diskMetadata.CreatedAt.Equal(metadata.CreatedAt) {
return unlockerIDFromDir(fs, unlockerDir, diskMetadata,
includeSecureEnclave), nil
}
}
return "", nil
}
// UnlockersList lists unlockers in the current vault
func (cli *Instance) UnlockersList(jsonOutput bool) error { func (cli *Instance) UnlockersList(jsonOutput bool) error {
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
@@ -413,58 +331,23 @@ func (cli *Instance) UnlockersList(jsonOutput bool) error {
currentUnlockerID = currentUnlocker.GetID() currentUnlockerID = currentUnlocker.GetID()
} }
// Get the metadata first unlockerMetadata, err := vlt.ListUnlockers()
unlockerMetadataList, err := vlt.ListUnlockers()
if err != nil { if err != nil {
return err return err
} }
// Load actual unlocker objects to get the proper IDs
var unlockers []UnlockerInfo var unlockers []UnlockerInfo
for _, metadata := range unlockerMetadataList { for _, unlockerID := range slices.Sorted(maps.Keys(unlockerMetadata)) {
// Create unlocker instance to get the proper ID metadata := unlockerMetadata[unlockerID]
vaultDir, err := vlt.GetDirectory()
if err != nil {
secret.Warn("Could not get vault directory while listing unlockers",
"error", err)
continue unlockers = append(unlockers, UnlockerInfo{
} ID: unlockerID,
// Find the unlocker directory by type and created time
unlockersDir := filepath.Join(vaultDir, "unlockers.d")
unlockerID, err := findUnlockerIDByMetadata(
cli.fs, unlockersDir, metadata, true,
)
if err != nil {
secret.Warn("Could not read unlockers directory, skipping unlocker",
"unlockers_dir", unlockersDir, "error", err)
continue
}
// Get the proper ID using the unlocker's ID() method
var properID string
if unlockerID != "" {
properID = unlockerID
} else {
// Generate ID as fallback
properID = fmt.Sprintf("%s-%s",
metadata.CreatedAt.Format("2006-01-02.15.04"), metadata.Type)
secret.Warn("Could not create unlocker instance, using fallback ID",
"fallback_id", properID, "type", metadata.Type)
}
unlockerInfo := UnlockerInfo{
ID: properID,
Type: metadata.Type, Type: metadata.Type,
CreatedAt: metadata.CreatedAt, CreatedAt: metadata.CreatedAt,
Flags: metadata.Flags, Flags: metadata.Flags,
IsCurrent: properID == currentUnlockerID, IsCurrent: unlockerID == currentUnlockerID,
} })
unlockers = append(unlockers, unlockerInfo)
} }
if jsonOutput { if jsonOutput {
@@ -697,9 +580,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
} }
// Check if this GPG key is already added // Check if this GPG key is already added
expectedID := "pgp-" + fingerprint exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
exists, err := cli.checkUnlockerExists(vlt, expectedID)
if err != nil { if err != nil {
return fmt.Errorf( return fmt.Errorf(
"could not check whether GPG key %s is already an unlocker: %w", "could not check whether GPG key %s is already an unlocker: %w",
@@ -804,13 +685,7 @@ func (cli *Instance) findUnlockerToRemove(
} }
if len(unlockers) == 1 { if len(unlockers) == 1 {
lastID, err := findUnlockerIDByMetadata( _, found.last = unlockers[unlockerID]
cli.fs, unlockersDir, unlockers[0], true)
if err != nil {
return unlockerToRemove{}, err
}
found.last = lastID == unlockerID
} }
// unlockerID may instead name a directory left out of the list. If its // unlockerID may instead name a directory left out of the list. If its
@@ -889,16 +764,16 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
return vlt.SelectUnlocker(unlockerID) return vlt.SelectUnlocker(unlockerID)
} }
// checkUnlockerExists reports whether the vault already has an unlocker // pgpUnlockerExists reports whether the vault already has a PGP unlocker
// with the given ID. It returns an error, and no answer, when unlockers.d // for the GPG key with the given fingerprint. It returns an error, and no
// or an unlocker's metadata file cannot be read; the caller must then not // answer, when unlockers.d or an unlocker's metadata file cannot be read;
// create the unlocker. It reads unlockers.d itself because // the caller must then not create the unlocker. It reads unlockers.d itself
// vault.ListUnlockers skips an unlocker it cannot read, which suits // because vault.ListUnlockers skips an unlocker it cannot read, which suits
// `unlocker list` but not this check: the skipped unlocker may be the // `unlocker list` but not this check: the skipped unlocker may be the
// duplicate. A directory whose metadata file is missing or corrupt is not // duplicate. A directory whose metadata file is missing or corrupt is not
// a working unlocker and is passed over. // a working unlocker and is passed over.
func (cli *Instance) checkUnlockerExists( func (cli *Instance) pgpUnlockerExists(
vlt *vault.Vault, unlockerID string, vlt *vault.Vault, fingerprint string,
) (bool, error) { ) (bool, error) {
vaultDir, err := vlt.GetDirectory() vaultDir, err := vlt.GetDirectory()
if err != nil { if err != nil {
@@ -937,14 +812,14 @@ func (cli *Instance) checkUnlockerExists(
) )
} }
var metadata secret.UnlockerMetadata var metadata secret.PGPUnlockerMetadata
err = json.Unmarshal(metadataBytes, &metadata) err = json.Unmarshal(metadataBytes, &metadata)
if err != nil { if err != nil {
continue continue
} }
if unlockerIDFromDir(cli.fs, unlockerDir, metadata, true) == unlockerID { if metadata.Type == unlockerTypePGP && metadata.GPGKeyID == fingerprint {
return true, nil return true, nil
} }
} }
+2 -2
View File
@@ -46,7 +46,7 @@ func TestUnlockerSelectSkipsCorruptUnlocker(t *testing.T) {
fs := newCorruptUnlockerVault(t) fs := newCorruptUnlockerVault(t)
instance, _ := newTestInstance(fs) instance, _ := newTestInstance(fs)
require.NoError(t, instance.UnlockerSelect("pgp-"+listTestGPGKeyID+"B")) require.NoError(t, instance.UnlockerSelect(listTestUnlockerDirTwo))
current, err := afero.ReadFile(fs, current, err := afero.ReadFile(fs,
filepath.Join(testVaultDir(listTestVaultName), "current-unlocker")) filepath.Join(testVaultDir(listTestVaultName), "current-unlocker"))
@@ -72,7 +72,7 @@ func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
}{ }{
{ {
name: "the other unlocker", name: "the other unlocker",
unlockerID: "pgp-" + listTestGPGKeyID + "B", unlockerID: listTestUnlockerDirTwo,
wantLast: true, wantLast: true,
wantEntries: []string{listTestUnlockerDirOne}, wantEntries: []string{listTestUnlockerDirOne},
}, },
+79
View File
@@ -0,0 +1,79 @@
//nolint:testpackage // white-box test of unexported internals
package cli
import (
"encoding/json"
"path/filepath"
"testing"
"time"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestSameMetadataUnlockersHaveTheirOwnIDs writes two passphrase unlockers
// side by side whose metadata is the same, creation time included, as
// copying an unlocker directory leaves them. It asserts that `unlocker
// list` and the shell completion of `unlocker select` and `unlocker remove`
// give each its own ID, and that each is selected and removed by its ID
// alone. Keychain and Secure Enclave unlockers, which only macOS can add,
// get their IDs the same way.
func TestSameMetadataUnlockersHaveTheirOwnIDs(t *testing.T) {
t.Parallel()
fs := afero.NewMemMapFs()
_, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName,
testMnemonicBuffer(t), nil)
require.NoError(t, err)
vaultDir := testVaultDir(listTestVaultName)
unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
dirNames := []string{
"passphrase-2026-10-04.12.30.00.000000000",
"passphrase-2026-10-04.12.30.00.000000000-copy",
}
metadata, err := json.Marshal(secret.UnlockerMetadata{
Type: unlockerTypePassphrase,
CreatedAt: time.Date(2026, time.October, 4, 12, 30, 0, 0, time.UTC),
})
require.NoError(t, err)
for _, dirName := range dirNames {
dir := filepath.Join(unlockersDir, dirName)
require.NoError(t, fs.MkdirAll(dir, listTestDirPerm))
require.NoError(t, afero.WriteFile(fs,
filepath.Join(dir, listTestMetadataFileName), metadata,
listTestFilePerm))
}
listed := listUnlockersJSON(t, fs)
require.Len(t, listed, len(dirNames))
completed, _ := getUnlockerIDsCompletionFunc(fs, listTestStateDir)(
nil, nil, "")
assert.Equal(t, dirNames, completed)
instance, cmd := newTestInstance(fs)
for i, unlocker := range listed {
assert.Equal(t, dirNames[i], unlocker.ID)
require.NoError(t, instance.UnlockerSelect(unlocker.ID))
current, err := afero.ReadFile(fs,
filepath.Join(vaultDir, "current-unlocker"))
require.NoError(t, err)
assert.Equal(t, dirNames[i], string(current))
}
// The second one first: an ID both shared would remove the first one
require.NoError(t, instance.UnlockersRemove(listed[1].ID, true, cmd))
assertDirEntries(t, fs, unlockersDir, dirNames[0])
require.NoError(t, instance.UnlockersRemove(listed[0].ID, true, cmd))
assertDirEntries(t, fs, unlockersDir)
}
+25 -80
View File
@@ -1,25 +1,13 @@
// Unlocker List Tests // Unlocker List Tests
// //
// Tests for `secret unlocker list` behavior when the unlockers.d directory, // Tests for `secret unlocker list` behavior when an unlocker's metadata
// or an unlocker's metadata in it, cannot be read while the listing is // cannot be read or used:
// being rendered:
// //
// - TestUnlockersListSkipsUnreadableUnlockersDir: an unreadable
// unlockers.d yields no rows rather than rows bearing synthesized IDs.
// - TestUnlockersListSkipsOnlyUnreadableEntries: a readable entry is
// still listed, with its real ID and its current-unlocker marker,
// when a later entry's scan fails.
// - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt // - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt
// metadata does not stop the others from being listed. // metadata does not stop the others from being listed.
// - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata // - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata
// file cannot be checked for or read is left out, and the other is // file cannot be checked for or read is left out, and the other is
// still listed. // still listed.
//
// The listing resolves each unlocker's real ID by rescanning unlockers.d
// after the vault has already enumerated it. If that rescan fails the ID
// is unknowable, so the entry must be skipped: a synthesized ID matches
// no `unlocker remove` or `unlocker select` argument and would also
// suppress the current-unlocker marker.
//nolint:testpackage // white-box test of unexported internals //nolint:testpackage // white-box test of unexported internals
package cli package cli
@@ -48,18 +36,16 @@ const (
// listTestVaultName is the name of that synthetic vault. // listTestVaultName is the name of that synthetic vault.
listTestVaultName = "default" listTestVaultName = "default"
// listTestGPGKeyID is the GPG key ID recorded in the readable PGP // listTestGPGKeyID is the GPG key ID recorded, with a letter appended,
// unlocker's metadata. The unlocker's real ID is derived from it, and // in the PGP unlockers' metadata.
// differs from the timestamp-derived fallback ID.
listTestGPGKeyID = "DEADBEEFDEADBEEF" listTestGPGKeyID = "DEADBEEFDEADBEEF"
// listTestUnlockerDirOne and listTestUnlockerDirTwo are the unlocker // listTestUnlockerDirOne and listTestUnlockerDirTwo are the unlocker
// directory names under unlockers.d. // directory names under unlockers.d, and so the unlockers' IDs.
listTestUnlockerDirOne = "host-pgp-2026-08-09" listTestUnlockerDirOne = "host-pgp-2026-08-09"
listTestUnlockerDirTwo = "host-pgp-2026-08-10" listTestUnlockerDirTwo = "host-pgp-2026-08-10"
// listTestUnlockersDirName is the directory the listing rescans to // listTestUnlockersDirName is the directory holding the unlockers.
// resolve unlocker IDs.
listTestUnlockersDirName = "unlockers.d" listTestUnlockersDirName = "unlockers.d"
// listTestMetadataFileName is the per-unlocker metadata file name. // listTestMetadataFileName is the per-unlocker metadata file name.
@@ -74,25 +60,16 @@ const (
// a successful open of unlockers.d. // a successful open of unlockers.d.
var errUnlockersDirUnreadable = errors.New("permission denied") var errUnlockersDirUnreadable = errors.New("permission denied")
// unlockersDirFailFs makes unlockers.d unreadable once it has been opened // unlockersDirFailFs fails every open of unlockers.d, as when the
// successfully openBudget times. This reproduces the directory becoming // directory cannot be read.
// unreadable (permission change, partially restored backup, EIO) between
// the vault's own enumeration and the per-entry rescan that resolves
// unlocker IDs.
type unlockersDirFailFs struct { type unlockersDirFailFs struct {
afero.Fs afero.Fs
openBudget int
opens int
} }
//nolint:ireturn // afero.File is the interface required by afero.Fs //nolint:ireturn // afero.File is the interface required by afero.Fs
func (f *unlockersDirFailFs) Open(name string) (afero.File, error) { func (f *unlockersDirFailFs) Open(name string) (afero.File, error) {
if filepath.Base(name) == listTestUnlockersDirName { if filepath.Base(name) == listTestUnlockersDirName {
f.opens++ return nil, errUnlockersDirUnreadable
if f.opens > f.openBudget {
return nil, errUnlockersDirUnreadable
}
} }
//nolint:wrapcheck // test double must return the wrapped Fs error as-is //nolint:wrapcheck // test double must return the wrapped Fs error as-is
@@ -142,8 +119,8 @@ func (f *metadataStatFailFs) Stat(name string) (os.FileInfo, error) {
return f.Fs.Stat(name) return f.Fs.Stat(name)
} }
// writePGPUnlocker writes a PGP unlocker directory with metadata that // writePGPUnlocker writes a PGP unlocker directory named dirName, with
// yields the real ID "pgp-<keyID>". // metadata recording the GPG key ID keyID.
func writePGPUnlocker( func writePGPUnlocker(
t *testing.T, fs afero.Fs, unlockersDir, dirName string, t *testing.T, fs afero.Fs, unlockersDir, dirName string,
createdAt time.Time, keyID string, createdAt time.Time, keyID string,
@@ -224,44 +201,6 @@ func listUnlockersJSON(t *testing.T, fs afero.Fs) []UnlockerInfo {
return decoded.Unlockers return decoded.Unlockers
} }
// TestUnlockersListSkipsUnreadableUnlockersDir asserts that an unlockers.d
// which becomes unreadable after the vault enumerated it produces no rows,
// rather than rows carrying fabricated fallback IDs.
func TestUnlockersListSkipsUnreadableUnlockersDir(t *testing.T) {
t.Parallel()
base := newListTestVault(t, 1)
// Budget of one: the vault's own ListUnlockers scan succeeds, the
// per-entry rescan that resolves the ID fails.
fs := &unlockersDirFailFs{Fs: base, openBudget: 1}
unlockers := listUnlockersJSON(t, fs)
assert.Empty(t, unlockers,
"an unreadable unlockers.d must yield no rows, not fabricated IDs")
}
// TestUnlockersListSkipsOnlyUnreadableEntries asserts that a readable
// entry survives with its real ID and current-unlocker marker when a later
// entry's rescan fails.
func TestUnlockersListSkipsOnlyUnreadableEntries(t *testing.T) {
t.Parallel()
base := newListTestVault(t, 2)
// Budget of two: ListUnlockers plus the first entry's rescan succeed,
// the second entry's rescan fails.
fs := &unlockersDirFailFs{Fs: base, openBudget: 2}
unlockers := listUnlockersJSON(t, fs)
require.Len(t, unlockers, 1,
"only the entry whose directory was readable may be listed")
assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID,
"the surviving row must carry the real unlocker ID")
assert.True(t, unlockers[0].IsCurrent,
"the current-unlocker marker must survive the skip")
}
// TestUnlockersListReadableEntriesAreListed is the control case: with a // TestUnlockersListReadableEntriesAreListed is the control case: with a
// fully readable unlockers.d every entry is listed with its real ID. // fully readable unlockers.d every entry is listed with its real ID.
func TestUnlockersListReadableEntriesAreListed(t *testing.T) { func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
@@ -272,20 +211,21 @@ func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
unlockers := listUnlockersJSON(t, base) unlockers := listUnlockersJSON(t, base)
require.Len(t, unlockers, 2) require.Len(t, unlockers, 2)
assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID) assert.Equal(t, listTestUnlockerDirOne, unlockers[0].ID)
assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[1].ID) assert.Equal(t, listTestUnlockerDirTwo, unlockers[1].ID)
assert.True(t, unlockers[0].IsCurrent) assert.True(t, unlockers[0].IsCurrent)
assert.False(t, unlockers[1].IsCurrent) assert.False(t, unlockers[1].IsCurrent)
} }
// TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with // TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with
// corrupt metadata does not stop the listing. Metadata that is not JSON // corrupt metadata does not stop the listing. Metadata that is not JSON
// leaves that unlocker out; PGP metadata without a usable GPG key ID lists // leaves that unlocker out; PGP metadata without a usable GPG key ID, and
// it as "pgp-unknown". The healthy unlocker is listed with its real ID. // metadata of an unknown type, are still listed, under the directory name
// like any other. The healthy unlocker is listed with its real ID.
func TestUnlockersListToleratesCorruptMetadata(t *testing.T) { func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
t.Parallel() t.Parallel()
healthyID := "pgp-" + listTestGPGKeyID + "A" healthyID := listTestUnlockerDirOne
tests := []struct { tests := []struct {
name string name string
@@ -300,12 +240,17 @@ func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
{ {
name: "GPG key ID of the wrong type", name: "GPG key ID of the wrong type",
metadata: `{"type": "pgp", "gpgKeyId": 42}`, metadata: `{"type": "pgp", "gpgKeyId": 42}`,
wantIDs: []string{healthyID, "pgp-unknown"}, wantIDs: []string{healthyID, listTestUnlockerDirTwo},
}, },
{ {
name: "GPG key ID missing", name: "GPG key ID missing",
metadata: `{"type": "pgp"}`, metadata: `{"type": "pgp"}`,
wantIDs: []string{healthyID, "pgp-unknown"}, wantIDs: []string{healthyID, listTestUnlockerDirTwo},
},
{
name: "unknown type",
metadata: `{"type": "unknown"}`,
wantIDs: []string{healthyID, listTestUnlockerDirTwo},
}, },
} }
@@ -371,7 +316,7 @@ func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
require.Len(t, unlockers, 1, require.Len(t, unlockers, 1,
"only the unlocker with usable metadata may be listed") "only the unlocker with usable metadata may be listed")
assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[0].ID, assert.Equal(t, listTestUnlockerDirTwo, unlockers[0].ID,
"the listed row must carry the real unlocker ID") "the listed row must carry the real unlocker ID")
}) })
} }
+1 -1
View File
@@ -290,7 +290,7 @@ func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
writeTestSecret(t, base, vaultDir) writeTestSecret(t, base, vaultDir)
instance, _ := newTestInstance(&statFailFs{Fs: base, path: path}) instance, _ := newTestInstance(&statFailFs{Fs: base, path: path})
_, err := instance.findUnlockerToRemove("pgp-" + listTestGPGKeyID + "A") _, err := instance.findUnlockerToRemove(listTestUnlockerDirOne)
require.ErrorIs(t, err, errStatFailed) require.ErrorIs(t, err, errStatFailed)
assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne) assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)
+2 -13
View File
@@ -156,20 +156,9 @@ func (k *KeychainUnlocker) GetDirectory() string {
return k.Directory return k.Directory
} }
// GetID implements Unlocker interface - generates ID from keychain item name // GetID implements Unlocker interface: the name of the unlocker's directory
func (k *KeychainUnlocker) GetID() string { func (k *KeychainUnlocker) GetID() string {
// Generate ID in the format YYYY-MM-DD.HH.mm-hostname-keychain return filepath.Base(k.Directory)
// This matches the passphrase unlocker format
hostname, err := os.Hostname()
if err != nil {
hostname = "unknown"
}
// Use the creation timestamp from metadata
createdAt := k.Metadata.CreatedAt
timestamp := createdAt.Format("2006-01-02.15.04")
return fmt.Sprintf("%s-%s-keychain", timestamp, hostname)
} }
// Remove implements Unlocker interface - removes the keychain unlocker // Remove implements Unlocker interface - removes the keychain unlocker
+3 -2
View File
@@ -4,6 +4,7 @@ package secret
import ( import (
"errors" "errors"
"path/filepath"
"filippo.io/age" "filippo.io/age"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -60,9 +61,9 @@ func (k *KeychainUnlocker) GetDirectory() string {
return k.Directory return k.Directory
} }
// GetID returns the unlocker ID // GetID returns the unlocker ID, the name of the unlocker's directory
func (k *KeychainUnlocker) GetID() string { func (k *KeychainUnlocker) GetID() string {
return k.Metadata.CreatedAt.Format("2006-01-02.15.04") + "-keychain" return filepath.Base(k.Directory)
} }
// GetKeychainItemName returns an error on non-Darwin platforms // GetKeychainItemName returns an error on non-Darwin platforms
+9 -6
View File
@@ -1,6 +1,7 @@
package secret package secret
import ( import (
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"path/filepath" "path/filepath"
@@ -10,6 +11,11 @@ import (
"github.com/spf13/afero" "github.com/spf13/afero"
) )
// ErrPassphraseNotRead is wrapped in the error of a passphrase unlocker
// that could not read its passphrase from the terminal, for example because
// there is none. The unlocker itself was not tried.
var ErrPassphraseNotRead = errors.New("failed to read passphrase")
// PassphraseUnlocker represents a passphrase-protected unlocker // PassphraseUnlocker represents a passphrase-protected unlocker
type PassphraseUnlocker struct { type PassphraseUnlocker struct {
Directory string Directory string
@@ -109,12 +115,9 @@ func (p *PassphraseUnlocker) GetDirectory() string {
return p.Directory return p.Directory
} }
// GetID implements Unlocker interface - generates ID from creation timestamp // GetID implements Unlocker interface: the name of the unlocker's directory
func (p *PassphraseUnlocker) GetID() string { func (p *PassphraseUnlocker) GetID() string {
// Generate ID using creation timestamp: YYYY-MM-DD.HH.mm-passphrase return filepath.Base(p.Directory)
createdAt := p.Metadata.CreatedAt
return createdAt.Format("2006-01-02.15.04") + "-passphrase"
} }
// Remove implements Unlocker interface - removes the passphrase unlocker // Remove implements Unlocker interface - removes the passphrase unlocker
@@ -152,7 +155,7 @@ func (p *PassphraseUnlocker) getPassphrase() (*memguard.LockedBuffer, error) {
if err != nil { if err != nil {
Debug("Failed to read passphrase", "error", err, "unlocker_id", p.GetID()) Debug("Failed to read passphrase", "error", err, "unlocker_id", p.GetID())
return nil, fmt.Errorf("failed to read passphrase: %w", err) return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, err)
} }
return secureBuffer, nil return secureBuffer, nil
+4 -54
View File
@@ -297,11 +297,6 @@ func TestPGPUnlockerWithRealFS(t *testing.T) {
// Create a PGP unlocker for the remaining tests // Create a PGP unlocker for the remaining tests
unlocker := secret.NewPGPUnlocker(fs, unlockerDir, metadata) unlocker := secret.NewPGPUnlocker(fs, unlockerDir, metadata)
// Test getting GPG key ID
t.Run("GetGPGKeyID", func(t *testing.T) {
testGetGPGKeyID(t, fs, unlocker, unlockerDir, metadata, fingerprint)
})
// Test getting identity from PGP unlocker // Test getting identity from PGP unlocker
t.Run("GetIdentity", func(t *testing.T) { t.Run("GetIdentity", func(t *testing.T) {
testPGPUnlockerGetIdentity(t, fs, unlocker, unlockerDir, keyID) testPGPUnlockerGetIdentity(t, fs, unlocker, unlockerDir, keyID)
@@ -396,10 +391,10 @@ func testCreatePGPUnlocker(
t.Errorf("Expected PGP unlock key type 'pgp', got '%s'", pgpUnlocker.GetType()) t.Errorf("Expected PGP unlock key type 'pgp', got '%s'", pgpUnlocker.GetType())
} }
// Check if the key ID includes the GPG fingerprint // Check that the ID is the name of the unlocker's directory
if !strings.Contains(pgpUnlocker.GetID(), fingerprint) { if pgpUnlocker.GetID() != filepath.Base(pgpUnlocker.GetDirectory()) {
t.Errorf("PGP unlock key ID '%s' does not contain GPG fingerprint '%s'", t.Errorf("PGP unlock key ID '%s' is not its directory name '%s'",
pgpUnlocker.GetID(), fingerprint) pgpUnlocker.GetID(), filepath.Base(pgpUnlocker.GetDirectory()))
} }
checkPGPUnlockerFiles(t, fs, pgpUnlocker.GetDirectory()) checkPGPUnlockerFiles(t, fs, pgpUnlocker.GetDirectory())
@@ -504,51 +499,6 @@ func checkPGPUnlockerMetadata(
} }
} }
// testGetGPGKeyID writes PGP unlocker metadata holding the GPG fingerprint
// into unlockerDir and checks that unlocker reads it back.
func testGetGPGKeyID(
t *testing.T, fs afero.Fs, unlocker *secret.PGPUnlocker,
unlockerDir string, metadata secret.UnlockerMetadata, fingerprint string,
) {
t.Helper()
// Create PGP metadata with GPG key ID
type PGPUnlockerMetadata struct {
secret.UnlockerMetadata
GPGKeyID string `json:"gpgKeyId"`
}
pgpMetadata := PGPUnlockerMetadata{
UnlockerMetadata: metadata,
GPGKeyID: fingerprint,
}
// Write metadata file
metadataPath := filepath.Join(unlockerDir, unlockerMetadataFile)
metadataBytes, err := json.MarshalIndent(pgpMetadata, "", " ")
if err != nil {
t.Fatalf("Failed to marshal metadata: %v", err)
}
err = afero.WriteFile(fs, metadataPath, metadataBytes, secret.FilePerms)
if err != nil {
t.Fatalf("Failed to write metadata: %v", err)
}
// Get GPG key ID
retrievedKeyID, err := unlocker.GetGPGKeyID()
if err != nil {
t.Fatalf("Failed to get GPG key ID: %v", err)
}
// Verify key ID (should be the fingerprint)
if retrievedKeyID != fingerprint {
t.Errorf("Expected GPG fingerprint '%s', got '%s'", fingerprint, retrievedKeyID)
}
}
// testPGPUnlockerGetIdentity writes an age identity encrypted to the GPG key // testPGPUnlockerGetIdentity writes an age identity encrypted to the GPG key
// keyID into unlockerDir and checks that unlocker decrypts it. // keyID into unlockerDir and checks that unlocker decrypts it.
func testPGPUnlockerGetIdentity( func testPGPUnlockerGetIdentity(
+2 -38
View File
@@ -155,21 +155,9 @@ func (p *PGPUnlocker) GetDirectory() string {
return p.Directory return p.Directory
} }
// GetID implements Unlocker interface - generates ID from GPG key ID. // GetID implements Unlocker interface: the name of the unlocker's directory
// If the metadata has no usable GPG key ID, it warns with the unlocker's
// directory and returns "pgp-unknown", so listing the other unlockers
// still works.
func (p *PGPUnlocker) GetID() string { func (p *PGPUnlocker) GetID() string {
// Generate ID using GPG key ID: pgp-<keyid> return filepath.Base(p.Directory)
gpgKeyID, err := p.GetGPGKeyID()
if err != nil {
Warn("PGP unlocker metadata is corrupt or missing its GPG key ID",
"directory", p.Directory, "error", err)
return "pgp-unknown"
}
return "pgp-" + gpgKeyID
} }
// Remove implements Unlocker interface - removes the PGP unlocker // Remove implements Unlocker interface - removes the PGP unlocker
@@ -184,30 +172,6 @@ func (p *PGPUnlocker) Remove() error {
return nil return nil
} }
// GetGPGKeyID returns the GPG key ID from metadata
func (p *PGPUnlocker) GetGPGKeyID() (string, error) {
// Load the metadata
metadataPath := filepath.Join(p.Directory, "unlocker-metadata.json")
metadataData, err := afero.ReadFile(p.fs, metadataPath)
if err != nil {
return "", fmt.Errorf("failed to read PGP metadata: %w", err)
}
var pgpMetadata PGPUnlockerMetadata
err = json.Unmarshal(metadataData, &pgpMetadata)
if err != nil {
return "", fmt.Errorf("failed to parse PGP metadata: %w", err)
}
if pgpMetadata.GPGKeyID == "" {
return "", fmt.Errorf("PGP metadata: %w", errGPGKeyIDEmpty)
}
return pgpMetadata.GPGKeyID, nil
}
// generatePGPUnlockerName generates a unique name for the PGP unlocker // generatePGPUnlockerName generates a unique name for the PGP unlocker
// based on hostname and time // based on hostname and time
func generatePGPUnlockerName() (string, error) { func generatePGPUnlockerName() (string, error) {
-193
View File
@@ -1,26 +1,18 @@
package secret package secret
import ( import (
"encoding/json"
"errors" "errors"
"fmt"
"log/slog" "log/slog"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
"filippo.io/age" "filippo.io/age"
"git.eeqj.de/sneak/secret/pkg/agehd"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
"github.com/spf13/afero" "github.com/spf13/afero"
) )
var ( var (
// errSecretNotFound carries only the message tail; callers compose
// "secret <name> not found" around it so the emitted text is
// unchanged.
errSecretNotFound = errors.New("not found")
errUnlockerRequired = errors.New("unlocker required to decrypt secret")
errGetEncryptedDataDeprecated = errors.New( errGetEncryptedDataDeprecated = errors.New(
"GetEncryptedData is deprecated - use version-specific methods") "GetEncryptedData is deprecated - use version-specific methods")
errGetCurrentVaultNotRegistered = errors.New( errGetCurrentVaultNotRegistered = errors.New(
@@ -81,73 +73,6 @@ func NewSecret(vault VaultInterface, name string) *Secret {
} }
} }
// GetValue retrieves and decrypts the current version's value, with the
// vault's long-term key derived from mnemonic when it is not nil, else
// obtained through unlocker
func (s *Secret) GetValue(
unlocker Unlocker, mnemonic *memguard.LockedBuffer,
) (*memguard.LockedBuffer, error) {
DebugWith("Getting secret value",
slog.String("secret_name", s.Name),
slog.String("vault_name", s.vault.GetName()),
)
// Check if secret exists
exists, err := s.Exists()
if err != nil {
Debug("Failed to check if secret exists during GetValue",
"error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to check if secret exists: %w", err)
}
if !exists {
Debug("Secret not found during GetValue",
"secret_name", s.Name, "vault_name", s.vault.GetName())
return nil, fmt.Errorf("secret %s %w", s.Name, errSecretNotFound)
}
Debug("Secret exists, getting current version", "secret_name", s.Name)
// Get current version
currentVersion, err := GetCurrentVersion(s.vault.GetFilesystem(), s.Directory)
if err != nil {
Debug("Failed to get current version", "error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to get current version: %w", err)
}
// Create version object
version := NewVersion(s.vault, s.Name, currentVersion)
if mnemonic != nil {
return s.getValueViaMnemonic(version, mnemonic.String())
}
Debug("Using unlocker for vault access", "secret_name", s.Name)
// Use the provided unlocker to get the vault's long-term private key
if unlocker == nil {
Debug("No unlocker provided for secret decryption", "secret_name", s.Name)
return nil, errUnlockerRequired
}
ltIdentity, err := s.getLongTermIdentityFromUnlocker(unlocker)
if err != nil {
return nil, err
}
DebugWith("Successfully obtained vault's long-term key",
slog.String("secret_name", s.Name),
slog.String("public_key", ltIdentity.Recipient().String()),
)
// Use the long-term key to decrypt the version
return version.GetValue(ltIdentity)
}
// LoadMetadata is deprecated - metadata is now per-version and encrypted // LoadMetadata is deprecated - metadata is now per-version and encrypted
func (s *Secret) LoadMetadata() error { func (s *Secret) LoadMetadata() error {
Debug("LoadMetadata called but is deprecated in versioned model", Debug("LoadMetadata called but is deprecated in versioned model",
@@ -215,124 +140,6 @@ func (s *Secret) Exists() (bool, error) {
return true, nil return true, nil
} }
// getValueViaMnemonic derives the vault's long-term key from the
// mnemonic and decrypts the version value with it.
func (s *Secret) getValueViaMnemonic(
version *Version, mnemonic string,
) (*memguard.LockedBuffer, error) {
Debug("Using mnemonic for direct long-term key derivation",
"secret_name", s.Name)
// Get vault directory to read metadata
vaultDir, err := s.vault.GetDirectory()
if err != nil {
Debug("Failed to get vault directory", "error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to get vault directory: %w", err)
}
// Load vault metadata to get the correct derivation index
metadataPath := filepath.Join(vaultDir, "vault-metadata.json")
metadataBytes, err := afero.ReadFile(s.vault.GetFilesystem(), metadataPath)
if err != nil {
Debug("Failed to read vault metadata", "error", err, "path", metadataPath)
return nil, fmt.Errorf("failed to read vault metadata: %w", err)
}
var metadata VaultMetadata
err = json.Unmarshal(metadataBytes, &metadata)
if err != nil {
Debug("Failed to parse vault metadata", "error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to parse vault metadata: %w", err)
}
DebugWith("Using vault derivation index from metadata",
slog.String("secret_name", s.Name),
slog.String("vault_name", s.vault.GetName()),
slog.Uint64("derivation_index", uint64(metadata.DerivationIndex)),
)
// Use mnemonic with the vault's derivation index from metadata
ltIdentity, err := agehd.DeriveIdentity(mnemonic, metadata.DerivationIndex)
if err != nil {
Debug("Failed to derive long-term key from mnemonic for secret",
"error", err, "secret_name", s.Name)
return nil, fmt.Errorf(
"failed to derive long-term key from mnemonic: %w", err)
}
Debug("Successfully derived long-term key from mnemonic", "secret_name", s.Name)
// Use the long-term key to decrypt the version
return version.GetValue(ltIdentity)
}
// getLongTermIdentityFromUnlocker uses the unlocker to obtain and parse
// the vault's long-term private key.
func (s *Secret) getLongTermIdentityFromUnlocker(
unlocker Unlocker,
) (*age.X25519Identity, error) {
DebugWith("Getting vault's long-term key using unlocker",
slog.String("secret_name", s.Name),
slog.String("unlocker_type", unlocker.GetType()),
slog.String("unlocker_id", unlocker.GetID()),
)
// Step 1: Use the unlocker to get the vault's long-term private key
unlockIdentity, err := unlocker.GetIdentity()
if err != nil {
Debug("Failed to get unlocker identity",
"error", err, "secret_name", s.Name,
"unlocker_type", unlocker.GetType())
return nil, fmt.Errorf("failed to get unlocker identity: %w", err)
}
// Read the encrypted long-term private key from the unlocker directory
encryptedLtPrivKeyPath := filepath.Join(unlocker.GetDirectory(), "longterm.age")
Debug("Reading encrypted long-term private key", "path", encryptedLtPrivKeyPath)
encryptedLtPrivKey, err := afero.ReadFile(
s.vault.GetFilesystem(), encryptedLtPrivKeyPath)
if err != nil {
Debug("Failed to read encrypted long-term private key",
"error", err, "path", encryptedLtPrivKeyPath)
return nil, fmt.Errorf(
"failed to read encrypted long-term private key: %w", err)
}
// Decrypt the encrypted long-term private key using the unlocker
Debug("Decrypting long-term private key using unlocker", "secret_name", s.Name)
ltPrivKeyBuffer, err := DecryptWithIdentity(encryptedLtPrivKey, unlockIdentity)
if err != nil {
Debug("Failed to decrypt long-term private key",
"error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to decrypt long-term private key: %w", err)
}
defer ltPrivKeyBuffer.Destroy()
// Parse the long-term private key
Debug("Parsing long-term private key", "secret_name", s.Name)
ltIdentity, err := age.ParseX25519Identity(ltPrivKeyBuffer.String())
if err != nil {
Debug("Failed to parse long-term private key",
"error", err, "secret_name", s.Name)
return nil, fmt.Errorf("failed to parse long-term private key: %w", err)
}
return ltIdentity, nil
}
// GetCurrentVault gets the current vault from the file system // GetCurrentVault gets the current vault from the file system
// This function is a wrapper around the actual implementation in the vault package // This function is a wrapper around the actual implementation in the vault package
// and exists to break the import cycle. // and exists to break the import cycle.
-45
View File
@@ -2,7 +2,6 @@
package secret package secret
import ( import (
"encoding/json"
"errors" "errors"
"os" "os"
"path/filepath" "path/filepath"
@@ -13,7 +12,6 @@ import (
"git.eeqj.de/sneak/secret/pkg/agehd" "git.eeqj.de/sneak/secret/pkg/agehd"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/require"
) )
// testMnemonicValue is the standard BIP39 test vector mnemonic. // testMnemonicValue is the standard BIP39 test vector mnemonic.
@@ -321,46 +319,3 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
t.Logf("Secret.Exists() works correctly") t.Logf("Secret.Exists() works correctly")
}) })
} }
// TestSecretGetValueWithMnemonicUsesVaultDerivationIndex checks that
// GetValue, given the mnemonic, derives the long-term key at the derivation
// index in the vault's metadata. At index 0 it could not decrypt the secret,
// which was encrypted to the key at index 1.
func TestSecretGetValueWithMnemonicUsesVaultDerivationIndex(t *testing.T) {
t.Parallel()
fs := afero.NewMemMapFs()
vaultDir := "/test-config/vaults.d/test-vault"
mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonicValue))
defer mnemonic.Destroy()
vlt := &MockVault{
name: "test-vault",
fs: fs,
directory: vaultDir,
derivationIndex: 1,
mnemonic: mnemonic,
}
metadata, err := json.Marshal(VaultMetadata{DerivationIndex: vlt.derivationIndex})
require.NoError(t, err)
require.NoError(t, fs.MkdirAll(vaultDir, DirPerms))
err = afero.WriteFile(
fs, filepath.Join(vaultDir, "vault-metadata.json"), metadata, FilePerms)
require.NoError(t, err)
secretName, secretValue := "x", "value"
err = vlt.AddSecret(secretName,
memguard.NewBufferFromBytes([]byte(secretValue)), false)
require.NoError(t, err)
value, err := NewSecret(vlt, secretName).GetValue(nil, mnemonic)
require.NoError(t, err)
defer value.Destroy()
require.Equal(t, secretValue, value.String())
}
+2 -10
View File
@@ -130,17 +130,9 @@ func (s *SecureEnclaveUnlocker) GetDirectory() string {
return s.Directory return s.Directory
} }
// GetID implements Unlocker interface. // GetID implements Unlocker interface: the name of the unlocker's directory.
func (s *SecureEnclaveUnlocker) GetID() string { func (s *SecureEnclaveUnlocker) GetID() string {
hostname, err := os.Hostname() return filepath.Base(s.Directory)
if err != nil {
hostname = "unknown"
}
createdAt := s.Metadata.CreatedAt
timestamp := createdAt.Format("2006-01-02.15.04")
return fmt.Sprintf("%s-%s-%s", timestamp, hostname, seUnlockerType)
} }
// Remove implements Unlocker interface. // Remove implements Unlocker interface.
+3 -2
View File
@@ -4,6 +4,7 @@ package secret
import ( import (
"errors" "errors"
"path/filepath"
"filippo.io/age" "filippo.io/age"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -67,9 +68,9 @@ func (s *SecureEnclaveUnlocker) GetDirectory() string {
return s.Directory return s.Directory
} }
// GetID returns the unlocker ID. // GetID returns the unlocker ID, the name of the unlocker's directory.
func (s *SecureEnclaveUnlocker) GetID() string { func (s *SecureEnclaveUnlocker) GetID() string {
return s.Metadata.CreatedAt.Format("2006-01-02.15.04") + "-" + seUnlockerType return filepath.Base(s.Directory)
} }
// Remove returns an error on non-Darwin platforms. // Remove returns an error on non-Darwin platforms.
+2 -3
View File
@@ -35,9 +35,8 @@ func TestNewSecureEnclaveUnlocker(t *testing.T) {
// Test GetDirectory returns the directory we passed in // Test GetDirectory returns the directory we passed in
assert.Equal(t, dir, unlocker.GetDirectory()) assert.Equal(t, dir, unlocker.GetDirectory())
// Test GetID returns a formatted string with the creation timestamp // Test GetID returns the name of the unlocker's directory
expectedID := "2026-01-15.10.30-secure-enclave" assert.Equal(t, "test-se-unlocker", unlocker.GetID())
assert.Equal(t, expectedID, unlocker.GetID())
} }
func TestSecureEnclaveUnlockerGetIdentityReturnsError(t *testing.T) { func TestSecureEnclaveUnlockerGetIdentityReturnsError(t *testing.T) {
+2 -4
View File
@@ -61,11 +61,9 @@ func TestSecureEnclaveUnlockerGetIDFormat(t *testing.T) {
} }
unlocker := NewSecureEnclaveUnlocker(fs, "/tmp/test", metadata) unlocker := NewSecureEnclaveUnlocker(fs, "/tmp/test", metadata)
id := unlocker.GetID()
// ID should contain the timestamp and "secure-enclave" type // The ID is the name of the unlocker's directory
assert.Contains(t, id, "2026-03-10.14.30") assert.Equal(t, "test", unlocker.GetID())
assert.Contains(t, id, seUnlockerType)
} }
func TestGenerateSEKeyLabel(t *testing.T) { func TestGenerateSEKeyLabel(t *testing.T) {
+1 -1
View File
@@ -10,6 +10,6 @@ type Unlocker interface {
GetType() string GetType() string
GetMetadata() UnlockerMetadata GetMetadata() UnlockerMetadata
GetDirectory() string GetDirectory() string
GetID() string // Generate ID based on unlocker type and data GetID() string // The name of the unlocker's directory, unique in its vault
Remove() error // Remove the unlocker and any associated resources Remove() error // Remove the unlocker and any associated resources
} }
+7 -7
View File
@@ -188,8 +188,9 @@ func (v *Vault) findUnlockerByID(
return nil, skippedDirPath, nil return nil, skippedDirPath, nil
} }
// ListUnlockers returns a list of available unlockers for this vault // ListUnlockers returns the metadata of each unlocker of this vault, keyed
func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) { // by the unlocker's ID, the name of its directory in unlockers.d
func (v *Vault) ListUnlockers() (map[string]UnlockerMetadata, error) {
vaultDir, err := v.GetDirectory() vaultDir, err := v.GetDirectory()
if err != nil { if err != nil {
return nil, err return nil, err
@@ -204,7 +205,7 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
} }
if !exists { if !exists {
return []UnlockerMetadata{}, nil return map[string]UnlockerMetadata{}, nil
} }
// List directories in unlockers.d // List directories in unlockers.d
@@ -213,7 +214,7 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
return nil, fmt.Errorf("failed to read unlockers directory: %w", err) return nil, fmt.Errorf("failed to read unlockers directory: %w", err)
} }
var unlockers []UnlockerMetadata unlockers := map[string]UnlockerMetadata{}
for _, file := range files { for _, file := range files {
if !file.IsDir() { if !file.IsDir() {
@@ -222,7 +223,7 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
metadata, ok := v.readUnlockerMetadataOrWarn(unlockersDir, file.Name()) metadata, ok := v.readUnlockerMetadataOrWarn(unlockersDir, file.Name())
if ok { if ok {
unlockers = append(unlockers, metadata) unlockers[file.Name()] = metadata
} }
} }
@@ -453,8 +454,7 @@ func writePassphraseUnlocker(
return nil, err return nil, err
} }
// Select the new unlocker by its directory, not by its ID: an old // Make the new unlocker the current one
// passphrase unlocker created in the same minute has the same ID.
currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker") currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
err = secret.WriteFileAtomic(fs, currentUnlockerPath, err = secret.WriteFileAtomic(fs, currentUnlockerPath,
+19 -4
View File
@@ -1,6 +1,7 @@
package vault package vault
import ( import (
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"path/filepath" "path/filepath"
@@ -302,8 +303,14 @@ func (v *Vault) unlockLongTermKey(
// vault, and how to give it a new unlocker. The advice is added only when the // vault, and how to give it a new unlocker. The advice is added only when the
// vault metadata records the key that the mnemonic derives; a vault created // vault metadata records the key that the mnemonic derives; a vault created
// without a mnemonic records none, and without its metadata the key cannot // without a mnemonic records none, and without its metadata the key cannot
// be derived. // be derived. It is not added when the passphrase could not be read: the
// unlocker was not tried, and adding one would need a passphrase read the
// same way.
func (v *Vault) withMnemonicAdvice(err error) error { func (v *Vault) withMnemonicAdvice(err error) error {
if errors.Is(err, secret.ErrPassphraseNotRead) {
return err
}
vaultDir, _ := v.GetDirectory() vaultDir, _ := v.GetDirectory()
metadata, metadataErr := LoadVaultMetadata(v.fs, vaultDir) metadata, metadataErr := LoadVaultMetadata(v.fs, vaultDir)
@@ -311,7 +318,15 @@ func (v *Vault) withMnemonicAdvice(err error) error {
return err return err
} }
return fmt.Errorf("%w; the vault still opens with its mnemonic: run "+ // 'secret unlocker add' acts on the current vault only.
"'secret unlocker add passphrase' with %s set to the mnemonic "+ steps := "'secret unlocker add passphrase'"
"to give it a new unlocker", err, secret.EnvMnemonic)
current, currentErr := GetCurrentVault(v.fs, v.stateDir)
if currentErr != nil || current.Name != v.Name {
steps = fmt.Sprintf("'secret vault select %s', then %s", v.Name, steps)
}
return fmt.Errorf("%w; the vault '%s' still opens with its mnemonic: run "+
"%s with %s set to the mnemonic to give it a new unlocker",
err, v.Name, steps, secret.EnvMnemonic)
} }