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)
+}