Pass the mnemonic in memory instead of the environment, and unset secret environment variables after reading #60

New Issue

2026-10-03T14:17:00+02:00

clawbot commented

2026-10-03 14:17:00 +02:00

Split from #42. That issue asked for this half to go into its own unit if it grew beyond a tight diff, and it did. The first half, the PGP unlocker panic, is fixed separately.

Problem

setMnemonicEnv (internal/cli/vault.go, used from vault.go and init.go) puts the mnemonic into the process environment while a vault is being created. During that time it can be read from /proc/<pid>/environ by any process running as the same user, and every child process inherits it, gpg included. SB_SECRET_MNEMONIC and SB_UNLOCK_PASSPHRASE are read in many places and never cleared. The README describes them as conveniences and gives no warning.

Findings

No child process needs the mnemonic in its environment. The only child processes are gpg and gpgconf, and neither reads these variables. The mnemonic goes through the environment only so that vault.CreateVault (processMnemonicForVault in internal/vault/management.go) can read it back with os.Getenv.
Dropping setMnemonicEnv means vault.CreateVault takes the mnemonic as a *memguard.LockedBuffer (nil when there is none). Callers: internal/cli/init.go, internal/cli/vault.go, and 20 test call sites in 9 test files. Most of those tests set SB_SECRET_MNEMONIC before the call.
The two variables are read at 13 places in 10 files: internal/cli/{init,vault,crypto,unlockers}.go, internal/secret/{secret,passphraseunlocker,keychainunlocker,seunlocker_darwin}.go, and internal/vault/{vault,management}.go. Some commands read the same variable twice. For example, secret get checks SB_SECRET_MNEMONIC in internal/cli/crypto.go and reads it again in internal/secret/secret.go. If the first read unset the variable, the second read would find nothing. Each variable therefore has to be read once per command into a locked buffer and that buffer passed down. Adding an unset call at each existing read site would not work.

Definition of done (carried over from #42)

setMnemonicEnv no longer round-trips the mnemonic through the process environment. Pass it in memory instead, as a *memguard.LockedBuffer argument through the call chain.
Environment variables holding secrets are unset immediately after being read into a locked buffer, at every read site.
README gains an explicit warning under Environment Variables that these variables are visible to other processes running as the same user, are inherited by child processes including gpg, and end up in shell history and CI logs, with the interactive prompt named as the safer default.
Test: the mnemonic is absent from the environment after vault creation.
TODO.md updated in the same commit.

Implementation requirement (carried over)

Unsetting an environment variable does not reliably erase it from the process's memory, because the C environ block may keep the bytes. Unset it anyway, because unsetting stops inheritance and /proc exposure. Comments and docs must not describe unsetting as wiping the value.

Model: opus-5-5

Split from https://git.eeqj.de/sneak/secret/issues/42. That issue asked for this half to go into its own unit if it grew beyond a tight diff, and it did. The first half, the PGP unlocker panic, is fixed separately. ## Problem `setMnemonicEnv` (`internal/cli/vault.go`, used from `vault.go` and `init.go`) puts the mnemonic into the process environment while a vault is being created. During that time it can be read from `/proc/<pid>/environ` by any process running as the same user, and every child process inherits it, `gpg` included. `SB_SECRET_MNEMONIC` and `SB_UNLOCK_PASSPHRASE` are read in many places and never cleared. The README describes them as conveniences and gives no warning. ## Findings - No child process needs the mnemonic in its environment. The only child processes are `gpg` and `gpgconf`, and neither reads these variables. The mnemonic goes through the environment only so that `vault.CreateVault` (`processMnemonicForVault` in `internal/vault/management.go`) can read it back with `os.Getenv`. - Dropping `setMnemonicEnv` means `vault.CreateVault` takes the mnemonic as a `*memguard.LockedBuffer` (nil when there is none). Callers: `internal/cli/init.go`, `internal/cli/vault.go`, and 20 test call sites in 9 test files. Most of those tests set `SB_SECRET_MNEMONIC` before the call. - The two variables are read at 13 places in 10 files: `internal/cli/{init,vault,crypto,unlockers}.go`, `internal/secret/{secret,passphraseunlocker,keychainunlocker,seunlocker_darwin}.go`, and `internal/vault/{vault,management}.go`. Some commands read the same variable twice. For example, `secret get` checks `SB_SECRET_MNEMONIC` in `internal/cli/crypto.go` and reads it again in `internal/secret/secret.go`. If the first read unset the variable, the second read would find nothing. Each variable therefore has to be read once per command into a locked buffer and that buffer passed down. Adding an unset call at each existing read site would not work. ## Definition of done (carried over from https://git.eeqj.de/sneak/secret/issues/42) - `setMnemonicEnv` no longer round-trips the mnemonic through the process environment. Pass it in memory instead, as a `*memguard.LockedBuffer` argument through the call chain. - Environment variables holding secrets are unset immediately after being read into a locked buffer, at every read site. - README gains an explicit warning under Environment Variables that these variables are visible to other processes running as the same user, are inherited by child processes including `gpg`, and end up in shell history and CI logs, with the interactive prompt named as the safer default. - Test: the mnemonic is absent from the environment after vault creation. - `TODO.md` updated in the same commit. ## Implementation requirement (carried over) Unsetting an environment variable does not reliably erase it from the process's memory, because the C environ block may keep the bytes. Unset it anyway, because unsetting stops inheritance and `/proc` exposure. Comments and docs must not describe unsetting as wiping the value. Model: opus-5-5

clawbot referenced this issue from a commit

2026-10-03 14:17:08 +02:00

Keep unlocker list working when unlocker metadata is corrupt (closes #42)

clawbot referenced this issue

2026-10-03 14:17:18 +02:00

Keep unlocker list working when unlocker metadata is corrupt (closes #42) #61

clawbot referenced this issue

2026-10-03 14:17:25 +02:00

PGPUnlocker.GetID panics on corrupt metadata, and setMnemonicEnv re-exports the mnemonic #42

Sign in to join this conversation.