fix(bonfire): mask secret flag values in the wrong-directory hint
- the Try line replaces the value of every long flag whose name contains password, passwd, secret, token or key with ***, in the --flag value and --flag=value forms; other arguments are unchanged - README and the writing-commands page say so (WR-08)
This commit is contained in:
@@ -202,7 +202,7 @@ An application binary also explains the most common mistake: running it from the
|
||||
cd ../..; bin/acme migrate
|
||||
```
|
||||
|
||||
Only the message changes: the binary still reads its configuration from `config/` in the working directory. `summer` is not affected, because its commands find the application root themselves.
|
||||
The suggested command repeats your arguments, except that the value of a flag whose name contains `password`, `passwd`, `secret`, `token` or `key` is masked as `***` (`--password ***`, `--api-token=***`), so a secret typed on the command line does not reach logs that capture stderr. Only the message changes: the binary still reads its configuration from `config/` in the working directory. `summer` is not affected, because its commands find the application root themselves.
|
||||
|
||||
## Documentation coverage
|
||||
|
||||
|
||||
@@ -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`. 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).
|
||||
- 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`. 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.
|
||||
- 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.
|
||||
|
||||
@@ -205,7 +205,8 @@ const wrongDirectoryExplanation = "This command runs from the application root."
|
||||
// name other than summer) failed while the working directory dir ("" means
|
||||
// os.Getwd) has no config directory and a parent directory holds
|
||||
// summer.yaml: compass then found no configuration, which is why the command
|
||||
// failed (D-19). The Try line is "cd <relative path>; bin/<name> <args>".
|
||||
// failed (D-19). The Try line is "cd <relative path>; bin/<name> <args>",
|
||||
// with the values of secret-looking flags masked (tryArgs).
|
||||
// Usage errors, Reported errors and summer are left alone; nothing else
|
||||
// about the failure changes.
|
||||
func wrongDirectory(name string, err error, args []string, dir string) error {
|
||||
@@ -239,13 +240,54 @@ func wrongDirectory(name string, err error, args []string, dir string) error {
|
||||
if rerr != nil {
|
||||
return err
|
||||
}
|
||||
try := "cd " + shellQuote(rel) + "; {bin}"
|
||||
for _, a := range args {
|
||||
try += " " + shellQuote(a)
|
||||
}
|
||||
try := "cd " + shellQuote(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 (the next argument, when it does not start with "-"), 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) && !strings.HasPrefix(args[i+1], "-") {
|
||||
b.WriteString(" ***")
|
||||
i++
|
||||
}
|
||||
}
|
||||
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'\"\\$`*?[]{}()<>|&;#~!") {
|
||||
|
||||
@@ -218,3 +218,47 @@ func TestWithExitCode(t *testing.T) {
|
||||
t.Errorf("exitCode through a wrapper = %d, want 4", code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWrongDirectoryMasksSecrets (WR-08): the wrong-directory Try line
|
||||
// replaces the value of every flag named like a password, secret, token or
|
||||
// key with ***, in both the "--flag value" and "--flag=value" forms, and
|
||||
// keeps every other argument.
|
||||
func TestWrongDirectoryMasksSecrets(t *testing.T) {
|
||||
plainEnv(t)
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "summer.yaml"), []byte("binary: acme\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sub := filepath.Join(root, "bin")
|
||||
if err := os.MkdirAll(sub, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fail := errors.New("lagoon: database.dsn is empty")
|
||||
cases := []struct {
|
||||
args []string
|
||||
want string
|
||||
}{
|
||||
{[]string{"admin:create", "--email", "a@b.c", "--password", "hunter2"}, "cd ..; {bin} admin:create --email a@b.c --password ***"},
|
||||
{[]string{"admin:create", "--email", "a@b.c", "--password=hunter2"}, "cd ..; {bin} admin:create --email a@b.c --password=***"},
|
||||
{[]string{"sync", "--client-secret", "s3cr3t", "--verbose"}, "cd ..; {bin} sync --client-secret *** --verbose"},
|
||||
{[]string{"sync", "--api-token=t0k3n"}, "cd ..; {bin} sync --api-token=***"},
|
||||
{[]string{"sync", "--key", "k3y", "acme blog"}, "cd ..; {bin} sync --key *** 'acme blog'"},
|
||||
{[]string{"sync", "--PASSWD", "hunter2"}, "cd ..; {bin} sync --PASSWD ***"},
|
||||
{[]string{"sync", "--token", "--force"}, "cd ..; {bin} sync --token --force"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
got := wrongDirectory("acme", fail, tc.args, sub)
|
||||
var hint *HintError
|
||||
if !errors.As(got, &hint) {
|
||||
t.Fatalf("%q: no hint: %v", tc.args, got)
|
||||
}
|
||||
if hint.Try != tc.want {
|
||||
t.Errorf("%q: Try = %q, want %q", tc.args, hint.Try, tc.want)
|
||||
}
|
||||
for _, secret := range []string{"hunter2", "s3cr3t", "t0k3n", "k3y"} {
|
||||
if strings.Contains(hint.Try, secret) {
|
||||
t.Errorf("%q: Try line repeats %q: %q", tc.args, secret, hint.Try)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user