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

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

internal/cli/lock_test.go

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

internal/cli/move_test.go

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

internal/cli/secrets.go

@@ -414,6 +414,9 @@ 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 {
@@ -424,10 +427,9 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	if err != nil {
 		return err
 	}
-	defer value.Destroy()
 
-	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	// Print the secret value to stdout
+	_, _ = cli.Print(string(value))
 
 	return nil
 }
@@ -440,6 +442,9 @@ 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 {
@@ -455,15 +460,22 @@ func (cli *Instance) GetSecretWithVersion(
 
 		return err
 	}
-	defer value.Destroy()
 
-	secret.Debug("Got secret value", "valueLength", value.Size())
+	secret.Debug("Got secret value", "valueLength", len(value))
 
-	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	// Print the secret value to stdout
+	_, _ = cli.Print(string(value))
 
 	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,10 +143,7 @@ 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)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
+	assert.Equal(t, testData, retrievedValue,
 		"Retrieved secret should match original (without newline)")
 }
 
@@ -196,11 +193,7 @@ 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)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
-		"Retrieved secret should match original")
+	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
 }
 
 // TestAddSecretVariousSizes tests adding secrets of various sizes through stdin
@@ -382,10 +375,7 @@ func TestAddSecretBufferGrowth(t *testing.T) {
 			// Verify the secret was stored correctly
 			retrievedValue, err := vlt.GetSecret(secretName)
 			require.NoError(t, err)
-
-			defer retrievedValue.Destroy()
-
-			assert.Equal(t, testData, retrievedValue.Bytes(),
+			assert.Equal(t, testData, retrievedValue,
 				"Retrieved secret should match original exactly")
 		})
 	}
@@ -426,11 +416,7 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 	// Verify the secret was stored correctly
 	retrievedValue, err := vlt.GetSecret("streaming-test")
 	require.NoError(t, err)
-
-	defer retrievedValue.Destroy()
-
-	assert.Equal(t, testData, retrievedValue.Bytes(),
-		"Retrieved secret should match original")
+	assert.Equal(t, testData, retrievedValue, "Retrieved secret should match original")
 }
 
 // slowReader simulates a reader that returns data in small chunks

internal/cli/unlockers.go

@@ -685,8 +685,7 @@ 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, once: the duplicate check
-	// and the new unlocker's metadata both use this result
+	// Resolve the GPG key ID to its fingerprint
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -707,8 +706,7 @@ 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, fingerprint)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID)
 	if err != nil {
 		return err
 	}

internal/cli/unlockers_add_test.go

@@ -1,35 +0,0 @@
-//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,7 +7,6 @@
 // - 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
 //
@@ -24,7 +23,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-	"unicode/utf8"
 
 	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
@@ -190,10 +188,7 @@ func TestPromoteVersionCommand(t *testing.T) {
 	// Current should be version-2
 	value, err := vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-
-	defer value.Destroy()
-
-	assert.Equal(t, []byte("version-2"), value.Bytes())
+	assert.Equal(t, []byte("version-2"), value)
 
 	// Promote first version
 	firstVersion := versions[1] // Older version
@@ -216,12 +211,9 @@ func TestPromoteVersionCommand(t *testing.T) {
 	assert.Contains(t, outputStr, firstVersion)
 
 	// Verify current is now version-1
-	promoted, err := vlt.GetSecret("test/secret")
+	value, err = vlt.GetSecret("test/secret")
 	require.NoError(t, err)
-
-	defer promoted.Destroy()
-
-	assert.Equal(t, []byte("version-1"), promoted.Bytes())
+	assert.Equal(t, []byte("version-1"), value)
 }
 
 //nolint:paralleltest // uses t.Setenv via setupTestVault
@@ -298,46 +290,6 @@ 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,10 +346,7 @@ func TestLongestNames(t *testing.T) {
 
 	got, err := vlt.GetSecret(name)
 	require.NoError(t, err)
-
-	defer got.Destroy()
-
-	assert.Equal(t, []byte("long"), got.Bytes())
+	assert.Equal(t, "long", string(got))
 
 	vaultDir, err := vlt.GetDirectory()
 	require.NoError(t, err)
@@ -387,10 +384,7 @@ func TestForcedCopyKeepsDestinationUntilReplaced(t *testing.T) {
 
 			value, err := dest.GetSecret("shared")
 			require.NoError(t, err)
-
-			defer value.Destroy()
-
-			assert.Equal(t, []byte("old"), value.Bytes())
+			assert.Equal(t, "old", string(value))
 		})
 	}
 }

internal/secret/keychainunlocker.go

@@ -419,18 +419,15 @@ 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 {
-		pubPath := filepath.Join(dir, "pub.txt")
-		if err := WriteFileAtomic(fs, pubPath, []byte(ageRecipient)); err != nil {
+		if err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"), []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 {
+		if err := WriteFileAtomic(fs, filepath.Join(dir, "priv.age"), 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 {
+		if err := WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKeyToAge); err != nil {
 			return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
 		}
 
@@ -438,8 +435,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 			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 {
+		if err := WriteFileAtomic(fs, filepath.Join(dir, "unlocker-metadata.json"), 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, fingerprint)
+		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID)
 		if err != nil {
 			t.Fatalf("Failed to create PGP unlock key: %v", err)
 		}

internal/secret/pgpunlocker.go

@@ -250,19 +250,22 @@ func pgpUnlockerDir(
 }
 
 // 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.
+// 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.
 func CreatePGPUnlocker(
-	fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
+	fs afero.Fs, stateDir string, gpgKeyID 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,16 +12,23 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
+// The one GPG key the gpg from installFakeGPG knows, its fingerprint, and a
+// key ID it does not know.
 const (
-	testGPGKeyID       = "0123456789ABCDEF"
-	testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
+	knownGPGKeyID       = "0123456789ABCDEF"
+	knownGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
+	unknownGPGKeyID     = "FEDCBA9876543210"
 )
 
-// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
+// fakeGPGScript is a gpg for which `gpg --version` succeeds, listing
+// knownGPGKeyID prints its fingerprint in gpg's colon format, and anything
 // else fails.
 const fakeGPGScript = `#!/bin/sh
-[ "$*" = --version ]
+case "$*" in
+--version) ;;
+*" ` + knownGPGKeyID + `") echo "fpr:::::::::` + knownGPGFingerprint + `:" ;;
+*) exit 2 ;;
+esac
 `
 
 // installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
@@ -38,32 +45,48 @@ func installFakeGPG(t *testing.T) {
 }
 
 // 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).
+// 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.
 //
 //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)
+	// The fake gpg resolves the known key and no other
+	fingerprint, err := secret.ResolveGPGKeyFingerprint(knownGPGKeyID)
 	require.NoError(t, err)
+	require.Equal(t, knownGPGFingerprint, fingerprint)
 
-	fs := hookFs{Fs: base, before: func(_, path string) error {
-		t.Errorf("changed %s", path)
-
-		return nil
-	}}
-
-	_, err = secret.CreatePGPUnlocker(
-		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint)
+	_, err = secret.ResolveGPGKeyFingerprint(unknownGPGKeyID)
 	require.Error(t, err)
 
-	vaultDir, err := vlt.GetDirectory()
-	require.NoError(t, err)
-	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
+	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")))
+		})
+	}
 }

internal/vault/integration_test.go

@@ -1,7 +1,6 @@
 package vault_test
 
 import (
-	"bytes"
 	"os"
 	"path/filepath"
 	"slices"
@@ -198,11 +197,10 @@ 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 !bytes.Equal(retrievedValue.Bytes(), expectedValue) {
+	if string(retrievedValue) != string(expectedValue) {
 		t.Errorf("Retrieved value doesn't match. Expected %q, got %q",
-			expectedValue, retrievedValue.Bytes())
+			string(expectedValue), string(retrievedValue))
 	}
 }
 

internal/vault/integration_version_test.go

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

internal/vault/secrets.go

@@ -301,9 +301,8 @@ func updateVersionMetadata(
 	return nil
 }
 
-// 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) {
+// GetSecret retrieves the current version of a secret from this vault
+func (v *Vault) GetSecret(name string) ([]byte, error) {
 	secret.DebugWith("Getting secret from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -327,10 +326,7 @@ func (v *Vault) GetSecret(name string) (*memguard.LockedBuffer, error) {
 
 // GetSecretVersion retrieves a specific version of a secret. The version
 // must be one of the secret's versions; GetSecret gets the current one.
-// The caller must destroy the returned buffer.
-func (v *Vault) GetSecretVersion(
-	name string, version string,
-) (*memguard.LockedBuffer, error) {
+func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
 	secret.DebugWith("Getting secret version from vault",
 		slog.String("vault_name", v.Name),
 		slog.String("secret_name", name),
@@ -376,14 +372,26 @@ func (v *Vault) GetSecretVersion(
 		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", decryptedValue.Size()),
+		slog.Int("decrypted_length", len(result)),
 	)
 
-	return decryptedValue, nil
+	// 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
 }
 
 // UnlockVault unlocks the vault and returns the long-term private key

internal/vault/secrets_version_test.go

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

internal/vault/vault_test.go

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