naps62/maestro
Archived
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(maestro): harden command resolution

Browse Source
This commit is contained in:
Miguel Palhas
2026-09-05 14:05:52 +01:00
parent 0af91296df
commit 758bacf3fc
3 changed files with 134 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/maestro-protocol/src/display.rs
+58 -34
View File
@@ -274,14 +274,11 @@ fn legacy_first_word(command: &str) -> String {
}
}
/// One token from [`tokenize`]: its dequoted text, and — when a literal,
/// unquoted/unescaped `=` occurred in the source before any quoting or
/// escaping ever touched this word — the byte length of the name preceding
/// it. Only a *plain* `=` counts as an assignment's: quoting or escaping it
/// (`"FOO=1"`, `FOO\=1`) makes the word an executable command named
/// `FOO=1`, not an assignment (#199) — quoting elsewhere in the word (a
/// value, or even part of the name, as in `FO"O"=1`) doesn't disqualify it,
/// matching a real shell's own assignment-word grammar.
/// One token from [`tokenize`]: its dequoted text, and — when both the name
/// and `=` were entirely unquoted/unescaped in the source — the byte length
/// of the name. Quoting or escaping either (`"FOO"=1`, `F\OO=1`, `FOO\=1`)
/// makes the word an executable named `FOO=1`; quoting/escaping only the value
/// preserves the assignment, matching Bash's assignment-word grammar (#199).
#[derive(Debug, Clone, PartialEq, Eq)]
struct Word {
text: String,
@@ -303,6 +300,7 @@ fn tokenize(command: &str) -> Option<Vec<Word>> {
let mut current = String::new();
let mut has_current = false;
let mut plain_eq: Option<usize> = None;
let mut assignment_name_is_plain = true;
let mut chars = command.chars().peekable();
while let Some(c) = chars.next() {
@@ -314,6 +312,7 @@ fn tokenize(command: &str) -> Option<Vec<Word>> {
plain_eq: plain_eq.take(),
});
has_current = false;
assignment_name_is_plain = true;
}
}
'\n' => return None,
@@ -321,6 +320,9 @@ fn tokenize(command: &str) -> Option<Vec<Word>> {
'$' if chars.peek() == Some(&'(') => return None,
'\'' => {
has_current = true;
if plain_eq.is_none() {
assignment_name_is_plain = false;
}
loop {
match chars.next() {
Some('\'') => break,
@@ -331,6 +333,9 @@ fn tokenize(command: &str) -> Option<Vec<Word>> {
}
'"' => {
has_current = true;
if plain_eq.is_none() {
assignment_name_is_plain = false;
}
loop {
match chars.next() {
Some('"') => break,
@@ -352,13 +357,16 @@ fn tokenize(command: &str) -> Option<Vec<Word>> {
Some('\n') => {} // line continuation, removed entirely
Some(c2) => {
has_current = true;
if plain_eq.is_none() {
assignment_name_is_plain = false;
}
current.push(c2);
}
None => return None, // trailing backslash
},
other => {
has_current = true;
if other == '=' && plain_eq.is_none() {
if other == '=' && plain_eq.is_none() && assignment_name_is_plain {
plain_eq = Some(current.len());
}
current.push(other);
@@ -406,8 +414,8 @@ impl OptionTable {
if word == "-" || !word.starts_with('-') {
return Some(i);
}
if let Some((flag, _)) = word.split_once('=') {
return if self.valued.contains(&flag) {
if let Some((flag, value)) = word.split_once('=') {
return if self.valued.contains(&flag) && !value.is_empty() {
i += 1;
continue;
} else {
@@ -419,7 +427,10 @@ impl OptionTable {
continue;
}
if self.valued.contains(&word) {
words.get(i + 1)?;
let value = words.get(i + 1)?;
if value.is_empty() || value.starts_with('-') {
return None;
}
i += 2;
continue;
}
@@ -523,11 +534,19 @@ const COMMAND_BUILTIN_OPTS: OptionTable = OptionTable {
/// `/usr/bin/time`, a different program a bare `time` word doesn't
/// necessarily name. Assuming them here would be guessing which `time`
/// resolves (#199).
const TIME_OPTS: OptionTable = OptionTable {
flags: &["-p"],
valued: &[],
bundles_short_options: true,
};
fn resolve_time(words: &[&str], i: usize) -> Option<usize> {
let mut next = i + 1;
if words.get(next) == Some(&"-p") {
next += 1;
}
if words.get(next) == Some(&"--") {
next += 1;
}
match words.get(next) {
Some(word) if word.starts_with('-') => None,
_ => Some(next),
}
}
/// `-c`/`--call` embeds a whole shell command string rather than naming the
/// program positionally — resolving past it would mean guessing at a
@@ -549,7 +568,7 @@ const PNPM_OPTS: OptionTable = OptionTable {
],
// `--package`/`--package=value` is documented in this pre-`dlx`
// position (`pnpm --package=x dlx y`), alongside the identical flag
// `dlx` itself also accepts (`DLX_TRAILING_OPTS`).
// `dlx` itself also accepts (`PNPM_DLX_OPTS`).
valued: &["-C", "--dir", "--filter", "--package"],
bundles_short_options: false,
};
@@ -560,20 +579,21 @@ const YARN_OPTS: OptionTable = OptionTable {
bundles_short_options: false,
};
/// `--package`/`--package=value` right after `dlx`: both pnpm's and yarn's
/// `dlx` document it there, to run a binary whose name differs from the
/// package that provides it (#199).
const DLX_TRAILING_OPTS: OptionTable = OptionTable {
flags: &[],
const PNPM_DLX_OPTS: OptionTable = OptionTable {
flags: &["-s", "--silent"],
valued: &["--package"],
bundles_short_options: false,
};
/// `--bun`, forcing bun's own runtime instead of Node — bun's docs place it
/// right after the `x`/`bunx` trigger, before the executable (#199).
const YARN_DLX_OPTS: OptionTable = OptionTable {
flags: &["-q", "--quiet"],
valued: &["-p", "--package"],
bundles_short_options: false,
};
const BUN_X_OPTS: OptionTable = OptionTable {
flags: &["--bun"],
valued: &[],
flags: &["--bun", "--no-install", "--verbose", "--silent"],
valued: &["-p", "--package"],
bundles_short_options: false,
};
@@ -589,10 +609,10 @@ fn wrapper_resolver(name: &str) -> Option<WrapperResolver> {
"sudo" => Some(|words, i| SUDO_OPTS.consume(words, i + 1)),
"command" => Some(|words, i| COMMAND_BUILTIN_OPTS.consume(words, i + 1)),
"nohup" => Some(|words, i| NO_OPTIONS.consume(words, i + 1)),
"time" => Some(|words, i| TIME_OPTS.consume(words, i + 1)),
"time" => Some(resolve_time),
"npx" => Some(|words, i| NPX_OPTS.consume(words, i + 1)),
"pnpm" => Some(|words, i| resolve_dlx(&PNPM_OPTS, words, i)),
"yarn" => Some(|words, i| resolve_dlx(&YARN_OPTS, words, i)),
"pnpm" => Some(|words, i| resolve_dlx(&PNPM_OPTS, &PNPM_DLX_OPTS, words, i)),
"yarn" => Some(|words, i| resolve_dlx(&YARN_OPTS, &YARN_DLX_OPTS, words, i)),
"bun" => Some(resolve_bun),
"bunx" => Some(|words, i| BUN_X_OPTS.consume(words, i + 1)),
"uvx" => Some(|_words, i| Some(i + 1)),
@@ -603,14 +623,18 @@ fn wrapper_resolver(name: &str) -> Option<WrapperResolver> {
/// `pnpm`/`yarn` only look through to the package behind an explicit `dlx`
/// — without it, the next word is a `package.json` script name, not a
/// package to run (#199: `pnpm claude` is not `pnpm dlx claude`). `dlx`
/// itself takes options too (`DLX_TRAILING_OPTS`), consumed before the
/// executable.
fn resolve_dlx(opts: &OptionTable, words: &[&str], i: usize) -> Option<usize> {
/// itself takes launcher-specific options too, consumed before the executable.
fn resolve_dlx(
opts: &OptionTable,
dlx_opts: &OptionTable,
words: &[&str],
i: usize,
) -> Option<usize> {
let j = opts.consume(words, i + 1)?;
if words.get(j) != Some(&"dlx") {
return None;
}
DLX_TRAILING_OPTS.consume(words, j + 1)
dlx_opts.consume(words, j + 1)
}
/// `bun x <pkg>` is bun's `dlx`/`npx` equivalent; `bun run <script>` is not
fixtures/command-resolution.json
+33
View File
@@ -8,6 +8,8 @@
{ "command": "./bin/claude", "program": "claude", "tool": "claude" },
{ "command": "FOO=1 BAR=2 claude", "program": "claude", "tool": "claude" },
{ "command": "FOO=\"bar baz\" claude", "program": "claude", "tool": "claude" },
{ "command": "FOO='bar baz' claude", "program": "claude", "tool": "claude" },
{ "command": "FOO=bar\\ baz claude", "program": "claude", "tool": "claude" },
{ "command": "'/usr/local/bin/claude'", "program": "claude", "tool": "claude" },
{ "command": "\"/opt/my agent/claude\" --resume", "program": "claude", "tool": "claude" },
{ "command": "env -i claude", "program": "claude", "tool": "claude" },
@@ -18,6 +20,10 @@
{ "command": "nohup claude", "program": "claude", "tool": "claude" },
{ "command": "time claude", "program": "claude", "tool": "claude" },
{ "command": "time -p claude", "program": "claude", "tool": "claude" },
{ "command": "time -- claude", "program": "claude", "tool": "claude" },
{ "command": "time -p -- claude", "program": "claude", "tool": "claude" },
{ "command": "time -pp claude", "program": "time", "tool": null },
{ "command": "time -p -p claude", "program": "time", "tool": null },
{ "command": "time -o /tmp/t.log claude", "program": "time", "tool": null },
{ "command": "command claude", "program": "claude", "tool": "claude" },
{ "command": "command -v claude", "program": "command", "tool": null },
@@ -27,31 +33,58 @@
{ "command": "pnpm --silent dlx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm -C ./pkg dlx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm dlx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm dlx -s claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm dlx --silent claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn --silent dlx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn dlx -q claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn dlx --quiet claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun x claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bunx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "uvx claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun x --bun claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bunx --bun claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun x -p @anthropic-ai/claude-code claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bunx --package @anthropic-ai/claude-code claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun x --package=@anthropic-ai/claude-code claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bunx --no-install claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun x --verbose claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bunx --silent claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "bun --bun x claude-code", "program": "bun", "tool": null },
{ "command": "pnpm dlx --package foo claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm dlx --package=foo claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "pnpm dlx --package= claude-code", "program": "pnpm", "tool": null },
{ "command": "pnpm dlx --package", "program": "pnpm", "tool": null },
{ "command": "pnpm dlx -c \"claude-code\"", "program": "pnpm", "tool": null },
{ "command": "pnpm --package=typescript dlx tsc", "program": "tsc", "tool": null },
{ "command": "pnpm dlx --unknown-flag claude-code", "program": "pnpm", "tool": null },
{ "command": "yarn dlx --package foo claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn dlx --package=foo claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn dlx -p foo claude-code", "program": "claude-code", "tool": "claude" },
{ "command": "yarn dlx --package= claude-code", "program": "yarn", "tool": null },
{ "command": "yarn dlx -p", "program": "yarn", "tool": null },
{ "command": "yarn dlx -p --quiet claude-code", "program": "yarn", "tool": null },
{ "command": "bunx --package= claude-code", "program": "bunx", "tool": null },
{ "command": "bunx --package", "program": "bunx", "tool": null },
{ "command": "bun x -p --silent claude-code", "program": "bun", "tool": null },
{ "command": "sudo env -i claude", "program": "claude", "tool": "claude" },
{ "command": "FOO=1 sudo BAR=2 claude", "program": "claude", "tool": "claude" },
{ "command": "env -iv claude", "program": "claude", "tool": "claude" },
{ "command": "sudo -nE claude", "program": "claude", "tool": "claude" },
{ "command": "sudo -uroot claude", "program": "claude", "tool": "claude" },
{ "command": "sudo -u", "program": "sudo", "tool": null },
{ "command": "env --unset= claude", "program": "env", "tool": null },
{ "command": "sudo --user= claude", "program": "sudo", "tool": null },
{ "command": "sudo --user \"\" claude", "program": "sudo", "tool": null },
{ "command": "sudo -nu claude", "program": "sudo", "tool": null },
{ "command": "env -S 'python3 -u' script.py", "program": "env", "tool": null },
{ "command": "npx -c \"claude --version\"", "program": "npx", "tool": null },
{ "command": "npx --call \"claude --version\"", "program": "npx", "tool": null },
{ "command": "\"FOO=1\" claude", "program": "FOO=1", "tool": null },
{ "command": "FOO\\=1 claude", "program": "FOO=1", "tool": null },
{ "command": "F\"O\"O=1 claude", "program": "FOO=1", "tool": null },
{ "command": "F\\OO=1 claude", "program": "FOO=1", "tool": null },
{ "command": "\"FOO\"=1 claude", "program": "FOO=1", "tool": null },
{ "command": "F\"\"OO=1 claude", "program": "FOO=1", "tool": null },
{ "command": "claude\nclaude-code", "program": "claude", "tool": null },
{ "command": "clau\\\nde --resume", "program": "claude", "tool": "claude" },
{ "command": "npx opencode", "program": "opencode", "tool": "opencode" },
web/src/toolIcon.ts
+43 -33
View File
@@ -86,14 +86,11 @@ function legacyFirstWord(command: string): string {
return first === undefined ? command.trim() : basename(first);
}
/** One token from {@link tokenize}: its dequoted text, and — when a
* literal, unquoted/unescaped `=` occurred in the source before any
* quoting or escaping ever touched this word — the index of the name
* preceding it. Only a *plain* `=` counts as an assignment's: quoting or
* escaping it (`"FOO=1"`, `FOO\=1`) makes the word an executable command
* named `FOO=1`, not an assignment (#199) — quoting elsewhere in the word
* (a value, or even part of the name, as in `FO"O"=1`) doesn't disqualify
* it, matching a real shell's own assignment-word grammar. */
/** One token from {@link tokenize}: its dequoted text, and — when both the
* name and `=` were entirely unquoted/unescaped in the source — the index of
* the name. Quoting or escaping either (`"FOO"=1`, `F\OO=1`, `FOO\=1`)
* makes the word an executable named `FOO=1`; quoting/escaping only the value
* preserves the assignment, matching Bash's assignment-word grammar (#199). */
interface Word {
text: string;
plainEq: number | null;
@@ -150,6 +147,7 @@ function tokenize(command: string): Word[] | null {
let current = "";
let hasCurrent = false;
let plainEq: number | null = null;
let assignmentNameIsPlain = true;
let i = 0;
const n = command.length;
@@ -161,6 +159,7 @@ function tokenize(command: string): Word[] | null {
current = "";
hasCurrent = false;
plainEq = null;
assignmentNameIsPlain = true;
}
i += 1;
} else if (c === "\n") {
@@ -171,6 +170,7 @@ function tokenize(command: string): Word[] | null {
return null;
} else if (c === "'") {
hasCurrent = true;
if (plainEq === null) assignmentNameIsPlain = false;
i += 1;
const close = command.indexOf("'", i);
if (close === -1) return null; // unterminated single quote
@@ -178,6 +178,7 @@ function tokenize(command: string): Word[] | null {
i = close + 1;
} else if (c === '"') {
hasCurrent = true;
if (plainEq === null) assignmentNameIsPlain = false;
i += 1;
for (;;) {
if (i >= n) return null; // unterminated double quote
@@ -208,11 +209,12 @@ function tokenize(command: string): Word[] | null {
continue;
}
hasCurrent = true;
if (plainEq === null) assignmentNameIsPlain = false;
current += escaped;
i += 2;
} else {
hasCurrent = true;
if (c === "=" && plainEq === null) {
if (c === "=" && plainEq === null && assignmentNameIsPlain) {
plainEq = current.length;
}
current += c;
@@ -272,7 +274,7 @@ function consumeOptions(opts: OptionTable, words: string[], i: number): number |
if (word === "-" || !word.startsWith("-")) return i;
const eq = word.indexOf("=");
if (eq >= 0) {
if (opts.valued.has(word.slice(0, eq))) {
if (opts.valued.has(word.slice(0, eq)) && word.slice(eq + 1).length > 0) {
i += 1;
continue;
}
@@ -283,7 +285,8 @@ function consumeOptions(opts: OptionTable, words: string[], i: number): number |
continue;
}
if (opts.valued.has(word)) {
if (words[i + 1] === undefined) return null;
const value = words[i + 1];
if (value === undefined || value.length === 0 || value.startsWith("-")) return null;
i += 2;
continue;
}
@@ -348,11 +351,13 @@ const COMMAND_BUILTIN_OPTS: OptionTable = {
* `/usr/bin/time`, a different program a bare `time` word doesn't
* necessarily name. Assuming them here would be guessing which `time`
* resolves (#199). */
const TIME_OPTS: OptionTable = {
flags: new Set(["-p"]),
valued: new Set(),
bundlesShortOptions: true,
};
function resolveTime(words: string[], i: number): number | null {
let next = i + 1;
if (words[next] === "-p") next += 1;
if (words[next] === "--") next += 1;
const command = words[next];
return command?.startsWith("-") ? null : next;
}
/** `-c`/`--call` embeds a whole shell command string rather than naming the
* program positionally — resolving past it would mean guessing at a
@@ -368,7 +373,7 @@ const PNPM_OPTS: OptionTable = {
flags: new Set(["--silent", "-s", "--stream", "--aggregate-output", "--no-progress"]),
// `--package`/`--package=value` is documented in this pre-`dlx` position
// (`pnpm --package=x dlx y`), alongside the identical flag `dlx` itself
// also accepts (`DLX_TRAILING_OPTS`).
// also accepts (`PNPM_DLX_OPTS`).
valued: new Set(["-C", "--dir", "--filter", "--package"]),
bundlesShortOptions: false,
};
@@ -379,33 +384,38 @@ const YARN_OPTS: OptionTable = {
bundlesShortOptions: false,
};
/** `--package`/`--package=value` right after `dlx`: both pnpm's and yarn's
* `dlx` document it there, to run a binary whose name differs from the
* package that provides it (#199). */
const DLX_TRAILING_OPTS: OptionTable = {
flags: new Set(),
const PNPM_DLX_OPTS: OptionTable = {
flags: new Set(["-s", "--silent"]),
valued: new Set(["--package"]),
bundlesShortOptions: false,
};
/** `--bun`, forcing bun's own runtime instead of Node — bun's docs place it
* right after the `x`/`bunx` trigger, before the executable (#199). */
const YARN_DLX_OPTS: OptionTable = {
flags: new Set(["-q", "--quiet"]),
valued: new Set(["-p", "--package"]),
bundlesShortOptions: false,
};
const BUN_X_OPTS: OptionTable = {
flags: new Set(["--bun"]),
valued: new Set(),
flags: new Set(["--bun", "--no-install", "--verbose", "--silent"]),
valued: new Set(["-p", "--package"]),
bundlesShortOptions: false,
};
/** `pnpm`/`yarn` only look through to the package behind an explicit `dlx`
* — without it, the next word is a `package.json` script name, not a
* package to run (#199: `pnpm claude` is not `pnpm dlx claude`). `dlx`
* itself takes options too (`DLX_TRAILING_OPTS`), consumed before the
* executable. */
function resolveDlx(opts: OptionTable, words: string[], i: number): number | null {
* itself takes launcher-specific options too, consumed before the executable. */
function resolveDlx(
opts: OptionTable,
dlxOpts: OptionTable,
words: string[],
i: number,
): number | null {
const j = consumeOptions(opts, words, i + 1);
if (j === null) return null;
if (words[j] !== "dlx") return null;
return consumeOptions(DLX_TRAILING_OPTS, words, j + 1);
return consumeOptions(dlxOpts, words, j + 1);
}
/** `bun x <pkg>` is bun's `dlx`/`npx` equivalent; `bun run <script>` is not
@@ -428,10 +438,10 @@ const WRAPPER_RESOLVERS: Record<string, WrapperResolver> = {
sudo: (words, i) => consumeOptions(SUDO_OPTS, words, i + 1),
command: (words, i) => consumeOptions(COMMAND_BUILTIN_OPTS, words, i + 1),
nohup: (words, i) => consumeOptions(NO_OPTIONS, words, i + 1),
time: (words, i) => consumeOptions(TIME_OPTS, words, i + 1),
time: resolveTime,
npx: (words, i) => consumeOptions(NPX_OPTS, words, i + 1),
pnpm: (words, i) => resolveDlx(PNPM_OPTS, words, i),
yarn: (words, i) => resolveDlx(YARN_OPTS, words, i),
pnpm: (words, i) => resolveDlx(PNPM_OPTS, PNPM_DLX_OPTS, words, i),
yarn: (words, i) => resolveDlx(YARN_OPTS, YARN_DLX_OPTS, words, i),
bun: resolveBun,
bunx: (words, i) => consumeOptions(BUN_X_OPTS, words, i + 1),
uvx: (_words, i) => i + 1,