From 72cd6c30a1290fc5a6167e96cc21d8f51e84cebd Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 23:48:50 +0200 Subject: [PATCH] A wrong passphrase gives an error that says so (closes #157) A passphrase that did not decrypt the passphrase unlocker failed with age's "no identity matched any of the recipients" and advice to give the vault a new unlocker from its mnemonic, which would replace an unlocker that works. GetIdentity now turns that age error into "wrong passphrase for unlocker ", wrapping the new secret.ErrWrongPassphrase, and withMnemonicAdvice leaves it alone, as it does when no passphrase was read. A passphrase from SB_UNLOCK_PASSPHRASE and a typed one both reach this code. A priv.age that is not an age file keeps the advice. Judgement call: age cannot tell a wrong passphrase from a damaged key inside a priv.age whose header reads, so that case now reads as a wrong passphrase. Model: opus-5-5 --- TODO.md | 8 ++++ internal/cli/unlock_failure_test.go | 57 ++++++++++++++++++--------- internal/secret/passphraseunlocker.go | 10 +++++ internal/vault/vault.go | 7 +++- internal/vault/vault_error_test.go | 5 +-- 5 files changed, 63 insertions(+), 24 deletions(-) diff --git a/TODO.md b/TODO.md index d659129..42302ae 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,14 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the + prompt, gives the error `wrong passphrase for unlocker ` + (https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no + identity matched any of the recipients", followed by advice to give the vault + a new unlocker from its mnemonic, which would have replaced an unlocker that + works. The error now wraps `secret.ErrWrongPassphrase` and carries no advice + to use the mnemonic. A `priv.age` that is not an age file still gets that + advice. - 2026-10-14: The passphrase and mnemonic prompts ask on the controlling terminal (`/dev/tty`) instead of stdin, so `echo v | secret add s1 --force`, `cat file | secret encrypt key` and `cat file.age | secret decrypt key` ask diff --git a/internal/cli/unlock_failure_test.go b/internal/cli/unlock_failure_test.go index 7bc9574..6c3859b 100644 --- a/internal/cli/unlock_failure_test.go +++ b/internal/cli/unlock_failure_test.go @@ -1,12 +1,12 @@ // Unlock Failure Tests // // 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 -// keeps its cause and ends by saying that the mnemonic still opens that -// vault, but only for a vault that the mnemonic does open, and not when the -// passphrase could not be read at all. When the current unlocker does not -// exist and the vault has another that `secret unlocker select` accepts, the -// error says to select one instead. +// file the unlocker needs is missing, the error keeps its cause and ends by +// saying that the mnemonic still opens that vault, but only for a vault that +// the mnemonic does open, and not when the passphrase could not be read at +// all. A wrong passphrase gives an error that says so, without that advice. +// When the current unlocker does not exist and the vault has another that +// `secret unlocker select` accepts, the error says to select one instead. // When a secret's current file is missing, the error says how to make a // version current again. Each test that pins such advice also follows it. @@ -21,7 +21,6 @@ import ( "syscall" "testing" - "filippo.io/age" "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/spf13/cobra" @@ -165,26 +164,48 @@ func TestUnlockFailureNamesMnemonic(t *testing.T) { } } -// TestWrongPassphraseNamesMnemonic checks the error of `secret get` given a -// passphrase that does not decrypt the passphrase unlocker: it keeps age's -// error and ends with the advice that the mnemonic still opens the vault. -func TestWrongPassphraseNamesMnemonic(t *testing.T) { +// TestWrongPassphraseNamesPassphrase checks the error of `secret get` given +// a passphrase that does not decrypt the passphrase unlocker: it says that +// the passphrase is wrong, without the advice to use the mnemonic, since the +// unlocker still opens the vault with the right passphrase. The test then +// gives the right passphrase. +func TestWrongPassphraseNamesPassphrase(t *testing.T) { t.Parallel() - _, c := newUnlockTestCLI(t, newTwoVaultFs(t)) + unlockerDir, c := newUnlockTestCLI(t, newTwoVaultFs(t)) c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte("wrong passphrase")) t.Cleanup(c.UnlockPassphrase.Destroy) err := c.GetSecret(discardCmd(), "x") - var noMatch *age.NoIdentityMatchError - - require.ErrorAs(t, err, &noMatch) - + require.ErrorIs(t, err, secret.ErrWrongPassphrase) + assert.NotContains(t, err.Error(), mnemonicAdvice) require.EqualError(t, err, "failed to unlock vault: "+ "failed to get long-term key: failed to get unlocker identity: "+ - "failed to decrypt unlocker private key: failed to create decryptor: "+ - noMatch.Error()+mnemonicAdvice) + "wrong passphrase for unlocker "+filepath.Base(unlockerDir)) + + c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase)) + t.Cleanup(c.UnlockPassphrase.Destroy) + assert.Equal(t, "value", getSecret(t, c, "x")) +} + +// TestDamagedUnlockerNamesMnemonic checks the error of `secret get` when the +// passphrase unlocker's priv.age is not an age file: the passphrase is not +// blamed, and the error ends with the advice that the mnemonic still opens +// the vault. +func TestDamagedUnlockerNamesMnemonic(t *testing.T) { + t.Parallel() + + fs := newTwoVaultFs(t) + unlockerDir, c := newUnlockTestCLI(t, fs) + + require.NoError(t, afero.WriteFile(fs, filepath.Join(unlockerDir, "priv.age"), + []byte("damaged"), secret.FilePerms)) + + err := c.GetSecret(discardCmd(), "x") + + require.NotErrorIs(t, err, secret.ErrWrongPassphrase) + require.ErrorContains(t, err, mnemonicAdvice) } // TestMoveUnlockFailureNamesVault checks the error of `secret move` into diff --git a/internal/secret/passphraseunlocker.go b/internal/secret/passphraseunlocker.go index 6d38520..91d47b3 100644 --- a/internal/secret/passphraseunlocker.go +++ b/internal/secret/passphraseunlocker.go @@ -16,6 +16,10 @@ import ( // empty. A passphrase unlocker that fails with it was not tried. var ErrPassphraseNotRead = errors.New("failed to read passphrase") +// ErrWrongPassphrase is wrapped in the error of GetIdentity when the +// passphrase does not decrypt the unlocker's private key. +var ErrWrongPassphrase = errors.New("wrong passphrase") + // PassphraseUnlocker represents a passphrase-protected unlocker type PassphraseUnlocker struct { Directory string @@ -73,6 +77,12 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) { Debug("Failed to decrypt unlocker private key", "error", err, "unlocker_id", p.GetID()) + // age says that no identity matched when the passphrase is wrong + var noMatch *age.NoIdentityMatchError + if errors.As(err, &noMatch) { + return nil, fmt.Errorf("%w for unlocker %s", ErrWrongPassphrase, p.GetID()) + } + return nil, fmt.Errorf("failed to decrypt unlocker private key: %w", err) } defer privKeyBuffer.Destroy() diff --git a/internal/vault/vault.go b/internal/vault/vault.go index d289bef..f43614a 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -291,10 +291,13 @@ func (v *Vault) unlockLongTermKey( // without a mnemonic records none, and without its metadata the key cannot // 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. Nor is it added when the current unlocker does not exist and the -// vault has others: the error already says to select one of them. +// same way. Nor is it added when the passphrase is wrong: the unlocker still +// opens the vault with the right one. Nor is it added when the current +// unlocker does not exist and the vault has others: the error already says +// to select one of them. func (v *Vault) withMnemonicAdvice(err error) error { if errors.Is(err, secret.ErrPassphraseNotRead) || + errors.Is(err, secret.ErrWrongPassphrase) || errors.Is(err, ErrUnlockerNotFound) { return err } diff --git a/internal/vault/vault_error_test.go b/internal/vault/vault_error_test.go index b28378a..5b0f9ef 100644 --- a/internal/vault/vault_error_test.go +++ b/internal/vault/vault_error_test.go @@ -5,7 +5,6 @@ import ( "path/filepath" "testing" - "filippo.io/age" "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -140,9 +139,7 @@ func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) { err = vlt.AddSecret(testSecretName, second, true) - var noMatch *age.NoIdentityMatchError - - require.ErrorAs(t, err, &noMatch) + require.ErrorIs(t, err, secret.ErrWrongPassphrase) assert.Equal(t, before, dirContents(t, fs, secretDir)) }