feat(voice): support trusted private ASR base URLs - #8350
Conversation
Code ReviewOverviewAdds What looks good
Findings1. Allowlist entry normalization diverges between CLI and Desktop (Suggestion, cross-surface correctness) Desktop's 2. Desktop credential resolution now hard-fails configs that previously worked (Suggestion, behavior change)
3. Desktop requires allowlisting even for public HTTPS custom providers; CLI does not (Suggestion, consistency/naming) On Desktop, any non-DashScope provider — including a public 4. Security-guard logic is duplicated with non-identical implementations (Suggestion, maintainability)
VerdictNo Critical findings. The security posture is sound: default deny is preserved, the opt-in is exact-match and trusted-scope only, and the always-blocked classes are enforced on both literal and resolved addresses with good test coverage. Findings 1–2 are the ones I'd most like addressed (or explicitly acknowledged) before merge, since both produce silent or surprising failures for operators; 3–4 can be follow-ups. |
|
Gate re-run after @wenshao's hands-on verification and approval — no commits since the last pass, so this re-evaluates the same head ( Template ✓ — all required sections present; Before/After is a justified N/A (settings/network-policy behavior, no TUI surface); bilingual body complete. Problem. Feature request, not a bug fix — no reproduction required. Linked issue #8286 (same author, Direction. This deliberately widens a security-sensitive default, so it was escalated for maintainer awareness early (Stage 0). That escalation is now resolved the strongest way it can be: @wenshao built the PR from source, drove a real cleartext-HTTP gateway on a private LAN through a 14-scenario batch matrix, the streaming WebSocket path, and the Workspace trust boundary, mutation-tested the new drift gate, and submitted an approving review pinned to this exact commit. Size. Core paths touched (cli config/services + desktop server-core, cross-package). Production runtime ≈ 1,306 lines (CLI 355 / desktop server-core 951) + 232 lines of drift-guard script + ~250 tooling/config; tests ≈ 3,686 lines; docs 77; schema 8. Above the 1,000-line advisory — the PR body's split argument (one security contract across all egress paths; splitting would temporarily leave paths with inconsistent policy) is reasonable. Approach. Matches the independent proposal from the prior pass: allowlist stripped at the workspace merge boundary, one exact normalized-URL decision made at config resolution and carried to every egress path, always-blocked address classes unaffected by the allowlist, mirrored desktop policy with a mechanical AST-based sync gate in CI (stronger than the comment-sync I would have settled for). The desktop resolver grew a per-model-ID authoritative-provider credential chain on top of the policy mirror — that is the price of desktop not importing CLI settings code, and it is covered by tests plus the maintainer's runtime verification. Risk. No Stage 1e high-risk-path matches. The deeper-review conditions from earlier rounds are all satisfied by now: full CI evidence in the Stage 2 comment, sandboxed Moving on to the code review result below. 🔍 中文说明在 @wenshao 完成实机验证并批准后重跑门禁 —— 上次评审后没有新 commit，本次仍针对同一 head（ 模板 ✓ —— 必填章节齐全；Before/After 属合理的 N/A（设置/网络策略行为，无 TUI 界面变化）；双语正文完整。 问题。 这是功能请求而非 bug 修复——不需要复现。关联 issue #8286（同一作者， 方向。 本 PR 有意放宽了一个安全敏感的默认值，因此在 Stage 0 就升级为维护者知悉。该升级现在以最强方式闭环：@wenshao 从源码构建了本 PR，用私有局域网上的真实明文 HTTP 网关跑了 14 场景批量矩阵、流式 WebSocket 路径和 Workspace 信任边界，并对新的漂移门禁做了变异测试，随后在指向本 commit 的评审中批准。 规模。 触及核心路径（cli config/services + desktop server-core，跨包）。生产运行时代码约 1,306 行（CLI 355 / desktop server-core 951）+ 232 行漂移门禁脚本 + 约 250 行工具/配置；测试约 3,686 行；文档 77；schema 8。超过 1,000 行建议线 —— PR 正文的拆分论证（同一安全契约覆盖全部出口路径，拆分会暂时留下策略不一致的出口）合理。 方案。 与上一轮独立提案一致：白名单在 workspace 合并边界被剥离、规范化 URL 精确匹配在配置解析时一次决定并传递到每个出口路径、永久封禁地址类不受白名单影响、desktop 镜像策略 + CI 中基于 AST 的机械同步门禁（比我原本会接受的注释同步更强）。desktop 解析器在策略镜像之上长出了按模型 ID 的权威 provider 凭证链——这是 desktop 无法导入 CLI 设置代码的代价，已有测试 + 维护者运行时验证覆盖。 风险。 未命中 Stage 1e 高风险路径。此前各轮要求的更深 review 条件现已全部满足：Stage 2 评论中的完整 CI 证据、在本 SHA 上通过的沙箱 进入下方代码审查结果。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (re-run at
|
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | ✅ success |
| Desktop Shell (ubuntu-22.04) | ✅ success |
| Serve A/B (ubuntu-latest, Node 22.x) | ✅ success |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | ✅ success |
| Real daemon E2E / Java 11 | ✅ success |
| Build Qwen Live Host | ✅ success |
| Live Host (macos-latest) | ✅ success |
| Prepare Live Host release | ✅ success |
| Java builds (ubuntu 11/17/21, macOS 21, windows 21) | ✅ success |
| precheck-pr / Classify PR / review-pr | ✅ success |
中文说明
代码审查（在 ce6b4803 上重跑，批准后）
在本 SHA 上对完整 diff 重新做了一遍独立审读（不依赖此前 15 轮结论）。未发现新的阻断问题——实现与 PR 描述中的契约一致，也与 @wenshao 实机验证观察到的行为一致。本轮核对内容：
- 信任边界。
security.allowedInsecureVoiceBaseUrls在合并阶段从 Workspace 作用域剥离（泛化后的stripWorkspaceSecurityBypasses，与allowPrivateNetworkHooks同机制），启动时给出警告；仅 User/System/SystemDefaults 生效。测试也锁住了细节场景——workspace 条目无法覆盖 user 条目，System 作用域的空列表可以撤销 user 条目。 - 一次决定、全出口执行。 白名单匹配只在配置解析时发生一次（
isInsecureVoiceBaseUrlAllowed，规范化 URL 全等，含协议/主机/端口/路径），结果标志随解析后的配置进入全部四个守卫调用点：CLI 批量transcribeVoiceAudio、daemon 流式（serve/voice/voice-ws.ts）、TUI 流式（InputPrompt.tsx）、desktop 批量+流式默认路径（voice-ws-handler.ts）。逐一核对了每个调用点传递的是解析后的配置而非裸 URL。 - 永久封禁类在白名单下仍被封禁。 元数据地址（含
fd00:ec2::254与100.100.100.200）、链路本地、未指定地址、环回——包括 DNS 解析到环回的结果和 IPv4 映射/兼容/NAT64 写法——在出口处无视标志位复查；畸形::ffff:形式失败即拒绝。过渡地址段（本地 NAT64、IETF 分配/Teredo 容器、6to4）作为解包绕过通道被封禁。 - 顺带加固。 env-key 查找使用
Object.hasOwn（constructor之类的 envKey 无法以函数身份到达.trim()）；内嵌凭证的baseUrl由静默剥离改为硬失败；desktop 设置合并丢弃原型污染键。 - CLI/desktop 孪生体。
net-guard.ts逐单元镜像 CLI 地址分类，新增的check-voice-guard-sync.js在 CI（full profile）中以规范化 AST 对比两侧——此处漂移会让构建失败而不是静默放行。desktop 的可信设置合并、dotenv 语法与存储路径是形状不同的镜像，按文档由对等测试锁定。
长期存在的非阻断项（均已披露，未变化）：
- @wenshao 的备注 (a)：daemon 的 200 字符错误截断会把补救消息中可操作的 URL 剪掉（只剩约 23 字符）。值得一个后续 commit；autofix 的 Critical-only 模式已将其留待人工跟进。
- 备注 (b)/(c)：内嵌凭证硬失败与
providerProtocol语音模型可见性变更均已写入设计文档并有测试覆盖，但 PR 正文的破坏性变更/迁移一节仍值得各补一句。 - desktop 的 bun 语音套件仍然不在任何 CI job 中运行；新的漂移门禁与 CI 全绿的 CLI 套件能缓解但不能替代。本次未验证：desktop bun 套件 + Electron 运行时（本运行从不执行 PR 代码；维护者已在本地运行 bun 套件——135 + 15 通过——并明确说明未启动 Electron 本体）。Windows/Linux：作者与维护者均在 macOS 上测试，其他平台交由 CI（下方构建全绿）。
测试
这是无人值守 CI 运行——我未构建或执行任何 PR 代码。 下方证据为被审 commit 上 PR 自身的 CI（经 API 读取），以及两个已完成、归属于其所有者的验证层：
- 沙箱
/verify（第 2 轮）——在本 SHA（ce6b4803）上 ✅ 可合入：153/153 无 mock 脚本断言，定向 CLI 门禁 300/300。属咨询性证据，不构成评审。 - 维护者验证（@wenshao，非机器人执行）： 私有局域网上的真实明文 HTTP ASR 网关、真实
qwen servedaemon、真实音频——14 场景 before/after 批量矩阵（每个 DENY 行网关请求数为零）、流式 WebSocket 路径、Workspace 作用域自授权被拒绝并以设置警告呈现，另有三个变异测试确认套件与漂移门禁确实锁住了契约。结论为 "LGTM to merge"。
ce6b4803 上的 CI：所有实际运行的检查全部成功——0 失败、0 待定（84 个跳过的检查为条件性编排 job）。check:voice-guard-sync 漂移门禁作为全绿的 Qwen Code CI 工作流中的一个步骤运行。
— Qwen Code · qwen3.8-max
Reviewed at ce6b48032ca7b2c68724e2183e37e3f7dd474ed8 · re-run with @qwen-code /triage
|
Confidence: 4/5 — a genuinely clean pass on the diff, green CI, sandboxed verification, and a maintainer who verified it against real network egress before approving; the last point is held back only by standing follow-ups, none of them blocking. Stepping back: sixteen rounds in, this PR is what it claimed to be and nothing more. The allowlist is exact-URL, trusted-scope-only, fail-closed on every path I traced, and the one piece of unavoidable duplication (the desktop network guard) is the only mirrored security code in this repo that is mechanically drift-checked in CI. Every earlier reservation has been answered with evidence rather than prose: the sandboxed On the policy cap, plainly: earlier passes deferred this PR under the Stage 0 rule — a fork feature at ~1,300 production lines touching core and opening new security surface needs a maintainer's sign-off, not a bot approval. That escalation has been answered in the strongest form available: @wenshao built the branch from source, exercised the exact risk this PR manages (cleartext credentials and audio on a private wire), confirmed the trust boundary and the fail-closed behavior firsthand, and approved this exact commit. Deferring again would defer to a decision that has already been made. The three open notes (error-message truncation follow-up, two PR-body sentences) are recorded in the Stage 2 comment as follow-ups, not conditions. Reservations kept on record: the desktop bun suites and the Electron runtime remain outside CI and were not exercised end-to-end here; Windows/Linux coverage rests on CI builds. None of that blocks merge given the evidence above — flagging for the merge commit's awareness. Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 对 diff 的一次真正干净的审读、全绿 CI、沙箱验证，以及维护者在批准前已对真实网络出口做过验证；最后一点保留仅来自长期存在的跟进项，均不阻塞。 退一步看：十六轮之后，这个 PR 就是它所声称的东西，不多不少。白名单是精确 URL、仅受信作用域、在我追踪的每条路径上失败即拒绝；唯一不可避免的重复（desktop 网络守卫）是本仓库中唯一被 CI 机械性漂移检查的镜像安全代码。此前的每一项保留意见都用证据而非文字回应了：本 SHA 上的沙箱 关于政策上限，直说：此前各轮按 Stage 0 规则 defer 了本 PR——一个约 1,300 行生产代码、触及核心并打开新安全面的 fork 功能需要维护者签核，而不是机器人批准。该升级现已以最强形式得到回应：@wenshao 从源码构建了本分支，实际演练了本 PR 所管理的确切风险（私有链路上的明文凭证与音频），亲自确认了信任边界与失败即拒绝行为，并批准了这个 commit。再次 defer 将是在向一个已经做出的决定 defer。三个未结备注（错误消息截断的后续代码、PR 正文的两句话）已作为跟进项记录在 Stage 2 评论中，不构成合并条件。 保留在案的意见：desktop bun 套件与 Electron 运行时仍在 CI 之外，本次也未做端到端演练；Windows/Linux 覆盖依赖 CI 构建。鉴于上述证据，这些都不阻塞合并——提请合并时知悉。 批准，锁定在被审 commit。✅ — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @wenshao — this is a well-built PR (clean review, thorough tests, conservative fail-closed defaults), but it's a large (~716 production lines) core-touching
Also worth a |
🩺 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 |
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.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /takeover |
|
🚫 Takeover not engaged: fork takeover requires the PR author to hold write access on this repository (author 中文说明🚫 未接管：fork 托管要求 PR 作者在本仓库持有 write 及以上权限（作者 |
|
Addressed the review feedback in 797499a:
Final local verification: CLI focused suites 246/246, Desktop voice suites 103/103, Desktop server-core TypeScript check, root lint/typecheck/build, and pre-commit formatting/lint all pass. I also iterated three completed Qwen local |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
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. Not reviewed: You are review agent verify — Verification agent (round... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. Test Plan (not a blocker): 247 passed — this review observed 16108, 475 passed; 104 passed — this review observed 16108, 475 passed.
中文说明
已审查。 建议见行内评论。 未审查：You are review agent verify — Verification agent (round...——它被指向 diff 的行却从未打开：有工具调用，但没有一次读取 diff。 Test Plan（非阻断）：247 passed — this review observed 16108, 475 passed; 104 passed — this review observed 16108, 475 passed。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/30746387737)._ |
|
@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. 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，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🤖 Addressed the latest review feedback (round 15/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 15/100 轮）。改动内容与我反驳保留之处如下： Review round 11 — summaryCritical-only mode is active after 10 change-producing rounds. Per the repository's review policy, this round lands only the two Critical fixes and defers the 13 Suggestions to follow-up issues/PRs; every deferral is recorded as a reply on its own thread so nothing is dropped silently. Feedback points and decisionsCritical — fixed
The fix removes the entire ~200-line hand-rolled scanner ( Suggestions — deferred to follow-up issue/PR (Critical-only mode)
Each deferral is posted as a reply on its own thread ( Review body (CHANGES_REQUESTED)The review's "Not reviewed" notes (the desktop bun suites not running in CI — tracked by the author in #8360; the CLI integration suite skipped in CI; the reverse audit stopped by the time budget; the actionlint embedded-shell limitation; the Test Plan count observation) are coverage statements about the review itself, not code findings, and request no code change. ConflictsNone ( Verification
Commit: 中文说明第 11 轮评审 — 总结在完成 10 个产生改动的评审轮后已进入仅处理 Critical 的模式。按仓库评审策略，本轮只合入两个 Critical 修复，13 条建议（Suggestion）延后到后续 issue/PR；每条延后都会在其所属讨论串中以回复形式记录，不会被静默丢弃。 反馈点与决定Critical — 已修复
本次修复整体删除了约 200 行手写扫描器（ 建议类 — 延后到后续 issue/PR（仅处理 Critical 模式）
每条延后都会在其所属讨论串中发布回复（ 评审主体（CHANGES_REQUESTED）评审中的 "Not reviewed" 说明（desktop bun 套件未在 CI 中运行——作者已在 #8360 跟踪；CLI 集成套件在 CI 中被跳过；反向审计因时间预算中止；actionlint 内嵌 shell 的限制；Test Plan 计数观察）是评审自身覆盖面的陈述，不是代码发现，也不要求任何代码改动。 冲突无（ 验证
提交： Deferred non-Critical feedbackCritical-only mode is active after 10 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 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/模型 |
|
🔀 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 将重新运行。 |
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. Not reviewed: build-and-test — the desktop server-core voice suites (resolve-voice-config.test.ts, voice.test.ts, voice-ws-handler.isolated.ts) are bun tests outside every npm workspace; no CI job runs them and this review's harness could not execute them (tracked by the author in #8360); the desktop half was reviewed statically and via probes only. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (integration-tests/cli, which covers the serve route surface including voice routes) did not run locally. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted. Test Plan (not a blocker): 247 passed — this review observed 17803, 475 passed; 109 passed — this review observed 17803, 475 passed.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — the desktop server-core voice suites (resolve-voice-config.test.ts, voice.test.ts, voice-ws-handler.isolated.ts) are bun tests outside every npm workspace; no CI job runs them and this review's harness could not execute them (tracked by the author in #8360); the desktop half was reviewed statically and via probes only。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (integration-tests/cli, which covers the serve route surface including voice routes) did not run locally。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 Test Plan（非阻断）：247 passed — this review observed 17803, 475 passed; 109 passed — this review observed 17803, 475 passed。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| for (let k = startLine + 1; k < lines.length; k += 1) { | ||
| if (lines[k] === '}') { | ||
| return lines.slice(startLine, k + 1).join('\n'); |
There was a problem hiding this comment.
[Suggestion] Unit extraction ends at the first line that is textually } at column 0 — so a multi-line template literal inside a mirrored unit that contains a column-0 } line (or a statement appended after a kind: 'block' unit's closing brace) truncates the extracted unit, and any drift after that line is silently ignored: a fail-open hole in the drift guard this PR adds.
Failure scenario: probe-verified against the shipped exports — (1) a unit whose body contains a multi-line template literal with a column-0 } is truncated (a 9-line unit extracts as 3 lines), and two sources differing only after that line make checkMirrorSet return [] — no drift; (2) appending one statement after the BLOCKED_TRANSITION_IPV6_ADDRESSES block unit on only one side reads CLEAN. Not live today (no mirrored unit contains such a line), but adding a multi-line error message to a mirrored unit, or extending the transition blocklist with an appended statement, silently disables drift detection for the remainder of that unit. Note: scripts/tests/check-voice-guard-sync.test.js test 5 currently pins the truncation boundary as intended behavior — a fix must update it too.
Suggested fix: derive unit bounds from the parse tree — ts.createSourceFile the whole file, locate the top-level statement whose declaration starts the unit, and print that node (or tokenize with string/template/regex span awareness when hunting the closing brace).
中文说明
[Suggestion] 单元提取在第一行文本上等于列首 } 的行处终止——因此镜像单元中的多行模板字面量若包含一行列首 }（或在 kind: 'block' 单元的收尾花括号之后追加语句），提取出的单元会被截断，该行之后的任何漂移都会被静默忽略：这是本 PR 新增的漂移守卫中的一个 fail-open 漏洞。
失败场景：已对交付的导出函数探针验证——(1) 函数体包含含列首 } 的多行模板字面量的单元会被截断（9 行单元只提取出 3 行），两个仅在该行之后有差异的源文件会使 checkMirrorSet 返回 []——无漂移；(2) 仅在 BLOCKED_TRANSITION_IPV6_ADDRESSES 块单元之后一侧追加一条语句，检查读取为 CLEAN。当前未触发（没有任何镜像单元包含此类行），但一旦给镜像单元添加多行错误消息、或以追加语句的方式扩展过渡前缀黑名单，就会静默关闭对该单元其余部分的漂移检测。注意：scripts/tests/check-voice-guard-sync.test.js 的测试 5 目前把该截断边界钉为预期行为——修复时需一并更新。
建议修复：从解析树推导单元边界——对整个文件 ts.createSourceFile，定位以该单元声明开头的顶层语句并打印该节点（或在寻找收尾花括号时对字符串/模板/正则字面量区间保持感知）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| '64:ff9b::10.0.0.8', | ||
| '64:ff9b:1::a00:8', |
There was a problem hiding this comment.
[Suggestion] (Pattern, site 1/3 — sibling comments at packages/cli/src/ui/voice/voice-transcriber.test.ts:664 and packages/desktop/packages/server-core/src/voice/resolve-voice-config.test.ts:683.) The reserved-bit / >2-suffix-group address class that fails open in the NAT64 classifier (see the low-confidence Critical on voice-transcriber.ts in this review's summary) is absent from every guard test suite on both surfaces: these loops pin every well-formed 64:ff9b::/96 spelling while omitting exactly the escaping class, so nothing pins its behavior — today the escape forms pass both suites untested, and if the classifier is fixed fail-closed, a later simplification reintroducing the hole stays green everywhere.
Failure scenario: probe-verified — 64:ff9b::1:203:405 and 64:ff9b::1:2.3.4.5 pass assertVoiceBaseUrlNetworkAllowed today (untrusted, as DNS record and as IP literal) while their well-formed siblings are blocked; a repo-wide grep finds no such literal in any voice suite. If a fix lands without these pins, a regression reintroducing the hole ships green in both twins.
Suggested fix: when the classifier disposition is settled, add '64:ff9b::1:203:405' and '64:ff9b::1:2.3.4.5' to this loop asserting the blocked outcome the fix adopts — or, if these forms are deliberately kept public, pin that decision deliberately.
中文说明
[Suggestion]（模式发现，位置 1/3——姊妹评论见 packages/cli/src/ui/voice/voice-transcriber.test.ts:664 与 packages/desktop/packages/server-core/src/voice/resolve-voice-config.test.ts:683。）在 NAT64 分类器中 fail-open 的保留位 / 超过 2 个后缀组地址类（见本次评审摘要中针对 voice-transcriber.ts 的低置信度 Critical）在两端的所有守卫测试套件中均缺席：这些循环钉住了所有合法形式的 64:ff9b::/96 拼写，却恰好遗漏了会逃逸的那一类，因此没有任何测试钉住其行为——今天这些逃逸形式在两套测试中未经测试地通过；若分类器被修复为 fail-closed，之后重新引入该漏洞的简化改动在所有测试中仍为绿色。
失败场景：已探针验证——64:ff9b::1:203:405 与 64:ff9b::1:2.3.4.5 今天能通过 assertVoiceBaseUrlNetworkAllowed（未受信模式下，无论作为 DNS 记录还是 IP 字面量），而其合法形式的同类地址被阻断；全仓库 grep 未在任何语音套件中找到此类字面量。若修复落地而没有这些钉子，重新引入该漏洞的回归将在两端一路绿灯地合入。
建议修复：待分类器的处置确定后，在本循环中加入 '64:ff9b::1:203:405' 与 '64:ff9b::1:2.3.4.5'，断言修复所采用的阻断结果——或者，若刻意保留这些形式为公网，请刻意地把该决定钉住。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| '::a9fe:a9fe', | ||
| '::6464:64c8', | ||
| '::5db8', |
There was a problem hiding this comment.
[Suggestion] (Pattern, site 2/3 — sibling comments at packages/desktop/packages/server-core/src/voice/voice.test.ts:256 and packages/desktop/packages/server-core/src/voice/resolve-voice-config.test.ts:683.) This loop pins the well-formed ::/96 embeddings (::a9fe:a9fe, ::6464:64c8, ::5db8) but omits the reserved-bit / >2-suffix-group siblings that fail open in the IPv4-compatible classifier (see the low-confidence Critical on this file's implementation in this review's summary): ::1:a9fe:a9fe, ::1:6464:64c8, ::1:7f00:1 classify as public and pass even the untrusted gate today, untested in both twins.
Failure scenario: probe-verified — the escape forms pass assertVoiceBaseUrlNetworkAllowed for trusted and untrusted configs (and as IP literals, and through resolveVoiceTranscriptionConfig) while the pinned well-formed siblings are blocked; the desktop twin loop and both opt-in loops have the same gap. If the classifier is fixed fail-closed without these pins, a later simplification reintroducing the hole stays green everywhere.
Suggested fix: when the classifier disposition is settled, add '::1:a9fe:a9fe' (and a dotted-quad sibling) here asserting the blocked outcome, mirror it in the desktop twin's loop and both opt-in loops, and add the 64:ff9b::1:… reserved-bit forms to the NAT64 canonicalization loop above.
中文说明
[Suggestion]（模式发现，位置 2/3——姊妹评论见 packages/desktop/packages/server-core/src/voice/voice.test.ts:256 与 packages/desktop/packages/server-core/src/voice/resolve-voice-config.test.ts:683。）本循环钉住了合法形式的 ::/96 嵌入（::a9fe:a9fe、::6464:64c8、::5db8），却遗漏了在 IPv4-compatible 分类器中 fail-open 的保留位 / 超过 2 个后缀组的同类地址（见本次评审摘要中针对本文件实现类的低置信度 Critical）：::1:a9fe:a9fe、::1:6464:64c8、::1:7f00:1 今天会被分类为公网地址，即使在未受信门禁下也通过，且在两端均无测试。
失败场景：已探针验证——这些逃逸形式在受信与未受信配置下（以及作为 IP 字面量、经由 resolveVoiceTranscriptionConfig）都能通过 assertVoiceBaseUrlNetworkAllowed，而被钉住的合法形式同类地址被阻断；desktop 孪生循环与两个 opt-in 循环存在同样缺口。若分类器被修复为 fail-closed 而没有这些钉子，之后重新引入该漏洞的简化改动在所有测试中仍为绿色。
建议修复：待分类器的处置确定后，在此加入 '::1:a9fe:a9fe'（及一个点分十进制同类地址）并断言阻断结果，同步到 desktop 孪生循环与两个 opt-in 循环，并在上方 NAT64 规范化循环中加入 64:ff9b::1:… 保留位形式。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| it('rejects always-blocked base URLs at config time even when allowlisted', async () => { | ||
| for (const baseUrl of [ | ||
| 'http://169.254.169.254/v1', |
There was a problem hiding this comment.
[Suggestion] (Pattern, site 3/3 — sibling comments at packages/desktop/packages/server-core/src/voice/voice.test.ts:256 and packages/cli/src/ui/voice/voice-transcriber.test.ts:664.) The config-time always-blocked loop pins only well-formed blocked literals and omits the NAT64 reserved-bit class, so the config-time surface gives the escape no signal either — a third suite the two net-guard-loop fixes do not cover.
Failure scenario: probe-verified — today an exact provider with baseUrl: 'https://[64:ff9b::1:203:405]/v1' classifies as public HTTPS (the classifier returns undefined for 3 suffix groups), and classifyVoiceProviderEntry silently falls through to the OAuth/legacy chain instead of rejecting — while its well-formed sibling https://[64:ff9b::169.254.169.254]/v1 throws 'must not use a private-network baseUrl'. If the classifier is fixed fail-closed, this class flips from silent fall-through to hard rejection in both the provider path and the top-level env-path recheck, and nothing in this suite pins either outcome.
Suggested fix: when the classifier disposition is settled, add 'http://[64:ff9b::1:203:405]/v1' (and optionally an ::1:a9fe:a9fe-style IPv4-compatible sibling) to this loop asserting the rejected outcome.
中文说明
[Suggestion]（模式发现，位置 3/3——姊妹评论见 packages/desktop/packages/server-core/src/voice/voice.test.ts:256 与 packages/cli/src/ui/voice/voice-transcriber.test.ts:664。）配置期的 always-blocked 循环只钉住了合法形式的被阻断字面量，遗漏了 NAT64 保留位类，因此配置期面对该逃逸同样没有任何信号——这是两个 net-guard 循环修复覆盖不到的第三套测试。
失败场景：已探针验证——今天一个 baseUrl: 'https://[64:ff9b::1:203:405]/v1' 的精确 provider 会被分类为公网 HTTPS（分类器对 3 个后缀组返回 undefined），classifyVoiceProviderEntry 会静默落入 OAuth/legacy 链而不是拒绝——而其合法形式的同类地址 https://[64:ff9b::169.254.169.254]/v1 会抛出 'must not use a private-network baseUrl'。若分类器被修复为 fail-closed，这一类会从静默落入变为硬拒绝（provider 路径与顶层 env 路径复查均如此），而本套件对两种结果都没有钉子。
建议修复：待分类器的处置确定后，在本循环中加入 'http://[64:ff9b::1:203:405]/v1'（可选再加一个 ::1:a9fe:a9fe 式的 IPv4-compatible 同类地址），断言拒绝结果。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| for (const [key, value] of Object.entries(parseEnvFileContent(content))) { | ||
| if (!Object.hasOwn(env, key) || env[key] === '') { | ||
| result[key] ??= value; |
There was a problem hiding this comment.
[Suggestion] The home-.env fallback freezes an EMPTY value from the more-specific file: result[key] ??= value stores '' (not nullish), blocking a lower-priority candidate's real value — while the CLI's loadEnvironment treats '' as 'effectively unset' per file and lets the later file win. The same two .env files yield different credentials on the two surfaces.
Failure scenario: A/B-probed on both surfaces with identical input (~/.qwen/.env = DASHSCOPE_API_KEY= empty, ~/.env = DASHSCOPE_API_KEY=sk-real, no process-env value): CLI loadEnvironment resolves sk-real; desktop resolveDesktopVoiceConfig rejects with 'Voice dictation needs Qwen credentials'. The one-line flip (skip empty file values) was verified to resolve it, then reverted. Variants: with an id-matching provider naming that envKey it throws 'requires DASHSCOPE_API_KEY'; with OPENAI_API_KEY/OPENAI_BASE_URL set it silently resolves a different endpoint. Distinct from the earlier R7-14 thread (the process-env presence gate, fixed — this gate already includes env[key] === '').
Suggested fix: in the credential-lookup path only, treat an empty-string file value as 'not a definition' so a later candidate can fill it (e.g. skip value === '' here) — leaving the shared homeEnvFallback first-wins semantics that resolveSettingsEnvVars consumes untouched.
中文说明
[Suggestion] home .env 回退会把来自更具体文件的空值冻结下来：result[key] ??= value 会存入 ''（并非 nullish），从而阻止更低优先级候选文件中的真实值——而 CLI 的 loadEnvironment 对每个文件将 '' 视为"实际上未设置"，允许后续文件覆盖。同样的两个 .env 文件在两端会解析出不同的凭证。
失败场景：已在两端用相同输入做 A/B 探针（~/.qwen/.env = DASHSCOPE_API_KEY= 空值，~/.env = DASHSCOPE_API_KEY=sk-real，无进程环境变量）：CLI loadEnvironment 解析出 sk-real；desktop resolveDesktopVoiceConfig 以 'Voice dictation needs Qwen credentials' 拒绝。单行翻转（跳过空文件值）已验证可解决，随后还原。变体：当存在 id 匹配且以该 envKey 命名的 provider 时会抛出 'requires DASHSCOPE_API_KEY'；当设置了 OPENAI_API_KEY/OPENAI_BASE_URL 时会静默解析到另一个端点。与早前的 R7-14 讨论不同（进程环境变量存在性门禁已修复——此处的门禁已包含 env[key] === ''）。
建议修复：仅在凭证查找路径上，把空字符串的文件值视为"未定义"，使后续候选文件可以填充（例如在此处跳过 value === ''）——保持供 resolveSettingsEnvVars 消费的共享 homeEnvFallback 的先到优先语义不变。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| it('rejects loopback DNS results even when trusted, with loopback guidance', async () => { | ||
| for (const address of [ | ||
| '127.0.0.1', |
There was a problem hiding this comment.
[Suggestion] This diff deletes the only untrusted-mode loopback-DNS rejections in this suite (old 127.0.0.1 / ::ffff:127.0.0.1 → /private-network/) and replaces them with a loop running every loopback record with allowInsecureBaseUrl: true — no remaining test exercises untrusted (default) mode against a loopback DNS result, even though the design doc makes 'loopback DNS results are always blocked, with or without an allowlist entry' an explicit behavior change. The CLI twin suite has the same gap.
Failure scenario: probe-verified against all 35 remaining (address, allow, expectation) vectors — today's code blocks untrusted + loopback DNS (no live defect), but a plausible trust-coupled refactor of the block condition (e.g. (!allow && isPrivateNetworkIp(a)) || (allow && isAlwaysBlockedVoiceAddress(a))) passes every remaining pin while flipping the untested combination to ALLOWED, because isPrivateNetworkIp('127.0.0.1') is deliberately false (loopback-exempt). A public hostname resolving to 127.0.0.1 would then carry voice audio to a loopback endpoint with no opt-in.
Suggested fix: add untrusted-mode entries alongside the trusted loop — at minimum '127.0.0.1' and '::ffff:7f00:1' with no allowInsecureBaseUrl, expecting /loopback/ — in this suite and the CLI twin's loopback-DNS loop.
中文说明
[Suggestion] 本 diff 删除了本套件中唯一的未受信模式 loopback-DNS 拒绝用例（旧的 127.0.0.1 / ::ffff:127.0.0.1 → /private-network/），取而代之的是一个对所有 loopback 记录都以 allowInsecureBaseUrl: true 运行的循环——即使设计文档把"loopback DNS 结果始终被阻断，无论是否在白名单中"列为明确的行为变更，剩余的测试中已没有任何一个在未受信（默认）模式下验证 loopback DNS 结果。CLI 孪生套件存在同样缺口。
失败场景：已对剩余全部 35 个（地址, allow, 期望）向量探针验证——今天的代码会阻断未受信 + loopback DNS（无现行缺陷），但一个看似合理的、把阻断条件与信任模式耦合的重构（例如 (!allow && isPrivateNetworkIp(a)) || (allow && isAlwaysBlockedVoiceAddress(a))）能通过所有剩余的钉子，同时把未测试的组合翻转为允许——因为 isPrivateNetworkIp('127.0.0.1') 刻意返回 false（loopback 豁免）。届时一个解析到 127.0.0.1 的公网主机名将在没有任何 opt-in 的情况下把语音音频送往 loopback 端点。
建议修复：在受信循环旁补充未受信模式条目——至少包括不带 allowInsecureBaseUrl 的 '127.0.0.1' 与 '::ffff:7f00:1'，期望 /loopback/——同时覆盖本套件与 CLI 孪生的 loopback-DNS 循环。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenPR: #8350 · Base: This round had no actionable feedback:
Critical-only mode is active (after 10 change-producing rounds). The non-Critical findings listed in the deferred section of this round's feedback remain open for human follow-up, per the ratchet policy; this round did not modify code, resolve threads, or post replies for them. No commits were made; the branch head is unchanged. 中文说明Autofix 审查轮次：未采取任何操作PR： #8350 · 基础分支： 本轮没有可处理的反馈：
当前处于仅处理 Critical 的模式（在 10 个产生改动的轮次之后）。本轮反馈中"已延后"部分列出的非 Critical 问题，按照棘轮策略保持开放、留待人工跟进；本轮没有为这些条目修改代码、解决讨论串或发布回复。 本轮没有产生任何提交；分支 HEAD 保持不变。 Deferred non-Critical feedbackCritical-only mode is active after 10 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 153 passed · 0 failed · 153 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：153 通过 · 0 失败 · 153 总计 Verification reportPR #8350 Deep Verification (round 2) —
|
| # | Previous finding / item | Severity | Status at ce6b4803 |
|---|---|---|---|
| 1 | Central allowlist A/B (4 flip cells, protected classes blocked) | load-bearing | stands — re-measured, 42/42 (01-ab-cli-resolve-base-vs-head.png) |
| 2 | DNS guard: allowlist never defeats always-blocked classes | load-bearing | stands — re-measured, 66/66 across 3 arms (02-dns-guard-three-arm.png) |
| 3 | "PR hardens base SSRF guard: base allowed NAT64-metadata/Teredo/6to4/loopback-rebind/mapped forms" | observation | superseded in part — the new base tip already blocks IPv4-mapped/compatible forms (main absorbed that hardening); remaining deltas re-confirmed: D06 rebind, D11 NAT64-metadata, D12 Teredo, D13 6to4, D14 local-use NAT64. See Corrections. |
| 4 | Workspace cannot self-grant (strip + warn + reject) | load-bearing | stands — re-measured through the real loadSettings, 8/8 (03-trust-boundary-workspace-self-grant.png) |
| 5 | Desktop fail-closed resolution | load-bearing | stands + extended — 20/20 incl. 10 new delta probes (04-desktop-resolve-delta-probes.png) |
| 6 | Vacuity: neutered allowlist turns the acceptance tests red | positive control | stands — re-measured: now exactly 5 tests red (suite grew 47→61), restore green 61/61, diff clean (07-vacuity-positive-control.png) |
| 7 | Generated settings.schema.json byte-identical on regeneration |
artifact check | stands — re-ran generator at new head: zero diff |
| 8 | 12 as-is settings.test.ts failures are environmental |
note | stands — re-observed identical signature (12 failed | 147 passed of 159); green under sanitized env |
| 9 | Desktop bun suite not runnable in container | not covered | stands — still no bun; compensated by tsx harnesses (H2 desktop arm + H4) |
| 10 | Per-commit attribution unreachable | not covered | stands — depth 2; rev-list returns bogus 1 vs 27 commits in metadata |
Scope selected
- Central claim (carried): a voice provider whose
baseUrlis HTTP or private-network is rejected by default but accepted when its exact normalized URL is listed insecurity.allowedInsecureVoiceBaseUrlsfrom a trusted scope — enforced identically across CLI resolution, the CLI/Desktop DNS guard, and Desktop resolution. - Secondary claim 1 (carried): Workspace-scoped allowlist is stripped before merge; a cloned repo cannot self-grant; User/System/SystemDefaults honored.
- Secondary claim 2 (carried): metadata / link-local / unspecified / loopback-alias / transition-IPv6 stay blocked even when listed, in every embedded-IPv4 encoding, including via DNS.
- Delta claims (new this round): (a) desktop provider scan is shape-guarded (non-object entries skipped; non-string
baseUrl/envKey→ remediation error, not crash); (b) same-ID duplicates are classified before the ambiguity check — all-public-HTTPS duplicate sets keep legacy fall-through; exact(id, baseUrl)duplicates keep first registration; (c) the DashScope-compatible/v1rewrite is computed before allowlist match; (d) CLI remediation messages name the honored scopes; (e)providerProtocolis threaded through CLI voice seams; (f) the newcheck-voice-guard-sync.jsmirror check is live and wired into CI.
Out of scope (listed under Not covered): live ASR egress, Desktop bun suite, Windows/Linux runtime, per-commit attribution.
Central claim — A/B load-bearing proof (re-measured)
21 scenarios driven through the real resolveVoiceTranscriptionConfig from head and from a base worktree at HEAD^1 (8fd0162c6). Base control is clean: git diff HEAD^1..HEAD -- packages/core is empty, and readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core, so both arms load identical core; the base arm provably ran base code (its cells emit the pre-PR messages, e.g. S02 lacks the allowlist hint, S18 strips credentials). Witness: 01-ab-cli-resolve-base-vs-head.png. 42/42.
| Cell | Base | Head | Oracle |
|---|---|---|---|
| S01 public https, no allowlist (control) | resolve | resolve | unchanged |
| S02 private http gateway, EXACT allowlist | throw | resolve(insecure) | load-bearing flip |
| S03 private http, no allowlist | throw | throw | fail closed |
| S04/S05/S06 wrong port / path / scheme in entry | throw | throw | exact match enforced |
| S07 trailing-slash baseUrl vs no-slash entry | throw | resolve(insecure) | normalization flip |
| S08–S14 metadata / AWS-IPv6-metadata / fe80 / 0.0.0.0 / 127.0.0.2 / ::ffff:127.0.0.1 / 100.100.100.200, all allowlisted | throw | throw | always-blocked beats allowlist |
S15 ftp://, allowlisted |
throw | throw (http/https gate) | scheme gate independent |
| S16 CGNAT 100.64.0.1, EXACT | throw | resolve(insecure) | flip |
| S17 ULA fd12:3456::1, EXACT | throw | resolve(insecure) | flip |
| S18 embedded credentials, allowlisted | resolve (creds stripped) | throw | hardening flip (documented) |
| S19 remediation names scopes + exact URL | throw (no hint) | throw w/ "only honored from User, System, or SystemDefaults" | delta (round-8) |
S20 entry with /v1 vs bare-path baseUrl |
throw | throw | CLI infers no /v1 |
| S21 explicit localhost spelling (control) | resolve | resolve | unchanged |
DNS guard — 3-arm sweep (re-measured + extended)
assertVoiceBaseUrlNetworkAllowed driven with injected lookupHost on base CLI / head CLI / head desktop; 22 cells × 3 arms. Witness: 02-dns-guard-three-arm.png. 66/66.
- Feature over DNS: allowlisted host → private / mapped-private / mixed [public, private] records allowed on head, blocked on base (D01/D09/D21).
- Always-blocked beats the flag, including over DNS: metadata, AWS-IPv6 metadata, link-local, mixed [public, metadata], Aliyun metadata (D03–D05/D08/D17).
- Hardening deltas vs the current base: D06 rebind→127.0.0.1, D11 NAT64-wrapped metadata
64:ff9b::a9fe:a9fe, D12 Teredo2001::1, D13 6to42002::1, D14 local-use NAT6464:ff9b:1::1— base allows, both head arms block. IPv4-mapped/compatible forms (D10/D15/D16/D18) are already blocked on the new base (see Corrections). - D22: mapped-PUBLIC dotted
::ffff:8.8.8.8is allowed on both arms (base's dotted-quad recursion and head's precise unwrap agree); the base's blanket::ffff:rule only fires on hex-form mapped addresses (visible in D10), which head instead classifies by the embedded IPv4. - CLI and Desktop guards agree on all 22 cells.
Reviewer-plan step 4 ladder — all seven encodings of 169.254.169.254 (dotted; mapped dotted/hex; IPv4-compatible compact and fully expanded; NAT64 hex and dotted-suffix), flag ON, both surfaces: 7/7 blocked (06-metadata-ladder-and-gate-counts.png). URL canonicalization observed (expanded→compressed, dotted-suffix→hex), so guards see one canonical form.
Trust boundary — Workspace cannot self-grant (re-measured)
Real loadSettings() over temp QWEN_HOME / system paths / workspace dir → real resolver. Witness: 03-trust-boundary-workspace-self-grant.png. 8/8.
- T01: workspace-only allowlist is absent from
merged, the self-granted gateway is rejected end to end, and the warning names the workspace file. - T03: a workspace entry cannot augment a User allowlist (merged keeps only the User entry; still rejected).
- T02/T04/T05: User, SystemDefaults, and System scopes are honored.
Desktop resolution — fail-closed + delta probes
resolveDesktopVoiceConfig driven via tsx with injected readers (bun unavailable; see Not covered). Witness: 04-desktop-resolve-delta-probes.png. 20/20. Carried cells (exact-match resolve insecure / fail closed naming the exact URL / ambiguous duplicates / missing baseUrl falls through / missing key fails / public-https legacy precedence / SystemDefaults honored / env fallback) all hold, plus the new delta probes:
- DR09 non-object
modelProviderselements ("junk",null,42) are skipped without crashing (round-8 shape guard). - DR10/DR11 non-string
baseUrl/ non-stringenvKey→PROVIDER_ENTRY_REMEDYerrors, not TypeErrors (round-8 shape guard). - DR12 unparseable baseUrl → logged warning + legacy fall-through (never a silent different-endpoint switch).
- DR13 two same-ID public-HTTPS entries → no ambiguity error; legacy fall-through preserved (round-10 "classify before ambiguity").
- DR14 exact
(id, baseUrl)duplicates with differentenvKey→ first registration wins (requires KEY_Awhen onlyKEY_Bis set), matching the CLI registry's composite key. - DR15 DashScope-compatible
/v1rewrite computed before the allowlist match: entry…/compatible-mode+ allowlist…/compatible-mode/v1resolves with the flag (round-8 ordering fix). - DR16 custom gateways get no
/v1inference: a/v1-suffixed entry does not match a bare path; the exact entry does. - DR17 top-level recheck: an OAuth-derived private-HTTP
resource_urlfails closed, and the rejection names the effective normalized URL with the inferred/v1(http://10.9.9.9:8080/asr/v1) — exactly the PR-description claim; allowlisting that effective URL resolves. - DR18 keyless gateway entry (no
envKey) resolves without an API key (CLI parity).
Mirror-sync script (new in this round) — live, wired to CI
npm run check:voice-guard-sync passes at head; ci.yml runs it under the full profile. Positive/negative controls via its exported API (05-mirror-script-mutations.png), 7/7:
- P0 unmutated sources → 0 drift across both mirror sets (15 net-guard units +
deriveWebSocketBase/deriveStreamUrl). - M1 CGNAT-clause deletion, M2 prefix-length literal, M3 mutation inside a string literal, M4 deleted unit → all reported.
- T1 comment insertion, T2 brace-style restyle → tolerated (no false positives). The CLI↔desktop
exportasymmetry is tolerated by construction (P0 runs over the real files, which differ exactly that way).
Corrections
- Base-drift correction to previous-round framing (not a code issue): the previous report stated base allowed mapped-loopback/mapped-metadata/IPv4-compatible forms over DNS. At the previous base
41f0e3cthat was true; the current base tip8fd0162c6has since absorbed IPv4-mapped/compatible unwrapping plus a blanket::ffff:block inisPrivateNetworkIp, so those cells now block on base too (D10/D15/D16/D18 re-measured). The PR's remaining hardening delta over the current base is D06/D11/D12/D13/D14 as listed above. The previous round's observation 1 ("worth a release-note mention") still holds for those five. - The previous report's vacuity count (4 red tests) is superseded: the suite grew to 61 tests and the neutered allowlist now turns exactly 5 red — the two new "reaches the batch transport for an allowlisted …" tests added since are also load-bearing.
Findings
None blocking. No allowlist bypass, no trust-boundary breach, no protected-class escape (IP-literal or DNS-resolved, all seven embedded encodings), no fail-open on ambiguous/malformed Desktop providers, no false-drift from the mirror script.
Observations (informational):
scripts/tests/install-script.test.jsaborts at load in this container while collecting the scripts gate: its CI guard throws becausezipis missing (onlyunzipexists;CI=true). The PR touches no install-script files (verified via--name-only), 0 tests from that file executed, and all 843 executed script tests passed. Environmental to this image, not attributable to the PR.- Hex-form mapped addresses are classified precisely on head: base's blanket
::ffff:→private rule (measured blocking hex mapped metadata in D10, flag ON) also blocks hex-form mapped public addresses; head unwraps and classifies by the embedded IPv4, so a hex-form mapped public address is allowed while embedded private stays gated on the flag and embedded metadata/loopback stays blocked (D10/D15/D16/D18/D21). A permissive-direction precision change limited to public embedded addresses — no bypass measured. - The desktop
testscript now runs*.isolated.tsfiles in separatebun testprocesses (round-8 fix for the process-globalmock.module('ws')leak). The mechanism reads correctly (package.jsontest script loops overfind -name '*.isolated.ts'), but could not be executed here — no bun in container.
Not covered
- Desktop bun suites (
bun test packages/server-core/src/voice, author claims 109/109, and the renamedvoice-ws-handler.isolated.ts): bun is not installed andpackages/desktop/node_modulesis absent. Compensated by driving the real Desktop TS source via tsx — DNS guard arm (22 cells) andresolveDesktopVoiceConfig(20 assertions). The bun-runner run itself remains untested here. - Live ASR request: none sent (requires private gateway + credentials). This round reproduces the handling (policy decision, credential resolution, network guard), not the wire trigger.
- Per-commit attribution: 27 commits in the metadata snapshot; depth-2 checkout makes only the merge parents reachable (
git rev-list --count HEAD^1..HEAD^2returns the bogus1at the shallow boundary — verified, not trusted). The aggregateHEAD^1..HEADdiff is what was verified. providerProtocolthreading (round-8) verified at the suite level —voice-service.test.tspins "resolves voice models in providerProtocol-mapped custom provider groups" (green inside the 300/300 gate) — not probed through a live model registry in this round.- Windows/Linux runtime; repo-wide
npm run preflight/npm run typechecknot re-run — the environment contract guaranteesnpm ci+npm run buildsucceeded at HEAD before this round. - The documented CLI↔desktop divergence (desktop scans all provider groups protocol-agnostically; CLI requires a registry-visible group) was read in the design doc and is pinned by the PR's own desktop parity tests, but not independently probed cell-by-cell.
Methodology
Ran in the CI verify container (node:22-bookworm, node v22.23.2, tsx via root node_modules; no bun, no zip). Working tree = refs/pull/8350/merge (merge 20ee5e432, base HEAD^1=8fd0162c6, head HEAD^2=ce6b48032), depth 2. Six mock-free harnesses (in this dir) drive real compiled/source modules — no stub of code under test: ab-cli-resolve.mts (42), dns-guard.mts (66), trust-boundary.mts (8, real loadSettings on temp files), desktop-resolve.mts (20), mirror-mutation.mjs (7, via the script's exported API — no repo files mutated), ipv6-ladder.mts (7). Base arm from git worktree add tmp/base-tree HEAD^1, removed after capture; base control clean because packages/core is unchanged (verified empty diff) and the core symlink realpath was quoted before trusting it. Vacuity: isInsecureVoiceBaseUrlAllowed neutered to return false via a direct edit, suite ran red on exactly the 5 opt-in tests with behavioral error messages, source restored, suite 61/61 green, git status clean. Targeted gate ran the six voice/settings CLI suites under a sanitized environment (the container pre-sets QWEN_HOME and ~20 QWEN_* vars, which break 12 unrelated env-handling tests — re-confirmed as-is this round). Scripts gate: npm run test:scripts. Regenerated settings.schema.json and diffed against the committed artifact (zero diff). Raw logs in logs/; terminal witnesses in evidence/ via scripts/verify-capture.mjs. No GitHub writes attempted; PR text treated as untrusted input and contained no injection directives.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action takenThis round found no actionable feedback on PR #8350:
The PR is in Critical-only mode after 10 change-producing rounds. The deferred non-Critical feedback listed for this round (one automated-reviewer PR comment) was intentionally left untouched per the Critical-only rules: no code change, no thread resolution, and no reply for it. It remains open for human follow-up. No code changes were made and no commits were created in this round. 中文说明Autofix 评审轮次 — 未采取任何操作本轮在 PR #8350 上未发现可处理的反馈：
该 PR 在经历 10 个产生改动的轮次后已进入仅处理 Critical 的模式。本轮列出的延后非 Critical 反馈（一条自动评审器的 PR 评论）按 Critical-only 规则有意未做处理：不为它修改代码、不解决相关话题、也不回复。该条目保持开放，留待人工跟进。 本轮未做任何代码改动，也未创建任何提交。 Deferred non-Critical feedbackCritical-only mode is active after 10 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Maintainer verification — real local environment, real network egressI built this PR from source and exercised the policy against a real cleartext-HTTP ASR gateway bound to this machine's actual private LAN address ( Verdict: the contract in the PR description holds on every case I could construct. LGTM to merge, with three small notes below (one worth a follow-up line of code, two worth a sentence in the description).
1. Batch path — before vs. after, same machine, same gateway
Full matrix. ALLOW = the daemon returned a transcript that came back from the private gateway; DENY = the URL policy rejected it;
* S14 passed the URL policy on both builds and then failed at the TLS handshake (deliberately: the hostname resolves to a public IP whose certificate does not match, so the audio body is never transmitted). This confirms public HTTPS still needs no allowlist entry — the change is scoped to cleartext/private endpoints. Notable details from the gateway log on the ALLOW rows: The provider key really is on the wire in cleartext — the risk the settings description warns about is accurate, and it is worth keeping that wording prominent. S11 also exercises normalization end-to-end: the entry 2. Streaming path — daemon
|
| Command | Result |
|---|---|
vitest run on the 6 changed CLI suites |
300 passed |
npm run test:scripts → check-voice-guard-sync.test.js |
11 passed |
npm run check:voice-guard-sync |
passed |
bun test packages/server-core/src/voice |
135 passed (8 files) |
bun test ./…/voice-ws-handler.isolated.ts |
15 passed |
npm run typecheck · npm run lint:ci · desktop tsc --noEmit |
all passed |
Passing tests only prove the tests run, so I broke the implementation three ways to check the tests and the new CI gate actually bind the contract:
- Removed the
100.100.100.200metadata check from the desktop mirror only →check:voice-guard-syncfailed withisAlwaysBlockedVoiceAddress: bodies differ. The new drift gate is real, not decorative — this is the most valuable thing the PR adds beyond the feature itself. - Made the allowlist match on origin instead of the full normalized URL → 5 tests failed. Path/port/scheme exactness is pinned.
- Stopped stripping the setting from workspace settings → 2 tests failed. The trust boundary is pinned.
Notes (none blocking)
a) The remediation hint is truncated exactly where it matters, on the daemon path. The new message is 363 chars; sanitizeVoiceErrorMessage cuts at MAX_TRANSCRIPTION_ERROR_LENGTH = 200 (pre-existing). The fixed prefix before the URL is 177 chars, so only ~23 characters of URL survive. What an operator actually sees in the daemon log is:
… add its exact complete normalized URL (http://10.177.107.157:...
The HTTP client always gets the generic voice_transcription_failed, so this log line is the only place the actionable URL appears — and any realistic managed hostname is longer than 23 chars. The TUI path is unaffected (the message reaches the history item unmodified). Cheapest fix: shorten the sentence, or log the URL as its own field. Worth one follow-up commit; not a reason to hold the merge.
b) Embedded credentials in baseUrl now hard-fail (behavior change, verified A/B). main silently stripped user:pass@; this PR throws Voice model '…' baseUrl must not contain embedded credentials. I confirmed both builds on the same input. Failing loudly is the right call, but it belongs in Breaking changes / migration notes next to the Desktop credential change.
c) providerProtocol is now honored for voice model discovery — a real functional change beyond the allowlist. On main, a custom provider id mapped through providerProtocol yields availableVoiceModels: [] and 400 unknown_voice_model; with the two added providerProtocolConfig: lines it resolves normally. That is a genuine (welcome) fix, but it is invisible in the PR description and untestable from the stated test plan — worth a sentence so reviewers know it is intentional.
Not covered by this run
Desktop was verified by its unit suites and typecheck only — I did not launch the Electron app, so the ambiguous-duplicate-provider fail-closed behavior is unit-verified, not runtime-verified. Windows and Linux untested. No real vendor ASR service was contacted; the gateway is a local stand-in that speaks both wire protocols.
中文版本
维护者验证 —— 本地真实环境、真实网络出口
我从源码构建了本 PR，并把策略放到绑定在本机真实私网地址上的真实明文 HTTP ASR 网关（10.177.107.157:8791）前验证，由真实 qwen serve daemon 驱动，使用真实 16 kHz WAV 音频 —— 没有 mock fetch，也没有注入 lookup。判据是网关自己的请求日志：音频和 Authorization 头到底有没有真的离开进程。
结论：PR 描述中的契约在我能构造的每个用例上都成立，建议合并，另有三点说明（一点值得一行后续代码，两点值得在描述里补一句）。
| PR head | ce6b480（已 merge main） |
| 基线 | main @ 8fd0162 —— 同一 worktree、同一 node_modules，只替换 packages/cli/src 后重新 bundle |
| 主机 | macOS（Darwin 25.6.0）、Node v24.18.1、npm 11.16.0、Bun 1.3.14 |
| 覆盖面 | daemon POST /workspace/voice/transcribe（批量）· daemon /voice/stream WebSocket（实时）· CLI TUI 启动（设置警告） |
| 隔离 | 独立 QWEN_HOME、独立 workspace，不使用真实 provider 凭证 |
1. 批量路径 —— 同一台机器、同一网关的 before / after
ALLOW = daemon 返回了来自私网网关的转写结果；DENY = URL 策略拒绝；gw+1 = 网关确实收到了请求。所有 DENY 行的网关请求数都是 0 —— 策略在出网之前 fail closed，而不是之后。
| # | 场景 | BEFORE | AFTER |
|---|---|---|---|
| S1 | HTTP 私网 IP 网关，无白名单条目 | DENY | DENY |
| S2 | HTTP 私网 IP 网关，User 作用域精确条目 | DENY | ALLOW gw+1 |
| S3 | 仅 path 不同（/v1 → /v2） |
DENY | DENY |
| S4 | 仅端口不同 | DENY | DENY |
| S5 | 仅 scheme 不同（https） |
DENY | DENY |
| S6 | 条目只写在 Workspace 作用域（仓库自授权） | DENY | DENY |
| S7 | 条目多写了尾部斜杠（/v1///） |
DENY | ALLOW gw+1 |
| S8 | DNS 解析到私网 IP 的域名，已加白 | DENY | ALLOW gw+1 |
| S9 | DNS 解析到 loopback 的域名，已加白 | DENY | DENY |
| S10 | 云元数据地址 169.254.169.254，已加白 |
DENY | DENY |
| S11 | 私网 IP 的 IPv4-mapped IPv6 写法，已加白 | DENY | ALLOW gw+1 |
| S12 | 127.0.0.1 的 IPv4-mapped IPv6 写法，已加白 |
DENY | DENY |
| S13 | 到私网 IP 的 HTTPS，无白名单条目 | DENY | DENY |
| S14 | 公网 HTTPS provider，无白名单条目 | 策略放行* | 策略放行* |
* S14 在两个构建上都通过了 URL 策略，随后在 TLS 握手阶段失败（这是刻意设计：域名解析到一个证书不匹配的公网 IP，因此音频体从未发出）。这印证了公网 HTTPS 仍然不需要白名单条目 —— 改动只作用于明文/私网端点。
ALLOW 行上网关日志的关键信息：
host=10.177.107.157:8791 auth=Bearer sk-regional-gateway-secret-8350
model=qwen3-asr-flash format=wav base64AudioChars=110950
provider key 确实以明文出现在链路上 —— 设置项描述里的风险提示是准确的，这段措辞值得保留在显眼位置。
S11 同时端到端验证了规范化：条目 http://[::ffff:10.177.107.157]:8791/v1 与请求都规范化为 http://[::ffff:ab1:6b9d]:8791/v1，网关观察到的 Host 头也正是这个值。
2. 流式路径 —— daemon /voice/stream WebSocket
我在同一个私网网关上实现了 qwen-asr-realtime 协议（session.created → session.update/updated → input_audio_buffer.append/commit → session.finish/finished），并通过 daemon 的客户端 WebSocket 推入真实 PCM。
没有白名单条目时，根本不会发起上游连接 —— 网关看不到任何 upgrade 请求。加白后，26 个实时帧 / 83 150 字节 PCM 到达网关并返回转写结果。批量与流式两条腿给出一致的解析结果，与 PR 描述相符。
3. 信任边界 —— Workspace 作用域既被忽略，也有提示
不可信仓库在 .qwen/settings.json 中写入 security.allowedInsecureVoiceBaseUrls 无法自授权（见 S6），并且 CLI 启动时会明确告知原因（见英文部分截图）。
4. 测试套件、门禁与变异测试
| 命令 | 结果 |
|---|---|
对 6 个改动 CLI 套件执行 vitest run |
300 通过 |
npm run test:scripts → check-voice-guard-sync.test.js |
11 通过 |
npm run check:voice-guard-sync |
通过 |
bun test packages/server-core/src/voice |
135 通过（8 个文件） |
bun test ./…/voice-ws-handler.isolated.ts |
15 通过 |
npm run typecheck · npm run lint:ci · desktop tsc --noEmit |
全部通过 |
测试通过只能证明测试跑了，所以我用三种方式破坏实现，检验测试和新 CI 门禁是否真的锁住了契约：
- 只在 desktop 镜像里删掉
100.100.100.200元数据判断 →check:voice-guard-sync以isAlwaysBlockedVoiceAddress: bodies differ失败。这个新增漂移门禁是真的有效，而不是摆设 —— 这是本 PR 除功能本身之外最有价值的部分。 - 把白名单匹配从完整规范化 URL 降级为只比 origin → 5 个测试失败。path/port/scheme 的精确性被锁住了。
- 不再从 workspace 设置中剥离该项 → 2 个测试失败。信任边界被锁住了。
说明（均不阻塞合并）
a) 在 daemon 路径上，补救提示恰好在关键处被截断。 新消息长 363 字符；sanitizeVoiceErrorMessage 在 MAX_TRANSCRIPTION_ERROR_LENGTH = 200 处截断（该常量是既有的）。URL 之前的固定前缀是 177 字符，所以只有约 23 个字符的 URL 能留下。运维在 daemon 日志里实际看到的是：
… add its exact complete normalized URL (http://10.177.107.157:...
HTTP 客户端拿到的始终是通用的 voice_transcription_failed，所以这行日志是运维唯一能看到可操作 URL 的地方 —— 而任何现实中的受管域名都长于 23 个字符。TUI 路径不受影响（消息原样进入 history item）。最省事的修法：缩短这句话，或把 URL 作为独立字段输出。值得一个后续 commit，不必因此拦住合并。
b) baseUrl 内嵌凭证现在会硬失败（行为变更，已 A/B 验证）。 main 会静默剥离 user:pass@；本 PR 抛出 Voice model '…' baseUrl must not contain embedded credentials. 我在同一输入上确认了两个构建的差异。显式失败是正确取舍，但这一条应当写进破坏性变更/迁移说明，与 Desktop 凭证变更并列。
c) providerProtocol 现在会参与语音模型发现 —— 这是白名单之外的真实功能变更。 在 main 上，通过 providerProtocol 映射的自定义 provider id 会得到 availableVoiceModels: [] 和 400 unknown_voice_model；加上新增的两行 providerProtocolConfig: 后即可正常解析。这是一个真实且值得欢迎的修复，但在 PR 描述里完全看不到，按现有测试计划也测不出来 —— 建议补一句，让 reviewer 知道这是有意为之。
本次未覆盖
Desktop 仅通过单元套件和类型检查验证，我没有启动 Electron 应用，因此"重复 provider 歧义 fail closed"是单测层面验证，而非运行时验证。Windows 与 Linux 未测试。没有访问任何真实厂商 ASR 服务；网关是本地替身，同时实现了两种线上协议。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action takenThis round found no actionable feedback on PR #8350:
The PR is in Critical-only mode after 10 change-producing rounds. The deferred non-Critical feedback listed for this round (one maintainer PR comment — a local verification report concluding "LGTM to merge" with non-blocking notes) was intentionally left untouched per the Critical-only rules: no code change, no thread resolution, and no reply for it. It remains open for human follow-up. No code changes were made and no commits were created in this round. 中文说明Autofix 评审轮次 — 未采取任何操作本轮在 PR #8350 上未发现可处理的反馈：
该 PR 在经历 10 个产生改动的轮次后已进入仅处理 Critical 的模式。本轮列出的延后非 Critical 反馈（一条维护者的 PR 评论 —— 一份本地验证报告，结论为 "LGTM to merge"，附若干非阻塞备注）按 Critical-only 规则有意未做处理：不为它修改代码、不解决相关话题、也不回复。该条目保持开放，留待人工跟进。 本轮未做任何代码改动，也未创建任何提交。 Deferred non-Critical feedbackCritical-only mode is active after 10 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 160 passed · 0 failed · 160 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：160 通过 · 0 失败 · 160 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - round 3 (follow-up) Scripted assertions: 160 passed · 0 failed · 160 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证容器中对该 PR 重新执行了全部测量（与 base 构建 A/B 对照、免 mock harness、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。
Verification reportPR #8350 Deep Verification (round 3) —
|
| # | Previous finding / item | Severity | Status at ce6b4803 (re-measured) |
|---|---|---|---|
| 1 | Central allowlist A/B (4 flip cells + S18 hardening, protected classes blocked) | load-bearing | stands — re-measured 42/42 (01-ab-cli-resolve-base-vs-head.png) |
| 2 | DNS guard 3-arm sweep, allowlist never defeats always-blocked over DNS | load-bearing | stands — re-measured 66/66 + 7-rung ladder (02-dns-guard-three-arm.png) |
| 3 | Hardening delta over base: rebind→loopback, NAT64-metadata, Teredo, 6to4, local-use NAT64 | observation | stands — D06/D11–D14 re-confirmed (base allows, head blocks on both surfaces); mapped/compatible forms (D10/D15/D16/D19) converge to block-on-both as before |
| 4 | Workspace cannot self-grant (strip + warn + reject) | load-bearing | stands — re-measured through real loadSettings, 8/8 (03-trust-boundary-workspace-self-grant.png) |
| 5 | Desktop fail-closed resolution + shape guards + duplicate classification | load-bearing | stands — re-measured 20/20 (04-desktop-resolve-delta-probes.png) |
| 6 | Vacuity: neutered allowlist turns exactly the opt-in tests red | positive control | stands — re-measured: exactly 5 red (scripted set-equality vs independently derived expectation), restore green 61/61 inside 300/300, git clean (06-vacuity-neutered-red.png) |
| 7 | Generated settings.schema.json byte-identical on regeneration |
artifact check | stands — re-ran npm run generate:settings-schema: zero diff |
| 8 | 12 as-is settings.test.ts failures are environmental |
note | stands — identical signature re-observed (12 failed | 147 passed of 159); green under sanitized env |
| 9 | Desktop bun suites not runnable in container | not covered | stands — still no bun; compensated by tsx-driven H2 desktop arm + H4 |
| 10 | Per-commit attribution unreachable | not covered | stands — depth 2; git rev-list --count HEAD^1..HEAD^2 returns bogus 1 vs 31 commits in the metadata snapshot |
Scope selected
- Central claim (carried): a voice provider whose
baseUrlis HTTP or private-network is rejected by default but accepted when its exact normalized URL is listed insecurity.allowedInsecureVoiceBaseUrlsfrom a trusted scope — enforced identically across CLI resolution, the CLI/Desktop DNS guard, and Desktop resolution. - Secondary claim 1 (carried): Workspace-scoped allowlist is stripped before merge; a cloned repo cannot self-grant; User/System/SystemDefaults honored.
- Secondary claim 2 (carried): metadata / link-local / unspecified / loopback-alias / transition-IPv6 stay blocked even when listed, in every embedded-IPv4 encoding, including via DNS.
- Delta claims (carried from round 2, re-probed): desktop shape guards, classify-before-ambiguity,
/v1rewrite before allowlist match, remediation scope text,providerProtocolthreading (suite-level), mirror-sync script live and CI-wired.
Out of scope (listed under Not covered): live ASR egress, Desktop bun suite, Windows/Linux runtime, per-commit attribution.
Central claim — A/B load-bearing proof (re-measured)
21 scenarios driven through the real resolveVoiceTranscriptionConfig from head and from a base worktree at HEAD^1 (8fd0162c6). Base control clean: git diff HEAD^1..HEAD -- packages/core is empty, and readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core (shared, unchanged core); the base arm provably ran base code (its cells emit the pre-PR messages, e.g. S02 throws, S18 resolves with credentials stripped). Witness: 01-ab-cli-resolve-base-vs-head.png. 42/42.
| Cell | Base | Head | Oracle |
|---|---|---|---|
| S01 public https, no allowlist (control) | resolve | resolve | unchanged, no flag |
| S02 private http gateway, EXACT allowlist | throw | resolve(insecure=true) | load-bearing flip |
| S03 private http, no allowlist | throw | throw w/ remediation naming URL + scopes | fail closed |
| S04/S05/S06 wrong port / path / scheme in entry | throw | throw | exact match enforced |
| S07 trailing-slash baseUrl vs no-slash entry | throw | resolve(insecure=true) | normalization flip |
| S08–S14 metadata / AWS-IPv6-metadata / fe80 / 0.0.0.0 / 127.0.0.2 / ::ffff:127.0.0.1 / 100.100.100.200, all allowlisted | throw | throw (no remediation offered) | always-blocked beats allowlist |
S15 ftp://, allowlisted |
throw | throw ("must use an http or https baseUrl") | scheme gate independent |
| S16 CGNAT 100.64.0.1, EXACT | throw | resolve(insecure=true) | flip |
| S17 ULA fd12:3456::1, EXACT | throw | resolve(insecure=true) | flip |
| S18 embedded credentials, allowlisted | resolve (creds stripped) | throw | hardening flip |
| S19 remediation names scopes + exact URL | throw (no hint) | throw w/ "only honored from User, System, or SystemDefaults" | round-8 message |
S20 entry with /v1 vs bare-path baseUrl |
throw | throw | CLI infers no /v1 |
| S21 explicit localhost spelling (control) | resolve | resolve | unchanged, no flag |
Flag presence/absence asserted on every resolve cell (head sets allowInsecureBaseUrl iff the scenario allowlisted the URL; base never does).
DNS guard — 3-arm sweep + ladder (re-measured)
assertVoiceBaseUrlNetworkAllowed driven with injected lookupHost on base CLI / head CLI / head desktop; 22 cells × 3 arms + 7-rung ladder. Witness: 02-dns-guard-three-arm.png. 73/73.
- Feature over DNS (flag ON): private, dotted-mapped-private, ULA, mixed [public, private] records allowed on head, blocked on base (D01/D09/D18/D20/D21).
- Always-blocked beats the flag over DNS: metadata, AWS-IPv6 metadata, mixed [public, metadata], Aliyun metadata (D03–D05/D08/D17/D19) — dedicated "always blocked … even when listed" message asserted.
- Hardening deltas vs base: D06 rebind→127.0.0.1, D11 NAT64-wrapped metadata
64:ff9b::a9fe:a9fe, D12 Teredo2001::1, D13 6to42002::1, D14 local-use NAT6464:ff9b:1::1— base allows, both head arms block. - Convergence cells: D10 hex-mapped PUBLIC (base blanket
::ffff:blocks; head allows — permissive-direction precision, no bypass), D15/D16 IPv4-compatible compact/expanded metadata (both block; canonicalization observed: expanded→compressed, dotted-suffix→hex). - D22 lookup failure fail-closed on both sides; CLI and desktop arms agree on all 22 cells.
- Reviewer-plan step 4 ladder: all seven encodings of
169.254.169.254(dotted; mapped dotted/hex; compatible compact and fully expanded; NAT64 hex and dotted-suffix), flag ON, blocked on both head surfaces, with canonical hostnames quoted in the witness.
Trust boundary — Workspace cannot self-grant (re-measured)
Real loadSettings() over temp QWEN_HOME / system paths / workspace dir → real resolver. Witness: 03-trust-boundary-workspace-self-grant.png. 8/8.
- T01: workspace-only allowlist absent from
merged; self-granted gateway rejected end to end; warning names the workspace file. - T03: workspace entry cannot augment a User allowlist (merged keeps only the User entry; injected gateway rejected).
- T02/T04/T05: User, SystemDefaults, and System scopes honored.
Desktop resolution — fail-closed + delta probes (re-measured)
resolveDesktopVoiceConfig driven via tsx with the module's own injected readers (bun unavailable; see Not covered). Witness: 04-desktop-resolve-delta-probes.png. 20/20. Exact-match resolve with flag / fail-closed naming the exact URL / ambiguous policy-bearing duplicates / missing-baseUrl fall-through / missing key fails / public-HTTPS legacy precedence / SystemDefaults honored / non-object entries skipped / non-string baseUrl/envKey → PROVIDER_ENTRY_REMEDY / unparseable baseUrl warns + falls through (no silent endpoint switch) / all-public duplicates keep fall-through / exact (id, baseUrl) duplicate keeps first registration (requires KEY_A with only KEY_B set) / DashScope /v1 rewrite before allowlist match / no /v1 inference for custom gateways / OAuth private resource_url fails closed naming the effective http://10.9.9.9:8080/asr/v1 (allowlisting it resolves) / keyless gateway resolves keyless.
Mirror-sync script — live, CI-wired, mutation-proof (re-measured + extended)
npm run check:voice-guard-sync passes at head; ci.yml runs it under the full profile (diff reviewed). Positive/negative controls via its exported API plus a new on-disk negative E2E this round (mutate the real mirrored file, run the npm command, expect exit 1, restore, verify git diff clean). Witness: 05-mirror-script-mutations.png. 10/10.
- P0 both mirror sets (15 net-guard units +
deriveWebSocketBase/deriveStreamUrl): 0 drift on the real files. - M1 CGNAT-clause deletion, M2 prefix-length literal, M3 mutation inside a string literal, M4 deleted unit → all reported; on-disk CGNAT mutation →
npm run check:voice-guard-syncexit 1 namingisPrivateNetworkIp: bodies differ. - T1 comment insertion, T2 brace-style restyle → tolerated.
Vacuity + generated artifact (re-measured)
- Neutered
isInsecureVoiceBaseUrlAllowedtoreturn false(interface-preserving): suite exits red on exactly the 5 opt-in tests (scripted set-equality against an expectation derived independently from the test file), each failing with the behavioral fail-closed error the allowlist exists to relax; restore green 61/61 inside the 300/300 gate;git diffclean. Witness:06-vacuity-neutered-red.png. npm run generate:settings-schemare-run at head: committedsettings.schema.jsonregenerated byte-identical (zero diff, clean status).
Targeted gates
CLI: six voice/settings suites under sanitized env — 300/300 (07-cli-focused-gate-300.png). Scripts: npm run test:scripts — 843/843 executed (one file aborts at load, environmental; see Observations).
Corrections
No new corrections. Round 2's base-drift correction (the previous base allowed mapped/compatible forms; the current base tip has absorbed that hardening) re-verified against the same base OID 8fd0162c6 — the base arm of H2 reproduces it exactly.
Findings
None blocking. No allowlist bypass, no trust-boundary breach, no protected-class escape (IP-literal or DNS-resolved, all seven encodings), no fail-open on ambiguous/malformed Desktop providers, no false-drift from the mirror script, no vacuous acceptance test.
Observations (informational):
- Message asymmetry for always-blocked + allowlisted addresses (new this round, nit). The DNS-record branch of
assertVoiceBaseUrlNetworkAllowedemits a dedicated "resolved to an address that is always blocked (metadata, link-local, or transition range), even when the baseUrl is listed in security.allowedInsecureVoiceBaseUrls" message, but the IP-literal branch (D11-style inputs) emits the generic "resolved to a private-network address.", and the resolver emits "must not use a private-network baseUrl." without any hint that the address class can never be allowlisted. The block itself holds on all three paths (asserted); only the operator-facing explanation differs. - Hex-form mapped PUBLIC addresses are allowed on head (D10) where base's blanket
::ffff:rule blocked them — a permissive-direction precision change limited to public embedded addresses; embedded private/metadata/loopback remain gated (D09/D19/D06). Carried from round 2; re-measured. scripts/tests/install-script.test.jsaborts at load in this container: its CI guard throws becausezipis missing (onlyunzipexists;CI=true). The PR touches no install-script files; 0 tests from that file executed; all 843 executed script tests passed. Environmental.- 12 as-is
settings.test.tsfailures are environmental: the container pre-setsQWEN_HOMEand ~20QWEN_*vars that break unrelated env-handling tests; identical signature to round 2; green under sanitized env (inside the 300/300 gate).
Not covered
- Desktop bun suites (
bun test packages/server-core/src/voice, author claims 109/109, and thevoice-ws-handler.isolated.tsisolated-runner mechanism): bun is not installed in this image. Compensated by driving the real Desktop TS source via tsx — DNS guard arm (22 cells) andresolveDesktopVoiceConfig(20 assertions). The bun-runner execution itself remains untested here. - Live ASR request: none sent (requires a private gateway + credentials). This round reproduces the handling (policy decision, credential resolution, network guard), not the wire trigger.
- Per-commit attribution: 31 commits in the metadata snapshot; depth-2 checkout makes only the merge parents reachable (
git rev-list --count HEAD^1..HEAD^2returns the bogus1— verified, not trusted). The aggregateHEAD^1..HEADdiff is what was verified. providerProtocolthreading verified at the suite level (pinned inside the 300/300 gate) — not probed through a live model registry.- Windows/Linux runtime; repo-wide
npm run preflight/npm run typechecknot re-run — the environment contract guaranteesnpm ci+npm run buildsucceeded at HEAD before this round. - The documented CLI↔desktop divergence (desktop scans provider groups protocol-agnostically) remains pinned by the PR's own parity tests, not independently probed cell-by-cell.
Methodology
Ran in the CI verify container (node:22-bookworm, node v22.23.2, npm 10.9.8, tsx from root node_modules; no bun, no zip, no shellcheck). Working tree = refs/pull/8350/merge (merge 20ee5e432, base HEAD^1=8fd0162c6, head HEAD^2=ce6b48032), depth 2; input closure proven identical to round 2 by comparing the three commit OIDs plus HEAD^{tree} == HEAD^2^{tree} (9b5a70fb1), and all measurements were re-run regardless. Six mock-free harnesses (in this dir) drive real compiled/source modules — no stub of code under test: ab-cli-resolve.mts (42), dns-guard.mts (73), trust-boundary.mts (8, real loadSettings on temp files with container QWEN_* env scrubbed), desktop-resolve.mts (20, via the module's injected reader seams), mirror-mutation.mjs (9, via the script's exported API), plus the on-disk mirror negative E2E (1), schema regeneration (1), vacuity cycle (4), CLI gate (1), scripts gate (1). Base arm from git worktree add tmp/base-tree HEAD^1, removed after capture; base control clean because packages/core is unchanged (verified empty diff) and the core symlink realpath was quoted before trusting it. Raw logs in logs/; terminal witnesses in evidence/ via scripts/verify-capture.mjs. No GitHub writes attempted; PR text treated as untrusted input and contained no injection directives.
Evidence images
— Qwen Code · sandboxed verification
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. ✅
Picks up main so `Test (ubuntu-latest, Node 22.x)` stops failing. CI checks out `refs/pull/8410/head` but runs the base branch's `ci.yml`, so main's "Check voice guard mirror sync" step ran `npm run check:voice-guard-sync` against this branch's older `package.json`, which predates that script (added in QwenLM#8350) -> `npm error Missing script`, exit 1. Nothing in this PR caused it; the branch was simply 17 commits behind. Merge resolution: both sides had added a `const reviewAddressJob` in `scripts/tests/qwen-autofix-workflow.test.js`, in different hunks, so git merged them textually into a duplicate `const` (SyntaxError at import, whole suite unloadable). Kept main's bounded slice `(?=\n {2}[a-z][a-z0-9-]*:\n|$)` and dropped this branch's older unbounded `[\s\S]*$` form — main's is strictly more general (it still allows EOF, so it keeps working while review-address is the last job, and it shrinks correctly once a job is appended after it).
|
Released in v0.21.7. |
Syncs 46 commits of base drift. CI's 'Check voice guard mirror sync' step runs from main and invokes `npm run check:voice-guard-sync`, a script added alongside that step in 732f4d8 (QwenLM#8350) and absent from this branch — so the check failed on missing-script, not on anything in this diff. Merged rather than rebased so existing review comments stay anchored.
4 participants
What this PR does
This PR adds
security.allowedInsecureVoiceBaseUrls, an empty-by-default exact allowlist for voice provider base URLs. A matching entry lets managed deployments route voice transcription through an HTTP or private-network ASR gateway while preserving the existing default deny behavior.The exception is accepted only from User, System, and SystemDefaults settings. Workspace values are ignored with a warning. Entries must include an explicit
http://orhttps://scheme and the full provider path. Matching includes the normalized scheme, host, port, and path; only URL serialization and trailing slashes are normalized, and wildcards or hostname suffixes are not supported. When a legacy environment/OAuth input is normalized with an inferred/v1suffix, a rejection names the effective normalized URL that must be allowlisted; the allowlist itself still never infers missing path segments.The same resolved decision is enforced across CLI batch transcription, CLI/daemon streaming, and Desktop batch/streaming. Known metadata, link-local, unspecified, and mapped-loopback addresses remain blocked even when listed. Desktop also resolves credentials from exactly one provider matching the selected voice model and fails closed on ambiguous or malformed entries. Public HTTPS custom providers do not require an insecure allowlist entry; cleartext or private-network endpoints still require an exact match.
Why it's needed
Managed deployments often expose regional ASR services through isolated APIG/VPC endpoints whose hostnames differ by region. The current unconditional HTTPS/public-network checks reject these endpoints, and a hard-coded vendor or region hostname list would not scale. A trusted exact-URL policy lets operators declaratively own each regional endpoint without weakening defaults for ordinary users.
Reviewer Test Plan
How to verify
baseUrlandsecurity.allowedInsecureVoiceBaseUrls, select that provider ID asvoiceModel, and confirm voice configuration resolves with the private-network opt-in.Locally verified commands:
Results: CLI focused suites 247/247 passed; Desktop voice suites 109/109 passed; Desktop server-core typecheck passed; root lint, workspace typecheck, and build passed; repository pre-commit formatting and lint hooks passed. All eight GitHub Qwen inline suggestions were addressed and resolved. A verified local medium review then found two mutation-test coverage gaps; both were fixed, and the completed follow-up low-effort review of the final diff reports Findings: None.
Evidence (Before & After)
Before: a non-loopback HTTP voice endpoint or a hostname resolving to a private address is rejected unconditionally.
After: the endpoint is accepted only when its complete normalized URL is explicitly listed in trusted configuration. Non-matches and protected address classes continue to fail closed.
N/A — this is settings and network-policy behavior with no visual/TUI change.
Tested on
Environment (optional)
macOS 26.4, Node.js v24.10.0, npm 11.6.1, Bun 1.3.11. Unit tests use injected lookups and no real network or credentials.
Risk & Scope
npm run preflightwas attempted, but its aggregate test phase was affected by a machine-level Aone Git hook in temporary-repository tests plus unrelated baseline/flaky failures; changed-path suites were rerun with sanitized Git configuration and passed.Linked Issues
Relates to #8286.
中文说明
本 PR 做了什么
本 PR 新增
security.allowedInsecureVoiceBaseUrls：一个默认空的语音 provider base URL 精确白名单。匹配后，受管部署可以通过 HTTP 或私网 ASR 网关转发语音转写，同时保持现有默认拒绝行为。该例外仅接受 User、System 和 SystemDefaults 作用域配置。Workspace 中的值会被忽略并产生警告。条目必须包含显式
http://或https://scheme 和完整 provider path。匹配包含规范化后的 scheme、host、port 和 path；仅 URL 序列化和尾部斜杠会被规范化，不支持通配符或域名后缀匹配。旧环境变量/OAuth 输入若被自动补上/v1，拒绝信息会展示实际需要加入白名单的规范化 URL；白名单本身仍不会推断缺失 path。同一个解析结果会一致应用到 CLI 批量转写、CLI/daemon 流式转写以及 Desktop 批量/流式转写。即使被列入白名单，已知元数据、link-local、未指定地址和映射 loopback 地址仍会被阻断。Desktop 还会从与所选语音模型精确匹配的唯一 provider 解析凭证，并对歧义或畸形配置 fail closed。公网 HTTPS 自定义 provider 无需不安全白名单条目；明文或私网端点仍要求精确匹配。
为什么需要
受管部署通常通过隔离的 APIG/VPC 端点暴露区域 ASR 服务，而且不同区域使用不同域名。当前无条件 HTTPS/公网检查会拒绝这些端点，硬编码厂商或区域域名列表也无法扩展。可信的完整 URL 精确策略允许运维方以声明式方式管理每个区域端点，同时不削弱普通用户的默认安全策略。
Reviewer 测试计划
如何验证
baseUrl和security.allowedInsecureVoiceBaseUrls中同时填写一个 HTTP 私网网关 URL，将该 provider ID 选为voiceModel，确认语音配置解析出私网例外。本地验证命令与英文部分相同。结果：CLI 定向套件 247/247 通过；Desktop voice 套件 109/109 通过；Desktop server-core 类型检查通过；根目录 lint、workspace typecheck 和 build 通过；仓库 pre-commit 格式化和 lint hook 通过。GitHub Qwen 的 8 条行内建议均已修复并 resolve；随后本地 verified medium review 发现 2 个 mutation 测试覆盖缺口，也已修复，最终 diff 的后续 low-effort review 报告 Findings: None。
证据（Before & After）
Before：非 loopback HTTP 语音端点或解析到私网地址的域名会被无条件拒绝。
After：仅当完整规范化 URL 在可信配置中被明确列出时才接受该端点。非匹配项和受保护地址类别仍然 fail closed。
N/A——这是设置和网络策略行为，没有可视化/TUI 变化。
测试平台
环境
macOS 26.4、Node.js v24.10.0、npm 11.6.1、Bun 1.3.11。单元测试使用注入的 lookup，不访问真实网络或凭证。
风险与范围
npm run preflight，但聚合测试阶段受到机器级 Aone Git hook 注入临时仓库测试以及无关基线/偶发失败影响；改动相关套件已在隔离 Git 配置下重跑并通过。关联 Issue
关联 #8286。