From bb8643c1fae87e511102535b56d2f60177573786 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Tue, 23 Jun 2026 16:05:57 +0200 Subject: [PATCH 1/2] fix(builtins): reject --flag=value on no-argument boolean flags pflag's BoolP silently accepts the explicit-value form (e.g. rm --force=false target), where GNU coreutils reject it before touching any operand. df already worked around this privately; promote that helper to builtins.NoArgBool and migrate all 24 builtins to it so the whole shell matches GNU. Co-Authored-By: Claude Opus 4.8 (1M context) --- builtins/cat/cat.go | 24 ++-- builtins/cd/cd.go | 2 +- builtins/cut/cut.go | 10 +- builtins/df/df.go | 104 +++--------------- builtins/du/du.go | 16 +-- builtins/grep/grep.go | 32 +++--- builtins/head/head.go | 4 +- builtins/help/help.go | 4 +- builtins/ip/ip.go | 12 +- builtins/ls/ls.go | 28 ++--- builtins/noargbool.go | 90 +++++++++++++++ builtins/ping/ping.go | 10 +- builtins/ps/ps.go | 6 +- builtins/pwd/pwd.go | 2 +- builtins/read/read.go | 6 +- builtins/rm/rm.go | 10 +- builtins/sed/sed.go | 14 +-- builtins/sort/sort.go | 22 ++-- builtins/ss/ss.go | 28 ++--- builtins/strings_cmd/strings.go | 8 +- builtins/tail/tail.go | 6 +- builtins/tr/tr.go | 12 +- builtins/truncate/truncate.go | 6 +- builtins/uname/uname.go | 6 +- builtins/uniq/uniq.go | 14 +-- builtins/wc/wc.go | 14 +-- builtins/xargs/xargs.go | 10 +- .../reject_ignore_case_equals_value.yaml | 23 ++++ .../cmd/rm/flags/reject_dir_equals_true.yaml | 23 ++++ .../rm/flags/reject_force_equals_false.yaml | 24 ++++ .../flags/reject_reverse_equals_value.yaml | 22 ++++ .../wc/flags/reject_lines_equals_value.yaml | 22 ++++ 32 files changed, 374 insertions(+), 240 deletions(-) create mode 100644 builtins/noargbool.go create mode 100644 tests/scenarios/cmd/grep/flags/reject_ignore_case_equals_value.yaml create mode 100644 tests/scenarios/cmd/rm/flags/reject_dir_equals_true.yaml create mode 100644 tests/scenarios/cmd/rm/flags/reject_force_equals_false.yaml create mode 100644 tests/scenarios/cmd/sort/flags/reject_reverse_equals_value.yaml create mode 100644 tests/scenarios/cmd/wc/flags/reject_lines_equals_value.yaml diff --git a/builtins/cat/cat.go b/builtins/cat/cat.go index 79530bd81..74ec2277d 100644 --- a/builtins/cat/cat.go +++ b/builtins/cat/cat.go @@ -94,17 +94,17 @@ const ( ) func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.Bool("help", false, "print usage and exit") - number := fs.BoolP("number", "n", false, "number all output lines") - numberNonblank := fs.BoolP("number-nonblank", "b", false, "number non-blank output lines, overrides -n") - squeezeBlank := fs.BoolP("squeeze-blank", "s", false, "suppress repeated empty output lines") - showEnds := fs.BoolP("show-ends", "E", false, "display $ at end of each line") - showTabs := fs.BoolP("show-tabs", "T", false, "display TAB characters as ^I") - showNonprinting := fs.BoolP("show-nonprinting", "v", false, "use ^ and M- notation, except for LFD and TAB") - showAll := fs.BoolP("show-all", "A", false, "equivalent to -vET") - flagE := fs.BoolP("show-nonprinting-ends", "e", false, "equivalent to -vE") - flagT := fs.BoolP("show-nonprinting-tabs", "t", false, "equivalent to -vT") - _ = fs.BoolP("unbuffered", "u", false, "ignored") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") + number := builtins.NoArgBool(fs, "number", "n", "number all output lines") + numberNonblank := builtins.NoArgBool(fs, "number-nonblank", "b", "number non-blank output lines, overrides -n") + squeezeBlank := builtins.NoArgBool(fs, "squeeze-blank", "s", "suppress repeated empty output lines") + showEnds := builtins.NoArgBool(fs, "show-ends", "E", "display $ at end of each line") + showTabs := builtins.NoArgBool(fs, "show-tabs", "T", "display TAB characters as ^I") + showNonprinting := builtins.NoArgBool(fs, "show-nonprinting", "v", "use ^ and M- notation, except for LFD and TAB") + showAll := builtins.NoArgBool(fs, "show-all", "A", "equivalent to -vET") + flagE := builtins.NoArgBool(fs, "show-nonprinting-ends", "e", "equivalent to -vE") + flagT := builtins.NoArgBool(fs, "show-nonprinting-tabs", "t", "equivalent to -vT") + _ = builtins.NoArgBool(fs, "unbuffered", "u", "ignored") return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { if *help { @@ -112,7 +112,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Concatenate FILE(s) to standard output.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/cd/cd.go b/builtins/cd/cd.go index b173510f3..ef27b06f7 100644 --- a/builtins/cd/cd.go +++ b/builtins/cd/cd.go @@ -129,7 +129,7 @@ var errBoolSeqValue = errors.New("option doesn't allow an argument") const boolSeqSentinel = "\x00rshell:cd:bare\x00" func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - helpFlag := fs.BoolP("help", "h", false, "print usage and exit") + helpFlag := builtins.NoArgBool(fs, "help", "h", "print usage and exit") // Disable interspersed flag parsing: once the directory operand // is seen, any following token (including flag-shaped ones like diff --git a/builtins/cut/cut.go b/builtins/cut/cut.go index 7dbf939b8..4c1915e13 100644 --- a/builtins/cut/cut.go +++ b/builtins/cut/cut.go @@ -97,14 +97,14 @@ const ( // registerFlags registers all cut flags on the framework-provided FlagSet and // returns a bound handler whose flag variables are captured by closure. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.Bool("help", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") bytesListStr := fs.StringP("bytes", "b", "", "select only these bytes") charsListStr := fs.StringP("characters", "c", "", "select only these characters") fieldsListStr := fs.StringP("fields", "f", "", "select only these fields") delimiter := fs.StringP("delimiter", "d", "\t", "use DELIM instead of TAB for field delimiter") - onlyDelimited := fs.BoolP("only-delimited", "s", false, "do not print lines not containing delimiters") - _ = fs.BoolP("", "n", false, "do not split multi-byte characters") - complement := fs.Bool("complement", false, "complement the set of selected bytes, characters, or fields") + onlyDelimited := builtins.NoArgBool(fs, "only-delimited", "s", "do not print lines not containing delimiters") + _ = builtins.NoArgBool(fs, "", "n", "do not split multi-byte characters") + complement := builtins.NoArgBool(fs, "complement", "", "complement the set of selected bytes, characters, or fields") outputDelimiter := fs.String("output-delimiter", "", "use STRING as the output delimiter") return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { @@ -128,7 +128,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Print selected parts of lines from each FILE to standard output.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/df/df.go b/builtins/df/df.go index e9751800a..1e69b39a7 100644 --- a/builtins/df/df.go +++ b/builtins/df/df.go @@ -106,18 +106,6 @@ const ( unitsHuman1000 // -H: powers of 1000 ) -// noArgSentinel is the NoOptDefVal we set on every no-argument flag -// (unitFlag and noArgBool). pflag passes this exact string to Set when -// the user writes the bare flag (`-h`, `--all`); for the explicit-value -// form (`--all=true`) pflag passes the user's literal value instead. -// -// We use a single NUL byte so the two cases are distinguishable: NUL -// is the C-string terminator and POSIX execve(2) refuses to pass argv -// elements containing it, so the user cannot forge this string from a -// shell. That lets Set reject every `=value` form — including `=true` -// — to match GNU df's "doesn't allow an argument" exit-1 error. -const noArgSentinel = "\x00" - // unitFlag is a pflag.Value that writes a fixed unitMode into a shared // target each time the flag is set. We use one instance for -h (writes // unitsHuman1024) and one for -H (writes unitsHuman1000) sharing a @@ -136,7 +124,7 @@ func (u *unitFlag) String() string { return "" } func (u *unitFlag) Type() string { return "bool" } // Set is called by pflag once per occurrence of the flag. It receives: -// - noArgSentinel for a bare flag (e.g. `-h`, `--human-readable`); +// - builtins.NoArgSentinel for a bare flag (e.g. `-h`, `--human-readable`); // - the user's literal value for `--name=value` / `-name=value`. // // GNU df rejects every explicit-value form (`gdf --human-readable=false` @@ -145,7 +133,7 @@ func (u *unitFlag) Type() string { return "bool" } // argv (NUL bytes can't be passed through execve), so any non-sentinel // value here means the user wrote `--flag=value` and must be rejected. func (u *unitFlag) Set(s string) error { - if s != noArgSentinel { + if s != builtins.NoArgSentinel { return errors.New("flag does not allow an argument") } *u.target = u.value @@ -158,45 +146,7 @@ func (u *unitFlag) Set(s string) error { // and rejects `-h` with "flag needs an argument". func registerUnitFlag(fs *builtins.FlagSet, target *unitMode, value unitMode, name, shorthand, usage string) { flag := fs.VarPF(&unitFlag{target: target, value: value}, name, shorthand, usage) - flag.NoOptDefVal = noArgSentinel -} - -// noArgBool is a pflag.Value that mirrors GNU getopt's no_argument -// behaviour: bare `--flag` and `-f` work, but `--flag=value` and -// `-f=value` are rejected with "flag does not allow an argument" for -// every value (including `=true`). pflag.BoolP treats the -// explicit-value form as a successful parse, which silently diverges -// from GNU. -// -// Like unitFlag, this relies on noArgSentinel (a NUL byte) to -// distinguish a bare flag from an explicit `=value`. -type noArgBool struct { - target *bool -} - -func (b *noArgBool) String() string { - if b.target != nil && *b.target { - return "true" - } - return "false" -} -func (b *noArgBool) Type() string { return "bool" } -func (b *noArgBool) Set(s string) error { - if s != noArgSentinel { - return errors.New("flag does not allow an argument") - } - *b.target = true - return nil -} - -// registerNoArgBool installs a noArgBool flag and returns the *bool -// target so the caller can read it like an ordinary fs.Bool result. -// Pass an empty shorthand for long-only flags (e.g. --total). -func registerNoArgBool(fs *builtins.FlagSet, name, shorthand, usage string) *bool { - target := new(bool) - flag := fs.VarPF(&noArgBool{target: target}, name, shorthand, usage) - flag.NoOptDefVal = noArgSentinel - return target + flag.NoOptDefVal = builtins.NoArgSentinel } // flags carries the parsed flag state. It is constructed once per @@ -217,20 +167,20 @@ type flags struct { func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { mode := unitsK - // All boolean options use registerNoArgBool so explicit-value + // All boolean options use builtins.NoArgBool so explicit-value // forms (`--all=false`, `--portability=false`, etc.) are // rejected with "flag does not allow an argument" — matching GNU // df's getopt(1)-style refusal for no-argument options. f := &flags{ - help: registerNoArgBool(fs, "help", "", "print usage and exit"), + help: builtins.NoArgBool(fs, "help", "", "print usage and exit"), mode: &mode, - posix: registerNoArgBool(fs, "portability", "P", "use the POSIX output format"), - printType: registerNoArgBool(fs, "print-type", "T", "print file system type"), - inodes: registerNoArgBool(fs, "inodes", "i", "list inode information instead of block usage"), - all: registerNoArgBool(fs, "all", "a", "include pseudo, duplicate, inaccessible file systems"), - local: registerNoArgBool(fs, "local", "l", "limit listing to local file systems"), - total: registerNoArgBool(fs, "total", "", "append a grand total row"), - noSync: registerNoArgBool(fs, "no-sync", "", "do not invoke sync before getting usage info (default; accepted for compatibility)"), + posix: builtins.NoArgBool(fs, "portability", "P", "use the POSIX output format"), + printType: builtins.NoArgBool(fs, "print-type", "T", "print file system type"), + inodes: builtins.NoArgBool(fs, "inodes", "i", "list inode information instead of block usage"), + all: builtins.NoArgBool(fs, "all", "a", "include pseudo, duplicate, inaccessible file systems"), + local: builtins.NoArgBool(fs, "local", "l", "limit listing to local file systems"), + total: builtins.NoArgBool(fs, "total", "", "append a grand total row"), + noSync: builtins.NoArgBool(fs, "no-sync", "", "do not invoke sync before getting usage info (default; accepted for compatibility)"), includeTypes: fs.StringArrayP("type", "t", nil, "limit listing to file systems of type TYPE"), excludeTypes: fs.StringArrayP("exclude-type", "x", nil, "limit listing to file systems not of type TYPE"), } @@ -252,7 +202,7 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // from the auto-generated help and handle the line manually in // printHelp. kFlag := fs.VarPF(&unitFlag{target: &mode, value: unitsK}, "", "k", "use 1024-byte blocks (POSIX default)") - kFlag.NoOptDefVal = noArgSentinel + kFlag.NoOptDefVal = builtins.NoArgSentinel kFlag.Hidden = true return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { @@ -892,33 +842,15 @@ func minColumnWidths(withType bool) []int { // skipped by PrintDefaults; we append the line manually so it still // appears in --help. // -// Every no-argument flag uses noArgSentinel (a NUL byte) as its -// NoOptDefVal so that explicit-value forms (--all=true, etc.) are -// rejected. PrintDefaults would happily render that NUL byte into the -// help text as `--all[= ]\x00…`, producing binary garbage. Clear the -// NoOptDefVal of every flag before printing — Parse has already run, so -// the cleared value cannot affect parsing for this invocation. The -// defer-restore is defensive: makeFlags currently builds a fresh -// FlagSet per invocation, but a future refactor that reuses or caches -// the FlagSet would silently regress the explicit-value rejection -// without it (--all=true would start being accepted). +// The no-argument flags carry builtins.NoArgSentinel (a NUL byte) as their +// NoOptDefVal so that explicit-value forms (--all=true, etc.) are rejected; +// builtins.PrintFlagDefaults clears those sentinels before printing so the +// NUL is not rendered into the help text as binary garbage. func printHelp(callCtx *builtins.CallContext, fs *builtins.FlagSet) { callCtx.Out("Usage: df [OPTION]...\n") callCtx.Out("Show information about the file system on which each FILE resides,\n") callCtx.Out("or all file systems by default.\n\n") - saved := make(map[*builtins.Flag]string) - fs.VisitAll(func(flag *builtins.Flag) { - if flag.NoOptDefVal == noArgSentinel { - saved[flag] = flag.NoOptDefVal - flag.NoOptDefVal = "" - } - }) - defer func() { - for f, v := range saved { - f.NoOptDefVal = v - } - }() fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) callCtx.Out(" -k use 1024-byte blocks (POSIX default)\n") } diff --git a/builtins/du/du.go b/builtins/du/du.go index b1244677b..13774f058 100644 --- a/builtins/du/du.go +++ b/builtins/du/du.go @@ -224,10 +224,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // shape rather than alphabetical. fs.SortFlags = false - all := fs.BoolP("all", "a", false, "write counts for all files, not just directories") - summarize := fs.BoolP("summarize", "s", false, "display only a total for each argument") - total := fs.BoolP("total", "c", false, "produce a grand total") - separateDirs := fs.BoolP("separate-dirs", "S", false, "for directories, do not include size of subdirectories") + all := builtins.NoArgBool(fs, "all", "a", "write counts for all files, not just directories") + summarize := builtins.NoArgBool(fs, "summarize", "s", "display only a total for each argument") + total := builtins.NoArgBool(fs, "total", "c", "produce a grand total") + separateDirs := builtins.NoArgBool(fs, "separate-dirs", "S", "for directories, do not include size of subdirectories") // Mutually-exclusive last-wins groups (-L vs -P, and the size-format // flags -b/-h/--si/-k/-m). Each Set() call increments a shared @@ -248,16 +248,16 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { derefL := registerSeq("dereference", "L", "dereference all symbolic links") derefP := registerSeq("no-dereference", "P", "don't follow any symbolic links (default)") - apparentSize := fs.Bool("apparent-size", false, "print apparent sizes rather than device usage") + apparentSize := builtins.NoArgBool(fs, "apparent-size", "", "print apparent sizes rather than device usage") bytesFlag := registerSeq("bytes", "b", "equivalent to --apparent-size --block-size=1") humanFlag := registerSeq("human-readable", "h", "print sizes in human-readable format") siFlag := registerSeq("si", "", "like -h, but use powers of 1000") kiloFlag := registerSeq("kilobytes", "k", "use 1024-byte blocks (default)") megaFlag := registerSeq("megabytes", "m", "use 1 MiB (1024*1024) blocks") - null := fs.BoolP("null", "0", false, "end each output line with NUL, not newline") + null := builtins.NoArgBool(fs, "null", "0", "end each output line with NUL, not newline") maxDepth := fs.IntP("max-depth", "d", -1, "print the total for a directory only if it is N or fewer levels deep") - helpFlag := fs.Bool("help", false, "print usage and exit") + helpFlag := builtins.NoArgBool(fs, "help", "", "print usage and exit") return func(ctx context.Context, callCtx *builtins.CallContext, paths []string) builtins.Result { if *helpFlag { @@ -265,7 +265,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Usage: du [OPTION]... [FILE]...\n") callCtx.Out("Summarize device usage of the set of FILEs, recursively for directories.\n") callCtx.Out("With no FILE, du operates on the current directory.\n\n") - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/grep/grep.go b/builtins/grep/grep.go index ae51dea2c..0a1a2d034 100644 --- a/builtins/grep/grep.go +++ b/builtins/grep/grep.go @@ -162,21 +162,21 @@ const ( func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // Binary mode flag. - textMode := fs.BoolP("text", "a", false, "process binary file as if it were text") + textMode := builtins.NoArgBool(fs, "text", "a", "process binary file as if it were text") // Pattern mode flags. - extendedRegexp := fs.BoolP("extended-regexp", "E", false, "use extended regular expressions") - fixedStrings := fs.BoolP("fixed-strings", "F", false, "interpret pattern as fixed strings") - basicRegexp := fs.BoolP("basic-regexp", "G", false, "use basic regular expressions (default)") + extendedRegexp := builtins.NoArgBool(fs, "extended-regexp", "E", "use extended regular expressions") + fixedStrings := builtins.NoArgBool(fs, "fixed-strings", "F", "interpret pattern as fixed strings") + basicRegexp := builtins.NoArgBool(fs, "basic-regexp", "G", "use basic regular expressions (default)") // Matching flags. - ignoreCase := fs.BoolP("ignore-case", "i", false, "ignore case distinctions") - invertMatch := fs.BoolP("invert-match", "v", false, "select non-matching lines") - wordRegexp := fs.BoolP("word-regexp", "w", false, "match only whole words") - lineRegexp := fs.BoolP("line-regexp", "x", false, "match only whole lines") + ignoreCase := builtins.NoArgBool(fs, "ignore-case", "i", "ignore case distinctions") + invertMatch := builtins.NoArgBool(fs, "invert-match", "v", "select non-matching lines") + wordRegexp := builtins.NoArgBool(fs, "word-regexp", "w", "match only whole words") + lineRegexp := builtins.NoArgBool(fs, "line-regexp", "x", "match only whole lines") // Output flags. - count := fs.BoolP("count", "c", false, "print only a count of matching lines per file") + count := builtins.NoArgBool(fs, "count", "c", "print only a count of matching lines per file") var outputSeq int filesWithMatches := newOrderedBoolFlag(&outputSeq) filesWithoutMatch := newOrderedBoolFlag(&outputSeq) @@ -184,7 +184,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { fs.VarP(filesWithoutMatch, "files-without-match", "L", "print only names of files without matches") fs.Lookup("files-with-matches").NoOptDefVal = "true" fs.Lookup("files-without-match").NoOptDefVal = "true" - lineNumber := fs.BoolP("line-number", "n", false, "prefix output with line numbers") + lineNumber := builtins.NoArgBool(fs, "line-number", "n", "prefix output with line numbers") var filenameSeq int withFilename := newOrderedBoolFlag(&filenameSeq) noFilename := newOrderedBoolFlag(&filenameSeq) @@ -192,10 +192,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { fs.VarP(noFilename, "no-filename", "h", "suppress filename prefix") fs.Lookup("with-filename").NoOptDefVal = "true" fs.Lookup("no-filename").NoOptDefVal = "true" - onlyMatching := fs.BoolP("only-matching", "o", false, "print only the matched parts") - quiet := fs.BoolP("quiet", "q", false, "suppress all output") - _ = fs.Bool("silent", false, "alias for --quiet") - noMessages := fs.BoolP("no-messages", "s", false, "suppress error messages") + onlyMatching := builtins.NoArgBool(fs, "only-matching", "o", "print only the matched parts") + quiet := builtins.NoArgBool(fs, "quiet", "q", "suppress all output") + _ = builtins.NoArgBool(fs, "silent", "", "alias for --quiet") + noMessages := builtins.NoArgBool(fs, "no-messages", "s", "suppress error messages") maxCount := fs.IntP("max-count", "m", -1, "stop after NUM matches per file") // Context flags. @@ -208,7 +208,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { fs.VarP(&patterns, "regexp", "e", "use PATTERN as the pattern") // Help flag (long-only; -h is taken by --no-filename). - help := fs.Bool("help", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { @@ -216,7 +216,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Search for PATTERN in each FILE.\n") callCtx.Out("When FILE is -, read standard input. With no FILE, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/head/head.go b/builtins/head/head.go index ab24cae2a..0f70a74b2 100644 --- a/builtins/head/head.go +++ b/builtins/head/head.go @@ -88,7 +88,7 @@ const MaxLineBytes = 1 << 20 // 1 MiB // returns a bound handler whose flag variables are captured by closure. The // framework calls Parse and passes positional arguments to the handler. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") // quietFlag, silentFlag, and verboseFlag share a sequence counter so that // after parsing we can determine which of -q/--quiet/--silent/-v/--verbose @@ -144,7 +144,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Print the first 10 lines of each FILE to standard output.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/help/help.go b/builtins/help/help.go index 30a62dd87..aa65e50e5 100644 --- a/builtins/help/help.go +++ b/builtins/help/help.go @@ -48,8 +48,8 @@ func printUsage(callCtx *builtins.CallContext) { } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - helpFlag := fs.Bool("help", false, "print usage and exit") - allFlag := fs.Bool("all", false, "show all commands (including not allowed) with descriptions; ignored when a topic is given") + helpFlag := builtins.NoArgBool(fs, "help", "", "print usage and exit") + allFlag := builtins.NoArgBool(fs, "all", "", "show all commands (including not allowed) with descriptions; ignored when a topic is given") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *helpFlag { diff --git a/builtins/ip/ip.go b/builtins/ip/ip.go index 7cffb6bba..54b82cf19 100644 --- a/builtins/ip/ip.go +++ b/builtins/ip/ip.go @@ -138,11 +138,11 @@ type displayOpts struct { } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - oneline := fs.BoolP("oneline", "o", false, "output each record on a single line") - brief := fs.Bool("brief", false, "print brief information in tabular format") - ipv4 := fs.BoolP("ipv4", "4", false, "show only IPv4 addresses") - ipv6 := fs.BoolP("ipv6", "6", false, "show only IPv6 addresses") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + oneline := builtins.NoArgBool(fs, "oneline", "o", "output each record on a single line") + brief := builtins.NoArgBool(fs, "brief", "", "print brief information in tabular format") + ipv4 := builtins.NoArgBool(fs, "ipv4", "4", "show only IPv4 addresses") + ipv6 := builtins.NoArgBool(fs, "ipv6", "6", "show only IPv6 addresses") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { @@ -198,7 +198,7 @@ func printHelp(callCtx *builtins.CallContext, fs *builtins.FlagSet) { callCtx.Out(" route get ADDRESS Show route to ADDRESS (Linux only)\n\n") callCtx.Out("Global options:\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) callCtx.Out("\nNote: -b/-B/-batch, -force, -n/--netns, and 'ip netns' are blocked for safety.\n") callCtx.Out("Note: the real ip command's -br flag is --brief in this builtin.\n") } diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index 1b0bdf679..3ac54433e 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -110,26 +110,26 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // This lets us determine which sort flag (-S or -t) was specified last. fs.SortFlags = false - _ = fs.BoolP("all", "a", false, "do not ignore entries starting with .") - _ = fs.BoolP("almost-all", "A", false, "do not ignore . and ..") - dirOnly := fs.BoolP("directory", "d", false, "list directories themselves, not their contents") - reverse := fs.BoolP("reverse", "r", false, "reverse order while sorting") - _ = fs.BoolP("sort-size", "S", false, "sort by file size, largest first") - _ = fs.BoolP("sort-time", "t", false, "sort by modification time, newest first") - classify := fs.BoolP("classify", "F", false, "append indicator to entries") - appendSlash := fs.BoolP("append-slash", "p", false, "append / indicator to directories") - recursive := fs.BoolP("recursive", "R", false, "list subdirectories recursively") - longFmt := fs.BoolP("long", "l", false, "use a long listing format") - humanReadable := fs.BoolP("human-readable", "h", false, "with -l, print human-readable sizes") + _ = builtins.NoArgBool(fs, "all", "a", "do not ignore entries starting with .") + _ = builtins.NoArgBool(fs, "almost-all", "A", "do not ignore . and ..") + dirOnly := builtins.NoArgBool(fs, "directory", "d", "list directories themselves, not their contents") + reverse := builtins.NoArgBool(fs, "reverse", "r", "reverse order while sorting") + _ = builtins.NoArgBool(fs, "sort-size", "S", "sort by file size, largest first") + _ = builtins.NoArgBool(fs, "sort-time", "t", "sort by modification time, newest first") + classify := builtins.NoArgBool(fs, "classify", "F", "append indicator to entries") + appendSlash := builtins.NoArgBool(fs, "append-slash", "p", "append / indicator to directories") + recursive := builtins.NoArgBool(fs, "recursive", "R", "list subdirectories recursively") + longFmt := builtins.NoArgBool(fs, "long", "l", "use a long listing format") + humanReadable := builtins.NoArgBool(fs, "human-readable", "h", "with -l, print human-readable sizes") // -1 is the default in non-terminal (always true here), accepted for compat. - _ = fs.Bool("one", false, "list one file per line") + _ = builtins.NoArgBool(fs, "one", "", "list one file per line") fs.Lookup("one").Shorthand = "1" // Pagination flags (long-only, non-standard). offset := fs.Int("offset", 0, "skip first N entries (pagination)") limit := fs.Int("limit", 0, "show at most N entries (capped at MaxDirEntries)") // Help flag (long-only; -h is taken by --human-readable). - help := fs.Bool("help", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { @@ -137,7 +137,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("List directory contents.\n") callCtx.Out("List information about the FILEs (the current directory by default).\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/noargbool.go b/builtins/noargbool.go new file mode 100644 index 000000000..c82bea3f4 --- /dev/null +++ b/builtins/noargbool.go @@ -0,0 +1,90 @@ +// 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" + +// NoArgSentinel is the NoOptDefVal that no-argument flags registered via +// [NoArgBool] (and df's unitFlag) carry. pflag passes this exact string to +// the flag's Set when the user writes the bare flag (`-f`, `--force`); for +// the explicit-value form (`--force=false`) pflag passes the user's literal +// value instead. +// +// We use a single NUL byte so the two cases are distinguishable: NUL is the +// C-string terminator and POSIX execve(2) refuses to pass argv elements +// containing it, so the user cannot forge this string from a shell. That lets +// Set reject every `=value` form — including `=true` — to match GNU's +// "doesn't allow an argument" exit-1 error. The flagparser package recognises +// the "flag does not allow an argument" error these flags return and rewrites +// it to the GNU getopt wording. +const NoArgSentinel = "\x00" + +// noArgBool is a pflag.Value that mirrors GNU getopt's no_argument behaviour: +// bare `--flag` and `-f` work, but `--flag=value` and `-f=value` are rejected +// with "flag does not allow an argument" for every value (including `=true`). +// pflag.BoolP treats the explicit-value form as a successful parse, which +// silently diverges from GNU — this type closes that gap. +type noArgBool struct { + target *bool +} + +func (b *noArgBool) String() string { + if b.target != nil && *b.target { + return "true" + } + return "false" +} + +func (b *noArgBool) Type() string { return "bool" } + +func (b *noArgBool) Set(s string) error { + if s != NoArgSentinel { + return errors.New("flag does not allow an argument") + } + *b.target = true + return nil +} + +// NoArgBool registers a GNU getopt no_argument-style boolean flag on fs and +// returns the *bool target, which the caller reads like an ordinary fs.Bool +// result. Bare `--name` / `-shorthand` set it to true; the explicit-value +// form (`--name=true`, `--name=false`, `-shorthand=x`) is rejected with the +// GNU "doesn't allow an argument" error. Pass an empty shorthand for +// long-only flags. +// +// Prefer this over pflag's BoolP/Bool for every no-argument boolean flag so +// the shell matches GNU coreutils, which reject `--flag=value` before acting +// on any operand. +func NoArgBool(fs *FlagSet, name, shorthand, usage string) *bool { + target := new(bool) + flag := fs.VarPF(&noArgBool{target: target}, name, shorthand, usage) + flag.NoOptDefVal = NoArgSentinel + return target +} + +// PrintFlagDefaults writes fs's flag usage to fs's configured output (set via +// fs.SetOutput), mirroring fs.PrintDefaults but first clearing the NUL +// NoOptDefVal that [NoArgBool] flags carry. Without this, pflag would render +// the sentinel into the help text as `--flag[=\x00]` binary garbage. The +// original NoOptDefVal values are restored before returning, so calling this +// after Parse cannot affect the explicit-value rejection. Builtins that +// register no-argument boolean flags must print their flag help through this +// helper rather than calling fs.PrintDefaults directly. +func PrintFlagDefaults(fs *FlagSet) { + saved := make(map[*Flag]string) + fs.VisitAll(func(flag *Flag) { + if flag.NoOptDefVal == NoArgSentinel { + saved[flag] = flag.NoOptDefVal + flag.NoOptDefVal = "" + } + }) + defer func() { + for f, v := range saved { + f.NoOptDefVal = v + } + }() + fs.PrintDefaults() +} diff --git a/builtins/ping/ping.go b/builtins/ping/ping.go index 122b54fc2..d5bc5897e 100644 --- a/builtins/ping/ping.go +++ b/builtins/ping/ping.go @@ -133,16 +133,16 @@ var Cmd = builtins.Command{ } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit 0") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit 0") count := fs.IntP("count", "c", defaultCount, fmt.Sprintf("number of ICMP packets to send (%d–%d)", minCount, maxCount)) // StringP instead of DurationP so we accept both Go duration literals // (e.g. "1s", "500ms") and the integer/float seconds that iputils ping // accepts (e.g. "-W 1", "-i 0.2"). parsePingDuration handles both forms. waitStr := fs.StringP("wait", "W", defaultWait.String(), fmt.Sprintf("time to wait for each reply (%v–%v)", minWait, maxWait)) intervalStr := fs.StringP("interval", "i", defaultInterval.String(), fmt.Sprintf("interval between packets (%v–%v)", minInterval, maxInterval)) - quiet := fs.BoolP("quiet", "q", false, "quiet output: suppress per-packet lines") - ipv4 := fs.BoolP("ipv4", "4", false, "use IPv4") - ipv6 := fs.BoolP("ipv6", "6", false, "use IPv6") + quiet := builtins.NoArgBool(fs, "quiet", "q", "quiet output: suppress per-packet lines") + ipv4 := builtins.NoArgBool(fs, "ipv4", "4", "use IPv4") + ipv6 := builtins.NoArgBool(fs, "ipv6", "6", "use IPv6") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { @@ -468,7 +468,7 @@ func printHelp(callCtx *builtins.CallContext, fs *builtins.FlagSet) { callCtx.Out("Send ICMP echo requests to HOST and report statistics.\n\n") callCtx.Out("Options:\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) callCtx.Out("\nNote: the following flags are not supported for safety and will be rejected:\n") callCtx.Out(" -f (flood), -b (broadcast), -s (packet size), -I (interface), -p (pattern), -R (record route)\n") } diff --git a/builtins/ps/ps.go b/builtins/ps/ps.go index 5db93768c..ce08051ee 100644 --- a/builtins/ps/ps.go +++ b/builtins/ps/ps.go @@ -65,16 +65,16 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { var showAll bool fs.BoolVarP(&showAll, "all", "e", false, "select all processes") fs.BoolVarP(&showAll, "All", "A", false, "select all processes (same as -e)") - fullFmt := fs.BoolP("full", "f", false, "full-format listing") + fullFmt := builtins.NoArgBool(fs, "full", "f", "full-format listing") pidList := fs.StringP("pid", "p", "", "select by PID list (comma or space separated)") - help := fs.Bool("help", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { callCtx.Out("Usage: ps [-e|-A] [-f] [-p PIDLIST]\n") callCtx.Out("Report process status.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go index 151f68a65..251d7f272 100644 --- a/builtins/pwd/pwd.go +++ b/builtins/pwd/pwd.go @@ -83,7 +83,7 @@ var errSymlinkLoop = errors.New("too many levels of symbolic links") const boolSeqSentinel = "\x00rshell:pwd:bare\x00" func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - helpFlag := fs.BoolP("help", "h", false, "print usage and exit") + helpFlag := builtins.NoArgBool(fs, "help", "h", "print usage and exit") // -L and -P share a sequence counter so that after parsing we can // compare their pos fields to determine which appeared last on the diff --git a/builtins/read/read.go b/builtins/read/read.go index 7530d2918..0dfaa8c70 100644 --- a/builtins/read/read.go +++ b/builtins/read/read.go @@ -107,8 +107,8 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // interspersed-flag parsing so later -tokens remain NAMEs. fs.SetInterspersed(false) - help := fs.BoolP("help", "h", false, "print usage and exit") - raw := fs.BoolP("raw", "r", false, "do not interpret backslashes") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + raw := builtins.NoArgBool(fs, "raw", "r", "do not interpret backslashes") prompt := fs.StringP("prompt", "p", "", "print PROMPT to stderr before reading") delim := fs.StringP("delim", "d", "", "use the first character of DELIM as the line terminator (empty = NUL)") timeoutStr := fs.StringP("timeout", "t", "", "time out after TIMEOUT seconds (decimal allowed)") @@ -138,7 +138,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("\n") callCtx.Out("Options:\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) callCtx.Out("\n") callCtx.Out("Field splitting: whitespace IFS chars (space, tab, newline) coalesce\n") callCtx.Out("into one separator; non-whitespace IFS chars do not. With more NAMEs\n") diff --git a/builtins/rm/rm.go b/builtins/rm/rm.go index 83aba028e..ae187f9ed 100644 --- a/builtins/rm/rm.go +++ b/builtins/rm/rm.go @@ -74,10 +74,10 @@ var Cmd = builtins.Command{ } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - force := fs.BoolP("force", "f", false, "ignore nonexistent files, never prompt") - dir := fs.BoolP("dir", "d", false, "remove empty directories") - verbose := fs.BoolP("verbose", "v", false, "print a message for each removed file") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + force := builtins.NoArgBool(fs, "force", "f", "ignore nonexistent files, never prompt") + dir := builtins.NoArgBool(fs, "dir", "d", "remove empty directories") + verbose := builtins.NoArgBool(fs, "verbose", "v", "print a message for each removed file") return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { // Capability check before everything else — including --help — so that @@ -94,7 +94,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Does not remove directories unless -d is specified (and only then if empty).\n") callCtx.Out("Symlinks are removed without following them.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/sed/sed.go b/builtins/sed/sed.go index d81532389..60742063b 100644 --- a/builtins/sed/sed.go +++ b/builtins/sed/sed.go @@ -152,18 +152,16 @@ func (e *expressionSlice) Type() string { return "string" } // registerFlags sets up sed flags and returns the handler. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - quiet := fs.BoolP("quiet", "n", false, "suppress automatic printing of pattern space") - fs.Lookup("quiet").NoOptDefVal = "true" + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + quiet := builtins.NoArgBool(fs, "quiet", "n", "suppress automatic printing of pattern space") // --silent is an alias for --quiet. - silent := fs.Bool("silent", false, "alias for --quiet") - fs.Lookup("silent").NoOptDefVal = "true" + silent := builtins.NoArgBool(fs, "silent", "", "alias for --quiet") var expressions expressionSlice fs.VarP(&expressions, "expression", "e", "add script commands") - extendedE := fs.BoolP("regexp-extended", "E", false, "use extended regular expressions") - extendedR := fs.BoolP("regexp-extended-r", "r", false, "use extended regular expressions (GNU alias for -E)") + extendedE := builtins.NoArgBool(fs, "regexp-extended", "E", "use extended regular expressions") + extendedR := builtins.NoArgBool(fs, "regexp-extended-r", "r", "use extended regular expressions (GNU alias for -E)") fs.Lookup("regexp-extended-r").Hidden = true return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { @@ -172,7 +170,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Stream editor for filtering and transforming text.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/sort/sort.go b/builtins/sort/sort.go index 9a5257fa8..588611797 100644 --- a/builtins/sort/sort.go +++ b/builtins/sort/sort.go @@ -145,23 +145,23 @@ const MaxTotalBytes = 5 * 1024 * 1024 // 5 MiB // registerFlags registers all sort flags and returns the bound handler. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.Bool("help", false, "print usage and exit") - reverse := fs.BoolP("reverse", "r", false, "reverse the result of comparisons") - numeric := fs.BoolP("numeric-sort", "n", false, "compare according to string numerical value") - humanNumeric := fs.BoolP("human-numeric-sort", "h", false, "compare human-readable numbers (e.g. 2K 1G)") - unique := fs.BoolP("unique", "u", false, "output only the first of an equal run") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") + reverse := builtins.NoArgBool(fs, "reverse", "r", "reverse the result of comparisons") + numeric := builtins.NoArgBool(fs, "numeric-sort", "n", "compare according to string numerical value") + humanNumeric := builtins.NoArgBool(fs, "human-numeric-sort", "h", "compare human-readable numbers (e.g. 2K 1G)") + unique := builtins.NoArgBool(fs, "unique", "u", "output only the first of an equal run") keyDefs := fs.StringArrayP("key", "k", nil, "sort via a key; KEYDEF is F[.C][OPTS][,F[.C][OPTS]]") fieldSep := fs.StringP("field-separator", "t", "", "use SEP as the field separator") - ignBlanks := fs.BoolP("ignore-leading-blanks", "b", false, "ignore leading blanks") - ignCase := fs.BoolP("ignore-case", "f", false, "fold lower case to upper case characters") - dictOrder := fs.BoolP("dictionary-order", "d", false, "consider only blanks and alphanumeric characters") + ignBlanks := builtins.NoArgBool(fs, "ignore-leading-blanks", "b", "ignore leading blanks") + ignCase := builtins.NoArgBool(fs, "ignore-case", "f", "fold lower case to upper case characters") + dictOrder := builtins.NoArgBool(fs, "dictionary-order", "d", "consider only blanks and alphanumeric characters") // --check accepts optional values: "diagnose" (default), "silent", "quiet". // -c is shorthand for --check (diagnose mode). // -C is shorthand for silent check mode. var checkFlag checkTracker fs.VarP(&checkFlag, "check", "c", "check for sorted input; optionally =silent or =quiet") - checkSilentShort := fs.BoolP("check-silent-short", "C", false, "like -c, but do not report first bad line") - stable := fs.BoolP("stable", "s", false, "stabilize sort by disabling last-resort comparison") + checkSilentShort := builtins.NoArgBool(fs, "check-silent-short", "C", "like -c, but do not report first bad line") + stable := builtins.NoArgBool(fs, "stable", "s", "stabilize sort by disabling last-resort comparison") // --check with no value means diagnose mode. fs.Lookup("check").NoOptDefVal = "diagnose" @@ -253,7 +253,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Write sorted concatenation of all FILE(s) to standard output.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/ss/ss.go b/builtins/ss/ss.go index 5f9277df3..f71377900 100644 --- a/builtins/ss/ss.go +++ b/builtins/ss/ss.go @@ -170,26 +170,26 @@ type options struct { // returns the bound handler. The framework parses args and passes positional // arguments (none expected for ss) to the handler. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - tcp := fs.BoolP("tcp", "t", false, "display only TCP sockets") - udp := fs.BoolP("udp", "u", false, "display only UDP sockets") - unix := fs.BoolP("unix", "x", false, "display only Unix domain sockets") - listening := fs.BoolP("listening", "l", false, "display only listening sockets") - all := fs.BoolP("all", "a", false, "display all sockets (listening and non-listening)") - numeric := fs.BoolP("numeric", "n", false, "do not resolve service names") - ipv4 := fs.BoolP("ipv4", "4", false, "display only IPv4 sockets") - ipv6 := fs.BoolP("ipv6", "6", false, "display only IPv6 sockets") - summary := fs.BoolP("summary", "s", false, "print summary statistics only") - noHeader := fs.BoolP("no-header", "H", false, "suppress column header") - showOpts := fs.BoolP("options", "o", false, "show timer information") - extended := fs.BoolP("extended", "e", false, "show extended socket info (uid, inode)") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + tcp := builtins.NoArgBool(fs, "tcp", "t", "display only TCP sockets") + udp := builtins.NoArgBool(fs, "udp", "u", "display only UDP sockets") + unix := builtins.NoArgBool(fs, "unix", "x", "display only Unix domain sockets") + listening := builtins.NoArgBool(fs, "listening", "l", "display only listening sockets") + all := builtins.NoArgBool(fs, "all", "a", "display all sockets (listening and non-listening)") + numeric := builtins.NoArgBool(fs, "numeric", "n", "do not resolve service names") + ipv4 := builtins.NoArgBool(fs, "ipv4", "4", "display only IPv4 sockets") + ipv6 := builtins.NoArgBool(fs, "ipv6", "6", "display only IPv6 sockets") + summary := builtins.NoArgBool(fs, "summary", "s", "print summary statistics only") + noHeader := builtins.NoArgBool(fs, "no-header", "H", "suppress column header") + showOpts := builtins.NoArgBool(fs, "options", "o", "show timer information") + extended := builtins.NoArgBool(fs, "extended", "e", "show extended socket info (uid, inode)") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { callCtx.Out("Usage: ss [OPTION]...\n") callCtx.Out("Display information about network sockets.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/strings_cmd/strings.go b/builtins/strings_cmd/strings.go index d29b7e361..6976d2627 100644 --- a/builtins/strings_cmd/strings.go +++ b/builtins/strings_cmd/strings.go @@ -146,8 +146,8 @@ func (o *octalFlagVal) IsBoolFlag() bool { return true } func (o *octalFlagVal) Type() string { return "bool" } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - _ = fs.BoolP("all", "a", false, "scan entire file (default; accepted for POSIX compatibility)") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + _ = builtins.NoArgBool(fs, "all", "a", "scan entire file (default; accepted for POSIX compatibility)") minLen := fs.IntP("bytes", "n", defaultMinLen, "minimum string length (default 4)") // format is shared by both -t and -o; pflag calls Set() in parse order so // whichever flag appears last on the command line wins (last-flag-wins). @@ -157,7 +157,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // (same as BoolVarP does internally), so -o alone calls Set("true"). oFlag := fs.VarPF(&octalFlagVal{target: &format}, "offset-octal", "o", "alias for -t o (print octal offsets)") oFlag.NoOptDefVal = "true" - printFileName := fs.BoolP("print-file-name", "f", false, "print file name before each string") + printFileName := builtins.NoArgBool(fs, "print-file-name", "f", "print file name before each string") separator := fs.StringP("output-separator", "s", "\n", "output separator between strings (default newline)") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { @@ -179,7 +179,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Print printable character sequences in files.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/tail/tail.go b/builtins/tail/tail.go index 5f89a9856..784a1ddec 100644 --- a/builtins/tail/tail.go +++ b/builtins/tail/tail.go @@ -137,8 +137,8 @@ type countMode struct { // returns a bound handler whose flag variables are captured by closure. The // framework calls Parse and passes positional arguments to the handler. func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - zeroTerminated := fs.BoolP("zero-terminated", "z", false, "use NUL as line delimiter") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + zeroTerminated := builtins.NoArgBool(fs, "zero-terminated", "z", "use NUL as line delimiter") // quietFlag, silentFlag, and verboseFlag share a sequence counter so that // after parsing we can tell which appeared last on the command line and @@ -201,7 +201,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Print the last 10 lines of each FILE to standard output.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/tr/tr.go b/builtins/tr/tr.go index 43133d75e..ea3e28ffc 100644 --- a/builtins/tr/tr.go +++ b/builtins/tr/tr.go @@ -76,14 +76,14 @@ const readBufSize = 32 * 1024 func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { fs.SetInterspersed(false) - help := fs.BoolP("help", "h", false, "print usage and exit") - deleteFlag := fs.BoolP("delete", "d", false, "delete characters in SET1") - squeeze := fs.BoolP("squeeze-repeats", "s", false, "squeeze repeated characters") - complement := fs.BoolP("complement", "c", false, "use complement of SET1") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + deleteFlag := builtins.NoArgBool(fs, "delete", "d", "delete characters in SET1") + squeeze := builtins.NoArgBool(fs, "squeeze-repeats", "s", "squeeze repeated characters") + complement := builtins.NoArgBool(fs, "complement", "c", "use complement of SET1") var bigC bool fs.BoolVarP(&bigC, "complement-alt", "C", false, "alias for -c/--complement") _ = fs.MarkHidden("complement-alt") - truncateSet1 := fs.BoolP("truncate-set1", "t", false, "truncate SET1 to length of SET2") + truncateSet1 := builtins.NoArgBool(fs, "truncate-set1", "t", "truncate SET1 to length of SET2") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { @@ -91,7 +91,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Translate, squeeze, and/or delete characters from standard input,\n") callCtx.Out("writing to standard output.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/truncate/truncate.go b/builtins/truncate/truncate.go index 50449e3e0..d72d7b61b 100644 --- a/builtins/truncate/truncate.go +++ b/builtins/truncate/truncate.go @@ -97,9 +97,9 @@ var errInvalidSize = errors.New("invalid size") var errRelativeSize = errors.New("relative size operators not supported") func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") sizeStr := fs.StringP("size", "s", "", "set file size to SIZE bytes") - noCreate := fs.BoolP("no-create", "c", false, "do not create files that do not exist") + noCreate := builtins.NoArgBool(fs, "no-create", "c", "do not create files that do not exist") return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { // Capability check before everything else — including --help — so that @@ -117,7 +117,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("larger than SIZE is truncated. Missing files are created unless\n") callCtx.Out("--no-create is given.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index 491f57383..cddab777f 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -69,12 +69,12 @@ var kernelFiles = [...]struct { } func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") var flags [len(kernelFiles)]*bool for i, entry := range kernelFiles { - flags[i] = fs.BoolP(entry.long, entry.short, false, "") + flags[i] = builtins.NoArgBool(fs, entry.long, entry.short, "") } - allFlag := fs.BoolP("all", "a", false, "print all information") + allFlag := builtins.NoArgBool(fs, "all", "a", "print all information") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { diff --git a/builtins/uniq/uniq.go b/builtins/uniq/uniq.go index fa7d21e05..a9004ade1 100644 --- a/builtins/uniq/uniq.go +++ b/builtins/uniq/uniq.go @@ -123,12 +123,12 @@ const ( ) func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.BoolP("help", "h", false, "print usage and exit") - count := fs.BoolP("count", "c", false, "prefix lines by the number of occurrences") - repeated := fs.BoolP("repeated", "d", false, "only print duplicate lines, one for each group") - unique := fs.BoolP("unique", "u", false, "only print unique lines") - ignoreCase := fs.BoolP("ignore-case", "i", false, "ignore differences in case when comparing") - zeroTerminated := fs.BoolP("zero-terminated", "z", false, "line delimiter is NUL, not newline") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") + count := builtins.NoArgBool(fs, "count", "c", "prefix lines by the number of occurrences") + repeated := builtins.NoArgBool(fs, "repeated", "d", "only print duplicate lines, one for each group") + unique := builtins.NoArgBool(fs, "unique", "u", "only print unique lines") + ignoreCase := builtins.NoArgBool(fs, "ignore-case", "i", "ignore differences in case when comparing") + zeroTerminated := builtins.NoArgBool(fs, "zero-terminated", "z", "line delimiter is NUL, not newline") skipFieldsStr := fs.StringP("skip-fields", "f", "0", "avoid comparing the first N fields") skipCharsStr := fs.StringP("skip-chars", "s", "0", "avoid comparing the first N characters") @@ -172,7 +172,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Filter adjacent matching lines from INPUT (or stdin),\n") callCtx.Out("writing to standard output.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/wc/wc.go b/builtins/wc/wc.go index a80059adb..e5cead477 100644 --- a/builtins/wc/wc.go +++ b/builtins/wc/wc.go @@ -115,12 +115,12 @@ func (o options) numCols() int { } func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { - help := fs.Bool("help", false, "print usage and exit") - lines := fs.BoolP("lines", "l", false, "print the newline counts") - words := fs.BoolP("words", "w", false, "print the word counts") - bytesFlag := fs.BoolP("bytes", "c", false, "print the byte counts") - chars := fs.BoolP("chars", "m", false, "print the character counts") - maxLineLen := fs.BoolP("max-line-length", "L", false, "print the maximum display width") + help := builtins.NoArgBool(fs, "help", "", "print usage and exit") + lines := builtins.NoArgBool(fs, "lines", "l", "print the newline counts") + words := builtins.NoArgBool(fs, "words", "w", "print the word counts") + bytesFlag := builtins.NoArgBool(fs, "bytes", "c", "print the byte counts") + chars := builtins.NoArgBool(fs, "chars", "m", "print the character counts") + maxLineLen := builtins.NoArgBool(fs, "max-line-length", "L", "print the maximum display width") // Security: --files0-from is intentionally NOT implemented. // GTFOBins: this flag reads filenames from a file, enabling @@ -132,7 +132,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Print newline, word, and byte counts for each FILE.\n") callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/builtins/xargs/xargs.go b/builtins/xargs/xargs.go index bacbfa1c4..3401280a6 100644 --- a/builtins/xargs/xargs.go +++ b/builtins/xargs/xargs.go @@ -159,7 +159,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // rather than re-interpreted by xargs. fs.SetInterspersed(false) - help := fs.BoolP("help", "h", false, "print usage and exit") + help := builtins.NoArgBool(fs, "help", "h", "print usage and exit") argFile := fs.StringP("arg-file", "a", "", "read items from FILE instead of stdin") eofStr := fs.StringP("eof", "E", "", "treat EOF-STR as a logical end-of-input marker") @@ -172,10 +172,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { fs.Lookup("null").NoOptDefVal = "true" delim := new(string) fs.VarP(&trackedString2{p: delim, key: sepDelim, t: &sep}, "delimiter", "d", "use DELIM as the single-byte item separator") - noRunIfEmpty := fs.BoolP("no-run-if-empty", "r", false, "do not run command if input is empty") + noRunIfEmpty := builtins.NoArgBool(fs, "no-run-if-empty", "r", "do not run command if input is empty") maxChars := fs.IntP("max-chars", "s", 0, "limit a single command line to N characters") - verbose := fs.BoolP("verbose", "t", false, "print the command line on stderr before running") - exitOnSize := fs.BoolP("exit", "x", false, "abort if -s is too small to fit a -n/-L batch") + verbose := builtins.NoArgBool(fs, "verbose", "t", "print the command line on stderr before running") + exitOnSize := builtins.NoArgBool(fs, "exit", "x", "abort if -s is too small to fit a -n/-L batch") // -n / -L / -I are mutually exclusive (GNU "last-wins" + warning). A // single tracker lets us tell which of the three was set most recently. @@ -192,7 +192,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Out("Usage: xargs [OPTION]... [COMMAND [INITIAL-ARGS]...]\n") callCtx.Out("Build and execute commands from standard input.\n\n") fs.SetOutput(callCtx.Stdout) - fs.PrintDefaults() + builtins.PrintFlagDefaults(fs) return builtins.Result{} } diff --git a/tests/scenarios/cmd/grep/flags/reject_ignore_case_equals_value.yaml b/tests/scenarios/cmd/grep/flags/reject_ignore_case_equals_value.yaml new file mode 100644 index 000000000..6afc350b1 --- /dev/null +++ b/tests/scenarios/cmd/grep/flags/reject_ignore_case_equals_value.yaml @@ -0,0 +1,23 @@ +# grep -i / --ignore-case is a no-argument option; GNU getopt rejects the +# explicit-value form. pflag.BoolP would have silently accepted +# --ignore-case=true. The GNU wording itself is bash-verified by df's +# non-skipped rejection scenarios (same flagparser path); bash assertion is +# skipped here only because rshell exits 1 uniformly on flag errors while GNU +# grep exits 2. +description: grep rejects --ignore-case=true (GNU no-argument refusal). +setup: + files: + - path: data.txt + content: "Apple\n" +input: + allowed_paths: ["$DIR"] + script: |+ + grep --ignore-case=true apple data.txt; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + grep: option '--ignore-case' doesn't allow an argument + Try 'grep --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/rm/flags/reject_dir_equals_true.yaml b/tests/scenarios/cmd/rm/flags/reject_dir_equals_true.yaml new file mode 100644 index 000000000..fc9975b0b --- /dev/null +++ b/tests/scenarios/cmd/rm/flags/reject_dir_equals_true.yaml @@ -0,0 +1,23 @@ +# Even `--dir=true` (the "expected" value) must be rejected: GNU getopt treats +# -d/--dir as a no-argument option and refuses any explicit value. The empty +# directory must survive and rm must exit 1. +description: rm rejects --dir=true and does not remove the directory (GNU no-argument refusal). +setup: + files: + - path: somedir/.keep + content: "" +input: + allowed_paths: ["$DIR"] + mode: "remediation" + script: |+ + rm --dir=true somedir; echo "code:$?" + ls +expect: + stdout: |+ + code:1 + somedir + stderr: |+ + rm: option '--dir' doesn't allow an argument + Try 'rm --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/rm/flags/reject_force_equals_false.yaml b/tests/scenarios/cmd/rm/flags/reject_force_equals_false.yaml new file mode 100644 index 000000000..03c192537 --- /dev/null +++ b/tests/scenarios/cmd/rm/flags/reject_force_equals_false.yaml @@ -0,0 +1,24 @@ +# Regression for the no-argument boolean flag migration: pflag.BoolP would +# accept `rm --force=false target` and delete the operand, where GNU rm +# rejects the explicit-value form before touching any file. The target must +# survive and rm must exit 1. +description: rm rejects --force=false and does not delete the operand (GNU no-argument refusal). +setup: + files: + - path: keep.txt + content: "data\n" +input: + allowed_paths: ["$DIR"] + mode: "remediation" + script: |+ + rm --force=false keep.txt; echo "code:$?" + cat keep.txt +expect: + stdout: |+ + code:1 + data + stderr: |+ + rm: option '--force' doesn't allow an argument + Try 'rm --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/sort/flags/reject_reverse_equals_value.yaml b/tests/scenarios/cmd/sort/flags/reject_reverse_equals_value.yaml new file mode 100644 index 000000000..e38b7c403 --- /dev/null +++ b/tests/scenarios/cmd/sort/flags/reject_reverse_equals_value.yaml @@ -0,0 +1,22 @@ +# sort -r / --reverse is a no-argument option; the explicit-value form must be +# rejected. pflag.BoolP would have silently accepted --reverse=false. The GNU +# wording is bash-verified by df's non-skipped rejection scenarios (same +# flagparser path); bash assertion is skipped here only because rshell exits 1 +# uniformly on flag errors while GNU sort exits 2. +description: sort rejects --reverse=false (GNU no-argument refusal). +setup: + files: + - path: data.txt + content: "b\na\n" +input: + allowed_paths: ["$DIR"] + script: |+ + sort --reverse=false data.txt; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + sort: option '--reverse' doesn't allow an argument + Try 'sort --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/wc/flags/reject_lines_equals_value.yaml b/tests/scenarios/cmd/wc/flags/reject_lines_equals_value.yaml new file mode 100644 index 000000000..3e4116e64 --- /dev/null +++ b/tests/scenarios/cmd/wc/flags/reject_lines_equals_value.yaml @@ -0,0 +1,22 @@ +# wc -l / --lines is a no-argument option; the explicit-value form must be +# rejected. pflag.BoolP would have silently accepted --lines=true. The GNU +# wording is bash-verified by df's non-skipped rejection scenarios (same +# flagparser path); bash assertion is skipped here only because the rshell and +# GNU coreutils exit-code conventions for flag errors are asserted elsewhere. +description: wc rejects --lines=true (GNU no-argument refusal). +setup: + files: + - path: data.txt + content: "a\nb\n" +input: + allowed_paths: ["$DIR"] + script: |+ + wc --lines=true data.txt; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + wc: option '--lines' doesn't allow an argument + Try 'wc --help' for more information. + exit_code: 0 +skip_assert_against_bash: true From 36f692fb1aeb47bd5dda4a952053c98af9bece35 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Wed, 24 Jun 2026 14:18:38 +0200 Subject: [PATCH 2/2] fix(builtins): reject --flag=value on custom no-argument bool flags The no-argument migration converted Bool/BoolP flags to builtins.NoArgBool but left the custom pflag.Value bool flags untouched, so they still silently accepted the explicit-value form GNU getopt rejects: - grep -H/-h/-l/-L (orderedBoolFlag) accepted --with-filename=true via strconv.ParseBool - strings -o (octalFlagVal) ignored -o=x - xargs -0/--null (trackedBool) accepted --null=true via strconv.ParseBool - head -q/--quiet/-v/--verbose (boolSeqFlag) used a literal "true" NoOptDefVal sentinel, which is forgeable via =true, so --quiet=true was silently accepted (only --quiet=false was rejected) All four now use the unforgeable builtins.NoArgSentinel (NUL byte) and return "flag does not allow an argument", which the flagparser rewrites to GNU's "option '--foo' doesn't allow an argument". head additionally gains the precise per-flag GNU wording it previously lacked. pwd/cd already use a NUL sentinel and reject correctly; du's seqBool still intentionally accepts --flag=false. Adds reject scenarios for grep/strings/xargs and tightens head's existing scenario to cover the previously-accepted =true case. Co-Authored-By: Claude Opus 4.8 (1M context) --- builtins/grep/grep.go | 21 ++++++++-------- builtins/head/head.go | 23 +++++++++-------- builtins/strings_cmd/strings.go | 14 +++++++---- builtins/xargs/xargs.go | 19 +++++++------- .../reject_with_filename_equals_value.yaml | 24 ++++++++++++++++++ .../errors/boolean_flag_with_argument.yaml | 25 +++++++++++++------ .../reject_offset_octal_equals_value.yaml | 22 ++++++++++++++++ .../xargs/null/reject_null_equals_value.yaml | 17 +++++++++++++ 8 files changed, 122 insertions(+), 43 deletions(-) create mode 100644 tests/scenarios/cmd/grep/flags/reject_with_filename_equals_value.yaml create mode 100644 tests/scenarios/cmd/strings/errors/reject_offset_octal_equals_value.yaml create mode 100644 tests/scenarios/cmd/xargs/null/reject_null_equals_value.yaml diff --git a/builtins/grep/grep.go b/builtins/grep/grep.go index 0a1a2d034..f47e9d57d 100644 --- a/builtins/grep/grep.go +++ b/builtins/grep/grep.go @@ -182,16 +182,16 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { filesWithoutMatch := newOrderedBoolFlag(&outputSeq) fs.VarP(filesWithMatches, "files-with-matches", "l", "print only names of files with matches") fs.VarP(filesWithoutMatch, "files-without-match", "L", "print only names of files without matches") - fs.Lookup("files-with-matches").NoOptDefVal = "true" - fs.Lookup("files-without-match").NoOptDefVal = "true" + fs.Lookup("files-with-matches").NoOptDefVal = builtins.NoArgSentinel + fs.Lookup("files-without-match").NoOptDefVal = builtins.NoArgSentinel lineNumber := builtins.NoArgBool(fs, "line-number", "n", "prefix output with line numbers") var filenameSeq int withFilename := newOrderedBoolFlag(&filenameSeq) noFilename := newOrderedBoolFlag(&filenameSeq) fs.VarP(withFilename, "with-filename", "H", "always print filename prefix") fs.VarP(noFilename, "no-filename", "h", "suppress filename prefix") - fs.Lookup("with-filename").NoOptDefVal = "true" - fs.Lookup("no-filename").NoOptDefVal = "true" + fs.Lookup("with-filename").NoOptDefVal = builtins.NoArgSentinel + fs.Lookup("no-filename").NoOptDefVal = builtins.NoArgSentinel onlyMatching := builtins.NoArgBool(fs, "only-matching", "o", "print only the matched parts") quiet := builtins.NoArgBool(fs, "quiet", "q", "suppress all output") _ = builtins.NoArgBool(fs, "silent", "", "alias for --quiet") @@ -419,13 +419,12 @@ func (f *orderedBoolFlag) String() string { } func (f *orderedBoolFlag) Set(s string) error { - b, err := strconv.ParseBool(s) - if err != nil { - return err - } - if !b { - f.pos = 0 - return nil + // These flags are GNU no-argument options: bare --with-filename / -H + // work, but the explicit-value form (--with-filename=true) must be + // rejected. pflag passes builtins.NoArgSentinel for the bare form and + // the user's literal value otherwise; anything else means --flag=value. + if s != builtins.NoArgSentinel { + return errors.New("flag does not allow an argument") } *f.seq = *f.seq + 1 f.pos = *f.seq diff --git a/builtins/head/head.go b/builtins/head/head.go index 0f70a74b2..439f00c8c 100644 --- a/builtins/head/head.go +++ b/builtins/head/head.go @@ -93,15 +93,17 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // quietFlag, silentFlag, and verboseFlag share a sequence counter so that // after parsing we can determine which of -q/--quiet/--silent/-v/--verbose // appeared last on the command line — the last flag wins, matching GNU head. - // NoOptDefVal is set to "true" so pflag treats these as boolean flags that - // can be given without a "=value" argument (e.g. "--quiet" not "--quiet=true"). + // NoOptDefVal is the NUL sentinel so pflag treats these as GNU no-argument + // boolean flags: bare "--quiet" works while "--quiet=true" / "--quiet=false" + // are rejected. A literal "true" sentinel would be forgeable via =true and + // let "--quiet=true" through silently. var headerSeq int quietFlag := newBoolSeqFlag(&headerSeq) silentFlag := newBoolSeqFlag(&headerSeq) verboseFlag := newBoolSeqFlag(&headerSeq) - fs.VarPF(quietFlag, "quiet", "q", "never print file name headers").NoOptDefVal = "true" - fs.VarPF(silentFlag, "silent", "", "alias for --quiet").NoOptDefVal = "true" - fs.VarPF(verboseFlag, "verbose", "v", "always print file name headers").NoOptDefVal = "true" + fs.VarPF(quietFlag, "quiet", "q", "never print file name headers").NoOptDefVal = builtins.NoArgSentinel + fs.VarPF(silentFlag, "silent", "", "alias for --quiet").NoOptDefVal = builtins.NoArgSentinel + fs.VarPF(verboseFlag, "verbose", "v", "always print file name headers").NoOptDefVal = builtins.NoArgSentinel // linesFlag and bytesFlag share a sequence counter so that after parsing // we can compare their pos fields to determine which appeared last on the @@ -465,11 +467,12 @@ func newBoolSeqFlag(seq *int) *boolSeqFlag { func (f *boolSeqFlag) String() string { return "false" } func (f *boolSeqFlag) Set(s string) error { // GNU head rejects --quiet= and --verbose= with an error. - // With NoOptDefVal = "true", pflag calls Set("true") for bare --quiet and - // Set("") when an explicit = is given. We accept only "true" - // (the NoOptDefVal) and reject any other value to match GNU head behaviour. - if s != "true" { - return errors.New("option doesn't allow an argument") + // pflag calls Set(builtins.NoArgSentinel) for the bare flag and Set("") + // for an explicit =; accept only the sentinel and reject any other + // value. Returning "flag does not allow an argument" lets the flagparser + // rewrite it to GNU's "option '--quiet' doesn't allow an argument" wording. + if s != builtins.NoArgSentinel { + return errors.New("flag does not allow an argument") } *f.seq++ f.pos = *f.seq diff --git a/builtins/strings_cmd/strings.go b/builtins/strings_cmd/strings.go index 6976d2627..cbf45153b 100644 --- a/builtins/strings_cmd/strings.go +++ b/builtins/strings_cmd/strings.go @@ -136,9 +136,13 @@ type octalFlagVal struct{ target *radixFormat } func (o *octalFlagVal) String() string { return "false" } func (o *octalFlagVal) Set(s string) error { - if s == "true" { - *o.target = radixOctal + // -o is a GNU no-argument option (alias for -t o); reject the + // explicit-value form (-o=x). pflag passes builtins.NoArgSentinel for + // the bare flag and the user's literal value otherwise. + if s != builtins.NoArgSentinel { + return errors.New("flag does not allow an argument") } + *o.target = radixOctal return nil } @@ -153,10 +157,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // whichever flag appears last on the command line wins (last-flag-wins). var format radixFormat fs.VarP(&radixFlagVal{target: &format}, "radix", "t", "print file offset in given radix: o=octal, d=decimal, x=hex") - // NoOptDefVal = "true" makes pflag treat -o as a no-argument boolean flag - // (same as BoolVarP does internally), so -o alone calls Set("true"). + // NoArgSentinel makes pflag treat -o as a GNU no-argument boolean flag: + // bare -o calls Set(NoArgSentinel) while -o=x is rejected. oFlag := fs.VarPF(&octalFlagVal{target: &format}, "offset-octal", "o", "alias for -t o (print octal offsets)") - oFlag.NoOptDefVal = "true" + oFlag.NoOptDefVal = builtins.NoArgSentinel printFileName := builtins.NoArgBool(fs, "print-file-name", "f", "print file name before each string") separator := fs.StringP("output-separator", "s", "\n", "output separator between strings (default newline)") diff --git a/builtins/xargs/xargs.go b/builtins/xargs/xargs.go index 3401280a6..994bec419 100644 --- a/builtins/xargs/xargs.go +++ b/builtins/xargs/xargs.go @@ -168,8 +168,9 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { var sep sepTracker null := new(bool) fs.VarP(&trackedBool{p: null, key: sepNull, t: &sep}, "null", "0", "input items are separated by a NUL character") - // Mark as a no-argument boolean flag so `-0` (no value) is accepted. - fs.Lookup("null").NoOptDefVal = "true" + // Mark as a GNU no-argument boolean flag: bare `-0` is accepted while + // `--null=value` is rejected. + fs.Lookup("null").NoOptDefVal = builtins.NoArgSentinel delim := new(string) fs.VarP(&trackedString2{p: delim, key: sepDelim, t: &sep}, "delimiter", "d", "use DELIM as the single-byte item separator") noRunIfEmpty := builtins.NoArgBool(fs, "no-run-if-empty", "r", "do not run command if input is empty") @@ -353,14 +354,14 @@ type trackedBool struct { func (b *trackedBool) String() string { return strconv.FormatBool(*b.p) } func (b *trackedBool) Set(s string) error { - v, err := strconv.ParseBool(s) - if err != nil { - return err - } - *b.p = v - if v { - b.t.last = b.key + // -0 / --null is a GNU no-argument option; reject the explicit-value + // form (--null=true). pflag passes builtins.NoArgSentinel for the bare + // flag and the user's literal value otherwise. + if s != builtins.NoArgSentinel { + return errors.New("flag does not allow an argument") } + *b.p = true + b.t.last = b.key return nil } func (b *trackedBool) Type() string { return "bool" } diff --git a/tests/scenarios/cmd/grep/flags/reject_with_filename_equals_value.yaml b/tests/scenarios/cmd/grep/flags/reject_with_filename_equals_value.yaml new file mode 100644 index 000000000..35394c906 --- /dev/null +++ b/tests/scenarios/cmd/grep/flags/reject_with_filename_equals_value.yaml @@ -0,0 +1,24 @@ +# grep -H / --with-filename is an ordered no-argument option implemented via a +# custom pflag.Value (orderedBoolFlag). Before the no-argument migration its +# Set used strconv.ParseBool and silently accepted --with-filename=true / +# =false, diverging from GNU getopt which rejects the explicit-value form. The +# GNU wording is bash-verified by df's non-skipped rejection scenarios (same +# flagparser path); bash assertion is skipped here only because rshell exits 1 +# uniformly on flag errors while GNU grep exits 2. +description: grep rejects --with-filename=true (GNU no-argument refusal). +setup: + files: + - path: data.txt + content: "apple\n" +input: + allowed_paths: ["$DIR"] + script: |+ + grep --with-filename=true apple data.txt; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + grep: option '--with-filename' doesn't allow an argument + Try 'grep --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/head/errors/boolean_flag_with_argument.yaml b/tests/scenarios/cmd/head/errors/boolean_flag_with_argument.yaml index 2ddc2e522..f5ff362a9 100644 --- a/tests/scenarios/cmd/head/errors/boolean_flag_with_argument.yaml +++ b/tests/scenarios/cmd/head/errors/boolean_flag_with_argument.yaml @@ -1,7 +1,9 @@ -# GNU head rejects --quiet= and --verbose= (options don't allow arguments). -# Our error message differs from GNU head so bash comparison is skipped. -description: head exits 1 when --quiet or --verbose are given an explicit argument. -# skip: rshell builtin error/output format differs from GNU coreutils +# GNU head rejects --quiet= and --verbose= for every value, +# including =true (options don't allow arguments). The previous "true" +# NoOptDefVal sentinel was forgeable, so --quiet=true was silently accepted; +# the NUL sentinel closes that. bash comparison is skipped because rshell +# exits 1 uniformly on flag errors. +description: head rejects --quiet=true and --quiet=false (GNU no-argument refusal). skip_assert_against_bash: true setup: files: @@ -10,8 +12,15 @@ setup: input: allowed_paths: ["$DIR"] script: |+ - head --quiet=false file.txt + head --quiet=true file.txt; echo "code:$?" + head --quiet=false file.txt; echo "code:$?" expect: - stdout: "" - stderr_contains: ["head:"] - exit_code: 1 + stdout: |+ + code:1 + code:1 + stderr: |+ + head: option '--quiet' doesn't allow an argument + Try 'head --help' for more information. + head: option '--quiet' doesn't allow an argument + Try 'head --help' for more information. + exit_code: 0 diff --git a/tests/scenarios/cmd/strings/errors/reject_offset_octal_equals_value.yaml b/tests/scenarios/cmd/strings/errors/reject_offset_octal_equals_value.yaml new file mode 100644 index 000000000..db75d4106 --- /dev/null +++ b/tests/scenarios/cmd/strings/errors/reject_offset_octal_equals_value.yaml @@ -0,0 +1,22 @@ +# strings -o / --offset-octal is a no-argument option (alias for -t o) +# implemented via a custom pflag.Value (octalFlagVal). Before the no-argument +# migration its Set silently ignored any explicit value (-o=x was accepted as +# a no-op), diverging from GNU getopt which rejects the explicit-value form. +# bash assertion is skipped because rshell exits 1 uniformly on flag errors. +description: strings rejects --offset-octal=true (GNU no-argument refusal). +setup: + files: + - path: data.bin + content: "hello\n" +input: + allowed_paths: ["$DIR"] + script: |+ + strings --offset-octal=true data.bin; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + strings: option '--offset-octal' doesn't allow an argument + Try 'strings --help' for more information. + exit_code: 0 +skip_assert_against_bash: true diff --git a/tests/scenarios/cmd/xargs/null/reject_null_equals_value.yaml b/tests/scenarios/cmd/xargs/null/reject_null_equals_value.yaml new file mode 100644 index 000000000..d8788fbb3 --- /dev/null +++ b/tests/scenarios/cmd/xargs/null/reject_null_equals_value.yaml @@ -0,0 +1,17 @@ +# xargs -0 / --null is a no-argument option implemented via a custom +# pflag.Value (trackedBool). Before the no-argument migration its Set used +# strconv.ParseBool and silently accepted --null=true / =false, diverging from +# GNU getopt which rejects the explicit-value form. bash assertion is skipped +# because rshell exits 1 uniformly on flag errors. +description: xargs rejects --null=true (GNU no-argument refusal). +input: + script: |+ + printf 'a\n' | xargs --null=true echo; echo "code:$?" +expect: + stdout: |+ + code:1 + stderr: |+ + xargs: option '--null' doesn't allow an argument + Try 'xargs --help' for more information. + exit_code: 0 +skip_assert_against_bash: true