From 6f2538cfd2ab70d92c84afdbd92815050da4dd1c Mon Sep 17 00:00:00 2001 From: sneak Date: Sat, 3 Oct 2026 12:15:47 +0000 Subject: [PATCH] Wipe memguard buffers on every exit, restore echo on Ctrl-C (closes #35) 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 on Ctrl-C the handler first restores the terminal settings saved at startup. It leaves the terminal alone on SIGTERM, which can reach a background process, and changing the terminal from the background would stop the process. Model: opus-5-5 --- TODO.md | 6 +++ cmd/secret/main.go | 8 ++- internal/cli/entry_test.go | 108 +++++++++++++++++++++++++++++++++++++ internal/cli/root.go | 30 +++++++++-- 4 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 internal/cli/entry_test.go diff --git a/TODO.md b/TODO.md index 281cbac..5e597f9 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,12 @@ Bring the repo into policy compliance in one commit: # Completed Steps +- 2026-10-03: Key material is wiped on every exit: `Entry()` returns + the exit code after its deferred `memguard.Purge()` has run, and only + `main` calls `os.Exit`. SIGINT and SIGTERM go through memguard's + handler, which wipes every buffer before exiting; on Ctrl-C it first + restores the terminal settings from startup, so an interrupted + passphrase prompt no longer leaves echo off. - 2026-10-02: A plain `docker build .` builds again: the size tests skip a case that needs more locked memory than the process can lock, and run every case under `script/cibuild`. The image stamps the diff --git a/cmd/secret/main.go b/cmd/secret/main.go index dd909ab..7de717b 100644 --- a/cmd/secret/main.go +++ b/cmd/secret/main.go @@ -1,8 +1,12 @@ // Package main is the entry point for the secret CLI application. package main -import "git.eeqj.de/sneak/secret/internal/cli" +import ( + "os" + + "git.eeqj.de/sneak/secret/internal/cli" +) func main() { - cli.Entry() + os.Exit(cli.Entry()) } diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go new file mode 100644 index 0000000..189b4a4 --- /dev/null +++ b/internal/cli/entry_test.go @@ -0,0 +1,108 @@ +package cli_test + +import ( + "bufio" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "git.eeqj.de/sneak/secret/internal/cli" + "git.eeqj.de/sneak/secret/internal/secret" + "github.com/awnumar/memguard" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Entry must return its exit code rather than exit, so that its deferred +// memguard purge runs on the success and the error path alike. +// +//nolint:paralleltest // sets os.Args, and Entry wipes every buffer in the process +func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) { + savedArgs := os.Args + + t.Cleanup(func() { os.Args = savedArgs }) + + tests := []struct { + args []string + exitCode int + }{ + {args: []string{"secret", "--help"}, exitCode: 0}, + {args: []string{"secret", "no-such-command"}, exitCode: 1}, + } + + for _, tt := range tests { + buf := memguard.NewBufferFromBytes([]byte("key material")) + os.Args = tt.args + + assert.Equal(t, tt.exitCode, cli.Entry(), "exit code for %v", tt.args) + assert.False(t, buf.IsAlive(), "Entry left a buffer unwiped for %v", tt.args) + } +} + +// Ctrl-C while `secret add` waits for the value on stdin must end the +// process through memguard's signal handler, which wipes every buffer and +// exits with status 1, not through Go's default handling, which kills the +// process with the buffers intact. +func TestInterruptExitsThroughMemguard(t *testing.T) { + t.Parallel() + + const waitingForValue = "Reading secret value from stdin" + + ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + defer cancel() + + wd, err := filepath.Abs("../..") + require.NoError(t, err) + + secretPath := filepath.Join(wd, "secret") + env := []string{ + secret.EnvStateDir + "=" + t.TempDir(), + secret.EnvMnemonic + "=" + testMnemonic, + secret.EnvUnlockPassphrase + "=test-passphrase", + "PATH=/usr/bin:/bin", + // The debug log on stderr shows when add starts waiting for the value. + "GODEBUG=berlin.sneak.pkg.secret", + } + + //nolint:gosec // G204: test executes the freshly built secret binary + initCmd := exec.CommandContext(ctx, secretPath, "init") + initCmd.Env = env + + output, err := initCmd.CombinedOutput() + require.NoError(t, err, "init should succeed: %s", output) + + //nolint:gosec // G204: test executes the freshly built secret binary + addCmd := exec.CommandContext(ctx, secretPath, "add", "test/secret") + addCmd.Env = env + + // Held open and never written, so add keeps waiting for the value. + stdin, err := addCmd.StdinPipe() + require.NoError(t, err) + + defer func() { _ = stdin.Close() }() + + stderr, err := addCmd.StderrPipe() + require.NoError(t, err) + require.NoError(t, addCmd.Start()) + + waiting := false + + scanner := bufio.NewScanner(stderr) + for !waiting && scanner.Scan() { + waiting = strings.Contains(scanner.Text(), waitingForValue) + } + + require.True(t, waiting, "add never logged %q", waitingForValue) + require.NoError(t, addCmd.Process.Signal(os.Interrupt)) + + err = addCmd.Wait() + + var exitErr *exec.ExitError + + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, 1, exitErr.ExitCode(), "add ended with %v", err) +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 94ddd13..87f8b7e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -2,19 +2,39 @@ package cli import ( "os" + "syscall" "git.eeqj.de/sneak/secret/internal/secret" + "github.com/awnumar/memguard" "github.com/spf13/cobra" + "golang.org/x/term" ) -// Entry is the entry point for the secret CLI application -func Entry() { - cmd := newRootCmd() +// Entry runs the secret CLI and returns the process exit code. It wipes +// every memguard buffer before it returns, so the caller must do nothing +// but exit with the code. +func Entry() int { + // On SIGINT or SIGTERM memguard runs this function, wipes every buffer + // and exits with status 1. The passphrase prompt turns terminal echo + // off until the read finishes, so Ctrl-C there would leave echo off. + // Ctrl-C means this process is in the terminal's foreground and may + // reset it; doing that from the background would stop the process. + terminalState, terminalErr := term.GetState(syscall.Stdin) - err := cmd.Execute() + memguard.CatchSignal(func(sig os.Signal) { + if sig == os.Interrupt && terminalErr == nil { + _ = term.Restore(syscall.Stdin, terminalState) + } + }, os.Interrupt, syscall.SIGTERM) + + defer memguard.Purge() + + err := newRootCmd().Execute() if err != nil { - os.Exit(1) + return 1 } + + return 0 } func newRootCmd() *cobra.Command {