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

@@ -27,14 +27,22 @@ Bring the repo into policy compliance in one commit:
 
 - 2026-10-04: A failed unlocker add no longer leaves a partial unlocker
   directory (https://git.eeqj.de/sneak/secret/issues/48).
-  `CreatePGPUnlocker` resolves the GPG key's fingerprint, and it and
-  `CreateKeychainUnlocker` get the long-term key and encrypt everything,
-  before writing anything. All four unlocker types write their files
+  `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

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_test.go

@@ -346,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)
@@ -384,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())
 		})
 	}
 }

internal/secret/keychainunlocker.go

@@ -419,15 +419,18 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	// 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 {
-		if err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"), []byte(ageRecipient)); err != nil {
+		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)
 		}
 
-		if err := WriteFileAtomic(fs, filepath.Join(dir, "priv.age"), encryptedAgePrivKey); err != nil {
+		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)
 		}
 
-		if err := WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKeyToAge); err != nil {
+		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)
 		}
 
@@ -435,7 +438,8 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 			return fmt.Errorf("failed to store data in keychain: %w", err)
 		}
 
-		if err := WriteFileAtomic(fs, filepath.Join(dir, "unlocker-metadata.json"), metadataBytes); err != nil {
+		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)
 		}
 

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

@@ -250,22 +250,19 @@ func pgpUnlockerDir(
 }
 
 // CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault.
-// Everything that can fail short of writing a file, resolving the GPG key's
-// fingerprint included, is done before anything is written, and the files
-// are written through WriteDir, so a failure leaves no partial unlocker.
+// 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) {
 	err := checkGPGAvailable()
 	if err != nil {
 		return nil, err
 	}
 
-	fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
-	}
-
 	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err

internal/secret/pgpunlocker_test.go

@@ -12,23 +12,16 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// The one GPG key the gpg from installFakeGPG knows, its fingerprint, and a
-// key ID it does not know.
+// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
 const (
-	knownGPGKeyID       = "0123456789ABCDEF"
-	knownGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
-	unknownGPGKeyID     = "FEDCBA9876543210"
+	testGPGKeyID       = "0123456789ABCDEF"
+	testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
 )
 
-// fakeGPGScript is a gpg for which `gpg --version` succeeds, listing
-// knownGPGKeyID prints its fingerprint in gpg's colon format, and anything
+// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
 // else fails.
 const fakeGPGScript = `#!/bin/sh
-case "$*" in
---version) ;;
-*" ` + knownGPGKeyID + `") echo "fpr:::::::::` + knownGPGFingerprint + `:" ;;
-*) exit 2 ;;
-esac
+[ "$*" = --version ]
 `
 
 // installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
@@ -45,48 +38,32 @@ func installFakeGPG(t *testing.T) {
 }
 
 // TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at
-// the two steps that used to come after part of the unlocker was written:
-// resolving the GPG key's fingerprint, and getting the vault's long-term key
-// (there is no mnemonic, and no current unlocker to get it from). Each must
-// fail before anything is written, leaving no unlocker directory.
+// 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, "")
 
-	// The fake gpg resolves the known key and no other
-	fingerprint, err := secret.ResolveGPGKeyFingerprint(knownGPGKeyID)
+	base := afero.NewMemMapFs()
+	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
 	require.NoError(t, err)
-	require.Equal(t, knownGPGFingerprint, fingerprint)
 
-	_, err = secret.ResolveGPGKeyFingerprint(unknownGPGKeyID)
+	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)
 
-	for _, tc := range []struct {
-		name     string
-		gpgKeyID string
-	}{
-		{"fingerprint not found", unknownGPGKeyID},
-		{"long-term key not available", knownGPGKeyID},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			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, tc.gpgKeyID)
-			require.Error(t, err)
-
-			vaultDir, err := vlt.GetDirectory()
-			require.NoError(t, err)
-			assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
-		})
-	}
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
 }

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/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())
 	}
 }