diff --git a/TODO.md b/TODO.md index 281cbac..6f3278e 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,13 @@ 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; when the process is + in the terminal's foreground process group 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..c911991 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -4,17 +4,38 @@ import ( "os" "git.eeqj.de/sneak/secret/internal/secret" + "github.com/awnumar/memguard" "github.com/spf13/cobra" + "golang.org/x/sys/unix" + "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 a signal there would leave echo off. + // Only a process in the terminal's foreground process group may reset + // it: one in the background that tries is stopped instead of exiting. + terminalState, terminalErr := term.GetState(unix.Stdin) - err := cmd.Execute() + memguard.CatchSignal(func(os.Signal) { + foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP) + if terminalErr == nil && err == nil && foreground == unix.Getpgrp() { + _ = term.Restore(unix.Stdin, terminalState) + } + }, os.Interrupt, unix.SIGTERM) + + defer memguard.Purge() + + err := newRootCmd().Execute() if err != nil { - os.Exit(1) + return 1 } + + return 0 } func newRootCmd() *cobra.Command {