fix(sandbox): classify destructive commands from bash AST - #697
fix(sandbox): classify destructive commands from bash AST#697gouhongshen wants to merge 3 commits into
Conversation
XuPeng-SH
left a comment
There was a problem hiding this comment.
方向正确:用 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
因此常见命令 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 数据不误报。
|
已在
新增了 |
XuPeng-SH
left a comment
There was a problem hiding this comment.
重新审查了最新 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
因此 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 的目标。
|
已按该 P1 的执行语义处理,修复在
验证通过: |
What type of PR is this?
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
ddcould 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:sudo,env, andexec;bash/sh/zsh -cprograms so real nested destructive commands remain blocked;busybox/toybox,xargs, andfind -exec/-execdirinstead of treating only the outer executable as authoritative;rm -rfguard;astra-tools, so heredoc and inline Python/Node data no longer trigger destructive-command false positives.Architecture and complexity delta
astra-sandboxBash AST risk analysis now owns destructive executable classification and reusesmain's existing literal command parser.parse_plain_bash_commands,analyze_command_risks,validate_execute_bash_command_in_workspace, wrapper handling, nested shell launchers, and existing command-risk tests.astra-tools.rm -rfguard remain inastra-tools; executable classification has one owner inastra-sandbox.Production wiring and verification
execute_bashvalidator used by Astra tool execution.--norc, value-consuming--rcfile, short-option clusters, multi-call applets,xargs,find -exec/-execdir, runtime-dependent executable positions, ambiguous shell options, malformed input, heredoc Python data, inline interpreter source, and benign command arguments.N/Awith reason: N/A; command validation is in-memory and this PR does not touch persistence.Verification:
cargo fmt --all -- --checkcargo test -p astra-sandbox --lib— 160 passedcargo test -p astra-tools --lib validate_execute_bash— 10 passedcargo clippy -p astra-sandbox -p astra-tools --all-targets -- -D warningsastra-toolslib run was attempted; unrelated process-detach and workspace-lease concurrency tests interfered and timed out under the parallel local run. The affected Bash validation suite above passes independently.