fix(core): remove env from read-only shell command allowlist#4932
Merged
Conversation
The `env` command is a command proxy that can execute arbitrary commands with side effects (e.g., `env open -a Calculator`, `env rm -rf ...`). Including it in the read-only allowlist bypasses the user confirmation prompt, enabling prompt-injection attacks to achieve arbitrary code execution without user interaction. Remove `env` from both `READ_ONLY_ROOT_COMMANDS` sets (regex-based checker in shellReadOnlyChecker.ts and AST-based checker in shellAstParser.ts) so all `env` invocations require user confirmation. Fixes #4930
Collaborator
|
Thanks for the PR! Template is mostly there — has Summary, Changes, Reviewer Test Plan with Before/After, and Chinese translation. Missing the On direction: this is a clean, well-motivated security fix. On approach: the scope is minimal and correct — two single-line deletions from two No existing tests assert Moving on to code review and testing. 🔍 中文说明感谢贡献！ 模板基本完整——有 Summary、Changes、Reviewer Test Plan（含 Before/After）和中文说明。缺少模板中的 方向：这是一个干净、有充分动机的安全修复。 方案：范围最小且正确——从两个 没有现有测试单独断言 进入代码审查和测试阶段 🔍 — Qwen Code · qwen3.7-max |
tanzhenxin
approved these changes
Jun 10, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
This closes a real prompt-injection-to-arbitrary-execution bypass, and I verified the path end-to-end: an allowlisted root command that has no command-specific analysis is treated as read-only unconditionally, which resolves the shell tool's default permission to auto-allow — so env open -a Calculator (or env rm -rf ...) ran with no confirmation prompt. Removing env flips that to a confirmation. Patching both the AST-based checker and the deprecated regex checker covers the complete set of read-only gates, since the regex path is still reachable as a fallback when WASM initialization fails. The removal is also complete rather than redundant: neither checker unwraps env to classify the inner command, and legitimate environment inspection stays prompt-free via printenv.
Two non-blocking notes. The 181 existing tests pass unmodified precisely because env <program> was never covered, so a small regression test asserting that env-wrapped commands are not read-only would guard against the allowlist regressing. And a couple of weaker pre-existing entries (sort -o / sort --compress-program, awk -f scriptfile) can still write or execute under specific flags with auto-approval — worth a follow-up, not this PR.
Verdict
APPROVE — correct, minimal, and complete fix for a silent auto-approval bypass.
Collaborator
Code ReviewIndependent proposal: Remove The PR matches this proposal exactly — two single-line deletions, nothing more. The diff is clean, the scope is minimal, and the change follows project conventions. No correctness issues, no security concerns, no regressions. One thing worth noting: Unit TestsAll 181 tests pass (148 in Real-Scenario TestingTested with Before (installed build)The installed build classified After (this PR via dev build)Both runs executed in Assessment: The code change is correct and minimal. The vulnerability is real, the fix is sound, and no regressions are introduced. 中文说明代码审查独立方案： 从 PR 与此方案完全一致——两处单行删除，没有多余内容。diff 干净、范围最小、符合项目规范。无正确性问题、无安全隐患、无回归。 值得注意的是： 单元测试181 个测试全部通过（ 真实场景测试使用 两个版本都在 评估： 代码变更正确且最小化。漏洞真实存在，修复合理，无回归。 — Qwen Code · qwen3.7-max |
Collaborator
|
Clean security fix, minimal scope, well-documented. The vulnerability is real ( Approving. ✅ 中文说明干净的安全修复，范围最小，文档完善。漏洞真实存在（ 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 10, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
envis a command proxy — it can execute arbitrary commands with side effects (env open -a Calculator,env rm -rf ..., etc.). Having it inREAD_ONLY_ROOT_COMMANDSbypasses the user confirmation prompt, allowing prompt-injection attacks to achieve arbitrary code execution without user interaction.This fix removes
envfrom both read-only command sets so allenvinvocations now require user confirmation.Fixes #4930.
Changes
packages/core/src/utils/shellReadOnlyChecker.ts— remove'env'fromREAD_ONLY_ROOT_COMMANDS(deprecated regex-based checker)packages/core/src/utils/shellAstParser.ts— remove'env'fromREAD_ONLY_ROOT_COMMANDS(current AST-based checker)Verification
All 181 existing tests in
shellAstParser.test.ts(148 tests) andshellReadOnlyChecker.test.ts(33 tests) pass without modification.Reviewer Test Plan
How to verify: Start
qwenin default mode and tryPlease help me run this command: env open -a Calculator. Before this fix, the command executes without confirmation. After this fix, the user is prompted for confirmation.Before:
env open -a Calculator→ no confirmation prompt → calculator opens.After:
env open -a Calculator→ user confirmation prompt appears.中文说明
概述
env不仅是"打印环境变量"的工具，它本质上是一个命令代理——可以执行任意带副作用的命令（如env open -a Calculator、env rm -rf ...等）。将其放在只读命令白名单中会跳过用户确认，使得 prompt injection 攻击可以在无确认情况下执行任意代码。改动
从
shellReadOnlyChecker.ts（已废弃的正则方案）和shellAstParser.ts（当前 AST 方案）的READ_ONLY_ROOT_COMMANDS中各移除'env'一行。验证
shellAstParser.test.ts（148 个测试）和shellReadOnlyChecker.test.ts（33 个测试）共 181 个测试全部通过，无需修改测试用例。