fix(sandbox): classify destructive commands from bash AST

[gouhongshen]

What type of PR is this?

• feat (new feature)
• fix (bug fix)
• docs (documentation)
• style (formatting, no code change)
• refactor (code change that neither fixes a bug nor adds a feature)
• perf (performance improvement)
• test (adding or updating tests)
• chore (maintenance, tooling)
• build / ci (build or CI changes)

Which issue(s) this PR fixes

N/A — main-branch port of the production MOI false-positive fix in #696.

What this PR does / why we need it

The Bash validator previously scanned the complete raw command text for destructive command names. That treated heredoc bodies and inline interpreter programs as shell commands, so an ordinary Python identifier such as dd could block the whole tool call and force the agent into repeated rewrites.

This port integrates the fix with main's stricter literal-argv parser and makes the Bash AST the canonical owner of destructive executable classification:

• classify only executable positions, including path-qualified commands and transparent launchers;
• recursively analyze literal bash/sh/zsh -c programs;
• parse shell and launcher option grammar explicitly, including value arity, and fail closed when an executable boundary is ambiguous;
• resolve busybox/toybox, xargs, find -exec/-execdir, timeout, nice, ionice, setsid, stdbuf, taskset, chroot, and unshare dispatch surfaces through one shared resolver contract;
• distinguish Dispatch, NoDispatch, and Ambiguous, so actual query/terminal modes do not inspect unexecuted operands;
• consume option values and required operands only when one source word is guaranteed to remain one runtime argv entry;
• model GNU xargs --eof[=END], --replace[=R], and --max-lines[=MAX-LINES] as optional inline-value forms; --show-limits continues to the child, while only --help/--version terminate dispatch;
• preserve dynamic-word provenance for find: an unknown quoted scalar is not assumed to be a path, while an explicit literal ./, ../, or / prefix proves the operand cannot become a predicate;
• preserve every configured destructive executable, process-control rule, fork-bomb rule, and independent rm -rf guard;
• remove duplicate raw-text command patterns from astra-tools, so heredoc and inline Python/Node data no longer trigger false positives.

Architecture and complexity delta

• Canonical owner changed or extended: astra-sandbox Bash AST risk analysis owns destructive executable classification and reuses main's literal command parser.
• Trust boundary: OS isolation and workspace capability enforcement remain authoritative. The AST registry is a finite, declarative defense-in-depth layer for standard dispatch surfaces in Astra's supported runtime environments.
• Existing implementations/callers searched: Bash command parsing, command-risk analysis, public execute_bash validation, wrapper handling, nested shell launchers, and existing risk tests.
• Superseded code, states, tables, shims, and self-only tests removed: the legacy raw-text destructive-command tokenizer and duplicate astra-tools executable patterns.
• Net code/state/table delta: three existing Rust files; no protocol, persistent state, database schema, configuration, fallback, or compatibility layer.
• Remaining parallel checks: shell syntax checks such as fork bombs and the dedicated catastrophic rm -rf guard remain in astra-tools; executable classification has one owner in astra-sandbox.

Production wiring and verification

• Public product entrypoint exercised: the execute_bash validator used by Astra tool execution.
• Unhappy paths exercised: every configured destructive executable, wrappers and value-bearing options, split-capable dynamic option values/operands, nested shell programs, multi-call applets, supported dispatchers, GNU xargs optional-value and nonterminal modes, find -exec/-execdir, split-capable and quoted-predicate dynamic find expressions, runtime-dependent executable positions, query/terminal modes, unknown option arity, malformed input, heredoc Python data, inline interpreter source, and benign controls.
• Database verification: N/A; command validation is in-memory and this PR does not touch persistence.

Verification:

• cargo fmt --check -- crates/astra-sandbox/src/bash_ast.rs crates/astra-tools/src/shell_ops.rs
• cargo test -p astra-sandbox --lib — 161 passed
• cargo test -p astra-tools validate_execute_bash --lib — 12 passed
• cargo clippy -p astra-sandbox -p astra-tools --all-targets -- -D warnings
• Linux CI also executes harmless GNU xargs --show-limits, bare --max-lines, and quoted dynamic find -print probes so the registry is checked against real utility dispatch semantics.

[XuPeng-SH]

方向正确：用 Bash AST 区分命令与 heredoc/解释器数据，解决 dd 文本误报是合理的。但当前 head 引入了一个安全绕过，需修复后再合并。

[P1] 长选项中任意字符 c 被误认为 shell 的 -c

nested_shell_script 使用 argument[1..].chars().any(|flag| flag == 'c') 判断 command-string 选项：

Astra/crates/astra-sandbox/src/bash_ast.rs

Lines 363 to 383 in f0e9e4e

	fn nested_shell_script(words: &[String]) -> Option<&str> {
	let index = effective_command_index(words)?;
	let executable = command_basename(words.get(index)?);
	if !matches!(executable.as_str(), "bash" | "sh" | "dash" | "zsh" | "ksh") {
	return None;
	}
	let mut argument_index = index + 1;
	while let Some(raw) = words.get(argument_index) {
	let argument = unquote_shell_word(raw);
	if argument == "--" || !argument.starts_with('-') || argument == "-" {
	return None;
	}
	if argument[1..].chars().any(|flag| flag == 'c') {
	return words
	.get(argument_index + 1)
	.map(|script| unquote_shell_word(script));
	}
	argument_index += 1;
	}
	None

因此常见命令 bash --norc -c 'dd if=/dev/zero of=/dev/sda' 会把 --norc 当作 -c，把下一个参数字面量 -c 当作脚本递归解析，真正的 destructive script 则完全跳过。bash --rcfile file -c ... 也有同类问题。由于本 PR 同时移除了 destructive substring fallback，这些命令会从 DestructiveCommand 降为未识别，形成策略绕过。

请区分精确长选项与合法的短选项簇，并正确跳过会消费参数的 shell 选项；至少补 bash --norc -c ...、bash --rcfile file -c ...（以及等价 wrapper）回归测试。解析无法可靠解析 executable/-c 边界时，安全策略应 fail closed，同时继续保证 Python heredoc 中普通 dd 数据不误报。

[gouhongshen]

已在 bdb5681b 修复，这条 P1 成立。

• 长选项改为精确匹配，不再把 --norc 中的 c 当成 -c；
• --rcfile / --init-file 会正确消费参数；
• 短选项簇按真实参数消费顺序处理，包括 -lc、-oc pipefail、-oO pipefail extglob -c；
• 无法可靠确定 shell command-string 边界时标记为 RemoteCodeExecution 并 fail closed；没有恢复原始命令全文扫描，因此 Python heredoc 中的 dd 仍不会误报。

新增了 bash --norc -c、bash --rcfile ... -c、sudo / env wrapper、短选项簇和歧义参数的回归覆盖。astra-sandbox 159 个单测、execute_bash 相关 10 个单测及两个 crate 的 all-target clippy 均通过。等价修复也已同步到 moi-dev 的 #696（7db624c4）。请重新 review。

[XuPeng-SH]

重新审查了最新 head bdb5681b。--norc/--rcfile 被误判为 -c 的问题已经修复，短选项簇、消费参数的选项、未知选项 fail-closed 及对应 validator 测试都合理。

仍有一个安全阻断项：

[P1] executable dispatcher 和动态 executable 仍可绕过 destructive 检测

当前 destructive_command_name 只检查 effective_command_index 指向的 executable：

Astra/crates/astra-sandbox/src/bash_ast.rs

Lines 459 to 492 in bdb5681

	const DESTRUCTIVE_COMMANDS: &[&str] = &[
	"dd",
	"mkswap",
	"truncate",
	"shred",
	"wipefs",
	"blkdiscard",
	"fdisk",
	"sfdisk",
	"parted",
	"cryptsetup",
	"pvremove",
	"vgremove",
	"lvremove",
	"zpool",
	"zfs",
	"shutdown",
	"reboot",
	"poweroff",
	"halt",
	"telinit",
	];
	fn destructive_command_name(words: &[String]) -> Option<&'static str> {
	let index = effective_command_index(words)?;
	let executable = command_basename(words.get(index)?);
	if executable == "mkfs" || executable.starts_with("mkfs.") {
	return Some("mkfs");
	}
	DESTRUCTIVE_COMMANDS
	.iter()
	.copied()
	.find(|candidate| executable.eq_ignore_ascii_case(candidate))
	}

因此 busybox dd if=/dev/zero of=/dev/sda 的 executable 是 busybox，真实执行的 applet dd 完全不会被识别；tool=dd; "$tool" if=/dev/zero of=/dev/sda 也因 command name 无法静态还原而直接漏过。两者在本 PR 移除 token fallback 前都会被识别，而现在不会产生 DestructiveCommand/fail-closed risk。仓库现有 rm validator 已经显式承认 busybox/toybox 这类 multi-call binary，因此该执行形态不是假设场景。

请把 command resolution 建模为“直接 executable / transparent launcher / multi-call dispatcher / unresolved dynamic”而不是继续堆字符串特例：至少覆盖 busybox/toybox applet；对位于 command position 且无法证明安全的动态 executable fail closed。也请评估并测试 xargs、find -exec/-execdir 这类从 argv 调度命令的边界，同时保留 Python/Node/heredoc 数据不被当成 shell command 的目标。

[gouhongshen]

已按该 P1 的执行语义处理，修复在 7e730079：

• 用统一的 command resolver 区分直接 executable、transparent launcher、multi-call dispatcher 和无法静态确定的 executable；没有恢复全文/token 子串扫描。
• busybox/toybox 会继续解析真实 applet；xargs 与 find -exec/-execdir 会继续解析被调度命令，嵌套 sh -c 也递归检查。
• 仅真实 executable 位置无法静态确定时 fail closed；Python/Node/heredoc 内容和普通参数仍作为数据，不会因出现 dd 被误拦。
• 增加了上述危险、动态和 benign 对照用例，并保留 malformed input 的原有行为。

验证通过：astra-sandbox 全部 160 个 lib tests、execute_bash 目标测试 10 个、受影响 crates 的 clippy -D warnings。同一修复已同步到 #696（df3bd3b1）。已更新 PR 描述。

[XuPeng-SH]

The AST direction is good, but the transparent-launcher parser is still fail-open for valid options that consume values. skip_literal_options silently treats every unknown option as a no-value flag. For example, Bash executes exec -a alias dd if=/dev/zero of=/dev/sda as dd, while this resolver skips -a, treats alias as the executable, and classifies the command as safe. Similar gaps exist whenever a supported launcher gains or already has an omitted value-bearing option.

This is a policy-boundary bypass, not an allowlist completeness nit. Please model the exact option grammar for each supported launcher, or return Ambiguous for options whose arity is not proven, then add public validator tests for destructive commands behind value-bearing launcher options. The existing exec -a workspace-write test is caught by independent destination-path analysis and does not cover this destructive-command bypass. Also rebase onto latest main; this head is behind #698.

[gouhongshen]

已修复这个 P1，并已 rebase 到包含 #698 的最新 main。当前 head 为 a687143f。

• 不再把未知 launcher option 默认视为无参数 flag；command、builtin、exec、nohup、env、sudo、doas、pkexec 分别声明已知短/长选项及参数 arity，未知 arity 统一返回 Ambiguous。
• exec -a alias dd ...、短选项簇、动态 option value，以及其他 launcher 的 value-bearing options 都会定位到真实 executable；env -S 这类会重新解释字符串且当前无法静态证明边界的形式 fail closed。
• 公共 execute_bash validator 增加了 exec -a、sudo -D、doas -a、pkexec --user、env -u 后隐藏 destructive command 的回归用例，并增加 benign 对照。

验证通过：astra-sandbox 全部 161 个 lib tests、目标 validator 10 个测试、受影响 crates 的 clippy -D warnings。PR 描述已同步更新。

[gouhongshen]

The same launcher-option arity fix is now synchronized to moi-dev in follow-up PR #701 because #696 had already merged before this review arrived.

[XuPeng-SH]

One blocking dispatcher gap remains in the new canonical executable resolver.\n\n recognizes several wrappers, but not standard command dispatchers such as , 0, none: prio 0, or . Because this PR removes the legacy destructive-token scan, a command such as resolves only the outer executable and is classified safe; the remaining command-specific checks do not catch or . The same applies after an already-supported wrapper, e.g. .\n\nPlease model the supported dispatchers with explicit option/value grammar (failing closed when the executable boundary is ambiguous) and add public regressions for direct, nested-wrapper, benign, and unknown-option cases. The current fix is sound, but executable-position ownership is not complete while common dispatchers can hide the child command.

Superseded by the corrected review below because CLI Markdown escaping corrupted this body.

[XuPeng-SH]

One blocking dispatcher gap remains in the new canonical executable resolver.

resolve_transparent_launcher recognizes several wrappers, but not standard command dispatchers such as timeout, nice, ionice, or setsid. Because this PR removes the legacy destructive-token scan, timeout 5 dd if=/dev/zero of=/dev/sda resolves only the outer timeout executable and is classified safe; the remaining checks do not catch dd or /dev/sda. The same gap remains after a supported wrapper, for example sudo timeout 5 dd ....

Please model supported command dispatchers with explicit option/value grammar and fail closed whenever the child executable boundary is ambiguous. Add public validate_execute_bash_command regressions covering direct dispatch, nested wrappers, benign child commands, and unknown option arity. The current exec -a fix is sound, but executable-position ownership is incomplete while common dispatchers can hide the child command.

[gouhongshen]

Fixed in 7642c9a. The canonical AST resolver now peels timeout/gtimeout, nice, ionice, and setsid using per-dispatcher option/value grammars; timeout also consumes its required duration operand before resolving the child. Unknown option arity remains fail-closed. Public validator regressions cover direct dispatch, nested wrappers, benign child commands, and unknown options. Verified with cargo test -p astra-sandbox --lib (161), cargo test -p astra-tools --lib validate_execute_bash (11), and strict clippy. The same fix is synchronized to moi-dev follow-up #701.

[XuPeng-SH]

Re-reviewed latest head 7642c9a3. The new timeout / nice / ionice / setsid grammars correctly address the previous finding, including option arity and nested wrappers. Two execution-semantics gaps remain.

1. P1 — a dynamic find expression can introduce an uninspected executable

resolve_find_commands silently skips every dynamic word outside an already-literal -exec. In Bash, an unquoted expansion is subject to word splitting, so it can create the complete predicate and command boundary at runtime:

find_args="-exec truncate -s 0 important.db {} ;"
find . $find_args

The expansion supplies literal argv entries -exec, truncate, and ; to find; find executes the destructive child. The AST resolver sees one Dynamic word, skips it, returns Safe, and the removed legacy scan no longer provides a second signal.

The current CommandWord::Dynamic also loses whether expansion is quoted or may split into multiple argv entries. Preserve that provenance and fail closed when a dynamic find expression can introduce predicates, while allowing a quoted dynamic path that remains one operand. Add public validator regressions for both the injected expression and the benign dynamic-path control.

2. P1 — launcher ownership is still an open-ended allowlist and remains fail-open

resolve_transparent_launcher returns the outer executable as final for every dispatcher not named in its match. Standard supported-host utilities such as:

stdbuf -o0 dd of=important.db
taskset -c 0 wipefs -a /dev/sdb
chroot /mnt dd of=important.db
unshare --fork truncate -s 0 important.db

therefore resolve only stdbuf, taskset, chroot, or unshare and are classified safe. All were caught by the removed token fallback. Adding four dispatchers per review does not close this class.

Please define the actual trust contract: destructive-name detection cannot be the sole security boundary because arbitrary interpreters can dispatch the same operation. Keep OS/workspace capability enforcement authoritative, and make the AST layer a declarative, testable defense-in-depth registry for the command-dispatch surfaces Astra supports. The registry needs one shared grammar representation and a coverage matrix for the standard launchers present in supported runtime images; unknown option arity must remain ambiguous.

3. P2 — option grammar currently conflates execution with terminal/query modes

The same resolver treats every recognized option as something to peel. For example, command -v dd and command -V dd only query command resolution, while sudo -l dd lists policy; none executes dd, but the current code reports a destructive command. --help and --version modes have similar behavior for several launchers. This reintroduces false positives in common feature-detection scripts.

Have launcher resolution return an execution semantic such as Dispatch(index), NoDispatch, or Ambiguous, rather than only an argv index. Add benign public-entrypoint tests for terminal/query modes alongside destructive dispatch tests.

The focused CI is green, git diff --check passes, and the current-main merge tree is clean. Those checks do not exercise these argv-expansion and dispatcher-semantics cases. The branch is behind current main; rebase after the resolver contract is corrected.

[gouhongshen]

Addressed all three findings in the rebased head 9ede5abe:

• P1 dynamic find expressions: CommandWord now preserves whether a dynamic word may split. An unquoted expansion that can mint -exec argv is Ambiguous/fail-closed, while a quoted scalar path remains one accepted operand.
• P1 launcher contract: OS isolation and workspace capabilities remain the authoritative boundary. The AST layer is explicitly documented as a finite defense-in-depth registry for supported runtime dispatch surfaces. stdbuf, taskset, chroot, and unshare now use the same shared option/value grammar, with unknown arity fail-closed.
• P2 non-dispatch modes: launcher resolution now returns Dispatch, NoDispatch, or Ambiguous; command -v/-V, sudo -l, and help/version/query modes no longer classify an unexecuted operand.

The branch was rebased onto current main. Public validator regressions cover each destructive case, benign quoted paths/children, terminal/query modes, and unknown options. Verification passed: astra-sandbox 161 tests, 12 targeted execute_bash tests, and strict all-target clippy for both affected crates. The PR description is updated. Since #701 had already merged, the equivalent moi-dev synchronization is in #711.

[XuPeng-SH]

结论

本 PR 将 destructive executable 分类迁移到 Bash AST 的方向正确，但当前 head 仍有两个可验证的 argv 边界解析漏洞；在旧 raw-text fallback 已删除后，它们会让实际执行的 destructive command 通过 validate_execute_bash_command，总体风险为 blocking。

发现

1. blocking — 动态 option value 的 word splitting 可注入新的 executable boundary

位置：nested_shell_script 对 shell 长选项值的消费，以及 skip_launcher_options。

问题：CommandWord 已记录 Dynamic { may_split }，但这些 value-consuming 路径只检查下一个 AST word 是否存在，随后按“恰好一个 argv”跳过它；没有检查未引用 expansion 是否会 split。于是一个 source word 可以在运行时生成 option value、后续 option 和 executable，而 resolver 会跨过整个动态 word。

最小触发场景：

opts='/tmp/rc -c reboot'
bash --rcfile $opts printf

Bash 实际 argv 是 bash --rcfile /tmp/rc -c reboot printf，因此执行 reboot；当前 nested_shell_script 将 $opts 整体当作 --rcfile 的一个值跳过，随后看到 printf，返回 None。同类 bypass 也存在于共享 launcher grammar，例如：

spec='HOME dd'
env -u $spec if=/dev/zero of=important.db count=1

运行时 env 执行 dd if=/dev/zero of=important.db count=1，但 resolver 把 $spec 当作 -u 的单个值，再把 if=... / of=... / count=1 当作 environment assignments，最终得到 NoDispatch。旧 destructive-token scan 已被本 PR 删除，因此这些命令不会再被第二层检测捕获。

影响：调用方会得到 Ok(())，destructive executable 分类可被稳定绕过。

建议：集中实现一个 value/operand consumer；只有 literal 或明确 may_split == false 的动态标量才能消费为单个 argv，may_split == true 必须返回 Ambiguous。将它用于 shell -o/-O、--rcfile/ --init-file、共享 launcher grammar 以及 xargs 的 value-bearing options，避免各解析器重复丢失 provenance。

2. blocking — xargs --eof / --replace 的可选值被误当成必需值，静态命令也可绕过

位置：resolve_xargs_command 的 option tables 和消费逻辑。

问题：GNU xargs 的 long forms 是 --eof[=END] 和 --replace[=R]；不带 = 时，后一个 argv 是 command，而不是 option value。当前代码把两者放入 OPTIONS_WITH_VALUE，在没有 = 时无条件消费下一个 word。

最小触发场景：

printf '' | xargs --eof dd if=/dev/zero of=important.db count=1

xargs 即使没有输入也默认执行 child 一次，实际 child 是 dd。resolver 却把 dd 消费为 --eof 的值，然后从 if=/dev/zero 开始解析，结果为 safe。这个问题完全由 literal argv 触发，不依赖动态 expansion。

影响：常见 dispatcher 可直接隐藏 destructive executable。相反，同一表还把 --help / --version 当作普通 flags，xargs --help dd 会被误报为执行 dd，说明 execution semantics 在两个方向都不正确。

建议：为 xargs 单独建模“仅允许 inline optional value”的 long options：--eof=... / --replace=... 消费 inline value，而裸 --eof / --replace 后一个 argv 应作为 child command；--help / --version 应直接返回 NoDispatch。

测试建议

• 在公开入口 validate_execute_bash_command 增加上述 bash --rcfile $opts 与 env -u $spec 回归，断言未引用动态 option value fail closed；同时保留 quoted scalar value 的 benign control。
• 增加 xargs --eof dd ...、xargs --replace dd ... 的阻断测试，以及 xargs --eof=STOP printf ...、xargs --help dd、xargs --version dd 的语义对照测试。

[gouhongshen]

Both blocking findings are valid and fixed in 44f89b4b.

• Added one shared single-argv consumer. Nested shell values, shared launcher option values, fixed leading operands, and xargs required values now advance only across literals or quoted scalar dynamics; an unquoted expansion that may field-split is Ambiguous/fail-closed. This covers the reported bash --rcfile $opts and env -u $spec bypasses, plus xargs values and launcher operands, without restoring raw-text scanning.
• Corrected GNU xargs --eof[=END] / --replace[=R]: bare long forms no longer consume the child executable, inline forms retain their value, and help/version/show-limits terminate without dispatch.

Public validator regressions include both reported exploits, dynamic xargs values and duration operands, destructive bare --eof/--replace, quoted scalar controls, inline optional values, and terminal modes. Verification passed: astra-sandbox 161 tests, 12 targeted execute_bash tests, and strict all-target clippy. The PR description is updated. Since #711 had already merged, the equivalent moi-dev synchronization is #713.

[XuPeng-SH]

Deep review of eea8ce0: REQUEST_CHANGES.

The previous split-capable option-value and xargs --eof/--replace fixes are present. Moving executable classification into the AST and explicitly documenting OS/workspace isolation as the authoritative boundary are sound architectural choices. Three verified utility-semantics mismatches remain in the supported dispatcher registry, detailed inline: xargs --show-limits still dispatches; bare --max-lines has an optional inline value; and a quoted scalar can supply find's -exec predicate. These are destructive-command classification regressions after removal of the raw-token fallback, not claims of an OS sandbox escape.

Validation performed:

• Reviewed all three changed files, surrounding risk propagation and the public execute_bash validator, prior reviews, and current CI.
• Executed GNU findutils 4.9.0 reproductions exclusively in disposable directories. Each reported case executed dd/truncate and changed a fixture from 21 bytes to 0; harmless printf controls separately confirmed child dispatch.
• git diff --check passed. GitHub PR Title, Static Checks, and Test Suite report success.
• Attempted cargo test --offline -p astra-sandbox --lib, but Cargo is not installed. No Rust unit tests or public-validator tests were executed locally; the Rust classification and downstream validator conclusions are based on source tracing.

For regression quality, pair public-validator assertions with harmless argv/dispatch probes against the supported utility implementations. The current string tables and tests agree with each other while disagreeing with actual execution semantics. Keep the existing heredoc/interpreter-data and query-mode controls so correcting these failures does not restore the original UX false positives.