fix(sandbox): classify destructive commands from bash AST - #696
fix(sandbox): classify destructive commands from bash AST#696gouhongshen wants to merge 3 commits into
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
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 99 to 119 in 0006ca0
因此常见命令 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 7db624c4。--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 199 to 231 in 7db624c
因此 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 — diagnosed from a production MOI run where Python source in a Bash heredoc was rejected as
destructive command (dd).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 change 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.astra-sandbox::analyze_command_risks,astra-tools::validate_execute_bash_command_in_workspace, wrapper handling, nested shell launchers, and the existing command-risk test suites.astra-tools.rm -rfguard remain inastra-tools; executable classification has one owner inastra-sandbox.Production wiring and verification
execute_bashvalidator used by Astra Server 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— 152 passedcargo test -p astra-tools --lib validate_execute_bash— 10 passedcargo clippy -p astra-sandbox -p astra-tools --all-targets -- -D warnings