Read secret environment variables once per command, then unset them (closes #60)

init and vault create put the mnemonic into the process environment for
vault.CreateVault to read back, so every program they ran, gpg included,
inherited it, and SB_SECRET_MNEMONIC and SB_UNLOCK_PASSPHRASE were read
at 13 places and never unset. Each command that may need them now reads
both once, in its RunE, into locked buffers on the CLI Instance, and
unsets them at once. The buffers are passed down: vault.CreateVault
takes the mnemonic, a Vault carries Mnemonic and UnlockPassphrase, and
the PGP, keychain and Secure Enclave unlocker constructors take both;
CreatePGPUnlocker sets them on the vault it loads through SetMnemonic
and SetUnlockPassphrase, new in VaultInterface. README warns against
both variables.

Model: opus-5-5

internal/secret/secret_test.go

@@ -2,6 +2,7 @@
 package secret
 
 import (
+	"encoding/json"
 	"errors"
 	"os"
 	"path/filepath"
@@ -22,7 +23,7 @@ const testMnemonicValue = "abandon abandon abandon abandon abandon abandon " +
 	"abandon abandon abandon abandon abandon about"
 
 var (
-	errMnemonicNotSet       = errors.New("SB_SECRET_MNEMONIC not set")
+	errMnemonicNotSet       = errors.New("mock vault has no mnemonic")
 	errNotImplementedInMock = errors.New("not implemented in mock")
 )
 
@@ -32,6 +33,7 @@ type MockVault struct {
 	fs              afero.Fs
 	directory       string
 	derivationIndex uint32
+	mnemonic        *memguard.LockedBuffer
 }
 
 func (m *MockVault) GetDirectory() (string, error) {
@@ -61,12 +63,11 @@ func (m *MockVault) AddSecret(name string, value *memguard.LockedBuffer, _ bool)
 	ltPubKeyPath := filepath.Join(m.directory, "pub.age")
 
 	// Derive long-term key using the vault's derivation index
-	mnemonic := os.Getenv(EnvMnemonic)
-	if mnemonic == "" {
+	if m.mnemonic == nil {
 		return errMnemonicNotSet
 	}
 
-	ltIdentity, err := agehd.DeriveIdentity(mnemonic, m.derivationIndex)
+	ltIdentity, err := agehd.DeriveIdentity(m.mnemonic.String(), m.derivationIndex)
 	if err != nil {
 		return err
 	}
@@ -111,6 +112,12 @@ func (m *MockVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
 	return nil, errNotImplementedInMock
 }
 
+func (m *MockVault) SetMnemonic(mnemonic *memguard.LockedBuffer) {
+	m.mnemonic = mnemonic
+}
+
+func (m *MockVault) SetUnlockPassphrase(_ *memguard.LockedBuffer) {}
+
 func (m *MockVault) CreatePassphraseUnlocker(
 	_ *memguard.LockedBuffer,
 ) (*PassphraseUnlocker, error) {
@@ -238,13 +245,13 @@ func verifySecretFiles(t *testing.T, fs afero.Fs, vaultDir, secretName string) {
 	}
 }
 
-//nolint:paralleltest // uses t.Setenv (process-global environment)
+//nolint:paralleltest // subtests share one vault, order matters
 func TestPerSecretKeyFunctionality(t *testing.T) {
 	// Create an in-memory filesystem for testing
 	fs := afero.NewMemMapFs()
 
-	// Set test mnemonic for direct encryption/decryption
-	t.Setenv(EnvMnemonic, testMnemonicValue)
+	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonicValue))
+	defer mnemonic.Destroy()
 
 	// Set up a test vault structure
 	baseDir := "/test-config/berlin.sneak.pkg.secret"
@@ -258,6 +265,7 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
 		fs:              fs,
 		directory:       vaultDir,
 		derivationIndex: 0,
+		mnemonic:        mnemonic,
 	}
 
 	// Test data
@@ -314,26 +322,45 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
 	})
 }
 
-func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) {
-	// This test demonstrates the bug where GetValue uses hardcoded index 0
-	// instead of the vault's actual derivation index when using environment mnemonic
+// TestSecretGetValueWithMnemonicUsesVaultDerivationIndex checks that
+// GetValue, given the mnemonic, derives the long-term key at the derivation
+// index in the vault's metadata. At index 0 it could not decrypt the secret,
+// which was encrypted to the key at index 1.
+func TestSecretGetValueWithMnemonicUsesVaultDerivationIndex(t *testing.T) {
+	t.Parallel()
 
-	// Set up test mnemonic
-	t.Setenv(EnvMnemonic, testMnemonicValue)
+	fs := afero.NewMemMapFs()
+	vaultDir := "/test-config/vaults.d/test-vault"
 
-	// Create temporary directory for vaults
-	fs := afero.NewOsFs()
-	tempDir, err := afero.TempDir(fs, "", "secret-test-")
+	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonicValue))
+	defer mnemonic.Destroy()
+
+	vlt := &MockVault{
+		name:            "test-vault",
+		fs:              fs,
+		directory:       vaultDir,
+		derivationIndex: 1,
+		mnemonic:        mnemonic,
+	}
+
+	metadata, err := json.Marshal(VaultMetadata{DerivationIndex: vlt.derivationIndex})
+	require.NoError(t, err)
+	require.NoError(t, fs.MkdirAll(vaultDir, DirPerms))
+
+	err = afero.WriteFile(
+		fs, filepath.Join(vaultDir, "vault-metadata.json"), metadata, FilePerms)
 	require.NoError(t, err)
 
-	defer func() {
-		_ = fs.RemoveAll(tempDir)
-	}()
+	secretName, secretValue := "x", "value"
 
-	stateDir := filepath.Join(tempDir, ".secret")
-	require.NoError(t, fs.MkdirAll(stateDir, 0o700))
+	err = vlt.AddSecret(secretName,
+		memguard.NewBufferFromBytes([]byte(secretValue)), false)
+	require.NoError(t, err)
 
-	// This test is now in the integration test file where it can use real vaults
-	// The bug is demonstrated there - see test31EnvMnemonicUsesVaultDerivationIndex
-	t.Log("This test demonstrates the bug in the integration test file")
+	value, err := NewSecret(vlt, secretName).GetValue(nil, mnemonic)
+	require.NoError(t, err)
+
+	defer value.Destroy()
+
+	require.Equal(t, secretValue, value.String())
 }