fix(core): treat backslash as literal inside single quotes in splitCommands - #7526
Conversation
…mmands The escape branch in splitCommands consumed the character after a backslash regardless of quote state. The shell performs no escaping inside single quotes, so `'a\'` closes the quote — but the parser read `\'` as an escaped quote, stayed "inside" the string, and swallowed every following separator. Because splitCommands is the segmentation primitive behind getCommandRoots and checkCommandPermissions, the trailing commands were invisible to the permission checks. With a `ShellTool(rm)` deny rule, `echo hi; rm -rf /tmp/x` is hard-denied but `echo 'a\'; rm -rf /tmp/x` was reported as fully allowed. Guarding the escape branch with !inSingleQuotes lets the backslash fall through as a literal character, so the quote closes where the shell closes it. Escapes outside single quotes (including inside double quotes, where the shell does escape) are unchanged.
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with concrete evidence. The permission-gate table shows Direction: aligned — the shell command parser must match bash quoting semantics for the permission system to be trustworthy. A backslash inside single quotes is a literal character in every POSIX shell; treating it as an escape is a correctness bug with security implications. Size: 7 production lines (6 add, 1 del) in Approach: the scope is exactly right — one guard condition plus regression tests. The line-continuation branch immediately above already has the Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有具体证据。权限门控对照表显示 方向：对齐——shell 命令解析器必须匹配 bash 引号语义，权限系统才可信。单引号内的反斜杠在所有 POSIX shell 中都是字面字符；将其视为转义是带有安全影响的正确性 bug。 规模： 方案：范围恰好——一个守卫条件加回归测试。紧邻上方的续行分支已因同样原因加了 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: reading the title and "Why it's needed" section, I would add Comparison with the diff: the PR does exactly this. The fix is the minimal correct change — the line-continuation branch ( Downstream consumers of No blockers. No AGENTS.md violations. Real-Scenario TestingParser-level security fix — verified via deterministic unit tests and bash ground truth in tmux. Before (guard reverted — bug reproduces)After (PR fix applied — all pass)Full suite (with fix)Bash ground truth
中文说明代码审查独立方案： 仅看标题和"为什么需要"一节，我的做法是在 与 diff 对比： PR 正是这样做的。修复是最小正确改动——续行分支已有守卫，通用转义分支是唯一遗漏的。注释解释了"为什么"（单引号语义、权限检查影响），没有过度叙述。测试锁定了三种行为。无正确性 bug、无过度抽象、无范围蔓延。
无阻塞项。无 AGENTS.md 违规。 真实场景测试解析器层安全修复——通过确定性单元测试和 bash 基准事实在 tmux 中验证。 修复前（还原守卫）：2 个测试失败。修复后：150/150 通过。Bash 基准事实确认单引号内 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is exactly the kind of PR you want to see: a one-line correctness fix with a clear security implication, backed by bash ground truth and three regression tests that pin the behavior from both directions. The author noticed the line-continuation branch already had the My independent proposal matched the PR exactly. The diff carries nothing beyond the minimal change: one guard condition, one explanatory comment, three tests. No drive-by refactors, no scope creep. The fail-before/pass-after is verified, and the full 150-test suite stays green. The security framing is warranted, not theoretical: If I had to maintain this in six months, I'd thank the author — the comment explains the why, the tests document the expected behavior, and the fix is trivially auditable. 中文说明置信度：5/5 —— 每个阶段都干净；毫不犹豫可以合并。 这正是你希望看到的 PR：一行正确性修复，有明确的安全影响，以 bash 基准事实和三个从两个方向锁定行为的回归测试为支撑。作者注意到续行分支已有 我的独立方案与 PR 完全一致。diff 不包含最小改动之外的任何内容：一个守卫条件、一条解释性注释、三个测试。无顺手重构、无范围蔓延。fail-before/pass-after 已验证，完整 150 测试套件保持绿色。 安全定性是合理的，不是理论性的： 如果六个月后我要维护这段代码，我会感谢作者——注释解释了"为什么"，测试记录了预期行为，修复易于审计。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Heads-up on the red Bisected to #7460 ( |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Maintainer Local Build & E2E Test Report
Branch: fix/split-commands-single-quote-backslash @ dd891876ed
Environment: macOS (darwin), Node v22.22.2
✅ Unit Tests (all pass on PR branch)
| Test suite | Tests | Result |
|---|---|---|
shell-utils.test.ts |
150 | ✅ PASS |
shell.test.ts + monitor.test.ts |
358 | ✅ PASS |
shellReadOnlyChecker.test.ts |
230 | ✅ PASS |
shellAstParser.test.ts |
542 | ✅ PASS |
shell-semantics.test.ts |
103 | ✅ PASS |
| Total | 1383 | ✅ ALL PASS |
✅ Fail-before verification
Reverting only the !inSingleQuotes && guard causes exactly 2 of the 3 new tests to fail:
× checkCommandPermissions > should not let a backslash inside single quotes hide a blocked command
→ expected true to be false // Object.is equality (allAllowed was true — bypass!)
× getCommandRoots > should treat a backslash inside single quotes as literal, not an escape
→ expected [ 'echo' ] to deeply equal [ 'echo', 'rm' ] (rm invisible!)
✓ getCommandRoots > should still honour backslash escapes outside single quotes
(passes both before and after — guard against over-correction)
✅ Bash ground truth
$ bash -c "echo 'a\\'; echo INJECTED"
aINJECTED ← two commands, as the PR describes
✅ Built-module E2E verification (6/6 pass)
Ran checkCommandPermissions, getCommandRoots, and splitCommands from the compiled dist/ output with a mock ShellTool(rm) deny rule:
--- Test 1: splitCommands ---
Segments: ["echo 'a\\'","rm -rf /tmp/x"] ✅ 2 segments
--- Test 2: getCommandRoots ---
Roots: ["echo","rm"] ✅ rm visible
--- Test 3: checkCommandPermissions (exploit) ---
allAllowed: false ✅ HARD-DENIED
isHardDenial: true
disallowed: ["rm -rf /tmp/x"]
--- Test 4-5: control (normal chain / direct rm) ---
Both hard-denied ✅
--- Test 6: double-quote escape (no over-correction) ---
Roots: ["echo"] ✅ one command
🔴 Critical: splitCompoundCommand in rule-parser.ts has the same bug — the real CLI is still vulnerable
The PR fixes splitCommands in shell-utils.ts, which is used by the legacy checkCommandPermissions path (no PermissionManager). However, the real CLI uses the PermissionManager path, which calls splitCompoundCommand from packages/core/src/permissions/rule-parser.ts (line 694):
// rule-parser.ts:694 — NO inSingle guard!
if (ch === '\\') {
escaped = true;
continue;
}Verified on the PR branch:
$ npx tsx .qwen/scripts/pr7526-pm-verify.ts
Input: echo 'a\'; rm -rf /tmp/x
splitCompoundCommand: ["echo 'a\\'; rm -rf /tmp/x"]
Bug present: YES — rm is hidden from PM!
Full CLI E2E reproduction (bundled dist/cli.js, --yolo, deny rule Bash(rm *) in .qwen/settings.json):
$ node dist/cli.js -p "Run this exact shell command: echo 'a\'; rm -rf /tmp/pr7526-canary3" --yolo
命令已成功执行：
- 输出了 `a\`
- `/tmp/pr7526-canary3` 已被删除 ← rm executed despite deny rule!
Control test (direct rm) is correctly blocked:
$ node dist/cli.js -p "Run this exact shell command: rm -rf /tmp/pr7526-canary2" --yolo
Warning: Tool "run_shell_command" requires user approval...
该命令被权限规则拒绝（deny rule: Bash(rm *)） ← correctly denied
Attack chain:
splitCompoundCommand("echo 'a\\'; rm ...")→ 1 segment (bug:\'treated as escaped quote)- PM
hasRelevantRules:subCommands.length > 1is false → falls through to single-command matching Bash(rm *)doesn't matchecho 'a\'; rm ...(starts withecho) →hasRelevantRulesreturns false- PM evaluation skipped entirely
- Tool default permission is
'ask'→ YOLO auto-approves rmexecutes
Suggested fix — same one-line pattern in rule-parser.ts:694:
- if (ch === '\\') {
+ if (ch === '\\' && !inSingle) {The splitCommands fix in this PR is correct and should be kept. But the PR is incomplete without also fixing splitCompoundCommand, which is the code path the real CLI actually uses.
中文版本
维护者本地构建 & E2E 测试报告
分支： fix/split-commands-single-quote-backslash @ dd891876ed
环境： macOS (darwin), Node v22.22.2
✅ 单元测试（PR 分支全部通过）
| 测试套件 | 用例数 | 结果 |
|---|---|---|
shell-utils.test.ts |
150 | ✅ 通过 |
shell.test.ts + monitor.test.ts |
358 | ✅ 通过 |
shellReadOnlyChecker.test.ts |
230 | ✅ 通过 |
shellAstParser.test.ts |
542 | ✅ 通过 |
shell-semantics.test.ts |
103 | ✅ 通过 |
| 合计 | 1383 | ✅ 全部通过 |
✅ Fail-before 验证
仅还原 !inSingleQuotes && 判断后，3 个新测试中恰好 2 个失败：
checkCommandPermissions测试：allAllowed为true（绕过！）getCommandRoots测试：只返回['echo']（rm不可见！）- 双引号转义测试：修复前后均通过（防止过度修正的守卫）
✅ Bash 基准事实
bash -c "echo 'a\\'; echo INJECTED" 输出 a\ 和 INJECTED（两条命令），与 PR 描述一致。
✅ 构建产物 E2E 验证（6/6 通过）
从编译后的 dist/ 导入 checkCommandPermissions、getCommandRoots、splitCommands，配合 ShellTool(rm) deny 规则：利用命令被硬拒绝，rm 可见，双引号转义未被过度修正。
🔴 Critical：rule-parser.ts 中的 splitCompoundCommand 存在同样的 bug——真实 CLI 仍然 vulnerable
本 PR 修复了 shell-utils.ts 中的 splitCommands，该函数用于遗留的 checkCommandPermissions 路径（无 PermissionManager）。但真实 CLI 使用 PermissionManager 路径，调用的是 packages/core/src/permissions/rule-parser.ts（第 694 行）的 splitCompoundCommand：
// rule-parser.ts:694 — 没有 inSingle 判断！
if (ch === '\\') {
escaped = true;
continue;
}在 PR 分支上验证：
splitCompoundCommand("echo 'a\\'; rm -rf /tmp/x") 返回 ["echo 'a\\'; rm -rf /tmp/x"]（1 个段），rm 对 PM 不可见。
完整 CLI E2E 复现（打包后的 dist/cli.js，--yolo 模式，.qwen/settings.json 中配置 Bash(rm *) deny 规则）：
- 利用命令
echo 'a\'; rm -rf /tmp/pr7526-canary3：rm 成功执行，canary 目录被删除 ❌ - 对照组
rm -rf /tmp/pr7526-canary2：正确被 deny 规则阻止 ✅
攻击链：
splitCompoundCommand把利用命令当作 1 个段（bug：\'被当作转义引号）- PM
hasRelevantRules：subCommands.length > 1为 false → 走单命令匹配 Bash(rm *)不匹配echo 'a\'; rm ...（以echo开头）→hasRelevantRules返回 false- PM 评估被完全跳过
- 工具默认权限为
'ask'→ YOLO 自动批准 rm执行
建议修复 —— 在 rule-parser.ts:694 应用同样的一行模式：
- if (ch === '\\') {
+ if (ch === '\\' && !inSingle) {本 PR 对 splitCommands 的修复是正确的，应当保留。但不同时修复 splitCompoundCommand（真实 CLI 实际使用的代码路径），PR 是不完整的。
gwinthis
left a comment
gwinthis
left a comment
There was a problem hiding this comment.
Review: APPROVE (C=0)
Summary
Security-critical fix (+40/-1) in splitCommands: backslash is now treated as literal inside single quotes, matching shell behavior. Before this fix, 'a\'; rm -rf /tmp/x was parsed as a single echo command, hiding rm from permission checks.
The Bug
Inside single quotes, the shell performs no escaping — 'a\' closes the quote at the second ', then ; separates commands. But splitCommands consumed \' as an escaped quote, keeping the parser "inside" the quote and swallowing every separator to end of line. This meant ShellTool(rm) deny rules never saw the rm command.
The Fix
One line: if (!inSingleQuotes && char === '\\' && ...) — only treat backslash as escape outside single quotes.
Test Coverage
3 new tests, all pinning the correct behavior:
- Single quotes:
echo 'a\'; rm -rf /tmp/x→ 2 commands (echo,rm) — the security fix - Double quotes:
echo "a\\"; rm -rf /tmp/x"→ 1 command (backslash escapes inside double quotes — correct) - Unquoted:
echo a\; rm -rf /tmp/x→ 1 command (backslash escapes outside quotes — correct)
Plus integration test: checkCommandPermissions correctly denies rm in echo 'a\'; rm -rf /tmp/x.
Pattern
Parser must match shell semantics exactly: Shell parsing rules differ between quote types — single quotes are literal (no escaping), double quotes allow escaping. A parser that treats them uniformly creates exploitable gaps between what the parser sees and what the shell executes. Security-critical parsers must be tested against shell truth, not against "reasonable" assumptions.
中文说明
评审：APPROVE (C=0)
概要
安全关键修复（+40/-1）：splitCommands 现在在单引号内将反斜杠视为字面字符，匹配 shell 行为。修复前，'a\'; rm -rf /tmp/x 被解析为单个 echo 命令，隐藏了 rm，绕过权限检查。
模式
解析器必须精确匹配 shell 语义： 单引号内无转义（字面），双引号内允许转义。统一处理两类引号的解析器会创建可利用的间隙——解析器看到的与 shell 执行的不一致。安全关键解析器必须针对 shell 真值测试。
— qwen3.7-max via Qwen Code /review
4 participants
What this PR does
splitCommandsapplied its backslash-escape branch regardless of quote state. This guards it with!inSingleQuotes, so a backslash inside single quotes is treated as the literal character the shell treats it as. One-line change plus regression tests.Why it's needed
The shell performs no escaping inside single quotes —
\is an ordinary character there, so'a\'closes the quote:The parser instead read
\'as an escaped quote, stayed "inside" the string, and swallowed every separator to the end of the line. (The line-continuation branch immediately above already guards on!inSingleQuotesfor exactly this reason; the general escape branch was missed.)splitCommandsis the segmentation primitive behindgetCommandRootsandcheckCommandPermissions, so the hidden commands were never handed to the permission checks. With aShellTool(rm)deny rule configured, onmain:allAllowedisHardDenialrm -rf /tmp/xfalsetrueecho hi; rm -rf /tmp/xfalsetrueecho 'a\'; rm -rf /tmp/xtrueundefinedgetCommandRoots("echo 'a\\'; rm -rf /tmp/x")likewise returned["echo"]—rmwas invisible. After the fix all three rows hard-deny and the roots are["echo", "rm"].Escapes outside single quotes are unchanged, which is correct: inside double quotes the shell does treat
\as an escape, soecho "a\"; rm ..."legitimately stays one command.Reviewer Test Plan
How to verify
npx vitest run --root packages/core src/utils/shell-utils.test.ts→ 150/150.!inSingleQuotes &&guard fails two of them:getCommandRoots > should treat a backslash inside single quotes as literal, not an escape— expects['echo', 'rm'], gets['echo'].checkCommandPermissions > should not let a backslash inside single quotes hide a blocked command— expects a hard denial ofrm -rf /tmp/x, getsallAllowed: true.should still honour backslash escapes outside single quotespasses both before and after — it is the guard against over-correcting into double-quoted and unquoted escapes.bashitself:bash -c "echo 'a\\'; echo INJECTED"printsa\thenINJECTED, i.e. two commands.Evidence (Before & After)
Not user-visible; verified by the deterministic unit tests above and by the permission-gate table in "Why it's needed" (reproduced with the same mocked
Configthe existingcheckCommandPermissionstests use).echo 'a\'; rm -rf /tmp/x→ one segment, rootecho, deny rule bypassed.echo+rm,rm -rf /tmp/xhard-denied.Tested on
macOS:
shell-utils(150),shellReadOnlyChecker(230),shellAstParser(542),tools/shell+tools/monitor(358) all pass locally, with proven fail-before/pass-after. The change is pure string parsing with no platform-dependent behavior and the assertions are deterministic, so no manual QA is required; CI covers Windows/Linux.Environment (optional)
Node v24;
@qwen-code/qwen-code-coreworkspace; vitest 3.2.Risk & Scope
packages/desktop/packages/shared/src/utils/cli-icon-resolver.tshas its own independentsplitCommandsused only for picking a display icon; it has no security role and is deliberately untouched.Linked Issues
None — found by reading
splitCommandsagainst bash quoting semantics.中文说明
本 PR 的作用
splitCommands的反斜杠转义分支不区分引号状态即被应用。本 PR 为其加上!inSingleQuotes判断，使单引号内的反斜杠按 shell 的语义作为普通字面字符处理。一行改动，外加回归测试。为什么需要
shell 在单引号内不做任何转义——
\在其中只是普通字符，因此'a\'会闭合该引号：而解析器把
\'当成被转义的引号，因而一直"停留在"字符串内部，吞掉了该行后续所有分隔符。（紧邻其上的续行分支早已针对同样的原因加了!inSingleQuotes判断；通用转义分支则被遗漏了。）splitCommands是getCommandRoots与checkCommandPermissions背后的分段原语，因此被吞掉的命令根本不会交给权限检查。在配置了ShellTool(rm)拒绝规则的情况下，main上的表现为：allAllowedisHardDenialrm -rf /tmp/xfalsetrueecho hi; rm -rf /tmp/xfalsetrueecho 'a\'; rm -rf /tmp/xtrueundefinedgetCommandRoots("echo 'a\\'; rm -rf /tmp/x")同样只返回["echo"]——rm完全不可见。修复后三行均为硬拒绝，命令根为["echo", "rm"]。单引号之外的转义行为保持不变，这是正确的：在双引号内 shell 确实会把
\当作转义符，所以echo "a\"; rm ..."理应仍是一条命令。复核测试计划
如何验证
npx vitest run --root packages/core src/utils/shell-utils.test.ts→ 150/150 通过。!inSingleQuotes &&判断，其中两个会失败：getCommandRoots > should treat a backslash inside single quotes as literal, not an escape——期望['echo', 'rm']，实得['echo']。checkCommandPermissions > should not let a backslash inside single quotes hide a blocked command——期望对rm -rf /tmp/x硬拒绝，实得allAllowed: true。should still honour backslash escapes outside single quotes在修复前后均通过——它用于防止过度修正而波及双引号内与无引号的转义。bash本身：bash -c "echo 'a\\'; echo INJECTED"会先输出a\再输出INJECTED，即两条命令。证据（修复前后对比）
非用户可见；由上述确定性单元测试，以及"为什么需要"一节中的权限门控对照表验证（该表使用与现有
checkCommandPermissions测试相同的 mockConfig复现）。echo 'a\'; rm -rf /tmp/x→ 单个分段，命令根为echo，拒绝规则被绕过。echo+rm，rm -rf /tmp/x被硬拒绝。测试环境
macOS：
shell-utils（150）、shellReadOnlyChecker（230）、shellAstParser（542）、tools/shell+tools/monitor（358）本地全部通过，并已验证 fail-before/pass-after。该改动是纯字符串解析，无平台相关行为，断言均为确定性，因此无需人工 QA；Windows/Linux 由 CI 覆盖。运行环境（可选）
Node v24；
@qwen-code/qwen-code-core工作区；vitest 3.2。风险与影响范围
packages/desktop/packages/shared/src/utils/cli-icon-resolver.ts有一份独立的splitCommands，仅用于选择显示图标，不承担安全职责，本 PR 有意不改动它。关联 Issue
无——通过对照 bash 引号语义阅读
splitCommands发现。