Leave no partial unlocker directory when adding an unlocker fails (closes #48)

CreatePGPUnlocker looked up the GPG key's fingerprint, and the keychain
unlocker got the long-term key, only after writing part of the unlocker,
so a failure there left a directory with no metadata. Both now do every
step that can fail before writing anything. `secret unlocker add pgp`
looks the fingerprint up once, for its duplicate check, and passes it to
CreatePGPUnlocker to record. All four unlocker types write their files
through the new secret.WriteDir, which builds a new directory in a
temporary directory, renames it into place when complete and removes it
on a failure. A directory that already exists, as when an unlocker
replaces one of the same name, is written in place and never removed.

Model: opus-5-5

TODO.md

@@ -25,6 +25,24 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-04: A failed unlocker add no longer leaves a partial unlocker
+  directory (https://git.eeqj.de/sneak/secret/issues/48).
+  `secret unlocker add pgp` resolves the GPG key's fingerprint once, for
+  its duplicate check, and passes it to `CreatePGPUnlocker` to record.
+  `CreatePGPUnlocker` and `CreateKeychainUnlocker` get the long-term key
+  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.
+  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 get` keeps the secret in locked memory until it
+  writes it out (https://git.eeqj.de/sneak/secret/issues/37):
+  `Vault.GetSecret` and `Vault.GetSecretVersion` return a
+  `*memguard.LockedBuffer`, which every caller destroys, and `secret get`
+  writes its bytes straight to stdout, still with no trailing newline.
+  Before, the value was copied into ordinary memory that nothing wiped,
+  and `get --version` also wrote it to the debug log.
 - 2026-10-04: The `Makefile` no longer sets `DOCKER_HOST`, so its docker
   targets use the local docker daemon, or whatever `DOCKER_HOST` the
   environment sets. `make build` calls the new `script/build`, which
@@ -103,13 +121,10 @@ Bring the repo into policy compliance in one commit:
     - 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;
-    - from an unlocker add stopped before its metadata is written, a
-      directory that `unlocker list` warns about and `unlocker rm`
-      cannot remove;
-    - data under a `.tmp-` name in the state directory: a secret or
-      version being added, or the secret, version, unlocker or vault
-      being removed, encrypted keys included. Nothing deletes it; it
-      must be deleted by hand
+    - data under a `.tmp-` name in the state directory: a secret,
+      version or unlocker being added, or the secret, version, unlocker
+      or vault being removed, encrypted keys included. Nothing deletes
+      it; it must be deleted by hand
       (https://git.eeqj.de/sneak/secret/issues/75).
 - 2026-10-03: The checks run before changing a vault now stop with an
   error naming the path and cause when they cannot read what they

internal/cli/cli.go

@@ -68,8 +68,3 @@ func (cli *Instance) SetStateDir(stateDir string) {
 func (cli *Instance) GetStateDir() string {
 	return cli.stateDir
 }
-
-// Print outputs to the command's configured output writer
-func (cli *Instance) Print(a ...any) (int, error) {
-	return fmt.Fprint(cli.cmd.OutOrStdout(), a...)
-}

internal/cli/create_vault_test.go

@@ -1,6 +1,7 @@
 package cli_test
 
 import (
+	"bytes"
 	"testing"
 
 	"git.eeqj.de/sneak/secret/internal/cli"
@@ -89,7 +90,11 @@ func TestCreateExistingVaultChangesNothing(t *testing.T) {
 	for _, name := range vaults {
 		value, err := vault.NewVault(fs, testStateDir, name).GetSecret("x")
 		require.NoError(t, err)
-		require.Equal(t, "value", string(value))
+
+		unchanged := bytes.Equal([]byte("value"), value.Bytes())
+		value.Destroy()
+
+		require.True(t, unchanged, "vault %q kept its secret", name)
 	}
 }
 

internal/cli/lock_test.go

@@ -147,7 +147,8 @@ func TestConcurrentAddsKeepEveryVersion(t *testing.T) {
 				value, err := vlt.GetSecretVersion("shared", version)
 				require.NoError(t, err)
 
-				values[string(value)] = true
+				values[string(value.Bytes())] = true
+				value.Destroy()
 			}
 
 			assert.Len(t, values, adds+1, "every add stored its own value")

internal/cli/move_test.go

@@ -178,7 +178,10 @@ func TestMoveOntoSameSecretUnderAnotherNameIsRejected(t *testing.T) {
 
 			value, err := vlt.GetSecret("x")
 			require.NoError(t, err)
-			require.Equal(t, "value", string(value))
+
+			defer value.Destroy()
+
+			require.Equal(t, []byte("value"), value.Bytes())
 
 			target, err := os.Readlink(link)
 			require.NoError(t, err)
@@ -222,7 +225,10 @@ func TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem(t *testing.T) {
 
 	value, err := vlt.GetSecret("foo")
 	require.NoError(t, err)
-	require.Equal(t, "upper", string(value))
+
+	defer value.Destroy()
+
+	require.Equal(t, []byte("upper"), value.Bytes())
 
 	_, err = vlt.GetSecret("Foo")
 	require.ErrorIs(t, err, vault.ErrSecretNotFound)

internal/cli/secrets.go

@@ -414,9 +414,6 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	secret.Debug("GetSecret called", "secretName", secretName)
 
-	// Store the command for output
-	cli.cmd = cmd
-
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -427,9 +424,10 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	if err != nil {
 		return err
 	}
+	defer value.Destroy()
 
-	// Print the secret value to stdout
-	_, _ = cli.Print(string(value))
+	// Write the value straight from locked memory, with no trailing newline
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	return nil
 }
@@ -442,9 +440,6 @@ func (cli *Instance) GetSecretWithVersion(
 	secret.Debug("GetSecretWithVersion called",
 		"secretName", secretName, "version", version)
 
-	// Store the command for output
-	cli.cmd = cmd
-
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -460,22 +455,15 @@ func (cli *Instance) GetSecretWithVersion(
 
 		return err
 	}
+	defer value.Destroy()
 
-	secret.Debug("Got secret value", "valueLength", len(value))
+	secret.Debug("Got secret value", "valueLength", value.Size())
 
-	// Print the secret value to stdout
-	_, _ = cli.Print(string(value))
+	// Write the value straight from locked memory, with no trailing newline
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	secret.Debug("Printed value to stdout")
 
-	// Debug: Log what we're actually printing
-	secret.Debug("Secret retrieval debug info",
-		"secretName", secretName,
-		"version", version,
-		"valueLength", len(value),
-		"valueAsString", string(value),
-		"isEmpty", len(value) == 0)
-
 	return nil
 }
 

internal/cli/secrets_size_test.go

@@ -143,7 +143,10 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, testData, retrievedValue,
+
+	defer retrievedValue.Destroy()
+
+	assert.Equal(t, testData, retrievedValue.Bytes(),
 		"Retrieved secret should match original (without newline)")
 }
 
@@ -193,7 +196,11 @@ func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
+
+	defer retrievedValue.Destroy()
+
+	assert.Equal(t, testData, retrievedValue.Bytes(),
+		"Retrieved secret should match original")
 }
 
 // TestAddSecretVariousSizes tests adding secrets of various sizes through stdin
@@ -375,7 +382,10 @@ func TestAddSecretBufferGrowth(t *testing.T) {
 			// Verify the secret was stored correctly
 			retrievedValue, err := vlt.GetSecret(secretName)
 			require.NoError(t, err)
-			assert.Equal(t, testData, retrievedValue,
+
+			defer retrievedValue.Destroy()
+
+			assert.Equal(t, testData, retrievedValue.Bytes(),
 				"Retrieved secret should match original exactly")
 		})
 	}
@@ -416,7 +426,11 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret("streaming-test")
 	require.NoError(t, err)
-	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
+
+	defer retrievedValue.Destroy()
+
+	assert.Equal(t, testData, retrievedValue.Bytes(),
+		"Retrieved secret should match original")
 }
 
 // slowReader simulates a reader that returns data in small chunks

internal/cli/unlockers.go

@@ -685,7 +685,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	// Resolve the GPG key ID to its fingerprint
+	// 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)
 	if err != nil {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -706,7 +707,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
 	}
 
-	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(
+		cli.fs, cli.stateDir, gpgKeyID, fingerprint)
 	if err != nil {
 		return err
 	}

internal/cli/unlockers_add_test.go

@@ -0,0 +1,35 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// unknownTestGPGUserID is a GPG user ID that no key in the test keyring has.
+const unknownTestGPGUserID = "not-in-keyring@example.com"
+
+// TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
+// the keyring does not hold fails at looking up the key's fingerprint and
+// leaves no new unlocker directory. The error must come from the lookup: a
+// lookup moved after anything is written would also come after getting the
+// vault's long-term key, which fails first on every platform but macOS
+// (https://git.eeqj.de/sneak/secret/issues/88).
+//
+//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
+func TestAddPGPUnlockerUnknownKey(t *testing.T) {
+	newTestGPGKey(t)
+
+	base := newListTestVault(t, 1)
+	instance, cmd := newTestInstance(base)
+	cmd.Flags().String("keyid", unknownTestGPGUserID, "")
+
+	err := instance.addPGPUnlocker(cmd)
+
+	require.ErrorContains(t, err, "failed to resolve GPG key fingerprint")
+	assertDirEntries(t, base,
+		filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
+		listTestUnlockerDirOne)
+}

internal/cli/version_test.go

@@ -7,6 +7,7 @@
 // - TestPromoteVersionCommand: Tests `secret version promote` command
 // - TestPromoteNonExistentVersion: Tests error handling for invalid promotion
 // - TestGetSecretWithVersion: Tests `secret get --version` flag functionality
+// - TestGetSecretWritesBinaryValue: Tests `secret get` output of binary values
 // - TestVersionCommandStructure: Tests command structure and help text
 // - TestListVersionsEmptyOutput: Tests edge case with no versions
 //
@@ -23,6 +24,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"unicode/utf8"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
@@ -188,7 +190,10 @@ func TestPromoteVersionCommand(t *testing.T) {
 	// Current should be version-2
 	value, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-2"), value.Bytes())
 
 	// Promote first version
 	firstVersion := versions[1] // Older version
@@ -211,9 +216,12 @@ func TestPromoteVersionCommand(t *testing.T) {
 	assert.Contains(t, outputStr, firstVersion)
 
 	// Verify current is now version-1
-	value, err = vlt.GetSecret("test/secret")
+	promoted, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1"), value)
+
+	defer promoted.Destroy()
+
+	assert.Equal(t, []byte("version-1"), promoted.Bytes())
 }
 
 //nolint:paralleltest // uses t.Setenv via setupTestVault
@@ -290,6 +298,46 @@ func TestGetSecretWithVersion(t *testing.T) {
 	assert.Equal(t, "version-1", buf.String())
 }
 
+//nolint:paralleltest // uses t.Setenv via setupTestVault
+func TestGetSecretWritesBinaryValue(t *testing.T) {
+	fs := afero.NewMemMapFs()
+	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
+
+	setupTestVault(t, fs)
+
+	vlt, err := vault.GetCurrentVault(fs, testStateDir)
+	require.NoError(t, err)
+
+	value := []byte{0x00, 'a', 0xff, 0xfe, 0x00, 0xc3, 0x28, 'z', 0x00}
+	require.False(t, utf8.Valid(value))
+	// A copy, since storing a value wipes the slice it came from
+	addTestSecret(t, vlt, bytes.Clone(value), false)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+
+	versions, err := secret.ListVersions(fs,
+		filepath.Join(vaultDir, "secrets.d", "test%secret"))
+	require.NoError(t, err)
+	require.Len(t, versions, 1)
+
+	cmd := newRootCmd()
+
+	var buf bytes.Buffer
+	cmd.SetOut(&buf)
+
+	// Each writes exactly the stored bytes, with no trailing newline
+	err = cli.GetSecret(cmd, "test/secret")
+	require.NoError(t, err)
+	assert.Equal(t, value, buf.Bytes())
+
+	buf.Reset()
+
+	err = cli.GetSecretWithVersion(cmd, "test/secret", versions[0])
+	require.NoError(t, err)
+	assert.Equal(t, value, buf.Bytes())
+}
+
 //nolint:paralleltest // reads process environment to determine the state dir
 func TestVersionCommandStructure(t *testing.T) {
 	// Test that version commands are properly structured

internal/secret/atomic.go

@@ -1,6 +1,7 @@
 package secret
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -61,6 +62,52 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
+// WriteDir calls write to write the files of the directory dir. When dir does
+// not exist yet, write writes them into a temporary directory from TempDirFor,
+// which is then renamed to dir, so that neither a failure nor a crash leaves
+// dir half-written; on a failure the temporary directory is removed, and a
+// failure to remove it is returned along with the first. A directory cannot be
+// 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 {
+		return fmt.Errorf("failed to check for %s: %w", dir, err)
+	}
+
+	if exists {
+		return write(dir)
+	}
+
+	// Create the directory the finished one is renamed into
+	err = fs.MkdirAll(filepath.Dir(dir), DirPerms)
+	if err != nil {
+		return fmt.Errorf("failed to create %s: %w", filepath.Dir(dir), err)
+	}
+
+	tmp, err := TempDirFor(fs, dir)
+	if err != nil {
+		return err
+	}
+
+	err = write(tmp)
+	if err == nil {
+		err = fs.Rename(tmp, dir)
+	}
+
+	if err != nil {
+		removeErr := fs.RemoveAll(tmp)
+		if removeErr != nil {
+			err = errors.Join(err,
+				fmt.Errorf("failed to remove %s: %w", tmp, removeErr))
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 // RemoveDirAtomic deletes the directory dir so that it disappears in one
 // rename: dir is moved into a new directory from TempDirFor, which is then
 // deleted. A crash part-way leaves only that temporary directory behind.

internal/secret/atomic_test.go

@@ -35,6 +35,10 @@ const currentFile = "current"
 // unlockerMetadataFile is the file a new unlocker writes last.
 const unlockerMetadataFile = "unlocker-metadata.json"
 
+// privKeyFile is the file that holds the encrypted private key of a version
+// or of a passphrase unlocker.
+const privKeyFile = "priv.age"
+
 // unlockerPassphrase protects the passphrase unlockers the tests create.
 //
 //nolint:gosec // G101: test data, not a real credential
@@ -342,7 +346,10 @@ func TestLongestNames(t *testing.T) {
 
 	got, err := vlt.GetSecret(name)
 	require.NoError(t, err)
-	assert.Equal(t, "long", string(got))
+
+	defer got.Destroy()
+
+	assert.Equal(t, []byte("long"), got.Bytes())
 
 	vaultDir, err := vlt.GetDirectory()
 	require.NoError(t, err)
@@ -380,7 +387,10 @@ func TestForcedCopyKeepsDestinationUntilReplaced(t *testing.T) {
 
 			value, err := dest.GetSecret("shared")
 			require.NoError(t, err)
-			assert.Equal(t, "old", string(value))
+
+			defer value.Destroy()
+
+			assert.Equal(t, []byte("old"), value.Bytes())
 		})
 	}
 }
@@ -447,7 +457,7 @@ func TestVersionSaveIsWholeOrAbsent(t *testing.T) {
 
 				if exists {
 					assert.ElementsMatch(t,
-						[]string{"pub.age", "value.age", "priv.age", "metadata.age"},
+						[]string{"pub.age", "value.age", privKeyFile, "metadata.age"},
 						dirNames(t, base, versionDir),
 						"version directory visible before it was complete")
 				}
@@ -490,7 +500,7 @@ func TestVersionSaveFailureLeavesNothing(t *testing.T) {
 			writeLongTermKey(t, base, stateDir)
 
 			fs := hookFs{Fs: base, before: func(op, path string) error {
-				if op == opRename && filepath.Base(path) == "priv.age" {
+				if op == opRename && filepath.Base(path) == privKeyFile {
 					return errInjected
 				}
 
@@ -636,15 +646,21 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 	require.Error(t, err)
 }
 
-// TestPassphraseUnlockerWritesMetadataLast checks that the last file a new
-// passphrase unlocker writes in its directory is its metadata: an unlocker
-// directory without metadata is never used, so one interrupted earlier
-// cannot be.
-func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
+// TestPassphraseUnlockerIsWholeOrAbsent checks, before every change that
+// creating a passphrase unlocker makes, that the unlocker's directory either
+// does not exist or holds all of its files: a crash or a failure at any point
+// leaves no partial unlocker.
+//
+//nolint:paralleltest // t.Setenv forbids t.Parallel
+func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	base := afero.NewMemMapFs()
-	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
+	files := []string{"pub.age", privKeyFile, "longterm.age", unlockerMetadataFile}
+
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			base, stateDir := tfs.open(t)
+			vlt, err := vault.CreateVault(base, stateDir, testVaultName)
 			require.NoError(t, err)
 
 			vaultDir, err := vlt.GetDirectory()
@@ -652,11 +668,13 @@ func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
 
 			unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
 
-	var last string
+			fs := hookFs{Fs: base, before: func(string, string) error {
+				exists, err := afero.DirExists(base, unlockerDir)
+				require.NoError(t, err)
 
-	fs := hookFs{Fs: base, before: func(_, path string) error {
-		if filepath.Dir(path) == unlockerDir {
-			last = filepath.Base(path)
+				if exists {
+					assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
+						"unlocker directory visible before it was complete")
 				}
 
 				return nil
@@ -665,8 +683,82 @@ func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
 			passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
 			defer passphrase.Destroy()
 
-	_, err = vault.NewVault(fs, testVaultStateDir, testVaultName).
+			_, err = vault.NewVault(fs, stateDir, testVaultName).
 				CreatePassphraseUnlocker(passphrase)
 			require.NoError(t, err)
-	assert.Equal(t, unlockerMetadataFile, last)
+			assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir))
+		})
+	}
+}
+
+// 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
+// cannot be removed either, that both failures are reported.
+func TestWriteDirFailureLeavesNothing(t *testing.T) {
+	t.Parallel()
+
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			t.Parallel()
+
+			base, dir := tfs.open(t)
+			listed := filepath.Join(dir, "unlockers.d")
+			target := filepath.Join(listed, "new")
+
+			writeThenFail := func(tmp string) error {
+				require.NoError(t, secret.WriteFileAtomic(base,
+					filepath.Join(tmp, unlockerMetadataFile), []byte("{}")))
+
+				return errInjected
+			}
+
+			err := secret.WriteDir(base, target, writeThenFail)
+			require.ErrorIs(t, err, errInjected)
+
+			// Nothing in the directory that is listed, nor beside it
+			assert.Empty(t, dirNames(t, base, listed))
+			assert.Equal(t, []string{"unlockers.d"}, dirNames(t, base, dir))
+
+			fs := hookFs{Fs: base, before: func(op, _ string) error {
+				if op == opRemove {
+					return os.ErrPermission
+				}
+
+				return nil
+			}}
+
+			err = secret.WriteDir(fs, target, writeThenFail)
+			require.ErrorIs(t, err, errInjected)
+			require.ErrorIs(t, err, os.ErrPermission)
+			assert.Empty(t, dirNames(t, base, listed))
+		})
+	}
+}
+
+// TestWriteDirKeepsExistingDir makes writing into a directory that already
+// exists fail, and checks that the directory, with what was in it, is still
+// there: WriteDir writes into it in place and never removes it.
+func TestWriteDirKeepsExistingDir(t *testing.T) {
+	t.Parallel()
+
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			t.Parallel()
+
+			fs, dir := tfs.open(t)
+			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(got string) error {
+				assert.Equal(t, target, got)
+
+				return errInjected
+			})
+			require.ErrorIs(t, err, errInjected)
+			assert.Equal(t, []string{unlockerMetadataFile}, dirNames(t, fs, target))
+		})
+	}
 }

internal/secret/keychainunlocker.go

@@ -341,16 +341,13 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate keychain item name: %w", err)
 	}
 
-	// Create unlocker directory using the keychain item name as the directory name
+	// The unlocker directory is named after the keychain item
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", keychainItemName)
-	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
-		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
-	}
 
 	// Step 1: Generate a new age keypair for the keychain unlocker
 	ageIdentity, err := age.GenerateX25519Identity()
@@ -358,6 +355,8 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
+	ageRecipient := ageIdentity.Recipient().String()
+
 	// Step 2: Generate a random passphrase for encrypting the age private key
 	agePrivKeyPassphrase, err := generateRandomPassphrase(agePrivKeyPassphraseLength)
 	if err != nil {
@@ -365,14 +364,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer agePrivKeyPassphrase.Destroy()
 
-	// Step 3: Store age recipient as plaintext
-	ageRecipient := ageIdentity.Recipient().String()
-	recipientPath := filepath.Join(unlockerDir, "pub.txt")
-	if err := WriteFileAtomic(fs, recipientPath, []byte(ageRecipient)); err != nil {
-		return nil, fmt.Errorf("failed to write age recipient: %w", err)
-	}
-
-	// Step 4: Encrypt age private key with the generated passphrase and store on disk
+	// Step 3: Encrypt age private key with the generated passphrase
 	// Create a secure buffer for the private key
 	agePrivKeyStr := ageIdentity.String()
 	agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
@@ -383,31 +375,20 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
 	}
 
-	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age")
-	if err := WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey); err != nil {
-		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
-	}
-
-	// Step 5: Get or derive the long-term private key
+	// Step 4: Get or derive the long-term private key
 	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
 	if err != nil {
 		return nil, err
 	}
 	defer ltPrivKeyData.Destroy()
 
-	// Step 6: Encrypt long-term private key to the new age unlocker
+	// Step 5: Encrypt long-term private key to the new age unlocker
 	encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient())
 	if err != nil {
 		return nil, fmt.Errorf("failed to encrypt long-term private key to age unlocker: %w", err)
 	}
 
-	// Write encrypted long-term private key
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-	if err := WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge); err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Step 7: Prepare keychain data
+	// Step 6: Prepare keychain data
 	keychainData := KeychainData{
 		AgePublicKey:         ageRecipient,
 		AgePrivKeyPassphrase: agePrivKeyPassphrase,
@@ -420,12 +401,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer keychainDataBuffer.Destroy()
 
-	// Step 8: Store data in keychain
-	if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
-		return nil, fmt.Errorf("failed to store data in keychain: %w", err)
-	}
-
-	// Step 9: Create and write enhanced metadata
+	// Step 7: Prepare enhanced metadata
 	keychainMetadata := KeychainUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      "keychain",
@@ -440,10 +416,37 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	if err := WriteFileAtomic(fs,
-		filepath.Join(unlockerDir, "unlocker-metadata.json"),
-		metadataBytes); err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+	// Step 8: Write the unlocker's files and store the data in the keychain,
+	// the metadata last
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		pubPath := filepath.Join(dir, "pub.txt")
+		if err := WriteFileAtomic(fs, pubPath, []byte(ageRecipient)); err != nil {
+			return fmt.Errorf("failed to write age recipient: %w", err)
+		}
+
+		privPath := filepath.Join(dir, "priv.age")
+		if err := WriteFileAtomic(fs, privPath, encryptedAgePrivKey); err != nil {
+			return fmt.Errorf("failed to write encrypted age private key: %w", err)
+		}
+
+		ltKeyPath := filepath.Join(dir, "longterm.age")
+		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtPrivKeyToAge); err != nil {
+			return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+		}
+
+		if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
+			return fmt.Errorf("failed to store data in keychain: %w", err)
+		}
+
+		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
+		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
+			return fmt.Errorf("failed to write unlocker metadata: %w", err)
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	return &KeychainUnlocker{

internal/secret/pgpunlock_test.go

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

internal/secret/pgpunlocker.go

@@ -222,20 +222,13 @@ func generatePGPUnlockerName() (string, error) {
 	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
 }
 
-// preparePGPUnlockerDir checks GPG availability and creates the
-// unlocker directory in the current vault, returning the vault and the
-// directory path.
+// pgpUnlockerDir returns the current vault and the directory in it for a
+// new PGP unlocker, named after the host and the day.
 //
 //nolint:ireturn // the vault is only available behind VaultInterface
-func preparePGPUnlockerDir(
+func pgpUnlockerDir(
 	fs afero.Fs, stateDir string,
 ) (VaultInterface, string, error) {
-	// Check if GPG is available
-	err := checkGPGAvailable()
-	if err != nil {
-		return nil, "", err
-	}
-
 	// Get current vault
 	vault, err := GetCurrentVault(fs, stateDir)
 	if err != nil {
@@ -248,27 +241,29 @@ func preparePGPUnlockerDir(
 		return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)
 	}
 
-	// Create unlocker directory using the generated name
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
-	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerName)
-
-	err = fs.MkdirAll(unlockerDir, DirPerms)
-	if err != nil {
-		return nil, "", fmt.Errorf("failed to create unlocker directory: %w", err)
+	return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
 }
 
-	return vault, unlockerDir, nil
-}
-
-// CreatePGPUnlocker creates a new PGP unlocker and stores it in the 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.
 func CreatePGPUnlocker(
-	fs afero.Fs, stateDir string, gpgKeyID string,
+	fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
 ) (*PGPUnlocker, error) {
-	vault, unlockerDir, err := preparePGPUnlockerDir(fs, stateDir)
+	err := checkGPGAvailable()
+	if err != nil {
+		return nil, err
+	}
+
+	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err
 	}
@@ -279,77 +274,13 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
-	// Step 2: Store age recipient as plaintext
-	ageRecipient := ageIdentity.Recipient().String()
-	recipientPath := filepath.Join(unlockerDir, "pub.txt")
-
-	err = WriteFileAtomic(fs, recipientPath, []byte(ageRecipient))
-	if err != nil {
-		return nil, fmt.Errorf("failed to write age recipient: %w", err)
-	}
-
-	// Step 3: Get or derive the long-term private key
-	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
+	// Step 2: Encrypt the long-term private key to the new keypair, and the
+	// keypair's private key to the GPG key
+	encryptedLtPrivKey, encryptedAgePrivKey, err := encryptPGPUnlockerKeys(
+		fs, vault, ageIdentity, gpgKeyID)
 	if err != nil {
 		return nil, err
 	}
-	defer ltPrivKeyData.Destroy()
-
-	// Step 7: Encrypt long-term private key to the new age unlocker
-	encryptedLtPrivKeyToAge, err := EncryptToRecipient(
-		ltPrivKeyData, ageIdentity.Recipient())
-	if err != nil {
-		return nil, fmt.Errorf(
-			"failed to encrypt long-term private key to age unlocker: %w", err)
-	}
-
-	// Write encrypted long-term private key
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-
-	err = WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Step 8: Encrypt age private key to the GPG key ID
-	// Use memguard to protect the private key in memory
-	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
-	defer agePrivateKeyBuffer.Destroy()
-
-	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to encrypt age private key with GPG: %w", err)
-	}
-
-	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age.gpg")
-
-	err = WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
-	}
-
-	// Steps 9-10: Resolve the fingerprint and write enhanced metadata
-	pgpMetadata, err := writePGPUnlockerMetadata(fs, unlockerDir, gpgKeyID)
-	if err != nil {
-		return nil, err
-	}
-
-	return &PGPUnlocker{
-		Directory: unlockerDir,
-		Metadata:  pgpMetadata.UnlockerMetadata,
-		fs:        fs,
-	}, nil
-}
-
-// writePGPUnlockerMetadata resolves the GPG key fingerprint and writes
-// the unlocker metadata file, returning the metadata written.
-func writePGPUnlockerMetadata(
-	fs afero.Fs, unlockerDir string, gpgKeyID string,
-) (*PGPUnlockerMetadata, error) {
-	fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
-	}
 
 	pgpMetadata := PGPUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
@@ -365,13 +296,85 @@ func writePGPUnlockerMetadata(
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	err = WriteFileAtomic(fs,
-		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
+	// Step 3: Write the unlocker's files, the metadata last
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		return writePGPUnlockerFiles(fs, dir, ageIdentity.Recipient(),
+			encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes)
+	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, err
 	}
 
-	return &pgpMetadata, nil
+	return &PGPUnlocker{
+		Directory: unlockerDir,
+		Metadata:  pgpMetadata.UnlockerMetadata,
+		fs:        fs,
+	}, nil
+}
+
+// encryptPGPUnlockerKeys returns the vault's long-term private key encrypted
+// to the new PGP unlocker's age keypair, and that keypair's private key
+// encrypted to the GPG key gpgKeyID.
+func encryptPGPUnlockerKeys(
+	fs afero.Fs, vault VaultInterface,
+	ageIdentity *age.X25519Identity, gpgKeyID string,
+) ([]byte, []byte, error) {
+	// Get or derive the long-term private key
+	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
+	if err != nil {
+		return nil, nil, err
+	}
+	defer ltPrivKeyData.Destroy()
+
+	encryptedLtPrivKey, err := EncryptToRecipient(
+		ltPrivKeyData, ageIdentity.Recipient())
+	if err != nil {
+		return nil, nil, fmt.Errorf(
+			"failed to encrypt long-term private key to age unlocker: %w", err)
+	}
+
+	// Use memguard to protect the private key in memory
+	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
+	defer agePrivateKeyBuffer.Destroy()
+
+	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
+	if err != nil {
+		return nil, nil, fmt.Errorf(
+			"failed to encrypt age private key with GPG: %w", err)
+	}
+
+	return encryptedLtPrivKey, encryptedAgePrivKey, nil
+}
+
+// writePGPUnlockerFiles writes the files of a PGP unlocker into dir, the
+// metadata last.
+func writePGPUnlockerFiles(
+	fs afero.Fs, dir string, ageRecipient *age.X25519Recipient,
+	encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes []byte,
+) error {
+	err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"),
+		[]byte(ageRecipient.String()))
+	if err != nil {
+		return fmt.Errorf("failed to write age recipient: %w", err)
+	}
+
+	err = WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	err = WriteFileAtomic(fs, filepath.Join(dir, "priv.age.gpg"), encryptedAgePrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted age private key: %w", err)
+	}
+
+	err = WriteFileAtomic(fs,
+		filepath.Join(dir, "unlocker-metadata.json"), metadataBytes)
+	if err != nil {
+		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+	}
+
+	return nil
 }
 
 // validateGPGKeyID validates that a GPG key ID is safe for command execution

internal/secret/pgpunlocker_test.go

@@ -0,0 +1,69 @@
+package secret_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
+const (
+	testGPGKeyID       = "0123456789ABCDEF"
+	testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
+)
+
+// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
+// else fails.
+const fakeGPGScript = `#!/bin/sh
+[ "$*" = --version ]
+`
+
+// installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
+func installFakeGPG(t *testing.T) {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	//nolint:gosec // G306: the script must be executable
+	err := os.WriteFile(filepath.Join(dir, "gpg"), []byte(fakeGPGScript), 0o700)
+	require.NoError(t, err)
+
+	t.Setenv("PATH", dir)
+}
+
+// TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at
+// getting the vault's long-term key, which used to come after part of the
+// unlocker was written, and asserts that nothing is written. Getting the key
+// fails because on macOS there is no mnemonic and no current unlocker, and
+// on every other platform it always fails
+// (https://git.eeqj.de/sneak/secret/issues/88).
+//
+//nolint:paralleltest // t.Setenv forbids t.Parallel
+func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
+	installFakeGPG(t)
+	t.Setenv(secret.EnvMnemonic, "")
+
+	base := afero.NewMemMapFs()
+	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
+	require.NoError(t, err)
+
+	fs := hookFs{Fs: base, before: func(_, path string) error {
+		t.Errorf("changed %s", path)
+
+		return nil
+	}}
+
+	_, err = secret.CreatePGPUnlocker(
+		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint)
+	require.Error(t, err)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
+}

internal/secret/seunlocker_darwin.go

@@ -254,7 +254,7 @@ func CreateSecureEnclaveUnlocker(
 		)
 	}
 
-	// Step 4: Create unlocker directory and write files
+	// Step 4: Prepare the unlocker directory's path and metadata
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
@@ -262,23 +262,7 @@ func CreateSecureEnclaveUnlocker(
 
 	unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel))
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName)
-	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
-		return nil, fmt.Errorf(
-			"failed to create unlocker directory: %w",
-			err,
-		)
-	}
 
-	// Write SE-encrypted long-term key
-	ltKeyPath := filepath.Join(unlockerDir, seLongtermFilename)
-	if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
-		return nil, fmt.Errorf(
-			"failed to write SE-encrypted long-term key: %w",
-			err,
-		)
-	}
-
-	// Write metadata
 	seMetadata := SecureEnclaveUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      seUnlockerType,
@@ -294,9 +278,25 @@ func CreateSecureEnclaveUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
+	// Step 5: Write the SE-encrypted long-term key, then the metadata
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		ltKeyPath := filepath.Join(dir, seLongtermFilename)
+		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
+			return fmt.Errorf(
+				"failed to write SE-encrypted long-term key: %w",
+				err,
+			)
+		}
+
+		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
 		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
-		return nil, fmt.Errorf("failed to write metadata: %w", err)
+			return fmt.Errorf("failed to write metadata: %w", err)
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	return &SecureEnclaveUnlocker{

internal/vault/integration_test.go

@@ -1,6 +1,7 @@
 package vault_test
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
 	"slices"
@@ -197,10 +198,11 @@ func testDeepPathSecrets(t *testing.T, fs afero.Fs, tempDir string) {
 	if err != nil {
 		t.Fatalf("Failed to retrieve deep path secret: %v", err)
 	}
+	defer retrievedValue.Destroy()
 
-	if string(retrievedValue) != string(expectedValue) {
+	if !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
 		t.Errorf("Retrieved value doesn't match. Expected %q, got %q",
-			string(expectedValue), string(retrievedValue))
+			expectedValue, retrievedValue.Bytes())
 	}
 }
 

internal/vault/integration_version_test.go

@@ -119,7 +119,10 @@ func testCreateInitialVersion(
 	// Verify secret can be retrieved
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1-data"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-1-data"), value.Bytes())
 
 	// Verify version directory structure
 	secretDir := filepath.Join(vaultDir, "secrets.d", "integration%test")
@@ -166,7 +169,10 @@ func testCreateSecondVersion(
 	// Verify new value is current
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2-data"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-2-data"), value.Bytes())
 
 	// Verify we now have two versions
 	versions, err = secret.ListVersions(fs, secretDir)
@@ -209,7 +215,10 @@ func testCreateThirdVersion(
 	// Current should be version-3
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-3-data"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-3-data"), value.Bytes())
 }
 
 func testRetrieveSpecificVersions(
@@ -225,15 +234,24 @@ func testRetrieveSpecificVersions(
 	// Get each version by its name
 	value1, err := vault.GetSecretVersion(secretName, versions[2]) // oldest
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1-data"), value1)
+
+	defer value1.Destroy()
+
+	assert.Equal(t, []byte("version-1-data"), value1.Bytes())
 
 	value2, err := vault.GetSecretVersion(secretName, versions[1]) // middle
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2-data"), value2)
+
+	defer value2.Destroy()
+
+	assert.Equal(t, []byte("version-2-data"), value2.Bytes())
 
 	value3, err := vault.GetSecretVersion(secretName, versions[0]) // newest
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-3-data"), value3)
+
+	defer value3.Destroy()
+
+	assert.Equal(t, []byte("version-3-data"), value3.Bytes())
 
 	// An empty version is not one of the versions; GetSecret gets the
 	// current one
@@ -259,7 +277,10 @@ func testPromoteOldVersion(
 	// Verify current now returns the old version's value
 	value, err := vault.GetSecret(secretName)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1-data"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-1-data"), value.Bytes())
 
 	// Verify the version metadata hasn't changed
 	// (promoting shouldn't modify timestamps)
@@ -353,8 +374,13 @@ func TestVersionConcurrency(t *testing.T) {
 				value, err := vault.GetSecret(secretName)
 				if err != nil {
 					errCh <- err
-				} else if string(value) != "initial" {
-					errCh <- fmt.Errorf("%w: %s", errUnexpectedValue, value)
+				} else {
+					if value.String() != "initial" {
+						errCh <- fmt.Errorf("%w: %s",
+							errUnexpectedValue, value.Bytes())
+					}
+
+					value.Destroy()
 				}
 
 				done <- true

internal/vault/secrets.go

@@ -301,8 +301,9 @@ func updateVersionMetadata(
 	return nil
 }
 
-// GetSecret retrieves the current version of a secret from this vault
-func (v *Vault) GetSecret(name string) ([]byte, error) {
+// GetSecret retrieves the current version of a secret from this vault.
+// The caller must destroy the returned buffer.
+func (v *Vault) GetSecret(name string) (*memguard.LockedBuffer, error) {
 	secret.DebugWith("Getting secret from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -326,7 +327,10 @@ func (v *Vault) GetSecret(name string) ([]byte, error) {
 
 // GetSecretVersion retrieves a specific version of a secret. The version
 // must be one of the secret's versions; GetSecret gets the current one.
-func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
+// The caller must destroy the returned buffer.
+func (v *Vault) GetSecretVersion(
+	name string, version string,
+) (*memguard.LockedBuffer, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -372,26 +376,14 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 		return nil, fmt.Errorf("failed to decrypt version: %w", err)
 	}
 
-	// Create a copy to return since the buffer will be destroyed
-	result := make([]byte, decryptedValue.Size())
-	copy(result, decryptedValue.Bytes())
-	decryptedValue.Destroy()
-
 	secret.DebugWith("Successfully decrypted secret version",
 		slog.String("secret_name", name),
 		slog.String("version", version),
 		slog.String("vault_name", v.Name),
-		slog.Int("decrypted_length", len(result)),
+		slog.Int("decrypted_length", decryptedValue.Size()),
 	)
 
-	// Debug: Log metadata about the decrypted value without exposing the actual secret
-	secret.Debug("Vault secret decryption debug info",
-		"secret_name", name,
-		"version", version,
-		"decrypted_value_length", len(result),
-		"is_empty", len(result) == 0)
-
-	return result, nil
+	return decryptedValue, nil
 }
 
 // UnlockVault unlocks the vault and returns the long-term private key

internal/vault/secrets_version_test.go

@@ -131,7 +131,10 @@ func TestVaultAddSecretCreatesVersion(t *testing.T) {
 	// Get the secret value
 	retrievedValue, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-	assert.Equal(t, expectedValue, retrievedValue)
+
+	defer retrievedValue.Destroy()
+
+	assert.Equal(t, expectedValue, retrievedValue.Bytes())
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -165,7 +168,10 @@ func TestVaultAddSecretMultipleVersions(t *testing.T) {
 	// Current value should be version-2
 	value, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer value.Destroy()
+
+	assert.Equal(t, []byte("version-2"), value.Bytes())
 }
 
 //nolint:paralleltest // createTestVaultWithKey uses t.Setenv
@@ -192,15 +198,21 @@ func TestVaultGetSecretVersion(t *testing.T) {
 
 	// Get specific version (first one)
 	firstVersion := versions[1] // Last in list is first created
-	value, err := vault.GetSecretVersion(testSecretPath, firstVersion)
+	first, err := vault.GetSecretVersion(testSecretPath, firstVersion)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-1"), value)
+
+	defer first.Destroy()
+
+	assert.Equal(t, []byte("version-1"), first.Bytes())
 
 	// Get specific version (second one)
 	secondVersion := versions[0] // First in list is most recent
-	value, err = vault.GetSecretVersion(testSecretPath, secondVersion)
+	second, err := vault.GetSecretVersion(testSecretPath, secondVersion)
 	require.NoError(t, err)
-	assert.Equal(t, []byte("version-2"), value)
+
+	defer second.Destroy()
+
+	assert.Equal(t, []byte("version-2"), second.Bytes())
 
 	// An empty version is not one of the versions; GetSecret gets the
 	// current one

internal/vault/unlockers.go

@@ -350,26 +350,14 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	// Create unlocker directory
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase)
 
-	err = v.fs.MkdirAll(unlockerDir, secret.DirPerms)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
-	}
-
 	// Generate new age keypair for unlocker
 	unlockerIdentity, err := age.GenerateX25519Identity()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate unlocker: %w", err)
 	}
 
-	// Write the unlocker keypair (public and passphrase-encrypted private)
-	err = v.writeUnlockerKeypair(unlockerDir, unlockerIdentity, passphrase)
-	if err != nil {
-		return nil, err
-	}
-
 	// Encrypt long-term private key to this unlocker
 	ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
 	defer ltPrivKeyBuffer.Destroy()
@@ -380,15 +368,6 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to encrypt long-term private key: %w", err)
 	}
 
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-
-	err = secret.WriteFileAtomic(v.fs, ltPrivKeyPath, encryptedLtPrivKey)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Write the metadata last: readers skip an unlocker directory without
-	// it, so an unlocker interrupted before this point is never used.
 	metadata := UnlockerMetadata{
 		Type:      unlockerTypePassphrase,
 		CreatedAt: time.Now(),
@@ -400,11 +379,13 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
-
-	err = secret.WriteFileAtomic(v.fs, metadataPath, metadataBytes)
+	// Write the unlocker's files, the metadata last
+	err = secret.WriteDir(v.fs, unlockerDir, func(dir string) error {
+		return v.writeUnlockerFiles(dir, unlockerIdentity, passphrase,
+			encryptedLtPrivKey, metadataBytes)
+	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, err
 	}
 
 	// Create the unlocker instance
@@ -450,12 +431,14 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro
 	return metadata, nil
 }
 
-// writeUnlockerKeypair writes the unlocker's public key and its
-// passphrase-encrypted private key into the unlocker directory.
-func (v *Vault) writeUnlockerKeypair(
+// writeUnlockerFiles writes the files of a passphrase unlocker into
+// unlockerDir: its public key, its passphrase-encrypted private key, the
+// long-term private key encrypted to it, and its metadata, last.
+func (v *Vault) writeUnlockerFiles(
 	unlockerDir string,
 	unlockerIdentity *age.X25519Identity,
 	passphrase *memguard.LockedBuffer,
+	encryptedLtPrivKey, metadataBytes []byte,
 ) error {
 	// Write public key
 	pubKeyPath := filepath.Join(unlockerDir, "pub.age")
@@ -485,5 +468,17 @@ func (v *Vault) writeUnlockerKeypair(
 		return fmt.Errorf("failed to write encrypted unlocker private key: %w", err)
 	}
 
+	err = secret.WriteFileAtomic(v.fs,
+		filepath.Join(unlockerDir, "longterm.age"), encryptedLtPrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	err = secret.WriteFileAtomic(v.fs,
+		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
+	if err != nil {
+		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+	}
+
 	return nil
 }

internal/vault/vault_test.go

@@ -1,6 +1,7 @@
 package vault_test
 
 import (
+	"bytes"
 	"path/filepath"
 	"slices"
 	"testing"
@@ -184,10 +185,11 @@ func testSecretOperations(t *testing.T, fs afero.Fs) {
 	if err != nil {
 		t.Fatalf("Failed to get secret: %v", err)
 	}
+	defer retrievedValue.Destroy()
 
-	if string(retrievedValue) != string(expectedValue) {
+	if !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
 		t.Errorf("Expected secret value '%s', got '%s'",
-			string(expectedValue), string(retrievedValue))
+			expectedValue, retrievedValue.Bytes())
 	}
 }