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 {