diff --git a/builtins/builtins.go b/builtins/builtins.go index 8eaeefe76..a5c81793f 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -13,6 +13,7 @@ import ( "io/fs" "os" "sort" + "strings" "syscall" "time" @@ -96,14 +97,250 @@ func (c Command) Register() { if normalize != nil { args = normalize(args) } + hasHelp := fs.Lookup("help") != nil + // Honor `--help` once it's reached in argv to match GNU coreutils: + // `cmd --no-arg-flag --help --bogus` should print help and exit 0. + // We trim args at the first `--help` only when every preceding + // token is a registered no-argument flag. Value-takers (`-n 5`), + // explicit-value forms (`--foo=bar`), positionals, and unknown + // flags all preclude the trim — those tokens may still fail + // later validation (e.g. `head -n nope`'s numeric check), and + // trimming would silently swallow that error by short-circuiting + // on `--help`. GNU's left-to-right semantics require the earlier + // failure to surface even when `--help` follows it. + if hasHelp { + if idx, ok := safeHelpTrimIndex(fs, args); ok { + args = args[:idx+1] + } + } if err := fs.Parse(args); err != nil { - callCtx.Errf("%s: %v\n", name, err) + callCtx.Errf("%s: %s\n", name, rewritePflagError(err, args)) + if hasHelp { + callCtx.Errf("Try '%s --help' for more information.\n", name) + } return Result{Code: 1} } return handler(ctx, callCtx, fs.Args()) }) } +// rewritePflagError translates pflag's default error messages to the +// GNU-getopt-style wording that matches GNU coreutils byte-for-byte. +// It returns the rewritten message without a trailing newline or any +// "cmd:" prefix; callers prepend the builtin name themselves. +// +// args is the argv that was handed to fs.Parse. It is consulted only +// for the unknown-long-flag case, where pflag strips the `=value` +// suffix from its error string but GNU getopt reports the full token +// (`--no-such=value`, not just `--no-such`). +// +// Patterns covered: +// +// pflag → GNU +// "unknown flag: --foo" → "unrecognized option '--foo'" +// ("--foo=value" if the original argv used =value) +// "unknown shorthand flag: 'X' in -..." → "invalid option -- 'X'" +// "flag needs an argument: --foo" → "option '--foo' requires an argument" +// "flag needs an argument: 'X' in -Y" → "option requires an argument -- 'X'" +// `invalid argument "..." for "DESC" flag: → "option '--LONG' doesn't allow +// flag does not allow an argument` an argument" +// +// Unknown messages are returned unchanged. +func rewritePflagError(err error, args []string) string { + msg := err.Error() + + // pflag wraps errors returned by a Var's Set(value) as + // `invalid argument "VALUE" for "DESC" flag: INNER` + // where DESC is e.g. `-h, --human-readable` or `--total`. df's + // noArgBool / unitFlag.Set returns the literal "flag does not + // allow an argument" so users (and tests) see GNU's "doesn't" + // instead of the wrapped pflag verbosity. + if strings.HasPrefix(msg, "invalid argument ") && + strings.HasSuffix(msg, "flag does not allow an argument") { + if d, ok := extractFlagDescriptor(msg); ok { + // `-X=value` for a no-arg shorthand: GNU getopt iterates + // shorthand chars and treats `=` as an unknown shorthand, + // emitting `invalid option -- '='`. pflag instead routes + // the value through Set, hitting our no-arg guard. Match + // GNU when the original argv used the shorthand=value form. + if shortFlagEqualsValueIn(d, args) { + return "invalid option -- '='" + } + return "option '" + longFlagName(d) + "' doesn't allow an argument" + } + } + + if rest, ok := strings.CutPrefix(msg, "unknown flag: "); ok { + // pflag's error carries only the flag name; GNU's reports the + // full token, so we recover `--foo=value` from argv when the + // user wrote it that way. + return "unrecognized option '" + recoverLongFlagToken(rest, args) + "'" + } + + const shortPrefix = "unknown shorthand flag: '" + if rest, ok := strings.CutPrefix(msg, shortPrefix); ok { + if char, _, ok := strings.Cut(rest, "'"); ok { + return "invalid option -- '" + char + "'" + } + } + + if rest, ok := strings.CutPrefix(msg, "flag needs an argument: "); ok { + // pflag emits two distinct payloads depending on the form: + // long form: `--foo` + // short form: `'X' in -Y` (X is the char, Y is the run + // of shorthand letters) + // GNU getopt uses different wording for each: long flags get + // `option '--foo' requires an argument`, short flags get + // `option requires an argument -- 'X'`. + if char, found := shortMissingArg(rest); found { + return "option requires an argument -- '" + char + "'" + } + return "option '" + rest + "' requires an argument" + } + + return msg +} + +// safeHelpTrimIndex returns the index of the first `--help` in args if +// trimming at it is provably safe — i.e. every preceding token is a +// registered no-argument flag and the scan does not cross a `--` +// end-of-flags marker. Returns false otherwise, signalling the caller +// to leave args alone and let pflag report any earlier validation +// error. +func safeHelpTrimIndex(fs *pflag.FlagSet, args []string) (int, bool) { + for i, a := range args { + if a == "--" { + return 0, false + } + if a == "--help" { + return i, true + } + if !isNoArgFlagToken(fs, a) { + return 0, false + } + } + return 0, false +} + +// isNoArgFlagToken reports whether a is a registered flag (long or +// shorthand cluster) whose every component takes no value. Tokens +// containing `=` (explicit-value form) or matching value-taking flags +// return false — both can fail at parse time. Non-ASCII bytes in a +// shorthand cluster also return false: pflag's ShorthandLookup panics +// on inputs longer than one byte and `string(byte)` for any byte ≥ +// 0x80 produces a 2-byte UTF-8 encoding. +func isNoArgFlagToken(fs *pflag.FlagSet, a string) bool { + if len(a) < 2 || a[0] != '-' || a == "-" { + return false + } + if strings.Contains(a, "=") { + return false + } + if strings.HasPrefix(a, "--") { + f := fs.Lookup(a[2:]) + return f != nil && f.NoOptDefVal != "" + } + for i := 1; i < len(a); i++ { + c := a[i] + if c > 0x7E || c <= ' ' { + return false + } + f := fs.ShorthandLookup(string(c)) + if f == nil || f.NoOptDefVal == "" { + return false + } + } + return true +} + +// recoverLongFlagToken returns the original argv token for an unknown +// long flag whose stripped name (e.g. `--no-such`) appears in pflag's +// error. If the user wrote `--no-such=value`, the full token is +// returned; otherwise the bare flag is returned. Stops at `--` so a +// later positional like `-- --no-such=foo` is never misclassified. +func recoverLongFlagToken(flag string, args []string) string { + prefix := flag + "=" + for _, a := range args { + if a == "--" { + break + } + if a == flag || strings.HasPrefix(a, prefix) { + return a + } + } + return flag +} + +// shortFlagEqualsValueIn reports whether args contains a token of the +// form `-X=...` whose shorthand char X matches the shorthand encoded +// in descriptor (e.g. `-h, --human-readable` → X=`h`). Used to detect +// the GNU-getopt shorthand=value error class. +func shortFlagEqualsValueIn(descriptor string, args []string) bool { + short, ok := shortFlagFromDescriptor(descriptor) + if !ok { + return false + } + for _, a := range args { + if a == "--" { + break + } + if len(a) >= 3 && a[0] == '-' && a[1] != '-' && a[1] == short && a[2] == '=' { + return true + } + } + return false +} + +// shortFlagFromDescriptor extracts the single shorthand char from a +// pflag descriptor like `-h, --human-readable`. Returns false for +// long-only descriptors (`--total`). +func shortFlagFromDescriptor(d string) (byte, bool) { + if len(d) >= 2 && d[0] == '-' && d[1] != '-' { + return d[1], true + } + return 0, false +} + +// shortMissingArg parses pflag's short-form payload for +// `flag needs an argument: 'X' in -Y` and returns the bare char +// (`X`). The second return is false when payload is in the long-form +// (`--foo`), letting the caller fall through. +func shortMissingArg(payload string) (string, bool) { + if !strings.HasPrefix(payload, "'") { + return "", false + } + char, _, found := strings.Cut(payload[1:], "'") + if !found { + return "", false + } + return char, true +} + +// extractFlagDescriptor parses pflag's `invalid argument "..." for +// "DESC" flag: ...` wrapper and returns the DESC segment. +func extractFlagDescriptor(msg string) (string, bool) { + _, after, found := strings.Cut(msg, ` for "`) + if !found { + return "", false + } + desc, _, found := strings.Cut(after, `" flag:`) + if !found { + return "", false + } + return desc, true +} + +// longFlagName returns the long-form (--name) flag name from a pflag +// descriptor like `-X, --LONG` or `--LONG`. For shorthand-only flags +// (rare; e.g. df's hidden `-k`) the descriptor is just `-X` and we +// return it unchanged. +func longFlagName(descriptor string) string { + if i := strings.LastIndex(descriptor, ", "); i >= 0 { + return descriptor[i+2:] + } + return descriptor +} + // CallContext provides the capabilities available to builtin commands. // It is created by the Runner for each builtin invocation. type CallContext struct { diff --git a/builtins/du/du_test.go b/builtins/du/du_test.go index 3f2c67241..e7a82e20a 100644 --- a/builtins/du/du_test.go +++ b/builtins/du/du_test.go @@ -273,7 +273,7 @@ func TestDuUnknownFlag(t *testing.T) { _, stderr, code := cmdRun(t, "du --no-such-flag .", dir) assert.Equal(t, 1, code) assert.Contains(t, stderr, "du:") - assert.Contains(t, stderr, "unknown flag") + assert.Contains(t, stderr, "unrecognized option") } // --- Security-sensitive flags must be rejected --- diff --git a/builtins/error_rewrite_test.go b/builtins/error_rewrite_test.go new file mode 100644 index 000000000..0dd0f2a4d --- /dev/null +++ b/builtins/error_rewrite_test.go @@ -0,0 +1,172 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package builtins + +import ( + "errors" + "testing" + + "github.com/spf13/pflag" +) + +func TestSafeHelpTrimIndex(t *testing.T) { + makeFS := func() *pflag.FlagSet { + fs := pflag.NewFlagSet("t", pflag.ContinueOnError) + fs.BoolP("help", "", false, "") + fs.BoolP("verbose", "v", false, "") + fs.BoolP("quiet", "q", false, "") + fs.StringP("name", "n", "", "") + return fs + } + + tests := []struct { + name string + args []string + wantIdx int + wantOK bool + }{ + {"--help first", []string{"--help"}, 0, true}, + {"--help after no-arg long", []string{"--verbose", "--help", "--bogus"}, 1, true}, + {"--help after no-arg short", []string{"-q", "--help", "--bogus"}, 1, true}, + {"--help after cluster of no-arg shorts", []string{"-qv", "--help", "--bogus"}, 1, true}, + {"--help after value-taker is unsafe", []string{"-n", "5", "--help"}, 0, false}, + {"--help after =value is unsafe", []string{"--verbose=true", "--help"}, 0, false}, + {"--help after positional is unsafe", []string{"foo", "--help"}, 0, false}, + {"--help after unknown flag is unsafe", []string{"--bogus", "--help"}, 0, false}, + {"--help after -- is unsafe", []string{"--", "--help"}, 0, false}, + {"no --help in args", []string{"--verbose"}, 0, false}, + {"empty args", nil, 0, false}, + // Multi-byte UTF-8 in a shorthand cluster MUST not crash: + // `string(byte)` for byte ≥ 0x80 produces 2-byte UTF-8, and + // pflag.ShorthandLookup panics on inputs longer than one byte. + // Caught by FuzzPwdArgs/9386e59311458487. + {"non-ASCII byte in cluster", []string{"-˞", "--help"}, 0, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + idx, ok := safeHelpTrimIndex(makeFS(), tt.args) + if idx != tt.wantIdx || ok != tt.wantOK { + t.Errorf("safeHelpTrimIndex(%v) = (%d, %v), want (%d, %v)", tt.args, idx, ok, tt.wantIdx, tt.wantOK) + } + }) + } +} + +func TestRewritePflagError(t *testing.T) { + tests := []struct { + name string + in string + args []string + want string + }{ + { + name: "unknown long flag", + in: "unknown flag: --no-such-flag", + args: []string{"--no-such-flag"}, + want: "unrecognized option '--no-such-flag'", + }, + { + name: "unknown long flag with =value preserved", + in: "unknown flag: --no-such", + args: []string{"--no-such=value"}, + want: "unrecognized option '--no-such=value'", + }, + { + name: "unknown long flag with empty =", + in: "unknown flag: --no-such", + args: []string{"--no-such="}, + want: "unrecognized option '--no-such='", + }, + { + name: "unknown long flag — args lookup stops at --", + in: "unknown flag: --no-such", + args: []string{"--no-such", "--", "--no-such=other"}, + want: "unrecognized option '--no-such'", + }, + { + name: "unknown shorthand single", + in: "unknown shorthand flag: 'X' in -X", + args: []string{"-X"}, + want: "invalid option -- 'X'", + }, + { + name: "unknown shorthand within group", + in: "unknown shorthand flag: 'X' in -aX", + args: []string{"-aX"}, + want: "invalid option -- 'X'", + }, + { + name: "missing argument long", + in: "flag needs an argument: --type", + args: []string{"--type"}, + want: "option '--type' requires an argument", + }, + { + name: "missing argument short single", + in: "flag needs an argument: 'n' in -n", + args: []string{"-n"}, + want: "option requires an argument -- 'n'", + }, + { + name: "missing argument short combined", + in: "flag needs an argument: 'n' in -an", + args: []string{"-an"}, + want: "option requires an argument -- 'n'", + }, + { + name: "no-arg with long+short descriptor", + in: `invalid argument "true" for "-a, --all" flag: flag does not allow an argument`, + args: []string{"--all=true"}, + want: "option '--all' doesn't allow an argument", + }, + { + name: "shorthand =value form maps to GNU's invalid =", + in: `invalid argument "true" for "-h, --human-readable" flag: flag does not allow an argument`, + args: []string{"-h=true"}, + want: "invalid option -- '='", + }, + { + name: "shorthand =value form requires shorthand match", + in: `invalid argument "true" for "-a, --all" flag: flag does not allow an argument`, + args: []string{"--all=true"}, + want: "option '--all' doesn't allow an argument", + }, + { + name: "no-arg with long-only descriptor", + in: `invalid argument "true" for "--total" flag: flag does not allow an argument`, + args: []string{"--total=true"}, + want: "option '--total' doesn't allow an argument", + }, + { + name: "no-arg with short-only descriptor — argv used -X=value", + in: `invalid argument "true" for "-k" flag: flag does not allow an argument`, + args: []string{"-k=true"}, + want: "invalid option -- '='", + }, + { + name: "wrapped Var.Set error not the no-arg case is left alone", + in: `invalid argument "z" for "-t, --radix" flag: invalid radix`, + args: []string{"-t", "z"}, + want: `invalid argument "z" for "-t, --radix" flag: invalid radix`, + }, + { + name: "unknown pattern passes through", + in: "some custom error", + args: nil, + want: "some custom error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := rewritePflagError(errors.New(tt.in), tt.args) + if got != tt.want { + t.Errorf("rewritePflagError(%q, %v) = %q, want %q", tt.in, tt.args, got, tt.want) + } + }) + } +} diff --git a/builtins/pwd/pwd_test.go b/builtins/pwd/pwd_test.go index ba33227cf..7a2d90959 100644 --- a/builtins/pwd/pwd_test.go +++ b/builtins/pwd/pwd_test.go @@ -164,7 +164,7 @@ func TestPwdUnknownLongFlagRejected(t *testing.T) { assert.Equal(t, 1, code) assert.Equal(t, "", stdout) assert.Contains(t, stderr, "pwd:") - assert.Contains(t, stderr, "unknown flag") + assert.Contains(t, stderr, "unrecognized option") } func TestPwdUnknownShortFlagRejected(t *testing.T) { diff --git a/builtins/pwd/testdata/fuzz/FuzzPwdArgs/9386e59311458487 b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/9386e59311458487 new file mode 100644 index 000000000..a4bfd49d1 --- /dev/null +++ b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/9386e59311458487 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("-˞") diff --git a/docs/RULES.md b/docs/RULES.md index 82b1184e8..60728aefe 100644 --- a/docs/RULES.md +++ b/docs/RULES.md @@ -26,17 +26,19 @@ fs.SetOutput(io.Discard) // suppress pflag's own error output; format errors you **Flags that need `+N` offset support** (e.g. `tail -n +5`): register as `StringP`, then post-process the value with a helper that detects the `+` prefix. -**Error handling:** pflag parse errors (unknown flag, missing argument) should be written -to `r.stderr` with a `"cmdname: "` prefix and set `r.exitCode = 1; return nil`. This -matches POSIX command-failure semantics — a bad flag fails the command but does not abort -the script. +**Error handling:** pflag parse errors (unknown flag, missing argument) are caught +by the shared wrapper in `builtins/builtins.go`, which rewrites them to the GNU +getopt wording (`unrecognized option '--foo'`, `invalid option -- 'X'`, +`option '--foo' doesn't allow an argument`, `option '--foo' requires an argument`) +and appends a `Try 'cmdname --help' for more information.` line. The command +exits 1 — a bad flag fails the command but does not abort the script. ### Supported Flags Only Commands MUST only implement the flags listed in their supported flag set. Any flag not -explicitly registered with pflag is automatically rejected with an "unknown flag" error -written to stderr and exit code 1. Do NOT add pre-scan loops or special-case logic to -reject specific flags by name — rely on pflag's unknown-flag handling instead. +explicitly registered with pflag is automatically rejected with an "unrecognized option" +error written to stderr and exit code 1. Do NOT add pre-scan loops or special-case logic +to reject specific flags by name — rely on pflag's unknown-flag handling instead. ### Help Flag diff --git a/tests/scenarios/cmd/df/basic/help_short_circuits_after_no_arg_flag.yaml b/tests/scenarios/cmd/df/basic/help_short_circuits_after_no_arg_flag.yaml new file mode 100644 index 000000000..c644fb0b1 --- /dev/null +++ b/tests/scenarios/cmd/df/basic/help_short_circuits_after_no_arg_flag.yaml @@ -0,0 +1,9 @@ +description: '`--help` is honored when it appears after one or more no-arg flags (here `-h` for human-readable, which is no-arg in df). The args-trim in builtins/builtins.go scans preceding tokens; if every one is a registered no-arg flag (no `=value`, no value-takers, no positionals), it trims at `--help`. Asserted via stdout_contains so both rshell''s minimal help and GNU''s longer help match.' +input: + script: |+ + df -h --help --bogus +expect: + stdout_contains: + - "Usage:" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/df/basic/help_short_circuits_before_unknown_flag.yaml b/tests/scenarios/cmd/df/basic/help_short_circuits_before_unknown_flag.yaml new file mode 100644 index 000000000..01329bcd5 --- /dev/null +++ b/tests/scenarios/cmd/df/basic/help_short_circuits_before_unknown_flag.yaml @@ -0,0 +1,9 @@ +description: '`--help` wins in argv order over a later unknown flag, matching GNU coreutils. Without the args-trim in builtins/builtins.go, pflag would reject --no-such-flag before reaching --help and df would exit 1; instead we print usage and exit 0. Asserted via stdout_contains "Usage:" so both rshell''s and bash''s help text match.' +input: + script: |+ + df --help --no-such-flag +expect: + stdout_contains: + - "Usage:" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/df/errors/unknown_flag.yaml b/tests/scenarios/cmd/df/errors/unknown_flag.yaml index 82b69c85b..a9a1d2bdf 100644 --- a/tests/scenarios/cmd/df/errors/unknown_flag.yaml +++ b/tests/scenarios/cmd/df/errors/unknown_flag.yaml @@ -1,9 +1,10 @@ -description: df rejects unknown flags with exit 1 -skip_assert_against_bash: true +description: df rejects unknown flags with exit 1. Matches GNU's "unrecognized option" wording byte-for-byte after the pflag-error rewriter in builtins/builtins.go. input: script: |+ df --no-such-flag expect: stdout: "" - stderr_contains: ["df:"] + stderr: |+ + df: unrecognized option '--no-such-flag' + Try 'df --help' for more information. exit_code: 1 diff --git a/tests/scenarios/cmd/df/errors/unknown_flag_before_help.yaml b/tests/scenarios/cmd/df/errors/unknown_flag_before_help.yaml new file mode 100644 index 000000000..6206e6806 --- /dev/null +++ b/tests/scenarios/cmd/df/errors/unknown_flag_before_help.yaml @@ -0,0 +1,11 @@ +description: An unknown flag positioned BEFORE --help still errors — GNU getopt processes argv left-to-right and reports the leftmost bad option. The args-trim in builtins/builtins.go only truncates AT --help, so a bad flag earlier in argv reaches pflag and is rejected. +input: + script: |+ + df --no-such-flag --help; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + df: unrecognized option '--no-such-flag' + Try 'df --help' for more information. + exit_code: 0 diff --git a/tests/scenarios/cmd/df/errors/unknown_flag_with_equals_value.yaml b/tests/scenarios/cmd/df/errors/unknown_flag_with_equals_value.yaml new file mode 100644 index 000000000..aabebcd26 --- /dev/null +++ b/tests/scenarios/cmd/df/errors/unknown_flag_with_equals_value.yaml @@ -0,0 +1,10 @@ +description: Unknown long flags written as `--foo=value` preserve the full token in the error, matching GNU getopt. pflag's error string only carries the bare flag name (`--no-such`); `recoverLongFlagToken` in builtins/builtins.go scans argv to recover the original `=value` suffix. +input: + script: |+ + df --no-such=value +expect: + stdout: "" + stderr: |+ + df: unrecognized option '--no-such=value' + Try 'df --help' for more information. + exit_code: 1 diff --git a/tests/scenarios/cmd/df/flags/rejected_all_equals_false.yaml b/tests/scenarios/cmd/df/flags/rejected_all_equals_false.yaml index 81022af0c..08a809e4a 100644 --- a/tests/scenarios/cmd/df/flags/rejected_all_equals_false.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_all_equals_false.yaml @@ -1,12 +1,11 @@ description: df rejects --all=false explicitly — matches GNU getopt's no-argument refusal for boolean options. The =false form would otherwise be silently swallowed by pflag.BoolP. -skip_assert_against_bash: true input: script: |+ df --all=false; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: option '--all' doesn't allow an argument + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_all_equals_true.yaml b/tests/scenarios/cmd/df/flags/rejected_all_equals_true.yaml index b9e0344b3..eda127404 100644 --- a/tests/scenarios/cmd/df/flags/rejected_all_equals_true.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_all_equals_true.yaml @@ -1,12 +1,11 @@ -description: df rejects --all=true (no-arg flags use a NUL-byte NoOptDefVal sentinel that argv cannot forge, so any explicit value is rejected with "flag does not allow an argument"). -skip_assert_against_bash: true +description: df rejects --all=true (no-arg flags use a NUL-byte NoOptDefVal sentinel that argv cannot forge, so any explicit value is rejected). Stderr matches GNU's "doesn't allow an argument" wording byte-for-byte after the pflag-error rewriter in builtins/builtins.go. input: script: |+ df --all=true; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: option '--all' doesn't allow an argument + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_h_equals_true.yaml b/tests/scenarios/cmd/df/flags/rejected_h_equals_true.yaml index edf312cb2..081098041 100644 --- a/tests/scenarios/cmd/df/flags/rejected_h_equals_true.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_h_equals_true.yaml @@ -1,12 +1,11 @@ -description: df rejects -h=true (the short shorthand also goes through the unitFlag NUL-byte sentinel guard, so the explicit-value form fails just like the long form --human-readable=true). -skip_assert_against_bash: true +description: df rejects -h=true. The error wording matches GNU getopt's shorthand-=-as-invalid behaviour after the rewriter in builtins/builtins.go detects the `-X=value` argv shape and substitutes `invalid option -- '='` for pflag's wrapped no-arg error. input: script: |+ df -h=true; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: invalid option -- '=' + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_help_equals_true.yaml b/tests/scenarios/cmd/df/flags/rejected_help_equals_true.yaml index 0f7caf126..c4638e09a 100644 --- a/tests/scenarios/cmd/df/flags/rejected_help_equals_true.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_help_equals_true.yaml @@ -1,12 +1,11 @@ description: df rejects --help=true — even --help (which would short-circuit to print usage) falls through the noArgBool sentinel guard when given an explicit value, exactly matching GNU df. -skip_assert_against_bash: true input: script: |+ df --help=true; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: option '--help' doesn't allow an argument + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_kibibytes_long.yaml b/tests/scenarios/cmd/df/flags/rejected_kibibytes_long.yaml index 66b7cafa0..e63b1b05f 100644 --- a/tests/scenarios/cmd/df/flags/rejected_kibibytes_long.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_kibibytes_long.yaml @@ -1,12 +1,11 @@ -description: df rejects --kibibytes (no GNU long form for -k; only the -k short form is recognised). -skip_assert_against_bash: true +description: df rejects --kibibytes (no GNU long form for -k; only the -k short form is recognised). GNU also rejects this; both emit the same "unrecognized option" line after the pflag-error rewriter. input: script: |+ df --kibibytes; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "kibibytes" + stdout: |+ + code:1 + stderr: |+ + df: unrecognized option '--kibibytes' + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_print_type_equals_value.yaml b/tests/scenarios/cmd/df/flags/rejected_print_type_equals_value.yaml index ddf68f5b3..68d2cfbcf 100644 --- a/tests/scenarios/cmd/df/flags/rejected_print_type_equals_value.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_print_type_equals_value.yaml @@ -1,12 +1,11 @@ -description: df rejects --print-type=ext4 — non-bool-looking values are equally invalid for a no-argument flag, so the NUL-byte sentinel guard rejects them with the same "does not allow an argument" error. -skip_assert_against_bash: true +description: df rejects --print-type=ext4 — non-bool-looking values are equally invalid for a no-argument flag, so the NUL-byte sentinel guard rejects them with the same "doesn't allow an argument" error. input: script: |+ df --print-type=ext4; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: option '--print-type' doesn't allow an argument + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/df/flags/rejected_total_equals_true.yaml b/tests/scenarios/cmd/df/flags/rejected_total_equals_true.yaml index a593db99c..630cab46d 100644 --- a/tests/scenarios/cmd/df/flags/rejected_total_equals_true.yaml +++ b/tests/scenarios/cmd/df/flags/rejected_total_equals_true.yaml @@ -1,12 +1,11 @@ description: df rejects --total=true — locks in the noArgBool sentinel for --total, the only no-argument bool flag without a short form. -skip_assert_against_bash: true input: script: |+ df --total=true; echo "code:$?" expect: - stdout_contains: - - "code:1" - stderr_contains: - - "df:" - - "does not allow an argument" + stdout: |+ + code:1 + stderr: |+ + df: option '--total' doesn't allow an argument + Try 'df --help' for more information. exit_code: 0 diff --git a/tests/scenarios/cmd/head/errors/help_after_value_does_not_swallow_unknown_flag.yaml b/tests/scenarios/cmd/head/errors/help_after_value_does_not_swallow_unknown_flag.yaml new file mode 100644 index 000000000..ca31ed30d --- /dev/null +++ b/tests/scenarios/cmd/head/errors/help_after_value_does_not_swallow_unknown_flag.yaml @@ -0,0 +1,18 @@ +# GNU fails on `-n nope` (numeric validation in handler) with exit 1; rshell fails on +# `--bogus` (unknown flag at parse time) also with exit 1. Both don't print help — +# we lock in that the trim does NOT swallow `--bogus` here. The skip is for the +# error-text divergence (different reason, same exit code); rshell's head doesn't +# validate -n's numeric value through pflag.Var.Set yet, which is a separate +# pre-existing issue out of scope for this PR. +description: '`head -n nope --help --bogus` must keep failing instead of silently printing help. The args-trim in builtins/builtins.go is restricted to the case where `--help` is the very first argument; here `-n nope` precedes it, so the trim is skipped and pflag still surfaces `--bogus` as an unrecognized option. Without that restriction the trim would discard `--bogus`, the handler would short-circuit on `--help`, and an invalid earlier value would be silently swallowed.' +skip_assert_against_bash: true +input: + script: |+ + head -n nope --help --bogus; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + head: unrecognized option '--bogus' + Try 'head --help' for more information. + exit_code: 0 diff --git a/tests/scenarios/cmd/head/errors/missing_arg_long.yaml b/tests/scenarios/cmd/head/errors/missing_arg_long.yaml new file mode 100644 index 000000000..68b97e2e4 --- /dev/null +++ b/tests/scenarios/cmd/head/errors/missing_arg_long.yaml @@ -0,0 +1,10 @@ +description: A long option that requires a value but is given none emits GNU's "option '--lines' requires an argument" wording. Verifies the long-flag branch of `rewritePflagError` in builtins/builtins.go (sibling of the short-flag form, which uses different wording). +input: + script: |+ + head --lines +expect: + stdout: "" + stderr: |+ + head: option '--lines' requires an argument + Try 'head --help' for more information. + exit_code: 1 diff --git a/tests/scenarios/cmd/head/errors/missing_arg_short.yaml b/tests/scenarios/cmd/head/errors/missing_arg_short.yaml new file mode 100644 index 000000000..7f95e9ffb --- /dev/null +++ b/tests/scenarios/cmd/head/errors/missing_arg_short.yaml @@ -0,0 +1,10 @@ +description: A short option that requires a value but is given none emits GNU's "option requires an argument -- 'X'" wording (note the inverted order vs. the long-flag form). pflag uses payload `'X' in -Y` for shorthand missing-arg, which `rewritePflagError` in builtins/builtins.go translates to GNU's getopt-style message. Without that translation `head -n` would print `option ''n' in -n' requires an argument`, breaking byte-for-byte agreement with GNU. +input: + script: |+ + head -n +expect: + stdout: "" + stderr: |+ + head: option requires an argument -- 'n' + Try 'head --help' for more information. + exit_code: 1 diff --git a/tests/scenarios/cmd/ip/hardening/batch_blocked.yaml b/tests/scenarios/cmd/ip/hardening/batch_blocked.yaml index 1134bc87e..27b84f66e 100644 --- a/tests/scenarios/cmd/ip/hardening/batch_blocked.yaml +++ b/tests/scenarios/cmd/ip/hardening/batch_blocked.yaml @@ -6,6 +6,6 @@ input: script: |+ ip -b /tmp/cmds addr show expect: - stderr: "ip: unknown shorthand flag: 'b' in -b\n" + stderr: "ip: invalid option -- 'b'\nTry 'ip --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ip/hardening/force_blocked.yaml b/tests/scenarios/cmd/ip/hardening/force_blocked.yaml index 94d687ea2..6449f25f1 100644 --- a/tests/scenarios/cmd/ip/hardening/force_blocked.yaml +++ b/tests/scenarios/cmd/ip/hardening/force_blocked.yaml @@ -6,6 +6,6 @@ input: script: |+ ip --force addr show expect: - stderr: "ip: unknown flag: --force\n" + stderr: "ip: unrecognized option '--force'\nTry 'ip --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ip/hardening/netns_flag_blocked.yaml b/tests/scenarios/cmd/ip/hardening/netns_flag_blocked.yaml index d05ed0186..90cfe33d1 100644 --- a/tests/scenarios/cmd/ip/hardening/netns_flag_blocked.yaml +++ b/tests/scenarios/cmd/ip/hardening/netns_flag_blocked.yaml @@ -6,6 +6,6 @@ input: script: |+ ip -n mynamespace addr show expect: - stderr: "ip: unknown shorthand flag: 'n' in -n\n" + stderr: "ip: invalid option -- 'n'\nTry 'ip --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/broadcast_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/broadcast_flag_rejected.yaml index 2c784ecce..1fdc4f98e 100644 --- a/tests/scenarios/cmd/ping/errors/broadcast_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/broadcast_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -b 255.255.255.255 expect: stdout: "" - stderr: "ping: unknown shorthand flag: 'b' in -b\n" + stderr: "ping: invalid option -- 'b'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/flood_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/flood_flag_rejected.yaml index bc32b4913..389df229e 100644 --- a/tests/scenarios/cmd/ping/errors/flood_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/flood_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -f localhost expect: stdout: "" - stderr: "ping: unknown shorthand flag: 'f' in -f\n" + stderr: "ping: invalid option -- 'f'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/interface_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/interface_flag_rejected.yaml index 95b3dc21e..971a5c3af 100644 --- a/tests/scenarios/cmd/ping/errors/interface_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/interface_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -I eth0 localhost expect: stdout: "" - stderr: "ping: unknown shorthand flag: 'I' in -I\n" + stderr: "ping: invalid option -- 'I'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/pattern_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/pattern_flag_rejected.yaml index 4aa683ce3..a91de8303 100644 --- a/tests/scenarios/cmd/ping/errors/pattern_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/pattern_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -p ff localhost expect: stdout: "" - stderr: "ping: unknown shorthand flag: 'p' in -p\n" + stderr: "ping: invalid option -- 'p'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/record_route_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/record_route_flag_rejected.yaml index d8b723eb7..bc8f88904 100644 --- a/tests/scenarios/cmd/ping/errors/record_route_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/record_route_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -R localhost expect: stdout: "" - stderr: "ping: unknown shorthand flag: 'R' in -R\n" + stderr: "ping: invalid option -- 'R'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/size_flag_rejected.yaml b/tests/scenarios/cmd/ping/errors/size_flag_rejected.yaml index 886a6f644..71de9692e 100644 --- a/tests/scenarios/cmd/ping/errors/size_flag_rejected.yaml +++ b/tests/scenarios/cmd/ping/errors/size_flag_rejected.yaml @@ -5,6 +5,6 @@ input: ping -s 1000 localhost expect: stdout: "" - stderr: "ping: unknown shorthand flag: 's' in -s\n" + stderr: "ping: invalid option -- 's'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/ping/errors/unknown_flag.yaml b/tests/scenarios/cmd/ping/errors/unknown_flag.yaml index 8f42603a8..fe8e04769 100644 --- a/tests/scenarios/cmd/ping/errors/unknown_flag.yaml +++ b/tests/scenarios/cmd/ping/errors/unknown_flag.yaml @@ -5,6 +5,6 @@ input: ping --no-such-flag localhost expect: stdout: "" - stderr: "ping: unknown flag: --no-such-flag\n" + stderr: "ping: unrecognized option '--no-such-flag'\nTry 'ping --help' for more information.\n" exit_code: 1 skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml b/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml index 4da43fc9d..cbd0202dc 100644 --- a/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml +++ b/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml @@ -6,5 +6,6 @@ input: expect: stdout: "" stderr: |+ - pwd: unknown flag: --no-such-flag + pwd: unrecognized option '--no-such-flag' + Try 'pwd --help' for more information. exit_code: 1 diff --git a/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml b/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml index c46de437b..6107c085a 100644 --- a/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml +++ b/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml @@ -6,5 +6,6 @@ input: expect: stdout: "" stderr: |+ - pwd: unknown shorthand flag: 'x' in -x + pwd: invalid option -- 'x' + Try 'pwd --help' for more information. exit_code: 1 diff --git a/tests/scenarios/cmd/pwd/errors/version_rejected.yaml b/tests/scenarios/cmd/pwd/errors/version_rejected.yaml index 0b3eaec9a..21438feb0 100644 --- a/tests/scenarios/cmd/pwd/errors/version_rejected.yaml +++ b/tests/scenarios/cmd/pwd/errors/version_rejected.yaml @@ -6,5 +6,6 @@ input: expect: stdout: "" stderr: |+ - pwd: unknown flag: --version + pwd: unrecognized option '--version' + Try 'pwd --help' for more information. exit_code: 1 diff --git a/tests/scenarios/cmd/sed/errors/blocked_inplace.yaml b/tests/scenarios/cmd/sed/errors/blocked_inplace.yaml index e2380acae..cd130175e 100644 --- a/tests/scenarios/cmd/sed/errors/blocked_inplace.yaml +++ b/tests/scenarios/cmd/sed/errors/blocked_inplace.yaml @@ -6,5 +6,5 @@ input: echo "test" | sed -i 's/test/replaced/' expect: stdout: "" - stderr_contains: ["unknown shorthand flag"] + stderr_contains: ["invalid option"] exit_code: 1 diff --git a/tests/scenarios/cmd/strings/errors/empty_radix.yaml b/tests/scenarios/cmd/strings/errors/empty_radix.yaml index 182708221..ac547dbc8 100644 --- a/tests/scenarios/cmd/strings/errors/empty_radix.yaml +++ b/tests/scenarios/cmd/strings/errors/empty_radix.yaml @@ -11,5 +11,5 @@ input: strings --radix= data.bin expect: stdout: "" - stderr: "strings: invalid argument \"\" for \"-t, --radix\" flag: invalid radix\n" + stderr: "strings: invalid argument \"\" for \"-t, --radix\" flag: invalid radix\nTry 'strings --help' for more information.\n" exit_code: 1 diff --git a/tests/scenarios/cmd/strings/errors/invalid_radix.yaml b/tests/scenarios/cmd/strings/errors/invalid_radix.yaml index 7c85c3071..1b9a5b6f3 100644 --- a/tests/scenarios/cmd/strings/errors/invalid_radix.yaml +++ b/tests/scenarios/cmd/strings/errors/invalid_radix.yaml @@ -11,5 +11,5 @@ input: strings -t z data.bin expect: stdout: "" - stderr: "strings: invalid argument \"z\" for \"-t, --radix\" flag: invalid radix\n" + stderr: "strings: invalid argument \"z\" for \"-t, --radix\" flag: invalid radix\nTry 'strings --help' for more information.\n" exit_code: 1 diff --git a/tests/scenarios/cmd/uniq/errors/unknown_flag.yaml b/tests/scenarios/cmd/uniq/errors/unknown_flag.yaml index d60a9808a..8750a2f3f 100644 --- a/tests/scenarios/cmd/uniq/errors/unknown_flag.yaml +++ b/tests/scenarios/cmd/uniq/errors/unknown_flag.yaml @@ -6,5 +6,5 @@ input: script: |+ uniq --definitely-invalid expect: - stderr_contains: ["unknown flag"] + stderr_contains: ["unrecognized option"] exit_code: 1