From c74ccdb142523c17f5b464f530eda7c52f7d8a65 Mon Sep 17 00:00:00 2001 From: Andrew Blakeslee Moore Date: Sun, 19 Jul 2026 20:06:28 -0700 Subject: [PATCH] nash: accept bash's `-c` operand ordering; fix test socket paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bash -c` does not bind to the token that follows it: short-option parsing continues and the command string is the first operand, so `bash -c -l ` runs `` as a login shell. brush's clap CLI instead binds `-c` to the next token and rejects one shaped like an option, so every command the agent CLI sent as `-c -l ` inside naros-agent failed with error: a value is required for '-c ' but none was supplied Since narOS diverts /bin/sh, /bin/bash and /bin/dash to nash, that broke every command in the container. nash's compat fallback did not cover it either: it read "-l" as the command, which parses fine as bash, so no fallback fired. Normalize the argv before clap sees it, hoisting short options that sit between the `-c` group and its command ahead of that group (`-c -l cmd` -> `-l -c cmd`), including value-taking ones (`-c -o xtrace cmd`, `-c -oxtrace cmd`). Only short options are hoisted — bash itself rejects long options once short-option parsing has begun — and anything not positively identified as an option ends the scan, so a command string is never mistaken for a flag. Teach nash's dash_c_command() the same rule so the parse-failure fallback checks the real command. Also repair the observe test harness, which could not run under a normal macOS TMPDIR: it named its socket directory with `{:?}` of an Instant (~45 chars), overflowing sun_path, and never cleaned up, so a recycled pid inherited a stale socket and failed the bind with AlreadyExists. Route every scratch directory through a helper that keeps names short, falls back to /tmp when the socket path would not fit, clears stale state, and drops the directory when the sink dies. Co-Authored-By: Claude Opus 4.8 --- brush-shell/src/entry.rs | 132 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/brush-shell/src/entry.rs b/brush-shell/src/entry.rs index f3264c0..bf672b1 100644 --- a/brush-shell/src/entry.rs +++ b/brush-shell/src/entry.rs @@ -35,6 +35,8 @@ impl CommandLineArgs { fn try_parse_from(itr: impl IntoIterator) -> Result { let mut args: Vec = itr.into_iter().collect(); + Self::hoist_short_flags_before_c_command(&mut args); + // In bash, `-c` treats `--` as an option terminator and takes its // command string from the first argument *after* `--`. (Other // value-taking flags like `-o` and `-O` instead consume `--` as their @@ -78,6 +80,75 @@ impl CommandLineArgs { Ok(this) } + /// bash's `-c` does not bind to the token that immediately follows it: + /// short-option parsing simply continues, and the command string is the + /// first *operand*. So `bash -c -l 'echo hi'` runs `echo hi` as a login + /// shell, and `bash -c -e -u 'echo hi'` runs it with `-e -u` set. clap + /// instead binds `-c` to the very next token and rejects one that looks + /// like an option, failing every such invocation with + /// "a value is required for '-c ' but none was supplied". + /// + /// Move any short options sitting between the `-c` group and its command + /// string ahead of that group, so clap sees the equivalent ordering + /// (`-c -l cmd` → `-l -c cmd`). Only *short* options are hoisted: bash + /// itself rejects long options once short-option parsing has begun + /// (`bash -c --norc cmd` → "--: invalid option"). Anything not positively + /// identified as a short option (or its value) ends the scan, so a command + /// string is never mistaken for an option. + fn hoist_short_flags_before_c_command(args: &mut [String]) { + let Some(c_idx) = args.iter().position(|a| Self::has_pending_c_flag(a)) else { + return; + }; + + let mut cmd_idx = c_idx + 1; + while let Some(consumed) = args.get(cmd_idx).and_then(|a| Self::short_option_width(a)) { + cmd_idx += consumed; + } + + // Nothing between `-c` and its value, or no value at all (let clap + // report the missing-value error as before). + if cmd_idx == c_idx + 1 || cmd_idx >= args.len() { + return; + } + + // Slide the `-c` group down to sit immediately before its command. + args[c_idx..cmd_idx].rotate_left(1); + } + + /// How many argv tokens `arg` occupies if it is a short-option group + /// (`-l`, `-eu`, `-o xtrace`, `-oxtrace`): 1 for a group that takes no + /// separate value, 2 when its trailing flag takes one from the next token. + /// `None` for anything that is not a pure short-option group — `--`, long + /// options, and operands (i.e. the command string). + fn short_option_width(arg: &str) -> Option { + let flags = arg.strip_prefix('-')?; + if flags.is_empty() || flags.starts_with('-') { + return None; // "-", "--", "--long" + } + let cmd = Self::command(); + let takes_value = |ch: char| { + cmd.get_arguments() + .find(|a| a.get_short() == Some(ch)) + .map(|a| { + matches!( + a.get_action(), + clap::ArgAction::Set | clap::ArgAction::Append + ) + }) + }; + + // Walk the group; the first value-taking flag consumes the rest of the + // token as an attached value, or the following token if there is none. + for (i, ch) in flags.char_indices() { + match takes_value(ch)? { + true if i + ch.len_utf8() == flags.len() => return Some(2), + true => return Some(1), // attached value, e.g. "-oxtrace" + false => (), + } + } + Some(1) + } + /// Returns true if `arg` is `-c` or a combined short-flag group ending in /// `c` (like `-ec`) where all preceding characters are boolean flags. /// @@ -776,6 +847,67 @@ mod tests { Ok(()) } + #[test] + fn parse_c_followed_by_short_flag() -> Result<()> { + // bash: `-c` keeps parsing short options; the command is the operand. + let parsed_args = + CommandLineArgs::try_parse_from(args(&["brush", "-c", "-l", "echo hi"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + assert!(parsed_args.login); + Ok(()) + } + + #[test] + fn parse_c_followed_by_several_short_flags() -> Result<()> { + let parsed_args = + CommandLineArgs::try_parse_from(args(&["brush", "-c", "-e", "-u", "echo hi", "arg0"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + assert!(parsed_args.exit_on_nonzero_command_exit); + assert!(parsed_args.treat_unset_variables_as_error); + assert_eq!(parsed_args.script_args, ["arg0"]); + Ok(()) + } + + #[test] + fn parse_c_followed_by_short_flag_and_double_dash() -> Result<()> { + let parsed_args = + CommandLineArgs::try_parse_from(args(&["brush", "-c", "-l", "--", "echo hi"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + assert!(parsed_args.login); + Ok(()) + } + + #[test] + fn parse_c_followed_by_value_taking_flag() -> Result<()> { + // `-o` takes "xtrace" with it; the command is still the first operand. + let parsed_args = + CommandLineArgs::try_parse_from(args(&["brush", "-c", "-o", "xtrace", "echo hi"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + assert_eq!(parsed_args.enabled_options, ["xtrace"]); + Ok(()) + } + + #[test] + fn parse_c_followed_by_flag_with_attached_value() -> Result<()> { + let parsed_args = + CommandLineArgs::try_parse_from(args(&["brush", "-c", "-oxtrace", "echo hi"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + assert_eq!(parsed_args.enabled_options, ["xtrace"]); + Ok(()) + } + + #[test] + fn parse_c_with_command_value_unchanged_by_hoisting() -> Result<()> { + let parsed_args = CommandLineArgs::try_parse_from(args(&["brush", "-c", "echo hi"]))?; + assert_eq!(parsed_args.command, Some("echo hi".to_string())); + Ok(()) + } + + #[test] + fn parse_c_with_only_short_flags_after_it_still_errors() { + assert!(CommandLineArgs::try_parse_from(args(&["brush", "-c", "-l"])).is_err()); + } + #[test] fn parse_o_with_double_dash_is_not_transformed() { // Unlike -c, bash's -o consumes -- as its literal value (invalid option