Read secret environment variables once per command, then unset them (closes #60)
check / check (push) Failing after 2s

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.
Nothing below the command reads the environment. README warns against
both variables.

Model: opus-5-5
This commit is contained in:
2026-10-04 11:30:08 +00:00
parent eb596b8be6
commit 682bf751e7
39 changed files with 620 additions and 386 deletions
+21 -9
View File
@@ -33,6 +33,17 @@ const (
missingFile = "/no/such/file"
)
// testMnemonicBuffer returns testMnemonic in a locked buffer that is
// destroyed when the test ends.
func testMnemonicBuffer(t *testing.T) *memguard.LockedBuffer {
t.Helper()
mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
t.Cleanup(mnemonic.Destroy)
return mnemonic
}
// The state directory newTwoVaultFs copies, recorded by snapshotStateDir.
// Creating a passphrase unlocker is slow by design, so the vaults are made
// once, by the first test that needs them.
@@ -52,13 +63,12 @@ var (
func newTwoVaultFs(t *testing.T) afero.Fs {
t.Helper()
t.Setenv(secret.EnvMnemonic, testMnemonic)
twoVaultsOnce.Do(func() {
fs := afero.NewMemMapFs()
mnemonic := testMnemonicBuffer(t)
for _, name := range []string{"work", "default"} {
vlt, err := vault.CreateVault(fs, testStateDir, name)
vlt, err := vault.CreateVault(fs, testStateDir, name, mnemonic)
require.NoError(t, err)
err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
@@ -163,7 +173,7 @@ func requireRejectedAndUnchanged(
// Moves and imports use --force, so that only the name check stands in
// the way.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
//nolint:paralleltest // the cases share cmd
func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
// Creating a passphrase unlocker is slow by design, so the vaults are
// created once and each case runs on its own copy of them.
@@ -259,7 +269,7 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
// `secret version rm x ""` every version of x. A version argument is
// accepted only if it is one of the versions `secret version list` lists.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
//nolint:paralleltest // the cases share cmd
func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
before := snapshotStateDir(t, newTwoVaultFs(t))
@@ -295,14 +305,16 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
// TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
// with a version that is not the current one removes that version and
// changes nothing else.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestRemoveVersionRemovesOnlyThatVersion(t *testing.T) {
t.Parallel()
fs := newTwoVaultFs(t)
vlt, err := vault.GetCurrentVault(fs, testStateDir)
require.NoError(t, err)
vlt.Mnemonic = testMnemonicBuffer(t)
// A second version of "x" becomes the current one.
err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("new")), true)
require.NoError(t, err)
@@ -336,9 +348,9 @@ func TestRemoveVersionRemovesOnlyThatVersion(t *testing.T) {
// TestMoveToVaultNameRenamesInCurrentVault checks that `secret mv x work`,
// where "work" is also the name of a vault, renames the secret "x" to "work"
// in the current vault and changes nothing else.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestMoveToVaultNameRenamesInCurrentVault(t *testing.T) {
t.Parallel()
before := snapshotStateDir(t, newTwoVaultFs(t))
fs := newFsFromSnapshot(t, before)