fix(bonfire): mask secret flag values that start with a dash (WR-11)

This commit is contained in:
Jakub Zych
2026-10-22 03:57:55 +02:00
parent fce232d7c3
commit 707372bf65
3 changed files with 19 additions and 10 deletions

View File

@@ -202,7 +202,7 @@ An application binary also explains the most common mistake: running it from the
cd ../..; bin/acme migrate
```
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.
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 ***` (also a value that starts with `-`), `--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

View File

@@ -306,8 +306,11 @@ func secretFlag(name string) bool {
// 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.
// 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.
func tryArgs(args []string) string {
var b strings.Builder
for i := 0; i < len(args); i++ {
@@ -324,7 +327,7 @@ func tryArgs(args []string) string {
continue
}
b.WriteString(shellQuote(a))
if i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
if i+1 < len(args) && args[i+1] != "--" {
b.WriteString(" ***")
i++
}

View File

@@ -219,10 +219,11 @@ func TestWithExitCode(t *testing.T) {
}
}
// 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.
// TestWrongDirectoryMasksSecrets (WR-08, WR-11): 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 whatever the value's first character, and keeps every other
// argument; only a following "--" is not taken as the value.
func TestWrongDirectoryMasksSecrets(t *testing.T) {
plainEnv(t)
root := t.TempDir()
@@ -244,7 +245,12 @@ func TestWrongDirectoryMasksSecrets(t *testing.T) {
{[]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"},
// pflag takes the next argument as the value of a non-boolean
// flag even when it starts with "-": "--force" is the token here.
{[]string{"sync", "--token", "--force"}, "cd ..; {bin} sync --token ***"},
{[]string{"admin:create", "--email", "a@b.c", "--password", "-Xk9!q"}, "cd ..; {bin} admin:create --email a@b.c --password ***"},
{[]string{"sync", "--client-secret", "-s3cr3t", "--verbose"}, "cd ..; {bin} sync --client-secret *** --verbose"},
{[]string{"sync", "--token", "--", "x"}, "cd ..; {bin} sync --token -- x"},
}
for _, tc := range cases {
got := wrongDirectory("acme", fail, tc.args, sub)
@@ -255,7 +261,7 @@ func TestWrongDirectoryMasksSecrets(t *testing.T) {
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"} {
for _, secret := range []string{"hunter2", "s3cr3t", "t0k3n", "k3y", "-Xk9", "-s3cr3t"} {
if strings.Contains(hint.Try, secret) {
t.Errorf("%q: Try line repeats %q: %q", tc.args, secret, hint.Try)
}