With SB_UNLOCK_PASSPHRASE set, secret unlocker add passphrase gave the new unlocker the passphrase in the variable without asking, so someone who set it only to unlock the vault kept the old passphrase and was not told. The command now asks on the terminal for the new passphrase, twice, and uses the variable to unlock the vault. With no controlling terminal, as under cron or in a CI job, the new unlocker takes the variable's passphrase, and the output says so. The README's secret unlocker add section says the same.
"A terminal is present" means what #144 made it mean: /dev/tty opens. secret.ErrNoTerminal is exported so the command can tell that case from other failures to read, such as nothing typed or a confirmation that does not match, which still fail the command.
Two tests run the built binary with the variable set. At a pseudo-terminal it must ask for the new passphrase, and the vault must then open with the one typed. With no terminal, its output must name the variable.
Disclosures:
The in-process tests that add a passphrase unlocker with the variable set rely on the Docker test phase having no controlling terminal; run at a terminal outside Docker, they would stop and ask.
Judgement call from the plan on the issue: a script run from a terminal with the variable set now stops at the prompt for the new passphrase.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/secret/issues/154.
With `SB_UNLOCK_PASSPHRASE` set, `secret unlocker add passphrase` gave the new unlocker the passphrase in the variable without asking, so someone who set it only to unlock the vault kept the old passphrase and was not told. The command now asks on the terminal for the new passphrase, twice, and uses the variable to unlock the vault. With no controlling terminal, as under cron or in a CI job, the new unlocker takes the variable's passphrase, and the output says so. The README's `secret unlocker add` section says the same.
"A terminal is present" means what https://git.eeqj.de/sneak/secret/issues/144 made it mean: `/dev/tty` opens. `secret.ErrNoTerminal` is exported so the command can tell that case from other failures to read, such as nothing typed or a confirmation that does not match, which still fail the command.
Two tests run the built binary with the variable set. At a pseudo-terminal it must ask for the new passphrase, and the vault must then open with the one typed. With no terminal, its output must name the variable.
Disclosures:
- The in-process tests that add a passphrase unlocker with the variable set rely on the Docker test phase having no controlling terminal; run at a terminal outside Docker, they would stop and ask.
- Judgement call from the plan on the issue: a script run from a terminal with the variable set now stops at the prompt for the new passphrase.
Model: opus-5-5
README.md lines 245-246 and internal/cli/unlockers.go lines 443-444 say the new passphrase is asked for "since the variable only unlocks the vault". The next sentence in each place says that with no terminal the new unlocker takes its passphrase from SB_UNLOCK_PASSPHRASE, so that reason is false in that case and the two sentences contradict each other. Acceptable: in both places, say what the command does without the reason, for example "asks on the terminal for the new passphrase even when SB_UNLOCK_PASSPHRASE is set, and uses the variable to unlock the vault".
Model: opus-5-5
`README.md` lines 245-246 and `internal/cli/unlockers.go` lines 443-444 say the new passphrase is asked for "since the variable only unlocks the vault". The next sentence in each place says that with no terminal the new unlocker takes its passphrase from `SB_UNLOCK_PASSPHRASE`, so that reason is false in that case and the two sentences contradict each other. Acceptable: in both places, say what the command does without the reason, for example "asks on the terminal for the new passphrase even when `SB_UNLOCK_PASSPHRASE` is set, and uses the variable to unlock the vault".
Model: opus-5-5
The README and the comment in addPassphraseUnlocker no longer give the false reason; they say the command asks for the new passphrase even when SB_UNLOCK_PASSPHRASE is set and uses the variable to unlock the vault. The commit message and the PR body had the same "only" wording and now match.
Model: opus-5-5
Rework:
- The README and the comment in `addPassphraseUnlocker` no longer give the false reason; they say the command asks for the new passphrase even when `SB_UNLOCK_PASSPHRASE` is set and uses the variable to unlock the vault. The commit message and the PR body had the same "only" wording and now match.
Model: opus-5-5
internal/cli/passphrase_prompt_test.go lines 220-227, in TestUnlockerAddAsksForNewPassphrase: the test opens the vault inside the test process with the typed passphrase. The binary wrote that unlocker at age's full scrypt work factor, and the test process runs under the race detector, so this one check takes about six seconds and raises the test process's peak memory from about 100 MB to about 900 MB. That makes it the slowest test in the suite, nearly twice the next one, in a suite already over its 20-second target, while the other tests keep passphrase work cheap (secret.ScryptWorkFactor, set in TestMain). Acceptable: check that the typed passphrase opens the vault by running the built binary, which has no race detector, with SB_UNLOCK_PASSPHRASE set to the typed passphrase and a command that has to unlock the vault, such as secret get of a secret added before the run.
Model: opus-5-5
- `internal/cli/passphrase_prompt_test.go` lines 220-227, in `TestUnlockerAddAsksForNewPassphrase`: the test opens the vault inside the test process with the typed passphrase. The binary wrote that unlocker at age's full scrypt work factor, and the test process runs under the race detector, so this one check takes about six seconds and raises the test process's peak memory from about 100 MB to about 900 MB. That makes it the slowest test in the suite, nearly twice the next one, in a suite already over its 20-second target, while the other tests keep passphrase work cheap (`secret.ScryptWorkFactor`, set in `TestMain`). Acceptable: check that the typed passphrase opens the vault by running the built binary, which has no race detector, with `SB_UNLOCK_PASSPHRASE` set to the typed passphrase and a command that has to unlock the vault, such as `secret get` of a secret added before the run.
Model: opus-5-5
TestUnlockerAddAsksForNewPassphrase adds a secret before the run, then checks the typed passphrase by running the built binary's secret get with SB_UNLOCK_PASSPHRASE set to it. The test process no longer opens the vault.
Judgement call: the no-terminal run of the binary is now a helper, runWithoutTerminal, which TestUnlockerAddWithoutTerminalSaysPassphraseIsVariable uses too; written out inline, the first test went over the linter's function length limit.
Model: opus-5-5
Rework:
- `TestUnlockerAddAsksForNewPassphrase` adds a secret before the run, then checks the typed passphrase by running the built binary's `secret get` with `SB_UNLOCK_PASSPHRASE` set to it. The test process no longer opens the vault.
Judgement call: the no-terminal run of the binary is now a helper, `runWithoutTerminal`, which `TestUnlockerAddWithoutTerminalSaysPassphraseIsVariable` uses too; written out inline, the first test went over the linter's function length limit.
Model: opus-5-5
With SB_UNLOCK_PASSPHRASE set, `secret unlocker add passphrase` gave the
new unlocker the passphrase in the variable without asking. It now asks
on the terminal for the new passphrase and uses the variable to unlock
the vault. With no controlling terminal the new unlocker takes the
variable's passphrase, and the output says so.
secret.ErrNoTerminal is exported so that the command can tell a missing
terminal from other failures to read, such as nothing typed or a
mismatched confirmation, which still fail the command.
The in-process tests that add a passphrase unlocker with the variable
set rely on the Docker test phase having no controlling terminal; at a
terminal they would ask.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #154.
With
SB_UNLOCK_PASSPHRASEset,secret unlocker add passphrasegave the new unlocker the passphrase in the variable without asking, so someone who set it only to unlock the vault kept the old passphrase and was not told. The command now asks on the terminal for the new passphrase, twice, and uses the variable to unlock the vault. With no controlling terminal, as under cron or in a CI job, the new unlocker takes the variable's passphrase, and the output says so. The README'ssecret unlocker addsection says the same."A terminal is present" means what #144 made it mean:
/dev/ttyopens.secret.ErrNoTerminalis exported so the command can tell that case from other failures to read, such as nothing typed or a confirmation that does not match, which still fail the command.Two tests run the built binary with the variable set. At a pseudo-terminal it must ask for the new passphrase, and the vault must then open with the one typed. With no terminal, its output must name the variable.
Disclosures:
Model: opus-5-5
f22a60eb23to3599a6fe943599a6fe94tob4b9499df6README.mdlines 245-246 andinternal/cli/unlockers.golines 443-444 say the new passphrase is asked for "since the variable only unlocks the vault". The next sentence in each place says that with no terminal the new unlocker takes its passphrase fromSB_UNLOCK_PASSPHRASE, so that reason is false in that case and the two sentences contradict each other. Acceptable: in both places, say what the command does without the reason, for example "asks on the terminal for the new passphrase even whenSB_UNLOCK_PASSPHRASEis set, and uses the variable to unlock the vault".Model: opus-5-5
b4b9499df6to22bda7b100Rework:
addPassphraseUnlockerno longer give the false reason; they say the command asks for the new passphrase even whenSB_UNLOCK_PASSPHRASEis set and uses the variable to unlock the vault. The commit message and the PR body had the same "only" wording and now match.Model: opus-5-5
22bda7b100to94dca4e80f94dca4e80fto42f49b443einternal/cli/passphrase_prompt_test.golines 220-227, inTestUnlockerAddAsksForNewPassphrase: the test opens the vault inside the test process with the typed passphrase. The binary wrote that unlocker at age's full scrypt work factor, and the test process runs under the race detector, so this one check takes about six seconds and raises the test process's peak memory from about 100 MB to about 900 MB. That makes it the slowest test in the suite, nearly twice the next one, in a suite already over its 20-second target, while the other tests keep passphrase work cheap (secret.ScryptWorkFactor, set inTestMain). Acceptable: check that the typed passphrase opens the vault by running the built binary, which has no race detector, withSB_UNLOCK_PASSPHRASEset to the typed passphrase and a command that has to unlock the vault, such assecret getof a secret added before the run.Model: opus-5-5
42f49b443eto0982d45868Rework:
TestUnlockerAddAsksForNewPassphraseadds a secret before the run, then checks the typed passphrase by running the built binary'ssecret getwithSB_UNLOCK_PASSPHRASEset to it. The test process no longer opens the vault.Judgement call: the no-terminal run of the binary is now a helper,
runWithoutTerminal, whichTestUnlockerAddWithoutTerminalSaysPassphraseIsVariableuses too; written out inline, the first test went over the linter's function length limit.Model: opus-5-5
0982d45868to7a80f4b882Review passed.
Model: opus-5-5