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

Model: opus-5-5

README.md

@@ -91,6 +91,9 @@ Lists all available vaults. The current vault is marked.
 
 Creates a new vault with the specified name.
 
+**Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
+are allowed, and a name must not be empty, `.` or `..`.
+
 #### `secret vault select <name>`
 
 Switches to the specified vault for subsequent operations.
@@ -323,7 +326,9 @@ line or in a CI job, they end up in shell history and CI logs. `secret` unsets
 each one as soon as it has read it, so that the programs it runs itself, such
 as `gpg`, do not inherit it, but that erases nothing: the environment the
 process started with, and its memory, still hold the value. The interactive
-prompt, used when the variable is not set, is the safer default.
+prompt, which every command except `secret vault import` offers when the
+variable is not set, is the safer default; `secret vault import` has no prompt
+and needs both variables.
 
 ## Security Features
 

TODO.md

@@ -37,6 +37,15 @@ Bring the repo into policy compliance in one commit:
   mnemonic into the environment. Unsetting erases nothing: the starting
   environment (`/proc/<pid>/environ`) and memory still hold the value. The
   README warns against both variables.
+- 2026-10-04: A vault name may use only lowercase ASCII letters, digits,
+  `.`, `-` and `_`, and must not be empty, `.` or `..`
+  (https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md`
+  state the rule. `vault create`, `vault import`, `vault select`,
+  `vault remove`, both vault names of `mv` and shell completion of a
+  `vault:secret` argument check the name as typed with
+  `vault.ValidateVaultName` before building any path from it. Before,
+  `vault import ..` wrote a long-term key and an unlocker into the state
+  directory itself, and `vault select ..` made that the current vault.
 - 2026-10-04: `script/cibuild` runs the checks again on an unchanged
   tree (https://git.eeqj.de/sneak/secret/issues/54). It passes the
   current time as the `CHECK_EPOCH` build argument, which both the lint

internal/cli/completions.go

@@ -123,7 +123,9 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
 }
 
 // completeVaultQualifiedSecrets completes "vault:secret" references once a
-// colon is present in the input
+// colon is present in the input. It completes nothing when the vault part
+// is not a valid vault name, so that a name such as ".." cannot list a
+// directory outside vaults.d.
 func completeVaultQualifiedSecrets(
 	fs afero.Fs, stateDir, toComplete string,
 ) []string {
@@ -134,6 +136,10 @@ func completeVaultQualifiedSecrets(
 	vaultName := parts[0]
 	secretPrefix := parts[1]
 
+	if vault.ValidateVaultName(vaultName) != nil {
+		return nil
+	}
+
 	vlt := vault.NewVault(fs, stateDir, vaultName)
 
 	secrets, err := vlt.ListSecrets()

internal/cli/completions_test.go

@@ -0,0 +1,41 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestVaultSecretCompletionRejectsInvalidVaultName is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/68: completing a `vault:secret`
+// argument lists nothing when the vault part is not a valid vault name, even
+// where that name, joined onto vaults.d, leads to a secrets.d directory.
+func TestVaultSecretCompletionRejectsInvalidVaultName(t *testing.T) {
+	t.Parallel()
+
+	const (
+		stateDir = "/state"
+		dirPerm  = 0o700
+	)
+
+	fs := afero.NewMemMapFs()
+
+	// The vault "work" holds the secret "x". So does every directory an
+	// invalid name below would lead to from vaults.d.
+	for _, vaultName := range []string{"work", ".", "..", "a/b"} {
+		secretDir := filepath.Join(stateDir, "vaults.d", vaultName, "secrets.d", "x")
+		require.NoError(t, fs.MkdirAll(secretDir, dirPerm))
+	}
+
+	assert.Equal(t, []string{"work:x"},
+		completeVaultQualifiedSecrets(fs, stateDir, "work:"))
+
+	for _, toComplete := range []string{".:", "..:", "a/b:"} {
+		assert.Empty(t, completeVaultQualifiedSecrets(fs, stateDir, toComplete),
+			"completing %q", toComplete)
+	}
+}

internal/cli/move_test.go

@@ -47,26 +47,25 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
 			"mv work:nosuch work:y", "work:nosuch", "work:y", false,
 			"secret 'nosuch' not found",
 		},
-		// Only an existing vault is used, so ".." cannot reach the state
-		// directory itself.
+		// Only an existing vault is used.
 		{
-			"mv --force ..:x ..:y", "..:x", "..:y", true,
-			"vault '..' does not exist",
+			"mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
+			"vault 'nosuch' does not exist",
 		},
-		// Each of these spells "work" a second way. The spelling is not an
-		// existing vault name, so the move is not taken for a move between
-		// two vaults, which would delete the destination, here the source.
+		// Each of these spells "work" a second way. The spelling is not a
+		// valid vault name, so the move is not taken for a move between two
+		// vaults, which would delete the destination, here the source.
 		{
 			"mv --force work:x work/:x", workX, "work/:x", true,
-			"vault 'work/' does not exist",
+			vault.ValidateVaultName("work/").Error(),
 		},
 		{
 			"mv --force work/:x work:", "work/:x", "work:", true,
-			"vault 'work/' does not exist",
+			vault.ValidateVaultName("work/").Error(),
 		},
 		{
 			"mv --force work:x ./work:x", workX, "./work:x", true,
-			"vault './work' does not exist",
+			vault.ValidateVaultName("./work").Error(),
 		},
 	}
 

internal/cli/path_traversal_test.go

@@ -302,6 +302,65 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
 	}
 }
 
+// TestInvalidVaultNameLeavesStateUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/68, where
+// `secret vault import ..` wrote a long-term key and an unlocker into the
+// state directory itself, and `secret vault select ..` made it the current
+// vault. Each command that takes a vault name must reject an invalid one
+// before building a path from it. The instance is given the mnemonic and
+// the passphrase, and moves and removals use --force, so that only the name
+// check stands in the way.
+//
+//nolint:paralleltest // the cases share cmd
+func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
+	before := snapshotStateDir(t, newTwoVaultFs(t))
+
+	mnemonic := testMnemonicBuffer(t)
+	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(passphrase.Destroy)
+
+	cmd := &cobra.Command{}
+
+	// Each command is a format with %q where the vault name goes.
+	commands := []struct {
+		command string
+		run     func(c *cli.Instance, name string) error
+	}{
+		{"vault create %q", func(c *cli.Instance, name string) error {
+			return c.CreateVault(cmd, name)
+		}},
+		{"vault import %q", func(c *cli.Instance, name string) error {
+			return c.VaultImport(cmd, name)
+		}},
+		{"vault select %q", func(c *cli.Instance, name string) error {
+			return c.SelectVault(cmd, name)
+		}},
+		{"vault remove --force %q", func(c *cli.Instance, name string) error {
+			return c.RemoveVault(cmd, name, true)
+		}},
+		{"mv --force %q:x work:x", func(c *cli.Instance, name string) error {
+			return c.MoveSecret(cmd, name+":x", "work:x", true)
+		}},
+		{"mv --force default:x %q:x", func(c *cli.Instance, name string) error {
+			return c.MoveSecret(cmd, "default:x", name+":x", true)
+		}},
+	}
+
+	for _, tt := range commands {
+		for _, name := range []string{"", ".", "..", "a/b"} {
+			t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) {
+				requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name),
+					func(c *cli.Instance) error {
+						c.Mnemonic = mnemonic
+						c.UnlockPassphrase = passphrase
+
+						return tt.run(c, name)
+					})
+			})
+		}
+	}
+}
+
 // TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
 // with a version that is not the current one removes that version and
 // changes nothing else.

internal/cli/secrets.go

@@ -831,9 +831,9 @@ func (cli *Instance) moveSecret(
 			cmd, vlt, srcSecretName, destSecretName, force)
 	}
 
-	// Both vaults must be existing vaults by exact name, so that two
-	// spellings of one vault, such as "work" and "work/", are never taken for
-	// two vaults. A named vault does not become the current vault.
+	// Both vault names must be valid and name existing vaults exactly, so
+	// that two spellings of one vault, such as "work" and "work/", are never
+	// taken for two vaults. A named vault does not become the current vault.
 	srcVault, err := cli.existingVault(srcVaultName)
 	if err != nil {
 		return err
@@ -853,9 +853,15 @@ func (cli *Instance) moveSecret(
 		cmd, srcVault, srcSecretName, destVault, destSecretName, force)
 }
 
-// existingVault returns the vault with the given name, or an error if there
-// is none. Unlike vault.SelectVault, it leaves the current vault as it is.
+// existingVault returns the vault with the given name, or an error if the
+// name is not a valid vault name or there is no such vault. Unlike
+// vault.SelectVault, it leaves the current vault as it is.
 func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
+	err := vault.ValidateVaultName(name)
+	if err != nil {
+		return nil, err
+	}
+
 	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list vaults: %w", err)

internal/cli/vault.go

@@ -433,6 +433,11 @@ func updateVaultImportMetadata(
 // VaultImport imports a mnemonic into a specific vault, holding the state
 // directory lock while importMnemonic runs
 func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
+	err := vault.ValidateVaultName(vaultName)
+	if err != nil {
+		return err
+	}
+
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err
@@ -584,6 +589,11 @@ func (cli *Instance) switchAwayFromVault(
 // RemoveVault removes a vault, holding the state directory lock while
 // removeVault runs
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
+	err := vault.ValidateVaultName(name)
+	if err != nil {
+		return err
+	}
+
 	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
 	if err != nil {
 		return err

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
 	}
@@ -234,13 +235,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"
@@ -254,6 +255,7 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
 		fs:              fs,
 		directory:       vaultDir,
 		derivationIndex: 0,
+		mnemonic:        mnemonic,
 	}
 
 	// Test data
@@ -310,26 +312,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())
 }

internal/vault/errors.go

@@ -17,9 +17,10 @@ var (
 		"derived public key does not match vault: mnemonic may be incorrect",
 	)
 
-	// ErrInvalidVaultName indicates a vault name that does not match the
-	// allowed pattern [a-z0-9.\-_]+. Composed as
-	// "invalid vault name '<name>': must match pattern [a-z0-9.\-_]+".
+	// ErrInvalidVaultName indicates a vault name that breaks the naming
+	// rule: only lowercase ASCII letters, digits, '.', '-' and '_'; not
+	// empty, "." or "..". Composed by ValidateVaultName as
+	// "invalid vault name '<name>': <the rule>".
 	ErrInvalidVaultName = errors.New("invalid vault name")
 
 	// ErrVaultNotFound indicates the named vault does not exist. Composed

internal/vault/management.go

@@ -24,10 +24,12 @@ func init() {
 		})
 }
 
-// isValidVaultName validates vault names according to the format [a-z0-9\.\-\_]+
-// Note: We don't allow slashes in vault names unlike secret names
+// isValidVaultName reports whether name is a valid vault name: only
+// lowercase ASCII letters, digits, '.', '-' and '_', and not empty, "." or
+// "..". With no path separator allowed, a vault is always one directory
+// directly under vaults.d.
 func isValidVaultName(name string) bool {
-	if name == "" {
+	if name == "" || name == "." || name == ".." {
 		return false
 	}
 
@@ -36,6 +38,21 @@ func isValidVaultName(name string) bool {
 	return matched
 }
 
+// ValidateVaultName returns an error wrapping ErrInvalidVaultName when name
+// is not a valid vault name. Call it on the name exactly as the user gave it,
+// before building any path from it.
+func ValidateVaultName(name string) error {
+	if !isValidVaultName(name) {
+		return fmt.Errorf(
+			"%w '%s': only lowercase ASCII letters, digits, '.', '-' and '_' "+
+				"are allowed, and a name must not be empty, '.' or '..'",
+			ErrInvalidVaultName, name,
+		)
+	}
+
+	return nil
+}
+
 // ResolveVaultSymlink reads the currentvault file to get the path to the current vault
 // The file contains just the vault name (e.g., "default")
 func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
@@ -203,14 +220,11 @@ func CreateVault(
 ) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
-	// Validate vault name
-	if !isValidVaultName(name) {
+	err := ValidateVaultName(name)
+	if err != nil {
 		secret.Debug("Invalid vault name provided", "vault_name", name)
 
-		return nil, fmt.Errorf(
-			"%w '%s': must match pattern [a-z0-9.\\-_]+",
-			ErrInvalidVaultName, name,
-		)
+		return nil, err
 	}
 
 	secret.Debug("Vault name validation passed", "vault_name", name)
@@ -292,14 +306,11 @@ func CreateVault(
 func SelectVault(fs afero.Fs, stateDir string, name string) error {
 	secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir)
 
-	// Validate vault name
-	if !isValidVaultName(name) {
+	err := ValidateVaultName(name)
+	if err != nil {
 		secret.Debug("Invalid vault name provided", "vault_name", name)
 
-		return fmt.Errorf(
-			"%w '%s': must match pattern [a-z0-9.\\-_]+",
-			ErrInvalidVaultName, name,
-		)
+		return err
 	}
 
 	secret.Debug("Vault name validation passed", "vault_name", name)