fix(core): treat only space/tab/newline as word separators in isAsyncOperator - #11865
yiliang114 wants to merge 1 commit into
Conversation
…Operator JavaScript's \s also matches \r, \v, \f and , none of which bash treats as word separators (its default IFS is space, tab and newline). When one of those characters sat between a redirection operator and a bare &, the backward scan in isAsyncOperator skipped past it, concluded the & belonged to a redirection, and kept both halves of something like 'echo x >\r& rm -rf ~/important' in one segment — letting the first command's allow rule cover the second (fail-open). Skip only [' ', '\t', '\n'] (BASH_WORD_SEPARATORS) instead, and pin all four characters, the spaced variant, and the 2>&1 exclusion with regression tests. Fixes #11851 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
github-actions
Bot
added
the
review/self-reported
|
Thanks for the PR! Template looks good ✓ — every required heading is filled in, including a real Tested-on table and a complete Chinese translation. Problem: observed, not theoretical. #11851 carries bash Direction: aligned, and the reference signal is unusually strong. Claude Code's CHANGELOG shows this class of shell-splitter permission bypass being fixed repeatedly, in the same fail-closed direction — "Fixed a Bash permission bypass where a crafted command could hide parts of itself from permission checks", "fixed trailing Size: core path touched ( Approach: the scope is right and I would not cut anything. Before reading the diff I sketched the same fix — narrow the backward scan's skip set to exactly space/tab/newline, pin the four characters with a regression test, leave sibling Risk: no Stage 1e high-risk path match. Two things still deserve reviewer attention, neither blocking:
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需标题都已填写，包括真实的 Tested-on 表格和完整的中文翻译。 问题： 是已观测到的 bug，不是理论性加固。#11851 附有 bash 方向： 对齐，而且参考信号异常明确。Claude Code 的 CHANGELOG 显示这类 shell 切分器权限绕过被反复修复，且方向一致（fail-closed）——"Fixed a Bash permission bypass where a crafted command could hide parts of itself from permission checks"、"fixed trailing 规模： 触及核心路径（ 方案： 范围合理，我不会砍任何部分。在读 diff 之前我先勾勒了自己的修法——把回扫的跳过集合收窄到恰好空格/tab/换行、用回归测试钉住那四个字符、不动同形态的其他 风险： 未命中 Stage 1e 高风险路径。有两点仍值得评审者注意，均非阻塞：
进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI sketched a fix from the title and the "Why it's needed" section before opening the diff: narrow the backward scan's skip set to exactly space/tab/newline, pin the four characters with a regression test, leave sibling The change is strictly monotonic toward splitting, and that is what makes it safe to reason about. More splits can only tighten the verdict. I traced both consumers rather than assuming:
No legitimate bash construct regresses. The scan returns
I also grepped every The new tests are load-bearing — I verified this statically rather than taking the description's word for it. Tracing the pre-fix code on The expected first segment Reuse ladder exhausted correctly. There is no existing shared constant to extend: Conventions: No blocking findings. One non-blocking note: once #11821 lands, this file will hold both I skipped the sequence diagram and the changed-files table — two files and a single predicate swap do not need either. Test evidenceThis was an unattended CI run ( Nothing is red at the time of writing. The two checks that matter most for this diff — Worth noting for the Tested-on table: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the one claim the suite does not cover: Real-scenario tmux testing: N/A — this is an unattended CI run, and the change has no TUI surface. 中文说明代码审查在看 diff 之前，我先根据标题和"为什么需要"部分勾勒了自己的修法：把回扫的跳过集合收窄到恰好空格/tab/换行，用回归测试钉住那四个字符，不动同形态的其他 这个改动在"切分"方向上是严格单调的，这正是它能被安全推理的原因。 切分更多只会让判定更严。 我没有靠假设，而是逐个追踪了消费者：
没有任何合法 bash 构造会因此退化。 只有在遇到未转义的
我还在所有 新增测试是"承重"的——这一点我做了静态验证，而不是采信描述里的说法。 对 第一段期望值 复用阶梯已正确走到尽头。 没有可扩展的既有共享常量： 约定： 无阻塞性问题。 一点非阻塞提示：#11821 合入后，本文件会同时存在 我省略了时序图和变更文件表——两个文件、一次谓词替换，两者都不必要。 测试证据本次为无人值守 CI 运行（ 撰写时无红灯。对本 diff 最关键的两项——承载五个新用例的 关于 Tested-on 表格值得一提：此配置下 沙箱验证可以解决测试套件未覆盖的那一项主张： 真实场景 tmux 测试：N/A —— 本次为无人值守 CI 运行，且改动没有 TUI 界面。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — the reasoning is airtight and I found no blocking issue; the missing point is CI still running plus three named non-blocking follow-ups, not doubt about the code. Stepping back: this is a permission-engine fix where the safety argument does not depend on enumerating every input. Narrowing the scan's skip set can only make it stop earlier, and stopping earlier always yields "this I also satisfied myself the problem is real rather than accepting the framing. I traced the pre-fix code on The tests are load-bearing, which I checked statically rather than on faith: each of the five cases fails against the unfixed predicate. That matters more than usual here, because a regression test that passes both ways would have made this PR green and worthless. On the "am I being worn down by volume" question — the same author has #11821 open with the sibling What keeps it at 4 rather than 5, none of it blocking:
One process note for the humans, not a criticism: the issue and the fix come from the same author ( CI has not settled yet ( 中文说明信心：4/5 —— 推理严密，未发现阻塞性问题；扣掉的一分来自 CI 仍在运行以及三项已点名的非阻塞后续项，而不是对代码本身的怀疑。 退一步看整体：这是一个权限引擎修复，其安全性论证不依赖于穷举所有输入。收窄扫描的跳过集合只可能让它更早停下，而更早停下总是得出"这个 我也确认了问题真实存在，而不是接受 PR 的叙述框架。我手工追踪了修复前的代码处理 这些测试是"承重"的，这一点我做了静态核查而非采信：五个用例在未修复的谓词下都会失败。在这里这一点比平常更重要，因为一个两边都通过的回归测试会让本 PR 既绿又无效。 关于"我是否被数量磨平了"这个问题——同一作者还有 #11821 在处理同形态的 之所以是 4 而不是 5，均非阻塞：
一点给人类评审的流程提示，并非批评：issue 与修复出自同一作者（ CI 尚未收敛（ — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent 5": none — I did not verify by execution that the reverted /\s/ line turns the five new cases red, because that requires editing the reviewed source in this share…; "agent reverse-audit (round 1)": none — every check I named above ran to completion.; "agent 6c": whether checkArgumentSafety (packages/cli/src/services/prompt-processors/shellProcessor.ts:104-110) rejects a raw CR/VT/FF, i.e. whether a model-supplied $AR…; "agent 6c": what the other splitCommands consumers conclude for the unsplit payload — tools/shell.ts:412, 680, 820, 862, 916 and shellReadOnlyChecker.ts:349 — specifi…; "agent 6c": running the PR's five new vitest cases in packages/core ; I verified the equivalent behaviour by executing the built dist ( splitCompoundCommand on the raw ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent 5"：none — I did not verify by execution that the reverted /\s/ line turns the five new cases red, because that requires editing the reviewed source in this share…；"agent reverse-audit (round 1)"：none — every check I named above ran to completion.；"agent 6c"：whether checkArgumentSafety (packages/cli/src/services/prompt-processors/shellProcessor.ts:104-110) rejects a raw CR/VT/FF, i.e. whether a model-supplied $AR…；"agent 6c"：what the other splitCommands consumers conclude for the unsplit payload — tools/shell.ts:412, 680, 820, 862, 916 and shellReadOnlyChecker.ts:349 — specifi…；"agent 6c"：running the PR's five new vitest cases in packages/core ; I verified the equivalent behaviour by executing the built dist ( splitCompoundCommand on the raw …。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| for (let j = index - 1; j >= 0; j--) { | ||
| const ch = command[j]!; | ||
| if (/\s/.test(ch)) { | ||
| if (BASH_WORD_SEPARATORS.includes(ch)) { |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [regression] Narrowing the skip set makes the & a command boundary immediately after a redirection target made only of characters String.prototype.trim strips — and the segment .trim() at rule-parser.ts:958/:969 then deletes that target, so the virtual write op disappears and a verdict that was deny at the merge base becomes allow.
The change itself is right and its direction is fail-closed at the splitter level. This is a consequence one layer up, so the description's "narrowing the skip set is monotonically fail-closed" does not hold all the way down to the decision.
Failure scenario. With permissions.allow: ['Bash(echo *)'] and permissions.deny: ['Edit(//project/**)', 'Write(//project/**)'], cwd /project, the command echo x >\u00a0& echo y (NBSP between the > and the & — bash really does create a file named NBSP) returned deny before this change, because the unsplit command reached extractShellOperations and produced write_file "/project/\u00a0", which matched the deny rule. After it, the segments are echo x > and echo y, the NBSP target sits at the segment edge, .trim() deletes it, extractShellOperationsAcrossCommand returns [], and evaluate returns allow. An unpermitted file whose name is invisible gets created inside a write-denied directory with no prompt.
Two precisions the measurement adds. The regressed characters are NBSP, VT and FF — CR was already missed at the merge base, so CR is not part of this regression. And an allow rule is not required: a deny-only configuration also loses, from a hard deny down to an ask prompt. A target carrying one visible character (echo x >\u00a0out& echo y) keeps its op and still returns deny on both arms, so the exposure is exactly the all-invisible target.
Witness — A/B against the built merge base 87437db784 and this commit 47d93007ab, same input on both arms:
BASE cmd="echo x >\u00a0& echo y" split=["echo x >\u00a0& echo y"] ops=write_file "/project/\u00a0" evaluate=deny
PR cmd="echo x >\u00a0& echo y" split=["echo x >", "echo y"] ops=(none) evaluate=allow
BASE write-deny/VT ops=write_file "/project/\u000b" evaluate=deny PR ops=(none) evaluate=allow
BASE write-deny/FF ops=write_file "/project/\f" evaluate=deny PR ops=(none) evaluate=allow
BASE write-deny/CR ops=(none) evaluate=allow PR ops=(none) evaluate=allow <- no change, CR was already missed
BASE+PR CTRL visible target "echo x >\u00a0out& echo y" ops=write_file "/project/\u00a0out" evaluate=deny (both arms)
BASE deny-only-no-allow/NBSP evaluate=deny -> PR deny-only-no-allow/NBSP evaluate=ask
Bash ground truth that the base arm's deny was the correct certification, not an accident:
$ printf 'echo x >\xc2\xa0& echo y\n' > s1.sh && bash s1.sh ; od -c <created file>
y 0000000 302 240 <- bash really creates a file named NBSP
Suggested fix. Keep the segment faithful to what bash runs, so the redirect target survives to op extraction. Either trim with a bash-faithful set at :958 and :969 — a small trimBashWordSeparators() that strips only BASH_WORD_SEPARATORS (plus the trailing \r of a CRLF pair, if that normalization is wanted) — or leave .trim() on the rule-matching projection but give CompoundCommandSegment the untrimmed slice and have walkCompoundCommand (shell-semantics.ts:2266) extract ops from that.
The bash-faithful trim is worth more than special-casing the all-invisible one: the same .trim() also mis-names a target that merely ends in an invisible character — echo x >out\u00a0& echo y yields filePath: "/project/out" while bash writes out\u00a0. That half is identical on both arms, so it predates this PR; fixing the trim closes both.
The fix must not violate permission-manager.test.ts:595, which pins expect(splitCompoundCommand(' git status && rm -rf / ')).toEqual(['git status', 'rm -rf /']) — ordinary space trimming has to survive. The five cases this PR adds at :666-668 assert ['echo x >', 'rm -rf /tmp/x'] and need a deliberate update if the trim changes. And const backgrounded = terminator === '&'; at shell-semantics.ts:2278 is a second consumer of the segment objects, so a shape change to CompoundCommandSegment reaches it too.
Please pin this with a test that fails without the fix: a case with deny Write(//project/**) and the payload echo x >\u00a0& echo y asserting evaluate({ toolName: 'run_shell_command', command: 'echo x >\u00a0& echo y' }) is not 'allow', plus a unit assertion that extractShellOperationsAcrossCommand('echo x >\u00a0& echo y', '/project') still yields { virtualTool: 'write_file', filePath: '/project/\u00a0' } — then remove the bash-faithful trim, run those two, and confirm both go red.
中文说明
收窄跳过集合后，当重定向目标完全由 String.prototype.trim 会剥离的字符组成时，& 会紧接在该目标之后成为命令边界——而 rule-parser.ts:958/:969 的 segment .trim() 随后把这个目标删掉，于是虚拟写操作消失，合并基线上原本为 deny 的判定变成了 allow。
改动本身是正确的，在切分器层面方向也是 fail-closed 的。问题出在上一层的后果，因此 PR 描述里"收窄跳过集合是单调 fail-closed"的说法，在判定层面并不成立。
失败场景。 配置 permissions.allow: ['Bash(echo *)']、permissions.deny: ['Edit(//project/**)', 'Write(//project/**)']，cwd 为 /project，命令 echo x >\u00a0& echo y（> 与 & 之间是 NBSP——bash 确实会创建一个名为 NBSP 的文件）。改动前返回 deny：未切分的整条命令进入 extractShellOperations，产生 write_file "/project/\u00a0"，命中 deny 规则。改动后 segment 变成 echo x > 和 echo y，NBSP 目标正好落在 segment 边缘被 .trim() 删掉，extractShellOperationsAcrossCommand 返回 []，evaluate 返回 allow。一个名字不可见、且未被授权的文件，就这样在禁止写入的目录里被创建，且不会弹出任何确认。
实测补充两点精确结论：发生回归的字符是 NBSP、VT、FF——CR 在合并基线上本来就已经漏掉，不属于本次回归；并且不需要 allow 规则，只配 deny 的场景同样会失守，从硬 deny 降级为 ask 询问。目标里只要有一个可见字符（echo x >\u00a0out& echo y），两侧都保留 op 并仍然返回 deny，所以暴露面恰好是"目标全为不可见字符"这一种。
证据——针对已构建的合并基线 87437db784 与本提交 47d93007ab 做 A/B，两侧输入完全相同（见上方英文部分的输出）。bash 侧的事实依据也确认基线的 deny 是正确判定：printf 'echo x >\xc2\xa0& echo y\n' > s1.sh && bash s1.sh 确实会创建一个名为 NBSP 的文件（od -c 显示 302 240）。
修复建议。 让 segment 忠实反映 bash 实际执行的内容，使重定向目标能存活到 op 提取阶段。两种做法：在 :958 和 :969 用符合 bash 语义的字符集来 trim——写一个只剥离 BASH_WORD_SEPARATORS 的 trimBashWordSeparators()（如需保留 CRLF 归一化，再加上成对 \r\n 的尾部 \r）；或者保留规则匹配投影上的 .trim()，但让 CompoundCommandSegment 携带未 trim 的原始切片，由 walkCompoundCommand（shell-semantics.ts:2266）从中提取 op。
采用符合 bash 语义的 trim 比只为"全不可见目标"打特例更有价值：同一个 .trim() 还会让以不可见字符结尾的目标报错文件名——echo x >out\u00a0& echo y 得到 filePath: "/project/out"，而 bash 实际写的是 out\u00a0。这一半在两侧完全相同，属于本 PR 之前就存在的问题；修好 trim 可以一并解决。
修复不得违反 permission-manager.test.ts:595：它钉住了 expect(splitCompoundCommand(' git status && rm -rf / ')).toEqual(['git status', 'rm -rf /'])，普通空格的 trim 必须保留。本 PR 在 :666-668 新增的五个用例断言 ['echo x >', 'rm -rf /tmp/x']，若 trim 行为改变需要有意识地同步更新。另外 shell-semantics.ts:2278 的 const backgrounded = terminator === '&'; 是 segment 对象的第二个消费者，因此改动 CompoundCommandSegment 的形状会波及它。
请补一个"去掉修复就会失败"的测试：在 deny Write(//project/**) 配置下，用载荷 echo x >\u00a0& echo y 断言 evaluate({ toolName: 'run_shell_command', command: 'echo x >\u00a0& echo y' }) 不是 'allow'；再加一条单元断言，确认 extractShellOperationsAcrossCommand('echo x >\u00a0& echo y', '/project') 仍然产出 { virtualTool: 'write_file', filePath: '/project/\u00a0' }。然后把符合 bash 语义的 trim 去掉、跑这两条，确认都变红。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| // Spaced variant: real separators around the non-IFS character. | ||
| ['echo x > \r & rm -rf /tmp/x'], | ||
| ])( | ||
| 'splits %s, where a non-IFS "whitespace" sits between the > and the &', |
There was a problem hiding this comment.
[Suggestion] R1-4: The it.each title interpolates the raw command via %s, so all five new test names embed a literal CR / VT / FF / NBSP — and the \v and \f rows collapse onto a single JUnit name=.
Failure scenario. Vitest strips U+000B and U+000C because they are illegal in XML 1.0, while U+000D survives. In the emitted junit.xml the \v case and the \f case therefore come out byte-identical, so a red \v case is indistinguishable from a red \f case in the CI test report — .github/workflows/ci.yml:807-809 feeds packages/*/junit.xml to a java-junit reporter that keys on name. In console output the CR rows rewind the cursor to column 0, so the remainder of the title overwrites the pass/fail glyph and the file path in the CI log. The cost is diagnosis time on a security regression rather than a wrong verdict — but this is precisely the block whose five rows exist to be told apart from one another.
Witness — real junit-reporter run against the reviewed tree (433 testcases emitted; same reporter and serialization as packages/core/vitest.config.ts:27,41, output redirected outside the worktree):
non-IFS cases: 5 distinct name= attributes: 4 DUPLICATED: 1
x2 b'splitCompoundCommand > splits echo x >& rm -rf /tmp/x, where a non-IFS "whitespace" sits between the > and the &'
<- the \v row and the \f row, one identifier
b'...splits echo x >\r& rm -rf /tmp/x...' <- raw 0x0D survives inside name="..."
b'...splits echo x >\xc2\xa0& rm -rf /tmp/x...' <- raw NBSP survives
b'...splits echo x > \r & rm -rf /tmp/x...' <- spaced CR row
file-level: 0x0b present: False 0x0c present: False 0x0d present: True
Suggested fix. Give each row a printable label column and interpolate that instead of the raw command, keeping the control characters only in the input. The it.each table at :654-660 and the title at :662 change together:
it.each([
['\\r', 'echo x >\r& rm -rf /tmp/x'],
['\\v', 'echo x >\v& rm -rf /tmp/x'],
['\\f', 'echo x >\f& rm -rf /tmp/x'],
['\\u00a0', 'echo x >\u00a0& rm -rf /tmp/x'],
// Spaced variant: real separators around the non-IFS character.
['\\r (spaced)', 'echo x > \r & rm -rf /tmp/x'],
])(
'splits a command with %s between the > and the &',
async (_label, command) => {
// ...body unchanged
},
);(This is a two-line-range change, so it is written out rather than offered as a one-click suggestion block anchored on the title line alone.) The repo already has this shape: packages/core/src/agents/runtime/workflow-meta-literal.test.ts:88-94 uses a two-column it.each with a ['CR', '\r']-style label precisely so titles stay printable, and the neighbouring it.each blocks in this same describe keep the interpolated column printable. Interpolating JSON.stringify(command) (or %j) also works if you would rather not add a column.
The fix must keep names XML-legal and unique per case: packages/core/vitest.config.ts:27 sets reporters: ['default', 'junit'] with junit: 'junit.xml' at :41, so every core test run writes a JUnit report that .github/workflows/ci.yml:807-809 collects via path: 'packages/*/junit.xml' into a java-junit reporter.
中文说明
it.each 的标题用 %s 直接插值了原始命令，因此五个新测试名里都嵌入了字面的 CR / VT / FF / NBSP——并且 \v 与 \f 两行在 JUnit 里塌缩成同一个 name=。
失败场景。 vitest 会剥离 U+000B 和 U+000C（它们在 XML 1.0 中非法），而 U+000D 会保留。于是生成的 junit.xml 里 \v 用例与 \f 用例的名字逐字节相同：在 CI 测试报告中，\v 变红与 \f 变红无法区分——.github/workflows/ci.yml:807-809 正是把 packages/*/junit.xml 交给按 name 索引的 java-junit reporter。在控制台输出中，CR 行会把光标退回第 0 列，标题的剩余部分覆盖掉通过/失败标记与文件路径，CI 日志因此不可读。代价是排查一次安全回归所需的时间，而不是错误结论——但这恰恰是那个"五行存在的意义就是彼此可区分"的测试块。
证据——针对被评审代码树的真实 junit reporter 运行（输出 433 个 testcase；reporter 与序列化方式与 packages/core/vitest.config.ts:27,41 一致，输出重定向到工作树之外），详见上方英文部分的原始输出：五个 non-IFS 用例只产生 4 个不同的 name=，其中 1 组重复（\v 行与 \f 行）；文件级检查为 0x0b present: False 0x0c present: False 0x0d present: True。
修复建议。 给每行加一个可打印的标签列，标题插值该标签而不是原始命令，控制字符只保留在输入里。:654-660 的 it.each 表格与 :662 的标题需要一起改（具体代码见上方英文部分）。因为这是跨两处的改动，所以直接写出代码，而没有提供只锚定标题行的 suggestion 一键应用块。仓库里已有同样写法：packages/core/src/agents/runtime/workflow-meta-literal.test.ts:88-94 用两列 it.each 配 ['CR', '\r'] 这样的标签，正是为了让标题保持可打印；同一个 describe 里相邻的 it.each 块也都让被插值的那一列保持可打印。如果不想加列，插值 JSON.stringify(command)（或用 %j）也可以。
修复必须保证测试名在 XML 中合法且每个用例唯一：packages/core/vitest.config.ts:27 设置了 reporters: ['default', 'junit']，:41 设置了 junit: 'junit.xml'，因此每次 core 测试都会写出 JUnit 报告，由 .github/workflows/ci.yml:807-809 通过 path: 'packages/*/junit.xml' 收集给 java-junit reporter。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| async (command) => { | ||
| // Segments are trimmed, and `String.prototype.trim` also strips these | ||
| // characters, so the first segment ends at the bare `>`. | ||
| expect(splitCompoundCommand(command)).toEqual([ |
There was a problem hiding this comment.
[Suggestion] R1-5: All five new cases pin only splitCompoundCommand's returned array. Nothing asserts the PermissionManager.evaluate verdict the fix actually changes, even though this file already has an evaluate()-level compound suite to hang it on (it('semicolon compound: deny in second → deny', …) at :2328).
Failure scenario. The guarantee is decided one level up, at permission-manager.ts:336-341: splitCompoundCommand → subCommands.length > 1 → evaluateCompoundCommand. When the splitter returns a single segment, evaluateSingle matches the rule against the whole command string, and matchesCommandPattern compiles its regex with the s flag (rule-parser.ts:1088), so Bash(echo *) matches echo x >\r& rm -rf /tmp/x end to end and the verdict is allow — exactly the reported bypass. That is the pre-fix behaviour, and after the fix no test states the new verdict. So a change that re-joins segments, matches the unsplit command before splitting, or re-widens the separator set at one of the three other splitCompoundCommand call sites (permission-manager.ts:956, :1112, :1210) leaves all five new splitter tests green while a user's Bash(echo *) allow rule once again authorises the backgrounded rm -rf.
Witness — executed A/B; the base arm is the reverted state (if (/\s/.test(ch)) {), so this is the mutation run rather than a read of it (allow ['Bash(echo *)'], deny ['Bash(rm *)'], expected ['echo x >', 'rm -rf /tmp/x']):
BASE FAIL cmd="echo x >\r& rm -rf /tmp/x" split=[1 segment] evaluate=allow
BASE FAIL cmd="echo x >\v& rm -rf /tmp/x" split=[1 segment] evaluate=allow
BASE FAIL cmd="echo x >\f& rm -rf /tmp/x" split=[1 segment] evaluate=allow
BASE FAIL cmd="echo x >\u00a0& rm -rf /tmp/x" split=[1 segment] evaluate=allow
BASE FAIL cmd="echo x > \r & rm -rf /tmp/x" split=[1 segment] evaluate=allow
PR PASS x5 (all split=["echo x >","rm -rf /tmp/x"]) and evaluate=deny x5
BASE+PR quote-guard: evaluate("echo 'a && b'") with allow Bash(echo *) = allow
Suggested fix. Add one case beside :2328 with permissionsAllow: ['Bash(echo *)'], permissionsDeny: ['Bash(rm *)'] and command: 'echo x >\r& rm -rf /tmp/x', expecting 'deny'. Using the deny-rule shape rather than asserting a bare 'ask' keeps the test off resolveDefaultPermission's AST verdict for the truncated echo x > first segment.
Note this does not cover R1-1: that regression is a missing virtual write op under a Write(...) deny with an invisible-only target, and echo x >\u00a0& echo y contains no rm — measured evaluate=allow with the Bash(rm *) deny irrelevant. Catching it needs its own case.
The new case must not be satisfied by splitting more aggressively: permission-manager.test.ts:2359 (it('operators inside quotes: treated as single command', …)) requires evaluate({ command: "echo 'a && b'" }) with allow Bash(echo *) to stay 'allow'. Verified on both arms above — that guard is unaffected.
Please confirm the acceptance criterion by mutation: with the new evaluate() case in place, revert if (BASH_WORD_SEPARATORS.includes(ch)) { at rule-parser.ts:868 to if (/\s/.test(ch)) {, run it, and check it goes red — the five splitter-level cases already do, but the new one is what pins the decision rather than the intermediate array.
中文说明
五个新用例只钉住了 splitCompoundCommand 返回的数组，没有任何测试断言这个修复真正改变的 PermissionManager.evaluate 判定结果——尽管本文件里已经有可以挂上去的 evaluate() 级复合命令测试套件（:2328 的 it('semicolon compound: deny in second → deny', …)）。
失败场景。 该保证是在上一层决定的，见 permission-manager.ts:336-341：splitCompoundCommand → subCommands.length > 1 → evaluateCompoundCommand。当切分器只返回一个 segment 时，evaluateSingle 会拿规则去匹配整条命令字符串，而 matchesCommandPattern 编译正则时带 s 标志（rule-parser.ts:1088），因此 Bash(echo *) 会端到端匹配上 echo x >\r& rm -rf /tmp/x，判定为 allow——正是被报告的绕过。这是修复前的行为，而修复之后没有测试陈述新的判定结果。所以只要有人重新合并 segment、在切分前先匹配未切分的命令、或在另外三个 splitCompoundCommand 调用点（permission-manager.ts:956、:1112、:1210）之一重新放宽分隔符集合，五个新的切分器测试仍然全绿，而用户的 Bash(echo *) allow 规则又会授权那条被后台化的 rm -rf。
证据——已实际执行 A/B；基线一侧就是回退后的状态（if (/\s/.test(ch)) {），所以这是真正的变异运行，而不是对它的静态阅读：基线五个载荷全部 FAIL 且 evaluate=allow，PR 侧五个全部 PASS 且 evaluate=deny（原始输出见上方英文部分）。引号保护用例在两侧均为 allow，说明修复约束成立。
修复建议。 在 :2328 旁边加一个用例，配置 permissionsAllow: ['Bash(echo *)']、permissionsDeny: ['Bash(rm *)']、command: 'echo x >\r& rm -rf /tmp/x'，期望 'deny'。采用 deny 规则的写法而不是断言裸 'ask'，可以让测试避开 resolveDefaultPermission 对被截断的首 segment echo x > 的 AST 判定。
注意这不能覆盖 R1-1：那个回归是在 Write(...) deny 下丢失虚拟写操作，且目标全为不可见字符，而 echo x >\u00a0& echo y 里根本没有 rm——实测在 Bash(rm *) deny 下 evaluate=allow，该 deny 规则完全不相干。要覆盖它需要单独的用例。
新用例不得靠"更激进地切分"来通过：permission-manager.test.ts:2359（it('operators inside quotes: treated as single command', …)）要求在 allow Bash(echo *) 下 evaluate({ command: "echo 'a && b'" }) 保持 'allow'。上方 A/B 已在两侧验证该保护不受影响。
请用变异来确认验收标准：加好新的 evaluate() 用例后，把 rule-parser.ts:868 的 if (BASH_WORD_SEPARATORS.includes(ch)) { 改回 if (/\s/.test(ch)) {，运行该用例，确认它变红——五个切分器层面的用例本来就会红，但新用例钉住的是判定结果，而不是中间数组。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';', '&', '\n']; | ||
|
|
||
| /** | ||
| * The characters bash treats as word separators (its default `IFS`): space, |
There was a problem hiding this comment.
[Suggestion] R1-6: This comment justifies the list by bash's default IFS, but the property isAsyncOperator actually depends on is the lexer's notion of whitespace — bash's whitespace(c) (space and tab) plus newline — which is what decides whether > and & are adjacent tokens. IFS governs field splitting of expansions after tokenization and has no effect on operator recognition, so on a security-critical list the stated rationale is the wrong rule.
Failure scenario. A maintainer who later needs to honour a shell started with a custom IFS (say IFS=$' \t\n:') reads "the characters bash treats as word separators (its default IFS)", concludes the list should track IFS, and appends ':'. Every added character widens the skip at rule-parser.ts:868, so echo x >:& rm -rf / stops splitting and the first command's allow rule again covers the second — silently reopening the exact fail-open this PR closes, with all five new regression tests still green because none of them exercises an added character.
Witness — real bash 5.x, confirming the tokenizer does not consult IFS while expansion field-splitting does:
$ printf "IFS=\$'\\r'\necho x >\\r& echo y\n" > s.sh # literal CR byte, confirmed by od -c
$ bash s.sh ; ls -b
y <- still two commands
\r s.sh <- CR still an ordinary word character: the tokenizer ignores IFS
$ bash -c "IFS=':'; set -- a:b; echo words=\$#" -> words=1 (literal words NOT split by IFS)
$ bash -c "IFS=':'; x=a:b; echo \$x" -> a b (expansion IS field-split by IFS)
grep -n IFS over the two changed files returns this comment (rule-parser.ts:819) plus the test at permission-manager.test.ts:659,662, so the framing appears only here.
Suggested fix. State the operative rule instead of IFS. The paragraph at :818-824 becomes something like:
/**
* The characters bash's lexer treats as whitespace (space, tab and newline —
* `whitespace(c)` in bash's `parse.y`). These coincide with bash's default
* `IFS`, but `IFS` is not what matters here: operator adjacency is decided by
* the lexer, so this list must not be widened to follow a custom `IFS` —
* doing so re-merges `>\r&`-style payloads into one segment.
*/Optionally rename the constant to BASH_LEXER_WHITESPACE, which makes the wrong mental model harder to reach for at the read site as well.
Worth noting in mitigation, since it bounds how much this costs: the accurate rationale ("which bash treats as ordinary word characters") already sits eight lines below at rule-parser.ts:857-861, and the test comment at permission-manager.test.ts:648-653 is accurate too. So this is the low end of Suggestion and asks for a wording change only — no behaviour.
中文说明
这段注释用 bash 的默认 IFS 来论证这个字符列表，但 isAsyncOperator 真正依赖的性质是词法分析器的空白定义——bash 的 whitespace(c)（空格与 tab）加上换行——决定 > 与 & 是否为相邻 token 的正是它。IFS 管的是分词之后展开结果的字段切分，对操作符识别没有任何影响。因此在一个安全关键的列表上，这里给出的理由是错的规则。
失败场景。 后来若有维护者需要兼容以自定义 IFS 启动的 shell（例如 IFS=$' \t\n:'），他读到"bash 视为词分隔符的字符（即其默认 IFS）"，就会认为这个列表应当跟随 IFS，于是加上 ':'。每多加一个字符都会放宽 rule-parser.ts:868 的跳过范围，于是 echo x >:& rm -rf / 不再被切分，第一条命令的 allow 规则又一次覆盖了第二条——本 PR 关闭的 fail-open 被悄悄重新打开，而五个新回归测试依然全绿，因为它们没有一个涉及新加的字符。
证据——真实 bash 5.x，确认分词器不查询 IFS，而展开的字段切分会查询（原始输出见上方英文部分）：设置 IFS=$'\r' 后 echo x >\r& echo y 仍然执行两条命令、仍然创建名为 CR 的文件；IFS=':' 时 set -- a:b; echo $# 得到 1（字面词不被 IFS 切分），而 x=a:b; echo $x 得到 a b（展开确实被 IFS 字段切分）。对两个改动文件执行 grep -n IFS 只命中本注释（rule-parser.ts:819）与测试的 permission-manager.test.ts:659,662，说明这一表述只出现在这里。
修复建议。 直接陈述真正起作用的规则，而不是 IFS。:818-824 这段可改为上方英文部分给出的写法（大意为：这些是 bash 词法分析器视为空白的字符，即 parse.y 中的 whitespace(c)；它们与 bash 默认 IFS 重合，但此处起作用的不是 IFS——操作符相邻性由词法分析器决定，因此不得为跟随自定义 IFS 而放宽本列表，否则会把 >\r& 形式的载荷重新合并成一个 segment）。也可以把常量重命名为 BASH_LEXER_WHITESPACE，让读取点也更难产生错误的心智模型。
一点减轻情节的说明，用以界定其影响范围：准确的表述（"bash 把它们当作普通词字符"）已经写在下方八行处的 rule-parser.ts:857-861，测试注释 permission-manager.test.ts:648-653 也是准确的。因此这是 Suggestion 里最轻的一档，只要求改措辞，不涉及行为。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| * separators — bash takes such a character as part of the neighbouring word | ||
| * instead. | ||
| */ | ||
| const BASH_WORD_SEPARATORS = [' ', '\t', '\n']; |
There was a problem hiding this comment.
[Suggestion] R1-3: This constant is module-local, so it adds a second private definition of "bash word separator" while the twin backward scan keeps using JS \s. previousNonWhitespaceChar in packages/core/src/utils/shell-utils.ts:285-293 (if (ch && !/\s/.test(ch)) return ch;) feeds both operator branches of splitCommands (:376 for &, :384 for |), and checkCommandPermissions (shell-utils.ts:2086) additionally rewrites the character to a plain space via cmd.trim().replace(/\s+/g, ' ') before PermissionManager ever sees it. The two splitters now disagree on the same input and nothing tracks the divergence.
Failure scenario. Measured against the built dist at this commit: splitCommands('echo x >\r& rm -rf /tmp/x') returns one segment and getCommandRoots(...) returns ['echo'], so the rm is invisible to every splitCommands consumer — packages/core/src/tools/shell.ts:412, 680, 820, 862, 916, 2142, packages/core/src/utils/shellReadOnlyChecker.ts:349 (the whole-command read-only gate), getCommandRoots (shell-utils.ts:523, used at shell.ts:5333), packages/core/src/tools/monitor.ts:212, and packages/cli/src/serve/daemon-git-worktree-guard.ts:2552. Because the constant is not exported, the twin cannot pick the fix up, and the next person to grep for this bug finds only the fixed copy. The concrete cost is a silently divergent pair of shell splitters inside one permission subsystem — not a wrong verdict on the model-facing path, which this PR does fix.
Witness — A/B across the merge base 87437db784 and this commit, every line identical on both arms, which is what bounds this to a Suggestion:
BASE+PR splitCommands("echo x >\r& rm -rf /tmp/x") = ["echo x >\r& rm -rf /tmp/x"] (twin unfixed)
BASE+PR getCommandRoots(...) = ["echo"] (rm invisible)
BASE+PR normalize -> pm.isCommandAllowed("echo x > & rm -rf /tmp/x") = allow
BASE+PR isShellCommandReadOnly("echo x >\r& rm -rf /tmp/x") = false (read-only gate fails closed)
BASE+PR checkArgumentSafety("echo x >\r& rm -rf /tmp/x") isSafe=false
patterns=["& background operator", "> output redirection"] ($ARGUMENTS route CLOSED)
git diff --stat 87437db784..47d93007ab -- packages/core/src/utils/shell-utils.ts \
packages/core/src/tools/shell.ts packages/core/src/utils/shellReadOnlyChecker.ts \
packages/cli/src/services/prompt-processors/shellProcessor.ts -> empty
Three things that measurement establishes, and why they matter to severity. The model-supplied route is closed: shell-utils.ts:2455-2459 (if (/>\s|\d>/.test(args))) flags >\r because JS \s includes CR, so shellProcessor.ts:109-112 replaces the raw $ARGUMENTS with escapeShellArg(...). The legacy isCommandAllowed(command, config) export (shell-utils.ts:2388) has no production callers (tests only). And the main model-facing path is fixed by this PR — evaluate goes allow at base to deny here for echo x >\r& rm -rf /tmp/x, because tools/shell.ts:2158 hands the raw sub-command to pm.isCommandAllowed where the corrected splitter runs. All four files above are byte-identical between base and this commit, so nothing here is newly wrong or newly reachable.
Suggested fix. Do not copy the array a second time. Export one predicate or constant — export const BASH_WORD_SEPARATORS, or an isBashWordSeparator(ch) in a small shared leaf module — and have both scans use it (rule-parser.ts:868 and shell-utils.ts:288). The import direction is safe: rule-parser.ts does not currently import shell-utils.js (its imports are node:path/fs/os, picomatch, shell-quote, ../utils/debugLogger.js, ../utils/errors.js), while permission-manager.ts:23 already imports ../utils/shell-utils.js, so a shared leaf module avoids any cycle.
If touching shell-utils.ts is out of scope for this PR — the description defers that audit, and #11851's triage agrees — then file a tracked follow-up naming #11851's twin in splitCommands, so the divergence is on a record rather than living only in an issue comment. Either way, exporting the constant here costs nothing and is what lets the follow-up reuse it, which the description already anticipates for #11821.
A fix in shell-utils.ts must not break CRLF splitting: shell-utils.ts:391 (} else if (char === '\r' && nextChar === '\n') {) treats CRLF as a command separator, mirrored at :1857, and is pinned by getCommandRoots('grep pattern file\r\ncurl evil.com') → ['grep', 'curl'] (shell-utils.test.ts:632-635) and getCommandRoots('echo SAFE \\\r\nrm -rf /') (same file, :660). Treating a lone \r as a word character must not stop \r\n from splitting, and since previousNonWhitespaceChar also feeds the | branch at :384 (prevChar === '>' for >|), both call sites change together.
Please pin any such fix with a test that fails without it: in packages/core/src/utils/shell-utils.test.ts, beside the existing splitCommands tables (:1497) and the getCommandRoots block (:597), assert splitCommands('echo x >\r& rm -rf /tmp/x') yields two segments and getCommandRoots yields ['echo', 'rm'] — both red today — then restore /\s/ in previousNonWhitespaceChar and confirm they red again.
中文说明
这个常量是模块内私有的，因此它实际上新增了第二份"bash 词分隔符"的私有定义，而孪生的回扫仍在使用 JS 的 \s。packages/core/src/utils/shell-utils.ts:285-293 的 previousNonWhitespaceChar（if (ch && !/\s/.test(ch)) return ch;）同时供给 splitCommands 的两个操作符分支（& 在 :376，| 在 :384）；此外 checkCommandPermissions（shell-utils.ts:2086）会在 PermissionManager 看到命令之前，通过 cmd.trim().replace(/\s+/g, ' ') 把该字符改写成普通空格。于是两个切分器对同一输入的判断不再一致，而没有任何机制跟踪这一分歧。
失败场景。 针对本提交已构建的 dist 实测：splitCommands('echo x >\r& rm -rf /tmp/x') 返回一个 segment，getCommandRoots(...) 返回 ['echo']，因此 rm 对每一个 splitCommands 消费者都不可见——packages/core/src/tools/shell.ts:412, 680, 820, 862, 916, 2142、packages/core/src/utils/shellReadOnlyChecker.ts:349（整条命令的只读判定门）、getCommandRoots（shell-utils.ts:523，在 shell.ts:5333 使用）、packages/core/src/tools/monitor.ts:212，以及 packages/cli/src/serve/daemon-git-worktree-guard.ts:2552。由于常量未导出，孪生扫描无法复用这次修复；下一个人来 grep 这个 bug 时只会找到已修好的那一份。具体代价是：同一个权限子系统内部存在一对静默分歧的 shell 切分器——而不是面向模型的主路径上出现错误判定，那条路径本 PR 确实修好了。
证据——针对合并基线 87437db784 与本提交做 A/B，每一行在两侧完全相同，这也正是把它限定为 Suggestion 的依据（原始输出见上方英文部分）。该实测确立了三件事，且都影响严重级别判定：模型可控载荷的路径是关闭的（shell-utils.ts:2455-2459 的 if (/>\s|\d>/.test(args)) 会因为 JS \s 包含 CR 而标记 >\r，于是 shellProcessor.ts:109-112 用 escapeShellArg(...) 替换原始 $ARGUMENTS）；遗留导出 isCommandAllowed(command, config)（shell-utils.ts:2388）没有生产调用方（仅测试）；而面向模型的主路径确实被本 PR 修好——对 echo x >\r& rm -rf /tmp/x，evaluate 从基线的 allow 变为这里的 deny，因为 tools/shell.ts:2158 把原始子命令交给 pm.isCommandAllowed，那里运行的正是修正后的切分器。上述四个文件在基线与本提交之间逐字节相同，所以这里没有任何"新变错"或"新可达"的成分。
修复建议。 不要把数组再复制一份。导出同一个谓词或常量——export const BASH_WORD_SEPARATORS，或在一个小的共享叶子模块里提供 isBashWordSeparator(ch)——让两处扫描都用它（rule-parser.ts:868 与 shell-utils.ts:288）。导入方向是安全的：rule-parser.ts 当前并不导入 shell-utils.js（它的导入是 node:path/fs/os、picomatch、shell-quote、../utils/debugLogger.js、../utils/errors.js），而 permission-manager.ts:23 已经导入了 ../utils/shell-utils.js，因此放在共享叶子模块可以避免任何循环依赖。
如果在本 PR 里改 shell-utils.ts 超出范围——PR 描述已把该排查延后，#11851 的 triage 也认同——那就开一个可跟踪的 follow-up，点名 splitCommands 里 #11851 的孪生问题，让这一分歧进入正式记录，而不是只存在于某条 issue 评论中。无论哪种做法，在这里导出常量都没有成本，而且正是它让 follow-up 得以复用；PR 描述对 #11821 也已有同样的预期。
在 shell-utils.ts 里修复时不得破坏 CRLF 切分：shell-utils.ts:391（} else if (char === '\r' && nextChar === '\n') {）把 CRLF 当作命令分隔符，:1857 有对应实现，并由 getCommandRoots('grep pattern file\r\ncurl evil.com') → ['grep', 'curl']（shell-utils.test.ts:632-635）与 getCommandRoots('echo SAFE \\\r\nrm -rf /')（同文件 :660）钉住。把单独的 \r 视为词字符，不得导致 \r\n 不再切分；此外由于 previousNonWhitespaceChar 也供给 :384 的 | 分支（>| 的 prevChar === '>'），两个调用点必须一起改。
任何这类修复都请配一个"去掉修复就会失败"的测试：在 packages/core/src/utils/shell-utils.test.ts 中，紧挨现有的 splitCommands 表格（约 :1497）与 getCommandRoots 块（约 :597），断言 splitCommands('echo x >\r& rm -rf /tmp/x') 产出两个 segment、getCommandRoots 产出 ['echo', 'rm']——这两条今天都是红的——然后把 previousNonWhitespaceChar 里的 /\s/ 恢复回去，确认它们再次变红。
— qwen3.8-max via Qwen Code /review (v0.23.3)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Verified the predicate change itself and I agree with the open R1-1 — plus one extension it does not mention (inline below).
The one-line change is correct and moves only in the tightening direction: the skip set strictly shrank (/\s/ → [' ', '\t', '\n']), so the backward scan can only stop earlier, and the only outcome flip is false → true — more splits, never fewer, and each new segment then gets its own rule check. &&, |&, &>, >&, <& still cannot reach the changed branch, and \> still reads as a literal argument through precedingBackslashCount. I also spot-checked R1-3/R1-4/R1-5/R1-6 — all grounded, nothing to dispute — and confirmed the sibling PRs do not collide with this one (#11821 inserts 9 lines below with no hunk overlap; #11765 reworks a different scan loop and composes either way).
The inline comment is the reason I am posting at all: the R1-1 class is slightly broader than the thread states.
| for (let j = index - 1; j >= 0; j--) { | ||
| const ch = command[j]!; | ||
| if (/\s/.test(ch)) { | ||
| if (BASH_WORD_SEPARATORS.includes(ch)) { |
There was a problem hiding this comment.
[extends R1-1] The same root cause also produces a wrong operation type, not just a lost deny, when the command consumes arguments.
Take cat >\u00a0& echo hi:
- At this head the new boundary splits at
&; the first segment's.trim()(:958/:969) strips the\u00a0, leavingcat >, andtokenizeyields[cat, >]. - In
extractRedirects(packages/core/src/permissions/shell-semantics.ts:248-250), a separate-token>with no following target is never added totoRemove, so the bare>survives intocat's positional args. catdispatches toreadOps(:513), andlooksLikePath('>')istrue(:195), so the operation mutates from the realwrite_file <cwd>/\u00a0to a spuriousread_file <cwd>/>— the invisible-file write is still uncertified, and now an invented read is what aReadallow could match.
The same fix should cover it (consume the dangling redirect, or keep the segment whole when its redirect has no in-segment target), but this shape is worth adding to the test matrix too: the failure is not only "no write op extracted" but "a wrong-typed op extracted".
4 participants
What this PR does
isAsyncOperatorinpackages/core/src/permissions/rule-parser.tsdecides whether a bare&is the async (background) operator or part of a redirection by scanning backward from the&and skipping whitespace. It skipped characters using JavaScript's/\s/, which also matches\r,\v,\f,\u00a0and other Unicode whitespace — none of which bash treats as word separators (bash's defaultIFSis space, tab and newline only). The scan now skips only those three characters, via a new module-localBASH_WORD_SEPARATORSconstant.Why it's needed
This was a fail-open permission bypass. For
echo x >\r& rm -rf ~/important, bash runs two commands (a backgroundedecho xwriting to a file literally named\r, then a foregroundrm), but the splitter kept both halves in one segment — so the second command never got its own permission rule check, and an allow rule matching only the first command (Bash(echo x)) covered therm. Full analysis and reproduction are in the linked issue.Reviewer Test Plan
How to verify
npx vitest run src/permissions/permission-manager.test.ts --root packages/core— 433/433 pass, including 5 new cases pinning\r,\v,\f,\u00a0(and the spaced variantecho x > \r & rm …) between>and&, asserting two segments. All 5 fail if the one-line fix is reverted (verified locally).packages/core/dist):splitCompoundCommandSegments('echo x >\r& echo DANGER'): 1 segment → 2 segments (same for\v,\f,\u00a0, spaced variant).Bash(echo x),PermissionManager.isCommandAllowed('echo x >\r& rm -rf /tmp/…'):allow→ask(fail-closed). Plain-space controlecho x & rm …staysask; benignecho x >out & echo donestaysallow.echo x >\r& echo DANGER_CRrunsecho DANGER_CRas its own foreground command and creates a 2-byte file named\r; the real-space controlecho x > & echo yis a bash syntax error — confirming these characters are not IFS whitespace.&>/&>>short-circuit before the scan,2>&1/>&2/exec 3<&4hit the redirection char adjacently, and escaped\>/\<still background.Evidence (Before & After)
N/A (non-UI logic change; witnesses are the command outputs above)
Tested on
Environment (optional)
Local vitest + node 24 against
packages/core/dist; bash 5.x xtrace for the shell ground truth.Risk & Scope
>&/<&descriptor duplication requires adjacency (echo x > &1is a syntax error), so no legitimate redirection can have one of these characters between the operator and the&— narrowing the skip set is monotonically fail-closed. Existing tests for&>,2>&1,>&2,exec 3<&4, quoted&, and arithmetic&all still pass (542/542 acrosspermission-manager.test.ts+shell-semantics.test.ts, 172/172 inshell-utils.test.ts;tsc --noEmitclean for packages/core).\s-as-bash-separator pattern inpackages/core/src/utils/shell-utils.ts(e.g. itsisCommentStart/isWordBoundary) — flagged in the issue's triage comment for a separate audit; the siblingisCommentStartinstance inrule-parser.tsis fix(core): treat a word-initial # as a comment when splitting shell commands #11821's scope. Sequencing note: this PR addsBASH_WORD_SEPARATORSadjacent to where fix(core): treat a word-initial # as a comment when splitting shell commands #11821 insertsisCommentStart, so whichever lands second needs a trivial rebase; the constant is here for it to reuse.BASH_WORD_SEPARATORSis module-local (not exported); fix(core): treat a word-initial # as a comment when splitting shell commands #11821 can derive its whitespace half from it when it lands.Linked Issues
Fixes #11851
中文说明
这个 PR 做了什么
packages/core/src/permissions/rule-parser.ts里的isAsyncOperator通过从&向前回扫并跳过空白字符，来判断裸&是异步（后台）操作符还是重定向的一部分。它之前用 JavaScript 的/\s/跳过字符，而\s还会匹配\r、\v、\f、\u00a0等 Unicode 空白——bash 并不把这些当作词分隔符（bash 默认IFS只有空格、tab、换行）。现在回扫只跳过这三个字符（新增模块内常量BASH_WORD_SEPARATORS）。为什么需要
这是一个 fail-open 权限绕过。对于
echo x >\r& rm -rf ~/important，bash 实际执行两条命令（后台的echo x写入名为\r的文件，然后前台执行rm），但切分器把两半留在同一个 segment 里——第二条命令没有经过自己的权限规则检查，只匹配第一条命令的 allow 规则（Bash(echo x)）就覆盖了rm。完整分析与复现见关联 issue。评审验证计划
见上方英文部分：核心是看 5 条新回归测试（钉住
\r/\v/\f/\u00a0及带空格变体），修复前它们全部失败、修复后全部通过；端到端 witness 从allow变为ask。风险与范围
>&/<&必须相邻，因此收窄跳过集合只会更安全（fail-closed 方向单调）。既有测试全部通过。shell-utils.ts里同形态候选点（另行排查）；isCommentStart的同类问题归 fix(core): treat a word-initial # as a comment when splitting shell commands #11821。本 PR 先落地共享常量，fix(core): treat a word-initial # as a comment when splitting shell commands #11821 合入时可复用。