nash: accept bash's -c operand ordering; fix test socket paths
`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 <cmd>`
runs `<cmd>` 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 <cmd>` inside naros-agent failed with
error: a value is required for '-c <COMMAND>' 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 <[email protected]>
This commit is contained in:
@@ -35,6 +35,8 @@ impl CommandLineArgs {
|
|||||||
fn try_parse_from(itr: impl IntoIterator<Item = String>) -> Result<Self, clap::Error> {
|
fn try_parse_from(itr: impl IntoIterator<Item = String>) -> Result<Self, clap::Error> {
|
||||||
let mut args: Vec<String> = itr.into_iter().collect();
|
let mut args: Vec<String> = itr.into_iter().collect();
|
||||||
|
|
||||||
|
Self::hoist_short_flags_before_c_command(&mut args);
|
||||||
|
|
||||||
// In bash, `-c` treats `--` as an option terminator and takes its
|
// In bash, `-c` treats `--` as an option terminator and takes its
|
||||||
// command string from the first argument *after* `--`. (Other
|
// command string from the first argument *after* `--`. (Other
|
||||||
// value-taking flags like `-o` and `-O` instead consume `--` as their
|
// value-taking flags like `-o` and `-O` instead consume `--` as their
|
||||||
@@ -78,6 +80,75 @@ impl CommandLineArgs {
|
|||||||
Ok(this)
|
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 <COMMAND>' 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<usize> {
|
||||||
|
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
|
/// Returns true if `arg` is `-c` or a combined short-flag group ending in
|
||||||
/// `c` (like `-ec`) where all preceding characters are boolean flags.
|
/// `c` (like `-ec`) where all preceding characters are boolean flags.
|
||||||
///
|
///
|
||||||
@@ -776,6 +847,67 @@ mod tests {
|
|||||||
Ok(())
|
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]
|
#[test]
|
||||||
fn parse_o_with_double_dash_is_not_transformed() {
|
fn parse_o_with_double_dash_is_not_transformed() {
|
||||||
// Unlike -c, bash's -o consumes -- as its literal value (invalid option
|
// Unlike -c, bash's -o consumes -- as its literal value (invalid option
|
||||||
|
|||||||
Reference in New Issue
Block a user