fix(core): strip Qwen-internal daemon secrets from agent-spawned child env

[chinesepowered]

What this PR does

Fixes #6601. Shell subprocesses (and the monitor tool and stdio MCP servers) spawned by Qwen Code inherited the full daemon process.env, including QWEN_SERVER_TOKEN — the serve-daemon bearer credential. An agent-run command like printenv QWEN_SERVER_TOKEN could therefore read an internal secret it should never see.

This adds a small shared sanitizeChildEnv() in packages/core/src/utils/ that removes Qwen-internal daemon/server secrets from an env before it is handed to a child process, and applies it at the four spawn sites that inherit process.env:

• services/shellExecutionService.ts — the child_process fallback and the PTY path
• tools/monitor.ts — the monitor command spawn
• tools/mcp-client.ts — the stdio MCP transport

The denylist is deliberately narrow — only internal bearer tokens:

INTERNAL_SECRET_ENV_VARS = ['QWEN_SERVER_TOKEN', 'QWEN_DAEMON_TOKEN']

Per the maintainer's design guidance on the issue, it does not reuse the desktop BLOCKED_ENV_VARS denylist, and it intentionally does not strip third-party credentials (GH_TOKEN/GITHUB_TOKEN, AWS_*, NPM_TOKEN, …): the shell tool exists to run whatever the user asks, and real workflows (gh, the AWS CLI, npm publish) legitimately depend on inheriting those. Stripping only Qwen-internal secrets is pure upside with no workflow breakage. sanitizeChildEnv is exported from the package root so the desktop denylists can consolidate onto it as a follow-up.

Why it's needed

QWEN_SERVER_TOKEN is the serve-daemon bearer token (packages/cli/src/serve/auth.ts); QWEN_DAEMON_TOKEN is the channel-daemon worker token (the daemon worker already scrubs both from its own process.env). Leaking either to arbitrary agent-run commands is a defense-in-depth credential-exposure gap — filed and confirmed as P1. This PR covers the four spawn sites in the scope agreed on #6601 — the shell tool's PTY and child_process paths, the monitor tool, and the stdio-MCP transport. Two more same-trust-class paths (tools/tool-registry.ts tool-call/discovery spawns and hooks/hookRunner.ts command hooks) also inherit the full env and are intentionally deferred to a follow-up (see Risk & Scope).

Reviewer Test Plan

How to verify

• npx vitest run src/utils/sanitize-child-env.test.ts (6/6): asserts the two internal tokens are removed while PATH, GH_TOKEN, GITHUB_TOKEN, AWS_ACCESS_KEY_ID, NPM_TOKEN, HOME survive; input is not mutated; and a guardrail test pins the denylist to exactly the two internal vars so it can't silently grow to include third-party creds.
• npx vitest run src/services/shellExecutionService.test.ts (132/132): includes a new regression test in both the PTY and child_process describe blocks asserting the spawn env has no QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN, while PATH, GH_TOKEN, and the tool's own QWEN_CODE=1 marker are present.
• npx vitest run src/tools/monitor.test.ts (80/80) and src/tools/mcp-client.test.ts (102/102): unchanged — confirms the one-line env wraps didn't regress those spawn paths.

Evidence (Before & After)

Shell spawn env (printenv QWEN_SERVER_TOKEN in an agent-run command):

• Before: ...normalizePathEnvForWindows(process.env) → child inherits QWEN_SERVER_TOKEN; the command prints the daemon bearer token.
• After: ...normalizePathEnvForWindows(sanitizeChildEnv(process.env)) → QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN absent; PATH, GH_TOKEN, AWS_*, NPM_TOKEN still inherited.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

macOS: the four suites above pass locally (320 tests across the changed files). The shell child env is asserted deterministically via the mocked pty/child_process spawn, so no manual QA is required.

Environment (optional)

Node v24; @qwen-code/qwen-code-core workspace; vitest 3.2.

Risk & Scope

• Main risk or tradeoff: the change removes exactly two Qwen-internal token vars from child envs and nothing else — no third-party credential is stripped, so gh/AWS/npm and similar workflows are unaffected. All existing tests for the four touched spawn paths pass unchanged.
• Deferred to a follow-up (same trust class, not covered here): tools/tool-registry.ts (the toolCallCommand/toolDiscoveryCommand spawns pass no env, inheriting everything) and hooks/hookRunner.ts (command hooks spread full process.env). The identical one-line sanitizeChildEnv(process.env) wrap applies; kept out of this PR to hold scope to the Shell subprocess inherits sensitive environment variables causing credential exposure #6601-agreed sites.
• Not validated / out of scope: broader third-party-credential stripping for the sandbox/MCP infrastructure paths stays where it already lives; consolidating the duplicated desktop BLOCKED_ENV_VARS lists onto the shared util is a suggested follow-up, not done here.
• Breaking changes / migration notes: none.

Linked Issues

Fixes #6601. Implementation follows the design outlined by @doudouOUC on the issue (narrow internal-secrets denylist, applied at the shell/monitor/MCP-stdio spawn sites, third-party creds left inherited).

中文说明

本 PR 的作用

修复 #6601。Qwen Code 启动的 shell 子进程（以及 monitor 工具、stdio MCP 服务）会继承完整的守护进程 process.env，其中包含 QWEN_SERVER_TOKEN——serve 守护进程的 bearer 凭证。因此像 printenv QWEN_SERVER_TOKEN 这样由 agent 执行的命令能读到本不该看到的内部密钥。

本 PR 在 packages/core/src/utils/ 新增一个小的共享 sanitizeChildEnv()，在把 env 交给子进程前移除 Qwen 内部守护/服务密钥，并应用于四处继承 process.env 的 spawn 点：

• services/shellExecutionService.ts——child_process 兜底路径与 PTY 路径
• tools/monitor.ts——monitor 命令 spawn
• tools/mcp-client.ts——stdio MCP 传输

denylist 刻意保持狭窄——仅内部 bearer token：

INTERNAL_SECRET_ENV_VARS = ['QWEN_SERVER_TOKEN', 'QWEN_DAEMON_TOKEN']

按照 issue 上维护者的设计意见，它不复用桌面端的 BLOCKED_ENV_VARS，也不剥离第三方凭证（GH_TOKEN/GITHUB_TOKEN、AWS_*、NPM_TOKEN 等）：shell 工具的存在就是为了运行用户所要求的任何命令，而真实工作流（gh、AWS CLI、npm publish）确实依赖继承这些凭证。仅剥离 Qwen 内部密钥是纯收益、不会破坏工作流。sanitizeChildEnv 从包根导出，便于后续将桌面端 denylist 收敛到它之上。

为什么需要

QWEN_SERVER_TOKEN 是 serve 守护进程 bearer token（packages/cli/src/serve/auth.ts）；QWEN_DAEMON_TOKEN 是 channel 守护 worker token（守护 worker 已在其自身 process.env 中清除二者）。将任一泄露给任意 agent 执行的命令是纵深防御上的凭证暴露缺口——已被确认为 P1。本 PR 覆盖 #6601 商定范围内的四个 spawn 点——shell 工具的 PTY 与 child_process 路径、monitor 工具，以及 stdio-MCP 传输。另有两处同一信任级别的路径（tools/tool-registry.ts 的 tool-call/discovery spawn，以及 hooks/hookRunner.ts 的命令 hook）同样继承完整 env，特意留待后续 PR 处理（见「风险与范围」）。

复核测试计划

如何验证

• npx vitest run src/utils/sanitize-child-env.test.ts（6/6）：断言两个内部 token 被移除，而 PATH、GH_TOKEN、GITHUB_TOKEN、AWS_ACCESS_KEY_ID、NPM_TOKEN、HOME 保留；输入不被修改；并有一个护栏测试将 denylist 固定为恰好两个内部变量，防止其悄然扩张纳入第三方凭证。
• npx vitest run src/services/shellExecutionService.test.ts（132/132）：在 PTY 与 child_process 两个 describe 块中各新增一个回归测试，断言 spawn env 中无 QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN，而 PATH、GH_TOKEN 及工具自身的 QWEN_CODE=1 标记均在。
• npx vitest run src/tools/monitor.test.ts（80/80）与 src/tools/mcp-client.test.ts（102/102）：保持通过——确认单行 env 包装未回归这些 spawn 路径。

证据（修复前后对比）

shell spawn env（agent 执行的命令中 printenv QWEN_SERVER_TOKEN）：

• 修复前：...normalizePathEnvForWindows(process.env) → 子进程继承 QWEN_SERVER_TOKEN；命令打印出守护 bearer token。
• 修复后：...normalizePathEnvForWindows(sanitizeChildEnv(process.env)) → QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN 不再存在；PATH、GH_TOKEN、AWS_*、NPM_TOKEN 仍被继承。

测试环境

系统	状态
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

macOS：上述四个套件在本地通过（改动文件共 320 个测试）。shell 子进程 env 通过 mock 的 pty/child_process spawn 确定性断言，无需人工 QA。

运行环境（可选）

Node v24；@qwen-code/qwen-code-core 工作区；vitest 3.2。

风险与影响范围

• 主要风险或权衡：本改动仅从子进程 env 移除恰好两个 Qwen 内部 token 变量，别无其他——不剥离任何第三方凭证，故 gh/AWS/npm 等工作流不受影响。四处受影响 spawn 路径的既有测试均不变通过。
• 留待后续（同一信任级别，本 PR 未覆盖）：tools/tool-registry.ts（toolCallCommand/toolDiscoveryCommand 的 spawn 未传 env，继承全部）与 hooks/hookRunner.ts（命令 hook 展开完整 process.env）。同样的一行 sanitizeChildEnv(process.env) 包装即可修复；为将范围收敛到 Shell subprocess inherits sensitive environment variables causing credential exposure #6601 商定的站点而未纳入本 PR。
• 未验证 / 范围之外：面向 sandbox/MCP 基础设施路径的更广第三方凭证剥离维持原处；将重复的桌面端 BLOCKED_ENV_VARS 收敛到共享 util 为后续建议项，本 PR 未做。
• 破坏性变更 / 迁移说明：无。

关联 Issue

Fixes #6601。实现遵循 @doudouOUC 在 issue 上给出的设计（狭窄的内部密钥 denylist，应用于 shell/monitor/MCP-stdio spawn 点，第三方凭证保留继承）。

[qwen-code-ci-bot]

Qwen precheck requires maintainer approval before automated triage/review.

Head SHA: 2f2d10f2f38b86e09aac6eda6a97d0e22ca01e01

Reason:

• prompt_injection:run_gh

A maintainer with write access can inspect the PR and manually request a run with @qwen-code /triage or @qwen-code /review. A new push requires a fresh precheck.

[wenshao]

Local verification report

Built and ran this PR locally, driving the real ShellExecutionService / createTransport / HookRunner against real spawned child processes — not the mocked spawns the PR's own tests use. A/B is merge-base 0764276 vs PR head 278aa21d, same build, same machine.

Platform: Linux (x86_64, node v22.22.2, vitest 3.2.4). The PR description marks Linux N/A, so this adds the missing Linux coverage.

Verdict: the fix does what it claims, and the design call is the right one. I'd merge it. Four follow-up notes below, one of which I'd like to see addressed before or right after merge (§1).

---

1. Does it actually fix the leak? Yes — on every path it touches

I exported all seven vars into the parent process, then let the service spawn a real child that reports its own environment.

Both spawn paths behave identically, before and after:

	QWEN_SERVER_TOKEN	QWEN_DAEMON_TOKEN	GH_TOKEN / GITHUB_TOKEN / AWS_ACCESS_KEY_ID / NPM_TOKEN / OPENAI_API_KEY	PATH	QWEN_CODE=1
merge-base	leaked	leaked	inherited	ok	ok
PR head	stripped	stripped	inherited	ok	ok

The stdio-MCP path (mcp-client.ts:2152) behaves the same — verified by spawning a real MCP child that dumps its env (screenshot §4, panel A). The monitor.ts site I verified at the source + emitted-dist level rather than by driving the tool.

The "don't strip third-party credentials" decision holds up in practice: gh, the AWS CLI and npm publish still see everything they need. Given the shell tool exists to run arbitrary user commands, I agree this is the correct scope — a broader denylist would break real workflows for no gain against an attacker who can already run commands.

Also worth noting: sanitizeChildEnv returns a fresh copy, so replacing { ...process.env } with sanitizeChildEnv(process.env) in mcp-client.ts preserves the no-aliasing property that line was relying on.

2. Are the new tests real guardrails? Partly

I reverted only the fix call sites and re-ran the PR's own suites.

• 320/320 green as submitted — matches the description exactly (6 + 132 + 80 + 102).
• Revert shellExecutionService.ts → 2 failures. Those two tests are genuine guardrails.
• Revert monitor.ts and mcp-client.ts → still 182/182 green.

So two of the four patched spawn sites have no regression coverage at all. A future refactor can silently drop either sanitizeChildEnv wrap and CI stays green. Two assertions in the shape of the existing shell ones would close this cheaply.

npx eslint on all six changed files: clean. packages/core builds clean.

3. Threat model — worth stating precisely

I checked how the token gets into the daemon's env in the first place: no production code writes QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN into process.env (only tests do). It arrives because the operator exports it, which is exactly what the serve docs instruct. And run-qwen-serve.ts — the process that actually spawns agent children — never scrubs its own process.env (scrubDaemonWorkerEnv() lives in channel/daemon-worker.ts:655, a different process).

So the realistic scenario is: operator sets the documented env var, starts qwen serve, and every agent-run command inherits the daemon's bearer token for the process lifetime. That's a real gap and this PR closes it for the shell/monitor/MCP paths.

4. What the PR leaves open

(a) hooks/hookRunner.ts:586 still leaks both tokens — with this PR applied. Same package, same { ...process.env, ... } shape, and command hooks fire automatically on tool events with no user confirmation. This looks like the same bug at a site the PR didn't enumerate; a fifth one-line wrap would cover it. Two others in the same family: tools/tool-registry.ts:63/:568 (spawn() with no env at all) and tools/computer-use/client.ts:149 (a second stdio-MCP transport, explicitly opting into full inheritance).

(b) serve/routes/a2ui-action.ts:45 has the identical hole this PR is fixing elsewhere — its SCRUBBED_STDIO_ENV_KEYS contains only QWEN_SERVER_TOKEN, not QWEN_DAEMON_TOKEN. Since the two hold the same secret value (channel-worker-supervisor.ts:348-351 copies the daemon bearer verbatim into QWEN_DAEMON_TOKEN), that set is missing exactly the var this PR added. Related: acp-bridge/src/spawnChannel.ts:269 is a third partial list, and it includes QWEN_CODE_SIMPLE, which the new shared list drops. Good candidates for the consolidation the PR already flags as follow-up.

(c) The denylist is exact-case. On Windows env keys are case-insensitive, but { ...env } produces a plain object, so delete sanitized['QWEN_SERVER_TOKEN'] won't match a Qwen_Server_Token key (panel D). Niche, but this code runs inside normalizePathEnvForWindows(...) — a helper that exists precisely because this repo already hit env-key casing problems on Windows. A case-insensitive delete would be a two-line change. I could not test on Windows.

(d) Defense-in-depth, not a boundary — and the PR is right to say so. With the fix applied, the sanitized child still recovers the token from /proc/<parent>/environ on Linux (panel C), since it runs as the same uid. Worth keeping in mind when closing #6601: this raises the bar meaningfully, but it doesn't make the token unreachable from an agent-run command. The PR body's "defense-in-depth" framing is accurate and shouldn't be upgraded to "leak closed".

Nit

The test plan references src/utils/sanitizeChildEnv.test.ts; the actual file is src/utils/sanitize-child-env.test.ts.

---

Summary: correct, narrowly-scoped, no regressions, verified on Linux against real child processes. §1 (hookRunner) is the one I'd want tracked — it's the same defect the PR is fixing, one file away. §2 (missing coverage on two of four sites) is cheap insurance. The rest can be follow-ups.

中文版

本地验证报告

我在本地构建并运行了本 PR，用真实的子进程驱动 ShellExecutionService / createTransport / HookRunner——而非 PR 自带测试所使用的 mock spawn。A/B 对比为 merge-base 0764276 与 PR head 278aa21d，同一构建、同一机器。

平台：Linux（x86_64，node v22.22.2，vitest 3.2.4）。PR 描述中 Linux 标为 N/A，本报告补上这块覆盖。

结论：修复确实达成了它声称的效果，设计取舍也是对的。我倾向合并。 下面有四点后续建议，其中 §1 希望在合并前后尽快处理。

---

1. 真的修好了吗？在它改动的每条路径上都修好了

我把七个变量导出到父进程，再让 service 真实 spawn 一个子进程来报告自身环境。

两条 spawn 路径的前后表现完全一致：

	QWEN_SERVER_TOKEN	QWEN_DAEMON_TOKEN	GH_TOKEN / GITHUB_TOKEN / AWS_ACCESS_KEY_ID / NPM_TOKEN / OPENAI_API_KEY	PATH	QWEN_CODE=1
merge-base	泄露	泄露	继承	正常	正常
PR head	已剥离	已剥离	继承	正常	正常

stdio-MCP 路径（mcp-client.ts:2152）表现相同——通过真实 spawn 一个会转储自身 env 的 MCP 子进程验证（截图 §4，面板 A）。monitor.ts 这处我是在源码与产物 dist 层面确认的，未实际驱动该工具。

「不剥离第三方凭证」的决策在实践中站得住：gh、AWS CLI、npm publish 仍能拿到所需的一切。鉴于 shell 工具的存在意义就是执行任意用户命令，我认同这个范围——更宽的 denylist 会破坏真实工作流，而对一个已经能执行命令的攻击者并无收益。

另外：sanitizeChildEnv 返回的是全新副本，因此在 mcp-client.ts 中用 sanitizeChildEnv(process.env) 替换 { ...process.env } 保留了该行原本依赖的「不与 process.env 别名共享」性质。

2. 新增测试是真正的护栏吗？只有一半是

我仅回退修复的调用点，再跑 PR 自己的测试套件：

• 按提交状态 320/320 全绿——与描述完全吻合（6 + 132 + 80 + 102）。
• 回退 shellExecutionService.ts → 2 个失败。这两个测试是真护栏。
• 同时回退 monitor.ts 与 mcp-client.ts → 仍然 182/182 全绿。

也就是说，四个被修补的 spawn 点中有两个完全没有回归覆盖。日后重构可以悄悄去掉任一处 sanitizeChildEnv 包装而 CI 仍然全绿。仿照现有 shell 测试补两条断言即可低成本补上。

对六个改动文件跑 npx eslint：干净。packages/core 构建干净。

3. 威胁模型——值得说准确

我核查了 token 最初是如何进入守护进程 env 的：没有任何生产代码把 QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN 写入 process.env（只有测试会）。它之所以存在，是因为运维按 serve 文档的指引导出了它。而真正 spawn agent 子进程的 run-qwen-serve.ts 从不清理自己的 process.env（scrubDaemonWorkerEnv() 位于 channel/daemon-worker.ts:655，属于另一个进程）。

因此现实场景是：运维设置了文档要求的环境变量，启动 qwen serve，于是在该进程生命周期内，每一条 agent 执行的命令都继承了守护进程的 bearer token。这是真实存在的缺口，本 PR 为 shell/monitor/MCP 路径关闭了它。

4. 本 PR 未覆盖的部分

(a) hooks/hookRunner.ts:586 在本 PR 生效后仍然泄露两个 token。 同一个包、同样的 { ...process.env, ... } 形态，而且 command hook 会在工具事件上自动触发、无需用户确认。这看起来就是同一个 bug 出现在 PR 未列举的位置；再加一行包装即可覆盖。同族的还有两处：tools/tool-registry.ts:63/:568（spawn() 完全没有 env 参数）与 tools/computer-use/client.ts:149（第二条 stdio-MCP 传输，显式选择完整继承）。

(b) serve/routes/a2ui-action.ts:45 存在与本 PR 正在修复的完全相同的漏洞——它的 SCRUBBED_STDIO_ENV_KEYS 只含 QWEN_SERVER_TOKEN，不含 QWEN_DAEMON_TOKEN。由于两者承载同一个密钥值（channel-worker-supervisor.ts:348-351 把守护 bearer 原样复制进 QWEN_DAEMON_TOKEN），该集合恰好缺了本 PR 新增的那个变量。相关地，acp-bridge/src/spawnChannel.ts:269 是第三份局部列表，且它包含新共享列表所遗漏的 QWEN_CODE_SIMPLE。这些都是 PR 已标注为后续项的「收敛」工作的好候选。

(c) denylist 区分大小写。 Windows 上环境变量名不区分大小写，但 { ...env } 产生的是普通对象，因此 delete sanitized['QWEN_SERVER_TOKEN'] 匹配不到 Qwen_Server_Token 这样的键（面板 D）。虽属边角，但这段代码正运行在 normalizePathEnvForWindows(...) 内部——而该 helper 的存在恰恰是因为本仓库已经在 Windows 上踩过环境变量大小写的坑。改成大小写不敏感的删除只需两行。我无法在 Windows 上实测。

(d) 这是纵深防御而非安全边界——PR 这样表述是对的。 修复生效后，被净化的子进程仍可在 Linux 上从 /proc/<父进程>/environ 取回 token（面板 C），因为二者同 uid 运行。在关闭 #6601 时值得留意：本 PR 显著抬高了门槛，但并未让该 token 对 agent 执行的命令不可达。PR 描述中「纵深防御」的措辞是准确的，不宜升级为「已封堵泄露」。

小问题

测试计划中写的是 src/utils/sanitizeChildEnv.test.ts，实际文件名为 src/utils/sanitize-child-env.test.ts。

---

总结： 修复正确、范围克制、无回归，并已在 Linux 上针对真实子进程验证。§1（hookRunner）是我最希望被跟踪的一点——同样的缺陷，就在隔壁文件。§2（四个点里两个缺覆盖）是廉价的保险。其余可作为后续项。

[chinesepowered]

Thank you for the exceptionally thorough Linux verification against real spawned children — the A/B on all four paths and the /proc/<parent>/environ note are exactly the right lens. Addressed the two things you flagged for this PR:

§2 — coverage on the two uncovered spawn sites (done, pushed). Added a guardrail test at each site, in the shape of the existing shell ones:

• monitor.test.ts → asserts the spawned monitor's env has neither QWEN_SERVER_TOKEN nor QWEN_DAEMON_TOKEN, while PATH and the QWEN_CODE=1 marker survive.
• mcp-client.test.ts → asserts the stdio transport's env strips both internal tokens while keeping GH_TOKEN.

Both proven as real guardrails the same way you did it — reverting just the sanitizeChildEnv wrap at each site fails the corresponding new test; restoring makes them green. Full files: monitor 81/81, mcp-client 103/103. New head is 92e7036.

Nit — test-plan filename. Fixed in the PR body: src/utils/sanitizeChildEnv.test.ts → src/utils/sanitize-child-env.test.ts.

On the rest, all correct and I'll track them as follow-ups rather than widen this PR:

• §1 hooks/hookRunner.ts:586 — agreed it's the same defect one file over, and command hooks firing without user confirmation makes it arguably the sharper one. It's a natural fast-follow because the one-line fix imports the sanitizeChildEnv helper this PR introduces — so I'll open it immediately once this lands.
• §4(b) a2ui-action.ts SCRUBBED_STDIO_ENV_KEYS missing QWEN_DAEMON_TOKEN, plus the tool-registry.ts / computer-use/client.ts sites and the three partial denylists (spawnChannel.ts dropping QWEN_CODE_SIMPLE) — these are the consolidation the PR already flags; I'd do them as a focused follow-up that unifies onto the shared list, so the QWEN_DAEMON_TOKEN gap and the case-insensitive-delete point (§4c) get fixed in one reviewable place rather than scattered here.
• §4(d) — fully agree; keeping the PR's "defense-in-depth" framing as-is, not upgrading to "leak closed."

[wenshao]

Review

Overview

Adds a shared sanitizeChildEnv() that strips the two Qwen-internal bearer tokens (QWEN_SERVER_TOKEN, QWEN_DAEMON_TOKEN) from the env handed to agent-spawned children, applied at four spawn sites: the shell tool's PTY and child_process paths, the monitor tool, and the core stdio MCP transport. The scope matches the design agreed on #6601 (narrow internal-secrets denylist; third-party credentials like GH_TOKEN/AWS_*/NPM_TOKEN deliberately stay inherited).

Overall: correct, well-scoped, and well-tested at the right observable (the actual spawn-call env). One test-hygiene issue worth fixing before merge, and two spawn sites in the same trust class that this PR leaves uncovered — one of which was listed in the issue's own analysis.

Verified

• sanitizeChildEnv returns a fresh shallow copy and doesn't mutate its input; ordering with normalizePathEnvForWindows is fine (on win32 it copies again; on other platforms it returns the same object, which is already a private copy).
• The mcp-client.ts change preserves the previous { ...process.env } copy semantics, and mcpServerConfig.env still spreads after sanitization, so a server config can deliberately re-provide a var — a reasonable explicit opt-in.
• Nothing downstream re-introduces the tokens: getShellContextEnvVars() only injects session/agent/prompt/trace IDs, and the QWEN_CODE/TERM/pager overlays are unrelated.
• No legitimate child depends on inheriting these vars: the channel worker receives QWEN_DAEMON_TOKEN explicitly (channel-worker-supervisor.ts builds its own env and deletes both tokens first), the daemon worker scrubs both from its own process.env after reading them, and env-snapshot.ts only reports presence. So stripping is behavior-safe.
• Denylist completeness: the two entries are exactly the internal bearer tokens defined in packages/cli/src/serve/channel-worker-env.ts; I didn't find a third.
• All four touched suites pass at the PR head (fresh npm ci worktree): sanitize-child-env 6/6, shellExecutionService 132/132, monitor 81/81, mcp-client 103/103 — 322 total.
• A/B check: reverting just the two shellExecutionService.ts fix lines (back to normalizePathEnvForWindows(process.env)) makes exactly the two new regression tests fail and nothing else; restoring the fix goes back to green. The tests genuinely pin the behavior.

Should fix

1. shellExecutionService.test.ts — the two new tests pollute process.env for every later test in the file. The file's restore pattern is by reference: beforeEach saves originalProcessEnv = process.env and afterEach does process.env = originalProcessEnv. The new tests mutate keys in place (process.env['QWEN_SERVER_TOKEN'] = 'serve-secret', process.env['PATH'] = '/usr/bin', …), so the mutations land on the saved object and survive the restore — after these tests run, PATH=/usr/bin, GH_TOKEN=gh-abc, and both fake tokens persist in the real env for the rest of the worker process. It happens to be benign today (everything is mocked and the sanitizer strips the tokens), but it's order-dependent fragility, and the file already has a convention for this — replacement, as in setupConflictingPathEnv:

   process.env = {
     ...originalProcessEnv,
     QWEN_SERVER_TOKEN: 'serve-secret',
     QWEN_DAEMON_TOKEN: 'daemon-secret',
     GH_TOKEN: 'gh-abc',
     PATH: '/usr/bin',
   };

   (The monitor test does this correctly with try/finally, and the mcp-client test already uses the replacement pattern — it's only the two shell tests.)

Follow-up gaps (non-blocking, but worth deciding on the record)

2. tool-registry.ts still leaks both tokens. The issue's stage-2 analysis listed five affected paths including this one; the PR covers four. Both spawn(callCommand, [this.toolName]) (~L63) and the discovery spawn (~L568) pass no env option, so the child inherits the full process.env implicitly. These run user-configured toolCallCommand/toolDiscoveryCommand — the same trust class as a stdio MCP server. The fix is the same one-liner ({ env: sanitizeChildEnv(process.env) }). If it's deliberately deferred, the PR text should say so — the current "the other child-process spawn paths already sanitize; the shell/monitor/stdio-MCP paths were the outliers" isn't accurate for this path.

3. hooks/hookRunner.ts (~L587) spreads full process.env into user-configured hook commands. Hooks run automatically on tool events and are configurable at workspace scope, so this is arguably closer to the MCP-stdio trust class than the shell tool is. Outside the scope the maintainer enumerated, so fine as a follow-up — flagging so it doesn't get lost.

   (The remaining env-inheriting spawns — computer-use client, git worktree service, browser launcher, file-search crawler — launch Qwen's own infrastructure or trusted binaries; leaving them is reasonable.)

Nits

• The INTERNAL_SECRET_ENV_VARS strings duplicate the canonical QWEN_SERVER_TOKEN_ENV/QWEN_DAEMON_TOKEN_ENV constants in packages/cli/src/serve/channel-worker-env.ts. The dependency direction (cli → core) means core can't import them today, but the suggested follow-up consolidation could move the canonical constants into core so cli/acp-bridge/desktop all reference one source of truth.
• The guardrail test pinning the denylist to exactly two entries is a nice touch — it turns any future scope creep into an explicit, reviewable test change.

[chinesepowered]

Thanks for the second pass and the completeness check on the denylist. Addressed both actionable points:

Should-fix — process.env pollution in the two shell tests (done, 2f2d10f). Switched both from in-place mutation to the replacement pattern (process.env = { ...originalProcessEnv, … }), matching setupConflictingPathEnv. Full file still 132/132; the leak into later tests is gone.

Follow-up gap #2 — PR text accuracy. You're right, the "other spawn paths already sanitize" line was wrong for tool-registry.ts. Corrected the description: it now states the PR covers the four #6601-agreed sites and explicitly lists tools/tool-registry.ts (tool-call/discovery spawns with no env) and hooks/hookRunner.ts (command hooks) as same-trust-class paths intentionally deferred to a follow-up, rather than implying they're already covered.

On the deferred paths themselves: hookRunner.ts (§3) is the one I'll open right after this merges — its fix imports the sanitizeChildEnv helper this PR introduces, so it's naturally a fast-follow. I'll fold tool-registry.ts (both spawn sites) into that same follow-up since it's the identical one-liner and the same trust class. The channel-worker-env.ts constant-consolidation nit (moving the canonical QWEN_*_TOKEN_ENV into core so cli/acp-bridge/desktop share one source) I'll keep as the separate consolidation PR the body already flags, to avoid widening the follow-up into a cross-package refactor.

Nothing else changed at the four covered sites — the fix commits are untouched, so your A/B verification of the head still holds.

[doudouOUC]

⚠️ Downgraded from Approve to Comment: CI failing: review-pr, review-config. Reviewed.

— qwen3.7-max via Qwen Code /review

[wenshao]

✅ Local verification — real build & tests

I built and ran this PR from source in an isolated worktree off the PR head (2f2d10f) on Linux / Node v22.22 / vitest 3.2.4. Beyond re-running the suites in the test plan, I added a RED/GREEN negative control and a real (nothing-mocked) child-process E2E so the evidence discriminates the fix rather than just showing "green is green". Verdict: verified — the fix does exactly what it claims, and reverting it reproduces the leak.

1 · Unit suites from the PR test plan — all green

Suite	Result
utils/sanitize-child-env.test.ts	6 / 6 ✓
services/shellExecutionService.test.ts	132 / 132 ✓
tools/monitor.test.ts	81 / 81 ✓
tools/mcp-client.test.ts	103 / 103 ✓
Total	322 / 322 ✓

2 · RED negative control — proves the tests catch the bug

Surgically reverting just the fix (sed 's/sanitizeChildEnv(process.env)/process.env/' on the three spawn-site files) makes all four spawn-site sanitization tests fail with expected 'serve-secret' to be undefined, while the util's own 6 tests still pass — i.e. the tests genuinely discriminate the change.

3 · Real child-process E2E — the actual leak, and its fix

The unit tests assert against a mocked spawn. To confirm real behavior I drove ShellExecutionService.execute() with nothing mocked — a genuine shell is spawned and the agent-run command echoes the env it inherited. Both the PTY and child_process paths behaved identically:

• Before (fix reverted): the command reads back the live daemon tokens — SERVER=[LEAKED_SERVER_TOKEN_…], DAEMON=[LEAKED_DAEMON_TOKEN_…].
• After (this PR): SERVER=[] / DAEMON=[], while GH_TOKEN, QWEN_CODE=1, and PATH are all preserved.

4 · CI gates on the changed files

• eslint --max-warnings 0 → exit 0 (all 6 changed files)
• tsc --noEmit --strict on the new util → exit 0

Minor note for the author (non-blocking)

The test plan lists monitor.test.ts as 80/80 and mcp-client.test.ts as 102/102 "unchanged", but the 2nd commit adds a sanitization test to each, so the real counts are 81 and 103. Just a stale number in the description — the added coverage is correct and welcome.

The deferred scope (tools/tool-registry.ts, hooks/hookRunner.ts) is clearly called out in the PR and is a reasonable follow-up; the narrow internal-only denylist with the guardrail test is the right call. LGTM from my side.

中文说明

✅ 本地验证 —— 真实构建与测试

我在独立 worktree 中基于 PR head（2f2d10f）从源码构建并运行了本 PR，环境为 Linux / Node v22.22 / vitest 3.2.4。除了复跑测试计划中的套件外，我还补做了 RED/GREEN 反向对照 和一个 真实（无任何 mock）的子进程 E2E，以便证据能够真正区分「是否修复」，而非仅仅「绿就是绿」。结论：已验证 —— 修复完全符合其声明，且回退修复即可复现泄露。

1 · 测试计划中的单测套件 —— 全部通过

套件	结果
utils/sanitize-child-env.test.ts	6 / 6 ✓
services/shellExecutionService.test.ts	132 / 132 ✓
tools/monitor.test.ts	81 / 81 ✓
tools/mcp-client.test.ts	103 / 103 ✓
合计	322 / 322 ✓

2 · RED 反向对照 —— 证明测试确实能抓到该缺陷

仅对修复做外科式回退（对三个 spawn 站点文件执行 sed 's/sanitizeChildEnv(process.env)/process.env/'）后，四个 spawn 站点的清理测试全部失败，报错均为 expected 'serve-secret' to be undefined；而 util 自身的 6 个测试仍然通过 —— 即这些测试确实能区分本次改动，而非恒绿。（见上方第一张截图）

3 · 真实子进程 E2E —— 真实的泄露与其修复

单测断言的是被 mock 的 spawn。为确认真实行为，我以 完全不 mock 的方式驱动了 ShellExecutionService.execute() —— 真正 spawn 一个 shell，由 agent 执行的命令回显它继承到的环境变量。PTY 路径与 child_process 路径表现一致：

• 修复前（回退修复）： 命令能读回存活的守护 token —— SERVER=[LEAKED_SERVER_TOKEN_…]、DAEMON=[LEAKED_DAEMON_TOKEN_…]。
• 修复后（本 PR）： SERVER=[] / DAEMON=[]，同时 GH_TOKEN、QWEN_CODE=1、PATH 均被保留。

（见上方第二张截图）

4 · 改动文件上的 CI 关卡

• eslint --max-warnings 0 → 退出码 0（全部 6 个改动文件）
• 对新 util 执行 tsc --noEmit --strict → 退出码 0

给作者的小提示（不阻塞合并）

测试计划将 monitor.test.ts 记为 80/80、mcp-client.test.ts 记为 102/102 且标注「unchanged」，但第 2 个 commit 实际上给二者各新增了一个清理测试，因此真实数量是 81 与 103。只是描述里的数字略有陈旧 —— 新增覆盖本身是正确且值得肯定的。

被推迟的范围（tools/tool-registry.ts、hooks/hookRunner.ts）在 PR 中已明确说明，作为后续跟进是合理的；采用狭窄的「仅内部密钥」denylist 并配合护栏测试是正确的取舍。在我这边 LGTM。

---

🤖 Generated with Claude Code — Claude Opus 4.8 (1M context)

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual, with a clear reviewer test plan.

Problem: Observed and confirmed. Issue #6601 is a P1 security bug with a clear reproduction — printenv QWEN_SERVER_TOKEN in an agent-run shell command leaks the serve-daemon bearer token. The daemon worker already scrubs both tokens from its own process.env, but the four spawn sites in core (shell PTY, shell child_process, monitor, stdio MCP) still pass the full env through. This is a real credential-exposure gap, not theoretical hardening.

Direction: Aligned. Stripping Qwen-internal daemon secrets from child process env is a defense-in-depth measure that follows the same pattern already used by the daemon worker (daemon-worker.ts deletes both tokens) and the desktop sandbox (BLOCKED_ENV_VARS). The narrow denylist (only QWEN_SERVER_TOKEN and QWEN_DAEMON_TOKEN) is the right call — third-party creds like GH_TOKEN and AWS_* are legitimately needed by shell workflows.

Size: 57 production lines (5 files) + 187 test lines (4 files) = 244 total. Well within bounds. Core paths touched (packages/core/src/**), but the change is a focused security fix, not a structural refactor.

Approach: The scope feels right. One shared utility (sanitizeChildEnv) + four one-line wraps at the spawn sites is the minimal change that solves the problem. The deferred sites (tool-registry.ts, hookRunner.ts) are documented and reasonable to split out. The guardrail test pinning the denylist to exactly two entries is a nice touch — prevents silent scope creep. No unrelated changes or drive-by refactors in the diff.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓——所有必填段落齐全，双语，附有清晰的复核测试计划。

问题： 已观测并确认。Issue #6601 是一个 P1 安全 bug，有明确复现——在 agent 执行的 shell 命令中运行 printenv QWEN_SERVER_TOKEN 会泄露 serve 守护进程的 bearer token。守护 worker 已在其自身 process.env 中清除两个 token，但 core 中的四个 spawn 点（shell PTY、shell child_process、monitor、stdio MCP）仍然透传完整 env。这是真实的凭证暴露缺口，而非理论性加固。

方向： 对齐。从子进程 env 中剥离 Qwen 内部守护密钥是纵深防御措施，遵循守护 worker（daemon-worker.ts 删除两个 token）和桌面端沙箱（BLOCKED_ENV_VARS）已有的相同模式。狭窄的 denylist（仅 QWEN_SERVER_TOKEN 和 QWEN_DAEMON_TOKEN）是正确选择——GH_TOKEN、AWS_* 等第三方凭证是 shell 工作流正常所需的。

规模： 57 行生产代码（5 个文件）+ 187 行测试代码（4 个文件）= 共 244 行。远在限制之内。触及核心路径（packages/core/src/**），但改动是聚焦的安全修复，非结构性重构。

方案： 范围合理。一个共享工具函数（sanitizeChildEnv）+ 四处一行包装是最小的解决方案。延迟处理的站点（tool-registry.ts、hookRunner.ts）已记录在案，拆分合理。将 denylist 固定为恰好两个条目的护栏测试是亮点——防止静默范围扩张。diff 中无无关改动或顺手重构。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 2f2d10f2f38b86e09aac6eda6a97d0e22ca01e01 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Independent proposal: given "shell subprocesses inherit QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN", I'd create a small shared utility in packages/core/src/utils/ that strips exactly those two vars from a shallow copy of process.env, wrap it at the four spawn sites (shell PTY, shell child_process, monitor, stdio MCP), export it from the package root, and add unit + regression tests. Keep the denylist narrow — no third-party creds.

Comparison with the diff: the PR matches this proposal almost line-for-line. The implementation is clean and minimal:

• sanitize-child-env.ts (45 lines): well-documented, exports both the denylist constant and the function. Shallow-copies the input, deletes the two keys, returns the copy. No mutation of the source.
• Four one-line wraps at the spawn sites — each replaces process.env (or { ...process.env }) with sanitizeChildEnv(process.env) in the existing env spread. The mcp-client.ts change is equivalent ({ ...process.env } → sanitizeChildEnv(process.env) — both shallow-copy, plus sanitization).
• Exported from index.ts in alphabetical order, following the existing pattern.
• Tests: 6 utility tests (strips secrets, preserves third-party creds, no mutation, fresh object, no-op, guardrail pinning the denylist to exactly 2 entries) + 4 regression tests at the spawn sites (PTY, child_process, monitor, MCP stdio).

No critical blockers. No AGENTS.md violations. File naming (kebab-case.ts), colocated tests, ESM imports — all follow conventions. The guardrail test that pins INTERNAL_SECRET_ENV_VARS to exactly ['QWEN_DAEMON_TOKEN', 'QWEN_SERVER_TOKEN'] is a good safeguard against silent scope creep.

Unit tests (all pass on the PR branch):

Suite	Result
sanitize-child-env.test.ts	6/6 ✅
shellExecutionService.test.ts	132/132 ✅
monitor.test.ts	81/81 ✅
mcp-client.test.ts	103/103 ✅

Real-Scenario Testing

Set QWEN_SERVER_TOKEN=test-secret-token-12345 and QWEN_DAEMON_TOKEN=test-daemon-secret-67890 in the environment, then asked the agent to run printenv QWEN_SERVER_TOKEN.

Before (installed qwen v0.20.0 — without fix)

$ qwen -p 'Run this exact shell command and show me the raw output: printenv QWEN_SERVER_TOKEN' --yolo

Raw output:

test-secret-token-12345

⚠️  Note: this is a secret token now visible in the conversation transcript.
If this session is logged or shared, consider rotating the token.

The daemon bearer token leaks to the agent-run command. ❌

After (this PR via npm run dev — with fix)

$ npm run dev -- -p 'Run this exact shell command and show me the raw output: printenv QWEN_SERVER_TOKEN' --yolo

`QWEN_SERVER_TOKEN` is not set in the current environment — `printenv` returned
empty output with exit code 1, which indicates the variable doesn't exist.

The token is stripped — printenv finds nothing. ✅

The fix works as advertised: internal daemon secrets are removed from the child env while the shell command still runs normally.

中文说明

代码审查

独立方案： 针对"shell 子进程继承 QWEN_SERVER_TOKEN/QWEN_DAEMON_TOKEN"的问题，我会在 packages/core/src/utils/ 创建一个小的共享工具函数，从 process.env 的浅拷贝中精确剥离这两个变量，在四个 spawn 点（shell PTY、shell child_process、monitor、stdio MCP）包装它，从包根导出，并添加单元测试和回归测试。denylist 保持狭窄——不剥离第三方凭证。

与 diff 的比较： PR 几乎逐行匹配此方案。实现干净且最小化：

• sanitize-child-env.ts（45 行）：文档完善，导出 denylist 常量和函数。浅拷贝输入，删除两个 key，返回拷贝。不修改源对象。
• 四处一行包装——每处将 env 展开中的 process.env（或 { ...process.env }）替换为 sanitizeChildEnv(process.env)。
• 按字母顺序从 index.ts 导出，遵循既有模式。
• 测试：6 个工具函数测试 + 4 个 spawn 点回归测试。

无关键阻塞项。无 AGENTS.md 违规。文件命名、共置测试、ESM 导入均符合规范。将 denylist 固定为恰好两个条目的护栏测试是防止静默范围扩张的良好保障。

单元测试（PR 分支全部通过）： 共 322 个测试通过。

真实场景测试

在环境中设置 QWEN_SERVER_TOKEN=test-secret-token-12345，然后让 agent 执行 printenv QWEN_SERVER_TOKEN。

• 修复前（已安装 qwen v0.20.0）：token 泄露到 agent 执行的命令中 ❌
• 修复后（本 PR 通过 npm run dev）：token 被剥离，printenv 找不到任何内容 ✅

修复按预期工作：内部守护密钥从子进程 env 中移除，shell 命令仍正常运行。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 2f2d10f2f38b86e09aac6eda6a97d0e22ca01e01 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — clean security fix, verified end-to-end, would merge without hesitation.

This is exactly the kind of PR you want to see for a P1 credential-exposure bug: narrow scope, clear reproduction, minimal code, comprehensive tests, and a before/after that proves it works. The sanitizeChildEnv utility is 15 lines of logic, the four spawn-site wraps are one line each, and the guardrail test pins the denylist so it can't silently grow. The deferred sites (tool-registry.ts, hookRunner.ts) are documented and reasonable to split out.

The before/after tmux test confirms the fix: printenv QWEN_SERVER_TOKEN leaks the bearer token on v0.20.0, and returns empty on the PR branch. All 322 unit tests pass. No regressions in the four touched spawn paths.

If I had to maintain this in six months, I'd thank the author — the code is self-documenting, the denylist is pinned, and the utility is reusable for the follow-up sites.

中文说明

置信度：5/5 ——干净的安全修复，端到端验证通过，毫不犹豫地合并。

这正是 P1 凭证暴露 bug 应有的 PR 样子：范围窄、复现清晰、代码最小化、测试全面、before/after 证明有效。sanitizeChildEnv 工具函数仅 15 行逻辑，四处 spawn 点包装各一行，护栏测试固定 denylist 防止静默扩张。延迟处理的站点已记录在案，拆分合理。

before/after tmux 测试确认修复有效：v0.20.0 上 printenv QWEN_SERVER_TOKEN 泄露 bearer token，PR 分支上返回空。全部 322 个单元测试通过，四处受影响 spawn 路径无回归。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 2f2d10f2f38b86e09aac6eda6a97d0e22ca01e01 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[chinesepowered]

Thanks for the merge! As promised, the follow-up is up: #7527 routes the three remaining agent-launched child processes through sanitizeChildEnv — hooks/hookRunner.ts (spreads ...process.env) and the two tools/tool-registry.ts spawns for the tool-call and tool-discovery commands, which pass no env option at all and so inherit implicitly. Same narrow denylist, no changes to sanitizeChildEnv itself, with fail-before/pass-after tests on both files.