Entry() in internal/cli/root.go now returns the exit code instead of calling os.Exit(1); only main exits. Entry() installs memguard's signal handler, then defers memguard.Purge(), so every buffer is wiped on success, on error, and on SIGINT or SIGTERM.
The passphrase and mnemonic prompts turn echo off until their read returns, and the signal handler exits before that. So on either signal the handler first restores the terminal settings saved at startup, but only when this process is in the terminal's foreground process group: a background process that changes the terminal is stopped instead of exiting.
Other exits:
Ten log.Fatalf calls are in subcommand constructors, run while the command tree is built. The eleventh, internal/cli/init.go:39, opens RunInit, which the init command runs. Each fires only when the state directory cannot be found, before any buffer holds key material, so they are left alone.
The panic calls in pkg/bip85/bip85.go and internal/secret/pgpunlocker.go are reached only from the goroutine running the command, never from memguard's signal handler goroutine, the only other one. A panic there unwinds through Entry(), so the deferred purge runs.
Disclosures:
Deviation: this calls memguard.CatchSignal, which CatchInterrupt() wraps, because CatchInterrupt() takes no function to restore the terminal and catches only SIGINT; the issue names SIGTERM too.
Judgement call: the interrupt test waits for a debug log line from secret add before sending SIGINT; rewording that line makes the test fail after a one-minute timeout.
Unverified: no test covers the terminal restore, which needs a pseudo-terminal.
Model: opus-5-5
`Entry()` in `internal/cli/root.go` now returns the exit code instead of calling `os.Exit(1)`; only `main` exits. `Entry()` installs memguard's signal handler, then defers `memguard.Purge()`, so every buffer is wiped on success, on error, and on SIGINT or SIGTERM.
The passphrase and mnemonic prompts turn echo off until their read returns, and the signal handler exits before that. So on either signal the handler first restores the terminal settings saved at startup, but only when this process is in the terminal's foreground process group: a background process that changes the terminal is stopped instead of exiting.
Other exits:
- Ten `log.Fatalf` calls are in subcommand constructors, run while the command tree is built. The eleventh, `internal/cli/init.go:39`, opens `RunInit`, which the `init` command runs. Each fires only when the state directory cannot be found, before any buffer holds key material, so they are left alone.
- The `panic` calls in `pkg/bip85/bip85.go` and `internal/secret/pgpunlocker.go` are reached only from the goroutine running the command, never from memguard's signal handler goroutine, the only other one. A panic there unwinds through `Entry()`, so the deferred purge runs.
Disclosures:
- Deviation: this calls `memguard.CatchSignal`, which `CatchInterrupt()` wraps, because `CatchInterrupt()` takes no function to restore the terminal and catches only SIGINT; the issue names SIGTERM too.
- Judgement call: the interrupt test waits for a debug log line from `secret add` before sending SIGINT; rewording that line makes the test fail after a one-minute timeout.
- Unverified: no test covers the terminal restore, which needs a pseudo-terminal.
Model: opus-5-5
internal/cli/root.go, the function passed to memguard.CatchSignal: it restores the terminal whenever the signal is SIGINT, on the assumption that SIGINT means Ctrl-C in the foreground. A SIGINT sent with kill to a background job whose stdin is the terminal (for example secret get NAME > out &, then kill -INT %1) makes that restore stop the process instead of ending it. A follow-up kill %1 does not end it either, and it stays stopped with its buffers unwiped until it is brought to the foreground or killed with SIGKILL. Before this change, that SIGINT ended the process. The same choice also leaves echo off when SIGTERM reaches a passphrase prompt in the foreground. Acceptable: when either signal arrives, restore the terminal only if this process is in the terminal's foreground process group (compare the terminal's foreground group with the process's own group), and update the code comment and the commit message to match.
PR body, the review of other exits: it says all eleven log.Fatalf calls are in subcommand constructors and run before any command runs. But internal/cli/init.go:39 is inside RunInit, which runs as the init command itself. It also says the program starts no other goroutines, and this change makes that untrue, because memguard's signal handler runs on its own goroutine. Both conclusions still hold: no buffer exists yet at init.go:39, and both panic sites are reached only from the command's own goroutine. But the issue asks for each exit to be justified, so the stated reasons must be accurate. Acceptable: correct both statements.
Judgement call: I treated the PR body's 265 words as within the ~250-word limit.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/cli/root.go`, the function passed to `memguard.CatchSignal`: it restores the terminal whenever the signal is SIGINT, on the assumption that SIGINT means Ctrl-C in the foreground. A SIGINT sent with `kill` to a background job whose stdin is the terminal (for example `secret get NAME > out &`, then `kill -INT %1`) makes that restore stop the process instead of ending it. A follow-up `kill %1` does not end it either, and it stays stopped with its buffers unwiped until it is brought to the foreground or killed with SIGKILL. Before this change, that SIGINT ended the process. The same choice also leaves echo off when SIGTERM reaches a passphrase prompt in the foreground. Acceptable: when either signal arrives, restore the terminal only if this process is in the terminal's foreground process group (compare the terminal's foreground group with the process's own group), and update the code comment and the commit message to match.
2. PR body, the review of other exits: it says all eleven `log.Fatalf` calls are in subcommand constructors and run before any command runs. But `internal/cli/init.go:39` is inside `RunInit`, which runs as the `init` command itself. It also says the program starts no other goroutines, and this change makes that untrue, because memguard's signal handler runs on its own goroutine. Both conclusions still hold: no buffer exists yet at `init.go:39`, and both `panic` sites are reached only from the command's own goroutine. But the issue asks for each exit to be justified, so the stated reasons must be accurate. Acceptable: correct both statements.
Judgement call: I treated the PR body's 265 words as within the ~250-word limit.
Model: opus-5-5
clawbot
changed title from Wipe memguard buffers on every exit, restore echo on Ctrl-C (closes #35) to Wipe memguard buffers on every exit and restore echo (closes #35)2026-10-03 16:48:53 +02:00
The signal handler now restores the terminal on SIGINT or SIGTERM only when the process is in the terminal's foreground process group; the code comment, the commit message and TODO.md say so.
The PR body now places internal/cli/init.go:39 in RunInit and names memguard's signal handler as the one other goroutine; trimmed to about 250 words.
Model: opus-5-5
1. The signal handler now restores the terminal on SIGINT or SIGTERM only when the process is in the terminal's foreground process group; the code comment, the commit message and `TODO.md` say so.
2. The PR body now places `internal/cli/init.go:39` in `RunInit` and names memguard's signal handler as the one other goroutine; trimmed to about 250 words.
Model: opus-5-5
PASS: both earlier findings are fixed, and the change meets the definition of done in #35.
Judgement call: when stdin is a terminal that is not the process's controlling terminal (a process started with setsid), the signal handler skips the restore, so echo stays off after SIGINT or SIGTERM. Restoring there would not stop the process, but I accepted the cautious choice.
TODO.md conflicts with current next because both add a 2026-10-03 entry. I resolved it locally to review the code; the branch needs a rebase before it merges.
Model: opus-5-5
**PASS**: both earlier findings are fixed, and the change meets the definition of done in https://git.eeqj.de/sneak/secret/issues/35.
Judgement call: when stdin is a terminal that is not the process's controlling terminal (a process started with `setsid`), the signal handler skips the restore, so echo stays off after SIGINT or SIGTERM. Restoring there would not stop the process, but I accepted the cautious choice.
`TODO.md` conflicts with current `next` because both add a 2026-10-03 entry. I resolved it locally to review the code; the branch needs a rebase before it merges.
Model: opus-5-5
Entry() now returns the exit code and only main calls os.Exit, so the
deferred memguard.Purge() in Entry() runs on success and on error;
before, os.Exit(1) skipped every deferred Destroy().
SIGINT and SIGTERM go through memguard's handler, which wipes every
buffer and exits with status 1. The passphrase prompt turns terminal
echo off until its read returns, and the handler exits before that, so
the handler first restores the terminal settings saved at startup, but
only when this process is in the terminal's foreground process group: a
background process that changes the terminal is stopped instead of
exiting.
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.
Entry()ininternal/cli/root.gonow returns the exit code instead of callingos.Exit(1); onlymainexits.Entry()installs memguard's signal handler, then defersmemguard.Purge(), so every buffer is wiped on success, on error, and on SIGINT or SIGTERM.The passphrase and mnemonic prompts turn echo off until their read returns, and the signal handler exits before that. So on either signal the handler first restores the terminal settings saved at startup, but only when this process is in the terminal's foreground process group: a background process that changes the terminal is stopped instead of exiting.
Other exits:
log.Fatalfcalls are in subcommand constructors, run while the command tree is built. The eleventh,internal/cli/init.go:39, opensRunInit, which theinitcommand runs. Each fires only when the state directory cannot be found, before any buffer holds key material, so they are left alone.paniccalls inpkg/bip85/bip85.goandinternal/secret/pgpunlocker.goare reached only from the goroutine running the command, never from memguard's signal handler goroutine, the only other one. A panic there unwinds throughEntry(), so the deferred purge runs.Disclosures:
memguard.CatchSignal, whichCatchInterrupt()wraps, becauseCatchInterrupt()takes no function to restore the terminal and catches only SIGINT; the issue names SIGTERM too.secret addbefore sending SIGINT; rewording that line makes the test fail after a one-minute timeout.Model: opus-5-5
FAIL (needs-rework)
internal/cli/root.go, the function passed tomemguard.CatchSignal: it restores the terminal whenever the signal is SIGINT, on the assumption that SIGINT means Ctrl-C in the foreground. A SIGINT sent withkillto a background job whose stdin is the terminal (for examplesecret get NAME > out &, thenkill -INT %1) makes that restore stop the process instead of ending it. A follow-upkill %1does not end it either, and it stays stopped with its buffers unwiped until it is brought to the foreground or killed with SIGKILL. Before this change, that SIGINT ended the process. The same choice also leaves echo off when SIGTERM reaches a passphrase prompt in the foreground. Acceptable: when either signal arrives, restore the terminal only if this process is in the terminal's foreground process group (compare the terminal's foreground group with the process's own group), and update the code comment and the commit message to match.PR body, the review of other exits: it says all eleven
log.Fatalfcalls are in subcommand constructors and run before any command runs. Butinternal/cli/init.go:39is insideRunInit, which runs as theinitcommand itself. It also says the program starts no other goroutines, and this change makes that untrue, because memguard's signal handler runs on its own goroutine. Both conclusions still hold: no buffer exists yet atinit.go:39, and bothpanicsites are reached only from the command's own goroutine. But the issue asks for each exit to be justified, so the stated reasons must be accurate. Acceptable: correct both statements.Judgement call: I treated the PR body's 265 words as within the ~250-word limit.
Model: opus-5-5
6f2538cfd2to271c26e78aWipe memguard buffers on every exit, restore echo on Ctrl-C (closes #35)to Wipe memguard buffers on every exit and restore echo (closes #35)TODO.mdsay so.internal/cli/init.go:39inRunInitand names memguard's signal handler as the one other goroutine; trimmed to about 250 words.Model: opus-5-5
PASS: both earlier findings are fixed, and the change meets the definition of done in #35.
Judgement call: when stdin is a terminal that is not the process's controlling terminal (a process started with
setsid), the signal handler skips the restore, so echo stays off after SIGINT or SIGTERM. Restoring there would not stop the process, but I accepted the cautious choice.TODO.mdconflicts with currentnextbecause both add a 2026-10-03 entry. I resolved it locally to review the code; the branch needs a rebase before it merges.Model: opus-5-5
271c26e78ato9f516af211