fix(summer): mask secret flag values in a delegate's error (WR-10)

This commit is contained in:
Jakub Zych
2026-10-22 04:00:31 +02:00
parent 707372bf65
commit 17c722da46
7 changed files with 147 additions and 65 deletions

View File

@@ -9,9 +9,9 @@ import (
"os/exec"
"os/signal"
"path/filepath"
"strings"
"syscall"
"git.golem15.com/golem15/summercms/internal/argmask"
"git.golem15.com/golem15/summercms/internal/build"
"git.golem15.com/golem15/summercms/internal/dev"
"git.golem15.com/golem15/summercms/modules/bonfire"
@@ -161,13 +161,17 @@ func appStreams(out bonfire.Output, terminal bool) (stdout, stderr io.Writer) {
// runAppCmd runs cmd, the app binary started with argv, forwarding SIGINT
// and SIGTERM to it. When the binary exits with a code, the error carries
// that code (bonfire.WithExitCode) and is marked as already reported.
// that code (bonfire.WithExitCode) and is marked as already reported. The
// error names the command through argmask.Join, with the values of
// secret-looking flags masked, on every branch: a binary that fails to
// start or is ended by a signal gets summer's error block, and a signal
// death keeps exit code 1.
func runAppCmd(cmd *exec.Cmd, argv []string) error {
sigs := make(chan os.Signal, 2)
signal.Notify(sigs, os.Interrupt, syscall.SIGTERM)
defer signal.Stop(sigs)
if err := runChild(cmd, sigs); err != nil {
err = fmt.Errorf("summer: %s: %w", strings.Join(argv, " "), err)
err = fmt.Errorf("summer: %s: %w", argmask.Join(argv), err)
var exit *exec.ExitError
if errors.As(err, &exit) && exit.Exited() {
// The binary printed its own error; summer exits with its

View File

@@ -321,14 +321,15 @@ func exitHelperRunner(t *testing.T, mode string) {
t.Cleanup(func() { appRunner = prev })
}
// TestDelegateExitCodePassthrough (CR-02): summer exits with the app
// binary's exit code and prints no error block of its own; a binary killed
// by a signal gets summer's error block and exit 1.
// TestDelegateExitCodePassthrough (CR-02, WR-10): summer exits with the
// app binary's exit code and prints no error block of its own; a binary
// killed by a signal gets summer's error block, with secret flag values
// masked, and exit 1.
func TestDelegateExitCodePassthrough(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("SIGKILL is POSIX-only")
}
run := func(t *testing.T, mode string) (string, string, int) {
run := func(t *testing.T, mode string, args ...string) (string, string, int) {
t.Helper()
exitHelperRunner(t, mode)
var out, errW syncBuffer
@@ -336,12 +337,12 @@ func TestDelegateExitCodePassthrough(t *testing.T) {
if err != nil {
t.Fatal(err)
}
code := bonfire.Execute(context.Background(), root, []string{"migrate", "--bogus"})
code := bonfire.Execute(context.Background(), root, args)
return out.String(), errW.String(), code
}
t.Run("exit code", func(t *testing.T) {
out, errOut, code := run(t, "exit2")
out, errOut, code := run(t, "exit2", "migrate", "--bogus")
if code != 2 {
t.Errorf("exit %d, want the binary's 2", code)
}
@@ -354,7 +355,7 @@ func TestDelegateExitCodePassthrough(t *testing.T) {
})
t.Run("killed by a signal", func(t *testing.T) {
out, errOut, code := run(t, "sigkill")
out, errOut, code := run(t, "sigkill", "migrate", "--bogus")
if code != bonfire.ExitFailure {
t.Errorf("exit %d, want 1", code)
}
@@ -362,6 +363,60 @@ func TestDelegateExitCodePassthrough(t *testing.T) {
t.Errorf("want one error block naming the signal, got %d:\nstdout %q\nstderr %q", n, out, errOut)
}
})
t.Run("killed by a signal masks secrets", func(t *testing.T) {
out, errOut, code := run(t, "sigkill", "migrate", "--password", "hunter2")
if code != bonfire.ExitFailure {
t.Errorf("exit %d, want 1", code)
}
if n := strings.Count(errOut, " ERROR "); n != 1 || !strings.Contains(errOut, "--password ***") {
t.Errorf("want one error block with the password masked, got %d:\nstdout %q\nstderr %q", n, out, errOut)
}
if strings.Contains(out+errOut, "hunter2") {
t.Errorf("the password reached the output:\nstdout %q\nstderr %q", out, errOut)
}
})
}
// TestRunAppCmdMasksSecrets (WR-10): the error of an app binary killed by a
// signal or failing to start names the command with the values of
// secret-named flags masked, as the wrong-directory Try line does.
func TestRunAppCmdMasksSecrets(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("SIGKILL is POSIX-only")
}
argv := []string{"admin:create", "--email", "a@b.c", "--password", "hunter2", "--api-token=t0k3n", "--client-secret", "-Xk9q"}
check := func(t *testing.T, err error, want ...string) {
t.Helper()
if err == nil {
t.Fatal("runAppCmd returned nil")
}
if errors.Is(err, bonfire.ErrReported) {
t.Errorf("error is marked as reported: %v", err)
}
msg := err.Error()
for _, w := range append(want, "--password ***", "--api-token=***", "--client-secret ***") {
if !strings.Contains(msg, w) {
t.Errorf("error lacks %q: %s", w, msg)
}
}
for _, secret := range []string{"hunter2", "t0k3n", "-Xk9q"} {
if strings.Contains(msg, secret) {
t.Errorf("error repeats %q: %s", secret, msg)
}
}
}
t.Run("killed by a signal", func(t *testing.T) {
cmd := newAppCmd(context.Background(), os.Args[0], ".", []string{"-test.run=^TestExitHelperProcess$"}, io.Discard, io.Discard)
cmd.Env = append(cmd.Env, "SUMMER_EXIT_HELPER=sigkill")
check(t, runAppCmd(cmd, argv), "signal: killed")
})
t.Run("start failure", func(t *testing.T) {
cmd := newAppCmd(context.Background(), filepath.Join(t.TempDir(), "missing"), ".", argv, io.Discard, io.Discard)
check(t, runAppCmd(cmd, argv))
})
}
// TestDelegateStreams (CR-03): off a terminal the app binary's stdout goes

View File

@@ -143,7 +143,7 @@ summer admin:token:create jane --scope journal.posts --json
The binary writes to your terminal directly, so it asks the production question, prints its styled startup summary and animates its spinners as if you had run it yourself. Its error stream stays separate from its output: `summer route:list --json > routes.json` captures only the JSON, while warnings and log lines still reach the terminal on stderr.
When the binary fails, `summer` exits with the binary's exit code (a usage error stays `2`) and prints nothing more: the binary has already printed its error. A binary killed by a signal gets an error block from `summer` and exit code `1`.
When the binary fails, `summer` exits with the binary's exit code (a usage error stays `2`) and prints nothing more: the binary has already printed its error. A binary killed by a signal gets an error block from `summer` and exit code `1`; the block names the command with the values of secret-looking flags (`--password`, `--api-token=...`) masked as `***`.
When `summer` receives SIGINT or SIGTERM while the binary runs, it passes the signal on and waits for the binary to shut down, so stopping `summer serve` with Ctrl+C or `kill` frees its port.

View File

@@ -0,0 +1,63 @@
// Package argmask renders a command line for messages with the values of
// secret-named long flags masked; bonfire's wrong-directory hint and
// summer's delegate errors share it.
package argmask
import "strings"
// secretWords mark a long flag whose value is masked.
var secretWords = []string{"password", "passwd", "secret", "token", "key"}
// secretFlag reports whether the long flag name (without "--") looks like it
// carries a secret.
func secretFlag(name string) bool {
name = strings.ToLower(name)
for _, w := range secretWords {
if strings.Contains(name, w) {
return true
}
}
return false
}
// Join renders args as one line, separated by single spaces and each
// quoted for a shell (Quote). The value of a long flag named like a secret
// becomes ***, in the "--flag=value" form and in the "--flag value" form,
// so a password typed on the command line does not reach stderr and CI
// logs. In the "--flag value" form the next argument is masked whatever its
// first character (pflag takes "-Xk9" as the value too); only "--" is left
// alone. Join cannot see flag types, so a secret-named boolean flag
// over-masks the argument after it, which is the safe side.
func Join(args []string) string {
var b strings.Builder
for i := 0; i < len(args); i++ {
if i > 0 {
b.WriteByte(' ')
}
a := args[i]
name, long := strings.CutPrefix(a, "--")
flag, _, inline := strings.Cut(name, "=")
if !long || flag == "" || !secretFlag(flag) {
b.WriteString(Quote(a))
continue
}
if inline {
b.WriteString(Quote("--"+flag) + "=***")
continue
}
b.WriteString(Quote(a))
if i+1 < len(args) && args[i+1] != "--" {
b.WriteString(" ***")
i++
}
}
return b.String()
}
// Quote single-quotes s when a shell would split or expand it.
func Quote(s string) string {
if s != "" && !strings.ContainsAny(s, " \t\n'\"\\$`*?[]{}()<>|&;#~!") {
return s
}
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
}

View File

@@ -46,7 +46,7 @@ Use cases:
- Hidden commands (`bonfire.Command.Hidden`) run but stay out of the list unless `list --all`.
- JSON catalog: `list --format=json` prints every command, hidden ones flagged, with arguments, options, defaults and examples as a `bonfire.Listing` (`bonfire.Describe` builds it from a command slice).
- Documentation coverage: `bonfire.CheckDocs` reports commands lacking a description, long help, examples, or argument and option descriptions.
- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line.
- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line, also a value that starts with `-`.
- Name checks at boot: two commands with the same name, or a command named `list` or `help`, make `bonfire.NewRoot` fail with `bonfire.ErrDuplicateCommand`; a command flag that reuses a global option's name or shorthand fails with `bonfire.ErrReservedFlag` instead of panicking.
- Injectable streams (`bonfire.NewRootIO`, `bonfire.NewOutput`) so commands can be tested against buffers.
- In-process calls: `bonfire.Call` runs a named command with arguments against any writer (Laravel `Artisan::call`), and `bonfire.Catalog` holds an application binary's final command list so code outside the Cobra root, such as the conga scheduler, can call any registered command.
@@ -191,7 +191,7 @@ Without these variables, color is enabled only when stdout is a terminal. `--ans
## Dependencies
- SummerCMS modules: `sunscreen` (scrubs error messages).
- SummerCMS modules: `sunscreen` (scrubs error messages); the framework-internal package `internal/argmask` (masks secret flag values in the Try line, shared with summer's delegate errors).
- Third-party: `github.com/spf13/cobra`, `github.com/spf13/pflag`, `golang.org/x/term`, `charm.land/lipgloss/v2` (with `lipgloss/v2/table`), `charm.land/bubbletea/v2`, `charm.land/bubbles/v2` (spinner, progress, key), `charm.land/huh/v2`, `charm.land/fang/v2` and `github.com/charmbracelet/colorprofile` (decided in `.planning/notes/2026-10-21-console-charm-stack.md`).
- Standard library: `bufio`, `context`, `encoding/json`, `errors`, `fmt`, `io`, `os`, `path/filepath`, `runtime/debug`, `slices`, `strconv`, `strings`, `sync`, `time`, `unicode/utf8`.

View File

@@ -8,6 +8,7 @@ import (
"strings"
"unicode"
"git.golem15.com/golem15/summercms/internal/argmask"
"git.golem15.com/golem15/summercms/modules/sunscreen"
"github.com/charmbracelet/colorprofile"
)
@@ -284,63 +285,20 @@ func wrongDirectory(name string, err error, args []string, dir string) error {
if rerr != nil {
return err
}
try := "cd " + shellQuote(rel) + "; {bin}" + tryArgs(args)
try := "cd " + argmask.Quote(rel) + "; {bin}" + tryArgs(args)
return &HintError{Err: err, Explanation: wrongDirectoryExplanation, Try: try}
}
// secretWords mark a long flag whose value the Try line masks.
var secretWords = []string{"password", "passwd", "secret", "token", "key"}
// secretFlag reports whether the long flag name (without "--") looks like it
// carries a secret.
func secretFlag(name string) bool {
name = strings.ToLower(name)
for _, w := range secretWords {
if strings.Contains(name, w) {
return true
}
}
return false
}
// tryArgs renders args for the wrong-directory Try line, each preceded by a
// space and quoted for a shell. The value of a long flag named like a
// secret becomes ***, in the "--flag=value" form and in the "--flag value"
// form, so a password typed on the command line does not reach stderr and
// CI logs. In the "--flag value" form the next argument is masked whatever
// its first character (pflag takes "-Xk9" as the value too); only "--"
// is left alone. tryArgs cannot see flag types, so a secret-named boolean
// flag over-masks the argument after it, which is the safe side.
// tryArgs renders args for the wrong-directory Try line: empty for no
// args, otherwise a space and argmask.Join(args), which quotes each
// argument for a shell and masks the values of secret-named long flags
// (also a value that starts with "-"; only "--" is left alone), so a
// password typed on the command line does not reach stderr and CI logs.
func tryArgs(args []string) string {
var b strings.Builder
for i := 0; i < len(args); i++ {
a := args[i]
b.WriteByte(' ')
name, long := strings.CutPrefix(a, "--")
flag, _, inline := strings.Cut(name, "=")
if !long || flag == "" || !secretFlag(flag) {
b.WriteString(shellQuote(a))
continue
}
if inline {
b.WriteString(shellQuote("--"+flag) + "=***")
continue
}
b.WriteString(shellQuote(a))
if i+1 < len(args) && args[i+1] != "--" {
b.WriteString(" ***")
i++
}
if len(args) == 0 {
return ""
}
return b.String()
}
// shellQuote single-quotes s when a shell would split or expand it.
func shellQuote(s string) string {
if s != "" && !strings.ContainsAny(s, " \t\n'\"\\$`*?[]{}()<>|&;#~!") {
return s
}
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
return " " + argmask.Join(args)
}
// parseUnknownCommand reads the command name and cobra's "Did you mean

View File

@@ -182,6 +182,8 @@ SUMMER_TESTS=(
TestDelegateExamples
# Plan 05 (review fixes): delegate exit codes and streams (CR-02, CR-03).
TestDelegateExitCodePassthrough TestAppStreams TestDelegateStreams
# quick 261022-4z8 (re-review fixes): WR-10.
TestRunAppCmdMasksSecrets
)
# hello stage.