fix(cli): extend the #8663 loader denylist and harden its scrub lifecycle - #8763
Conversation
…ycle Follow-up to #8663. Its inherited-env denylist closed the NODE_OPTIONS/ NODE_PATH class but left sibling code-execution and TLS-trust-anchor vars that reach the same #8653 cross-workspace outcome — an untrusted workspace `.env` is frozen into daemonRuntimeBaseEnv and distributed to every workspace's session subprocesses. Denylist additions, split by the PR's own tiering: - Scrubbed loader tier (INHERITED_LOADER_ENV_KEYS — scrubbed from the inherited launch env and rejected from every `.env`/settings.env scope), for pure-injection vars with no legitimate operator-shell use: OPENSSL_CONF (startup dlopen of an attacker OpenSSL engine), NODE_REPL_EXTERNAL_MODULE, npm_config_node_gyp, npm_config_init_module. - Reject-from-project-`.env` tier (PROJECT_ENV_HARDCODED_EXCLUSIONS — rejected from project files, preserved from the shell / home `.env`), for vars with a legitimate operator-shell use whose only exposed vector is an untrusted project file: * TLS trust anchors SSL_CERT_FILE, SSL_CERT_DIR, CURL_CA_BUNDLE, REQUESTS_CA_BUNDLE, GIT_SSL_CAINFO (siblings of NODE_EXTRA_CA_CERTS; an attacker CA MITMs a session's git/npm/pip/curl traffic). * git command-execution family GIT_SSH_COMMAND, GIT_EXTERNAL_DIFF, GIT_CONFIG_GLOBAL/SYSTEM/COUNT and the numbered GIT_CONFIG_KEY_<n>/ GIT_CONFIG_VALUE_<n> pairs (matched by prefix). core/utils/git-branches.ts already scrubs these from the repo's own git invocations. * node-gyp interpreter selection NODE_GYP_FORCE_PYTHON, npm_config_python, PYTHON (run as the build Python during native-addon installs). Concurrency: the daemon's process.env scrub/restore and the loader-key rejection reporter were process-global with no guard for concurrent embedded daemons in one process (a documented supported config). The first daemon's close() restored loader vars into the shared env, re-poisoning a still-live sibling's sessions, and dropped its reporter. The scrub is now reference counted (acquireInheritedLoaderEnvScrub — snapshot on first acquire, restore only on last release) and the reporter is cleared only when still active. Test hardening from the same review: pin the daemon-worker scrub breadcrumb (not just key removal); pin the fast-path settings.env case-folded hardcoded-exclusion gate; drain the module-global fast-path stash so the accumulate assertion is order-independent. Docs updated for the new keys.
e925043 to
92c525f
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
Re-run after two more fix commits landed on top of the previously reviewed head — Template: unchanged from the last pass — the body uses a custom structure rather than the template headings ( Problem: observed, not theoretical — unchanged assessment. #8653 documented the cross-workspace loader hijack with a reproduction, #8663 shipped the denylist, and this PR addresses named, still-unresolved findings from the post-merge Direction: aligned — unchanged. Extends #8663's own two-tier model; the direction is well-established across the ecosystem (Claude Code's CHANGELOG carries the analogous subprocess-env-scrub entries). Size: 321 production lines ( Approach: the two new commits are exactly the response to the review findings they claim to be — no drive-by content. Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在之前评审的头之上又落了两个修复提交后重跑—— 模板：与上一轮相同——正文使用自定义结构而非模板标题（ 问题：已观测，非理论性——判断不变。#8653 有复现地记录了跨 workspace loader 劫持，#8663 落地拒绝列表，本 PR 处理 #8663 合入后 方向：对齐——不变。沿 #8663 自身的两层模型扩展；该方向在生态中已有充分先例（Claude Code CHANGELOG 有对应的子进程环境剥离条目）。 规模：生产代码 321 行（ 方案：两个新提交恰是其声称回应的评审发现——无夹带内容。 风险：无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run reviewed against What I verified against the tree, not taken on faith:
No blockers. Non-blocking notes, carried forward: (1) the PR body's key enumeration remains stale relative to the shipped denylist — cosmetic while settings.md is complete; (2) the same-class vectors the verify sweep flagged ( Test evidence — the PR's own CIAll checks green on the reviewed commit
Sandboxed verification: the round-2 中文说明代码审查：针对 关键论断均对照代码树核实：R4 清单七条发现（六条 Critical）全部在本头上落地——八个键在 无阻塞项。非阻塞备注延续：(1) PR 正文键枚举相对落地列表仍过时——settings.md 完整，属表面问题；(2) verify 扫出的同族向量（ 测试证据：评审提交 沙箱验证：round-2 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the review loop visibly converged: every named finding from two review rounds is answered at this head with test pins, CI is fully green on the exact commit, and the prior sandboxed A/B already proved the central claims; remaining notes are cosmetic or tracked-follow-up scope. This is the follow-up working the way the loop is supposed to. Since the last pass, two commits landed and each maps one-to-one onto a named review round: What keeps this at 4 rather than 5 is unchanged from the last pass and none of it is code: the PR body's key enumeration is stale relative to the shipped denylist (settings.md is complete, so purely cosmetic), and the same-class vectors the verify sweep flagged — Housekeeping for the humans in the loop: both standing Approval: all stages clean on this head, CI complete and green with nothing pending, and no guardrail applies (same-repo branch, maintainer author, no Stage 0 escalation), so this run approves pinned to the reviewed commit below. The fresh sandboxed verification triggered on this head will post its own report; it is confirmatory for the eight newest keys, not a precondition the code review is waiting on. 中文说明信心 4/5——评审循环可见地收敛了：两轮评审点名的每条发现都在本头上以测试钉住作答，CI 在该提交上全绿，沙箱 A/B 已在先前头上证实核心主张；其余备注均为表面问题或已跟踪的后续范围。 这是后续修复该有的运转方式。上一轮之后落了两个提交，各自与一轮点名的评审一一对应： 给 4 而不是 5 的原因与上一轮相同，且都不是代码问题：PR 正文的键枚举相对落地列表过时（settings.md 完整，纯表面）；verify 扫出的同族向量—— 给人类的杂务说明：现存两条 审批：本头上各阶段干净、CI 完成且全绿、无 pending，且无任何护栏触发（同仓分支、维护者作者、无 Stage 0 升级），故本次运行按下方评审提交钉住审批。针对本头新触发的沙箱验证会自行发布报告；它对八个最新键是确认性的，不是代码审查等待的前置条件。 — Qwen Code · qwen3.8-max Reviewed at |
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. |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31283575452 🧠 Handled by Qwen Code · model/模型 |
… surface The refcounted acquireInheritedLoaderEnvScrub read/wrote process.env from config/shared-env-keys.ts, which the serve process.env guard does not scan — moving the access out of run-qwen-serve.ts dropped its allowlisted count and failed process-env-guard.test.ts. Pass the env into the coordinator instead so run-qwen-serve.ts still owns the process.env reference (matching the existing scrub helpers), and update the allowlist to the new count.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action requiredNothing to address this round on PR #8763 (head Feedback triage
No code changes were made and no commit was created; no verification commands were required because the working tree is untouched. 中文说明Autofix 评审轮次：无需处理本轮在 PR #8763（head 反馈分类
未做任何代码改动，也未创建提交；由于工作区未被修改，无需运行任何验证命令。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status): reports the autofix agent's own failed run on a stale base ('Qwen failed during address-review: status 125') and names no code defect to trace against this diff — cannot tell
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8763 is a security-hardening follow-up to #8663: it e...: whether any close()-teardown step before restoreScrubbedLoaderEnv() (specifically cleanupLiveDiscovery() at run-qwen-serve.ts:6427/7143 or daemonLog.close(….
中文说明
未决，请确认：[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status): reports the autofix agent's own failed run on a stale base ('Qwen failed during address-review: status 125') and names no code defect to trace against this diff — cannot tell
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8763 is a security-hardening follow-up to #8663: it e...：whether any close()-teardown step before restoreScrubbedLoaderEnv() (specifically cleanupLiveDiscovery() at run-qwen-serve.ts:6427/7143 or daemonLog.close(…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'GIT_CONFIG_GLOBAL', | ||
| 'GIT_CONFIG_SYSTEM', | ||
| 'GIT_CONFIG_COUNT', |
There was a problem hiding this comment.
[Critical] The git config-injection family this tier blocks remains fully reachable through GIT_CONFIG_PARAMETERS — git honors it identically to GIT_CONFIG_COUNT and the numbered GIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n> pairs, and core/utils/git-branches.ts GIT_ENV_VARS_TO_CLEAR (the list the new comment claims to mirror) already strips it. The new test even pins the omission (shared-env-keys.test.ts asserts isHardcodedProjectEnvExclusion('GIT_CONFIG_PARAMETERS') is false). — Failure scenario: attacker repo ships .env with GIT_CONFIG_PARAMETERS="'core.hooksPath=<attacker-dir>'"; the key passes every gate, is applied to process.env, frozen into daemonRuntimeBaseEnv and distributed to every workspace's session subprocesses; any session git commit then executes attacker hooks as the daemon user. Probe-verified end-to-end on this checkout (git 2.43.0): loadEnvironment applied the value from a project .env, and git commit executed the injected hook (core.fsmonitor and url.<base>.insteadOf injection work through the same variable).
| 'GIT_CONFIG_GLOBAL', | |
| 'GIT_CONFIG_SYSTEM', | |
| 'GIT_CONFIG_COUNT', | |
| 'GIT_CONFIG_GLOBAL', | |
| 'GIT_CONFIG_SYSTEM', | |
| 'GIT_CONFIG_COUNT', | |
| 'GIT_CONFIG_PARAMETERS', |
Also flip the pinned assertion in shared-env-keys.test.ts (currently cementing the hole).
中文说明
此层阻断的 git 配置注入家族仍可通过 GIT_CONFIG_PARAMETERS 完全绕过——git 对它的处理与 GIT_CONFIG_COUNT 及编号 GIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n> 对完全相同，而且 core/utils/git-branches.ts 的 GIT_ENV_VARS_TO_CLEAR（新注释声称要对齐的列表）本来就清除了它。新增测试甚至把这一遗漏钉死了（shared-env-keys.test.ts 断言 isHardcodedProjectEnvExclusion('GIT_CONFIG_PARAMETERS') 为 false）。失败场景：攻击者仓库的 .env 写入 GIT_CONFIG_PARAMETERS="'core.hooksPath=<attacker-dir>'"；该键通过所有门控，被写入 process.env，冻结进 daemonRuntimeBaseEnv 并分发到每个 workspace 的会话子进程；任何会话执行 git commit 都会以 daemon 用户身份执行攻击者的 hook。已在本检出上端到端探针证实（git 2.43.0）：loadEnvironment 从项目 .env 应用了该值，git commit 执行了被注入的 hook（core.fsmonitor 与 url.<base>.insteadOf 注入走同一变量）。同时需翻转 shared-env-keys.test.ts 中被钉死的断言（目前它把这个洞固化了下来）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'GIT_SSH_COMMAND', | ||
| 'GIT_EXTERNAL_DIFF', |
There was a problem hiding this comment.
[Critical] GIT_SSH — the documented legacy counterpart of GIT_SSH_COMMAND, which git still executes for SSH transports — is not excluded. — Failure scenario: attacker repo .env sets GIT_SSH=<path>/evil.sh; it passes every gate, and any session git fetch/push/clone/ls-remote over SSH execs the attacker script as the daemon user. Probe-confirmed on this checkout: GIT_SSH=<canary> git ls-remote ssh://… → fatal: cannot exec '<canary>' — git attempted the exec. The sibling-state rule applies: the exec-redirect rationale for GIT_SSH_COMMAND one line away does not license leaving its legacy twin open.
| 'GIT_SSH_COMMAND', | |
| 'GIT_EXTERNAL_DIFF', | |
| 'GIT_SSH_COMMAND', | |
| 'GIT_SSH', | |
| 'GIT_EXTERNAL_DIFF', |
中文说明
GIT_SSH——GIT_SSH_COMMAND 的文档化前身，git 在 SSH 传输时仍会执行它——没有被排除。失败场景：攻击者仓库 .env 设置 GIT_SSH=<path>/evil.sh；它通过所有门控，任何会话经 SSH 的 git fetch/push/clone/ls-remote 都会以 daemon 用户身份执行攻击者脚本。已在本检出上探针证实：GIT_SSH=<canary> git ls-remote ssh://… → fatal: cannot exec '<canary>'——git 尝试了 exec。同族一致性规则适用：上一行 GIT_SSH_COMMAND 的"执行重定向"理由不能允许留下它的旧版孪生变量。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| export function clearLoaderKeyRejectionReporterIfCurrent( | ||
| reporter: LoaderKeyRejectionReporter, | ||
| ): void { |
There was a problem hiding this comment.
[Suggestion] The clear-if-current guard only fixes the FIFO close order. The reporter slot is a single process-global last-writer-wins value, so when the later-installed daemon closes first (LIFO — the common embedded teardown order), it clears a slot whose previous occupant (a still-live co-resident daemon) was already overwritten at install and is never reinstalled. — Failure scenario: embedded daemons A then B; B closes first → the slot is emptied; every later loader-key rejection from still-live daemon A falls back to writeStderrLineSafe instead of A's durable daemon log — exactly the diagnostic loss this guard exists to prevent, in the other install order. Probe-confirmed the sequence (routedTo: [] until A reinstalled). Enforcement is unaffected; diagnostics only. Concrete cost: security-relevant rejections (a workspace .env attempted to inject loader keys) vanish without a durable record under the systemd/desktop daemon launches this reporter exists for. Suggested fix: make the slot a stack/registry — push on setLoaderKeyRejectionReporter, and on clear remove only the matching entry and reactivate the new top (or fan out to all registered reporters).
中文说明
仅在"仍是当前 reporter"时清除的守卫只修复了 FIFO 关闭顺序。reporter 槽是进程全局、后写者胜的单值：当后安装的 daemon 先关闭（LIFO——内嵌场景常见的拆除顺序）时，它清空了一个其前任占用者（仍存活的同进程 daemon）早在安装时就被覆盖、且永不会被重新安装的槽。失败场景：内嵌 daemon A 然后 B；B 先关闭 → 槽被清空；仍存活的 daemon A 之后每一次 loader 键拒绝都退回到 writeStderrLineSafe 而非 A 的持久 daemon 日志——这正是该守卫要防止的诊断丢失，只是发生在另一种安装顺序下。已用探针确认该序列（在 A 重新安装前 routedTo: []）。强制拒绝不受影响；仅诊断受损。具体代价：安全相关的拒绝记录（某 workspace .env 试图注入 loader 键）在该 reporter 为之存在的 systemd/桌面 daemon 启动下不留持久痕迹。建议修复：把槽改为栈/注册表——setLoaderKeyRejectionReporter 时入栈，清除时只移除自己的条目并重新激活新的栈顶（或向所有已注册 reporter 扇出）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid finding: the clear-if-current guard only covers FIFO close order, so the LIFO teardown order (later-installed daemon closes first) still loses a co-resident survivor's reporter — diagnostics only, enforcement is unaffected, as the finding notes. This round was budget-limited to the two blocking Criticals; converting the reporter slot into a stack/registry is a mechanism redesign that deserves a dedicated change rather than a time-boxed round. Queued for the follow-up.
中文说明
延迟到后续处理。发现有效：clear-if-current 守卫只覆盖 FIFO 关闭顺序，因此 LIFO 拆除顺序（后安装的 daemon 先关闭）仍会丢失同进程幸存者的 reporter——正如发现所述，仅影响诊断，不影响强制拒绝。本轮受时间预算限制，只处理了两个阻断性 Critical；把 reporter 槽改造为栈/注册表属于机制重构，适合放在专门的变更中，而不适合在限时轮次里做。已列入后续计划。
| 'REQUESTS_CA_BUNDLE', | ||
| 'GIT_SSL_CAINFO', |
There was a problem hiding this comment.
[Suggestion] This TLS tier blocks CA-injection vars but not git's verification-disable var GIT_SSL_NO_VERIFY, which achieves the tier's stated MITM outcome without any attacker CA. (NODE_TLS_REJECT_UNAUTHORIZED is already covered above, so only the git side is open.) — Failure scenario: attacker repo .env sets GIT_SSL_NO_VERIFY=true; it passes every gate, is frozen into the daemon base env, and every session git HTTPS call skips certificate verification — MITM of token-bearing git traffic with one boolean, no attacker CA file needed. Demonstrated against a self-signed local endpoint on git 2.43.0.
| 'REQUESTS_CA_BUNDLE', | |
| 'GIT_SSL_CAINFO', | |
| 'REQUESTS_CA_BUNDLE', | |
| 'GIT_SSL_CAINFO', | |
| 'GIT_SSL_NO_VERIFY', |
中文说明
该 TLS 层阻断了 CA 注入变量，但没有阻断 git 的验证禁用变量 GIT_SSL_NO_VERIFY——无需任何攻击者 CA 即可达成该层声明的 MITM 结果。（NODE_TLS_REJECT_UNAUTHORIZED 上方已在列，因此只剩 git 一侧开放。）失败场景：攻击者仓库 .env 设置 GIT_SSL_NO_VERIFY=true；它通过所有门控，被冻结进 daemon 基础环境，每个会话的 git HTTPS 调用都跳过证书验证——一个布尔值即可 MITM 携带 token 的 git 流量，无需攻击者 CA 文件。已在 git 2.43.0 上对自签名本地端点演示。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to the immediate follow-up. Valid: GIT_SSL_NO_VERIFY reaches the TLS tier's stated MITM outcome without any attacker CA, and it belongs next to GIT_SSL_CAINFO in that tier. This round was budget-limited to the smallest blocking subset (the two Critical git-exec findings), so this one-line tier addition rides in the follow-up round, where it is the first candidate.
中文说明
延迟到紧接的下一轮。发现有效：GIT_SSL_NO_VERIFY 无需任何攻击者 CA 即可达成该 TLS 层声明的 MITM 结果，应与 GIT_SSL_CAINFO 同列于该层。本轮受时间预算限制，只处理最小阻断子集（两个 Critical 的 git 执行类发现），因此这个单行的层补充放到下一轮，且是下一轮的第一候选。
| > [!warning] | ||
| > | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `NODE_EXTRA_CA_CERTS`, or `DEV` (the dev-harness launch marker); those stay settable from the shell environment or a user-level `.env`. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `OPENSSL_CONF` (dlopens an attacker OpenSSL engine at startup), `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `DEV` (the dev-harness launch marker), the TLS trust-anchor variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, `GIT_SSL_CAINFO` — an attacker CA there would enable MITM of the token-bearing traffic a session's `git`/`npm`/`pip`/`curl` calls make), the git command-execution variables (`GIT_SSH_COMMAND`, `GIT_EXTERNAL_DIFF`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT` and the numbered `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` pairs — git runs these on any session `git` invocation), or the node-gyp interpreter-selection variables (`NODE_GYP_FORCE_PYTHON`, `npm_config_python`, `PYTHON` — run as the build Python during native-addon installs). Those stay settable from the shell environment or a user-level `.env`; unlike the loader list above they are rejected from project files only, so a value you export yourself is preserved. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. |
There was a problem hiding this comment.
[Suggestion] This sentence overstates the tier: the new keys are rejected from the top-level env section of every settings.json — user-level included — not "project files only". The .env gate is scope-aware (!envFile.isHomeScopedEnvFile && isHardcodedProjectEnvExclusion(key) in environment.ts), but all four settings.env gates (environment.ts buildRuntimeEnvironment / loadEnvironment / reloadEnvironment and the serve fast path) call isHardcodedProjectEnvExclusion on the merged user+workspace settings object with no scope check. — Failure scenario: an operator following this doc places SSL_CERT_FILE (or any new reject-tier key) in the env section of their user-level ~/.qwen/settings.json → the value is silently dropped (probe observed no warning on any path — reportRejectedLoaderKeys only covers loader keys), TLS falls back to defaults, and no diagnostic points at the gate. Suggested fix: reword to "rejected from project .env files and from the top-level settings.json env section (any scope); preserved from the shell environment and user-level .env" — or scope the settings.env gates to match the .env gate.
中文说明
这句话夸大了该层的语义：新增键会从所有 settings.json（包括用户级）的顶层 env 段被拒绝，而不仅是"仅从项目文件拒绝"。.env 门控是区分作用域的（environment.ts 中 !envFile.isHomeScopedEnvFile && isHardcodedProjectEnvExclusion(key)），但全部四个 settings.env 门控（environment.ts 的 buildRuntimeEnvironment / loadEnvironment / reloadEnvironment 与 serve 快速路径）都在合并后的 user+workspace 设置对象上调用 isHardcodedProjectEnvExclusion，没有作用域检查。失败场景：运维按照本文档把 SSL_CERT_FILE（或任何新的拒绝层键）放进用户级 ~/.qwen/settings.json 的 env 段 → 值被静默丢弃（探针观察到任何路径都没有警告——reportRejectedLoaderKeys 只覆盖 loader 键），TLS 回退到默认，且没有任何诊断指向该门控。建议修复：改写为"从项目 .env 文件与（任意作用域的）顶层 settings.json env 段拒绝；shell 环境与用户级 .env 保留"——或者让 settings.env 门控与 .env 门控的作用域一致。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred — needs a maintainer decision between your two options: (a) reword the docs to state that the new keys are rejected from the top-level env section of any-scope settings.json, or (b) make the four settings.env gates scope-aware like the .env gate. (b) changes this PR's security semantics (a user-level settings.json value would start applying), so it is not a call for a review round to make silently. Recommendation: (a) — minimal and factually accurate. Which option should we take?
中文说明
延迟处理——需要维护者在你的两个选项之间做决定：(a) 改写文档，说明新增键会从任意作用域的 settings.json 顶层 env 段被拒绝；或 (b) 让四个 settings.env 门控像 .env 门控一样区分作用域。(b) 会改变本 PR 的安全语义（用户级 settings.json 的值将开始生效），因此不适合由评审轮次悄悄决定。建议选 (a)——最小且与事实一致。请问选哪个？
| for (const key of Object.keys(env)) { | ||
| if (!isLoaderEnvKey(key)) continue; | ||
| const value = env[key]; | ||
| if (value !== undefined) sharedProcessEnvScrubOriginals.set(key, value); |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (harness validated), surviving mutant: forcing this guard's condition to true leaves every affected test green — no test pins when the guard must NOT fire. — Failure scenario: with the guard disabled, an undefined value gets snapshotted and the release path would write the literal string "undefined" into process.env, injecting a bogus loader value on restore; no test catches the guard rotting. (In practice Object.keys of a real env never yields undefined, which is why this stays green — but the defensive branch can now silently decay.) Suggested fix: add a negative case — an env object with an explicitly undefined loader key — asserting it is not snapshotted/restored; the two-cycle test proposed for the adjacent clears shares the same setup.
中文说明
测试有效性探针（harness 已验证），存活变异体：将该守卫条件强制为 true 后所有受影响测试仍为绿色——没有测试钉住守卫"不应触发"的情形。失败场景：守卫失效后，undefined 值会被快照，release 路径会把字面字符串 "undefined" 写入 process.env，在恢复时注入伪造的 loader 值；没有测试能发现守卫腐化。（实际上真实 env 的 Object.keys 从不会产生 undefined，这正是它保持绿色的原因——但该防御分支现在可以静默腐化。）建议修复：增加负例——一个显式包含 undefined loader 键的 env 对象——断言它不被快照/恢复；为相邻 clear 建议的双周期测试可共用同一 setup。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid surviving mutant: no test pins when the undefined-guard must not fire. The negative case (an explicitly undefined loader key is neither snapshotted nor restored) shares its setup with the two-cycle acquire/release test and will land with it.
中文说明
延迟到下一轮。存活变异体属实：没有测试钉住 undefined 守卫"不应触发"的情形。负例（显式为 undefined 的 loader 键既不被快照也不被恢复）与双周期 acquire/release 测试共用 setup，将与其一起合入。
| if (!Object.hasOwn(env, key)) env[key] = value; | ||
| } | ||
| sharedProcessEnvScrubOriginals.clear(); |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (harness validated), surviving mutant: deleting the release-time sharedProcessEnvScrubOriginals.clear(); leaves every affected test green — same masking chain as the acquire-time clear: the snapshot map keeps stale entries until the next depth-0 acquire clears it, and a regression that also loses the acquire-time clear restores stale loader values into process.env with no failing test. Suggested fix: the same two-cycle acquire/release test (first cycle snapshots a loader key the second cycle's env lacks; assert no stale restore).
中文说明
测试有效性探针（harness 已验证），存活变异体：删除 release 时的 sharedProcessEnvScrubOriginals.clear(); 后所有受影响测试仍为绿色——与 acquire 时 clear 相同的掩盖链：快照 map 会保留陈旧条目直到下一次 depth-0 acquire 清空；若回归同时丢失 acquire 时的 clear，陈旧 loader 值会被恢复进 process.env 而没有任何测试失败。建议修复：同一个双周期 acquire/release 测试（第一周期快照一个第二周期环境中不存在的 loader 键；断言无陈旧恢复）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid surviving mutant with the same masking chain as the acquire-time clear; the shared two-cycle acquire/release test covers both and will land in the follow-up round.
中文说明
延迟到下一轮。存活变异体属实，掩盖链与 acquire 时的 clear 相同；共用的双周期 acquire/release 测试同时覆盖两者，将在下一轮合入。
| sharedProcessEnvScrubDepth = 0; | ||
| sharedProcessEnvScrubOriginals.clear(); |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (harness validated), surviving mutant: deleting the test-only reset's sharedProcessEnvScrubOriginals.clear(); leaves every affected test green — tests calling the reset rely on the depth-0 acquire's own clear to empty the map. — Failure scenario: any test later written to depend on the reset truly emptying the snapshot would pass against a broken reset until the masking changes. Suggested fix: assert observable post-reset state instead (e.g. a snapshot taken before reset is not restored after it).
中文说明
测试有效性探针（harness 已验证），存活变异体：删除仅测试用 reset 中的 sharedProcessEnvScrubOriginals.clear(); 后所有受影响测试仍为绿色——调用 reset 的测试依赖 depth-0 acquire 自身的 clear 来清空 map。失败场景：未来任何依赖 reset 真正清空快照的测试，在掩盖关系改变之前都会对着一个坏掉的 reset 通过。建议修复：改为断言 reset 后的可观察状态（例如 reset 前取得的快照在 reset 后不被恢复）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid surviving mutant: tests calling the reset rely on the depth-0 acquire's own clear. Asserting observable post-reset state (a snapshot taken before reset is not restored after it) will be added together with the two-cycle scrub test work.
中文说明
延迟到下一轮。存活变异体属实：调用 reset 的测试依赖 depth-0 acquire 自身的 clear。将与双周期剥离测试一起，增加对 reset 后可观察状态的断言（reset 前取得的快照在 reset 后不被恢复）。
| import { | ||
| scrubAndReportInheritedLoaderEnv, | ||
| acquireInheritedLoaderEnvScrub, | ||
| clearLoaderKeyRejectionReporterIfCurrent, |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (harness validated), hunk-survived: reverting this import hunk — i.e. switching the serve entrypoint back from the refcounted acquireInheritedLoaderEnvScrub to the one-shot scrub — leaves every test in this diff green. The gating tests live in shared-env-keys.test.ts against the helper itself, not its adoption here. — Failure scenario: a future change that reverts the daemon serve path to the one-shot scrub undoes the concurrent-embedded-daemon fix; the first daemon's close() again restores loader vars into a still-live sibling's sessions — with no failing test. Suggested fix: add a serve-surface test with two overlapping embedded runQwenServe instances asserting the first close does not restore loader vars into the survivor's env.
中文说明
测试有效性探针（harness 已验证），hunk 存活：单独回退此 import hunk——即把 serve 入口从引用计数的 acquireInheritedLoaderEnvScrub 换回一次性剥离——本 diff 中所有测试仍为绿色。钉住行为的测试位于 shared-env-keys.test.ts，针对的是辅助函数本身，而非此处的采用。失败场景：未来把 daemon serve 路径回退到一次性剥离的改动会撤销并发内嵌 daemon 修复；第一个 daemon 的 close() 会再次把 loader 变量恢复进仍存活同伴的会话——且没有任何测试失败。建议修复：增加 serve 层测试，两个重叠的内嵌 runQwenServe 实例，断言第一个 close 不会把 loader 变量恢复进幸存者的环境。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid hunk-survived probe: reverting the adoption of the refcounted scrub ships green. A serve-surface test with two overlapping embedded runQwenServe instances is non-trivial harness work and is deferred under this round's budget limit; it is queued together with the reporter startup-failure test.
中文说明
延迟到下一轮。hunk 存活探针有效：回退对引用计数剥离的采用仍能绿色合入。两个重叠内嵌 runQwenServe 实例的 serve 层测试是不简单的 harness 工作，在本轮预算限制下延迟；已与 reporter 启动失败测试一起排队。
| // Called with the loader-key rejection reporter this run installed, so the | ||
| // startup-failure catch can clear it only when it is still the active one — | ||
| // a co-resident daemon that installed after us must keep its own reporter. | ||
| reporterInstalled(reporter: LoaderKeyRejectionReporter): void; |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (harness validated), hunk-survived: reverting the reporterInstalled lifecycle-callback hunk leaves every test green. — Failure scenario: if a refactor drops this callback and the startup-failure catch falls back to unconditionally clearing the reporter (setLoaderKeyRejectionReporter(undefined)), the first daemon's failed startup again drops a co-resident daemon's reporter — silently routing its loader-key rejections to the stderr fallback, the exact race this hunk fixes — with no failing test. Suggested fix: add a test where daemon A installs a reporter, daemon B installs after, and A's startup-failure path runs; assert B's reporter is still the active one.
中文说明
测试有效性探针（harness 已验证），hunk 存活：单独回退 reporterInstalled 生命周期回调 hunk 后所有测试仍为绿色。失败场景：若重构删除该回调、启动失败 catch 退回无条件清除 reporter（setLoaderKeyRejectionReporter(undefined)），第一个 daemon 的启动失败会再次丢掉同进程 daemon 的 reporter——其 loader 键拒绝被静默路由到 stderr 回退，正是该 hunk 修复的竞态——且没有任何测试失败。建议修复：增加测试，daemon A 安装 reporter，daemon B 随后安装，然后走 A 的启动失败路径；断言 B 的 reporter 仍是当前活跃者。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Deferred to a follow-up. Valid hunk-survived probe: dropping the reporterInstalled lifecycle callback would let a failed startup clear a co-resident daemon's reporter, with no failing test. The A-installs / B-installs / A-fails reporter test will be added together with the overlapping-daemon serve-surface test.
中文说明
延迟到下一轮。hunk 存活探针有效：删除 reporterInstalled 生命周期回调后，启动失败会清掉同进程 daemon 的 reporter，且没有任何测试失败。A 安装 / B 安装 / A 失败的 reporter 测试将与重叠 daemon 的 serve 层测试一起添加。
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31290546316 🧠 Handled by Qwen Code · model/模型 |
…denylist Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8763 (round after budget timeout)Scope decisionThis round carried a budget warning: the previous round exhausted its time budget without finishing anything. Per the workflow rules, this round addressed only the smallest blocking subset — the two Critical findings — committed as soon as complete, and deferred the twelve Suggestions with per-thread replies ( Feedback dispositionsAddressed (2 Criticals, one commit:
|
| Finding | Disposition | Change |
|---|---|---|
[Critical] GIT_CONFIG_PARAMETERS left the git config-injection family reachable (inline 3742328269) |
Fixed — verified: core/utils/git-branches.ts GIT_ENV_VARS_TO_CLEAR (the list this PR's comment claims to mirror) already strips it, and the test at shared-env-keys.test.ts:166 pinned the omission |
Added 'GIT_CONFIG_PARAMETERS' to PROJECT_ENV_HARDCODED_EXCLUSIONS; flipped the pinned assertion (the prefix-overmatch negative case now uses GIT_CONFIG_NOSYSTEM, which only skips the system gitconfig read and injects nothing); extended the case-insensitive coverage; extended the end-to-end loadEnvironment rejection test (environment.test.ts); updated the docs enumeration |
[Critical] GIT_SSH (legacy counterpart of the blocked GIT_SSH_COMMAND) not excluded (inline 3742328276) |
Fixed — verified: git still execs GIT_SSH for SSH transports, and the denylist is centralized in isHardcodedProjectEnvExclusion, which all six application gates (4 in environment.ts, 2 in fast-path-settings.ts) route through |
Added 'GIT_SSH' next to 'GIT_SSH_COMMAND'; same test/doc updates as above |
Both keys stay in the reject-from-project-files tier (not the scrubbed loader set), consistent with their siblings: shell-exported and user-level .env values are preserved.
Deferred with replies (12 Suggestions — see comment-replies.json)
- 3742328281
GIT_SSL_NO_VERIFYTLS-tier addition — valid; first candidate for the follow-up round. - 3742328283 docs overstate the settings.env tier scope — needs a maintainer decision between rewording the docs and making the settings.env gates scope-aware (option (b) changes the PR's security semantics); recommendation recorded in the reply.
- 3742328285
npm_config_gitmissing from the docs enumeration — valid; lands with the follow-up. - 3742328278 reporter slot LIFO-order diagnostic loss — valid; stack/registry redesign deferred to a dedicated change.
- 3742328286 no diagnostics for hardcoded-tier rejections — valid; warn-once machinery mirror deferred to a dedicated change.
- 3742328289, 3742328293, 3742328295, 3742328297, 3742328299, 3742328302, 3742328305 test-efficacy probes (surviving mutants / hunk-survived reverts) — all valid; the proposed tests are grouped into the follow-up (one two-cycle scrub test covers three of the mutants).
Review-level notes
- The
CHANGES_REQUESTEDstate was driven by issue-level comment 5228685910, which reports the autofix agent's own failed run on a stale base (status 125) and names no code defect — there is nothing to trace in this diff; the substantive unresolved findings were the two Criticals above, now fixed. - The reviewer's "not reviewed" note (Integration Tests (CLI, No Sandbox) skipped in CI) is acknowledged: the changed behavior is exercised at the
loadEnvironment/fast-path unit surface, which the focused suites cover end-to-end, including the exact attack scenario (project.env→ rejected). - The reviewer's budget-cut exploration (close()-teardown ordering before
restoreScrubbedLoaderEnv()) was left open by the reviewer itself and is not a finding; the scrub-lifecycle questions it raises are covered by the deferred test-efficacy replies above.
Conflict notes
--conflict false: no merge of origin/main performed; none needed.
Verification
Commands actually run this round (repo root unless noted):
npm run build— passed (also produced the missingpackages/channels/base/distentry that madefast-path.test.tsuncollectable before the build — a stale-checkout artifact, not related to this change)npm run typecheck— passednpm run lint— passednpx vitest run src/config/shared-env-keys.test.ts src/config/environment.test.ts src/serve/fast-path.test.ts(inpackages/cli) — 141 passed (3 files)npx vitest run src/commands/channel/daemon-worker.test.ts src/serve/process-env-guard.test.ts(inpackages/cli) — 81 passed (2 files)npx prettier --checkon the four touched files — passed
Integration tests after npm run bundle were not run: the touched behavior (project .env/settings.env rejection) is fully exercised by the unit surfaces above, not only through the bundled CLI. npm run generate:settings-schema was not needed: no settings source changed.
中文说明
处理评审总结 — PR #8763（预算超时后的本轮）
范围决定
本轮带有预算警告：上一轮耗尽了时间预算且没有完成任何内容。按照工作流规则，本轮只处理最小阻断子集——两条 Critical 发现——完成后立即提交，并将十二条 Suggestion 以逐线程回复（comment-replies.json）的方式延迟处理，确保没有任何发现被悄悄丢弃。
反馈处置
已处理（2 条 Critical，单次提交：46e2690465）
| 发现 | 处置 | 变更 |
|---|---|---|
[Critical] GIT_CONFIG_PARAMETERS 使 git 配置注入家族仍然可达（inline 3742328269） |
已修复——已核实：core/utils/git-branches.ts 的 GIT_ENV_VARS_TO_CLEAR（本 PR 注释声称要对齐的列表）本来就清除了它，而 shared-env-keys.test.ts:166 的测试把这一遗漏钉死了 |
在 PROJECT_ENV_HARDCODED_EXCLUSIONS 中新增 'GIT_CONFIG_PARAMETERS'；翻转被钉死的断言（前缀过度匹配的负例改用 GIT_CONFIG_NOSYSTEM——它只跳过系统 gitconfig 读取，不能注入任何配置）；补充大小写不敏感覆盖；扩展端到端 loadEnvironment 拒绝测试（environment.test.ts）；更新文档枚举 |
[Critical] GIT_SSH（已被阻断的 GIT_SSH_COMMAND 的旧版对应变量）未被排除（inline 3742328276） |
已修复——已核实：git 在 SSH 传输时仍会执行 GIT_SSH，且拒绝名单集中在 isHardcodedProjectEnvExclusion，全部六个应用门控（environment.ts 中 4 处、fast-path-settings.ts 中 2 处）都经由它 |
在 'GIT_SSH_COMMAND' 旁新增 'GIT_SSH'；测试/文档更新同上 |
两个键都保留在"仅从项目文件拒绝"层（不进入被剥离的 loader 集合），与其同族键一致：shell 导出值与用户级 .env 值仍然保留。
以回复延迟处理（12 条 Suggestion——见 comment-replies.json）
- 3742328281 TLS 层补充
GIT_SSL_NO_VERIFY——有效；下一轮的第一候选。 - 3742328283 文档夸大了 settings.env 层的作用域——需要维护者在"改写文档"与"让 settings.env 门控区分作用域"之间做决定（选项 (b) 会改变本 PR 的安全语义）；回复中已记录建议。
- 3742328285 文档枚举缺少
npm_config_git——有效；随下一轮一起合入。 - 3742328278 reporter 槽在 LIFO 顺序下的诊断丢失——有效；栈/注册表改造延迟到专门的变更。
- 3742328286 硬编码层拒绝没有任何诊断——有效；复刻 warn-once 机制延迟到专门的变更。
- 3742328289、3742328293、3742328295、3742328297、3742328299、3742328302、3742328305 测试有效性探针（存活变异体 / hunk 存活回退）——全部有效；所提议的测试归入下一轮（其中一个双周期剥离测试可覆盖其中三个变异体）。
评审层面的说明
CHANGES_REQUESTED状态源自 issue 级评论 5228685910，该评论报告的是 autofix 代理自身在陈旧 base 上的失败运行（status 125），没有指出任何代码缺陷——本 diff 中无可追溯的内容；实质性的未决发现就是上面两条 Critical，现已修复。- 评审者的"未审查"说明（CI 中跳过了 Integration Tests (CLI, No Sandbox)）已知悉：本次变更的行为在
loadEnvironment/fast-path 单元层面被完整覆盖，聚焦套件已端到端覆盖，包括确切的攻击场景（项目.env→ 被拒绝）。 - 评审者因预算用尽而未完成的探索（
restoreScrubbedLoaderEnv()之前的 close() 拆除顺序）由评审者自己留作未决，不构成发现；其引发的剥离生命周期问题已由上面延迟的测试有效性回复覆盖。
冲突说明
--conflict false：未合并 origin/main，也无需合并。
验证
本轮实际执行的命令（未注明者均在仓库根目录）：
npm run build—— 通过（同时生成了此前缺失的packages/channels/base/dist入口文件——该缺失曾导致fast-path.test.ts无法收集，属于检出环境的陈旧产物，与本变更无关）npm run typecheck—— 通过npm run lint—— 通过npx vitest run src/config/shared-env-keys.test.ts src/config/environment.test.ts src/serve/fast-path.test.ts（在packages/cli内）—— 141 通过（3 个文件）npx vitest run src/commands/channel/daemon-worker.test.ts src/serve/process-env-guard.test.ts（在packages/cli内）—— 81 通过（2 个文件）- 对四个被改动文件执行
npx prettier --check—— 通过
未运行 npm run bundle 之后的集成测试：本次变更的行为（项目 .env/settings.env 拒绝）已由上述单元层面完整覆盖，并非只能经由打包后的 CLI 验证。未运行 npm run generate:settings-schema：未改动任何 settings 源。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'GIT_SSH_COMMAND', | ||
| 'GIT_SSH', |
There was a problem hiding this comment.
[Critical] The git command-execution family this tier blocks leaves sibling variables that git also executes — GIT_EXEC_PATH, GIT_TEMPLATE_DIR, and more conditionally GIT_ASKPASS / GIT_PROXY_COMMAND — absent from both denylist tiers, so they pass every gate from a project .env. Probe-confirmed at this commit: the predicates return false for all of them, loadEnvironment applies them from a project .env while the comparator GIT_SSH_COMMAND is rejected, and adding one line to PROJECT_ENV_HARDCODED_EXCLUSIONS flips the probe. — Failure scenario: untrusted repo .env sets GIT_EXEC_PATH=<repo>/.evil (with an executable git-remote-https) or GIT_TEMPLATE_DIR=<repo>/.tpl (with hooks/post-checkout); the value is frozen into daemonRuntimeBaseEnv, reaches every workspace's session subprocesses, and a session git fetch/git clone over HTTPS executes attacker code as the daemon user — verified against real git on this checkout (GIT_EXEC_PATH ran a canary remote helper; GIT_TEMPLATE_DIR planted the hook). This is the same cross-workspace exec outcome GIT_SSH_COMMAND is blocked for, and the same sibling shape as the round-1 GIT_SSH / GIT_CONFIG_PARAMETERS Criticals fixed at this commit. Evaluate GIT_EDITOR on the same terms.
| 'GIT_SSH_COMMAND', | |
| 'GIT_SSH', | |
| 'GIT_SSH_COMMAND', | |
| 'GIT_SSH', | |
| 'GIT_EXEC_PATH', | |
| 'GIT_TEMPLATE_DIR', | |
| 'GIT_ASKPASS', | |
| 'GIT_PROXY_COMMAND', |
中文说明
此层阻断的 git 命令执行家族遗漏了 git 同样会执行的同族变量——GIT_EXEC_PATH、GIT_TEMPLATE_DIR，以及条件稍多的 GIT_ASKPASS / GIT_PROXY_COMMAND——它们不在任何一层拒绝名单中，可以经由项目 .env 通过所有门控。已在本提交上探针证实：谓词对它们全部返回 false；loadEnvironment 从项目 .env 应用了它们，而对照组 GIT_SSH_COMMAND 被拒绝；向 PROJECT_ENV_HARDCODED_EXCLUSIONS 增加一行即可翻转探针。失败场景：不受信仓库的 .env 设置 GIT_EXEC_PATH=<repo>/.evil（内含可执行的 git-remote-https）或 GIT_TEMPLATE_DIR=<repo>/.tpl（内含 hooks/post-checkout）；该值被冻结进 daemonRuntimeBaseEnv，到达每个 workspace 的会话子进程，任何会话经 HTTPS 的 git fetch/git clone 都会以 daemon 用户身份执行攻击者代码——已在本检出的真实 git 上验证（GIT_EXEC_PATH 执行了金丝雀 remote helper；GIT_TEMPLATE_DIR 植入了 hook）。与 GIT_SSH_COMMAND 被阻断的跨 workspace 执行后果相同，也与本轮已在该提交修复的 GIT_SSH / GIT_CONFIG_PARAMETERS 两条 Critical 属于同族同型。请以同样标准评估 GIT_EDITOR。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'SSL_CERT_FILE', | ||
| 'SSL_CERT_DIR', | ||
| 'CURL_CA_BUNDLE', | ||
| 'REQUESTS_CA_BUNDLE', | ||
| 'GIT_SSL_CAINFO', |
There was a problem hiding this comment.
[Suggestion] The TLS trust-anchor tier states its goal as stopping MITM of the token-bearing git/npm/pip/curl traffic, but the knobs npm and pip actually honor — npm_config_cafile, npm_config_ca, npm_config_strict_ssl, PIP_CERT, and git's directory twin GIT_SSL_CAPATH — pass every gate from a project .env (probe-confirmed at this commit). (GIT_SSL_NO_VERIFY is the same class but was already discussed and deferred — deliberately not re-reported here.) — Failure scenario: untrusted repo .env sets npm_config_cafile=<repo>/evil-ca.pem or PIP_CERT=<repo>/evil-ca.pem, or simply npm_config_strict_ssl=false (which needs no attacker CA at all); a session npm install / pip install then trusts the attacker CA — MITM of registry traffic (token theft, malicious package delivery). Consider PIP_INDEX_URL too (index redirection bypasses TLS entirely).
| 'SSL_CERT_FILE', | |
| 'SSL_CERT_DIR', | |
| 'CURL_CA_BUNDLE', | |
| 'REQUESTS_CA_BUNDLE', | |
| 'GIT_SSL_CAINFO', | |
| 'SSL_CERT_FILE', | |
| 'SSL_CERT_DIR', | |
| 'CURL_CA_BUNDLE', | |
| 'REQUESTS_CA_BUNDLE', | |
| 'GIT_SSL_CAINFO', | |
| 'GIT_SSL_CAPATH', | |
| 'npm_config_cafile', | |
| 'npm_config_ca', | |
| 'npm_config_strict_ssl', | |
| 'PIP_CERT', |
中文说明
TLS 信任锚层的既定目标是阻断对携带 token 的 git/npm/pip/curl 流量的 MITM，但 npm 与 pip 实际读取的旋钮——npm_config_cafile、npm_config_ca、npm_config_strict_ssl、PIP_CERT，以及 git 的目录孪生变量 GIT_SSL_CAPATH——都可以从项目 .env 通过所有门控（已在本提交探针证实）。（GIT_SSL_NO_VERIFY 同属此类，但已在早前讨论并延期处理——此处有意不再重复报告。）失败场景：不受信仓库的 .env 写入 npm_config_cafile=<repo>/evil-ca.pem 或 PIP_CERT=<repo>/evil-ca.pem，甚至只需 npm_config_strict_ssl=false（完全不需要攻击者 CA）；会话执行 npm install / pip install 时即信任攻击者 CA——注册表流量被 MITM（窃取 token、投递恶意包）。另请考虑 PIP_INDEX_URL（索引重定向完全绕过 TLS）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| > [!warning] | ||
| > | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `NODE_EXTRA_CA_CERTS`, or `DEV` (the dev-harness launch marker); those stay settable from the shell environment or a user-level `.env`. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `OPENSSL_CONF` (dlopens an attacker OpenSSL engine at startup), `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `DEV` (the dev-harness launch marker), the TLS trust-anchor variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, `GIT_SSL_CAINFO` — an attacker CA there would enable MITM of the token-bearing traffic a session's `git`/`npm`/`pip`/`curl` calls make), the git command-execution variables (`GIT_SSH_COMMAND`, `GIT_SSH`, `GIT_EXTERNAL_DIFF`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT`, `GIT_CONFIG_PARAMETERS` and the numbered `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` pairs — git runs these on any session `git` invocation), or the node-gyp interpreter-selection variables (`NODE_GYP_FORCE_PYTHON`, `npm_config_python`, `PYTHON` — run as the build Python during native-addon installs). Those stay settable from the shell environment or a user-level `.env`; unlike the loader list above they are rejected from project files only, so a value you export yourself is preserved. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. |
There was a problem hiding this comment.
[Suggestion] This diff updates the settings.md enumeration for the four new loader keys (OPENSSL_CONF, NODE_REPL_EXTERNAL_MODULE, npm_config_node_gyp, npm_config_init_module), but the sibling enumeration in docs/users/qwen-serve.md:559 ("Loader-affecting variables (NODE_OPTIONS, … BASH_FUNC_*) are likewise never passed to session subprocesses") was not updated — the two docs that enumerate the same denylist now disagree. — Failure scenario: an operator auditing the qwen serve security model reads qwen-serve.md and concludes an inherited OPENSSL_CONF / NODE_REPL_EXTERNAL_MODULE / npm_config_node_gyp / npm_config_init_module reaches daemon session subprocesses, or cannot find the key while debugging why their exported value has no effect in a daemon session. — Suggested fix: add the four keys to the qwen-serve.md enumeration (matching how this PR handled settings.md).
中文说明
本 diff 更新了 settings.md 中四个新 loader 键（OPENSSL_CONF、NODE_REPL_EXTERNAL_MODULE、npm_config_node_gyp、npm_config_init_module）的枚举，但 docs/users/qwen-serve.md:559 的同源枚举（"Loader-affecting variables (NODE_OPTIONS, … BASH_FUNC_*) are likewise never passed to session subprocesses"）未同步更新——两份枚举同一拒绝名单的文档现已不一致。失败场景：审计 qwen serve 安全模型的运维者阅读 qwen-serve.md 时，会误以为继承的 OPENSSL_CONF / NODE_REPL_EXTERNAL_MODULE / npm_config_node_gyp / npm_config_init_module 仍会传入 daemon 会话子进程，或在排查自己导出的值为何在 daemon 会话中无效时找不到该键。修复建议：在 qwen-serve.md 的枚举中补上这四个键（与本 PR 处理 settings.md 的方式一致）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const HARDCODED_PROJECT_ENV_EXCLUSION_PREFIXES = [ | ||
| 'git_config_key_', | ||
| 'git_config_value_', | ||
| ] as const; |
There was a problem hiding this comment.
[Suggestion] The prefix-matched keys are hardcoded exclusions but not members of RELOAD_EXCLUDED_KEYS, whose gate (isReloadExcludedKey) is literal-set membership — unlike their literal sibling GIT_CONFIG_COUNT, which is reload-excluded via the PROJECT_ENV_HARDCODED_EXCLUSIONS spread. Probe-confirmed: on a mid-session reload from a home-scoped ~/.env, GIT_CONFIG_COUNT stayed frozen at the boot value while GIT_CONFIG_KEY_<n>/VALUE_<n> from the same file rotated freely — one half of a single mechanism frozen, the other half live. The home file is trusted, so this is the asymmetric guard, not an exploitable hole. — Failure scenario: a user editing GIT_CONFIG_COUNT and the numbered pairs together in ~/.env gets a half-applied git config after a settings reload. — Suggested fix: give the reload gate the same prefix handling:
return (
RELOAD_EXCLUDED_KEYS_CASEFOLDED.has(lowerKey) ||
HARDCODED_PROJECT_ENV_EXCLUSION_PREFIXES.some((p) => lowerKey.startsWith(p))
);
中文说明
前缀匹配的键是硬编码排除项，但不是 RELOAD_EXCLUDED_KEYS 的成员——该重载门控（isReloadExcludedKey）只做字面量集合匹配——与其同族的字面量兄弟 GIT_CONFIG_COUNT（经由 PROJECT_ENV_HARDCODED_EXCLUSIONS 展开而成为重载排除项）不一致。探针证实：来自 home 作用域 ~/.env 的会话中重载时，GIT_CONFIG_COUNT 保持启动时的冻结值，而同一文件中的 GIT_CONFIG_KEY_<n>/VALUE_<n> 自由轮换——同一机制的一半被冻结、另一半却生效。home 文件受信任，因此这是门控不对称，而非可利用漏洞。失败场景：用户在 ~/.env 中同时修改 GIT_CONFIG_COUNT 与编号对，settings 重载后会得到半应用的 git 配置。修复建议：让重载门控具备相同的前缀处理（见代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'NODE_GYP_FORCE_PYTHON', | ||
| 'npm_config_python', | ||
| 'PYTHON', |
There was a problem hiding this comment.
[Suggestion] The new keys join RELOAD_EXCLUDED_KEYS via the PROJECT_ENV_HARDCODED_EXCLUSIONS spread (environment.ts), and reloadEnvironment's deletion loop skips reload-excluded keys — so deleting one of these keys from a home-scoped .env (exempt from rejection, applied at boot) leaves the stale value in process.env until process restart. Probe-confirmed: post-PR, removing SSL_CERT_FILE from ~/.env no longer propagates on reload while an ordinary key's removal does; pre-PR the removal propagated. — Failure scenario: operator sets SSL_CERT_FILE=/corp/ca.pem in ~/.env, later removes it for CA rotation; every session subprocess keeps inheriting the stale cert path with no diagnostic. (The same sharp edge pre-exists for NODE_EXTRA_CA_CERTS; this PR extends it to ~14 additional keys.) — Suggested fix: propagate deletions for keys whose provenance is a home-scoped .env (provenance is already tracked), or document the freeze-at-boot semantics for removals in the upgrade note.
中文说明
新键经由 PROJECT_ENV_HARDCODED_EXCLUSIONS 展开（environment.ts）进入 RELOAD_EXCLUDED_KEYS，而 reloadEnvironment 的删除循环会跳过重载排除键——因此从 home 作用域 .env（该层豁免拒绝、启动时已应用）中删除这些键后，陈旧值会一直留在 process.env 中直到进程重启。探针证实：PR 之后从 ~/.env 删除 SSL_CERT_FILE 不再随重载传播，而普通键的删除正常传播；PR 之前该删除是传播的。失败场景：运维者在 ~/.env 设置 SSL_CERT_FILE=/corp/ca.pem，后因 CA 轮换将其删除；所有会话子进程继续继承陈旧的证书路径，且无任何诊断。（同样的尖锐边在 NODE_EXTRA_CA_CERTS 上早已存在；本 PR 将其扩展到约 14 个新键。）修复建议：对来源为 home 作用域 .env 的键传播删除（来源已有跟踪），或在升级说明中注明删除同样冻结。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const loaderEnvScrub = acquireInheritedLoaderEnvScrub( | ||
| process.env, | ||
| 'qwen serve', | ||
| 'daemon', | ||
| ); | ||
| const scrubbedLoaderEnvKeys = loaderEnvScrub.removedKeys; |
There was a problem hiding this comment.
[Suggestion] In the co-resident multi-daemon configuration this PR introduces, the daemon performing the final refcount release can have empty removedKeys — and both the stderr breadcrumb (scrubAndReportInheritedLoaderEnv skips on empty removals) and the durable-log entry (if (scrubbedLoaderEnvKeys.length > 0) below) are gated on removal count, while the final restore itself runs after this daemon's own daemonLog is already closed and is logged nowhere. — Failure scenario: operator exports NODE_OPTIONS; daemon A boots (scrubs, logs), co-resident daemon B boots (already-scrubbed env, removes nothing, logs nothing); A closes (no restore while B holds); B closes and the last release writes the loader vars back into the shared process.env. Investigating where/when the vars reappeared, the operator finds no scrub and no restore entry in B's durable log — the audit trail exists only in A's log. Diagnostics only; reject/restore behavior itself is correct and test-pinned. — Suggested fix: log the coordination state durably when a handle joins an already-active scrub, and surface the depth-0 restore through the callback channel before daemonLog.close().
中文说明
在本 PR 引入的同进程多 daemon 并发配置下，执行最后一次引用计数释放的 daemon 可能拥有空的 removedKeys——而 stderr 面包屑（scrubAndReportInheritedLoaderEnv 在空删除时跳过）与持久日志条目（下方 if (scrubbedLoaderEnvKeys.length > 0)）都以删除数量为非空门控，同时最终的恢复本身发生在本 daemon 的 daemonLog 关闭之后，没有任何日志记录。失败场景：运维者导出 NODE_OPTIONS；daemon A 启动（剥离并记录），同进程 daemon B 启动（环境已剥离、无键可删、无日志）；A 关闭（B 仍持有时不恢复）；B 关闭时最后一次释放把 loader 变量写回共享 process.env。运维者排查这些变量何时何地重新出现时，在 B 的持久日志中既找不到剥离条目也找不到恢复条目——审计线索只存在于 A 的日志中。仅影响诊断；拒绝/恢复行为本身正确且有测试钉住。修复建议：当 handle 加入已激活的剥离时，将协调状态持久记录；并在 daemonLog.close() 之前通过回调通道上报 depth-0 恢复。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Declined this round (recorded, not dropped): the finding is diagnostics-only — reject/restore behavior itself is correct and test-pinned, as the finding notes. Durably logging the coordination state when a handle joins an already-active scrub, and surfacing the depth-0 restore through the callback channel before daemonLog.close(), is a non-trivial rework of the serve daemon lifecycle (new callback surface plus durable-log ordering), which is disproportionate diff growth for a diagnostics gap in this budget-constrained round. The audit-trail scenario stays traceable through the first daemon's log. Happy to take this as a follow-up if a maintainer wants it sooner.
中文说明
本轮已记录并拒绝（非静默丢弃）：该发现仅影响诊断——正如发现本身所述，拒绝/恢复行为是正确的，且有测试钉住。在 handle 加入已激活的剥离时持久记录协调状态、并在 daemonLog.close() 之前通过回调通道上报 depth-0 恢复，需要对 serve daemon 生命周期做不小的改造（新增回调面 + 持久日志排序），对于预算受限轮次中的一个诊断缺口而言属于不成比例的 diff 增长。该审计线索场景仍可通过第一个 daemon 的日志追溯。若维护者希望更早解决，愿意作为后续跟进处理。
| // The non-Node TLS trust-anchor vars reach the SAME MITM outcome for the | ||
| // curl/git/openssl/python tools a session subprocess routinely shells out |
There was a problem hiding this comment.
[Suggestion] The rc-file redirect knobs CURL_HOME and WGETRC are on neither tier, yet they reach the exact MITM outcome this comment names — for curl, one of the four tools it lists. Probe-confirmed end-to-end at this commit: both keys pass every gate and are applied from a project .env, and real curl/wget routed the session request through an attacker-designated proxy via $CURL_HOME/.curlrc (proxy = …) and WGETRC directives (http_proxy = …), with control arms failing closed. This is the config-file-redirect class the PR actively blocks in every other tier (npm_config_userconfig, GIT_CONFIG_GLOBAL, OPENSSL_CONF), not the declared search-path deferral. — Failure scenario: untrusted workspace .env sets CURL_HOME=<dir> with a .curlrc containing proxy = https://attacker:3128, cacert = <attacker CA>, or insecure; a session's token-bearing curl call is proxied/intercepted. WGETRC is the wget twin (https_proxy/check_certificate=off). — Suggested fix: add 'CURL_HOME' and 'WGETRC' beside the TLS anchors (the verifier applied this flip and it closed the gate on every surface).
中文说明
rc 文件重定向旋钮 CURL_HOME 与 WGETRC 不在任何一层拒绝名单中，却能达成本注释所列的确切 MITM 后果——而且针对的正是注释点名的四个工具之一 curl。已在本提交端到端探针证实：两个键通过所有门控并从项目 .env 被应用；真实 curl/wget 分别经由 $CURL_HOME/.curlrc（proxy = …）与 WGETRC 指令（http_proxy = …）把会话请求路由到攻击者指定的代理，对照组则按预期失败。这属于本 PR 在其他各层主动阻断的"配置文件重定向"类（npm_config_userconfig、GIT_CONFIG_GLOBAL、OPENSSL_CONF），而不属于已声明延期的搜索路径类。失败场景：不受信 workspace 的 .env 设置 CURL_HOME=<dir>，其 .curlrc 含 proxy = https://attacker:3128、cacert = <attacker CA> 或 insecure；会话携带 token 的 curl 调用即被代理/拦截。WGETRC 是 wget 的孪生入口（https_proxy/check_certificate=off）。修复建议：在 TLS 信任锚旁新增 'CURL_HOME' 与 'WGETRC'（验证者应用该翻转后，所有应用面的门控均已关闭）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 4/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31307298125 🧠 Handled by Qwen Code · model/模型 |
…c-file tiers Close the round-2 review findings: block the remaining git command-execution siblings (GIT_EXEC_PATH, GIT_TEMPLATE_DIR, GIT_ASKPASS, GIT_PROXY_COMMAND, GIT_EDITOR), the npm/pip TLS trust knobs (npm_config_cafile, npm_config_ca, npm_config_strict_ssl, PIP_CERT, GIT_SSL_CAPATH), and the curl/wget rc-file redirects (CURL_HOME, WGETRC) from project .env files. Freeze the numbered GIT_CONFIG_KEY_/VALUE_ pairs on reload together with GIT_CONFIG_COUNT, and sync the qwen-serve.md loader-key enumeration with settings.md.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Round response to round-2 review feedback (PR #8763)Commit: Note: this round ran under a budget warning from the previous round, so the work focused on the blocking subset plus the cheap in-scope fixes; the one decline below is recorded with its reason and its thread stays open. Feedback dispositions
What changedrc:3743394920 (Critical). Added the remaining git command-execution siblings to the reject-from-project- rc:3743394923. Added the npm/pip/git trust knobs to the TLS tier: rc:3743394925. The rc:3743394926. rc:3743394927. Took the finding's documentation option rather than the provenance-based deletion change: settings.md now states the reject-only keys are frozen at boot from a user-level rc:3743394928. Declined this round, with a reply on the thread: the finding is diagnostics-only (reject/restore behavior is correct and test-pinned, as the finding itself notes), and the durable-log coordination plus pre-close callback rework is disproportionate diff growth for a budget-constrained round. The thread stays open so the suggestion is not silently dropped. Review-level CHANGES_REQUESTED (integration suite). The "Integration Tests (CLI, No Sandbox)" job is merge-queue-gated by design ( Verification
中文说明对第 2 轮审查反馈的回应（PR #8763）提交： 说明：由于上一轮耗尽时间预算，本轮带有预算警告，因此工作聚焦于阻塞子集加上低成本的范围内修复；下方唯一一条拒绝已记录理由，其讨论串保持打开。 反馈处置
变更内容rc:3743394920（Critical）。 将剩余的 git 命令执行家族变量加入"拒绝来自项目 rc:3743394923。 将 npm/pip/git 的信任旋钮加入 TLS 层： rc:3743394925。 rc:3743394926。 rc:3743394927。 采用该发现提供的文档选项，而非基于来源跟踪的删除传播：settings.md 现已说明这些"仅拒绝项目文件"的键在来自用户级 rc:3743394928。 本轮拒绝，并已在讨论串回复：该发现仅影响诊断（拒绝/恢复行为正确且有测试钉住，发现本身也如此说明），而持久日志协调加上关闭前回调通道的改造，对于预算受限轮次而言属于不成比例的 diff 增长。讨论串保持打开，该建议不会被静默丢弃。 审查级 CHANGES_REQUESTED（集成套件）。 "Integration Tests (CLI, No Sandbox)" 任务在设计上仅由合并队列触发（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Could not produce a passing fix for this feedback (round 6/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31322652532 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status): reports the autofix agent's own failed run on a stale base ('Qwen failed during address-review: status 125') and names no code defect to trace against this diff — cannot tell Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Not explored to full depth (tool budget reached): PR #8763 is a security-hardening follow-up to #8663: it e...: full fast-path.test.ts and daemon-worker.test.ts suite runs (only filtered runs of the new fast-path tests; daemon-worker breadcrumb change was verified by read…; PR #8763 is a security-hardening follow-up to #8663: it e...: full unfiltered runs of fast-path.test.ts and daemon-worker.test.ts (I verified the daemon-worker change by code inspection and ran only the filtered new fa….
中文说明
已审查。 建议见行内评论。 未决，请确认：[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status): reports the autofix agent's own failed run on a stale base ('Qwen failed during address-review: status 125') and names no code defect to trace against this diff — cannot tell 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 未探索到全部深度（达到工具调用预算）：PR #8763 is a security-hardening follow-up to #8663: it e...：full fast-path.test.ts and daemon-worker.test.ts suite runs (only filtered runs of the new fast-path tests; daemon-worker breadcrumb change was verified by read…；PR #8763 is a security-hardening follow-up to #8663: it e...：full unfiltered runs of fast-path.test.ts and daemon-worker.test.ts (I verified the daemon-worker change by code inspection and ran only the filtered new fa…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| > [!warning] | ||
| > | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `NODE_EXTRA_CA_CERTS`, or `DEV` (the dev-harness launch marker); those stay settable from the shell environment or a user-level `.env`. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `OPENSSL_CONF` (dlopens an attacker OpenSSL engine at startup), `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `DEV` (the dev-harness launch marker), the TLS trust-anchor variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, `GIT_SSL_CAINFO`, `GIT_SSL_CAPATH`, `npm_config_cafile`, `npm_config_ca`, `npm_config_strict_ssl`, `PIP_CERT` — an attacker CA there, or `npm_config_strict_ssl=false`, would enable MITM of the token-bearing traffic a session's `git`/`npm`/`pip`/`curl` calls make), the git command-execution variables (`GIT_SSH_COMMAND`, `GIT_SSH`, `GIT_EXEC_PATH`, `GIT_TEMPLATE_DIR`, `GIT_ASKPASS`, `GIT_PROXY_COMMAND`, `GIT_EDITOR`, `GIT_EXTERNAL_DIFF`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT`, `GIT_CONFIG_PARAMETERS` and the numbered `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` pairs — git runs these on any session `git` invocation), the curl/wget rc-file redirects (`CURL_HOME`, `WGETRC` — their rc files can install an attacker proxy or CA), `PIP_CONFIG_FILE` (redirects all of pip's configuration — `index-url`, `trusted-host`, proxy, or cert settings in an attacker file send session pip traffic or credentials to attacker infrastructure), `SSH_ASKPASS` (git/ssh execute it as the fallback passphrase-prompt program on an auth challenge), `LESSOPEN` and `LESSCLOSE` (`less` executes them as input preprocessors on every file a session views), or the node-gyp interpreter-selection variables (`NODE_GYP_FORCE_PYTHON`, `npm_config_python`, `PYTHON` — run as the build Python during native-addon installs — and `npm_config_git`, run as npm's git binary). Those stay settable from the shell environment or a user-level `.env`; unlike the loader list above they are rejected from project files only, so a value you export yourself is preserved. They are also frozen at boot from a user-level `.env`: a settings reload does not apply edits to them there — or their removal — until the process restarts. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. |
There was a problem hiding this comment.
[Suggestion] The rewritten denylist paragraph enumerates npm_config_strict_ssl but omits the hyphen twin npm_config_strict-ssl, which the code deliberately blocks as well (shared-env-keys.ts lists both spellings: "npm treats underscore/hyphen spellings of a config key as the same key … so both forms of strict-ssl are listed"). Every other key of the reject-only tier is enumerated here. — Failure scenario: a project .env or settings.json env sets npm_config_strict-ssl=false; it is rejected silently (hardcoded-tier rejections emit no warning); the user consults this paragraph, finds only npm_config_strict_ssl, and concludes their spelling is not blocked — debugging a "bug" that is documented behavior for every other key in the list.
Suggested fix: in the TLS trust-anchor parenthetical, spell the alias the way the code does — npm_config_strict_ssl (and npm's equivalent npm_config_strict-ssl spelling).
中文说明
重写后的拒绝列表段落列出了 npm_config_strict_ssl，但遗漏了连字符孪生形式 npm_config_strict-ssl——代码有意将两者同时阻断（shared-env-keys.ts 同时列出两种拼写，并注明"npm 将配置键的下划线/连字符拼写视为同一键……因此 strict-ssl 的两种形式都被列出"）。该层其余每个键都已在此列出。失败场景：项目 .env 或 settings.json env 设置 npm_config_strict-ssl=false；它被静默拒绝（硬编码层的拒绝不发出任何警告）；用户查阅本段落，只找到 npm_config_strict_ssl，便以为自己的拼写未被阻断——为一个对清单中其他所有键而言属于既有行为的"bug"反复排查。
修复建议：在 TLS 信任锚括注中按代码的写法同时写出该别名——npm_config_strict_ssl（以及 npm 的等价拼写 npm_config_strict-ssl）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| > [!warning] | ||
| > | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `NODE_EXTRA_CA_CERTS`, or `DEV` (the dev-harness launch marker); those stay settable from the shell environment or a user-level `.env`. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. | ||
| > **Loader-affecting variables are always rejected:** Variables that make a spawned Node.js process or OS loader execute an attacker-chosen file — `NODE_OPTIONS`, `npm_config_node_options` (and npm's config-file redirects `npm_config_userconfig`, `npm_config_globalconfig`, `npm_config_script_shell`, `npm_config_prefix`), `NODE_PATH`, `OPENSSL_CONF` (dlopens an attacker OpenSSL engine at startup), `NODE_REPL_EXTERNAL_MODULE`, `npm_config_node_gyp`, `npm_config_init_module`, `LD_PRELOAD`, `LD_AUDIT`, `DYLD_INSERT_LIBRARIES`, `BASH_ENV`, `ZDOTDIR`, and exported bash function definitions (`BASH_FUNC_*`) — are never loaded from `.env` files (any scope, including `.qwen/.env` and user-level files) or from the top-level `settings.json` `env` section. A workspace-controlled value there could hijack module resolution for every subprocess Qwen Code spawns, so Qwen Code prints a warning when it rejects such a key (once per process, per key and source — in a multi-workspace daemon each workspace's rejection is reported separately). To use one of these variables, export it in the environment you launch Qwen Code from; sessions hosted by a `qwen serve` daemon deliberately do not inherit them, while direct editor (ACP) sessions and the plain CLI keep the exported value. Library _search_ paths (`LD_LIBRARY_PATH`, `DYLD_LIBRARY_PATH`) and the interactive-shell-only `ENV` are intentionally not on this list — rejecting them breaks mainstream toolchains (`ENV=production`, conda/CUDA library dirs) — but a project `.env` still cannot apply them on reload. This rejection applies to the top-level `env` section only: per-server `mcpServers[].env` and per-hook `hooks[].env` are intentionally scoped to that server or hook and still apply (both surfaces are gated by folder trust for workspace-provided configs). Separately, a project `.env` can never set `QWEN_CLI_ENTRY` (the daemon's session-process entrypoint), `DEV` (the dev-harness launch marker), the TLS trust-anchor variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, `CURL_CA_BUNDLE`, `REQUESTS_CA_BUNDLE`, `GIT_SSL_CAINFO`, `GIT_SSL_CAPATH`, `npm_config_cafile`, `npm_config_ca`, `npm_config_strict_ssl`, `PIP_CERT` — an attacker CA there, or `npm_config_strict_ssl=false`, would enable MITM of the token-bearing traffic a session's `git`/`npm`/`pip`/`curl` calls make), the git command-execution variables (`GIT_SSH_COMMAND`, `GIT_SSH`, `GIT_EXEC_PATH`, `GIT_TEMPLATE_DIR`, `GIT_ASKPASS`, `GIT_PROXY_COMMAND`, `GIT_EDITOR`, `GIT_EXTERNAL_DIFF`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_CONFIG_COUNT`, `GIT_CONFIG_PARAMETERS` and the numbered `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` pairs — git runs these on any session `git` invocation), the curl/wget rc-file redirects (`CURL_HOME`, `WGETRC` — their rc files can install an attacker proxy or CA), `PIP_CONFIG_FILE` (redirects all of pip's configuration — `index-url`, `trusted-host`, proxy, or cert settings in an attacker file send session pip traffic or credentials to attacker infrastructure), `SSH_ASKPASS` (git/ssh execute it as the fallback passphrase-prompt program on an auth challenge), `LESSOPEN` and `LESSCLOSE` (`less` executes them as input preprocessors on every file a session views), or the node-gyp interpreter-selection variables (`NODE_GYP_FORCE_PYTHON`, `npm_config_python`, `PYTHON` — run as the build Python during native-addon installs — and `npm_config_git`, run as npm's git binary). Those stay settable from the shell environment or a user-level `.env`; unlike the loader list above they are rejected from project files only, so a value you export yourself is preserved. They are also frozen at boot from a user-level `.env`: a settings reload does not apply edits to them there — or their removal — until the process restarts. Upgrade note: before this denylist existed, some of these keys could load from `.env` files or `settings.json` `env` on some paths; they are now rejected everywhere with a warning, and a `qwen serve` daemon no longer passes inherited values of them to session subprocesses. |
There was a problem hiding this comment.
[Suggestion] The trailing "Upgrade note" this rewritten paragraph carries now also covers the newly added reject-only tier keys, for which all three of its claims are false: they are rejected from project files only, not "everywhere" (shell and home-.env values are preserved — as this paragraph itself states two sentences earlier); their rejection is silent (every hardcoded-tier rejection is a bare continue, and reportRejectedLoaderKeys filters through isLoaderEnvKey, so no warning path is reachable for these keys); and a qwen serve daemon still passes inherited values of them to session subprocesses (none of them are in INHERITED_LOADER_ENV_KEYS). — Failure scenario: an operator relying on this note either hunts for a rejection warning that is never emitted for the TLS/git/curl/pip/less/node-gyp keys, or believes the daemon scrubs exported GIT_SSH_COMMAND/PYTHON from session env — an overstatement of the security boundary in a hardening doc.
Suggested fix: scope the upgrade note to the loader tier and describe the project-file-only tier accurately — e.g. "the loader keys above are now rejected everywhere with a warning, and a qwen serve daemon no longer passes inherited values of them to session subprocesses. The project-file-only keys in this paragraph are rejected from project .env/settings.json env (currently without a per-key warning), remain settable from the shell environment or a user-level .env, and are inherited by session subprocesses when exported."
中文说明
本段重写后保留的结尾"升级说明"现在也覆盖了新增的"仅拒绝项目文件"层键，而它对这三类键的三条断言全部不成立：它们只从项目文件拒绝，并非"处处拒绝"（shell 与用户级 .env 中的值会被保留——本段前两句正是这么说的）；它们的拒绝是静默的（硬编码层的每处拒绝都是裸 continue，且 reportRejectedLoaderKeys 经由 isLoaderEnvKey 过滤，这些键没有任何可达的告警路径）；qwen serve daemon 仍会把这些键的继承值传给会话子进程（它们都不在 INHERITED_LOADER_ENV_KEYS 中）。失败场景：依赖此说明的运维要么为 TLS/git/curl/pip/less/node-gyp 键寻找一条永远不会出现的拒绝告警，要么以为 daemon 会从会话环境中剥离导出的 GIT_SSH_COMMAND/PYTHON——在一份安全加固文档中高估了安全边界。
修复建议：将升级说明限定在 loader 层，并准确描述仅项目文件层——例如："上述 loader 键现在处处被拒绝且伴随告警，qwen serve daemon 不再把它们的继承值传给会话子进程。本段中仅拒绝项目文件的键从项目 .env/settings.json env 被拒绝（目前无逐键告警），仍可从 shell 环境或用户级 .env 设置，导出时会被会话子进程继承。"
— qwen3.8-max via Qwen Code /review (v0.21.8)
| handle.release(); | ||
| handle.release(); // idempotent |
There was a problem hiding this comment.
[Suggestion] The "release is idempotent" half of this test is vacuous: probe-verified at this commit, deleting the if (released) return; guard in release() (shared-env-keys.ts) leaves all 38 tests in this file green. The double release underflows the refcount (0 → −1) invisibly; the poison only becomes observable in the next cycle — a later acquire sees depth ≠ 0 and skips its snapshot-map clear, a nested acquire then hits depth === 0 mid-cycle and clears the in-flight snapshot, so the final restore silently loses the host's loader value. — Failure scenario: a future refactor drops the guard; every test stays green while a double-close poisons the refcount and the next nested scrub cycle loses a host assignment on final release — the shared-env corruption class this PR exists to prevent.
Suggested fix: after the double release, exercise a fresh balanced cycle with nested acquires and assert the final restore (the verification probe that flips under the mutant: double release → fresh cycle with a host assignment between nested acquires → the final-restore assertion fails with the guard removed and passes with it). The new cycle must be constructed so the assertion depends on depth being exactly 0 at its first acquire — under the mutant it is −1.
中文说明
本测试中"release 幂等"的一半是空洞的：已在本提交上探针证实，删除 release()（shared-env-keys.ts）中的 if (released) return; 守卫后，本文件全部 38 个测试依旧全绿。双重 release 会隐式地把引用计数下溢（0 → −1）；毒性要到下一个周期才可观测——之后的 acquire 看到 depth ≠ 0 会跳过对快照 Map 的清空，随后嵌套 acquire 在周期中途命中 depth === 0 并清空进行中的快照，最终 release 的恢复就会悄悄丢掉宿主的 loader 赋值。失败场景：未来某次重构删掉该守卫；所有测试保持绿灯，而一次双重 close 已污染引用计数，下一个嵌套剥离周期在最终 release 时丢失宿主赋值——正是本 PR 要防止的共享环境污染一类问题。
修复建议：在双重 release 之后，再跑一个配平的完整周期（嵌套 acquire，宿主在两次 acquire 之间赋值），并断言最终恢复（验证探针在删除守卫的变异体下翻转：双重 release → 新周期中宿主在嵌套 acquire 之间赋值 → 最终恢复断言在守卫被删时失败、保留时通过）。新周期的构造必须使断言依赖首个 acquire 时 depth 恰为 0——在变异体下它是 −1。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'GIT_SSL_CAINFO', | ||
| 'GIT_SSL_CAPATH', |
There was a problem hiding this comment.
[Suggestion] R2-2: GIT_SSL_NO_VERIFY remains accepted and can disable git's certificate verification from a project file — still standing from the round-2 ledger, absent from the tree at this commit (probe-verified: isHardcodedProjectEnvExclusion('GIT_SSL_NO_VERIFY') returns false and loadEnvironment applies it from a project .env; adding the one line below flips the probe). — Failure scenario: an untrusted project .env sets GIT_SSL_NO_VERIFY=true; it passes every gate and is frozen into daemonRuntimeBaseEnv for every workspace's session subprocesses; any session git fetch/clone/push over HTTPS then skips certificate verification, enabling MITM of token-bearing git traffic by a network attacker with no attacker CA — the exact outcome this tier's comment says it blocks, and the same class as NODE_TLS_REJECT_UNAUTHORIZED, which the file already hardcodes as must-never-be-settable-from-project-files. The round-1 thread deferred this with the promise that it "rides in the follow-up round, where it is the first candidate" — this follow-up landed without it.
| 'GIT_SSL_CAINFO', | |
| 'GIT_SSL_CAPATH', | |
| 'GIT_SSL_CAINFO', | |
| 'GIT_SSL_CAPATH', | |
| 'GIT_SSL_NO_VERIFY', |
中文说明
R2-2： GIT_SSL_NO_VERIFY 仍可被接受，并能经由项目文件关闭 git 的证书校验——第 2 轮台账中的未决项，在本提交的代码树中仍不存在（探针证实：isHardcodedProjectEnvExclusion('GIT_SSL_NO_VERIFY') 返回 false，loadEnvironment 会从项目 .env 应用该值；补上下面这一行即可翻转探针）。失败场景：不受信项目的 .env 设置 GIT_SSL_NO_VERIFY=true；它通过所有门控，被冻结进 daemonRuntimeBaseEnv 并分发到每个 workspace 的会话子进程；任何会话经 HTTPS 的 git fetch/clone/push 都会跳过证书校验，使网络攻击者无需任何攻击者 CA 即可对携带 token 的 git 流量做 MITM——正是本层注释声称要阻断的结果，也与 NODE_TLS_REJECT_UNAUTHORIZED 同类（后者已被文件硬编码为绝不允许由项目文件设置）。第 1 轮的讨论串将此项延期时承诺"它随下一轮落地，且是第一候选"——本轮后续已落地，却没有它。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'qwen serve', | ||
| 'daemon', | ||
| ); | ||
| const scrubbedLoaderEnvKeys = loaderEnvScrub.removedKeys; |
There was a problem hiding this comment.
[Suggestion] R2-6: A second co-resident daemon's acquire removes no keys, so neither the stderr breadcrumb (scrubAndReportInheritedLoaderEnv reports only when keys were removed) nor the durable daemon-log entry below (gated on scrubbedLoaderEnvKeys.length > 0) fires — that daemon's log contains no evidence the inherited-loader-env scrub applies to its sessions. Still standing from the round-2 ledger: declined in round 5 as diagnostics-only, thread left open; the code is unchanged. — Failure scenario: daemon B boots into an already-scrubbed env and writes no scrub record to its own durable log; an operator later investigating why daemon B's sessions do not inherit NODE_OPTIONS finds nothing in B's log and concludes the scrub is inactive there — or a refcount leak leaves the env permanently scrubbed with no log line anywhere explaining why.
Suggested fix: log the nested case too — e.g. emit a durable-log entry when removedKeys is empty at acquire ("inherited loader env already scrubbed in this process; joining the shared refcounted scrub"), so every daemon's log records its scrub state.
中文说明
R2-6： 第二个同进程 daemon 的 acquire 不会移除任何键，因此 stderr 面包屑（scrubAndReportInheritedLoaderEnv 仅在移除了键时才输出）与下方的持久 daemon 日志条目（以 scrubbedLoaderEnvKeys.length > 0 为门）都不会触发——该 daemon 的日志中没有任何证据表明继承环境剥离作用于它的会话。第 2 轮台账未决项：第 5 轮以"仅影响诊断"为由拒绝，讨论串保持打开；代码未变。失败场景：daemon B 在已被剥离的环境中启动，其持久日志不写入任何剥离记录；运维之后排查"为何 daemon B 的会话没有继承 NODE_OPTIONS"时在 B 的日志中一无所获，便以为剥离在 B 上未生效——或者引用计数泄漏使环境被永久剥离，而没有任何日志能解释原因。
修复建议：把嵌套情形也记录下来——例如在 acquire 时若 removedKeys 为空则写一条持久日志（"继承环境在本进程中已被剥离；加入共享的引用计数剥离"），使每个 daemon 的日志都记录自身的剥离状态。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — PR #8763 (no action)Feedback triageCritical-only mode is active (after 5 change-producing rounds). Everything listed under No failed checks and no still-red checks were reported for this round. The single actionable item and its dispositionThe Disposition: no code action required (declined with evidence).
Verification performed this roundThe review recorded a coverage gap from its own run: only filtered runs of the new fast-path tests had executed, and the The review also noted "Integration Tests (CLI, No Sandbox) was skipped in CI". That is a workflow skip, not a failing check, and no named defect is attached to it — no code action to take. VerificationCommands actually run this round and their results:
OutcomeNo code changes this round; no commit created. Working tree clean; HEAD unchanged at 中文说明Autofix 本轮总结 — PR #8763（无操作）反馈分类当前处于仅处理 Critical 的模式（在 5 个产生改动的轮次之后）。 本轮没有报告失败的检查，也没有报告持续红色的检查。 唯一的可处理条目及其处置该 处置：无需代码操作（附证据予以驳回）。
本轮执行的验证审查记录了其自身运行时留下的覆盖缺口：当时只对新增的 fast-path 测试做了过滤运行，且 审查还提到 "Integration Tests (CLI, No Sandbox) was skipped in CI"（集成测试在 CI 中被跳过）。这是工作流层面的跳过，并非检查失败，且没有与之关联的已指名缺陷——无需采取代码操作。 验证本轮实际执行的命令及结果：
结果本轮无代码改动；未创建提交。工作树干净；HEAD 保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 966 passed · 0 failed · 966 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：966 通过 · 0 失败 · 966 总计 Verification reportPR 8763 Deep Verification (round 2 — follow-up at an unchanged head)Verdict: Assertions: 966 pass / 0 fail / 966 total · Verified head: 中文摘要这是在同一 head 上的复验轮：本轮 结论
Previous-finding status (round 1 → this round)Identity first: this round re-verifies a bit-identical head.
No declined or deferred rows existed in round 1, so there was nothing to re-adjudicate; nothing worsened — the code is byte-identical and every re-measurement matched round 1's value. Witness for the re-probed findings: Central claim and A/B result (re-measured live this round)Central claim: the 4 pure-injection loader keys are scrubbed from the inherited env and rejected from every The key lists were derived by diffing the modules' own exports between arms (not transcribed):
Cell totals: 420 scripted assertions across 23 cell runs (15 head + 8 base), 0 fail ( Load-bearing proofPR's own tests overlaid on base: 20 failed / 46 passed (66). Head's Mutation matrix (head): 6/6 killed, 0 survivors, unmutated control 66/66 green; each mutant died on exactly its intended pin (
This also serves as the vitest-liveness proof (the same machinery went red on demand), subsuming a planted-violation check. Targeted gates (head): FindingsF1 — [medium, completeness, stands] Package-registry/index redirects still apply from a project F2 — [low, completeness, stands] Interpreter/REPL startup-file siblings of F3 — [nit, pre-existing, stands] Project-tier rejections are unobservable. A F4 — [nit, new this round] The PR body enumerates a subset of the shipped denylist. The description names 13 project-tier keys (+ numbered pairs); the shipped list is 33 literals + 2 numbered-pair regexes (the round-2/3 commits added the git-exec siblings No injection-style steering instructions were present in the PR text or the previous report; author claims were treated as hypotheses and re-tested (the "458 tests" figure re-measured as 473 at this head, same as round 1 — consistent, suites grew across the PR's later commits). Not covered
MethodologyEnvironment: the CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
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. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action required on PR #8763This review round has no actionable items:
Critical-only mode is active after 5 change-producing rounds. The only item in No code was changed and no commit was made in this round. The branch stays at 中文说明PR #8763 无需处理本轮评审没有任何可处理的项目：
在 5 个产生改动的轮次之后，已进入仅处理 Critical 的模式。"Deferred non-Critical feedback"（延后的非 Critical 反馈）部分中唯一的条目是 @qwen-code-ci-bot 的一条自动化 PR 评论。按照本轮规则，该部分属于审计记录而非待办工作：未修改任何代码、未解决任何讨论串，也未就该条目撰写任何回复。该条目保持开放，留待人工跟进。 本轮未改动任何代码，也未创建任何提交。分支保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] issue-level comment 5228685910: autofix status — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell. Re-checked against the code at 3b70728: the comment is an autofix infrastructure status report, not a code defect.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status) — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell
中文说明
未决，请确认：[Critical] issue-level comment 5228685910: autofix status — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell. Re-checked against the code at 3b70728: the comment is an autofix infrastructure status report, not a code defect.
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
[Critical] issue-level comment 5228685910 (@qwen-code-dev-bot autofix status) — reports the autofix agent's own failed run on a stale base; names no code defect to trace against this diff — cannot tell
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| for (const key of Object.keys(env)) { | ||
| if (!isLoaderEnvKey(key)) continue; | ||
| const value = env[key]; | ||
| if (value !== undefined) sharedProcessEnvScrubOriginals.set(key, value); | ||
| } | ||
| sharedProcessEnvScrubDepth++; | ||
| const removedKeys = scrubAndReportInheritedLoaderEnv( |
There was a problem hiding this comment.
[Suggestion] acquireInheritedLoaderEnvScrub double-iterates process.env — the snapshot loop (lines 371-375) and the subsequent scrubAndReportInheritedLoaderEnv call both iterate Object.keys(env) and both call isLoaderEnvKey on every key. — Failure scenario: On every daemon startup (including overlapping embedded daemon acquires), process.env is iterated twice instead of once. For a typical ~150-entry env, this is ~300 isLoaderEnvKey calls per acquire, each doing a canonicalLoaderKey transform (2 regex substitutions + .replace()). Suggested fix: Merge the snapshot and the scrub into a single Object.keys(env) pass that both records the value and deletes the key.
中文说明
acquireInheritedLoaderEnvScrub 对 process.env 做了两次迭代——快照循环（第 371-375 行）和后续的 scrubAndReportInheritedLoaderEnv 调用都遍历了 Object.keys(env) 并对每个键调用了 isLoaderEnvKey。失败场景：每次 daemon 启动时（包括重迭内嵌 daemon 的 acquire），process.env 被迭代两次而非一次。建议：将快照和剥离合并为一次 Object.keys(env) 遍历，同时记录值并删除键。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (sharedProcessEnvScrubDepth === 0) { | ||
| sharedProcessEnvScrubOriginals.clear(); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() at acquire time (line 364) leaves every affected test green. — Failure scenario: A future change that removes or skips this clear() call would not be caught by any test in this diff, allowing stale originals from a prior scrub cycle to leak into the next scrub's snapshot. Suggested fix: Add a test that verifies sharedProcessEnvScrubOriginals is empty when the scrub depth drops to zero after a full acquire/release cycle.
中文说明
变体存活：删除 acquire 时的 sharedProcessEnvScrubOriginals.clear()（第 364 行）所有测试仍然通过。失败场景：未来删除了该 clear() 调用不会被任何测试捕获。建议：增加测试验证 sharedProcessEnvScrubOriginals 在完整 acquire/release 周期后为空。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (sharedProcessEnvScrubDepth > 0) return; | ||
| for (const [key, value] of sharedProcessEnvScrubOriginals) { |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() at release time (line 397) leaves every affected test green. — Failure scenario: Same as the acquire-time clear — a future removal would not be caught by any test. Suggested fix: Confirm or add coverage for this reset path.
中文说明
变体存活：删除 release 时的 sharedProcessEnvScrubOriginals.clear()（第 397 行）所有测试仍然通过。建议：确认或补充此重置路径的测试覆盖。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| export function resetInheritedLoaderEnvScrubForTesting(): void { | ||
| sharedProcessEnvScrubDepth = 0; | ||
| sharedProcessEnvScrubOriginals.clear(); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: deleting the sharedProcessEnvScrubOriginals.clear() in the test-only reset helper (line 403) leaves every affected test green. — Failure scenario: A future removal of this clear() call would compromise test isolation between dependent test suites. Suggested fix: Confirm or add coverage for this reset path.
中文说明
变体存活：删除仅测试重置辅助函数中的 sharedProcessEnvScrubOriginals.clear()（第 403 行）所有测试仍然通过。建议：确认或补充此重置路径的测试覆盖。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const value = env[key]; | ||
| if (value !== undefined) sharedProcessEnvScrubOriginals.set(key, value); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Mutant-survived: the if (value !== undefined) guard (line 374) is never exercised — no test pins when the guard must NOT fire. — Failure scenario: A change that removes the value !== undefined guard would not be caught by any test, allowing undefined values to be stored in sharedProcessEnvScrubOriginals. Suggested fix: Add a test case that exercises the value === undefined branch and verifies the key is not stored.
中文说明
变体存活：if (value !== undefined) 守卫（第 374 行）从未被触发——没有测试验证守卫在什么情况下必须不触发。建议：增加测试用例，验证当 value === undefined 时该键不会被存储。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
acquireInheritedLoaderEnvScrub iterated process.env twice (a snapshot pass, then the scrub); record the originals inside the scrub's single pass instead. Drop the acquire-time snapshot clear, which the release-time clear made unreachable defense, and add tests that kill the previously surviving mutants on the release-time clear, the test-only reset, and the undefined-value guard.
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8763Feedback points and dispositionsReview-level Critical: issue-level comment 5228685910 (autofix status) — Declined (no code defect)Confirmed, as the reviewer's own re-check concluded: comment 5228685910 is the [rc:3746607294] Double iteration of process.env in acquire — ResolvedMerged the snapshot and the scrub into one pass: [rc:3746607299] Mutant survived: acquire-time
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-gated by design) and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8763 is a security-hardening follow-up to #8663: it e...: run the new/changed unit tests ( shared-env-keys.test.ts , environment.test.ts , fast-path.test.ts , process-env-guard.test.ts , daemon-worker.test.ts ) — t…; PR #8763 is a security-hardening follow-up to #8663: it e...: unit tests not executed (worktree has no node_modules) — logic traced statically; process-env-guard count (whole:5, removal of computed:key) not recomputed beyo…; PR #8763 is a security-hardening follow-up to #8663: it e...: did not execute the diff's unit tests — the review worktree has no node_modules ( vitest unresolvable) and a monorepo install was not viable in budget; refco….
[Critical] R4-1: GIT_SEQUENCE_EDITOR is absent from PROJECT_ENV_HARDCODED_EXCLUSIONS even though git executes it as a command exactly like its blocked sibling GIT_EDITOR (todo-list edit on git rebase -i). Failure scenario: an untrusted repo ships a project .env with GIT_SEQUENCE_EDITOR=<repo>/evil.sh and tracks the script in-repo; it passes every gate and is applied by loadEnvironment/buildRuntimeEnvironment, and when a daemon-hosted session runs git rebase -i, git execs the attacker script as the daemon user. Probe-verified end-to-end at this commit (git 2.47.3): the headless rebase executed the named file and exited 0; an application probe applied the key from a project .env while the blocked sibling GIT_EDITOR one line away was rejected, and adding the key flipped the probe. Fix: add 'GIT_SEQUENCE_EDITOR' beside 'GIT_EDITOR'. (Relocated to the body: its resolved line 113 already carries an existing, unrelated comment.) · 中文说明：GIT_SEQUENCE_EDITOR 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS，而 git 对它的执行方式与被阻断的 GIT_EDITOR 完全相同（git rebase -i 时编辑 todo 列表）。失败场景：不受信仓库的 .env 设置 GIT_SEQUENCE_EDITOR=<repo>/evil.sh 并在仓库内跟踪该脚本；它通过所有门控并被 loadEnvironment/buildRuntimeEnvironment 应用，daemon 托管的会话运行 git rebase -i 时以 daemon 用户身份执行攻击者脚本。已在本提交端到端探针验证。修复：在 'GIT_EDITOR' 旁加入 'GIT_SEQUENCE_EDITOR'。（因解析出的行 113 上已存在一条无关评论，改置于正文。）
[Critical] R4-5: VISUAL and EDITOR are missing from PROJECT_ENV_HARDCODED_EXCLUSIONS, leaving git's documented editor fallback chain (GIT_EDITOR → core.editor → $VISUAL → $EDITOR) plantable from a project .env now that GIT_EDITOR is blocked. Failure scenario: attacker repo ships .env with VISUAL=/repo/evil.sh; the load is no-override, so it applies whenever the launch env does not export the variable. Probe-verified: git var GIT_EDITOR resolves to the VISUAL/EDITOR value when GIT_EDITOR is unset, so any session git commit without -m or git rebase -i execs the attacker script as the daemon user — the outcome GIT_EDITOR was blocked for. Independently, packages/cli/src/ui/hooks/useLaunchEditor.ts returns process.env['VISUAL'] || process.env['EDITOR'] || 'vi' and hands it to spawnSync from ordinary interactive flows (external-editor modify, memory/skill dialogs) — no git required. Both application paths probe-confirmed; adding the two keys flipped the probe. Fix: add 'VISUAL' and 'EDITOR' to the reject-only tier. (Relocated to the body: its resolved line 111 already carries an existing, unrelated comment.) · 中文说明：VISUAL 与 EDITOR 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS；在 GIT_EDITOR 已被阻断后，git 文档化的编辑器回退链（GIT_EDITOR → core.editor → $VISUAL → $EDITOR）仍可由项目 .env 植入。失败场景：攻击者仓库的 .env 设置 VISUAL=/repo/evil.sh；加载采用未设置才写入语义，只要启动环境未导出该变量即被应用。探针验证：GIT_EDITOR 未设置时 git var GIT_EDITOR 解析到 VISUAL/EDITOR 的值，会话中任何不带 -m 的 git commit 或 git rebase -i 都会以 daemon 用户身份执行攻击者脚本——正是 GIT_EDITOR 被阻断的后果。另有独立路径：packages/cli/src/ui/hooks/useLaunchEditor.ts 返回 process.env['VISUAL'] || process.env['EDITOR'] || 'vi' 并交给 spawnSync，普通交互流程（外部编辑器修改、memory/skill 对话框）即可触达，无需 git。两处应用路径均经探针确认，加入两个键后探针翻转。修复：将 'VISUAL'、'EDITOR' 加入仅拒绝项目文件层。（因解析出的行 111 上已存在一条无关评论，改置于正文。）
[Critical] R4-7: PYTHONSTARTUP is absent from both tiers — the exact Python analogue of NODE_REPL_EXTERNAL_MODULE, which this PR adds to the strongest (scrubbed loader) tier for precisely this mechanism. Failure scenario: attacker repo ships .env with PYTHONSTARTUP=/repo/evil.py; the variable is almost never set in launch envs, so the value applies from the project .env (probe-confirmed on load and runtime-env paths). CPython executes $PYTHONSTARTUP at interactive startup — probe-verified: a true-TTY bare python3 and python3 -i both ran the file before the first prompt, while python3 -c did not. The first interactive python a session runs executes attacker code as the daemon user. The deferred-class comment covers search paths (PYTHONPATH/JAVA_TOOL_OPTIONS), not startup-exec hooks. Fix: add 'PYTHONSTARTUP' to PROJECT_ENV_HARDCODED_EXCLUSIONS (reject-only tier, like PYTHON). (Relocated to the body: its resolved line 141 already carries an existing, unrelated comment.) · 中文说明：PYTHONSTARTUP 在两层名单中均不存在——它是 NODE_REPL_EXTERNAL_MODULE 的 Python 同型变体，而本 PR 正是为该机制把后者加入了最强的剥离 loader 层。失败场景：攻击者仓库的 .env 设置 PYTHONSTARTUP=/repo/evil.py；启动环境几乎从不设置该变量，因此项目 .env 的值会被应用（加载路径与运行时环境构建路径均经探针确认）。CPython 在交互式启动时执行 $PYTHONSTARTUP——探针验证：真 TTY 裸 python3 与 python3 -i 都会在首个提示符前执行该文件，python3 -c 则不会。会话第一次运行交互式 python 即以 daemon 用户身份执行攻击者代码。代码中延期类别的注释只覆盖搜索路径（PYTHONPATH/JAVA_TOOL_OPTIONS），不覆盖启动执行钩子。修复：将 'PYTHONSTARTUP' 加入 PROJECT_ENV_HARDCODED_EXCLUSIONS（仅拒绝项目文件层，与 PYTHON 相同）。（因解析出的行 141 上已存在一条无关评论，改置于正文。）
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-gated by design) and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8763 is a security-hardening follow-up to #8663: it e...：run the new/changed unit tests ( shared-env-keys.test.ts , environment.test.ts , fast-path.test.ts , process-env-guard.test.ts , daemon-worker.test.ts ) — t…；PR #8763 is a security-hardening follow-up to #8663: it e...：unit tests not executed (worktree has no node_modules) — logic traced statically; process-env-guard count (whole:5, removal of computed:key) not recomputed beyo…；PR #8763 is a security-hardening follow-up to #8663: it e...：did not execute the diff's unit tests — the review worktree has no node_modules ( vitest unresolvable) and a monorepo install was not viable in budget; refco…。
[Critical] R4-1: GIT_SEQUENCE_EDITOR is absent from PROJECT_ENV_HARDCODED_EXCLUSIONS even though git executes it as a command exactly like its blocked sibling GIT_EDITOR (todo-list edit on git rebase -i). Failure scenario: an untrusted repo ships a project .env with GIT_SEQUENCE_EDITOR=<repo>/evil.sh and tracks the script in-repo; it passes every gate and is applied by loadEnvironment/buildRuntimeEnvironment, and when a daemon-hosted session runs git rebase -i, git execs the attacker script as the daemon user. Probe-verified end-to-end at this commit (git 2.47.3): the headless rebase executed the named file and exited 0; an application probe applied the key from a project .env while the blocked sibling GIT_EDITOR one line away was rejected, and adding the key flipped the probe. Fix: add 'GIT_SEQUENCE_EDITOR' beside 'GIT_EDITOR'. (Relocated to the body: its resolved line 113 already carries an existing, unrelated comment.) · 中文说明：GIT_SEQUENCE_EDITOR 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS，而 git 对它的执行方式与被阻断的 GIT_EDITOR 完全相同（git rebase -i 时编辑 todo 列表）。失败场景：不受信仓库的 .env 设置 GIT_SEQUENCE_EDITOR=<repo>/evil.sh 并在仓库内跟踪该脚本；它通过所有门控并被 loadEnvironment/buildRuntimeEnvironment 应用，daemon 托管的会话运行 git rebase -i 时以 daemon 用户身份执行攻击者脚本。已在本提交端到端探针验证。修复：在 'GIT_EDITOR' 旁加入 'GIT_SEQUENCE_EDITOR'。（因解析出的行 113 上已存在一条无关评论，改置于正文。）
[Critical] R4-5: VISUAL and EDITOR are missing from PROJECT_ENV_HARDCODED_EXCLUSIONS, leaving git's documented editor fallback chain (GIT_EDITOR → core.editor → $VISUAL → $EDITOR) plantable from a project .env now that GIT_EDITOR is blocked. Failure scenario: attacker repo ships .env with VISUAL=/repo/evil.sh; the load is no-override, so it applies whenever the launch env does not export the variable. Probe-verified: git var GIT_EDITOR resolves to the VISUAL/EDITOR value when GIT_EDITOR is unset, so any session git commit without -m or git rebase -i execs the attacker script as the daemon user — the outcome GIT_EDITOR was blocked for. Independently, packages/cli/src/ui/hooks/useLaunchEditor.ts returns process.env['VISUAL'] || process.env['EDITOR'] || 'vi' and hands it to spawnSync from ordinary interactive flows (external-editor modify, memory/skill dialogs) — no git required. Both application paths probe-confirmed; adding the two keys flipped the probe. Fix: add 'VISUAL' and 'EDITOR' to the reject-only tier. (Relocated to the body: its resolved line 111 already carries an existing, unrelated comment.) · 中文说明：VISUAL 与 EDITOR 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS；在 GIT_EDITOR 已被阻断后，git 文档化的编辑器回退链（GIT_EDITOR → core.editor → $VISUAL → $EDITOR）仍可由项目 .env 植入。失败场景：攻击者仓库的 .env 设置 VISUAL=/repo/evil.sh；加载采用未设置才写入语义，只要启动环境未导出该变量即被应用。探针验证：GIT_EDITOR 未设置时 git var GIT_EDITOR 解析到 VISUAL/EDITOR 的值，会话中任何不带 -m 的 git commit 或 git rebase -i 都会以 daemon 用户身份执行攻击者脚本——正是 GIT_EDITOR 被阻断的后果。另有独立路径：packages/cli/src/ui/hooks/useLaunchEditor.ts 返回 process.env['VISUAL'] || process.env['EDITOR'] || 'vi' 并交给 spawnSync，普通交互流程（外部编辑器修改、memory/skill 对话框）即可触达，无需 git。两处应用路径均经探针确认，加入两个键后探针翻转。修复：将 'VISUAL'、'EDITOR' 加入仅拒绝项目文件层。（因解析出的行 111 上已存在一条无关评论，改置于正文。）
[Critical] R4-7: PYTHONSTARTUP is absent from both tiers — the exact Python analogue of NODE_REPL_EXTERNAL_MODULE, which this PR adds to the strongest (scrubbed loader) tier for precisely this mechanism. Failure scenario: attacker repo ships .env with PYTHONSTARTUP=/repo/evil.py; the variable is almost never set in launch envs, so the value applies from the project .env (probe-confirmed on load and runtime-env paths). CPython executes $PYTHONSTARTUP at interactive startup — probe-verified: a true-TTY bare python3 and python3 -i both ran the file before the first prompt, while python3 -c did not. The first interactive python a session runs executes attacker code as the daemon user. The deferred-class comment covers search paths (PYTHONPATH/JAVA_TOOL_OPTIONS), not startup-exec hooks. Fix: add 'PYTHONSTARTUP' to PROJECT_ENV_HARDCODED_EXCLUSIONS (reject-only tier, like PYTHON). (Relocated to the body: its resolved line 141 already carries an existing, unrelated comment.) · 中文说明：PYTHONSTARTUP 在两层名单中均不存在——它是 NODE_REPL_EXTERNAL_MODULE 的 Python 同型变体，而本 PR 正是为该机制把后者加入了最强的剥离 loader 层。失败场景：攻击者仓库的 .env 设置 PYTHONSTARTUP=/repo/evil.py；启动环境几乎从不设置该变量，因此项目 .env 的值会被应用（加载路径与运行时环境构建路径均经探针确认）。CPython 在交互式启动时执行 $PYTHONSTARTUP——探针验证：真 TTY 裸 python3 与 python3 -i 都会在首个提示符前执行该文件，python3 -c 则不会。会话第一次运行交互式 python 即以 daemon 用户身份执行攻击者代码。代码中延期类别的注释只覆盖搜索路径（PYTHONPATH/JAVA_TOOL_OPTIONS），不覆盖启动执行钩子。修复：将 'PYTHONSTARTUP' 加入 PROJECT_ENV_HARDCODED_EXCLUSIONS（仅拒绝项目文件层，与 PYTHON 相同）。（因解析出的行 141 上已存在一条无关评论，改置于正文。）
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'GIT_CONFIG_GLOBAL', | ||
| 'GIT_CONFIG_SYSTEM', |
There was a problem hiding this comment.
[Critical] R4-6: XDG_CONFIG_HOME is not excluded, giving a project .env a config-discovery redirect into git that bypasses every GIT_CONFIG_* block this PR adds — git merges $XDG_CONFIG_HOME/git/config with ~/.gitconfig (verified at this commit: git config --list --show-origin reads both). — Failure scenario: attacker repo ships .env with XDG_CONFIG_HOME=/repo/.xdg plus a committed .xdg/git/config containing [core] hooksPath = /repo/.hooks → with GIT_CONFIG_GLOBAL/SYSTEM/COUNT/PARAMETERS all blocked by this PR, the first session git commit still executes the attacker hook as the daemon user. Probe-reproduced end-to-end at this commit (control arm without the variable did not execute the hook); adding the key to the list flipped the probe on both application paths. Same config-file-redirect class this list already blocks for GIT_CONFIG_GLOBAL, CURL_HOME/WGETRC, and PIP_CONFIG_FILE.
| 'GIT_CONFIG_GLOBAL', | |
| 'GIT_CONFIG_SYSTEM', | |
| 'GIT_CONFIG_GLOBAL', | |
| 'GIT_CONFIG_SYSTEM', | |
| 'XDG_CONFIG_HOME', |
中文说明
R4-6： XDG_CONFIG_HOME 未被排除，项目 .env 可以借此重定向 git 的配置发现路径，绕过本 PR 新增的全部 GIT_CONFIG_* 阻断——git 会将 $XDG_CONFIG_HOME/git/config 与 ~/.gitconfig 合并读取（已在本提交验证：git config --list --show-origin 两个文件都会读取）。失败场景：攻击者仓库的 .env 设置 XDG_CONFIG_HOME=/repo/.xdg，并提交一个含 [core] hooksPath = /repo/.hooks 的 .xdg/git/config → 即使 GIT_CONFIG_GLOBAL/SYSTEM/COUNT/PARAMETERS 都已被本 PR 阻断，会话第一次 git commit 仍会以 daemon 用户身份执行攻击者的 hook。已在本提交端到端探针复现（不设置该变量的对照组未执行 hook）；将该键加入名单后探针在两处应用路径上均翻转。与本列表已阻断的 GIT_CONFIG_GLOBAL、CURL_HOME/WGETRC、PIP_CONFIG_FILE 属于同一“配置文件重定向”类别。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // npm runs `$npm_config_git` as the git binary for install-from-git and | ||
| // similar flows, so a project `.env` pointing it at an attacker script is | ||
| // the same exec redirect as the interpreter keys above. | ||
| 'npm_config_git', |
There was a problem hiding this comment.
[Critical] R4-8: BROWSER is absent from PROJECT_ENV_HARDCODED_EXCLUSIONS, yet the CLI itself execs $BROWSER via openBrowserSecurely (packages/core/src/utils/secure-browser-launcher.ts) — the same exec-redirect class this tier exists to block. — Failure scenario: attacker repo ships .env with BROWSER=/repo/evil.sh → the value is applied on all four boundaries (probe-verified: initial load, reload, buildRuntimeEnvironment, settings.env; the comparator keys npm_config_git/QWEN_CLI_ENTRY in the same .env were rejected) → any browser-launch flow executes it. In openBrowserSecurely, a set BROWSER is passed straight to spawn() before the shouldAttemptBrowserLaunch CI/DISPLAY gates are consulted, and the only blocklisted command is www-browser — probe: with CI=true and no DISPLAY, openBrowserSecurely('https://…') spawned /repo/evil.sh. Reachable from /docs, /bug, /extensions, /insight, qwen serve --open, MCP OAuth, qwen auth, and the artifact tool's auto-open (default permission ask; zero-interaction under yolo/allow-rule modes). Folder trust is off by default, so no trust grant is needed.
| // npm runs `$npm_config_git` as the git binary for install-from-git and | |
| // similar flows, so a project `.env` pointing it at an attacker script is | |
| // the same exec redirect as the interpreter keys above. | |
| 'npm_config_git', | |
| // npm runs `$npm_config_git` as the git binary for install-from-git and | |
| // similar flows, so a project `.env` pointing it at an attacker script is | |
| // the same exec redirect as the interpreter keys above. | |
| 'npm_config_git', | |
| 'BROWSER', |
中文说明
R4-8： BROWSER 不在 PROJECT_ENV_HARDCODED_EXCLUSIONS 中，而 CLI 自身会经由 openBrowserSecurely（packages/core/src/utils/secure-browser-launcher.ts）执行 $BROWSER——与该层要阻断的“执行重定向”类别完全相同。失败场景：攻击者仓库的 .env 设置 BROWSER=/repo/evil.sh → 该值在全部四处边界被应用（探针验证：初始加载、重载、buildRuntimeEnvironment、settings.env；同一 .env 中的对照键 npm_config_git/QWEN_CLI_ENTRY 均被拒绝）→ 任何打开浏览器的流程都会执行它。在 openBrowserSecurely 中，已设置的 BROWSER 会在 shouldAttemptBrowserLaunch 的 CI/DISPLAY 门控之前直接交给 spawn()，且命令黑名单中仅有 www-browser——探针：在 CI=true 且无 DISPLAY 时，openBrowserSecurely('https://…') 启动了 /repo/evil.sh。可经 /docs、/bug、/extensions、/insight、qwen serve --open、MCP OAuth、qwen 认证以及 artifact 工具的自动打开触达（默认权限为 ask；在 yolo/allow-rule 模式下无需交互）。folder trust 默认关闭，无需任何信任授权。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // QWEN_CLI_ENTRY is the script path daemon-spawned session processes run. | ||
| // A project `.env` or settings.env fixing it turns | ||
| // `cd <untrusted repo> && qwen serve` into code execution as the daemon |
There was a problem hiding this comment.
[Critical] R4-9: QWEN_CDP_MCP_COMMAND (a command the daemon spawns as the browser-automation MCP adapter) and QWEN_SERVE_CDP_TUNNEL_OVER_WS (which switches the tunnel surface on) are settable from a project .env, violating the exact rationale the adjacent QWEN_CLI_ENTRY exclusion below states. — Failure scenario: attacker repo ships .env with QWEN_CDP_MCP_COMMAND=/repo/evil-adapter and QWEN_SERVE_CDP_TUNNEL_OVER_WS=1 → probe-confirmed applied on all four boundaries; the serve fast path writes both into process.env before runQwenServe reads them → the env var alone makes cdpTunnelOverWs true (envFlagDisabled('1') is false) → on the default tokenless daemon, when a WS client initializes with the CDP bridge client name, ensureChromeDevToolsMcpRegistered builds a runtime MCP config whose command is resolveCdpMcpCommand(daemonEnv) — the attacker path — and addRuntimeMcpServer spawns it: code execution as the daemon user. Both cdp_tunnel_over_ws and browser_automation_mcp capability advertisements are flipped by project-controlled values. Fix (the key line sits just below this anchor, so a regular block rather than a suggestion):
'QWEN_CLI_ENTRY',
'QWEN_CDP_MCP_COMMAND',
'QWEN_SERVE_CDP_TUNNEL_OVER_WS',
中文说明
R4-9： QWEN_CDP_MCP_COMMAND（daemon 作为浏览器自动化 MCP 适配器启动的命令）与 QWEN_SERVE_CDP_TUNNEL_OVER_WS（开启 tunnel 应用面的开关）可以从项目 .env 设置，直接违反了下文 QWEN_CLI_ENTRY 排除项自身写明的理由。失败场景：攻击者仓库的 .env 设置 QWEN_CDP_MCP_COMMAND=/repo/evil-adapter 与 QWEN_SERVE_CDP_TUNNEL_OVER_WS=1 → 探针证实在全部四处边界被应用；serve 快速路径会在 runQwenServe 读取它们之前把两个键写入 process.env → 仅凭该环境变量即可使 cdpTunnelOverWs 为 true（envFlagDisabled('1') 为 false）→ 在默认无 token 的 daemon 上，任何以 CDP bridge 客户端名初始化的 WS 连接都会触发 ensureChromeDevToolsMcpRegistered，其构建的运行时 MCP 配置的 command 即 resolveCdpMcpCommand(daemonEnv)——攻击者路径——addRuntimeMcpServer 会将其启动：以 daemon 用户身份执行代码。cdp_tunnel_over_ws 与 browser_automation_mcp 两个能力声明也都会被项目可控的值翻转。修复（键所在行位于本锚点下方，故用普通代码块而非 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'NODE_REPL_EXTERNAL_MODULE', | ||
| 'OPENSSL_CONF', |
There was a problem hiding this comment.
[Suggestion] R4-2: The four new scrubbed loader keys (OPENSSL_CONF, NODE_REPL_EXTERNAL_MODULE, npm_config_node_gyp, npm_config_init_module) are pinned only by the list-pin and isLoaderEnvKey unit tests; the behavioural scrub test 'removes every loader-affecting key and keeps the rest' (~line 346, unchanged by this PR) never sets them, so its for (const key of INHERITED_LOADER_ENV_KEYS) expect(env[key]).toBeUndefined() loop passes vacuously for the four new keys. — Failure scenario (mutation probe-verified): replacing scrubInheritedLoaderEnv's isLoaderEnvKey(key) filter with a stale literal list missing the four new keys leaves 473/473 tests green, yet a daemon launched from a shell exporting a malicious OPENSSL_CONF would pass it straight into session subprocesses — the dlopen-engine code execution this PR's list addition is meant to block. The probe also verified the fix below kills the mutant and stays green on the real code. Add the four keys to that fixture and its expected removedKeys list:
process.env['OPENSSL_CONF'] = '/evil.cnf';
process.env['NODE_REPL_EXTERNAL_MODULE'] = '/evil.mjs';
process.env['npm_config_node_gyp'] = '/evil-gyp.js';
process.env['npm_config_init_module'] = '/evil-init.js';
中文说明
R4-2： 四个新增的被剥离 loader 键（OPENSSL_CONF、NODE_REPL_EXTERNAL_MODULE、npm_config_node_gyp、npm_config_init_module）只被列表钉住测试与 isLoaderEnvKey 单元测试覆盖；行为级剥离测试 'removes every loader-affecting key and keeps the rest'（约第 346 行，本 PR 未改动）从未在 fixture 中设置它们，因此其 for (const key of INHERITED_LOADER_ENV_KEYS) expect(env[key]).toBeUndefined() 循环对这四个新键是空洞通过。失败场景（变异探针验证）：把 scrubInheritedLoaderEnv 的 isLoaderEnvKey(key) 过滤替换为遗漏这四个新键的陈旧字面量列表，473/473 个测试仍全绿，但从导出了恶意 OPENSSL_CONF 的 shell 启动的 daemon 会把它直接传进会话子进程——正是本 PR 新增名单要阻断的 dlopen 引擎代码执行。探针同时验证了下方修复能杀死该变异体，且在真实代码上保持全绿。请将这四个键加入该 fixture 及其期望的 removedKeys 列表。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 10/100 轮）。改动内容与我反驳保留之处如下： Round summary (PR #8763, review round 4)All seven actionable findings were verified against the exact code at this Findings and dispositions
No finding was declined or escalated. Also in this commit
Verification
中文说明轮次总结（PR #8763，审查第 4 轮）全部 7 条可执行发现均已在本提交的代码上逐一核实，并在一次提交（ 发现与处置
无发现被拒绝或升级待决。 本提交的其他内容
验证
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
📊 Takeover milestone — round 10/100, in the current window. Census: 6 pushed fix(es), 3 no-change review(s), 2 timeout(s), 1 rejected attempt(s), 1 other round(s) (crash / model error / gate error / infra), 2 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮（当前窗口）。统计：推送修复 6 次、审阅无需改动 3 次、超时 2 次、验证拒绝 1 次、其他轮次（崩溃/模型错误/门错误/infra）1 次、base 更新 2 次。 轮次到这个量值得人工看一眼。可选：继续（无需操作）；若轮次持续累积，考虑拆分或缩减 PR；或释放接管（移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Not explored to full depth (tool budget reached): PR #8763 is a security-hardening follow-up to #8663: it e...: could not execute the test suite ( shared-env-keys.test.ts , process-env-guard.test.ts , environment.test.ts , etc.) — neither the worktree nor the parent che….
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 未探索到全部深度（达到工具调用预算）：PR #8763 is a security-hardening follow-up to #8663: it e...：could not execute the test suite ( shared-env-keys.test.ts , process-env-guard.test.ts , environment.test.ts , etc.) — neither the worktree nor the parent che…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // and the CLI's own useLaunchEditor spawns them from ordinary interactive | ||
| // flows. | ||
| 'VISUAL', | ||
| 'EDITOR', |
There was a problem hiding this comment.
[Suggestion] npm_config_editor is missing from PROJECT_ENV_HARDCODED_EXCLUSIONS — npm maps it onto its editor config and npm edit executes it, the same exec-redirect class this round blocks for VISUAL/EDITOR/GIT_EDITOR. — Failure scenario: an untrusted repo ships a project .env with npm_config_editor=<repo>/evil.sh; probe-verified the key passes every gate and is applied by all four application boundaries, and when a session runs npm edit <pkg> npm spawns the attacker script as the daemon/CLI user (end-to-end exec probe-verified; adding the key flipped the probe).
| // and the CLI's own useLaunchEditor spawns them from ordinary interactive | |
| // flows. | |
| 'VISUAL', | |
| 'EDITOR', | |
| // and the CLI's own useLaunchEditor spawns them from ordinary interactive | |
| // flows. npm maps npm_config_editor onto the same `editor` config that | |
| // `npm edit` executes, so it belongs in this tier too. | |
| 'VISUAL', | |
| 'EDITOR', | |
| 'npm_config_editor', |
中文说明
npm_config_editor 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS——npm 将其映射为 editor 配置，而 npm edit 会执行它，与本轮已阻断的 VISUAL/EDITOR/GIT_EDITOR 属于同一“执行重定向”类别。失败场景：不受信仓库的 .env 设置 npm_config_editor=<repo>/evil.sh；探针验证该键通过所有门控并在全部四处应用边界被应用，会话运行 npm edit <pkg> 时 npm 会以 daemon/CLI 用户身份启动攻击者脚本（已端到端探针验证；加入该键后探针翻转）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'LESSOPEN', | ||
| 'LESSCLOSE', |
There was a problem hiding this comment.
[Suggestion] MANPAGER is missing from PROJECT_ENV_HARDCODED_EXCLUSIONS — it is the one pager-exec variable that survives the CLI's spawn-env construction (getShellPagerEnv overrides only PAGER/GIT_PAGER), the same conditional-exec class as the blocked LESSOPEN/LESSCLOSE just above. — Failure scenario: a project .env sets MANPAGER=<repo>/evil.sh; probe-verified it passes every gate and is applied by all four boundaries; a pty-backed session running man <topic> or git help <cmd> executes the attacker script as the daemon/CLI user (empirically confirmed under a pty: man runs $MANPAGER as a shell command; adding the key flipped the probe).
| 'LESSOPEN', | |
| 'LESSCLOSE', | |
| 'LESSOPEN', | |
| 'LESSCLOSE', | |
| 'MANPAGER', |
中文说明
MANPAGER 未列入 PROJECT_ENV_HARDCODED_EXCLUSIONS——它是唯一能穿透 CLI 子进程环境构造的 pager 执行变量（getShellPagerEnv 只覆盖 PAGER/GIT_PAGER），与上方已阻断的 LESSOPEN/LESSCLOSE 属于同一条件执行类别。失败场景：项目 .env 设置 MANPAGER=<repo>/evil.sh；探针验证其通过所有门控并在全部四处边界被应用；pty 会话运行 man <topic> 或 git help <cmd> 时会以 daemon/CLI 用户身份执行攻击者脚本（已在 pty 下实证：man 会把 $MANPAGER 作为 shell 命令执行；加入该键后探针翻转）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // attacker file sends session pip traffic or credentials to attacker | ||
| // infrastructure — the same config-file-redirect class as | ||
| // npm_config_userconfig and GIT_CONFIG_GLOBAL. | ||
| 'PIP_CONFIG_FILE', |
There was a problem hiding this comment.
[Suggestion] AWS_CONFIG_FILE is not excluded from project .env — the AWS analog of the config-file-redirect class this diff blocks for pip (PIP_CONFIG_FILE), npm, git, curl and wget. — Failure scenario: a project .env sets AWS_CONFIG_FILE=<repo>/aws-config whose profile declares credential_process = <repo>/evil.sh; probe-verified the key passes every gate and is applied on initial load, reload, runtime build and settings.env (and sanitize-child-env.ts explicitly preserves third-party credential vars into session subprocesses); the AWS CLI executes that process on any credential resolution (e.g. aws s3 ls) — code execution as the daemon user plus credential diversion (fix flip-checked: adding the key closes all four boundaries).
| 'PIP_CONFIG_FILE', | |
| 'PIP_CONFIG_FILE', | |
| 'AWS_CONFIG_FILE', |
中文说明
AWS_CONFIG_FILE 未从项目 .env 排除——这是本 diff 已为 pip（PIP_CONFIG_FILE）、npm、git、curl、wget 阻断的“配置文件重定向”类别的 AWS 同型变体。失败场景：项目 .env 设置 AWS_CONFIG_FILE=<repo>/aws-config，其 profile 声明 credential_process = <repo>/evil.sh；探针验证该键通过所有门控，并在初始加载、重载、运行时环境构建与 settings.env 四处边界被应用（且 sanitize-child-env.ts 明确保留第三方凭据变量进入会话子进程）；AWS CLI 在任何凭据解析时（如 aws s3 ls）都会执行该进程——以 daemon 用户身份执行代码并转移凭据（修复已翻转验证：加入该键后四处边界全部关闭）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 699 passed · 1 failed · 700 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：699 通过 · 1 失败 · 700 总计 Verification reportPR 8763 Deep Verification (round 3 — follow-up at a NEW head)Verdict: Assertions: 699 pass / 1 fail / 700 total · Verified head: 中文摘要这是 head 变更后的复验轮（round 3）： 结论
Previous-finding status (rounds 1-2 → this round)Identity first: this round verifies a new head.
No declined rows existed; nothing worsened — every previously-green behavior re-measured green (all A/B head cells, 482/482 gates). The 8 round-4 keys are the only behavior delta, and they only tighten rejection. Central claim and A/B result (re-measured live at the new head)Central claim: the 4 pure-injection loader keys are scrubbed from the inherited env and rejected from every Key lists were derived by diffing the modules' own exports between arms (not transcribed):
Cell totals: 137 pass + 1 fail across 21 cell runs (12 head + 9 base); the one fail is the F5 docs assertion ( Delta verification (the two new commits)
5/5 killed, 0 survivors; unmutated control 74/74 green; every mutant died on exactly its intended pin with expected-vs-actual assertion messages (behavioral mismatch, not import breakage); tree restored clean after each mutant. The one-pass merge's observable semantics are additionally pinned live by the A/B refcount cells (newest-wins snapshot, later-assignment-wins, cycle-2 freshness).
FindingsF1 — [medium, completeness, stands since round 1] Package-registry/index redirects still apply from a project F2 — [low, completeness, partially fixed] Interpreter/REPL startup-file siblings: F3 — [nit, pre-existing, stands] Project-tier rejections are unobservable. A F4 — [nit, stands, gap widened] The PR body enumerates a subset of the shipped denylist. The description names 13 project-tier keys + the numbered pairs; the shipped list at this head is 55 literals + 2 pattern regexes (41 PR-added literals, derived). F5 — [nit, new this round] No injection-style steering instructions were present in the PR text or the previous report; author claims were treated as hypotheses and re-tested (the "458 tests" figure in the description re-measured as 482 across the six affected suites at this head — consistent with suite growth across the PR's later commits). Not covered
MethodologyEnvironment: the CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
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. ✅
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31381560750 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed the exact current head (02fcc3b). I revalidated the unresolved Critical threads against the tree rather than relying on GitHub thread state: the previously reported git config/SSH, config-discovery, browser-launch, and CDP command surfaces are now present in the hardcoded project-env gate and covered across the initial load, reload, runtime-env build, and serve fast path. The ref-counted inherited-env scrub also has balanced startup/close cleanup.
The remaining open items are non-blocking Suggestions: reporter diagnostics for overlapping embedded daemons, hardcoded-tier warning/docs accuracy, a few adjacent denylist candidates, and mutation-strength test gaps. This PR has already exceeded the repository review-round budget, so those should stay in the stated follow-up rather than widening this round. Approving with suggestions; CI should still finish green.
…eak (QwenLM#8816) * feat(ci): A/B deterministic gate rejections against the pre-round ref A deterministic rejection in the autofix verification gate is only chargeable to the round if the same check passes without the round's commit. The gate charged every red to the fix unconditionally, and run 31276008548 measured what that costs when the premise is false: PR 8614's branch predated QwenLM#8693's tsconfig guard while node_modules came from the post-QwenLM#8693 trusted base, so `npm run build` was equally red at origin/<branch> — 63 minutes of accepted agent work discarded, an 18-minute repair burned on a failure the repair agent is forbidden to touch (it may only amend the round's own fix), thirteen rounds in a row, and the same again on the QwenLM#8616 leg. On rejection the gate now re-runs the failing check at origin/<branch> (the branch as pushed, before the round) in the same environment: - baseline green: today's path exactly — outcome=failed, retryable=true, the repair pass gets its chance. - baseline red too: outcome=failed with preexisting=true and NO retryable. The repair step keys on retryable and is skipped — it cannot reach a failure outside the round's diff by construction — and gate-rejection.md says outright that the branch needs a base update (merge main), which flows into the failure comment as-is. Fail-closed toward today's semantics: any A/B infrastructure problem (missing ref, checkout failure) charges the fix as before, and a restore failure after the baseline run rejects outright since the tree can no longer be trusted. The round's work is still not pushed — this changes the verdict's honesty and cost, not the push policy. Tested by executing the real script in a real two-remote git repo with an npm stub whose failures are keyed by commit SHA: round-caused red (baseline green), pre-existing red (both red), and the untouched green path. Mutation-tested, 3 of 3 caught: skipping the A/B, claiming pre-existing without measuring, and dropping the tree restore. * Address review: bound the A/B to checks it can honestly compare All seven findings verified before fixing; the three Criticals were each a way the A/B compared something other than the check that failed. R1-1 — the contracts check feeds on stdin, which its first run drains; the baseline leg re-ran against EOF and checked an empty file list. R1-3 — the schema check's verdict rides on packages/core/dist, which the core-rebuild guard built from ROUND sources and which, being gitignored, survives the detach. Both checks are now A/B-exempt (run_check_no_ab): their baseline verdicts prove nothing, and their rejections stay where the repair agent can actually act on them. R1-2 — a workspace the round ADDS does not exist at the baseline, and npm exits 1 there with "No workspaces found" (measured; --if-present forgives a missing script, not a missing workspace) — a round-caused failure misread as pre-existing, skipping the one repair that can fix the round's own package. The per-package loop now A/Bs only when the workspace exists at origin/<branch>. R1-4 — a chatty PASSING baseline used to flood the tail -c 3000 evidence window and push the actual failure text out of gate-rejection.md, the sole carrier into the repair feedback, the PR comment, and the next round's LAST_REJECTION. The baseline transcript now goes to a side log and only a FAILING tail is merged back, where it is the evidence. R1-5 — the pre-existing paragraph pushed gate-rejection.md past the report's head -c 3500 cap, truncating the closing fence for branch names past 44 characters. Cap raised to 3900, invariant comment updated with the new arithmetic. R1-6 — preexisting=true had no read site. It now flows verify → Finalize verification → the failure report, whose headline swaps the generic gate clause for "PRE-EXISTING failure … needs a base update (merge main)". R1-7 — the no-round-commit guard was unpinned (deleting it kept all tests green). Now exercised through the core-rebuild path, the one A/B-eligible check that runs before the commit gate. Four new behavioral scenarios (chatty baseline, no-commit round, A/B-exempt checks, round-added workspace) plus workflow pins for the forwarding, the clause, and the cap. Mutation-tested, 4 of 4 caught: schema back to A/B (3 tests), guard dropped, side log reverted, no-commit guard dropped. * Address review round 2: A/B only what it can prove, prove what it claims Ten findings across two rounds, each verified before fixing. The three deepest share one lesson: the A/B is only sound for a check whose inputs travel entirely with the git ref, and whose failure it can IDENTIFY, not merely observe. R2-1 — rc=1 at both legs does not make them the same failure: the branch can fail for reason A while the round fails for reason B, and a baseline infrastructure hiccup is a nonzero exit too. Pre-existing now requires a MATCHING failure identity — tsc diagnostics normalized to file + error code (positions shift with the round's edits), compared via comm(1) on a per-check transcript. No diagnostics on either side means identity cannot be established and the round stays charged. R2-2 / R2-7 — gitignored dist survives the detach carrying the ROUND's build, so any dist-consuming check A/Bs reverted sources against round-built artifacts: package tests (channel-base resolved through dist exports) and typecheck (sdk-typescript resolves core's d.ts — probe-verified three-arm flip). Both are now A/B-exempt, as is lint, leaving `npm run build` — the incident class, and the one check that rebuilds its own inputs from the checked-out sources — as the sole A/B candidate. The workspace-existence guard dissolves with it. R2-3 — the fixture inherited the caller's global git config; a failing global pre-commit hook broke all seven cases. The harness now isolates GIT_CONFIG_GLOBAL/SYSTEM for every git child, and the suite is proven green under a deliberately hostile hooksPath. R2-4 — Finalize verification now selects preexisting from the same attempt whose outcome it selects (repair verification included). R2-5 / R2-8 — the "merge main" advice is now conditional at both layers: the script paragraph states the measured fact and hedges the remedy; the report headline uses the compare the step already ran — behind/diverged gets the base-update clause, an up-to-date branch is told its own pre-round code needs attention. R2-6 — the rejection document now sizes its evidence tail against its preamble (floor 500 bytes, total under the 3900-byte render cap), so the closing fence can no longer be truncated off by a long branch name. R2-9 — dissolved by R2-2: package tests no longer A/B, the guard and its uncovered positive branch are gone. R2-10 — the baseline-evidence merge is now pinned: the pre-existing scenario asserts the baseline leg's own failure line (keyed by its SHA) reaches gate-rejection.md. Eight behavioral scenarios; mutation-tested 5 of 5: identity dropped, typecheck re-enrolled, package tests re-enrolled, evidence merge dropped, fixed tail restored. * Address review round 4: sharpen identity, stage the git failures, sync prose Nine findings, all refinements — the design held, the edges did not. Identity now keeps the diagnostic MESSAGE (file + code collide: two unrelated TS2339s in one file compared equal, skipping a repair that could have shipped — probe-reproduced by the review), and the fixture emits a SHIFTED position on the baseline leg so the position strip is load-bearing instead of decorative (deleting the sed survived every test before; it fails one now). vite/esbuild failures still yield an empty signature by design — documented as the fail-closed limit rather than half-widened. The fail_signature assignments take `|| true`: grep exits 1 on the normal no-match case and survives errexit today only because the caller sits in an if-condition — a future unconditional call site would crash the gate verdict-less. The restore-failure branch is now stageable and staged: the baseline leg recreates (untracked) a file the branch tracks, the checkout back refuses, and the test pins retryable-not-preexisting with the 'could not restore' label. Relaxing the branch to `|| true` fails it. Prose synced to the mechanisms that replaced it: the render-cap invariant restates against the dynamic tail budget (the old 3000-based arithmetic would misguide the next retune), the no-round-commit guard comment names the core rebuild (schema/contracts left the A/B last round), the describe wording counts both A/B-eligible builds, and the pre-existing clauses no longer claim "the repair pass was skipped" — with REPAIR_PREEXISTING forwarded, repair may have RUN; they now state the invariant that is true either way: repair may only amend the round's own fix, so it cannot reach this failure. Mutation-tested, 3 of 3 caught: position strip dropped, message dropped from the identity, restore rejection relaxed. * fix(ci): watchdog silent sandbox hangs and reap the containers they leak Four autofix rounds have died the same way (QwenLM#8663 twice, QwenLM#8761 r3, QwenLM#8763 r4): the agent's last output is the sandbox wrapper's "ContainerName (regular): …" line at docker container entry, then nothing — not one event — until the 2-hour absolute budget kills the round. Four different runners, two image versions: systemic, not a bad machine. Where exactly the container wedges is still unknown (that needs docker state on the runner); what is certain from the logs is the shape — a wedged sandbox produces NOTHING, and a legitimate run is never silent for long (the fleet's longest tolerated quiet is the review pipeline's 10-minute stream-idle window for thinking phases). Two mitigations, each aimed at a measured half of the damage: - run-agent.mjs gains an idle watchdog (QWEN_IDLE_TIMEOUT_MS, default 20 minutes = 2x that longest legitimate silence): zero output for the window kills the agent with a distinct "idle-timeout … the sandbox likely hung at startup" detail, so the failure comment names the right knob and a hung round costs 20 minutes instead of 120. Polled, not reset-per-chunk — a busy stream should not spend its time re-arming timers. - Both sandboxed jobs reap stale qwen-code-* containers at job start: a budget kill reaps the HOST-side docker client, not the container, so every killed sandbox keeps running on the persistent runner — observed directly when a later leg's container-name counter found qwen-code-0.21.8-0 already occupied and picked -1. One job per runner at a time makes any container alive at job start stale by definition. Tested by executing the real run-agent.mjs end to end with stub agents: the hang shape (one line, then silence) dies at the idle window naming the idle limit, and a slow-but-talking agent that outputs every 400ms across a 1500ms window survives to a clean exit — the test that distinguishes a watchdog from a disguised absolute timer. Mutation- tested, 3 of 3 caught: watchdog disabled, last-output tracking dropped (the disguised-timer regression), cleanup dropped from a job. * Address review round 5: the gate's verdict defects and the reaper's live kill Budget-warning round — the five Criticals from both reviewers, no suggestions (each deferred with a recorded reply). fail_signature: `[^\n]*` in an ERE bracket expression does not mean "rest of line" — in POSIX bracket expressions `\` is literal, so it matched "neither backslash nor the letter n" and truncated every tsc message at its first n. Nearly every real message has an early n ("Cannot find name", "is not assignable"), so distinct same-file failures collapsed into identical signatures and a round-caused failure could be labeled pre-existing, skipping the repair. grep is line-oriented: `.*` is exactly the rest of the line. New fixture: two messages differing only after their first n. Pre-existing verdict: the intersection test mislabeled in both directions. A round that ADDS a diagnostic sharing one normalized line with the baseline was called pre-existing (repair skipped for a round-caused, repairable failure); and `comm -12 | grep -q` under `set -eo pipefail` SIGPIPEs comm (exit 141) once the shared output outruns the pipe buffer, charging true pre-existing failures to the round — the exact 18-minute repair waste the gate exists to kill. Pre-existing now means the round's failing set is a SUBSET of the baseline's, and the difference is captured before testing. New fixture: a round adding a second diagnostic to a failing baseline. Restore failure after the baseline leg: was retryable=true with HEAD still detached at the baseline commit — the repair agent works in that very checkout and does no git recovery, so its commit would land on the baseline and be orphaned. Now rejected non-retryable (reject_fix grows a third arg); the next round starts clean from the trusted checkout. The restoreClash test pins the new semantics. Stale-container reap: the premise "a runner runs one job at a time, so any live qwen-code-* container is stale" holds per runner registration, but the filter queries the docker daemon, which is per host — and this pool runs several registrations on one OS. With per-issue/PR serialization only, a concurrent job's sandbox is a substring match away from `docker rm -f`. The reap now takes only provably-dead containers (--filter status=exited/dead, both jobs) and the comment says why a running one is left alone. Preamble printf: the `\`` escapes sat inside a single-quoted format where backslash is literal, so every pre-existing rejection rendered raw backticks instead of code spans (shellcheck SC2016). Backticks need no escaping there. Also syncs the side-log comment to the dynamic tail_budget it actually renders. Verified: scripts suite 140/140 (was 138; the two new fixtures and the rewritten restoreClash test all fail against the pre-fix script), npm run build / typecheck / lint pass, bash -n clean. * Address review round 6: reap the kill's own orphan, tolerate the reaper * Address review: hang-bound the reaper, unblock the kill path, pin the unpinned arms - Wrap every docker call in the stale-container reap with timeout 30: an alive-but-wedged daemon blocks docker ps indefinitely, and the existing || guards only catch nonzero exits, not hangs (R3-1). - Make the kill-path container removal async in run-agent.mjs: the spawnSync blocked the event loop between SIGTERM and the 10s SIGKILL backstop for up to its 30s timeout — in exactly the wedged-daemon scenario the watchdog exists for. The main flow awaits the removal so the leak warning stays deterministic (R3-6). - Split the pre-existing gate clause for an empty CMP_R: a transient compare-API failure is "never measured", not "measured not-behind", and must not assert the branch's own code is at fault (R3-7). - Swap the timeout breaker's closing remedy to the sandbox investigation when every counted timeout was idle, mirroring the round-level split (R3-11). - Tests: pin the budget kill path separately from the idle kill path (R3-3), parameterize the idle-window parse guard over -1/0/NaN (R3-5), add a stderr-only liveness case (R3-12), pin the strict-subset A/B arm via a baseline-superset fixture knob (R3-15), and pin the breaker's current-round idle increment (R3-18). --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
Released in v0.21.9. |
4 participants
What this PR does
Follow-up to #8663. The
/reviewpass that ran right after #8663 merged surfaced 14 findings that were never addressed (all still unresolved on that PR). This PR handles the substantive ones.#8663's inherited-env denylist closed the
NODE_OPTIONS/NODE_PATHclass but left sibling variables that reach the same #8653 cross-workspace outcome: an untrusted workspace.envis frozen intodaemonRuntimeBaseEnvand distributed to every workspace's session subprocesses. This PR extends the denylist along #8663's own two tiers, hardens the scrub lifecycle against concurrent embedded daemons, and closes the test/diagnostic gaps the review named.Denylist additions
Scrubbed loader tier (
INHERITED_LOADER_ENV_KEYS— scrubbed from the inherited launch env and rejected from every.env/settings.envscope). Pure-injection vars with no legitimate operator-shell use:OPENSSL_CONF— Node's startup crypto init dlopens an attacker-configured OpenSSL engine/provider.sobefore any user code runs.NODE_REPL_EXTERNAL_MODULE— a spawnednodeREPLrequire()s an attacker file at startup.npm_config_node_gyp— npm's shim runs"$npm_config_node_gyp" "$@"verbatim.npm_config_init_module—require()d bynpm init(evennpm init -y).Reject-from-project-
.envtier (PROJECT_ENV_HARDCODED_EXCLUSIONS— rejected from project files, but a value the operator sets in their own shell or home.envis preserved). Vars with legitimate operator-shell use whose only exposed vector is an untrusted project file:SSL_CERT_FILE,SSL_CERT_DIR,CURL_CA_BUNDLE,REQUESTS_CA_BUNDLE,GIT_SSL_CAINFO— siblings of the already-blockedNODE_EXTRA_CA_CERTS; an attacker CA MITMs the token-bearing traffic a session'sgit/npm/pip/curlcalls make.GIT_SSH_COMMAND,GIT_EXTERNAL_DIFF,GIT_CONFIG_GLOBAL/SYSTEM/COUNTand the numberedGIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n>pairs (matched by prefix).core/utils/git-branches.tsalready scrubs exactly these from the repo's own git invocations, so a project.envsetting them contradicts our own model.NODE_GYP_FORCE_PYTHON,npm_config_python,PYTHON— run as the build Python during native-addonnpm install.Concurrency hardening
The daemon's
process.envscrub/restore and the loader-key rejection reporter were process-global with no guard for concurrent embedded daemons in one process — a documented supported config (acp-bridge/src/bridgeOptions.tschildEnvOverrides). The first daemon'sclose()restored loader vars into the sharedprocess.env, re-poisoning a still-live sibling's sessions, and dropped its reporter. The scrub is now reference-counted (acquireInheritedLoaderEnvScrub— snapshot on first acquire, restore only on last release) and the reporter is cleared only when still the active one.Test / diagnostic hardening
settings.envcase-folded hardcoded-exclusion gate (previously only the.envloop's case-fold was pinned).accumulatesassertion is order-independent (no longer depends on a sibling test consuming first).settings.md) updated for all new keys.Deliberately not changed
LD_LIBRARY_PATH,PYTHONPATH, …) and the residualPATH-prefix leak remain the deferred follow-up fix(cli): scrub inherited loader env vars from daemon session subprocesses #8663 already tracks — rejecting them breaks mainstream toolchains.Verification
npm run buildclean;tsc --noEmit,eslint, andprettier --checkclean on all changed files. Affected suites:shared-env-keys.test.ts,environment.test.ts,fast-path.test.ts,daemon-worker.test.ts,run-qwen-serve.test.ts— 458 tests pass. New tests cover every added key at the predicate and.env/settings.envapplication layers, the refcounted scrub (restore does not re-poison while a second holder is live), and the reporter clear-if-current guard.这个 PR 做了什么
#8663 的后续。#8663 合入后紧接着跑的
/review给出了 14 条一直未处理的评审意见（在该 PR 上至今全部 unresolved）。本 PR 处理其中实质性的部分。#8663 的继承环境拒绝列表关闭了
NODE_OPTIONS/NODE_PATH这一类，但遗漏了通向同一 #8653 跨 workspace 结果的同族变量：不受信 workspace 的.env会被冻结进daemonRuntimeBaseEnv并分发到每个 workspace 的会话子进程。本 PR 沿 #8663 自身的两层结构扩展拒绝列表，加固剥离生命周期以应对同进程并发内嵌 daemon，并补齐评审指出的测试/诊断缺口。拒绝列表新增
剥离 loader 层（从继承的启动环境剥离，且在所有
.env/settings.env作用域被拒绝）——无正当登录 shell 用途的纯注入变量：OPENSSL_CONF（启动时 dlopen 攻击者 OpenSSL engine）、NODE_REPL_EXTERNAL_MODULE、npm_config_node_gyp、npm_config_init_module。仅拒绝项目
.env层（从项目文件拒绝，但运维在自己 shell 或 home.env设置的值保留）——有正当 shell 用途、仅经不受信项目文件暴露的变量：TLS 信任锚SSL_CERT_FILE/SSL_CERT_DIR/CURL_CA_BUNDLE/REQUESTS_CA_BUNDLE/GIT_SSL_CAINFO（NODE_EXTRA_CA_CERTS的同族，MITM 会话的 git/npm/pip/curl 携带 token 的流量）；git 命令执行家族GIT_SSH_COMMAND/GIT_EXTERNAL_DIFF/GIT_CONFIG_GLOBAL/SYSTEM/COUNT及编号GIT_CONFIG_KEY_<n>/GIT_CONFIG_VALUE_<n>对（按前缀匹配；core/utils/git-branches.ts已剥离这些）；node-gyp 解释器选择NODE_GYP_FORCE_PYTHON/npm_config_python/PYTHON。并发加固
daemon 对
process.env的剥离/恢复与 loader 键拒绝 reporter 是进程全局的，对同进程并发内嵌 daemon（文档化的受支持配置）无保护。第一个 daemon 的close()会把 loader 变量恢复进共享process.env，重新污染仍存活的同伴会话，并丢掉其 reporter。现改为引用计数（acquireInheritedLoaderEnvScrub——首次 acquire 快照、仅最后一次 release 恢复），reporter 仅在仍是当前活跃者时才清除。测试/诊断加固
钉住 daemon-worker channel 边界剥离的 breadcrumb（不仅是键删除）；钉住快速路径
settings.env的大小写折叠硬编码排除门控；在accumulates断言前排空模块全局 stash 使其与测试顺序无关；settings.md更新所有新键。有意未改动
warn-once 去重按进程生效（移除后再加不再复警）是 R3-4 既定设计；库搜索路径（
LD_LIBRARY_PATH、PYTHONPATH等）与残留的PATH前缀泄漏仍为 #8663 已跟踪的 deferred 项——拒绝它们会破坏主流工具链。