feat(ipc): authenticate cross-session inbox connections with per-session tokens - #10636
Conversation
E2E test reportEnvironment: Linux, 1. Registry record publishes the credential (mode 600) 2. Wrong token → silent drop, no delivery, no receipt The injector connected, sent an auth line with a 64-hex token of all 3. Correct token → frame admitted, gate policy applied, receipt authenticated back Receiver was in auto mode and the injected frame asserted no mode class, so the mode-parity default holds it — and the 4. Unit coverage (all against real UNIX sockets where applicable)
5. Known limits of this run
🤖 Generated with Claude Code |
|
Thanks for the PR! Template looks good ✓ Problem: not a bugfix — this is the next hardening step for the experimental cross-session messaging inbox. Today its entire access control is filesystem permissions, and that is a real, documented limitation: it cannot carry over to a permissionless transport (named pipes), the socket path is guessable by PID, and the inbox cannot safely open to the session's own child processes without connection-level auth. For a design-driven feature PR that is the right shape of problem, and the committed design doc scopes what is in and out. Direction: aligned. Cross-session messaging is an actively invested area (the reference product's changelog shows continued work on it), and connection-level auth is the stated prerequisite for the named-pipe transport this design targets. Experimental flag, off by default, no registry schema bump. This is also the author's own feature area (#10542). Size: touches core paths ( Approach: the scope looks right — every edit serves the stated goal: token generation and publication, first-line authentication, reply tokens for receipts, env-var injection, stripping the token from Risk: no elevated risk signals (no revert-prone paths touched). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ **问题：**这不是 bug 修复，而是实验性跨会话消息 inbox 的下一步加固。目前它的全部访问控制只有文件权限，这是一个真实且有据可查的局限：无法迁移到无权限语义的传输（命名管道）、socket 路径可按 PID 猜测、没有连接级认证就无法向会话自身的子进程安全开放 inbox。对设计驱动的特性 PR 来说这是正确形态的问题，且附带了划定范围的已提交设计文档。 **方向：**对齐。跨会话消息是持续投入中的领域（参考产品的 changelog 也在持续改进它），连接级认证是本设计所面向的命名管道传输的既定前提。实验开关、默认关闭，注册表 schema 不升版。这也是作者自己的特性领域（#10542）。 **规模：**触及核心路径（ **方案：**范围合理——每处改动都服务于既定目标：令牌生成与发布、首行认证、回执用的回复令牌、环境变量注入、 **风险：**无升级风险信号（未触及易回退路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI read the full diff against the base and walked every consumer of the changed APIs. No blockers — this is careful work. What I verified rather than assumed:
One observation, not blocking: exporting The handshakesequenceDiagram
participant P1 as Sender session
participant P2 as Session registry
participant P3 as Receiver inbox
participant P4 as Inbound gate
P1->>P2: read peer record
P2-->>P1: ipcPath and ipcToken in one read
P1->>P3: connect - auth line, then user frame with replyToken
alt token matches
P3->>P4: frame admitted to the gate
P4-->>P1: receipt authenticated with replyToken
else first line fails auth
P3--xP1: connection dropped, nothing parsed
end
Files changed (18)
TestingPer triage rules I do not build or run PR code; the evidence below is the PR's own CI (read via the API) plus static verification of every claim the diff makes about existing behavior. The three red checks share one root cause, and it fires during This is not caused by this PR — it is a stale base:
Not verified here — named so it can be settled: the CI run never reached the peer-messaging suites, so nothing in it pins the behavioral claim (a tokenless connection is dropped before the gate sees anything; a correct-token frame still flows through the mode-parity policy; receipts authenticate back with The author posted a separate E2E report in this thread — attributed as the author's claim, not independently re-run: live dev-build session on Linux, registry record carried 中文说明代码审查逐行读了完整 diff，并走查了所有被改动 API 的使用方。没有阻塞项——实现很细致。以下是我实际核验过（而非想当然）的点：
一个非阻塞观察：导出 测试按审查规则，我不构建或运行 PR 代码；以上证据来自 PR 自身的 CI（经 API 读取）以及对 diff 所作行为声明的静态核验。 三个红色检查同根同源，且在 （错误同上文英文：SDK 浏览器 daemon 捆包 221287 字节，超出 221184 硬上限。） 这不是本 PR 造成的——是基线过旧：
（CI 结果表见英文部分机器可读区域；绿色：分类、桌面壳、密钥扫描、依赖 CVE 审计；红色：Linux 单测、集成测试（no-AK）、web-shell E2E 冒烟；跳过：macOS/Windows 单测与 CLI 集成。） 此处未验证、点名以便补上：本次 CI 根本没跑到跨会话消息相关套件，因此其中没有任何东西钉住行为性声明（无令牌连接在闸门前被丢弃；正确令牌的帧仍按模式对等策略流转；回执以 作者在本帖另附了 E2E 报告——按作者自述引用、未在此独立复跑：Linux 真实 dev 构建会话，注册记录携带 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review, but the branch's CI is red on a stale base that Stepping back: this is what a good incremental security change looks like. The design doc names what is in and out, the wire change is one admission line, every receipt path carries the reply token, and the tests pin the nasty cases (terminal refusal, frame-before-auth, old-inbox compatibility, registry round-trip). My independent proposal — random token in the 0600 record, first-line auth, constant-time compare, reply tokens for receipts, strip from tool output — matches what landed; I did not find a simpler path it missed. The one thing I'd want a human to keep an eye on as the feature matures is the exported env token (every child process holds a working credential); the PR is honest that this is by design and not new exposure under the same-uid model, so it is not blocking here. Why I'm not approving yet: the reviewed commit's CI is red. All three failures are the stale-base artifact documented above (SDK bundle-size budget, fixed on 中文说明置信度：3/5——审查本身干净，但该分支的 CI 因其过旧的基线而红，而 退一步看：这是一次规范的增量安全改动。设计文档写明了范围内外，协议变更只有一行准入认证，所有回执路径都携带回复令牌，测试钉住了刁钻场景（终态拒绝、认证前发帧、旧 inbox 兼容、注册表往返）。我独立设想的方案——0600 记录中的随机令牌、首行认证、常量时间比较、回执用回复令牌、工具输出剥离令牌——与落地实现一致，没有发现被遗漏的更简路径。唯一希望在特性成熟过程中有人持续关注的，是导出的环境变量令牌（每个子进程都持有可用凭据）；PR 对此坦诚——属设计如此，且在同 uid 模型下不构成新暴露，因此在本 PR 中不构成阻塞。 为何暂不批准：被审查提交的 CI 是红的。三个失败全部是上文记录的基线过旧所致（SDK 捆包体积预算—— — Qwen Code · qwen3.8-max Reviewed at |
…ion tokens Each peer-messaging inbox now generates a random token, published in the session's 0600 registry record beside the socket address, and requires it on the first line of every connection before any frame is read. This narrows "can reach the socket path" to "can read this session's registry record", is the foundation a permissionless transport (Windows named pipes) needs, and lets a session export QWEN_CODE_MESSAGING_SOCKET/_TOKEN so its own child processes can inject messages back through the same inbound gate. User frames carry a replyToken so delivery receipts authenticate on the way back. Receipts, discovery, gate policy and the misaddressed path are unchanged for same-build sessions; a pre-token sender's frames to a new inbox are dropped (documented experimental break), while a new sender still reaches a pre-token inbox. qwen sessions ps --json strips the token — it is a credential, not data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5JUqFcorwYzUSpH7nxqoT
f673bf5 to
87f8ae7
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 from 0 |
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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 1/100) — it will retry on the next scan. Autofix round summary — PR #10636One commit this round: Feedback dispositions[Critical] R1-8 (rc:3896555421) — FIXEDThe env-export contract was never invalidated on any no-inbox path: a process inheriting
Witnesses (all three scrub sites mutation-probed): bind-failure case in 中文说明🤖 AutoFix 在得出结论之前遇到验证门错误（第 1/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33418628581 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 2/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33419595292 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Autofix stop — PR #10636 (same-run verification repair)BlockerThe deterministic gate rejected commit The failure is host CPU contention on the shared ECS runner, not a defect in Evidence that the rejection is contention, not code
中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 Autofix 停止 — PR #10636（同轮验证修复）阻塞点确定性门禁拒绝了提交 该失败是共享 ECS 运行主机上的 CPU 争用所致，并非分支缺陷。只要不与饱和的主机争抢资源，这棵代码树上所有可运行的检查都能通过；而在范围内的代码修改无法解决共享主机负载问题，因此本轮按客观停止规则停止，不再提交。按照同轮修复规则，现有提交 证据：拒绝源于资源争用，而非代码
Run log: https://github.com/QwenLM/qwen-code/actions/runs/33447209481 🧠 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.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/ipc/peer-directory.ts:45 — [probe] D2-1 the "Never printed" invariant on ipcToken has no token-bearing test — a projection leak into list_agents llmContent would ship green
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 4/100) — it will retry on the next scan. Round summary — PR #10636 (peer-messaging inbox auth)Commit: This round ran under a budget warning (the previous round exhausted its time budget), so it implements the Critical fix and the seven small witnesses, and defers the single large item (R1-3) to the next round. 8 of 9 inline findings are resolved in code; 1 is deferred with a thread reply. Feedback dispositions
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误（第 4/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33447925770 🧠 Handled by Qwen Code · model/模型 |
qqqys
removed
the
autofix/takeover
…children Addresses review rounds 1-2 on QwenLM#10636. R1-8 (Critical). A session that inherits QWEN_CODE_MESSAGING_SOCKET / QWEN_CODE_MESSAGING_TOKEN but binds no inbox of its own passed the ancestor's pair straight down to its children: the feature-off branch, a registration that never completes, a failed bind, and headless `-p` all left the inherited values in place. A hook following the documented injection pattern then authenticated to the ANCESTOR's inbox and, under the default policy, landed its message in the wrong session's context while reporting success. The pair is now dropped at the top of `main()`, before anything can be spawned, and again before the bind in `PeerMessaging.start` so no future early return can reintroduce the leak; the success path re-exports this session's own pair as before. The two constants and the scrub moved to a leaf module (`peerMessaging/env.ts`) so the startup path does not pull in the messaging stack to call it; `peer-messaging.ts` re-exports them. R1-6, R1-7 (docs). The injection example hard-coded a msgId, which the inbound gate tombstones for the session's life, so a recurring hook was delivered once and silently deduplicated afterwards — it now mints a fresh id and says why. The `ipcToken` field doc claimed a pre-token record's inbox "refuses frames"; it requires no token and admits everything, so the doc named the wrong lossy direction. R1-1..R1-5, R1-9 (test coverage). Each gap was confirmed by mutation against this branch, and every test below was verified to fail on the mutant and pass without it: - R1-1 startInteractiveUI: the token-forwarding callback, which could be narrowed to one parameter and still type-check. - R1-2 config: ipcToken on the initial patch and on the fd-pressure retry, which `toEqual` could not distinguish from undefined. - R1-3 a held receipt carries replyToken, not only the misaddressed one. - R1-4 the generated token's 64-hex shape, previously never executed because every test injects the seam. - R1-5 a wrong-LENGTH token, previously untested in either direction. - R1-9 both leak directions of the per-connection auth state: one refusal must not brick the inbox, one admission must not admit the next. No production change beyond the scrub and the two doc corrections.
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 5/100) — it will retry on the next scan. Round summary — PR #10636 (address-review)Commit: This round ran under an active budget warning (the previous round exhausted its time budget before finishing anything), so the batch was bounded per the round rules: the Critical first, then the cheap findings, with the single most involved finding (R1-3) explicitly deferred to the next round via Finding dispositions
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误（第 5/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33464671357 🧠 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.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/peerMessaging/peer-messaging.ts:380 — [review] D3-1 close() hand-writes the pair-removal the new clearInheritedPeerMessagingEnv() helper performs
Convergence: round 3 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 9 (0 new). Findings keep coming back to the same files: docs/users/features/commands.md (findings in round 1; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 9 条（其中 0 条首次提出）。发现反复回到同一批文件：docs/users/features/commands.md（第 1 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
Addresses R3-1 (Critical) and D3-1 from review round 3 on QwenLM#10636. R3-1. The scrub added last round sits in `main()`, but `runCliEntry` dispatches routes that spawn children without ever reaching it: the managed-npm-update branch, and the `serve` and `mcp` fast paths. The concrete leak: session A binds an inbox and exports its socket/token pair; the user accepts an auto-update; `handleAutoUpdate` spawns the detached update child with the full environment; that child takes the managed branch, which deletes its own marker and the guard token but not the messaging pair; `installManagedNpmUpdate` then spawns npm the same way — so the installed package's lifecycle scripts inherit a live socket address and a valid token for a running session, and can inject frames into it through the pattern this PR documents. Moved to the top of `runCliEntry`, beside the QWEN_CODE_EXTERNAL_TOOL_GUARD_TOKEN precedent that exists for this exact class ("before any other subcommand handler can start a child process"). That one needs a serve carve-out; this one does not — no route consumes the pair, and a session that binds its own inbox re-exports it from `PeerMessaging.start`. The `main()` and `PeerMessaging.start` scrubs stay as idempotent belt-and-suspenders, which `tryRunServeFastPath` returning false and falling through to `main()` relies on. D3-1. `close()` hand-wrote the same two deletes; it calls the helper now, so the pair has one removal site. Two cli.test.ts cases, both verified to fail with the new scrub removed: one drives the managed-update route and asserts the pair is gone by the time `installManagedNpmUpdate` runs, the other covers the `serve` and `mcp` fast paths.
qqqys
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 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
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): "agent 1a": executing the new cli.test.ts tests to confirm green — the workspace-wide npm run build that packages/cli vitest runs require (core/acp-bridge/channels/web-sh…; "agent 6a": live mutation run of the two new tests in packages/cli/src/cli.test.ts — blocked by the fresh-worktree vitest build-prerequisite guard (requires npm run buil….
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 1a"：executing the new cli.test.ts tests to confirm green — the workspace-wide npm run build that packages/cli vitest runs require (core/acp-bridge/channels/web-sh…；"agent 6a"：live mutation run of the two new tests in packages/cli/src/cli.test.ts — blocked by the fresh-worktree vitest build-prerequisite guard (requires npm run buil…。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 1/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 1/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33501673191 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 2/100) — it will retry on the next scan. Address-review summary — PR #10636, round 5Commit: Addressed this round (implemented)
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误（第 2/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33502682175 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
Local verification report — PR #10636 @
|
| Claim | Result |
|---|---|
record carries ipcPath + 64-hex ipcToken, mode 600 |
✅ /^[0-9a-f]{64}$/, file 600, sessions/ dir 700, socket 600, socket dir 700 |
| raw user frame with no auth line | ✅ nothing delivered, nothing held; --debug: dropping a connection whose first line did not authenticate |
| wrong token then a user frame | ✅ same — dropped, nothing after the failed line parsed |
documented $QWEN_CODE_MESSAGING_* injection |
✅ arrives through the inbound gate — held peer message … (cause=no-mode-asserted), visible in /peers |
qwen sessions ps --json |
✅ no ipcToken key, and the token string does not appear anywhere in the output |
Two same-build sessions behave exactly as before. list_agents → send_message → the
receiver's model gets the <cross_session_message> envelope → the delivered receipt
authenticates back to the sender (delivery status from wB-d4: pending -> delivered).
I also checked the token does not escape into anything model-visible. It does not:
list_agents projects explicit fields (name/ref/cwd/started_at), and the
receiver's prompt contains neither the sender's replyToken value nor the string
replyToken. The only two consumers of a registry record are sessions ps (stripped)
and peer-directory (feeds peer-send and the projecting list_agents).
2. Both Critical fixes are load-bearing — same-tree A/B
R3-1, the managed-npm-update leak, driven for real: a fake node prefix so
process.execPath resolves to a stand-in npm-cli.js that records the environment it
is handed, with the managed-update route dispatched end to end.
With the scrub removed from runCliEntry, npm — and every lifecycle script of the
package it installs — receives a live socket address and a valid token for a running
session. With the PR as submitted, both are null. R1-8 reproduces too: a feature-off
session hands its children nothing, and a feature-on session hands them its own pair,
never the ancestor's.
3. Mutation: 15/18 killed; the 3 survivors are non-equivalent
I ran each survivor against the real bundle rather than leaving it as an argument.
M1 — the a.length === b.length && guard in tokenMatches (uds-inbox.ts). Removing
it keeps the whole suite green. On the bundled CLI, a first line presenting a
wrong-length token then raises RangeError: Input buffers must have the same byte length out of the socket data handler; handleUncaughtException prints it and calls
process.exit(1). The session dies, triggerable by any same-uid process that can
reach the socket path — the path this PR's threat model treats as guessable. The shipped
code is correct; nothing pins it.
M5 — randomBytes(32).toString('hex') replaced by a constant 64-hex. Also green.
On the bundled CLI an attacker that never read the 0600 record then authenticates by
guessing, and the injection lands — the exact capability the token exists to remove.
(This is the standing R3-3; it survives because sendPeerFrame resolves on a clean close
whether or not the inbox accepted the token, so the test's final step asserts nothing.)
Same two probes on the PR bundle: both refused, session healthy.
M15 — the main() scrub removed. Also green, but I'd rank this low: main() is
reached only from cli.ts:555, inside runCliEntry, which scrubs first on every route.
It is genuine defence in depth, as the commit message says. One nit that follows from
that: the comment above it in llm.tsx ("Modes that never bind an inbox … reach no
other scrub, so it happens here for all of them") was true before the round-3 fix and is
now stale — those modes do reach the runCliEntry scrub.
4. Mixed-build: the reverse direction is one step lossier than the PR body says
Two real bundles, only the peer's build swapped, identical script.
Direction 1 (pre-token sender → token-requiring inbox) is exactly the documented break.
Direction 2 is where I'd adjust the wording. The message does travel new→old, as
claimed — but the pre-token receiver's delivered receipt comes back without an auth
line and the new sender's own inbox refuses it. Verified with a control arm that changes
only the receiver's bundle: PR receiver → sender applies the receipt, 0 auth drops;
pre-token receiver → no receipt applied, 1 auth drop, receiver having sent exactly one.
So in a mixed pair, held / denied / expired / misaddressed never reach a new
sender either.
Worth noting for both directions: the sender's transcript shows
✓ SendMessage "…" → <name> identically whether the message was delivered or silently
dropped. A user on a mixed-build machine has no signal at all. The feature is
experimental and off by default, so I read this as a body/docs accuracy point rather
than a blocker — but "the reverse direction still works" is the sentence a maintainer
will rely on.
5. The documented injection recipe fails silently on common hosts
Running the recipe verbatim from inside a live session: with uuidgen present it is
held and visible in /peers; without it, bash expands $(uuidgen) to the empty string
(the "command not found" goes to stderr, discarded in a hook or cron), the auth line
authenticates, and the user frame is then dropped by MSG_ID_RE —
dropping unparseable frame: {"msgV":1,"msgId":"",…}. Silent non-delivery that the
recipe reports as success.
Both of the recipe's external dependencies are missing from stock images I checked:
uuidgen is absent from alpine:3.20 and ubuntu:24.04; socat is absent from
ubuntu:24.04 and from this macOS host. A shell-only id
(msgId":"'"$$-$(date +%s)-$RANDOM"'") or a command -v uuidgen guard would close it.
Suggested before merge
- Pin the
tokenMatcheslength guard (M1) — one test with a wrong-length first line asserting a clean refusal and a still-live inbox. This is the one I'd actually gate on. - Pin the generated token (M5) with an assertion that can fail: read the record's
ipcTokenin one session and show a different session's inbox rejects it, or assert twoPeerMessaging.startcalls produce different tokens. - Replace
$(uuidgen)indocs/users/features/commands.mdwith something that exists everywhere, or guard it. - Soften "the reverse direction still works" to name the receipt, and refresh the stale
llm.tsxcomment.
Scope — what I did not verify
Linux (the author verified there; I ran macOS) · Windows (feature unavailable; suites are skipIf(isWindows)) · the serve / mcp fast-path leak specifically — I exercised the managed-update route empirically and the ordering invariant only through mutation M14, which the suite kills · named-pipe transport and kernel peer credentials (explicit follow-ups) · sender identity, which the PR states is out of scope.
CI note: reviewDecision is still CHANGES_REQUESTED from earlier bot rounds; the round-4 review was COMMENTED.
中文说明
PR #10636 本地实测报告 @ 2c2f66f253
我在 macOS 上构建了本分支并跑了真实环境：从本树 npm run build && npm run bundle 出的打包 CLI（dist/cli.js），用 pty 驱动两个共享同一 QWEN_HOME 的真实交互会话，真实 UNIX 域套接字、真实 ~/.qwen/sessions/<pid>.json 记录，配本地假 OpenAI 上游让模型真的去调 list_agents / send_message。混用版本那几组，我另外从 df6c361dd2（本分支所基于的 main 提交，即真正的无令牌旧版）构建了第二份打包产物，让两个版本互打。
环境：macOS 15（Darwin 25.6.0）、node v24.18.1、agents.crossSessionMessaging: true、隔离的 QWEN_HOME 与 XDG_RUNTIME_DIR。
**结论：PR 声称的全部可复现。**触及套件上 18 个变异体杀掉 15 个。生产代码本身我没有发现缺陷。下面报告的是两处测试缺口（其影响半径我在真实打包产物上做了演示）、一条在常见宿主上静默失效的文档配方，以及 PR 正文对混用版本某个效应的低估。
1. 审阅验证方式每一步都复现
| 声称 | 结果 |
|---|---|
记录含 ipcPath + 64 位 hex ipcToken，权限 600 |
✅ 符合 /^[0-9a-f]{64}$/，文件 600、sessions/ 目录 700、socket 600、socket 目录 700 |
| 不带 auth 行的原始用户帧 | ✅ 无投递无 held；--debug 打出 dropping a connection whose first line did not authenticate |
| 错误令牌后接用户帧 | ✅ 同样丢弃，失败行之后一概不解析 |
文档中的 $QWEN_CODE_MESSAGING_* 注入 |
✅ 经入站闸门到达 —— held peer message …(cause=no-mode-asserted)，/peers 可见 |
qwen sessions ps --json |
✅ 无 ipcToken 键，输出中也不含令牌字符串 |
同版本两个会话行为与之前一致：list_agents → send_message → 接收方模型拿到 <cross_session_message> 信封 → delivered 回执反向认证成功（delivery status from wB-d4: pending -> delivered）。
我另外核查了令牌是否会外泄到模型可见的地方——没有：list_agents 只投影显式字段（name/ref/cwd/started_at），接收方 prompt 里既没有发送方的 replyToken 值，也没有 replyToken 这个串。注册记录的消费者只有 sessions ps（已剥离）和 peer-directory（喂给 peer-send 与做投影的 list_agents）。
2. 两条 Critical 修复都是承重件——同树 A/B
R3-1 的 managed-npm-update 泄漏，真跑：伪造 node 前缀让 process.execPath 解析到一个记录自身环境的替身 npm-cli.js，managed-update 路由端到端派发。
把 runCliEntry 里的清除去掉后，npm——以及它所安装包的全部生命周期脚本——拿到了一个运行中会话的活 socket 地址加有效令牌；按 PR 提交的样子两者都是 null。R1-8 同样复现：特性关闭的会话不向子进程传任何东西，特性开启的会话传的是它自己的那一对，绝不是祖先的。
3. 变异测试：18 杀 15；3 个存活体均非等价
三个存活体我都在真实打包产物上跑了，而不是停在论证层面。
M1 —— tokenMatches 里的 a.length === b.length && 守卫。去掉它全套件仍绿。在打包 CLI 上，首行出示一个长度不对的令牌会从 socket data 回调里抛出 RangeError: Input buffers must have the same byte length，handleUncaughtException 打印后 process.exit(1)——会话直接死掉，且任何同 uid、能连到该 socket 路径的进程都能触发，而该路径正是本 PR 威胁模型里认定"可猜"的那条。现有代码是对的，但没有任何测试钉住它。
M5 —— randomBytes(32).toString('hex') 换成常量 64 位 hex。同样全绿。在打包 CLI 上，一个从未读过 0600 记录的攻击者靠猜就认证成功、注入落地——恰是令牌要消除的那个能力。（即 standing 的 R3-3；它能存活是因为 sendPeerFrame 在连接干净关闭时就 resolve，无论 inbox 是否接受了令牌，所以那条测试的最后一步其实什么都没断言。）
同样两个探针打在 PR 产物上：都被拒，会话健在。
M15 —— 去掉 main() 里的清除。也是全绿，但我给它的权重很低：main() 只从 cli.ts:555 进入，而那是在 runCliEntry 内部、后者已在每条路由上先行清除。它确实是 commit message 所说的纵深防御。由此带出一个小问题：llm.tsx 里它上面那段注释（"Modes that never bind an inbox … reach no other scrub, so it happens here for all of them"）在第 3 轮修复之前成立，现在已过时——那些模式是会经过 runCliEntry 的清除的。
4. 混用版本：反方向比 PR 正文所说的多丢一层
两份真实产物，只换对端的 build，脚本完全相同。
方向 1（旧发送端 → 需令牌 inbox）就是文档化的那个破坏。方向 2 是我想修正措辞的地方：消息确实能新→旧走通，与所述一致；但旧版接收方回的 delivered 回执不带 auth 行，会被新发送方自己的 inbox 拒掉。用只改接收方 build 的对照臂验证：PR 接收方 → 发送方应用了回执、0 次认证丢弃；旧版接收方 → 未应用任何回执、1 次认证丢弃，而接收方确实只发了一条。也就是说混用组合里，held / denied / expired / misaddressed 同样到不了新发送端。
两个方向都值得一提的是：无论消息是投递成功还是被静默丢弃，发送方 transcript 都一样显示 ✓ SendMessage "…" → <name>。混用版本机器上的用户没有任何信号。特性是实验性且默认关闭，所以我把它看作正文/文档的准确性问题而非阻断项——但"反方向仍然可用"正是维护者会据以判断的那句话。
5. 文档里的注入配方在常见宿主上静默失效
在真实会话内逐字执行该配方：有 uuidgen 时消息被 held 并在 /peers 可见；没有时，bash 把 $(uuidgen) 展开成空串（"command not found" 走 stderr，在 hook / cron 里被丢弃），auth 行认证通过，随后用户帧被 MSG_ID_RE 拒绝——dropping unparseable frame: {"msgV":1,"msgId":"",…}。配方报告成功，实际静默不投递。
该配方的两个外部依赖在我核查的官方镜像里都缺失：alpine:3.20 与 ubuntu:24.04 都没有 uuidgen；ubuntu:24.04 和本 macOS 宿主都没有 socat。改用纯 shell 的 id（msgId":"'"$$-$(date +%s)-$RANDOM"'"）或加一道 command -v uuidgen 判断即可闭合。
合入前建议
- 钉住
tokenMatches的长度守卫（M1）——一条用长度不符的首行、断言干净拒绝且 inbox 仍存活的测试。这条是我唯一会真正设卡的。 - 用能失败的断言钉住生成的令牌（M5）：读出某会话记录里的
ipcToken，证明另一个会话的 inbox 拒绝它；或断言两次PeerMessaging.start产生不同令牌。 - 把
docs/users/features/commands.md里的$(uuidgen)换成到处都有的写法，或加判断。 - 把"反方向仍然可用"改成点明回执的说法，并更新
llm.tsx那段已过时的注释。
范围——我没有验证的部分
Linux（作者已在该平台验证，我跑的是 macOS）· Windows（特性不可用，相关套件 skipIf(isWindows)）· serve / mcp 快速路径的泄漏本身——我实测的是 managed-update 路由，顺序不变式只经变异体 M14 覆盖（套件能杀掉它）· 命名管道传输与内核对端凭证（明确的后续工作）· 发送方身份，PR 已声明范围外。
CI 说明：reviewDecision 仍是早前几轮留下的 CHANGES_REQUESTED，第 4 轮评审是 COMMENTED。
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 3/100) — it will retry on the next scan. Review round on PR #10636 — all actionable findings closedCommit Implemented this round
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误（第 3/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33528746812 🧠 Handled by Qwen Code · model/模型 |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Reviewed at head 2c2f66f2.
- The round-3 Critical is fixed exactly as requested: the inherited messaging pair is now scrubbed at the top of
runCliEntry— before every route that can spawn children (managed npm update,mcp,serve), not justmain()— with two new entry tests driving the previously-bypassable routes and asserting the pair is gone at child-spawn time; theclose()path reuses the same single-writer helper (the round's D3-1 note). - The capability model reads sound: the pair is exported only by a session that binds its own inbox, the registry carries address+token together, connections require the exact per-session token (length-checked
timingSafeEqual), and a headlessqwen servekeeps the previous unauthenticated behavior. - 26/26 threads resolved; no new Critical issues found. CI on this head has no failures (two cancelled entries from superseded pushes); per the channel convention the call is on the review itself.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review passed (security-sensitive, reviewed in full).
- Token design is sound: 64-hex randomBytes per inbox (the generated path is pinned by a test), constant-time compare with the length short-circuit before timingSafeEqual, and the auth line is shaped like a frame so pre-token receivers skip it as an unknown type — senders can always lead with it without knowing the peer's build.
- Connection-state scope is correct and regression-pinned: refusal and admission are per-connection closures (a dropped connection cannot brick the inbox; an admitted one cannot admit the next), a failed first line is terminal for that connection, and one auth line admits all frames on that same connection.
- Capability hygiene: the inherited socket/token pair is scrubbed at every route before any child can spawn (cli entry — including the managed-npm-update and serve/mcp fast paths — and first in main()), re-exported only when this session's own inbox is accepting, cleared on close. The token stays out of
sessions psJSON and model-visible output; the registry record carries it because it is 0600 and discovery+auth are one capability. - Receipts authenticate with the frame-carried replyToken (justified: an accepted peer could read the sender's record anyway); pre-token records omit tokens on both directions, and the lossy upgrade direction is documented.
Verified locally on the PR head: core ipc + session-registry tests 214/214, cli peer-messaging + sessions ps tests 58/58; CI green on this head.
|
Released in v0.23.0. |
5 participants
What this PR does
Adds connection-level authentication to the experimental cross-session messaging inbox. Every session's inbox now generates a random per-session token and publishes it in the session's registry record (0600, owner-only) beside the socket address it already advertises. A connection to the inbox must present that token on its first line before any message is read; a connection that fails to authenticate is dropped immediately and permanently, with nothing after the failed line ever parsed. Messages a session sends now carry a reply token of their own, so the delivery receipts flowing back (held / delivered / denied / expired / misaddressed) authenticate in the reverse direction through the same mechanism.
The session also exports its inbox address and token to child processes as
QWEN_CODE_MESSAGING_SOCKETandQWEN_CODE_MESSAGING_TOKEN, so a script or hook the session runs can inject a message back into it. An injected message goes through the same inbound gate as one from another session — it is marked as not coming from the user, and theagents.crossSessionInboundpolicy (or the mode-parity default) decides whether it is delivered or held for review. The user docs gain a short section with a working injection example.qwen sessions ps --jsonstrips the token from its output: it is a credential, not data — tooling that genuinely needs it can read the record file, but it must not spill into logs and pipelines by default.Why it's needed
Today the inbox's entire access-control story is filesystem permissions (0700 directory, 0600 socket). That holds on POSIX, but it cannot carry over to a transport without those semantics — native named-pipe support needs connection-level authentication as its foundation — and the socket path itself is guessable (keyed by PID), while the registry record is not readable without owner access. Requiring a token that lives in the record narrows "can reach the socket path" to "can read this session's registry record", which is also the same capability discovery already requires, so senders get address and credential in one read with no extra round trip. It is also what makes officially opening the inbox to the session's own child processes safe: without authentication, exporting the socket address alone would have widened the unauthenticated surface.
Reviewer Test Plan
How to verify
settings.json({ "agents": { "crossSessionMessaging": true } }) and start two interactive sessions. Messaging between them vialist_agents/send_message, held-message review via/peers, and the receipt lines in the sender's transcript should all behave exactly as before — same-build sessions are unaffected.cat ~/.qwen/sessions/<pid>.jsonfor a running session: it should containipcPathplus a 64-hexipcToken, with file mode 600.socator a small node script): nothing should be delivered or held, and with--debugthe log names the unauthenticated drop.{"msgV":1,"type":"auth","token":"'$QWEN_CODE_MESSAGING_TOKEN'"}, second line a normal user frame, written to$QWEN_CODE_MESSAGING_SOCKET. The message should arrive through the inbound gate (delivered, or held and visible in/peers, depending on the mode).qwen sessions ps --jsonshould not containipcToken.Evidence (Before & After)
Verified against a live dev-build session (Linux): the registry record carried
ipcPath+ a 64-hexipcTokenat mode 600; an injection with a wrong token produced no delivery and no receipt; an injection with the correct token (receiver in auto mode, sender asserting no mode class) came back with aheldreceipt to the injector's own socket, matching the existing mode-parity policy. Full log in the follow-up comment.Tested on
Environment (optional)
npm run devunder tmux with an isolatedQWEN_HOME; unit suites run against real UNIX sockets.Risk & Scope
misaddressedreceipt — the window is milliseconds because every send re-reads the registry, and same-process session swaps (/clear,/resume) still producemisaddressedas before.fromstays reply-routing-only, and the inbound gate plus envelope remain the authority model. Kernel-level peer credentials and named-pipe transport are follow-ups.ipcTokenis an optional field older readers ignore.Linked Issues
None.
中文说明
本 PR 做了什么
为实验性的跨会话消息 inbox 增加连接级认证。每个会话的 inbox 生成一个随机的会话级令牌，与已发布的 socket 地址一起写进该会话的注册记录（0600，仅属主可读）。连接 inbox 必须在首行出示该令牌，之后消息才会被读取；认证失败的连接立即被断开且永久拒绝，失败行之后的内容一概不解析。会话发出的消息带有自己的回复令牌，使回程的投递回执（held / delivered / denied / expired / misaddressed）通过同一机制反向认证。
会话还会把自己的 inbox 地址与令牌以
QWEN_CODE_MESSAGING_SOCKET和QWEN_CODE_MESSAGING_TOKEN环境变量导出给子进程，会话运行的脚本或 hook 因此可以向本会话回注消息。注入的消息与来自其他会话的消息走同一入站闸门——被标记为并非来自用户，由agents.crossSessionInbound策略（或模式对等的默认规则）决定投递还是留待审阅。用户文档新增了带可用注入示例的小节。qwen sessions ps --json会从输出中剥离令牌：它是凭据而非数据——确实需要它的工具可以直接读记录文件，但它不应默认流入日志与管道。为什么需要
目前 inbox 的全部访问控制是文件权限（目录 0700、socket 0600）。这在 POSIX 上成立，但无法迁移到没有这类语义的传输上——原生命名管道支持需要以连接级认证为基础——而且 socket 路径本身按 PID 可猜测，注册记录则没有属主权限读不到。要求出示存放在记录里的令牌，把"能连上 socket 路径"收紧为"能读到该会话的注册记录"，而这恰好也是发现机制本就要求的能力，发送方一次读取同时获得地址与凭据，没有额外往返。这也是安全地向会话自身子进程开放 inbox 的前提：没有认证，仅导出 socket 地址就会扩大未认证暴露面。
审阅验证方式
settings.json开启实验特性（{ "agents": { "crossSessionMessaging": true } }）并启动两个交互式会话。经list_agents/send_message互发、/peers审阅 held 消息、发送方 transcript 中的回执行为应与之前完全一致——同版本会话不受影响。cat ~/.qwen/sessions/<pid>.json：应包含ipcPath与 64 位 hex 的ipcToken，文件权限 600。--debug日志会记录未认证丢弃。{"msgV":1,"type":"auth","token":"'$QWEN_CODE_MESSAGING_TOKEN'"}，次行普通用户帧，写入$QWEN_CODE_MESSAGING_SOCKET。消息应经入站闸门到达（按模式投递或 held 并在/peers可见）。qwen sessions ps --json输出不应包含ipcToken。证据（Before & After）
已在 Linux 上对 dev 构建的真实会话验证：注册记录携带
ipcPath与 64 位 hexipcToken（权限 600）；错误令牌注入无投递无回执；正确令牌注入（接收方 auto 模式、发送方未声明模式类）收到送达注入方自建 socket 的held回执，与既有模式对等策略一致。完整日志见后续评论。测试平台
Linux ✅；macOS⚠️ 未本地验证（CI 覆盖）；Windows N/A（跨会话消息尚不支持该平台，相关测试套件 skipIf(isWindows)）。
风险与范围
misaddressed回执——每次发送都会现读注册表，窗口只有毫秒级，同进程内的会话切换（/clear、/resume）仍照旧产生misaddressed。from仍仅用于回复路由，入站闸门与信封仍是权限模型的主体。内核级对端凭证与命名管道传输属后续工作。ipcToken为可选字段，旧读者忽略。关联 Issue
无。