Say the mnemonic still opens a vault its unlocker cannot (closes #47)

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

TODO.md

@@ -18,6 +18,24 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
+- 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
+  or Secure Enclave key is gone, or the passphrase is wrong, 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 to it, gives the vault a new unlocker; for a vault that is not the
+  current one, as in `secret move` between vaults, it says to run
+  `secret vault select` first (https://git.eeqj.de/sneak/secret/issues/47).
+  Before, it ended with the bare cause. The advice is given only when the
+  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
+  secret through `vault.GetSecret`, as `secret get` does, so they give the
+  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`
+  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
@@ -406,8 +424,6 @@ https://git.eeqj.de/sneak/secret/milestone/12
       (`secret.IdentityToLockedBuffer` overwrites only the string itself).
 - Medium priority:
     - Standardize error messages; stop leaking internals.
-    - Graceful handling of corrupted or missing key files with recovery
-      suggestions.
     - Split oversized CLI functions.
 - Cleanups: read statedir from environment or default instead of
   passing it around.

internal/cli/crypto.go

@@ -130,7 +130,7 @@ func (cli *Instance) resolveEncryptionKey(
 	}
 
 	// Secret exists, get the age secret key from it
-	secretBuffer, err := cli.getSecretValue(vlt, secretObj)
+	secretBuffer, err := vlt.GetSecret(secretName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get secret value: %w", err)
 	}
@@ -249,7 +249,7 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 	}
 
 	// Get the age secret key from the secret
-	secretBuffer, err := cli.getSecretValue(vlt, secretObj)
+	secretBuffer, err := vlt.GetSecret(secretName)
 	if err != nil {
 		return fmt.Errorf("failed to get secret value: %w", err)
 	}
@@ -313,20 +313,3 @@ func isValidAgeSecretKey(key string) bool {
 
 	return err == nil
 }
-
-// getSecretValue retrieves the value of a secret with the vault's mnemonic
-// when it has one, else with the current unlocker
-func (cli *Instance) getSecretValue(
-	vlt *vault.Vault, secretObj *secret.Secret,
-) (*memguard.LockedBuffer, error) {
-	if vlt.Mnemonic != nil {
-		return secretObj.GetValue(nil, vlt.Mnemonic)
-	}
-
-	unlocker, err := vlt.GetCurrentUnlocker()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get current unlocker: %w", err)
-	}
-
-	return secretObj.GetValue(unlocker, nil)
-}

internal/cli/unlock_failure_test.go

@@ -0,0 +1,374 @@
+// 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 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.
+
+package cli_test
+
+import (
+	"bytes"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"filippo.io/age"
+	"git.eeqj.de/sneak/secret/internal/cli"
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// mnemonicAdvice ends the error when the current vault "default", which
+	// its mnemonic opens, cannot be opened through its current unlocker.
+	mnemonicAdvice = "; the vault 'default' still opens with its mnemonic: " +
+		"run 'secret unlocker add passphrase' with SB_SECRET_MNEMONIC set " +
+		"to the mnemonic to give it a new unlocker"
+
+	// versionAdvice ends the error when a secret's current file cannot be
+	// read.
+	versionAdvice = "; this file only names the current version: " +
+		"'secret version list' lists the secret's versions, and " +
+		"'secret version promote' makes one of them current"
+
+	// unlockTestVaultDir is the directory of the vault "default" of
+	// newTwoVaultFs, the current vault, whose secret "x" is "value".
+	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
+// 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,
+// but not the mnemonic.
+func newUnlockTestCLI(t *testing.T, fs afero.Fs) (string, *cli.Instance) {
+	t.Helper()
+
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(c.UnlockPassphrase.Destroy)
+
+	return currentUnlockerDir(t, fs, unlockTestVaultDir), c
+}
+
+// discardCmd returns a command whose output is discarded.
+func discardCmd() *cobra.Command {
+	cmd := &cobra.Command{}
+	cmd.SetOut(io.Discard)
+
+	return cmd
+}
+
+// getSecret returns what `secret get name` prints.
+func getSecret(t *testing.T, c *cli.Instance, name string) string {
+	t.Helper()
+
+	var out bytes.Buffer
+
+	cmd := &cobra.Command{}
+	cmd.SetOut(&out)
+	require.NoError(t, c.GetSecret(cmd, name))
+
+	return out.String()
+}
+
+// TestUnlockFailureNamesMnemonic checks the error of `secret get` when a
+// file that opening the vault through its current unlocker needs is
+// missing: it keeps the cause, which names the file, and ends with the
+// advice that the mnemonic still opens the vault. The test then follows
+// that advice: `secret unlocker add passphrase`, with the mnemonic, gives
+// the vault a new unlocker, which opens it.
+func TestUnlockFailureNamesMnemonic(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		file       string // the file removed
+		inVaultDir bool   // the file is the vault's, not the unlocker's
+		want       string // the message before the cause
+	}{
+		{
+			file:       "current-unlocker",
+			inVaultDir: true,
+			want: "failed to unlock vault: failed to get long-term key: " +
+				"failed to get current unlocker: " +
+				"failed to read current unlocker: ",
+		},
+		{
+			file: "priv.age",
+			want: "failed to unlock vault: failed to get long-term key: " +
+				"failed to get unlocker identity: " +
+				"failed to read unlocker private key: ",
+		},
+		{
+			file: "longterm.age",
+			want: "failed to unlock vault: failed to get long-term key: " +
+				"failed to read encrypted long-term private key: ",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.file, func(t *testing.T) {
+			t.Parallel()
+
+			fs := newTwoVaultFs(t)
+			unlockerDir, c := newUnlockTestCLI(t, fs)
+
+			path := filepath.Join(unlockerDir, tt.file)
+
+			if tt.inVaultDir {
+				path = filepath.Join(unlockTestVaultDir, tt.file)
+			}
+
+			require.NoError(t, fs.Remove(path))
+
+			err := c.GetSecret(discardCmd(), "x")
+
+			var cause *os.PathError
+
+			require.ErrorAs(t, err, &cause)
+			require.ErrorIs(t, err, os.ErrNotExist)
+			assert.Equal(t, path, cause.Path)
+
+			require.EqualError(t, err, tt.want+cause.Error()+mnemonicAdvice)
+
+			c.Mnemonic = testMnemonicBuffer(t)
+			require.NoError(t, c.UnlockersAdd("passphrase", discardCmd()))
+
+			c.Mnemonic = nil
+			assert.Equal(t, "value", getSecret(t, c, "x"))
+		})
+	}
+}
+
+// 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) {
+	t.Parallel()
+
+	_, 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.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)
+}
+
+// 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
+// `secret decrypt`, reading the key secret, end with the same advice as
+// `secret get` when the vault cannot be opened through its current
+// unlocker.
+func TestCryptoUnlockFailureNamesMnemonic(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		command string
+		run     func(c *cli.Instance) error
+	}{
+		{"encrypt", func(c *cli.Instance) error { return c.Encrypt("x", "", "") }},
+		{"decrypt", func(c *cli.Instance) error { return c.Decrypt("x", "", "") }},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			t.Parallel()
+
+			fs := newTwoVaultFs(t)
+			unlockerDir, c := newUnlockTestCLI(t, fs)
+
+			path := filepath.Join(unlockerDir, "priv.age")
+			require.NoError(t, fs.Remove(path))
+
+			err := tt.run(c)
+
+			var cause *os.PathError
+
+			require.ErrorAs(t, err, &cause)
+			assert.Equal(t, path, cause.Path)
+
+			require.EqualError(t, err, "failed to get secret value: "+
+				"failed to unlock vault: failed to get long-term key: "+
+				"failed to get unlocker identity: "+
+				"failed to read unlocker private key: "+cause.Error()+
+				mnemonicAdvice)
+		})
+	}
+}
+
+// TestMissingCurrentFileNamesVersionCommands checks the error of `secret
+// get` when the secret's current file is missing: it keeps the cause, which
+// names the file, and ends with the advice that says how to make a version
+// current again. The test then follows that advice.
+func TestMissingCurrentFileNamesVersionCommands(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	_, c := newUnlockTestCLI(t, fs)
+
+	secretDir := filepath.Join(unlockTestVaultDir, "secrets.d", "x")
+	path := filepath.Join(secretDir, "current")
+	require.NoError(t, fs.Remove(path))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	var cause *os.PathError
+
+	require.ErrorAs(t, err, &cause)
+	require.ErrorIs(t, err, os.ErrNotExist)
+	assert.Equal(t, path, cause.Path)
+
+	require.EqualError(t, err, "failed to get current version: "+
+		"failed to read current version file: "+cause.Error()+versionAdvice)
+
+	versions, err := afero.ReadDir(fs, filepath.Join(secretDir, "versions"))
+	require.NoError(t, err)
+	require.Len(t, versions, 1)
+
+	var out bytes.Buffer
+
+	cmd := &cobra.Command{}
+	cmd.SetOut(&out)
+	require.NoError(t, c.ListVersions(cmd, "x"))
+	assert.Contains(t, out.String(), versions[0].Name())
+
+	require.NoError(t, c.PromoteVersion(cmd, "x", versions[0].Name()))
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
+// TestUnlockFailureWithoutLongTermKeyNamesNoMnemonic checks that a vault
+// created without a mnemonic, which no mnemonic opens, gets no advice to
+// use one: `secret unlocker add passphrase` there fails with the cause
+// alone.
+func TestUnlockFailureWithoutLongTermKeyNamesNoMnemonic(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+
+	_, err := vault.CreateVault(fs, testStateDir, "keyless", nil, nil)
+	require.NoError(t, err)
+
+	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
+	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(c.UnlockPassphrase.Destroy)
+
+	err = c.UnlockersAdd("passphrase", discardCmd())
+
+	var cause *os.PathError
+
+	require.ErrorAs(t, err, &cause)
+
+	require.EqualError(t, err, "failed to get long-term key: "+
+		"failed to get current unlocker: failed to read current unlocker: "+
+		cause.Error())
+}

internal/secret/passphraseunlocker.go

@@ -1,6 +1,7 @@
 package secret
 
 import (
+	"errors"
 	"fmt"
 	"log/slog"
 	"path/filepath"
@@ -10,6 +11,11 @@ import (
 	"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
 type PassphraseUnlocker struct {
 	Directory  string
@@ -149,7 +155,7 @@ func (p *PassphraseUnlocker) getPassphrase() (*memguard.LockedBuffer, error) {
 	if err != nil {
 		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

internal/secret/secret.go

@@ -1,26 +1,18 @@
 package secret
 
 import (
-	"encoding/json"
 	"errors"
-	"fmt"
 	"log/slog"
 	"path/filepath"
 	"strings"
 	"time"
 
 	"filippo.io/age"
-	"git.eeqj.de/sneak/secret/pkg/agehd"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 )
 
 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(
 		"GetEncryptedData is deprecated - use version-specific methods")
 	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
 func (s *Secret) LoadMetadata() error {
 	Debug("LoadMetadata called but is deprecated in versioned model",
@@ -215,124 +140,6 @@ func (s *Secret) Exists() (bool, error) {
 	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
 // This function is a wrapper around the actual implementation in the vault package
 // and exists to break the import cycle.

internal/secret/secret_test.go

@@ -2,7 +2,6 @@
 package secret
 
 import (
-	"encoding/json"
 	"errors"
 	"os"
 	"path/filepath"
@@ -13,7 +12,6 @@ import (
 	"git.eeqj.de/sneak/secret/pkg/agehd"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
-	"github.com/stretchr/testify/require"
 )
 
 // testMnemonicValue is the standard BIP39 test vector mnemonic.
@@ -321,46 +319,3 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
 		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())
-}

internal/secret/version.go

@@ -557,13 +557,18 @@ func VersionExists(fs afero.Fs, secretDir string, version string) (bool, error)
 }
 
 // GetCurrentVersion returns the version that the "current" file points to
-// The file contains just the version name (e.g., "20231215.001")
+// The file contains just the version name (e.g., "20231215.001"). If it
+// cannot be read, the error says how to make a version current again: the
+// versions themselves are not in the file.
 func GetCurrentVersion(fs afero.Fs, secretDir string) (string, error) {
 	currentPath := filepath.Join(secretDir, "current")
 
 	fileData, err := afero.ReadFile(fs, currentPath)
 	if err != nil {
-		return "", fmt.Errorf("failed to read current version file: %w", err)
+		return "", fmt.Errorf("failed to read current version file: %w; "+
+			"this file only names the current version: 'secret version list' "+
+			"lists the secret's versions, and 'secret version promote' makes "+
+			"one of them current", err)
 	}
 
 	version := strings.TrimSpace(string(fileData))

internal/vault/vault.go

@@ -1,6 +1,7 @@
 package vault
 
 import (
+	"errors"
 	"fmt"
 	"log/slog"
 	"path/filepath"
@@ -98,7 +99,8 @@ func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
 	if err != nil {
 		secret.Debug("Failed to get current unlocker", "error", err, "vault_name", v.Name)
 
-		return nil, fmt.Errorf("failed to get current unlocker: %w", err)
+		return nil, v.withMnemonicAdvice(
+			fmt.Errorf("failed to get current unlocker: %w", err))
 	}
 
 	secret.DebugWith("Retrieved current unlocker for vault unlock",
@@ -112,7 +114,7 @@ func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
 	// Other unlockers return their own identity, used to decrypt longterm.age.
 	ltIdentity, err := v.unlockLongTermKey(unlocker)
 	if err != nil {
-		return nil, err
+		return nil, v.withMnemonicAdvice(err)
 	}
 
 	secret.DebugWith("Successfully obtained long-term identity via unlocker",
@@ -295,3 +297,36 @@ func (v *Vault) unlockLongTermKey(
 
 	return ltIdentity, nil
 }
+
+// withMnemonicAdvice returns err, a failure to get the long-term key through
+// the current unlocker, with advice added: that the mnemonic still opens 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
+// 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.
+func (v *Vault) withMnemonicAdvice(err error) error {
+	if errors.Is(err, secret.ErrPassphraseNotRead) {
+		return err
+	}
+
+	vaultDir, _ := v.GetDirectory()
+
+	metadata, metadataErr := LoadVaultMetadata(v.fs, vaultDir)
+	if metadataErr != nil || metadata.PublicKeyHash == "" {
+		return err
+	}
+
+	// 'secret unlocker add' acts on the current vault only.
+	steps := "'secret unlocker add passphrase'"
+
+	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)
+}