fix(maestro): scope assignment-skipping to wrappers that accept it
`command FOO=1 claude` and `nohup FOO=1 claude` are invalid shells (they exec `FOO=1` as the program, exit 127), but the resolver skipped the assignment unconditionally after any wrapper. Assignment prefixes are only real at the start of a simple command or after a wrapper that itself interprets them as env vars (env, sudo) or a keyword that restarts the assignment grammar (time) — not after command, nohup, or a package manager's subcommand, which just exec their next argument as the program. Attach an accepts_assignment flag to each wrapper transition in both the Rust and TS resolvers and gate the assignment-skip loop on the flag from the most recently consumed wrapper. Extends the shared fixture with the two reported cases plus every other wrapper transition and several nested combinations (sudo+time valid, sudo+command / command+nohup / time+command invalid, nohup+env valid) to lock the per-wrapper policy in for both languages.
This commit is contained in:
@@ -235,23 +235,34 @@ struct Resolution {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Tokenize `command` and walk past leading `FOO=bar` assignments and known
|
/// Tokenize `command` and walk past leading `FOO=bar` assignments and known
|
||||||
/// wrapper forms (interleaved in any order/count) to find the program. `None`
|
/// wrapper forms (interleaved in any order/count) to find the program. An
|
||||||
/// when [`tokenize`] can't safely make sense of the line at all.
|
/// assignment run is only consumed at the very start of the line or right
|
||||||
|
/// after a wrapper whose own semantics accept one there ([`Wrapper::accepts_assignment`]) —
|
||||||
|
/// `command FOO=1 claude` and `nohup FOO=1 claude` stop at `FOO=1` rather than
|
||||||
|
/// skipping to `claude`, since `command`/`nohup` just exec their next argument
|
||||||
|
/// as the program, they don't setenv from an assignment-shaped one (#199).
|
||||||
|
/// `None` when [`tokenize`] can't safely make sense of the line at all.
|
||||||
fn resolve(command: &str) -> Option<Resolution> {
|
fn resolve(command: &str) -> Option<Resolution> {
|
||||||
let words = tokenize(command)?;
|
let words = tokenize(command)?;
|
||||||
let refs: Vec<&str> = words.iter().map(|w| w.text.as_str()).collect();
|
let refs: Vec<&str> = words.iter().map(|w| w.text.as_str()).collect();
|
||||||
let mut i = 0;
|
let mut i = 0;
|
||||||
while let Some(word) = words.get(i) {
|
let mut assignment_allowed = true;
|
||||||
if is_env_assignment(word) {
|
loop {
|
||||||
i += 1;
|
if assignment_allowed {
|
||||||
continue;
|
while matches!(words.get(i), Some(word) if is_env_assignment(word)) {
|
||||||
|
i += 1;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
let Some(word) = words.get(i) else { break };
|
||||||
let wrapper = basename(&word.text).to_lowercase();
|
let wrapper = basename(&word.text).to_lowercase();
|
||||||
let Some(resolver) = wrapper_resolver(&wrapper) else {
|
let Some(w) = wrapper_resolver(&wrapper) else {
|
||||||
break;
|
break;
|
||||||
};
|
};
|
||||||
match resolver(&refs, i) {
|
match (w.resolve)(&refs, i) {
|
||||||
Some(next) if next > i => i = next,
|
Some(next) if next > i => {
|
||||||
|
i = next;
|
||||||
|
assignment_allowed = w.accepts_assignment;
|
||||||
|
}
|
||||||
// Not a form of the wrapper we recognise (an unknown option, or
|
// Not a form of the wrapper we recognise (an unknown option, or
|
||||||
// — for `pnpm`/`yarn`/`bun` — no `dlx`/`x` subcommand behind it):
|
// — for `pnpm`/`yarn`/`bun` — no `dlx`/`x` subcommand behind it):
|
||||||
// the wrapper word itself is the program, not a launcher.
|
// the wrapper word itself is the program, not a launcher.
|
||||||
@@ -603,21 +614,45 @@ const BUN_X_OPTS: OptionTable = OptionTable {
|
|||||||
/// isn't a form we resolve through.
|
/// isn't a form we resolve through.
|
||||||
type WrapperResolver = fn(&[&str], usize) -> Option<usize>;
|
type WrapperResolver = fn(&[&str], usize) -> Option<usize>;
|
||||||
|
|
||||||
fn wrapper_resolver(name: &str) -> Option<WrapperResolver> {
|
/// One recognised wrapper form: how to walk past it, and whether an
|
||||||
match name {
|
/// assignment word is a valid follow-on once it's been walked past.
|
||||||
"env" => Some(|words, i| ENV_OPTS.consume(words, i + 1)),
|
struct Wrapper {
|
||||||
"sudo" => Some(|words, i| SUDO_OPTS.consume(words, i + 1)),
|
resolve: WrapperResolver,
|
||||||
"command" => Some(|words, i| COMMAND_BUILTIN_OPTS.consume(words, i + 1)),
|
/// `true` only for wrappers that themselves interpret a following
|
||||||
"nohup" => Some(|words, i| NO_OPTIONS.consume(words, i + 1)),
|
/// `FOO=bar` as an environment assignment for the command they launch
|
||||||
"time" => Some(resolve_time),
|
/// (`env`, `sudo`), or a shell keyword that restarts the assignment-prefix
|
||||||
"npx" => Some(|words, i| NPX_OPTS.consume(words, i + 1)),
|
/// grammar for a brand new simple command (`time`). Every other wrapper
|
||||||
"pnpm" => Some(|words, i| resolve_dlx(&PNPM_OPTS, &PNPM_DLX_OPTS, words, i)),
|
/// here is a plain executable that execs its next argument as the
|
||||||
"yarn" => Some(|words, i| resolve_dlx(&YARN_OPTS, &YARN_DLX_OPTS, words, i)),
|
/// program outright — an assignment-shaped one names a program literally
|
||||||
"bun" => Some(resolve_bun),
|
/// called `FOO=1`, not an env var (#199).
|
||||||
"bunx" => Some(|words, i| BUN_X_OPTS.consume(words, i + 1)),
|
accepts_assignment: bool,
|
||||||
"uvx" => Some(|_words, i| Some(i + 1)),
|
}
|
||||||
_ => None,
|
|
||||||
}
|
fn wrapper_resolver(name: &str) -> Option<Wrapper> {
|
||||||
|
let (resolve, accepts_assignment): (WrapperResolver, bool) = match name {
|
||||||
|
"env" => (|words, i| ENV_OPTS.consume(words, i + 1), true),
|
||||||
|
"sudo" => (|words, i| SUDO_OPTS.consume(words, i + 1), true),
|
||||||
|
"command" => (|words, i| COMMAND_BUILTIN_OPTS.consume(words, i + 1), false),
|
||||||
|
"nohup" => (|words, i| NO_OPTIONS.consume(words, i + 1), false),
|
||||||
|
"time" => (resolve_time, true),
|
||||||
|
"npx" => (|words, i| NPX_OPTS.consume(words, i + 1), false),
|
||||||
|
"pnpm" => (
|
||||||
|
|words, i| resolve_dlx(&PNPM_OPTS, &PNPM_DLX_OPTS, words, i),
|
||||||
|
false,
|
||||||
|
),
|
||||||
|
"yarn" => (
|
||||||
|
|words, i| resolve_dlx(&YARN_OPTS, &YARN_DLX_OPTS, words, i),
|
||||||
|
false,
|
||||||
|
),
|
||||||
|
"bun" => (resolve_bun, false),
|
||||||
|
"bunx" => (|words, i| BUN_X_OPTS.consume(words, i + 1), false),
|
||||||
|
"uvx" => (|_words, i| Some(i + 1), false),
|
||||||
|
_ => return None,
|
||||||
|
};
|
||||||
|
Some(Wrapper {
|
||||||
|
resolve,
|
||||||
|
accepts_assignment,
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `pnpm`/`yarn` only look through to the package behind an explicit `dlx`
|
/// `pnpm`/`yarn` only look through to the package behind an explicit `dlx`
|
||||||
|
|||||||
@@ -79,6 +79,19 @@
|
|||||||
{ "command": "env -S 'python3 -u' script.py", "program": "env", "tool": null },
|
{ "command": "env -S 'python3 -u' script.py", "program": "env", "tool": null },
|
||||||
{ "command": "npx -c \"claude --version\"", "program": "npx", "tool": null },
|
{ "command": "npx -c \"claude --version\"", "program": "npx", "tool": null },
|
||||||
{ "command": "npx --call \"claude --version\"", "program": "npx", "tool": null },
|
{ "command": "npx --call \"claude --version\"", "program": "npx", "tool": null },
|
||||||
|
{ "command": "command FOO=1 claude", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "nohup FOO=1 claude", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "npx FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "pnpm dlx FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "yarn dlx FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "bun x FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "bunx FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "uvx FOO=1 claude-code", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "sudo time FOO=1 claude", "program": "claude", "tool": "claude" },
|
||||||
|
{ "command": "nohup env FOO=1 claude", "program": "claude", "tool": "claude" },
|
||||||
|
{ "command": "sudo command FOO=1 claude", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "command nohup FOO=1 claude", "program": "FOO=1", "tool": null },
|
||||||
|
{ "command": "time command FOO=1 claude", "program": "FOO=1", "tool": null },
|
||||||
{ "command": "\"FOO=1\" claude", "program": "FOO=1", "tool": null },
|
{ "command": "\"FOO=1\" claude", "program": "FOO=1", "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\"O\"O=1 claude", "program": "FOO=1", "tool": null },
|
||||||
|
|||||||
+64
-20
@@ -105,24 +105,33 @@ function isEnvAssignment(word: Word): boolean {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Walk `words` past leading `FOO=bar` assignments and known wrapper forms
|
/** Walk `words` past leading `FOO=bar` assignments and known wrapper forms
|
||||||
* (interleaved in any order/count) to the index of the actual program. */
|
* (interleaved in any order/count) to the index of the actual program. An
|
||||||
|
* assignment run is only consumed at the very start of the line or right
|
||||||
|
* after a wrapper whose own semantics accept one there ({@link Wrapper.acceptsAssignment}) —
|
||||||
|
* `command FOO=1 claude` and `nohup FOO=1 claude` stop at `FOO=1` rather than
|
||||||
|
* skipping to `claude`, since `command`/`nohup` just exec their next argument
|
||||||
|
* as the program, they don't setenv from an assignment-shaped one (#199). */
|
||||||
function programIndex(words: Word[]): number {
|
function programIndex(words: Word[]): number {
|
||||||
const texts = words.map((w) => w.text);
|
const texts = words.map((w) => w.text);
|
||||||
let i = 0;
|
let i = 0;
|
||||||
while (i < words.length) {
|
let assignmentAllowed = true;
|
||||||
if (isEnvAssignment(words[i])) {
|
for (;;) {
|
||||||
i += 1;
|
if (assignmentAllowed) {
|
||||||
continue;
|
while (i < words.length && isEnvAssignment(words[i])) {
|
||||||
|
i += 1;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
if (i >= words.length) break;
|
||||||
const wrapper = basename(texts[i]).toLowerCase();
|
const wrapper = basename(texts[i]).toLowerCase();
|
||||||
const resolver = WRAPPER_RESOLVERS[wrapper];
|
const w = WRAPPER_RESOLVERS[wrapper];
|
||||||
if (!resolver) break;
|
if (!w) break;
|
||||||
const next = resolver(texts, i);
|
const next = w.resolve(texts, i);
|
||||||
// Not a form of the wrapper we recognise (an unknown option, or — for
|
// Not a form of the wrapper we recognise (an unknown option, or — for
|
||||||
// `pnpm`/`yarn`/`bun` — no `dlx`/`x` subcommand behind it): the wrapper
|
// `pnpm`/`yarn`/`bun` — no `dlx`/`x` subcommand behind it): the wrapper
|
||||||
// word itself is the program, not a launcher.
|
// word itself is the program, not a launcher.
|
||||||
if (next === null || next <= i) break;
|
if (next === null || next <= i) break;
|
||||||
i = next;
|
i = next;
|
||||||
|
assignmentAllowed = w.acceptsAssignment;
|
||||||
}
|
}
|
||||||
return i;
|
return i;
|
||||||
}
|
}
|
||||||
@@ -433,18 +442,53 @@ function resolveBun(words: string[], i: number): number | null {
|
|||||||
* isn't a form we resolve through. */
|
* isn't a form we resolve through. */
|
||||||
type WrapperResolver = (words: string[], i: number) => number | null;
|
type WrapperResolver = (words: string[], i: number) => number | null;
|
||||||
|
|
||||||
const WRAPPER_RESOLVERS: Record<string, WrapperResolver> = {
|
/** One recognised wrapper form: how to walk past it, and whether an
|
||||||
env: (words, i) => consumeOptions(ENV_OPTS, words, i + 1),
|
* assignment word is a valid follow-on once it's been walked past. */
|
||||||
sudo: (words, i) => consumeOptions(SUDO_OPTS, words, i + 1),
|
interface Wrapper {
|
||||||
command: (words, i) => consumeOptions(COMMAND_BUILTIN_OPTS, words, i + 1),
|
resolve: WrapperResolver;
|
||||||
nohup: (words, i) => consumeOptions(NO_OPTIONS, words, i + 1),
|
/** `true` only for wrappers that themselves interpret a following
|
||||||
time: resolveTime,
|
* `FOO=bar` as an environment assignment for the command they launch
|
||||||
npx: (words, i) => consumeOptions(NPX_OPTS, words, i + 1),
|
* (`env`, `sudo`), or a shell keyword that restarts the assignment-prefix
|
||||||
pnpm: (words, i) => resolveDlx(PNPM_OPTS, PNPM_DLX_OPTS, words, i),
|
* grammar for a brand new simple command (`time`). Every other wrapper here
|
||||||
yarn: (words, i) => resolveDlx(YARN_OPTS, YARN_DLX_OPTS, words, i),
|
* is a plain executable that execs its next argument as the program
|
||||||
bun: resolveBun,
|
* outright — an assignment-shaped one names a program literally called
|
||||||
bunx: (words, i) => consumeOptions(BUN_X_OPTS, words, i + 1),
|
* `FOO=1`, not an env var (#199). */
|
||||||
uvx: (_words, i) => i + 1,
|
acceptsAssignment: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
const WRAPPER_RESOLVERS: Record<string, Wrapper> = {
|
||||||
|
env: { resolve: (words, i) => consumeOptions(ENV_OPTS, words, i + 1), acceptsAssignment: true },
|
||||||
|
sudo: {
|
||||||
|
resolve: (words, i) => consumeOptions(SUDO_OPTS, words, i + 1),
|
||||||
|
acceptsAssignment: true,
|
||||||
|
},
|
||||||
|
command: {
|
||||||
|
resolve: (words, i) => consumeOptions(COMMAND_BUILTIN_OPTS, words, i + 1),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
nohup: {
|
||||||
|
resolve: (words, i) => consumeOptions(NO_OPTIONS, words, i + 1),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
time: { resolve: resolveTime, acceptsAssignment: true },
|
||||||
|
npx: {
|
||||||
|
resolve: (words, i) => consumeOptions(NPX_OPTS, words, i + 1),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
pnpm: {
|
||||||
|
resolve: (words, i) => resolveDlx(PNPM_OPTS, PNPM_DLX_OPTS, words, i),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
yarn: {
|
||||||
|
resolve: (words, i) => resolveDlx(YARN_OPTS, YARN_DLX_OPTS, words, i),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
bun: { resolve: resolveBun, acceptsAssignment: false },
|
||||||
|
bunx: {
|
||||||
|
resolve: (words, i) => consumeOptions(BUN_X_OPTS, words, i + 1),
|
||||||
|
acceptsAssignment: false,
|
||||||
|
},
|
||||||
|
uvx: { resolve: (_words, i) => i + 1, acceptsAssignment: false },
|
||||||
};
|
};
|
||||||
|
|
||||||
/** Anthropic's burst: tapered rays around a centre, long ones on the axes. */
|
/** Anthropic's burst: tapered rays around a centre, long ones on the axes. */
|
||||||
|
|||||||
Reference in New Issue
Block a user