Set the time zone in main and run Entry in its own test process (closes #192)

cli.Entry() set time.Local to UTC and ended with memguard.Purge(), and
TestEntryWipesBuffersAndReturnsExitCode ran it in the same process as
the other tests. A timer left by an earlier test could read time.Local
as it changed, which the race detector reported on some runs, and the
purge could destroy a buffer still in use, the likeliest cause of a
crash seen on another run.

main() in cmd/secret now sets time.Local before it calls cli.Entry();
the test that runs the built binary with TZ=America/New_York still
covers it. The Entry test runs cli.Entry() in a copy of the test binary
started with SECRET_TEST_RUN_ENTRY set, which init picks up so that
TestMain does not run there.

Model: opus-5-5

README.md

@@ -440,10 +440,11 @@ 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`, and `secret add`,
+`secret init`, `secret vault create`, `secret unlocker add`, `secret mv` from
-`secret import` and `secret generate secret` with `--force`, ask for the
+one vault to another, and `secret add`, `secret import` and
-mnemonic or passphrase before they take the lock, so a prompt of theirs left
+`secret generate secret` with `--force`, ask for the mnemonic or passphrase
-open in one terminal holds up no command in another.
+before they take the lock, so a prompt of theirs left open in one terminal holds
+up no command in another.
 
 ## Security Features
 

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,183 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
+- 2026-10-15: `make test` no longer fails on some runs in `internal/cli`
+  (https://git.eeqj.de/sneak/secret/issues/192). `cli.Entry()` set `time.Local`
+  to UTC and ended by wiping every memguard buffer, and a test ran it in the
+  same process as the other tests. A timer left by an earlier test could read
+  `time.Local` as it changed, which the race detector reported, and the wipe
+  could destroy a buffer still in use, the likeliest cause of a crash of the
+  test process on another run. `main()` in `cmd/secret` now sets `time.Local`
+  before it calls `cli.Entry()`, and that test runs `cli.Entry()` in a process
+  of its own.
+- 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). It unlocked them while holding
+  the lock, and unlocking can ask for a passphrase, so every other command that
+  changes anything waited at that prompt. `moveSecretCrossVault` now checks the
+  move, refusing an existing destination secret without `--force`, unlocks both
+  vaults, and only then takes the lock. Under the lock, it checks that the
+  destination vault still has the long-term key it unlocked, and
+  `copySecretAndRemoveSource` checks the move again before it copies. A move
+  within a vault takes the lock in `moveSecretWithinVault`, so `MoveSecret`
+  takes none itself. The error for a vault created again with another key, which
+  `secret unlocker add` shares, now ends "nothing was changed".
+- 2026-10-15: `secret init`, `secret vault create`, `secret unlocker add`, and
+  `secret add`, `secret import` and `secret generate secret` with `--force` ask
+  for the mnemonic or passphrase, or unlock the vault, before they take the
+  state directory lock (https://git.eeqj.de/sneak/secret/issues/152). They did
+  so while holding the lock, so every other command that changes anything waited
+  at the prompt. Under the lock, `secret unlocker add` fails if the vault was
+  removed, or created again with another key, and refuses a GPG key added
+  meanwhile; a forced add of a secret created meanwhile lets go of the lock,
+  gets the key and takes the lock again. `secret.CreatePGPUnlocker` takes the
+  unlocked vault; `SetMnemonic` and `SetUnlockPassphrase` are gone.
+- 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, which 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-14: `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
+  `secret vault create` with an invalid name also fails before the prompts.
+  `secret init` prints the "Initialized" line only once the vault is created.
+- 2026-10-14: `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-14: `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-14: `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

cmd/secret/main.go

@@ -3,10 +3,16 @@ package main
 
 import (
 	"os"
+	"time"
 
 	"sneak.berlin/go/secret/internal/cli"
 )
 
 func main() {
+	// Every time secret prints or writes is in UTC, whatever TZ or the
+	// system's time zone is set to. It is set here, before cli.Entry starts
+	// any goroutine that could read time.Local while it changes.
+	time.Local = time.UTC //nolint:gosmopolitan // this makes local time UTC
+
 	os.Exit(cli.Entry())
 }

internal/cli/entry_test.go

@@ -3,6 +3,7 @@ package cli_test
 import (
 	"bufio"
 	"context"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -16,29 +17,67 @@ import (
 	"sneak.berlin/go/secret/internal/secret"
 )
 
-// Entry must return its exit code rather than exit, so that its deferred
+const (
-// memguard purge runs on the success and the error path alike.
+	// envRunEntry, when set, makes this test binary run cli.Entry with its
-//
+	// own arguments and exit, instead of running the tests.
-//nolint:paralleltest // sets os.Args, and Entry wipes every buffer in the process
+	envRunEntry = "SECRET_TEST_RUN_ENTRY"
-func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) {
-	savedArgs := os.Args
 
-	t.Cleanup(func() { os.Args = savedArgs })
+	// bufferWiped is what the test binary prints, run with envRunEntry set,
+	// when cli.Entry wiped the buffer made before it ran.
+	bufferWiped = "Entry wiped the buffer made before it ran"
+)
+
+// With envRunEntry set, the test binary makes a buffer, runs cli.Entry,
+// says whether the buffer is wiped, and exits with Entry's exit code. This
+// is done in init, not in a test, so that TestMain does not run in that
+// process: it would build the secret binary again, and remove it at the end,
+// while the other tests run it.
+//
+//nolint:gochecknoinits // must run before TestMain, as above
+func init() {
+	if os.Getenv(envRunEntry) == "" {
+		return
+	}
+
+	buf := memguard.NewBufferFromBytes([]byte("key material"))
+	code := cli.Entry()
+
+	if !buf.IsAlive() {
+		fmt.Fprintln(os.Stderr, bufferWiped)
+	}
+
+	os.Exit(code)
+}
+
+// Entry must return its exit code rather than exit, so that its deferred
+// memguard purge runs on the success and the error path alike. Entry wipes
+// every memguard buffer in the process, including those other tests are
+// using, so it runs in a process of its own: this test binary, run again
+// with envRunEntry set.
+func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) {
+	t.Parallel()
 
 	tests := []struct {
 		args     []string
 		exitCode int
 	}{
-		{args: []string{"secret", "--help"}, exitCode: 0},
+		{args: []string{"--help"}, exitCode: 0},
-		{args: []string{"secret", "no-such-command"}, exitCode: 1},
+		{args: []string{"no-such-command"}, exitCode: 1},
 	}
 
 	for _, tt := range tests {
-		buf := memguard.NewBufferFromBytes([]byte("key material"))
+		//nolint:gosec // G204: test runs its own test binary
-		os.Args = tt.args
+		cmd := exec.CommandContext(t.Context(), os.Args[0], tt.args...)
 
-		assert.Equal(t, tt.exitCode, cli.Entry(), "exit code for %v", tt.args)
+		cmd.Env = append(os.Environ(), envRunEntry+"=1")
-		assert.False(t, buf.IsAlive(), "Entry left a buffer unwiped for %v", tt.args)
+
+		output, err := cmd.CombinedOutput()
+		require.NotNil(t, cmd.ProcessState, "%v did not run: %v", tt.args, err)
+
+		assert.Equal(t, tt.exitCode, cmd.ProcessState.ExitCode(),
+			"exit code for %v: %s", tt.args, output)
+		assert.Contains(t, string(output), bufferWiped,
+			"Entry left a buffer unwiped for %v", tt.args)
 	}
 }
 

internal/cli/move_cross_vault_test.go

@@ -5,11 +5,13 @@ 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"
 )
 
@@ -92,7 +94,8 @@ 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 moveSecret, which MoveSecret runs once it holds the lock.
+// calls copySecretAndRemoveSource, which a move between vaults runs once it
+// holds the lock.
 func TestCrossVaultMoveFailsWhenSourceRemains(t *testing.T) {
 	t.Parallel()
 
@@ -103,9 +106,12 @@ func TestCrossVaultMoveFailsWhenSourceRemains(t *testing.T) {
 		Fs:   base,
 		path: testStateDir + "/vaults.d/default/secrets.d/x",
 	}, testStateDir)
-	instance.Mnemonic = testMnemonicBuffer(t)
 
-	err := instance.moveSecret(&cobra.Command{}, "default:x", "work:y", false)
+	source := vault.NewVault(instance.fs, testStateDir, "default")
+	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")
@@ -128,9 +134,12 @@ func TestCrossVaultMoveFailsWhenOldFilesRemain(t *testing.T) {
 	fs := &removeAllFailFs{Fs: base, dir: sourceVaultDir}
 
 	instance := NewCLIInstanceWithStateDir(fs, testStateDir)
-	instance.Mnemonic = testMnemonicBuffer(t)
 
-	err := instance.moveSecret(&cobra.Command{}, "default:x", "work:y", false)
+	source := vault.NewVault(fs, testStateDir, "default")
+	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")
@@ -150,3 +159,157 @@ 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/root.go

@@ -2,7 +2,6 @@ package cli
 
 import (
 	"os"
-	"time"
 
 	"github.com/awnumar/memguard"
 	"github.com/spf13/cobra"
@@ -15,10 +14,6 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
-	// Every time secret prints or writes is in UTC, whatever TZ or the
-	// system's time zone is set to.
-	time.Local = time.UTC //nolint:gosmopolitan // this makes local time UTC
-
 	raiseLockedMemoryLimit()
 
 	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer

internal/cli/secrets.go

@@ -875,24 +875,12 @@ func (cli *Instance) findSecretToRemove(
 	}, nil
 }
 
-// MoveSecret moves or renames a secret (within or across vaults), holding
+// MoveSecret moves or renames a secret (within or across vaults). It
-// the state directory lock.
+// leaves taking the state directory lock to moveSecretWithinVault and
+// 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)
@@ -992,8 +980,9 @@ 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. Its caller,
+// moveSecretWithinVault renames a secret within the vault vlt, holding the
-// MoveSecret, has already checked both secret names.
+// state directory lock. Its caller, MoveSecret, has already checked both
+// secret names.
 func (cli *Instance) moveSecretWithinVault(
 	cmd *cobra.Command, vlt *vault.Vault, source, dest string, force bool,
 ) error {
@@ -1003,6 +992,12 @@ 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
@@ -1089,59 +1084,75 @@ 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.
+// vaults exist. The copy needs the source vault's long-term key and is
+// 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 {
-	srcVaultDir, err := srcVault.GetDirectory()
+	// Before unlocking, check that the source secret exists, that the
-	if err != nil {
+	// destination is not the source under another name, and, without
-		return fmt.Errorf("failed to get source vault directory: %w", err)
+	// --force, that the destination secret does not exist
-	}
+	_, 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)
-	_, err = destVault.GetOrDeriveLongTermKey()
+	destIdentity, 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)
@@ -1167,6 +1178,65 @@ func (cli *Instance) moveSecretCrossVault(
 	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,6 +15,7 @@ import (
 	"strings"
 	"time"
 
+	"filippo.io/age"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/secret/internal/secret"
@@ -42,7 +43,7 @@ var (
 	// 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 added")
+		"with another key while it was being unlocked; nothing was changed")
 )
 
 // UnlockerInfo represents unlocker information for display
@@ -442,29 +443,41 @@ func (cli *Instance) unlockThenLock(vlt *vault.Vault) (func(), error) {
 		return nil, err
 	}
 
-	_, err = cli.existingVault(vlt.GetName())
+	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 {
-		release()
+		return fmt.Errorf("failed to read long-term public key: %w", err)
-
-		return nil, fmt.Errorf("failed to read long-term public key: %w", err)
 	}
 
 	if string(ltPubKey) != ltIdentity.Recipient().String() {
-		release()
+		return fmt.Errorf("vault '%s' %w", vlt.GetName(), errVaultCreatedAgain)
-
-		return nil, fmt.Errorf("vault '%s' %w", vlt.GetName(), errVaultCreatedAgain)
 	}
 
-	return release, nil
+	return nil
 }
 
 // autoSelectUnlocker selects the newly created unlocker as current,