Log the audit fixes in TODO.md (closes #173)

Completed Steps gets one entry, dated 2026-10-15, for each fix merged
into next since v1.0.0-rc2 that had none, and for
#152 and
#189, whose PRs are still in
review. Each entry keeps to what its landing commit or PR says. The five
fixes that logged themselves before this issue's decision keep their
entries. Status no longer says there are no git tags.

Unverified: the entries for 152 and 189 describe the PRs as they stand;
neither had merged when this was pushed.

Model: opus-5-5

README.md

@@ -438,14 +438,6 @@ asks when stdin is a pipe, as in `echo value | secret add name --force` or
 `cat file | secret encrypt key`. With no controlling terminal, as under cron or
 in a CI job, the command fails and names the variable to set.
 
-A command that changes anything locks the state directory, through its file
-`lock`, and another such command waits until the lock is released.
-`secret init`, `secret vault create`, `secret unlocker add`, `secret mv` from
-one vault to another, and `secret add`, `secret import` and
-`secret generate secret` with `--force`, ask for the mnemonic or passphrase
-before they take the lock, so a prompt of theirs left open in one terminal holds
-up no command in another.
-
 ## Security Features
 
 ### Encryption

TODO.md

@@ -8,8 +8,7 @@
 
 # Status
 
-pre-1.0. No git tags. Open work is tracked on the issue tracker, which is
+pre-1.0. Open work is tracked on the issue tracker, which is authoritative.
-authoritative.
 
 # Next Step
 
@@ -18,6 +17,176 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
+- 2026-10-15: `secret mv` from one vault to another unlocks both vaults before
+  it takes the state directory lock
+  (https://git.eeqj.de/sneak/secret/issues/189). Unlocking can ask for a
+  passphrase, so while the move held the lock, every other command that changes
+  anything waited at that prompt. `moveSecretCrossVault` checks that the source
+  secret and the destination vault exist, unlocks both vaults, and only then
+  locks. Under the lock, `copySecretAndRemoveSource` checks again that the
+  source and the destination vault still exist, then copies the secret and
+  removes the source as before. `MoveSecret` takes no lock itself; a move within
+  a vault takes it in `moveSecretWithinVault`.
+- 2026-10-15: `secret init`, `secret vault create`, `secret unlocker add`, and
+  `secret add`, `secret import` and `secret generate secret` with `--force` on
+  an existing secret ask for the mnemonic or passphrase and unlock the vault
+  before they take the state directory lock
+  (https://git.eeqj.de/sneak/secret/issues/152). While one of them waited at its
+  prompt, every other command that changes anything waited with it. What a
+  command checked before asking is checked again under the lock, since another
+  command may have changed it meanwhile. `secret unlocker add` fails if the
+  vault was removed, and `secret unlocker add pgp` refuses a GPG key added in
+  the meantime. A forced add of a secret that another command created meanwhile
+  lets go of the lock, gets the key and takes the lock again, so it still
+  replaces the secret. `secret.CreatePGPUnlocker` now takes the vault, already
+  unlocked; `SetMnemonic` and `SetUnlockPassphrase`, which only it used, are
+  gone. The README says which commands ask before they lock.
+- 2026-10-15: A `secret mv` to another vault that copies the secret but cannot
+  remove the source fails (https://git.eeqj.de/sneak/secret/issues/159). It
+  printed a warning and the usual "Moved" line and exited 0. If the source
+  secret's directory is still in place, the error says the copy holds all
+  versions and the source remains. If the source left its vault and only the
+  temporary directory it was moved into could not be deleted, the error says the
+  secret is moved and names that directory, as the error of `RemoveDirAtomic`
+  now does.
+- 2026-10-15: `secret encrypt` and `secret decrypt` with `--output` hold the
+  result in memory and open the output file only once encryption or decryption
+  has succeeded (https://git.eeqj.de/sneak/secret/issues/143). They emptied that
+  file before reading any input, so a failed command left it empty, and
+  encrypting a file onto itself gave the encryption of nothing. A failure to
+  encrypt or decrypt now leaves `--output` untouched, and `--input` can name the
+  same file. A failure while writing `--output`, such as a full disk, can still
+  leave it empty or partly written; an error from closing it is reported.
+  Without `--output` the result still goes to stdout as it is produced.
+- 2026-10-15: A command that uses the current vault fails when `currentvault` is
+  empty or names no vault (https://git.eeqj.de/sneak/secret/issues/156).
+  `secret list` reported an empty vault with exit status 0, and `secret add`
+  failed on a missing directory. `ResolveVaultSymlink` now refuses an empty or
+  invalid name, or one with no directory under `vaults.d`, with an error that
+  wraps `ErrVaultNotFound`, names the file and says to run
+  `secret vault select <name>`. `secret vault list` and `secret info` still run.
+- 2026-10-15: `secret` prints every time in UTC, whatever the machine's time
+  zone (https://git.eeqj.de/sneak/secret/issues/169). `secret list`,
+  `secret version list`, `secret unlocker list` and `secret info` printed times
+  in the local time zone without naming it, so one vault showed different times
+  on two machines. `Entry()` now sets `time.Local` to UTC before anything else,
+  as policy 6 in `AGENTS.md` asks, so every time the program prints or writes is
+  in UTC. `--json` times end in `Z`, and a new version's name takes the UTC
+  date. The README says so.
+- 2026-10-15: `secret vault`, `secret unlocker`, `secret version` and
+  `secret generate` fail when given a subcommand they do not have
+  (https://git.eeqj.de/sneak/secret/issues/183). They printed their help and
+  exited 0, so a mistyped command looked like one that worked. Each now exits 1
+  with `unknown command "<name>" for "secret <parent>"`, and with no arguments
+  still prints its help. Cobra checks a command's `Args` only when it has a
+  `RunE`, so each gets `Args: cobra.NoArgs` and a `RunE` that prints its help;
+  their help now shows a `secret <parent> [flags]` usage line.
+  `secret vault help` fails as an unknown subcommand, while `secret help vault`
+  and `secret vault --help` still work.
+- 2026-10-15: With `SB_UNLOCK_PASSPHRASE` set, `secret unlocker add passphrase`
+  asks on the terminal for the new unlocker's passphrase and uses the variable
+  to unlock the vault (https://git.eeqj.de/sneak/secret/issues/154). It gave the
+  new unlocker the passphrase in the variable without asking. With no
+  controlling terminal, the new unlocker takes the variable's passphrase and the
+  output says so. `secret.ErrNoTerminal` is exported so that the command can
+  tell a missing terminal from other failures to read, such as nothing typed or
+  a confirmation that does not match, which still fail the command.
+- 2026-10-15: `secret init` and `secret vault create` stop with an error naming
+  the file when a vault's `vault-metadata.json` cannot be read or parsed, and
+  leave the state directory as it was
+  (https://git.eeqj.de/sneak/secret/issues/140). `GetNextDerivationIndex` passed
+  over such a vault, so the next vault made from the same mnemonic could get its
+  derivation index and so the same long-term key. A vault directory with no
+  metadata file is still skipped.
+- 2026-10-15: The README says that `secret add` drops one trailing newline from
+  stdin, that `secret import` stores a file's bytes unchanged, and that
+  `secret get` writes the value with no newline added
+  (https://git.eeqj.de/sneak/secret/issues/158). It said none of this, so a PEM
+  key piped into `secret add` came back one byte short with no hint why. A new
+  test stores `a\n\n` through `add` and through `import` and checks that `add`
+  kept two bytes and `import` three. No program code changes.
+- 2026-10-15: When gpg fails to decrypt, to encrypt or to look up a key's
+  fingerprint, the error ends with what gpg wrote to stderr, trimmed
+  (https://git.eeqj.de/sneak/secret/issues/146). It gave only gpg's exit status,
+  so the user could not tell why. `cmd.Output` keeps gpg's stderr in the
+  `*exec.ExitError` it returns, and the three errors now use it. When gpg wrote
+  nothing to stderr the error is as before, and when gpg wrote several lines the
+  error has them all.
+- 2026-10-15: `secret version list` no longer shows a version without
+  `NOT_AFTER` as expired (https://git.eeqj.de/sneak/secret/issues/150). STATUS
+  was `expired` for every version other than the current one, so after a promote
+  the newest version, which has no `NOT_AFTER`, read `expired`. STATUS is now
+  `current` for the current version, `expired` for another version whose
+  `NOT_AFTER` has passed, and `inactive` for any other version. The README
+  describes the three.
+- 2026-10-15: `secret unlocker list --json` for a vault with no unlockers, and
+  `secret vault list --json` when `vaults.d` holds no vaults, print the empty
+  list as `[]` instead of `null` (https://git.eeqj.de/sneak/secret/issues/148).
+  Both lists were nil slices, which `encoding/json` writes as `null`; each is
+  now made as an empty slice before it is filled. `vault.ListVaults` therefore
+  returns an empty slice instead of nil for an empty `vaults.d`.
+- 2026-10-15: `secret add`, `secret import` and `secret generate secret` fail
+  with an error, not memguard's panic, when a secret is too large for the
+  locked-memory limit (https://git.eeqj.de/sneak/secret/issues/145). The value
+  was held twice, so half the usual 8 MiB limit was enough to crash them.
+  `secret` now raises the limit to the hard limit at startup, and each of the
+  three commands refuses, before its first locked allocation, a limit that
+  leaves fewer than 8 pages for its keys and buffers. The value is read a page
+  at a time into a `memguard.Stream`, which keeps it encrypted, and copied once
+  into locked memory after its size is checked against 100 MiB and the limit. It
+  leaves the stream through `Next`, not `Read`, because `Read` re-encrypts a
+  leftover piece and memguard hangs if that runs out of locked memory.
+- 2026-10-15: Total Size in `secret info` counts every file of each version: the
+  value, the metadata and the version's two keys
+  (https://git.eeqj.de/sneak/secret/issues/147). It added up each version's
+  `data.age` and `metadata.age`, but a version keeps its value in `value.age`,
+  so no secret's value was counted. A version directory that cannot be read adds
+  nothing and gets a warning naming it.
+- 2026-10-15: `secret vault import` is gone, with its tests and its README entry
+  (https://git.eeqj.de/sneak/secret/issues/139). It took only a vault without a
+  long-term key, and `secret init` and `secret vault create` give every vault
+  one, so it always failed; a mnemonic enters through those two commands. The
+  help and the note of `secret generate mnemonic` named `secret import`, which
+  imports a file as a secret, and now name `secret init` and
+  `secret vault create`.
+- 2026-10-15: `secret version promote --help` says promote rewrites the
+  `current` file, not a "current symlink"
+  (https://git.eeqj.de/sneak/secret/issues/153). `current` is a plain file
+  holding the version name, as the README already said, and the comment in
+  `PromoteVersion` now says so too.
+- 2026-10-15: `secret init` and `secret vault create` refuse a vault that
+  already exists before they ask for the mnemonic or the unlocker passphrase
+  (https://git.eeqj.de/sneak/secret/issues/151). They asked first, and
+  `secret init` printed "Initialized secrets manager at" before refusing. The
+  check `vault.CreateVault` made before writing is now
+  `vault.CheckNewVaultName`, which both commands call before the first prompt
+  and `CreateVault` still calls. It validates the name too, so an invalid name
+  also fails before the prompts. `secret init` prints the "Initialized" line
+  only once the vault is created.
+- 2026-10-15: `secret get` fails when it cannot write the value
+  (https://git.eeqj.de/sneak/secret/issues/155). `GetSecret` and
+  `GetSecretWithVersion` discarded the result of writing it to stdout, so a full
+  disk or a failing stdout left an empty or cut-short file and exit status 0.
+  The write error is now returned, so the command prints it and exits 1. A
+  stdout the shell closed before `secret` started still exits 0, because the Go
+  runtime opens `/dev/null` in its place and the write there succeeds.
+- 2026-10-15: `secret list` and `secret list --json` give each secret's own
+  creation and last-update times (https://git.eeqj.de/sneak/secret/issues/142).
+  They gave the time of the listing for every secret, because
+  `Secret.LoadMetadata` set both to the current time. It now reads file
+  modification times, so listing still needs no passphrase. The last update is
+  when the secret's `current` file was written. The creation is when its oldest
+  version's `value.age` was written, not that version's directory, whose time
+  changes when the next add rewrites its `metadata.age`. A move to another vault
+  gives the copied files the times of the originals. `GetSecretObject` no longer
+  loads the times, so `secret get` keeps its error for a missing `current` file.
+- 2026-10-15: `secret unlocker add pgp` without `--keyid` uses the `default-key`
+  in `gpg.conf` (https://git.eeqj.de/sneak/secret/issues/141).
+  `getDefaultGPGKey` took that value from `gpgconf --list-options gpg` as
+  printed, with a leading `"` and percent-escaped, so gpg rejected the key ID.
+  The new `parseDefaultGPGKey` drops the `"` and percent-decodes the rest with
+  `url.PathUnescape`, which leaves `+` alone. A value that does not decode is an
+  error, not a fallback to the first secret key.
 - 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the
   prompt, gives the error `wrong passphrase for unlocker <id>`
   (https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no

internal/cli/generate.go

@@ -175,6 +175,12 @@ func (cli *Instance) GenerateSecret(
 		return fmt.Errorf("failed to generate random secret: %w", err)
 	}
 
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Store the secret in the vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -187,7 +193,7 @@ func (cli *Instance) GenerateSecret(
 	secretBuffer := memguard.NewBufferFromBytes([]byte(secretValue))
 	defer secretBuffer.Destroy()
 
-	err = cli.storeSecret(vlt, secretName, secretBuffer, force)
+	err = vlt.AddSecret(secretName, secretBuffer, force)
 	if err != nil {
 		return err
 	}

internal/cli/init.go

@@ -67,15 +67,36 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
 	return mnemonicBuffer, mnemonicBuffer.Destroy, nil
 }
 
-// Init initializes the secret manager. It asks for the mnemonic and the
+// Init initializes the secret manager, holding the state directory lock
-// passphrase before it takes the state directory lock, so that no other
+// while initialize runs
-// command waits while the user answers, and holds the lock while
-// initialize runs.
 func (cli *Instance) Init(cmd *cobra.Command) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	return cli.initialize(cmd)
+}
+
+// initialize creates the state directory, the default vault and its first
+// unlocker
+func (cli *Instance) initialize(cmd *cobra.Command) error {
 	secret.Debug("Starting secret manager initialization")
 
+	// Create state directory
+	stateDir := cli.GetStateDir()
+	secret.DebugWith("Creating state directory", slog.String("path", stateDir))
+
+	err := cli.fs.MkdirAll(stateDir, secret.DirPerms)
+	if err != nil {
+		secret.Debug("Failed to create state directory", "error", err)
+
+		return fmt.Errorf("failed to create state directory: %w", err)
+	}
+
 	// Refuse an existing default vault before asking for anything
-	err := vault.CheckNewVaultName(cli.fs, cli.stateDir, defaultVaultName)
+	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, defaultVaultName)
 	if err != nil {
 		return fmt.Errorf("failed to create default vault: %w", err)
 	}
@@ -114,38 +135,11 @@ func (cli *Instance) Init(cmd *cobra.Command) error {
 	}
 	defer cleanupPassphrase()
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
-	if err != nil {
-		return err
-	}
-	defer release()
-
-	return cli.initialize(cmd, mnemonic, passphraseBuffer)
-}
-
-// initialize creates the state directory, and the default vault from
-// mnemonic with a passphrase unlocker protected by passphrase. Another
-// command may have created the vault since Init checked: vault.CreateVault
-// checks again.
-func (cli *Instance) initialize(
-	cmd *cobra.Command, mnemonic, passphrase *memguard.LockedBuffer,
-) error {
-	// Create state directory
-	stateDir := cli.GetStateDir()
-	secret.DebugWith("Creating state directory", slog.String("path", stateDir))
-
-	err := cli.fs.MkdirAll(stateDir, secret.DirPerms)
-	if err != nil {
-		secret.Debug("Failed to create state directory", "error", err)
-
-		return fmt.Errorf("failed to create state directory: %w", err)
-	}
-
 	// Create the default vault with its passphrase unlocker
 	secret.Debug("Creating default vault")
 
 	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, defaultVaultName,
-		mnemonic, passphrase)
+		mnemonic, passphraseBuffer)
 	if err != nil {
 		secret.Debug("Failed to create default vault", "error", err)
 

internal/cli/lock_test.go

@@ -280,9 +280,9 @@ func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
 	return modTimes
 }
 
-// setupEveryCommand makes what the commands that the lock tests below run
+// setupEveryCommand makes what each command in
-// need: the current vault "work" with two versions of "test/secret", the
+// TestChangingCommandsWaitForLock needs: the current vault "work" with two
-// vault "other", and the file testInput.
+// versions of "test/secret", the vault "other", and the file testInput.
 // There is no vault "default", which init creates. If withUnlocker is set,
 // it also gives "work" a passphrase unlocker, which is slow. It returns the
 // older version and the unlocker's ID.
@@ -346,25 +346,6 @@ func waitingForLock() bool {
 	return false
 }
 
-// requireWaitingForLock returns once a command waits for the state
-// directory lock. It fails the test if the command finishes first, sending
-// its error on done, or never waits.
-func requireWaitingForLock(t *testing.T, done <-chan error) {
-	t.Helper()
-
-	timeout := time.After(lockWait)
-
-	for !waitingForLock() {
-		select {
-		case err := <-done:
-			t.Fatalf("finished while the lock was held, with error %v", err)
-		case <-timeout:
-			t.Fatal("never waited for the lock")
-		case <-time.After(time.Millisecond):
-		}
-	}
-}
-
 // requireWaitsForLock runs a command, given what setupEveryCommand made,
 // while holding the state directory lock. The command must neither finish
 // nor change anything before it waits for the lock, and must succeed once
@@ -404,7 +385,17 @@ func requireWaitsForLock(
 
 	go func() { done <- run(cli, olderVersion, unlockerID) }()
 
-	requireWaitingForLock(t, done)
+	timeout := time.After(lockWait)
+
+	for !waitingForLock() {
+		select {
+		case err := <-done:
+			t.Fatalf("finished while the lock was held, with error %v", err)
+		case <-timeout:
+			t.Fatal("never waited for the lock")
+		case <-time.After(time.Millisecond):
+		}
+	}
 
 	assert.Equal(t, before, stateDirModTimes(t, fs),
 		"changed the state directory before waiting for the lock")
@@ -481,311 +472,6 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
 	}
 }
 
-// TestCommandsAskBeforeLocking is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/152: these commands took the
-// state directory lock before asking for the mnemonic or a passphrase, so
-// while one waited at its prompt, every other command that changes anything
-// waited for it. Each runs here while the test holds the lock, given what
-// takes it to the last step at which it can ask: the passphrase prompt of
-// init and vault create, and the unlock of the vault, which asks for the
-// passphrase of its passphrase unlocker, for the others. The tests have no
-// terminal, so a prompt fails at once. Each command must get there, and
-// fail, without waiting for the lock.
-//
-//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
-func TestCommandsAskBeforeLocking(t *testing.T) {
-	newTestGPGKey(t)
-
-	mnemonic := testMnemonicBuffer(t)
-
-	wrongPassphrase := memguard.NewBufferFromBytes([]byte("wrong-passphrase"))
-	t.Cleanup(wrongPassphrase.Destroy)
-
-	for _, tc := range []struct {
-		name string
-		run  func(cli *Instance) error
-		// want is wrapped in the error the command fails with
-		want error
-	}{
-		{"init", func(cli *Instance) error {
-			cli.Mnemonic = mnemonic
-
-			return cli.Init(cli.cmd)
-		}, secret.ErrNoTerminal},
-		{"vault create", func(cli *Instance) error {
-			cli.Mnemonic = mnemonic
-
-			return cli.CreateVault(cli.cmd, "created")
-		}, secret.ErrNoTerminal},
-		// With no terminal to ask for the new passphrase on, the new
-		// unlocker takes the one in SB_UNLOCK_PASSPHRASE, which then fails
-		// to unlock the vault
-		{"unlocker add passphrase", func(cli *Instance) error {
-			cli.UnlockPassphrase = wrongPassphrase
-
-			return cli.UnlockersAdd("passphrase", cli.cmd)
-		}, secret.ErrWrongPassphrase},
-		{"unlocker add pgp", func(cli *Instance) error {
-			cli.cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
-
-			return cli.UnlockersAdd("pgp", cli.cmd)
-		}, secret.ErrNoTerminal},
-		{"add --force", func(cli *Instance) error {
-			return cli.AddSecret("test/secret", true)
-		}, secret.ErrNoTerminal},
-		{"import --force", func(cli *Instance) error {
-			return cli.ImportSecret(cli.cmd, "test/secret", testInput, true)
-		}, secret.ErrNoTerminal},
-		{"generate secret --force", func(cli *Instance) error {
-			return cli.GenerateSecret(cli.cmd, "test/secret", 16, "base58", true)
-		}, secret.ErrNoTerminal},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			requireAsksBeforeLocking(t, tc.run, tc.want)
-		})
-	}
-}
-
-// startWhileLocked makes what setupEveryCommand makes, with the passphrase
-// unlocker, on fs, an empty in-memory filesystem, takes the state directory
-// lock there, and starts run, given a CLI instance on fs with "value" on
-// stdin. It returns the function that releases the lock, which does so only
-// the first time it is called, and the channel that run's error comes on.
-// The lock is also released when the test ends, so that later tests can
-// take it.
-func startWhileLocked(
-	t *testing.T, fs afero.Fs, run func(cli *Instance) error,
-) (func(), <-chan error) {
-	t.Helper()
-
-	setupEveryCommand(t, fs, true)
-
-	release, err := vault.LockStateDir(fs, testStateDir)
-	require.NoError(t, err)
-
-	release = sync.OnceFunc(release)
-	t.Cleanup(release)
-
-	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
-	cli.cmd = &cobra.Command{}
-	cli.cmd.SetIn(strings.NewReader("value"))
-	cli.cmd.SetOut(io.Discard)
-
-	done := make(chan error, 1)
-
-	go func() { done <- run(cli) }()
-
-	return release, done
-}
-
-// requireAsksBeforeLocking starts run while holding the state directory
-// lock, as startWhileLocked does. run must fail, with an error wrapping
-// want, without waiting for the lock.
-func requireAsksBeforeLocking(
-	t *testing.T, run func(cli *Instance) error, want error,
-) {
-	t.Helper()
-
-	release, done := startWhileLocked(t, afero.NewMemMapFs(), run)
-
-	timeout := time.After(lockWait)
-
-	for !waitingForLock() {
-		select {
-		case err := <-done:
-			require.ErrorIs(t, err, want)
-
-			return
-		case <-timeout:
-			t.Fatal("neither failed nor waited for the lock")
-		case <-time.After(time.Millisecond):
-		}
-	}
-
-	// Let the command go on, so that it releases the lock
-	release()
-
-	select {
-	case <-done:
-	case <-time.After(lockWait):
-	}
-
-	t.Fatal("waited for the lock before asking")
-}
-
-// TestCommandsCheckAgainOnceLocked checks that commands which check what
-// they change before they ask for anything, and so before they take the
-// state directory lock, check again once they hold it: another command may
-// have changed it in the meantime. Each runs while the test holds the lock,
-// given what it needs to ask for nothing. Once it waits for the lock, the
-// test makes the change another command could have made, then releases the
-// lock. The command must fail, and change nothing.
-//
-//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
-func TestCommandsCheckAgainOnceLocked(t *testing.T) {
-	fingerprint := newTestGPGKey(t)
-
-	mnemonic := testMnemonicBuffer(t)
-
-	otherMnemonic := memguard.NewBufferFromBytes([]byte("legal winner thank " +
-		"year wave sausage worth useful legal winner thank yellow"))
-	t.Cleanup(otherMnemonic.Destroy)
-
-	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
-	t.Cleanup(passphrase.Destroy)
-
-	// work returns the vault "work" on fs, opened with the mnemonic
-	work := func(fs afero.Fs) *vault.Vault {
-		vlt := vault.NewVault(fs, testStateDir, "work")
-		vlt.Mnemonic = mnemonic
-
-		return vlt
-	}
-
-	for _, tc := range []struct {
-		name string
-		run  func(cli *Instance) error
-		// change is what another command changes while run waits
-		change func(fs afero.Fs) error
-		// want is wrapped in the error the command fails with
-		want error
-	}{
-		{
-			"unlocker add, vault removed",
-			func(cli *Instance) error {
-				cli.Mnemonic, cli.UnlockPassphrase = mnemonic, passphrase
-
-				return cli.UnlockersAdd("passphrase", cli.cmd)
-			},
-			func(fs afero.Fs) error {
-				return fs.RemoveAll(testStateDir + "/vaults.d/work")
-			},
-			vault.ErrVaultNotFound,
-		},
-		{
-			"unlocker add, vault created again from another mnemonic",
-			func(cli *Instance) error {
-				cli.Mnemonic, cli.UnlockPassphrase = mnemonic, passphrase
-
-				return cli.UnlockersAdd("passphrase", cli.cmd)
-			},
-			func(fs afero.Fs) error {
-				err := fs.RemoveAll(testStateDir + "/vaults.d/work")
-				if err != nil {
-					return err
-				}
-
-				_, err = vault.CreateVault(fs, testStateDir, "work",
-					otherMnemonic, passphrase)
-
-				return err
-			},
-			errVaultCreatedAgain,
-		},
-		{
-			"unlocker add pgp, key added",
-			func(cli *Instance) error {
-				cli.Mnemonic = mnemonic
-				cli.cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
-
-				return cli.UnlockersAdd("pgp", cli.cmd)
-			},
-			func(fs afero.Fs) error {
-				_, err := secret.CreatePGPUnlocker(work(fs),
-					unreadableTestGPGUserID, fingerprint)
-
-				return err
-			},
-			errGPGKeyAlreadyUnlocker,
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			requireChecksAgainOnceLocked(t, tc.run, tc.change, tc.want)
-		})
-	}
-}
-
-// requireChecksAgainOnceLocked starts run while holding the state directory
-// lock, as startWhileLocked does. Once run waits for the lock, change makes
-// the change that another command could have made in the meantime, and the
-// lock is released. run must then fail, with an error wrapping want, and
-// change nothing.
-func requireChecksAgainOnceLocked(
-	t *testing.T, run func(cli *Instance) error,
-	change func(fs afero.Fs) error, want error,
-) {
-	t.Helper()
-
-	fs := afero.NewMemMapFs()
-	release, done := startWhileLocked(t, fs, run)
-
-	requireWaitingForLock(t, done)
-	require.NoError(t, change(fs))
-
-	// Left out: taking and releasing the lock writes the lock file
-	lockFile := testStateDir + "/lock"
-
-	before := stateDirModTimes(t, fs)
-	delete(before, lockFile)
-
-	release()
-
-	select {
-	case err := <-done:
-		require.ErrorIs(t, err, want)
-	case <-time.After(lockWait):
-		t.Fatal("did not finish once the lock was released")
-	}
-
-	after := stateDirModTimes(t, fs)
-	delete(after, lockFile)
-
-	assert.Equal(t, before, after, "changed the state directory")
-}
-
-// TestForcedAddReplacesSecretAddedMeanwhile runs a forced add of a secret
-// that does not exist yet while the test holds the state directory lock.
-// Once it waits for the lock, the test adds the secret, as another command
-// could have, and releases the lock. The forced add must replace that
-// secret, as if it had run after the other command.
-//
-//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
-func TestForcedAddReplacesSecretAddedMeanwhile(t *testing.T) {
-	mnemonic := testMnemonicBuffer(t)
-
-	fs := afero.NewMemMapFs()
-	release, done := startWhileLocked(t, fs, func(cli *Instance) error {
-		cli.Mnemonic = mnemonic
-
-		return cli.AddSecret("added", true)
-	})
-
-	requireWaitingForLock(t, done)
-
-	work := vault.NewVault(fs, testStateDir, "work")
-	work.Mnemonic = mnemonic
-
-	other := memguard.NewBufferFromBytes([]byte("other"))
-	t.Cleanup(other.Destroy)
-
-	require.NoError(t, work.AddSecret("added", other, false))
-
-	release()
-
-	select {
-	case err := <-done:
-		require.NoError(t, err)
-	case <-time.After(lockWait):
-		t.Fatal("did not finish once the lock was released")
-	}
-
-	value, err := work.GetSecret("added")
-	require.NoError(t, err)
-	t.Cleanup(value.Destroy)
-
-	assert.Equal(t, "value", string(value.Bytes()))
-}
-
 // TestEncryptWithExistingKeyTakesNoLock checks that secret encrypt with a
 // key that already exists, which only reads the state directory, finishes
 // while another command holds the state directory lock.

internal/cli/move_cross_vault_test.go

@@ -5,13 +5,11 @@ import (
 	"errors"
 	"path/filepath"
 	"testing"
-	"time"
 
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
 )
 
@@ -94,8 +92,7 @@ func requireValue(t *testing.T, fs afero.Fs, vaultName, secretName string) {
 // vaults whose source could not be removed reported the secret moved and
 // succeeded. It must fail, and the copy and the source must both be there.
 // The state directory lock refuses the wrapped filesystem, so the test
-// calls copySecretAndRemoveSource, which a move between vaults runs once it
+// calls moveSecret, which MoveSecret runs once it holds the lock.
-// holds the lock.
 func TestCrossVaultMoveFailsWhenSourceRemains(t *testing.T) {
 	t.Parallel()
 
@@ -106,12 +103,9 @@ func TestCrossVaultMoveFailsWhenSourceRemains(t *testing.T) {
 		Fs:   base,
 		path: testStateDir + "/vaults.d/default/secrets.d/x",
 	}, testStateDir)
+	instance.Mnemonic = testMnemonicBuffer(t)
 
-	source := vault.NewVault(instance.fs, testStateDir, "default")
+	err := instance.moveSecret(&cobra.Command{}, "default:x", "work:y", false)
-	source.Mnemonic = testMnemonicBuffer(t)
-
-	err := instance.copySecretAndRemoveSource(&cobra.Command{}, source, "x",
-		vault.NewVault(instance.fs, testStateDir, "work"), "y", false)
 	require.ErrorIs(t, err, errPermissionDenied)
 	require.ErrorContains(t, err, "secret 'default:x' is copied with all "+
 		"1 version(s) to 'work:y', but the source remains")
@@ -134,12 +128,9 @@ func TestCrossVaultMoveFailsWhenOldFilesRemain(t *testing.T) {
 	fs := &removeAllFailFs{Fs: base, dir: sourceVaultDir}
 
 	instance := NewCLIInstanceWithStateDir(fs, testStateDir)
+	instance.Mnemonic = testMnemonicBuffer(t)
 
-	source := vault.NewVault(fs, testStateDir, "default")
+	err := instance.moveSecret(&cobra.Command{}, "default:x", "work:y", false)
-	source.Mnemonic = testMnemonicBuffer(t)
-
-	err := instance.copySecretAndRemoveSource(&cobra.Command{}, source, "x",
-		vault.NewVault(fs, testStateDir, "work"), "y", false)
 	require.ErrorIs(t, err, errPermissionDenied)
 	require.ErrorContains(t, err, "secret 'default:x' is moved with all "+
 		"1 version(s) to 'work:y', but its old files could not all be deleted")
@@ -159,157 +150,3 @@ func TestCrossVaultMoveFailsWhenOldFilesRemain(t *testing.T) {
 
 	requireValue(t, base, "work", "y")
 }
-
-// TestCrossVaultMoveUnlocksBeforeLocking is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/189: a move between two vaults
-// unlocked them while holding the state directory lock, and unlocking a
-// vault can ask for its passphrase, so every other command that changes
-// anything waited while the user answered. Here each vault has a passphrase
-// unlocker whose passphrase is the vault's name, and the move is given the
-// passphrase of only one of them. Run while the test holds the lock, it
-// must fail to unlock the other vault without waiting for the lock.
-//
-//nolint:paralleltest // times commands against the in-memory lock all tests share
-func TestCrossVaultMoveUnlocksBeforeLocking(t *testing.T) {
-	fs := afero.NewMemMapFs()
-	createCrossVaults(t, fs)
-
-	vaults, err := vault.ListVaults(fs, testStateDir)
-	require.NoError(t, err)
-
-	for _, name := range vaults {
-		vlt := vault.NewVault(fs, testStateDir, name)
-		vlt.Mnemonic = testMnemonicBuffer(t)
-
-		passphrase := memguard.NewBufferFromBytes([]byte(name))
-		_, err = vlt.CreatePassphraseUnlocker(passphrase)
-		passphrase.Destroy()
-		require.NoError(t, err)
-	}
-
-	// The move is from "default" to "work"
-	for _, tc := range []struct {
-		// failing is the vault that the move cannot unlock, given the
-		// passphrase of the vault named given
-		failing, given string
-	}{
-		{"source", "work"},
-		{"destination", "default"},
-	} {
-		t.Run(tc.failing, func(t *testing.T) {
-			passphrase := memguard.NewBufferFromBytes([]byte(tc.given))
-			defer passphrase.Destroy()
-
-			instance := NewCLIInstanceWithStateDir(fs, testStateDir)
-			instance.UnlockPassphrase = passphrase
-
-			release, err := vault.LockStateDir(fs, testStateDir)
-			require.NoError(t, err)
-			// Also frees a waiting move if the test fails, so that it
-			// releases the lock the other tests use
-			defer release()
-
-			done := make(chan error, 1)
-
-			go func() {
-				done <- instance.MoveSecret(&cobra.Command{},
-					"default:x", "work:y", false)
-			}()
-
-			select {
-			case err := <-done:
-				require.ErrorIs(t, err, secret.ErrWrongPassphrase)
-				require.ErrorContains(t, err,
-					"failed to unlock "+tc.failing+" vault")
-			case <-time.After(lockWait):
-				t.Fatal("waited for the lock before unlocking the " +
-					tc.failing + " vault")
-			}
-		})
-	}
-}
-
-// TestCrossVaultMoveChecksAgainOnceLocked runs a move between vaults, given
-// the mnemonic, as TestCommandsCheckAgainOnceLocked runs the commands in its
-// table: once the move waits for the state directory lock, the test removes
-// the source secret or the destination vault, or creates the destination
-// vault again from another mnemonic, and the move must then fail and change
-// nothing.
-//
-//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
-func TestCrossVaultMoveChecksAgainOnceLocked(t *testing.T) {
-	mnemonic := testMnemonicBuffer(t)
-
-	otherMnemonic := memguard.NewBufferFromBytes([]byte("legal winner thank " +
-		"year wave sausage worth useful legal winner thank yellow"))
-	t.Cleanup(otherMnemonic.Destroy)
-
-	move := func(cli *Instance) error {
-		cli.Mnemonic = mnemonic
-
-		return cli.MoveSecret(cli.cmd, "work:test/secret", "other:moved", false)
-	}
-
-	t.Run("source removed", func(t *testing.T) {
-		requireChecksAgainOnceLocked(t, move, func(fs afero.Fs) error {
-			return fs.RemoveAll(testStateDir + "/vaults.d/work/secrets.d/test%secret")
-		}, vault.ErrSecretNotFound)
-	})
-
-	t.Run("destination vault removed", func(t *testing.T) {
-		requireChecksAgainOnceLocked(t, move, func(fs afero.Fs) error {
-			return fs.RemoveAll(testStateDir + "/vaults.d/other")
-		}, vault.ErrVaultNotFound)
-	})
-
-	t.Run("destination vault created again from another mnemonic", func(t *testing.T) {
-		// Set by the move, which has finished once
-		// requireChecksAgainOnceLocked returns
-		var moveErr error
-
-		requireChecksAgainOnceLocked(t, func(cli *Instance) error {
-			moveErr = move(cli)
-
-			return moveErr
-		}, func(fs afero.Fs) error {
-			err := fs.RemoveAll(testStateDir + "/vaults.d/other")
-			if err != nil {
-				return err
-			}
-
-			_, err = vault.CreateVault(fs, testStateDir, "other", otherMnemonic, nil)
-
-			return err
-		}, errVaultCreatedAgain)
-
-		// secret unlocker add fails with the same error, so its message
-		// must fit a move too
-		require.EqualError(t, moveErr, "vault 'other' was removed and "+
-			"created again with another key while it was being unlocked; "+
-			"nothing was changed")
-	})
-}
-
-// TestCrossVaultMoveRefusesExistingSecretBeforeUnlocking checks that a move
-// between vaults onto a secret that exists, without --force, is refused
-// before either vault is unlocked, since unlocking can ask for a
-// passphrase. The vaults have no unlockers and the move is given no
-// mnemonic, so unlocking either vault fails.
-func TestCrossVaultMoveRefusesExistingSecretBeforeUnlocking(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewMemMapFs()
-	createCrossVaults(t, fs)
-
-	work := vault.NewVault(fs, testStateDir, "work")
-	work.Mnemonic = testMnemonicBuffer(t)
-
-	err := work.AddSecret("y",
-		memguard.NewBufferFromBytes([]byte("other")), false)
-	require.NoError(t, err)
-
-	instance := NewCLIInstanceWithStateDir(fs, testStateDir)
-
-	err = instance.MoveSecret(&cobra.Command{}, "default:x", "work:y", false)
-	require.ErrorIs(t, err, vault.ErrSecretExists)
-}

internal/cli/secrets.go

@@ -422,92 +422,26 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 	// Locked only now that stdin has been read: in `secret encrypt key |
 	// secret add name`, holding the lock while reading would leave each
 	// command waiting for the other.
-	return cli.storeSecret(vlt, secretName, valueBuffer, force)
-}
-
-// storeSecret adds the secret name with value to vlt, as vault.AddSecret
-// does, holding the state directory lock while it does. With force, adding
-// an existing secret replaces its current version, which needs the vault's
-// long-term key. Getting the key can ask for the passphrase, so storeSecret
-// gets it before it takes the lock, and no other command waits while the
-// user answers. If another command adds the secret while this one waits for
-// the lock, storeSecret lets go of the lock, gets the key, and takes the
-// lock again.
-func (cli *Instance) storeSecret(
-	vlt *vault.Vault, name string, value *memguard.LockedBuffer, force bool,
-) error {
-	err := vault.ValidateSecretName(name)
-	if err != nil {
-		return err
-	}
-
-	replacing, err := replacesVersion(vlt, name, force)
-	if err != nil {
-		return err
-	}
-
-	if replacing {
-		_, err = vlt.GetOrDeriveLongTermKey()
-		if err != nil {
-			return fmt.Errorf("failed to get long-term key: %w", err)
-		}
-	}
-
-	needsKey, err := cli.addWithoutAsking(vlt, name, value, force)
-	if !needsKey {
-		return err
-	}
-
-	_, err = vlt.GetOrDeriveLongTermKey()
-	if err != nil {
-		return fmt.Errorf("failed to get long-term key: %w", err)
-	}
-
-	// With the key in memory, this adds the secret
-	_, err = cli.addWithoutAsking(vlt, name, value, force)
-
-	return err
-}
-
-// addWithoutAsking takes the state directory lock and adds the secret name
-// with value to vlt, as vault.AddSecret does. If that would replace the
-// secret's current version while the vault's long-term key is not in
-// memory, getting the key could ask for the passphrase with the lock held:
-// addWithoutAsking then adds nothing and returns true.
-func (cli *Instance) addWithoutAsking(
-	vlt *vault.Vault, name string, value *memguard.LockedBuffer, force bool,
-) (bool, error) {
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
-		return false, err
+		return err
 	}
 	defer release()
 
-	replacing, err := replacesVersion(vlt, name, force)
+	// Add the secret to the vault
+	secret.Debug("Calling vault.AddSecret", "secret_name", secretName,
+		"value_length", valueBuffer.Size(), "force", force)
+
+	err = vlt.AddSecret(secretName, valueBuffer, force)
 	if err != nil {
-		return false, err
+		secret.Debug("vault.AddSecret failed", "error", err)
+
+		return err
 	}
 
-	if replacing && vlt.Locked() {
+	secret.Debug("vault.AddSecret completed successfully")
-		return true, nil
-	}
 
-	return false, vlt.AddSecret(name, value, force)
+	return nil
-}
-
-// replacesVersion reports whether adding the secret name to vlt replaces
-// the secret's current version: whether force is set and the secret has one.
-func replacesVersion(vlt *vault.Vault, name string, force bool) (bool, error) {
-	if !force {
-		return false, nil
-	}
-
-	exists, err := secret.NewSecret(vlt, name).Exists()
-	if err != nil {
-		return false, fmt.Errorf("failed to check if secret exists: %w", err)
-	}
-
-	return exists, nil
 }
 
 // GetSecret retrieves and prints the current version of a secret
@@ -773,7 +707,14 @@ func (cli *Instance) ImportSecret(
 
 	// Locked only now that the file has been read, as in AddSecret: the
 	// file may be a pipe written by another secret command.
-	err = cli.storeSecret(vlt, secretName, valueBuffer, force)
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	// Store the secret in the vault
+	err = vlt.AddSecret(secretName, valueBuffer, force)
 	if err != nil {
 		return err
 	}
@@ -875,12 +816,24 @@ func (cli *Instance) findSecretToRemove(
 	}, nil
 }
 
-// MoveSecret moves or renames a secret (within or across vaults). It
+// MoveSecret moves or renames a secret (within or across vaults), holding
-// leaves taking the state directory lock to moveSecretWithinVault and
+// the state directory lock.
-// moveSecretCrossVault, since a move between vaults unlocks both vaults
-// before it takes the lock.
 func (cli *Instance) MoveSecret(
 	cmd *cobra.Command, source, dest string, force bool,
+) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	return cli.moveSecret(cmd, source, dest, force)
+}
+
+// moveSecret does the work of MoveSecret. The caller holds the state
+// directory lock.
+func (cli *Instance) moveSecret(
+	cmd *cobra.Command, source, dest string, force bool,
 ) error {
 	// Parse source and destination
 	srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
@@ -980,9 +933,8 @@ func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
 	return vault.NewVault(cli.fs, cli.stateDir, name), nil
 }
 
-// moveSecretWithinVault renames a secret within the vault vlt, holding the
+// moveSecretWithinVault renames a secret within the vault vlt. Its caller,
-// state directory lock. Its caller, MoveSecret, has already checked both
+// MoveSecret, has already checked both secret names.
-// secret names.
 func (cli *Instance) moveSecretWithinVault(
 	cmd *cobra.Command, vlt *vault.Vault, source, dest string, force bool,
 ) error {
@@ -992,12 +944,6 @@ func (cli *Instance) moveSecretWithinVault(
 		return fmt.Errorf("secret '%s' %w", source, errMoveOntoItself)
 	}
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
-	if err != nil {
-		return err
-	}
-	defer release()
-
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
 		return err
@@ -1084,75 +1030,59 @@ func (cli *Instance) sameDirectory(dir, other string) (bool, error) {
 
 // moveSecretCrossVault handles moving between two different vaults. Its
 // caller, MoveSecret, has already checked both secret names and that both
-// vaults exist. The copy needs the source vault's long-term key and is
+// vaults exist.
-// encrypted to the destination vault's long-term public key. The
-// destination vault is unlocked to check that the copy will open with what
-// the user gave, and, once the state directory lock is held, that it still
-// has the key unlocked. Either unlock can ask for a passphrase, so both
-// happen before the lock is taken, and no other command waits while the
-// user answers.
 func (cli *Instance) moveSecretCrossVault(
 	cmd *cobra.Command,
 	srcVault *vault.Vault, srcSecretName string,
 	destVault *vault.Vault, destSecretName string,
 	force bool,
 ) error {
-	// Before unlocking, check that the source secret exists, that the
+	srcVaultDir, err := srcVault.GetDirectory()
-	// destination is not the source under another name, and, without
+	if err != nil {
-	// --force, that the destination secret does not exist
+		return fmt.Errorf("failed to get source vault directory: %w", err)
-	_, err := cli.checkCrossVaultMove(
+	}
-		srcVault, srcSecretName, destVault, destSecretName, force)
+
+	// Verify source secret exists
+	srcStorageName := strings.ReplaceAll(srcSecretName, "/", "%")
+	srcSecretDir := filepath.Join(srcVaultDir, "secrets.d", srcStorageName)
+
+	exists, err := afero.DirExists(cli.fs, srcSecretDir)
+	if err != nil || !exists {
+		return fmt.Errorf("secret '%s' %w in vault '%s'",
+			srcSecretName, vault.ErrSecretNotFound, srcVault.Name)
+	}
+
+	// The source is removed after the copy, so a destination that is the
+	// source under another name would be lost with it.
+	destVaultDir, err := destVault.GetDirectory()
+	if err != nil {
+		return fmt.Errorf("failed to get destination vault directory: %w", err)
+	}
+
+	destStorageName := strings.ReplaceAll(destSecretName, "/", "%")
+	destSecretDir := filepath.Join(destVaultDir, "secrets.d", destStorageName)
+
+	same, err := cli.sameDirectory(srcSecretDir, destSecretDir)
 	if err != nil {
 		return err
 	}
 
+	if same {
+		return fmt.Errorf("secret '%s:%s' %w: '%s:%s' is the same secret on "+
+			"this filesystem", srcVault.Name, srcSecretName, errMoveOntoItself,
+			destVault.Name, destSecretName)
+	}
+
+	// Copying needs the long-term keys of both vaults
 	srcVault.Mnemonic, srcVault.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 	destVault.Mnemonic, destVault.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 
 	// Unlock destination vault (will fail if neither mnemonic nor unlocker available)
-	destIdentity, err := destVault.GetOrDeriveLongTermKey()
+	_, err = destVault.GetOrDeriveLongTermKey()
 	if err != nil {
 		return fmt.Errorf("failed to unlock destination vault '%s': %w", destVault.Name, err)
 	}
 
-	_, err = srcVault.GetOrDeriveLongTermKey()
-	if err != nil {
-		return fmt.Errorf("failed to unlock source vault '%s': %w",
-			srcVault.Name, err)
-	}
-
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
-	if err != nil {
-		return err
-	}
-	defer release()
-
-	err = cli.checkLongTermKey(destVault, destIdentity)
-	if err != nil {
-		return err
-	}
-
-	return cli.copySecretAndRemoveSource(
-		cmd, srcVault, srcSecretName, destVault, destSecretName, force)
-}
-
-// copySecretAndRemoveSource copies the secret srcSecretName of srcVault,
-// with all its versions, to destSecretName in destVault, and removes the
-// source. moveSecretCrossVault calls it holding the state directory lock.
-// Another command may have changed either vault since moveSecretCrossVault
-// checked them, so it checks them again first.
-func (cli *Instance) copySecretAndRemoveSource(
-	cmd *cobra.Command,
-	srcVault *vault.Vault, srcSecretName string,
-	destVault *vault.Vault, destSecretName string,
-	force bool,
-) error {
-	srcSecretDir, err := cli.checkCrossVaultMove(
-		srcVault, srcSecretName, destVault, destSecretName, force)
-	if err != nil {
-		return err
-	}
-
 	// Count versions for user feedback
 	versions, _ := secret.ListVersions(cli.fs, srcSecretDir)
 	versionCount := len(versions)
@@ -1178,65 +1108,6 @@ func (cli *Instance) copySecretAndRemoveSource(
 	return nil
 }
 
-// checkCrossVaultMove checks that the secret srcSecretName exists in
-// srcVault, that destSecretName in destVault is not the source under
-// another name, and, unless force is set, that destSecretName does not
-// exist in destVault. It returns the source's directory.
-func (cli *Instance) checkCrossVaultMove(
-	srcVault *vault.Vault, srcSecretName string,
-	destVault *vault.Vault, destSecretName string,
-	force bool,
-) (string, error) {
-	srcVaultDir, err := srcVault.GetDirectory()
-	if err != nil {
-		return "", fmt.Errorf("failed to get source vault directory: %w", err)
-	}
-
-	// Verify source secret exists
-	srcStorageName := strings.ReplaceAll(srcSecretName, "/", "%")
-	srcSecretDir := filepath.Join(srcVaultDir, "secrets.d", srcStorageName)
-
-	exists, err := afero.DirExists(cli.fs, srcSecretDir)
-	if err != nil || !exists {
-		return "", fmt.Errorf("secret '%s' %w in vault '%s'",
-			srcSecretName, vault.ErrSecretNotFound, srcVault.Name)
-	}
-
-	// The source is removed after the copy, so a destination that is the
-	// source under another name would be lost with it.
-	destVaultDir, err := destVault.GetDirectory()
-	if err != nil {
-		return "", fmt.Errorf("failed to get destination vault directory: %w", err)
-	}
-
-	destStorageName := strings.ReplaceAll(destSecretName, "/", "%")
-	destSecretDir := filepath.Join(destVaultDir, "secrets.d", destStorageName)
-
-	same, err := cli.sameDirectory(srcSecretDir, destSecretDir)
-	if err != nil {
-		return "", err
-	}
-
-	if same {
-		return "", fmt.Errorf("secret '%s:%s' %w: '%s:%s' is the same secret "+
-			"on this filesystem", srcVault.Name, srcSecretName, errMoveOntoItself,
-			destVault.Name, destSecretName)
-	}
-
-	exists, err = afero.DirExists(cli.fs, destSecretDir)
-	if err != nil {
-		return "", fmt.Errorf("failed to check if destination secret exists: %w", err)
-	}
-
-	if exists && !force {
-		return "", fmt.Errorf(
-			"secret '%s' %w in vault '%s' (use --force to overwrite)",
-			destSecretName, vault.ErrSecretExists, destVault.Name)
-	}
-
-	return srcSecretDir, nil
-}
-
 // sourceRemovalError returns the error of a move between vaults that
 // copied all versionCount versions of the secret srcName to destName, and
 // then failed with removeErr to remove srcSecretDir, the directory of

internal/cli/unlockers.go

@@ -15,7 +15,6 @@ import (
 	"strings"
 	"time"
 
-	"filippo.io/age"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/secret/internal/secret"
@@ -40,10 +39,6 @@ var (
 	// composes "GPG key <id> is already added as an unlocker".
 	errGPGKeyAlreadyUnlocker = errors.New(
 		"is already added as an unlocker")
-	// errVaultCreatedAgain carries only the message tail; the caller
-	// composes "vault '<name>' was removed and created again ...".
-	errVaultCreatedAgain = errors.New("was removed and created again " +
-		"with another key while it was being unlocked; nothing was changed")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -409,6 +404,12 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
 
 // UnlockersAdd adds a new unlocker
 func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	switch unlockerType {
 	case unlockerTypePassphrase:
 		return cli.addPassphraseUnlocker(cmd)
@@ -422,64 +423,6 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error
 	}
 }
 
-// unlockThenLock gets the long-term key of vlt, the vault an unlocker is
-// added to, then takes the state directory lock and returns the function
-// that releases it. Getting the key can ask for a passphrase, so it comes
-// before the lock, as the question of askThenLock does, and no other
-// command waits while the user answers. Once the lock is held, it fails if
-// another command has removed the vault in the meantime, since writing the
-// unlocker would create the vault's directory again, holding nothing else.
-// It also fails if the vault was removed and created again with another
-// long-term key, which the unlocker would then hold instead of the new
-// vault's own.
-func (cli *Instance) unlockThenLock(vlt *vault.Vault) (func(), error) {
-	ltIdentity, err := vlt.GetOrDeriveLongTermKey()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get long-term key: %w", err)
-	}
-
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
-	if err != nil {
-		return nil, err
-	}
-
-	err = cli.checkLongTermKey(vlt, ltIdentity)
-	if err != nil {
-		release()
-
-		return nil, err
-	}
-
-	return release, nil
-}
-
-// checkLongTermKey checks, for the vault vlt unlocked as ltIdentity before
-// the state directory lock was taken, that vlt still exists and that its
-// long-term public key is still that of ltIdentity. Another command may have
-// removed the vault in the meantime, or removed it and created it again with
-// another long-term key. The caller holds the lock.
-func (cli *Instance) checkLongTermKey(
-	vlt *vault.Vault, ltIdentity *age.X25519Identity,
-) error {
-	_, err := cli.existingVault(vlt.GetName())
-	if err != nil {
-		return err
-	}
-
-	vaultDir, _ := vlt.GetDirectory()
-
-	ltPubKey, err := afero.ReadFile(cli.fs, filepath.Join(vaultDir, "pub.age"))
-	if err != nil {
-		return fmt.Errorf("failed to read long-term public key: %w", err)
-	}
-
-	if string(ltPubKey) != ltIdentity.Recipient().String() {
-		return fmt.Errorf("vault '%s' %w", vlt.GetName(), errVaultCreatedAgain)
-	}
-
-	return nil
-}
-
 // autoSelectUnlocker selects the newly created unlocker as current,
 // printing a warning if selection fails
 func autoSelectUnlocker(cmd *cobra.Command, vlt *vault.Vault, unlockerID string) {
@@ -499,6 +442,9 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
+	// For passphrase unlockers, we don't need the vault to be unlocked
+	// The CreatePassphraseUnlocker method will handle getting the
+	// long-term key
 	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 
 	// The new passphrase is asked for even when SB_UNLOCK_PASSPHRASE is
@@ -518,12 +464,6 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 		defer passphraseBuffer.Destroy()
 	}
 
-	release, err := cli.unlockThenLock(vlt)
-	if err != nil {
-		return err
-	}
-	defer release()
-
 	passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer)
 	if err != nil {
 		return err
@@ -562,13 +502,12 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		cmd.Printf("Using default GPG key: %s\n", gpgKeyID)
 	}
 
+	// Check if this key is already added as an unlocker
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
-
 	// Resolve the GPG key ID to its fingerprint, once: the duplicate check
 	// and the new unlocker's metadata both use this result
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
@@ -576,26 +515,21 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
 	}
 
-	// Refuse a key that is already an unlocker before asking for anything,
+	// Check if this GPG key is already added
-	// and again once the lock is held, since another command may have added
+	exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
-	// it in the meantime
-	err = cli.checkPGPKeyNotAdded(vlt, gpgKeyID, fingerprint)
 	if err != nil {
-		return err
+		return fmt.Errorf(
+			"could not check whether GPG key %s is already an unlocker: %w",
+			gpgKeyID, err,
+		)
 	}
 
-	release, err := cli.unlockThenLock(vlt)
+	if exists {
-	if err != nil {
+		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
-		return err
-	}
-	defer release()
-
-	err = cli.checkPGPKeyNotAdded(vlt, gpgKeyID, fingerprint)
-	if err != nil {
-		return err
 	}
 
-	pgpUnlocker, err := secret.CreatePGPUnlocker(vlt, gpgKeyID, fingerprint)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir,
+		gpgKeyID, fingerprint, cli.Mnemonic, cli.UnlockPassphrase)
 	if err != nil {
 		return err
 	}
@@ -770,27 +704,6 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
-// checkPGPKeyNotAdded fails when vlt already has a PGP unlocker for the GPG
-// key gpgKeyID, whose fingerprint is fingerprint, or when that cannot be
-// checked.
-func (cli *Instance) checkPGPKeyNotAdded(
-	vlt *vault.Vault, gpgKeyID, fingerprint string,
-) error {
-	exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
-	if err != nil {
-		return fmt.Errorf(
-			"could not check whether GPG key %s is already an unlocker: %w",
-			gpgKeyID, err,
-		)
-	}
-
-	if exists {
-		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
-	}
-
-	return nil
-}
-
 // pgpUnlockerExists reports whether the vault already has a PGP unlocker
 // for the GPG key with the given fingerprint. It returns an error, and no
 // answer, when unlockers.d or an unlocker's metadata file cannot be read;

internal/cli/unreadable_dir_test.go

@@ -9,12 +9,11 @@
 // "nothing there" and go ahead.
 //
 // The tests make the look fail with a wrapper around the in-memory
-// filesystem, which the state directory lock refuses. So they call a
+// filesystem, which the state directory lock refuses. So they call the
-// function that makes the look before any lock is taken: addPGPUnlocker for
+// function each command runs once it holds the lock, such as addPGPUnlocker
-// UnlockersAdd, which looks again once it holds the lock, or, for a removal,
+// for UnlockersAdd, or, for a removal, the function that makes its checks,
-// the function that makes its checks, such as findVaultToRemove for
+// such as findVaultToRemove for RemoveVault, which runs again under the
-// RemoveVault, which runs again under the lock before anything is removed,
+// lock before anything is removed, with --force or without.
-// with --force or without.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli

internal/cli/vault.go

@@ -222,14 +222,18 @@ func (cli *Instance) resolvePassphrase() (*memguard.LockedBuffer, func(), error)
 	return passphraseBuffer, passphraseBuffer.Destroy, nil
 }
 
-// CreateVault creates a new vault. It asks for the mnemonic and the
+// CreateVault creates a new vault
-// passphrase before it takes the state directory lock, so that no other
-// command waits while the user answers.
 func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	secret.Debug("Creating new vault", "name", name, "state_dir", cli.stateDir)
 
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Refuse an invalid name or an existing vault before asking for anything
-	err := vault.CheckNewVaultName(cli.fs, cli.stateDir, name)
+	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, name)
 	if err != nil {
 		return err
 	}
@@ -261,14 +265,7 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	}
 	defer cleanupPassphrase()
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	// Create the vault with its passphrase unlocker
-	if err != nil {
-		return err
-	}
-	defer release()
-
-	// Create the vault with its passphrase unlocker. Another command may
-	// have created it since the check above: vault.CreateVault checks again.
 	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, name,
 		mnemonic, passphraseBuffer)
 	if err != nil {

internal/secret/pgpunlock_test.go

@@ -375,7 +375,8 @@ func testCreatePGPUnlocker(
 	}
 
 	// Now create a PGP unlock key (this will use our custom GPGEncryptFunc)
-	pgpUnlocker, err := secret.CreatePGPUnlocker(vlt, keyID, fingerprint)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(
+		fs, stateDir, keyID, fingerprint, mnemonic, nil)
 	if err != nil {
 		t.Fatalf("Failed to create PGP unlock key: %v", err)
 	}

internal/secret/pgpunlocker.go

@@ -192,45 +192,58 @@ func generatePGPUnlockerName() (string, error) {
 	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentTime), nil
 }
 
-// pgpUnlockerDir returns the directory in vault for a new PGP unlocker,
+// pgpUnlockerDir returns the current vault and the directory in it for a
-// named after the host and the time.
+// new PGP unlocker, named after the host and the time.
-func pgpUnlockerDir(vault VaultInterface) (string, error) {
+//
+//nolint:ireturn // the vault is only available behind VaultInterface
+func pgpUnlockerDir(
+	fs afero.Fs, stateDir string,
+) (VaultInterface, string, error) {
+	// Get current vault
+	vault, err := GetCurrentVault(fs, stateDir)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to get current vault: %w", err)
+	}
+
 	// Generate the unlocker name based on hostname and time
 	unlockerName, err := generatePGPUnlockerName()
 	if err != nil {
-		return "", fmt.Errorf("failed to generate unlocker name: %w", err)
+		return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)
 	}
 
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
-		return "", fmt.Errorf("failed to get vault directory: %w", err)
+		return nil, "", fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
-	return filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
+	return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
 }
 
-// CreatePGPUnlocker creates a new PGP unlocker and stores it in vault.
+// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault.
 // It encrypts to the GPG key gpgKeyID and records fingerprint, that key's
 // fingerprint as ResolveGPGKeyFingerprint returns it, in the metadata.
 // Everything that can fail short of writing a file is done before anything
 // is written, and the files are written through WriteDir, so a failure
-// leaves no partial unlocker. The long-term key comes from vault's
+// leaves no partial unlocker. The long-term key comes from mnemonic when it
-// GetOrDeriveLongTermKey: the key it holds when it is unlocked, else the
+// is not nil, else from the current unlocker, which is given passphrase when
-// key from its mnemonic or its current unlocker.
+// it is a passphrase unlocker.
 func CreatePGPUnlocker(
-	vault VaultInterface, gpgKeyID, fingerprint string,
+	fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
+	mnemonic, passphrase *memguard.LockedBuffer,
 ) (*PGPUnlocker, error) {
 	err := checkGPGAvailable()
 	if err != nil {
 		return nil, err
 	}
 
-	unlockerDir, err := pgpUnlockerDir(vault)
+	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err
 	}
 
-	fs := vault.GetFilesystem()
+	// The vault's GetOrDeriveLongTermKey, in step 2, uses both
+	vault.SetMnemonic(mnemonic)
+	vault.SetUnlockPassphrase(passphrase)
 
 	// Step 1: Generate a new age keypair for the PGP unlocker
 	ageIdentity, err := age.GenerateX25519Identity()
@@ -282,7 +295,7 @@ func CreatePGPUnlocker(
 func encryptPGPUnlockerKeys(
 	vault VaultInterface, ageIdentity *age.X25519Identity, gpgKeyID string,
 ) ([]byte, []byte, error) {
-	// The key vault holds, or else from its mnemonic or current unlocker
+	// From the mnemonic or the current unlocker, as for a passphrase unlocker
 	ltIdentity, err := vault.GetOrDeriveLongTermKey()
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to get long-term key: %w", err)

internal/secret/pgpunlocker_test.go

@@ -58,8 +58,7 @@ func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
 	}}
 
 	_, err = secret.CreatePGPUnlocker(
-		vault.NewVault(fs, testVaultStateDir, testVaultName),
+		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint, nil, nil)
-		testGPGKeyID, testGPGFingerprint)
 	require.Error(t, err)
 
 	vaultDir, err := vlt.GetDirectory()
@@ -88,15 +87,17 @@ func TestPGPUnlockerAddedTwiceKeepsFirst(t *testing.T) {
 
 	fs := afero.NewMemMapFs()
 	mnemonic := testMnemonicBuffer(t)
-	vlt, err := vault.CreateVault(fs, testVaultStateDir, testVaultName, mnemonic, nil)
+	_, err := vault.CreateVault(fs, testVaultStateDir, testVaultName, mnemonic, nil)
 	require.NoError(t, err)
 
-	first, err := secret.CreatePGPUnlocker(vlt, testGPGKeyID, testGPGFingerprint)
+	first, err := secret.CreatePGPUnlocker(
+		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint, mnemonic, nil)
 	require.NoError(t, err)
 
 	firstFiles := dirFiles(t, fs, first.GetDirectory())
 
-	second, err := secret.CreatePGPUnlocker(vlt, testGPGKeyID, testGPGFingerprint)
+	second, err := secret.CreatePGPUnlocker(
+		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint, mnemonic, nil)
 	require.NoError(t, err)
 	assert.NotEqual(t, first.GetDirectory(), second.GetDirectory())
 	assert.Equal(t, firstFiles, dirFiles(t, fs, first.GetDirectory()))

internal/secret/secret.go

@@ -28,6 +28,11 @@ type VaultInterface interface {
 	GetFilesystem() afero.Fs
 	GetCurrentUnlocker() (Unlocker, error)
 	GetOrDeriveLongTermKey() (*age.X25519Identity, error)
+	// SetMnemonic and SetUnlockPassphrase give GetOrDeriveLongTermKey the
+	// mnemonic to derive the long-term key from, and the passphrase for a
+	// current passphrase unlocker; nil for none.
+	SetMnemonic(mnemonic *memguard.LockedBuffer)
+	SetUnlockPassphrase(passphrase *memguard.LockedBuffer)
 	CreatePassphraseUnlocker(
 		passphrase *memguard.LockedBuffer) (*PassphraseUnlocker, error)
 }

internal/secret/secret_test.go

@@ -118,6 +118,12 @@ func (m *MockVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
 	return nil, errNotImplementedInMock
 }
 
+func (m *MockVault) SetMnemonic(mnemonic *memguard.LockedBuffer) {
+	m.mnemonic = mnemonic
+}
+
+func (m *MockVault) SetUnlockPassphrase(_ *memguard.LockedBuffer) {}
+
 func (m *MockVault) CreatePassphraseUnlocker(
 	_ *memguard.LockedBuffer,
 ) (*PassphraseUnlocker, error) {

internal/secret/version_test.go

@@ -90,6 +90,10 @@ func (m *MockVersionVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error)
 	return nil, errNotImplementedInMock
 }
 
+func (m *MockVersionVault) SetMnemonic(_ *memguard.LockedBuffer) {}
+
+func (m *MockVersionVault) SetUnlockPassphrase(_ *memguard.LockedBuffer) {}
+
 func (m *MockVersionVault) CreatePassphraseUnlocker(
 	_ *memguard.LockedBuffer,
 ) (*secret.PassphraseUnlocker, error) {

internal/vault/errors_test.go

@@ -132,7 +132,7 @@ func TestGetSecretWithWrongMnemonic(t *testing.T) {
 	t.Cleanup(mnemonic.Destroy)
 
 	vlt := vault.NewVault(created.GetFilesystem(), testStateDir, testVaultName)
-	vlt.Mnemonic = mnemonic
+	vlt.SetMnemonic(mnemonic)
 
 	_, err := vlt.GetSecret(testSecretName)
 	require.ErrorIs(t, err, vault.ErrMnemonicMismatch)

internal/vault/management.go

@@ -235,9 +235,9 @@ func processMnemonicForVault(
 // vault: one wrapping ErrInvalidVaultName when it is not a valid vault name,
 // and one wrapping ErrVaultExists when that vault already exists. Creating an
 // existing vault again would replace its keys, and its secrets could no
-// longer be decrypted. The commands that create a vault check it before they
+// longer be decrypted. CreateVault checks this before writing anything; the
-// ask for the mnemonic or passphrase, and CreateVault checks it again, under
+// commands that create a vault also check it before they ask for the
-// the state directory lock, before writing anything.
+// mnemonic or passphrase, holding the state directory lock.
 func CheckNewVaultName(fs afero.Fs, stateDir string, name string) error {
 	err := ValidateVaultName(name)
 	if err != nil {

internal/vault/secrets_version_test.go

@@ -336,7 +336,7 @@ func TestVaultVersionTimestampsAfterUnlock(t *testing.T) {
 
 	// A new instance is locked; AddSecret unlocks it with the passphrase
 	vault := NewVault(fs, testStateDir, "test")
-	vault.UnlockPassphrase = passphrase
+	vault.SetUnlockPassphrase(passphrase)
 
 	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
 	require.False(t, fs.readAt.IsZero(), "AddSecret did not unlock the vault")

internal/vault/vault.go

@@ -64,6 +64,18 @@ func (v *Vault) ClearLongTermKey() {
 	v.longTermKey = nil
 }
 
+// SetMnemonic sets v.Mnemonic, for code that has v only as a
+// secret.VaultInterface.
+func (v *Vault) SetMnemonic(mnemonic *memguard.LockedBuffer) {
+	v.Mnemonic = mnemonic
+}
+
+// SetUnlockPassphrase sets v.UnlockPassphrase, for code that has v only as
+// a secret.VaultInterface.
+func (v *Vault) SetUnlockPassphrase(passphrase *memguard.LockedBuffer) {
+	v.UnlockPassphrase = passphrase
+}
+
 // GetOrDeriveLongTermKey gets the long-term key from memory or derives it
 // from available sources
 func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {

internal/vault/vault_error_test.go

@@ -128,7 +128,7 @@ func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
 	t.Cleanup(wrong.Destroy)
 
 	vlt := vault.NewVault(fs, testStateDir, testVaultName)
-	vlt.UnlockPassphrase = wrong
+	vlt.SetUnlockPassphrase(wrong)
 
 	secretDir := filepath.Join(testStateDir, "vaults.d", testVaultName,
 		"secrets.d", testSecretName)