Ask before removing a secret, version, vault or unlocker (closes #39)

secret rm, secret version rm, secret vault remove and secret unlocker
remove ask [y/N] on a terminal, naming what they remove, and go ahead
only on y or yes. Without --force, a command whose stdin is not a
terminal fails at once. --force, now also on rm and version rm,
removes without asking; it replaces the old refusals to remove a vault
with secrets or the last unlocker without --force. The checks run and
the question is asked before the state directory lock is taken; under
the lock the checks run again, and nothing is removed if they would
ask a different question.

Model: opus-5-5

README.md

@@ -220,9 +220,6 @@ Creates a new unlocker of the specified type:
 **Options:**
 - `--keyid <id>`: GPG key ID (optional for PGP type, uses default key if not specified)
 
-A vault has one passphrase unlocker: adding one replaces the one the vault
-has, which is removed only once the new one is the current unlocker.
-
 #### `secret unlocker remove <unlocker-id> [--force]` / `secret unlocker rm` ⚠️ 🛑
 
 **DANGER**: Permanently removes an unlocker. It first asks for confirmation,
@@ -272,8 +269,8 @@ Decrypts data using an Age key stored as a secret.
 ├── vaults.d/
 │   ├── default/
 │   │   ├── unlockers.d/
-│   │   │   ├── passphrase-<time>/       # Passphrase unlocker
+│   │   │   ├── passphrase/              # Passphrase unlocker
-│   │   │   └── <host>-pgp-<time>/       # PGP unlocker
+│   │   │   └── pgp/                     # PGP unlocker
 │   │   ├── secrets.d/
 │   │   │   ├── api%key/                 # Secret: api/key
 │   │   │   │   ├── versions/
@@ -289,7 +286,7 @@ Decrypts data using an Age key stored as a secret.
 │   │   │       └── current -> versions/20231215.001
 │   │   ├── vault-metadata.json          # Vault metadata
 │   │   ├── pub.age                      # Long-term public key
-│   │   └── current-unlocker             # Current unlocker's directory name
+│   │   └── current-unlocker -> ../unlockers.d/passphrase
 │   └── work/
 │       ├── unlockers.d/
 │       ├── secrets.d/

TODO.md

@@ -39,20 +39,6 @@ Bring the repo into policy compliance in one commit:
   question is asked, before the state directory lock is taken; under the
   lock the checks run again, and if they would ask a different question,
   nothing is removed. `secret rm` fails when it cannot count the versions.
-- 2026-10-04: A crash while an unlocker is being replaced no longer leaves a
-  current unlocker that cannot open the vault
-  (https://git.eeqj.de/sneak/secret/issues/71). Every new unlocker gets a
-  directory of its own, named with the time to the nanosecond:
-  `passphrase-<time>`, `<host>-pgp-<time>`, and for a keychain or Secure
-  Enclave unlocker the keychain item or Secure Enclave key, which names the
-  directory, carries the time instead of the day. `secret.WriteDir` fails on a
-  directory that exists instead of writing into it. `unlocker add passphrase`
-  writes the new unlocker, makes it current, and only then removes the vault's
-  other passphrase unlockers; a crash between the last two steps leaves the old
-  one beside the new, and the old passphrase still opens the vault through it
-  until the next `unlocker add passphrase` or an `unlocker remove` removes it.
-  A PGP, keychain or Secure Enclave unlocker added on the same host and day as
-  another of its type is added beside it instead of replacing it.
 - 2026-10-04: `SB_SECRET_MNEMONIC` and `SB_UNLOCK_PASSPHRASE` are read once
   per command, in its `RunE`, into locked buffers on the CLI `Instance`, and
   unset at once, so that no program the command runs, `gpg` included,
@@ -107,7 +93,9 @@ Bring the repo into policy compliance in one commit:
   and encrypt everything before writing anything. All four unlocker
   types write their files through `secret.WriteDir`: a new unlocker is
   built in a temporary directory, renamed into place when complete and
-  removed on a failure.
+  removed on a failure. One added under the directory name of an
+  existing unlocker is still written into that directory in place
+  (https://git.eeqj.de/sneak/secret/issues/71).
 - 2026-10-04: `secret unlocker select` and `secret unlocker remove`
   skip, with the warning `unlocker list` gives, an unlocker directory
   whose metadata file cannot be checked for, read or parsed, instead of
@@ -203,6 +191,12 @@ Bring the repo into policy compliance in one commit:
   into place, and removals rename out of the way first, so a version
   or secret is never half-added and never half-removed. An
   interrupted command can still leave:
+    - a broken unlocker, when it was replacing one: an unlocker added
+      under the directory name of an existing one is rewritten file by
+      file. That happens to a passphrase unlocker added to a vault that
+      has one, and to a PGP, keychain or Secure Enclave unlocker added
+      on the same host and day as another of its type
+      (https://git.eeqj.de/sneak/secret/issues/71);
     - from `init` or `vault create` killed after the passphrase prompt
       but before the unlocker is written, a vault with no unlocker,
       which `vault create` has already made the current vault;

go.mod

@@ -9,7 +9,6 @@ require (
 	github.com/btcsuite/btcd/btcec/v2 v2.1.3
 	github.com/btcsuite/btcd/btcutil v1.1.6
 	github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
-	github.com/creack/pty v1.1.24
 	github.com/keybase/go-keychain v0.0.0-20230307172405-3e4884637dd1
 	github.com/oklog/ulid/v2 v2.1.1
 	github.com/spf13/afero v1.14.0

go.sum

@@ -35,8 +35,6 @@ github.com/btcsuite/snappy-go v1.0.0/go.mod h1:8woku9dyThutzjeg+3xrA5iCpBRH8XEEg
 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
 github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
-github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
-github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

internal/cli/confirm_test.go

@@ -7,8 +7,8 @@
 // terminal fails at once, since nobody is there to answer.
 //
 // The tests answer through Instance.terminal, which stands in for a
-// terminal. Without it, whether stdin is a terminal decides; the tests in
+// terminal; without it, the command's input decides, and a test's input is
-// integration_test.go that run `secret rm` on a pseudo-terminal cover that.
+// never a terminal.
 
 //nolint:testpackage // sets the unexported terminal field of Instance
 package cli

internal/cli/integration_test.go

@@ -2,13 +2,10 @@
 package cli_test
 
 import (
-	"bufio"
-	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -22,7 +19,6 @@ import (
 	"git.eeqj.de/sneak/secret/internal/vault"
 	"git.eeqj.de/sneak/secret/pkg/agehd"
 	"github.com/awnumar/memguard"
-	"github.com/creack/pty"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -373,15 +369,8 @@ func test01Initialize(t *testing.T, tempDir, testMnemonic, testPassphrase string
 	unlockersDir := filepath.Join(defaultVaultDir, "unlockers.d")
 	verifyFileExists(t, unlockersDir)
 
-	// Check current-unlocker file names the unlocker's directory
-	currentUnlockerFile := filepath.Join(defaultVaultDir, "current-unlocker")
-	verifyFileExists(t, currentUnlockerFile)
-
-	currentUnlockerContent := readFile(t, currentUnlockerFile)
-	assert.Contains(t, string(currentUnlockerContent), "passphrase", "current unlocker should point to passphrase type")
-
 	// Verify passphrase unlocker was created
-	passphraseUnlockerDir := filepath.Join(unlockersDir, string(currentUnlockerContent))
+	passphraseUnlockerDir := filepath.Join(unlockersDir, "passphrase")
 	verifyFileExists(t, passphraseUnlockerDir)
 
 	// Check unlocker metadata
@@ -396,6 +385,13 @@ func test01Initialize(t *testing.T, tempDir, testMnemonic, testPassphrase string
 	encryptedLTPubKey := filepath.Join(passphraseUnlockerDir, "pub.age")
 	verifyFileExists(t, encryptedLTPubKey)
 
+	// Check current-unlocker file contains the relative path
+	currentUnlockerFile := filepath.Join(defaultVaultDir, "current-unlocker")
+	verifyFileExists(t, currentUnlockerFile)
+
+	currentUnlockerContent := readFile(t, currentUnlockerFile)
+	assert.Contains(t, string(currentUnlockerContent), "passphrase", "current unlocker should point to passphrase type")
+
 	// Verify vault-metadata.json in vault
 	vaultMetadata := filepath.Join(defaultVaultDir, "vault-metadata.json")
 	verifyFileExists(t, vaultMetadata)
@@ -544,8 +540,7 @@ func test04ImportMnemonic(t *testing.T, tempDir, testMnemonic, testPassphrase st
 	verifyFileExists(t, pubKeyFile)
 
 	// Verify passphrase unlocker was created
-	currentUnlocker := readFile(t, filepath.Join(workVaultDir, "current-unlocker"))
+	passphraseUnlockerDir := filepath.Join(workVaultDir, "unlockers.d", "passphrase")
-	passphraseUnlockerDir := filepath.Join(workVaultDir, "unlockers.d", string(currentUnlocker))
 	verifyFileExists(t, passphraseUnlockerDir)
 
 	// Check unlocker files
@@ -2551,12 +2546,12 @@ func copyFile(src, dst string) error {
 	return nil
 }
 
-// secretRmCommand makes a state directory whose vault "default" holds the
+// TestRemoveWithoutTerminalFailsAtOnce runs `secret rm` without --force,
-// secret "x", and returns `secret rm x` on the built binary against it, and
+// with a stdin that is not a terminal and never delivers anything, as in a
-// the directory of "x". The vault has no unlocker, so making it derives no
+// script or a CI job. It must fail at once, telling the user to pass
-// key from a passphrase.
+// --force, instead of waiting for an answer, and remove nothing.
-func secretRmCommand(ctx context.Context, t *testing.T) (*exec.Cmd, string) {
+func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
-	t.Helper()
+	t.Parallel()
 
 	stateDir := t.TempDir()
 
@@ -2571,24 +2566,6 @@ func secretRmCommand(ctx context.Context, t *testing.T) (*exec.Cmd, string) {
 
 	require.NoError(t, vlt.AddSecret("x", value, false))
 
-	//nolint:gosec // G204: test executes the freshly built secret binary
-	cmd := exec.CommandContext(ctx, secretBinaryPath(t), "rm", "x")
-	cmd.Env = []string{
-		secret.EnvStateDir + "=" + stateDir,
-		"PATH=" + os.Getenv("PATH"),
-		"HOME=" + os.Getenv("HOME"),
-	}
-
-	return cmd, filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "x")
-}
-
-// TestRemoveWithoutTerminalFailsAtOnce runs `secret rm` without --force,
-// with a stdin that is not a terminal and never delivers anything, as in a
-// script or a CI job. It must fail at once, telling the user to pass
-// --force, instead of waiting for an answer, and remove nothing.
-func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
-	t.Parallel()
-
 	// Nobody writes to or closes the pipe, so reading it would block for good.
 	stdin, stdinWriter, err := os.Pipe()
 	require.NoError(t, err)
@@ -2601,7 +2578,13 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
 	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
 	defer cancel()
 
-	cmd, secretDir := secretRmCommand(ctx, t)
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), "rm", "x")
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
 	cmd.Stdin = stdin
 
 	output, err := cmd.CombinedOutput()
@@ -2609,86 +2592,6 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) {
 	require.NoError(t, ctx.Err(), "secret rm waited for an answer")
 	require.Error(t, err)
 	assert.Contains(t, string(output), "pass --force")
-	assert.DirExists(t, secretDir)
+	assert.DirExists(t,
-}
+		filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "x"))
-
-// The next two tests run `secret rm` with a terminal on stdin or on stdout
-// and stderr, not both: whether it asks must depend on stdin alone, where
-// the answer is read from. pty.Open returns the two ends of a new terminal:
-// tty is the end a program uses as its terminal, and ptmx the end the test
-// reads what the terminal shows from and types into.
-
-// TestRemoveIgnoresTerminalOnStdout runs `echo y | secret rm x` at a
-// terminal. stdin is a pipe, so nobody can answer there, and the command
-// must fail as in a script, removing nothing.
-func TestRemoveIgnoresTerminalOnStdout(t *testing.T) {
-	t.Parallel()
-
-	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
-	defer cancel()
-
-	cmd, secretDir := secretRmCommand(ctx, t)
-
-	ptmx, tty, err := pty.Open()
-	require.NoError(t, err)
-
-	defer func() { _ = ptmx.Close() }()
-
-	cmd.Stdin = strings.NewReader("y\n")
-	cmd.Stdout = tty
-	cmd.Stderr = tty
-
-	require.NoError(t, cmd.Start())
-
-	_ = tty.Close()
-
-	// The read ends once secret rm has exited and so closed the terminal.
-	shown, _ := io.ReadAll(ptmx)
-
-	require.Error(t, cmd.Wait())
-	assert.Contains(t, string(shown), "pass --force")
-	assert.DirExists(t, secretDir)
-}
-
-// TestRemoveAsksAtTerminalOnStdin runs `secret rm x | cat` at a terminal.
-// It must ask on the terminal, and remove the secret when y is typed there.
-func TestRemoveAsksAtTerminalOnStdin(t *testing.T) {
-	t.Parallel()
-
-	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
-	defer cancel()
-
-	cmd, secretDir := secretRmCommand(ctx, t)
-
-	ptmx, tty, err := pty.Open()
-	require.NoError(t, err)
-
-	defer func() { _ = ptmx.Close() }()
-
-	cmd.Stdin = tty
-	// Not a file, so exec.Cmd connects stdout through a pipe.
-	cmd.Stdout = io.Discard
-	cmd.Stderr = tty
-
-	require.NoError(t, cmd.Start())
-
-	_ = tty.Close()
-
-	var (
-		shown []byte
-		char  byte
-	)
-
-	terminal := bufio.NewReader(ptmx)
-	for !bytes.HasSuffix(shown, []byte("[y/N] ")) {
-		char, err = terminal.ReadByte()
-		require.NoError(t, err, "secret rm ended without asking: %s", shown)
-
-		shown = append(shown, char)
-	}
-
-	_, err = ptmx.WriteString("y\n")
-	require.NoError(t, err)
-	require.NoError(t, cmd.Wait())
-	assert.NoDirExists(t, secretDir)
 }

internal/cli/path_traversal_test.go

@@ -181,7 +181,7 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
 
 	vaultDir := testStateDir + "/vaults.d/default"
 	require.Contains(t, before, vaultDir+"/secrets.d/x/")
-	require.Contains(t, before, vaultDir+"/current-unlocker")
+	require.Contains(t, before, vaultDir+"/unlockers.d/passphrase/")
 	require.Equal(t, "default", before[testStateDir+"/currentvault"])
 
 	cmd := &cobra.Command{}

internal/cli/unlockers.go

@@ -603,8 +603,8 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 
 	cmd.Printf("Created passphrase unlocker: %s\n", passphraseUnlocker.GetID())
 
-	// CreatePassphraseUnlocker has already made it the current unlocker
+	// Auto-select the newly created unlocker
-	cmd.Printf("Automatically selected as current unlocker\n")
+	autoSelectUnlocker(cmd, vlt, passphraseUnlocker.GetID())
 
 	return nil
 }

internal/cli/unreadable_dir_test.go

@@ -2,9 +2,8 @@
 //
 // The checks that guard adding a PGP unlocker (is this key already an
 // unlocker?), removing the last unlocker and removing a vault (does the
-// vault hold secrets?), removing a secret (how many versions does it
+// vault hold secrets?), and importing a mnemonic (does the vault already
-// have?), and importing a mnemonic (does the vault already have a
+// have a long-term key?) each look at the vault on disk before acting.
-// long-term key?) each look at the vault on disk before acting.
 // When that look fails they must refuse to act, not read the failure as
 // "nothing there" and go ahead.
 //
@@ -347,31 +346,6 @@ func TestRemoveVaultAbortsWhenSecretsDirUnreadable(t *testing.T) {
 	}
 }
 
-// TestRemoveSecretAbortsWhenVersionsUnreadable asserts that a secret is
-// kept when its versions directory exists but cannot be listed, so that
-// the question cannot say how many versions would be removed.
-func TestRemoveSecretAbortsWhenVersionsUnreadable(t *testing.T) {
-	t.Parallel()
-
-	secretDir := filepath.Join(testVaultDir(listTestVaultName),
-		unreadableTestSecretsDirName, unreadableTestSecretName)
-	versionsDir := filepath.Join(secretDir, "versions")
-
-	base := newListTestVault(t, 1)
-	writeTestSecret(t, base, testVaultDir(listTestVaultName))
-	require.NoError(t, base.MkdirAll(versionsDir, listTestDirPerm))
-
-	instance, _ := newTestInstance(&openFailFs{Fs: base, path: versionsDir})
-
-	_, err := instance.findSecretToRemove(unreadableTestSecretName)
-
-	require.ErrorIs(t, err, errOpenFailed)
-
-	exists, err := afero.DirExists(base, secretDir)
-	require.NoError(t, err)
-	assert.True(t, exists, "the secret must not be removed")
-}
-
 // TestVaultImportAbortsWhenPubKeyUnreadable asserts that a mnemonic import
 // stops when whether the vault already has a long-term key cannot be
 // determined.

internal/secret/atomic.go

@@ -3,7 +3,6 @@ package secret
 import (
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 
 	"github.com/spf13/afero"
@@ -63,12 +62,13 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
-// WriteDir calls write to write the files of the new directory dir into a
+// WriteDir calls write to write the files of the directory dir. When dir does
-// temporary directory from TempDirFor, which is then renamed to dir, so that
+// not exist yet, write writes them into a temporary directory from TempDirFor,
-// neither a failure nor a crash leaves dir half-written; on a failure the
+// which is then renamed to dir, so that neither a failure nor a crash leaves
-// temporary directory is removed, and a failure to remove it is returned
+// dir half-written; on a failure the temporary directory is removed, and a
-// along with the first. A directory cannot be replaced in one rename, so if
+// failure to remove it is returned along with the first. A directory cannot be
-// dir already exists, WriteDir fails without calling write.
+// renamed over one that has files in it, so when dir already exists, write
+// writes into it in place; dir is then never removed.
 func WriteDir(fs afero.Fs, dir string, write func(dir string) error) error {
 	exists, err := afero.Exists(fs, dir)
 	if err != nil {
@@ -76,7 +76,7 @@ func WriteDir(fs afero.Fs, dir string, write func(dir string) error) error {
 	}
 
 	if exists {
-		return fmt.Errorf("failed to create %s: %w", dir, os.ErrExist)
+		return write(dir)
 	}
 
 	// Create the directory the finished one is renamed into

internal/secret/atomic_test.go

@@ -191,22 +191,6 @@ func dirNames(t *testing.T, fs afero.Fs, dir string) []string {
 	return names
 }
 
-// dirFiles returns the contents of the files in dir, by name.
-func dirFiles(t *testing.T, fs afero.Fs, dir string) map[string]string {
-	t.Helper()
-
-	files := map[string]string{}
-
-	for _, name := range dirNames(t, fs, dir) {
-		data, err := afero.ReadFile(fs, filepath.Join(dir, name))
-		require.NoError(t, err)
-
-		files[name] = string(data)
-	}
-
-	return files
-}
-
 // writeLongTermKey gives the test vault under stateDir a new long-term key
 // and returns it.
 func writeLongTermKey(
@@ -684,14 +668,14 @@ func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 			vaultDir, err := vlt.GetDirectory()
 			require.NoError(t, err)
 
-			// The vault has no unlocker yet, so any directory in here is
+			unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
-			// the new one
-			unlockersDir := filepath.Join(vaultDir, "unlockers.d")
 
 			fs := hookFs{Fs: base, before: func(string, string) error {
-				for _, name := range dirNames(t, base, unlockersDir) {
+				exists, err := afero.DirExists(base, unlockerDir)
-					assert.ElementsMatch(t, files,
+				require.NoError(t, err)
-						dirNames(t, base, filepath.Join(unlockersDir, name)),
+
+				if exists {
+					assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
 						"unlocker directory visible before it was complete")
 				}
 
@@ -704,130 +688,13 @@ func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 			hooked := vault.NewVault(fs, stateDir, testVaultName)
 			hooked.Mnemonic = vlt.Mnemonic
 
-			unlocker, err := hooked.CreatePassphraseUnlocker(passphrase)
+			_, err = hooked.CreatePassphraseUnlocker(passphrase)
 			require.NoError(t, err)
-			assert.ElementsMatch(t, files, dirNames(t, base, unlocker.GetDirectory()))
+			assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir))
 		})
 	}
 }
 
-// TestPassphraseUnlockerReplacementKeepsVaultOpen replaces the vault's
-// passphrase unlocker twice, each time with only the current unlocker to open
-// the vault. The first replacement fails right after making the new unlocker
-// current, so the old one is not removed. The second checks, before every
-// change it makes, that the vault opens with the passphrase through its
-// current unlocker, which is what a crash at that change would leave; once it
-// returns, the vault must have one passphrase unlocker left.
-func TestPassphraseUnlockerReplacementKeepsVaultOpen(t *testing.T) {
-	t.Parallel()
-
-	for _, tfs := range testFilesystems {
-		t.Run(tfs.name, func(t *testing.T) {
-			t.Parallel()
-
-			base, stateDir := tfs.open(t)
-			vlt, err := vault.CreateVault(base, stateDir, testVaultName,
-				testMnemonicBuffer(t))
-			require.NoError(t, err)
-
-			ltIdentity, err := vlt.GetOrDeriveLongTermKey()
-			require.NoError(t, err)
-
-			passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
-			defer passphrase.Destroy()
-
-			_, err = vlt.CreatePassphraseUnlocker(passphrase)
-			require.NoError(t, err)
-
-			vaultDir, err := vlt.GetDirectory()
-			require.NoError(t, err)
-
-			currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
-
-			// Every change after the switch to the new unlocker fails
-			switched := false
-			failAfterSwitch := hookFs{Fs: base, before: func(op, path string) error {
-				if switched {
-					return errInjected
-				}
-
-				switched = op == opRename && path == currentUnlockerPath
-
-				return nil
-			}}
-
-			replacing := vault.NewVault(failAfterSwitch, stateDir, testVaultName)
-			replacing.Unlock(ltIdentity)
-
-			_, err = replacing.CreatePassphraseUnlocker(passphrase)
-			require.ErrorIs(t, err, errInjected)
-
-			unlockers, err := vlt.ListUnlockers()
-			require.NoError(t, err)
-			assert.Len(t, unlockers, 2, "the old unlocker is left beside the new")
-
-			assertOpens := vaultOpensCheck(t, base, stateDir, ltIdentity, passphrase)
-			checked := hookFs{Fs: base, before: func(string, string) error {
-				assertOpens()
-
-				return nil
-			}}
-
-			replacing = vault.NewVault(checked, stateDir, testVaultName)
-			replacing.Unlock(ltIdentity)
-
-			_, err = replacing.CreatePassphraseUnlocker(passphrase)
-			require.NoError(t, err)
-			assertOpens()
-
-			unlockers, err = vlt.ListUnlockers()
-			require.NoError(t, err)
-			assert.Len(t, unlockers, 1)
-		})
-	}
-}
-
-// vaultOpensCheck returns a function that checks that the test vault under
-// stateDir opens through its current unlocker, with passphrase, to the
-// long-term key ltIdentity. Opening it takes a second, so an unlocker
-// directory it has opened through before is not opened again: it must hold
-// the same files as then.
-func vaultOpensCheck(
-	t *testing.T, fs afero.Fs, stateDir string, ltIdentity *age.X25519Identity,
-	passphrase *memguard.LockedBuffer,
-) func() {
-	t.Helper()
-
-	vaultDir := filepath.Join(stateDir, "vaults.d", testVaultName)
-
-	// The files of each unlocker directory the vault has opened through
-	opened := map[string]map[string]string{}
-
-	return func() {
-		t.Helper()
-
-		current, err := afero.ReadFile(fs, filepath.Join(vaultDir, "current-unlocker"))
-		require.NoError(t, err)
-
-		files := dirFiles(t, fs, filepath.Join(vaultDir, "unlockers.d", string(current)))
-
-		if before, ok := opened[string(current)]; ok {
-			assert.Equal(t, before, files, "unlocker changed since it opened the vault")
-
-			return
-		}
-
-		opener := vault.NewVault(fs, stateDir, testVaultName)
-		opener.UnlockPassphrase = passphrase
-
-		key, err := opener.UnlockVault()
-		require.NoError(t, err)
-		assert.Equal(t, ltIdentity.Recipient().String(), key.Recipient().String())
-
-		opened[string(current)] = files
-	}
-}
-
 // TestWriteDirFailureLeavesNothing makes writing a new directory fail after
 // a file has been written in it, and checks that neither the directory nor
 // its temporary directory is left behind; and, when the temporary directory
@@ -873,10 +740,10 @@ func TestWriteDirFailureLeavesNothing(t *testing.T) {
 	}
 }
 
-// TestWriteDirRefusesExistingDir checks that WriteDir fails, without calling
+// TestWriteDirKeepsExistingDir makes writing into a directory that already
-// write, when the directory already exists, and leaves the directory as it
+// exists fail, and checks that the directory, with what was in it, is still
-// was: it never writes into a directory in place.
+// there: WriteDir writes into it in place and never removes it.
-func TestWriteDirRefusesExistingDir(t *testing.T) {
+func TestWriteDirKeepsExistingDir(t *testing.T) {
 	t.Parallel()
 
 	for _, tfs := range testFilesystems {
@@ -884,17 +751,17 @@ func TestWriteDirRefusesExistingDir(t *testing.T) {
 			t.Parallel()
 
 			fs, dir := tfs.open(t)
-			target := filepath.Join(dir, "unlockers.d", "existing")
+			target := filepath.Join(dir, "unlockers.d", "passphrase")
 			require.NoError(t, fs.MkdirAll(target, secret.DirPerms))
 			require.NoError(t, secret.WriteFileAtomic(fs,
 				filepath.Join(target, unlockerMetadataFile), []byte("{}")))
 
-			err := secret.WriteDir(fs, target, func(string) error {
+			err := secret.WriteDir(fs, target, func(got string) error {
-				t.Error("write called for a directory that exists")
+				assert.Equal(t, target, got)
 
-				return nil
+				return errInjected
 			})
-			require.ErrorIs(t, err, os.ErrExist)
+			require.ErrorIs(t, err, errInjected)
 			assert.Equal(t, []string{unlockerMetadataFile}, dirNames(t, fs, target))
 		})
 	}

internal/secret/constants.go

@@ -16,12 +16,6 @@ const (
 	EnvUnlockPassphrase = "SB_UNLOCK_PASSPHRASE"
 	// EnvGPGKeyID is the environment variable for providing the GPG key ID
 	EnvGPGKeyID = "SB_GPG_KEY_ID"
-
-	// UnlockerTimeFormat is the layout of the time, in UTC, in the name of a
-	// new unlocker's directory, keychain item and Secure Enclave key. It runs
-	// to the nanosecond, so that every new unlocker, even one added right
-	// after another, gets a directory of its own.
-	UnlockerTimeFormat = "2006-01-02.15.04.05.000000000"
 )
 
 // File system permission constants

internal/secret/keychainunlocker.go

@@ -233,10 +233,10 @@ func generateKeychainUnlockerName(vaultName string) (string, error) {
 		return "", fmt.Errorf("failed to get hostname: %w", err)
 	}
 
-	// Format: secret-<vault>-<hostname>-<time>
+	// Format: secret-<vault>-<hostname>-<date>
-	enrollmentTime := time.Now().UTC().Format(UnlockerTimeFormat)
+	enrollmentDate := time.Now().Format("2006-01-02")
 
-	return fmt.Sprintf("secret-%s-%s-%s", vaultName, hostname, enrollmentTime), nil
+	return fmt.Sprintf("secret-%s-%s-%s", vaultName, hostname, enrollmentDate), nil
 }
 
 // getLongTermPrivateKey derives the long-term private key from mnemonic when

internal/secret/pgpunlocker.go

@@ -209,20 +209,21 @@ func (p *PGPUnlocker) GetGPGKeyID() (string, error) {
 }
 
 // generatePGPUnlockerName generates a unique name for the PGP unlocker
-// based on hostname and time
+// based on hostname and date
 func generatePGPUnlockerName() (string, error) {
 	hostname, err := os.Hostname()
 	if err != nil {
 		return "", fmt.Errorf("failed to get hostname: %w", err)
 	}
 
-	enrollmentTime := time.Now().UTC().Format(UnlockerTimeFormat)
+	// Format: hostname-pgp-YYYY-MM-DD
+	enrollmentDate := time.Now().Format("2006-01-02")
 
-	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentTime), nil
+	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
 }
 
 // pgpUnlockerDir returns the current vault and the directory in it for a
-// new PGP unlocker, named after the host and the time.
+// new PGP unlocker, named after the host and the day.
 //
 //nolint:ireturn // the vault is only available behind VaultInterface
 func pgpUnlockerDir(
@@ -234,7 +235,7 @@ func pgpUnlockerDir(
 		return nil, "", fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	// Generate the unlocker name based on hostname and time
+	// Generate the unlocker name based on hostname and date
 	unlockerName, err := generatePGPUnlockerName()
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)

internal/secret/pgpunlocker_test.go

@@ -7,7 +7,6 @@ import (
 
 	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
-	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -65,40 +64,3 @@ func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
 	require.NoError(t, err)
 	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
 }
-
-// TestPGPUnlockerAddedTwiceKeepsFirst adds two PGP unlockers one right after
-// the other, so on the same host and day, and checks that the second gets a
-// directory of its own and leaves the first one's files as they were.
-// CreatePGPUnlocker does not check whether the GPG key already has an
-// unlocker, so the test key serves for both.
-//
-//nolint:paralleltest // installFakeGPG uses t.Setenv
-func TestPGPUnlockerAddedTwiceKeepsFirst(t *testing.T) {
-	installFakeGPG(t)
-
-	original := secret.GPGEncryptFunc
-
-	t.Cleanup(func() { secret.GPGEncryptFunc = original })
-
-	// Stands in for gpg, which the test does not have: "encrypts" by copying
-	secret.GPGEncryptFunc = func(data *memguard.LockedBuffer, _ string) ([]byte, error) {
-		return []byte(data.String()), nil
-	}
-
-	fs := afero.NewMemMapFs()
-	mnemonic := testMnemonicBuffer(t)
-	_, err := vault.CreateVault(fs, testVaultStateDir, testVaultName, mnemonic)
-	require.NoError(t, err)
-
-	first, err := secret.CreatePGPUnlocker(
-		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint, mnemonic, nil)
-	require.NoError(t, err)
-
-	firstFiles := dirFiles(t, fs, first.GetDirectory())
-
-	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/seunlocker_darwin.go

@@ -193,14 +193,14 @@ func generateSEKeyLabel(vaultName string) (string, error) {
 		return "", fmt.Errorf("failed to get hostname: %w", err)
 	}
 
-	enrollmentTime := time.Now().UTC().Format(UnlockerTimeFormat)
+	enrollmentDate := time.Now().UTC().Format("2006-01-02")
 
 	return fmt.Sprintf(
 		"%s.%s-%s-%s",
 		seKeyLabelPrefix,
 		vaultName,
 		hostname,
-		enrollmentTime,
+		enrollmentDate,
 	), nil
 }
 

internal/vault/unlockers.go

@@ -2,10 +2,8 @@ package vault
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log/slog"
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -105,7 +103,7 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
 
 // resolveUnlockerDirectory reads the current-unlocker file to get the
 // unlocker directory path
-// The file contains just the name of the unlocker's directory in unlockers.d
+// The file contains just the unlocker name (e.g., "passphrase")
 func (v *Vault) resolveUnlockerDirectory(currentUnlockerPath string) (string, error) {
 	secret.Debug("Reading current-unlocker file", "path", currentUnlockerPath)
 
@@ -361,10 +359,7 @@ func (v *Vault) SelectUnlocker(unlockerID string) error {
 	return nil
 }
 
-// CreatePassphraseUnlocker creates a new passphrase-protected unlocker in a
+// CreatePassphraseUnlocker creates a new passphrase-protected unlocker
-// directory of its own, makes it the current unlocker, and only then removes
-// the vault's other passphrase unlockers: a vault keeps one. A crash at any
-// point leaves a complete current unlocker, the old one or the new.
 // The passphrase must be provided as a LockedBuffer for security
 func (v *Vault) CreatePassphraseUnlocker(
 	passphrase *memguard.LockedBuffer,
@@ -376,23 +371,13 @@ func (v *Vault) CreatePassphraseUnlocker(
 
 	// We need to get the long-term key (either from memory if unlocked, or
 	// derive it). Getting it before anything is written means failing to
-	// get it changes nothing.
+	// get it changes nothing, even when replacing the current unlocker.
 	ltIdentity, err := v.GetOrDeriveLongTermKey()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	unlockersDir := filepath.Join(vaultDir, "unlockers.d")
+	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase)
-
-	// The passphrase unlockers the new one replaces
-	oldDirs, err := v.passphraseUnlockerDirs(unlockersDir)
-	if err != nil {
-		return nil, err
-	}
-
-	createdAt := time.Now()
-	unlockerDir := filepath.Join(unlockersDir, unlockerTypePassphrase+"-"+
-		createdAt.UTC().Format(secret.UnlockerTimeFormat))
 
 	// Generate new age keypair for unlocker
 	unlockerIdentity, err := age.GenerateX25519Identity()
@@ -412,7 +397,7 @@ func (v *Vault) CreatePassphraseUnlocker(
 
 	metadata := UnlockerMetadata{
 		Type:      unlockerTypePassphrase,
-		CreatedAt: createdAt,
+		CreatedAt: time.Now(),
 		Flags:     []string{},
 	}
 
@@ -430,54 +415,16 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, err
 	}
 
-	// Select the new unlocker by its directory, not by its ID: an old
+	// Create the unlocker instance
-	// passphrase unlocker created in the same minute has the same ID.
+	unlocker := secret.NewPassphraseUnlocker(v.fs, unlockerDir, metadata)
-	currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
 
-	err = secret.WriteFileAtomic(v.fs, currentUnlockerPath,
+	// Select this unlocker as current
-		[]byte(filepath.Base(unlockerDir)))
+	err = v.SelectUnlocker(unlocker.GetID())
 	if err != nil {
 		return nil, fmt.Errorf("failed to select new unlocker: %w", err)
 	}
 
-	for _, oldDir := range oldDirs {
+	return unlocker, nil
-		err = secret.RemoveDirAtomic(v.fs, oldDir)
-		if err != nil {
-			return nil, fmt.Errorf(
-				"created and selected the new passphrase unlocker: %w", err)
-		}
-	}
-
-	return secret.NewPassphraseUnlocker(v.fs, unlockerDir, metadata), nil
-}
-
-// passphraseUnlockerDirs returns the directories in unlockersDir that hold
-// passphrase unlockers. A directory ListUnlockers skips is left out, with the
-// same warning.
-func (v *Vault) passphraseUnlockerDirs(unlockersDir string) ([]string, error) {
-	files, err := afero.ReadDir(v.fs, unlockersDir)
-	if errors.Is(err, os.ErrNotExist) {
-		return nil, nil
-	}
-
-	if err != nil {
-		return nil, fmt.Errorf("failed to read unlockers directory: %w", err)
-	}
-
-	var dirs []string
-
-	for _, file := range files {
-		if !file.IsDir() {
-			continue
-		}
-
-		metadata, ok := v.readUnlockerMetadataOrWarn(unlockersDir, file.Name())
-		if ok && metadata.Type == unlockerTypePassphrase {
-			dirs = append(dirs, filepath.Join(unlockersDir, file.Name()))
-		}
-	}
-
-	return dirs, nil
 }
 
 // readUnlockerMetadata reads and parses the unlocker-metadata.json file in