Set the time zone in main and run Entry in its own test process (closes #192)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
cli.Entry() set time.Local to UTC and ended with memguard.Purge(), and TestEntryWipesBuffersAndReturnsExitCode ran it in the same process as the other tests. A timer left by an earlier test could read time.Local as it changed, which the race detector reported on some runs, and the purge could destroy a buffer still in use, the likeliest cause of a crash seen on another run. main() in cmd/secret now sets time.Local before it calls cli.Entry(); the test that runs the built binary with TZ=America/New_York still covers it. The Entry test runs cli.Entry() in a copy of the test binary started with SECRET_TEST_RUN_ENTRY set, which init picks up so that TestMain does not run there. Model: opus-5-5
This commit is contained in:
@@ -18,6 +18,15 @@ https://git.eeqj.de/sneak/secret/milestone/12
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-15: `make test` no longer fails on some runs in `internal/cli`
|
||||||
|
(https://git.eeqj.de/sneak/secret/issues/192). `cli.Entry()` set `time.Local`
|
||||||
|
to UTC and ended by wiping every memguard buffer, and a test ran it in the
|
||||||
|
same process as the other tests. A timer left by an earlier test could read
|
||||||
|
`time.Local` as it changed, which the race detector reported, and the wipe
|
||||||
|
could destroy a buffer still in use, the likeliest cause of a crash of the
|
||||||
|
test process on another run. `main()` in `cmd/secret` now sets `time.Local`
|
||||||
|
before it calls `cli.Entry()`, and that test runs `cli.Entry()` in a process
|
||||||
|
of its own.
|
||||||
- 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the
|
- 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the
|
||||||
prompt, gives the error `wrong passphrase for unlocker <id>`
|
prompt, gives the error `wrong passphrase for unlocker <id>`
|
||||||
(https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no
|
(https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no
|
||||||
|
|||||||
@@ -3,10 +3,16 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
|
"time"
|
||||||
|
|
||||||
"sneak.berlin/go/secret/internal/cli"
|
"sneak.berlin/go/secret/internal/cli"
|
||||||
)
|
)
|
||||||
|
|
||||||
func main() {
|
func main() {
|
||||||
|
// Every time secret prints or writes is in UTC, whatever TZ or the
|
||||||
|
// system's time zone is set to. It is set here, before cli.Entry starts
|
||||||
|
// any goroutine that could read time.Local while it changes.
|
||||||
|
time.Local = time.UTC //nolint:gosmopolitan // this makes local time UTC
|
||||||
|
|
||||||
os.Exit(cli.Entry())
|
os.Exit(cli.Entry())
|
||||||
}
|
}
|
||||||
|
|||||||
+51
-13
@@ -3,6 +3,7 @@ package cli_test
|
|||||||
import (
|
import (
|
||||||
"bufio"
|
"bufio"
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -16,29 +17,66 @@ import (
|
|||||||
"sneak.berlin/go/secret/internal/secret"
|
"sneak.berlin/go/secret/internal/secret"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Entry must return its exit code rather than exit, so that its deferred
|
const (
|
||||||
// memguard purge runs on the success and the error path alike.
|
// envRunEntry, when set, makes this test binary run cli.Entry with its
|
||||||
//
|
// own arguments and exit, instead of running the tests.
|
||||||
//nolint:paralleltest // sets os.Args, and Entry wipes every buffer in the process
|
envRunEntry = "SECRET_TEST_RUN_ENTRY"
|
||||||
func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) {
|
|
||||||
savedArgs := os.Args
|
|
||||||
|
|
||||||
t.Cleanup(func() { os.Args = savedArgs })
|
// bufferWiped is what the test binary prints, run with envRunEntry set,
|
||||||
|
// when cli.Entry wiped the buffer made before it ran.
|
||||||
|
bufferWiped = "Entry wiped the buffer made before it ran"
|
||||||
|
)
|
||||||
|
|
||||||
|
// With envRunEntry set, the test binary makes a buffer, runs cli.Entry,
|
||||||
|
// says whether the buffer is wiped, and exits with Entry's exit code. This
|
||||||
|
// is done in init, not in a test, so that TestMain does not run in that
|
||||||
|
// process: it would build the secret binary again, and remove it at the end,
|
||||||
|
// while the other tests run it.
|
||||||
|
//
|
||||||
|
//nolint:gochecknoinits // must run before TestMain, as above
|
||||||
|
func init() {
|
||||||
|
if os.Getenv(envRunEntry) == "" {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
buf := memguard.NewBufferFromBytes([]byte("key material"))
|
||||||
|
code := cli.Entry()
|
||||||
|
|
||||||
|
if !buf.IsAlive() {
|
||||||
|
fmt.Fprintln(os.Stderr, bufferWiped)
|
||||||
|
}
|
||||||
|
|
||||||
|
os.Exit(code)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Entry must return its exit code rather than exit, so that its deferred
|
||||||
|
// memguard purge runs on the success and the error path alike. Entry wipes
|
||||||
|
// every memguard buffer in the process, including those other tests are
|
||||||
|
// using, so it runs in a process of its own: this test binary, run again
|
||||||
|
// with envRunEntry set.
|
||||||
|
func TestEntryWipesBuffersAndReturnsExitCode(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
args []string
|
args []string
|
||||||
exitCode int
|
exitCode int
|
||||||
}{
|
}{
|
||||||
{args: []string{"secret", "--help"}, exitCode: 0},
|
{args: []string{"--help"}, exitCode: 0},
|
||||||
{args: []string{"secret", "no-such-command"}, exitCode: 1},
|
{args: []string{"no-such-command"}, exitCode: 1},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
buf := memguard.NewBufferFromBytes([]byte("key material"))
|
//nolint:gosec // G204: test runs its own test binary
|
||||||
os.Args = tt.args
|
cmd := exec.CommandContext(t.Context(), os.Args[0], tt.args...)
|
||||||
|
cmd.Env = append(os.Environ(), envRunEntry+"=1")
|
||||||
|
|
||||||
assert.Equal(t, tt.exitCode, cli.Entry(), "exit code for %v", tt.args)
|
output, err := cmd.CombinedOutput()
|
||||||
assert.False(t, buf.IsAlive(), "Entry left a buffer unwiped for %v", tt.args)
|
require.NotNil(t, cmd.ProcessState, "%v did not run: %v", tt.args, err)
|
||||||
|
|
||||||
|
assert.Equal(t, tt.exitCode, cmd.ProcessState.ExitCode(),
|
||||||
|
"exit code for %v: %s", tt.args, output)
|
||||||
|
assert.Contains(t, string(output), bufferWiped,
|
||||||
|
"Entry left a buffer unwiped for %v", tt.args)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -2,7 +2,6 @@ package cli
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
"time"
|
|
||||||
|
|
||||||
"github.com/awnumar/memguard"
|
"github.com/awnumar/memguard"
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
@@ -15,10 +14,6 @@ import (
|
|||||||
// every memguard buffer before it returns, so the caller must do nothing
|
// every memguard buffer before it returns, so the caller must do nothing
|
||||||
// but exit with the code.
|
// but exit with the code.
|
||||||
func Entry() int {
|
func Entry() int {
|
||||||
// Every time secret prints or writes is in UTC, whatever TZ or the
|
|
||||||
// system's time zone is set to.
|
|
||||||
time.Local = time.UTC //nolint:gosmopolitan // this makes local time UTC
|
|
||||||
|
|
||||||
raiseLockedMemoryLimit()
|
raiseLockedMemoryLimit()
|
||||||
|
|
||||||
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
|
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
|
||||||
|
|||||||
Reference in New Issue
Block a user