fix(desktop): support enterprise LAN addresses - #8866
Merged
Conversation
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
|
Revalidated the network-selection path used by the Desktop 0.1.1 integration candidate. This Mac currently routes through physical |
Collaborator
Author
Desktop
|
yiliang114
marked this pull request as ready for review
August 10, 2026 20:11
Collaborator
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. This is the follow-up to #8806, whose fail-closed private-address filter produces a false negative on enterprise networks — the physical interface carries a non-RFC1918 address, every candidate is dropped, and Local Control reports Direction: aligned. Local Control just shipped via #8806, and this fixes its false-fail case without reopening the boundary it closed — the address-class filter is replaced by a netmask-enforced subnet gate at the proxy, which is the right shape. CHANGELOG: no direct reference, but the area is live (#8806 just landed). Size: not applicable — a single Rust file under Approach: the scope feels right. Returning the netmask alongside the selected address and letting the OS-routed physical interface win is the minimal change that solves this, and requiring a verifiable netmask for every candidate keeps the boundary honest (fail closed). Since the previous pass the diff also absorbed the review feedback: the missing security-boundary tests were added, the Risk: no elevated risk signals — no high-risk paths matched. One thing worth stating plainly for reviewers: this deliberately widens eligibility. A machine whose default route sits on a genuinely public interface now starts Local Control where #8806 refused; the subnet gate + pairing token are the compensating controls. Reviewed in the code review below. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，不是理论问题。这是 #8806 的后续修复——它的"非私有地址直接失败关闭"过滤在企业网络上产生了误伤：物理网卡分配的是非 RFC1918 地址，所有候选被剔除，Local Control 报 方向：对齐。Local Control 刚通过 #8806 上线，本 PR 修复的是它的误判场景，且没有重新打开 #8806 关闭的边界——地址类别过滤被替换为代理层基于 netmask 的同子网门禁，形态正确。CHANGELOG：无直接引用，但该领域刚有变更（#8806）。 规模：不适用——仅 方案：范围合理。把 netmask 随选中地址一起返回、让操作系统默认路由所在的物理网卡优先，是解决该问题的最小改动；要求每个候选都有可验证的 netmask（失败关闭）也保持了边界的严谨。相比上一轮，diff 还吸收了评审反馈：补齐了缺失的安全边界测试，封堵了 风险：无升级风险信号——未命中高风险路径。需要向 reviewer 明确说明：本 PR 有意放宽了准入——默认路由落在真正公网网卡上的机器，现在会启动 Local Control（#8806 之前是拒绝的）；补偿控制是子网门禁 + 配对 token，已在代码审查中核对。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI formed my independent proposal before reading the diff: let the OS-routed physical interface win regardless of address class, keep the VPN/virtual exclusion, and move the LAN boundary from address-class filtering to a netmask check at the proxy, failing closed when no netmask is available. The PR does exactly this, minimally — and this revision closes every substantive gap from the earlier rounds. What I verified against the code at this head:
Non-blocking items left open in the review threads, deliberately deferred by the author and fine to land without: distinguishing off-subnet 403s from auth 403s server-side (this module has no logging at all today — a diagnostics gap that predates this PR), naming the offending adapter in the unverified-netmask error, exercising the Also worth repeating for the merge record: this intentionally allows a genuinely-public routed interface to host Local Control (previously refused), gated by the same-subnet check plus the existing Host/Origin/pairing-token validation. That tradeoff is documented in the PR body and the compensating checks are in place. Testing evidenceUnattended CI run — I did not build or execute PR code. Evidence below is the PR's own CI on the reviewed commit, fetched via the API. The relevant gate here is the
Not verified here: the packaged-app behavior on a live enterprise network — the integration results posted in the thread are the author's own report from the affected machine, not independently re-run in this pass. Naming why there is no sandboxed-lane line this time: the claim 中文说明代码审查在读 diff 之前我先给出了自己的独立方案：让操作系统默认路由所在的物理网卡胜出（不看地址类别），保留 VPN/虚拟网卡排除，并把 LAN 边界从地址类别过滤移到代理层的 netmask 检查，拿不到 netmask 时失败关闭。PR 的实现与此完全一致、改动最小——并且本次修订关闭了前几轮提出的全部实质性问题。 在当前提交上对照代码核实过的点：
评审线程中仍开放的非阻塞项，作者有意推迟、可以随本 PR 直接合并：在服务端区分"跨子网 403"与"鉴权 403"（该模块目前完全没有日志——这是早于本 PR 的诊断缺口）、在"netmask 不可验证"错误里点名具体网卡、独立覆盖 为合并记录再强调一次：本 PR 有意允许真正的公网路由网卡承载 Local Control（此前是拒绝的），由同子网检查加既有的 Host/Origin/配对 token 校验共同把关。该取舍已写入 PR 正文，补偿性检查均已就位。 测试证据无人值守 CI 运行——未构建或执行 PR 代码。以上证据为被审提交上 PR 自身 CI 经 API 拉取的结果。此处相关的门禁是 未在此验证：真实企业网络上的安装包行为——讨论串中的集成结果是作者在受影响机器上的自述，本次未独立复跑。说明这次为何没有给出沙箱验证触发语： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal fix for a real regression; every substantive finding from the earlier rounds is now closed with tests, CI is green on the relevant gate, and what remains is deferred polish, not doubt. Stepping back: my independent proposal before reading the diff was exactly what this PR does — routed physical interface wins, VPN stays excluded, and the LAN boundary moves from an address-class filter to a netmask gate at the proxy. What happened since the last pass is the revision working as it should: the off-subnet 403 now has an end-to-end test (deleting the gate block fails CI), both selection error arms are asserted, each degenerate netmask state has a fixture, duplicate addresses keep the narrowest mask, and the rejection write happens after the socket returns to blocking mode. Nothing in the diff is unrelated to the goal. The honest reservations, all named above and none blocking: off-subnet 403s are indistinguishable from auth 403s until someone adds logging to this module, the unverified-netmask error doesn't name the adapter, one condition disjunct lacks independent coverage, and the packaged-app behavior on a live enterprise network rests on the author's same-machine report rather than CI. That last one carries real weight here regardless — the author is the maintainer who built and hardened this exact surface in #8806, and the thread documents the revalidation on the affected topology. The deliberate widening (public-routed interfaces now eligible, subnet-gated) is documented and compensated; if it ever misbehaves, the change is one file and trivially revertible. This solves a problem enterprise users actually hit, does it with the smallest reasonable diff, and leaves the boundary no looser than same-subnet plus the existing pairing auth. Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 针对真实回归的干净、最小修复；前几轮的全部实质性问题已用测试关闭，相关 CI 门禁通过，剩余的是有意推迟的打磨项，不构成疑虑。 退一步看：我在读 diff 之前的独立方案与本 PR 完全一致——路由命中的物理网卡优先、VPN 保持排除、LAN 边界从地址类别过滤迁移到代理层的 netmask 门禁。上一轮之后发生的变化正是修订应有的样子：跨子网 403 有了端到端测试（删除门禁代码块会让 CI 变红）、两条选择错误分支都有断言、每种退化 netmask 状态都有 fixture、重复地址保留最窄掩码、拒绝写入移到阻塞模式恢复之后。diff 中没有与目标无关的改动。 如实保留的意见，上文均已点名，均不阻塞：在有人给这个模块加日志之前，跨子网 403 与鉴权 403 无法区分；netmask 不可验证的错误没有点名具体网卡；有一个条件分支缺少独立覆盖；真实企业网络上的安装包行为依赖作者同机复验而非 CI。最后一点在此处分量不同——作者正是在 #8806 中构建并加固这一安全面的维护者，讨论串记录了在受影响拓扑上的复验过程。有意的放宽（公网路由网卡现在可被选中、受子网门禁约束）已写明且有补偿控制；若日后出现问题，改动只有一个文件，回滚极其简单。 这解决的是企业用户真实会遇到的问题，用了最小合理改动，边界最终不宽于"同子网 + 既有配对认证"。予以批准，锚定在被审提交。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Aug 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): PR #8866 changes Desktop Local Control network selection ...: cargo check / cargo test -p desktop-shell was not run (Tauri desktop-shell system dependencies likely unavailable on this runner); type-level correctness of t…; PR #8866 changes Desktop Local Control network selection ...: cargo test inside the real qwen-code-desktop crate (blocked by missing system libraries on this runner); the module's own tests were instead run verbatim in….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：PR #8866 changes Desktop Local Control network selection ...：cargo check / cargo test -p desktop-shell was not run (Tauri desktop-shell system dependencies likely unavailable on this runner); type-level correctness of t…；PR #8866 changes Desktop Local Control network selection ...：cargo test inside the real qwen-code-desktop crate (blocked by missing system libraries on this runner); the module's own tests were instead run verbatim in…。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Collaborator
Author
|
Changed: added focused security-boundary coverage for off-subnet rejection, ambiguous/no LAN selection, and missing or zero netmask fail-closed behavior. Product code was unchanged. Verified: all 6 focused Rust local-control tests passed; Intentionally not changed: none. Pending: exact-head CI and automatic review. 中文摘要补齐本地控制安全边界测试，覆盖跨子网、网卡歧义和无效掩码；产品代码未改，6 个聚焦 Rust 测试通过。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): PR #8866 (QwenLM/qwen-code) changes Desktop Local Control...: none — cargo test was not run locally (Tauri system deps), but the exact-head CI on both Desktop Shell platforms covers compile + test and is green.; PR #8866 (QwenLM/qwen-code) changes Desktop Local Control...: none — all checks above completed within budget..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：PR #8866 (QwenLM/qwen-code) changes Desktop Local Control...：none — cargo test was not run locally (Tauri system deps), but the exact-head CI on both Desktop Shell platforms covers compile + test and is green.；PR #8866 (QwenLM/qwen-code) changes Desktop Local Control...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
doudouOUC
previously approved these changes
Aug 11, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 0ea28aa. The selected network remains process-local and physical-interface scoped; public routed addresses are allowed only when they match the OS-selected physical candidate, and accepted peers are constrained by the selected netmask before a connection slot is allocated. The latest head also fails closed for missing, zero, and /32 masks, handles duplicate addresses by retaining the narrowest mask, distinguishes ambiguous/no-network states, and covers the off-subnet 403 path. Exact-head Desktop Shell tests are green on Ubuntu and Windows. No blocking issues found.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...: none — all checks I wanted to run completed within budget.; PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...: could not compile/run the Rust unit tests locally (no cargo toolchain on this runner); relied on green Desktop Shell CI jobs (ubuntu-22.04, windows-2022) plus c…; PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...: none — all checks I started were completed; I did not attempt a full cargo test run (Tauri system deps unavailable on this runner), but I verified the only co…; PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...: no Rust toolchain exists on this runner ( cargo / rustc not installed) and there is no pre-built target/ directory, so I could not compile the crate or run …; PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...: running cargo test -p desktop-shell local_control (or an extracted standalone harness) to empirically confirm the two surviving mutations..
中文说明
已审查。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...：none — all checks I wanted to run completed within budget.；PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...：could not compile/run the Rust unit tests locally (no cargo toolchain on this runner); relied on green Desktop Shell CI jobs (ubuntu-22.04, windows-2022) plus c…；PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...：none — all checks I started were completed; I did not attempt a full cargo test run (Tauri system deps unavailable on this runner), but I verified the only co…；PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...：no Rust toolchain exists on this runner ( cargo / rustc not installed) and there is no pre-built target/ directory, so I could not compile the crate or run …；PR #8866 (QwenLM/qwen-code) allows Desktop Local Control ...：running cargo test -p desktop-shell local_control (or an extracted standalone harness) to empirically confirm the two surviving mutations.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
chiga0
reviewed
Aug 11, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8866 fix(desktop): support enterprise LAN addresses
Type: Bug Fix
Change size: +187/-27 across 1 file
HEAD reviewed: 0ea28aa4602573f6a328a4a3e08dc683ec69f61f
Findings Summary
- Critical/Major: 0
- Minor: 1
- Nit: 2
Key Observations
This PR relaxes the previous RFC1918-only LAN filter so enterprise networks that assign public IPv4s to physical interfaces can use Local Control. The compensation is a same-subnet check at connection time, which is sound. The change is focused and well-tested. The main robustness gap is that the new off-subnet 403 is emitted before the accepted socket is switched to blocking mode, so the response may not be reliably delivered on Unix.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
| Missing test for off-subnet rejection | qwen-code-ci-bot (R1-1) | Addressed — rejects_off_subnet_peers added at HEAD lines 830-870 |
Missing error-path tests for choose_lan_ipv4 |
qwen-code-ci-bot (R1-2) | Addressed — empty and multi-network error cases tested at lines 681-689 |
| Netmask validation unexercised | qwen-code-ci-bot (R1-3) | Addressed — rejects_unverified_networks_when_interface_enumeration_fails covers None/UNSPECIFIED/BROADCAST at lines 701-735 |
/32 host netmask creates a useless session |
qwen-code-ci-bot (R2-1) | Not confirmed — Ipv4Addr::BROADCAST (255.255.255.255) is already rejected in the netmask filter at line 497 |
dedup keeps widest mask for duplicate addresses |
qwen-code-ci-bot (R2-2) | Not confirmed — the code sorts by (address, Reverse(netmask)) then `dedup_by_key( |
| Fail-closed when routed lacks netmask but another interface has one | qwen-code-ci-bot (R2-3) | Confirmed as implemented — matches the PR's documented tradeoff; acceptable fail-closed behavior |
| Off-subnet 403 indistinguishable from auth 403 / no logging | qwen-code-ci-bot (R2-4) | Confirmed concern — additionally note the 403 is written before set_nonblocking(false), so delivery is unreliable on Unix |
| Generic error masks netmask-related failures | qwen-code-ci-bot (R2-5) | Partially addressed — select_lan_ipv4 already returns a distinct "verifiable netmask" error when candidates were dropped; only the truly-no-candidates path uses the generic message |
| RFC1918/link-local fallback filter untested | qwen-code-ci-bot (R2-6) | Addressed — assert!(choose_lan_ipv4(None, vec![enterprise]).is_err()) at line 656 covers it |
Additional Audit Coverage
- Caller impact: all modified selection functions are private to
local_control.rs; the publicLocalControlSession::startsignature is unchanged. - Project convention scope: desktop-shell is not a core module per
AGENTS.md, so the maintainer-only two-tier gate does not apply. - Subnet check arithmetic:
(peer & mask) == (address & mask)correctly enforces same-subnet. - Mask rejection: verified
/32masks are rejected;/31and normal LAN masks pass. - Dedup behavior: verified sorting by
(address, Reverse(netmask))and dedup-by-address keeps the narrowest reported mask.
Final Verdict
COMMENT — code is directionally correct and the security invariant holds, but the off-subnet rejection should be made reliable before merge. The two Nit items are optional cleanups.
This review was generated by QoderWork AI
Collaborator
Author
|
Resolved the current feedback on |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
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: 32 passed · 0 failed · 32 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：32 通过 · 0 失败 · 32 总计 Verification reportPR 8866 Deep Verification —
|
| # | Scenario | Oracle | BASE (control) | HEAD |
|---|---|---|---|---|
| 1 | Routed 203.0.113.10 (non-RFC1918) + matching physical /24 |
choose_lan_ipv4 result |
Err "could not find a usable IPv4 network" | Ok 203.0.113.10/255.255.255.0 |
| 2 | Off-subnet TCP peer connects to the proxy | bytes observed on the wire | HTTP/1.1 408 after ~10 s (no boundary enforcement; falls into the header-timeout path) |
HTTP/1.1 403 Forbidden (off-network) immediately |
| 3 | Routed adapter reports no netmask | select_lan_ipv4 result |
Ok 192.168.1.20 (accepted — the pre-PR hole) |
Err "found an IPv4 adapter without a verifiable netmask" |
All three cells flip; control cells encode the pre-PR expectations and passed as predicted (expected-control-red counts as a passing assertion). Consistency cells that must NOT flip also held on head: unmatched public route still falls back to a single private candidate (H3); an unverified non-routed sibling does not poison a verified routed adapter (H7) nor mask the multiple-network ambiguity error (H8); contains() boundary census (self, subnet broadcast, network address, adjacent subnet, VPN subnet, IPv6, IPv4-mapped IPv6) behaves as specified (H6).
Witnesses: 01-ab-head-all-green.png (head arm, 14/14), 02-ab-base-controls.png (base arm, 8/8 incl. the observed 408 wire bytes).
Mutation matrix (head arm, full 14-test suite per mutant)
Every guard the PR introduces, deleted or inverted one at a time; survivors would be reported — there were none. Positive control M6 proves the harness can turn the suite red.
| Mutant (guard removed) | Result | Killed by |
|---|---|---|
| M1 no subnet-containment call site | killed | rejects_off_subnet_peers (PR test), probe_off_subnet_peer_gets_403 |
M2 accept degenerate masks (0.0.0.0//32) |
killed | rejects_unverified_networks… (PR test, UNSPECIFIED/BROADCAST cases) |
| M3 dedup keeps WIDEST mask | killed | selects_and_limits_the_physical_lan (PR test, "narrowest duplicate") |
| M4 private filter runs before route match | killed | PR test + both enterprise probes (H1, H2) |
| M5 no fail-closed early return | killed | rejects_unverified_networks… (mixed routed+verified case) |
| M6 positive control: pair-token check always passes | killed | enforces_the_pairing_boundary |
M7 contains() always true |
killed | H4, H6, rejects_off_subnet_peers, selects_and_limits… |
Witness: 03-mutation-matrix-all-killed.png. Per-mutant raw logs: logs/matrix-m*.txt.
Vacuity check: reverting M1 makes the PR's own new test rejects_off_subnet_peers fail with the behavioral mismatch it exists to catch — no 403 arrives inside the 2 s read window (read response: Os { code: 11, kind: WouldBlock }), not a compile error. Granularity note: M5 is killed only by the mixed-interface case of the PR test; the single-interface case still errors (with a different message) after the mutation, so a coarser probe would have misjudged coverage.
Reviewer Test Plan, per step
| Step | Result |
|---|---|
| 1. Mac on non-RFC1918 physical network, VPN running | Not reproducible in-container (needs the hardware/network). The selection half is exactly A/B cell 1; VPN/virtual exclusion rides the pre-existing internal/MAC filter, byte-identical to base in the diff (verified by inspection — not a live VPN run). |
| 2. Local Control becomes active instead of erroring | Verified at the selection level (cell 1 flips Err→Ok). Full LocalControlSession::start needs the real crate build, blocked environmentally (A/A). |
| 3. Phone on same subnet loads the session | Device-level part not covered. The in-subnet relay path through the new containment gate is covered by relays_a_delayed_http_response (green on head). |
| 4. Off-subnet client receives HTTP 403 | Covered end-to-end at socket level — cell 2 observes the exact HTTP/1.1 403 Forbidden (off-network) status line off a real TCP connection. |
Findings
None blocking. Two design observations (not defects, no change requested):
- Non-contiguous netmasks are not validated. Selection rejects only
0.0.0.0and255.255.255.255; a hypothetical non-contiguous mask would be applied bitwise bycontains(). Real OS interface enumerations produce contiguous masks, so this is theoretical. routed_unverifieddominates a verified fallback. If the default-route adapter lacks a netmask, Local Control errors even when another fully verified adapter exists (select_lan_ipv4returns Err rather than falling back). This is stricter than necessary but matches the PR's stated tradeoff ("interfaces without a netmask fail closed"), and M5 shows the strictness is pinned by tests.
Not covered
- Repo's
desktop_shellCI gate (cargo test --manifest-path src-tauri/Cargo.toml) could not run in this container: it needslibwebkit2gtk-4.1-dev/libdbus-1-devetc. installed viasudo apt(ci.yml "Install Linux dependencies"), and this container has no sudo (uid 1000). Proven environmental by A/A: both a head checkout and a scratch copy with the base file fail identically atlibdbus-sys's build script — inside the dependency graph, before any PR code compiles (the PR touches no Cargo.toml/lockfile). Witness:04-real-gate-environmental.png; logs07/08. Theinclude!harness substitutes at module level with the crate's exact locked dependency versions; residual risk (full-crate link) is bounded by the unchanged public API (above). - Reviewer Test Plan steps 1–3 at device level (real enterprise Wi-Fi, packaged app, phone on same subnet): require macOS hardware + the affected network; the PR itself marks packaged-app E2E pending. Socket-level equivalents are covered: step 4's exact
HTTP/1.1 403(cell 2), and the in-subnet relay path through the new gate (relays_a_delayed_http_response, green on head). - Per-commit attribution: shallow checkout (depth 2) makes only 1 of the snapshot's 6 commits reachable (
git rev-list HEAD^1..HEAD^2= 1 at the shallow boundary). Verified the aggregateHEAD^1..HEADdiff only. - Windows/Linux: author marks untested; the diff contains no OS-specific branches (platform
cfgs are in unchanged code). - fmt/clippy: not run — the repo's own CI for this crate explicitly does not gate on them ("fmt/clippy are not run here because the release pipeline does not gate on them either").
- Snapshot
baseRefOid(3f6551f9…) drifted behind the merge-ref base (7ebd137e…, the PR merged main mid-flight); verification used the merge-ref base per the CI contract.
Methodology
Container: node:22-bookworm (the verify lane's own runtime), no sudo, rustc 1.97.1 via user-level rustup. Harness (harness/): a scratch crate whose lib.rs does include! of the arm's unmodified local_control.rs plus an arm-specific probe module, sharing one cargo target dir across arms; deps pinned with = to the versions in src-tauri/Cargo.lock. Arms: run-arm.sh head|base|mutant:<file>. Probes drive the private functions directly (same-crate visibility via include!) and real TcpListener/TcpStream sockets — no mocks of any code under test. Base file extracted with git show HEAD^1:… (no worktree needed for a single-file diff). Raw per-arm output in logs/01–08; mutation logs logs/matrix-*; assertions.json recomputed from those logs by script (14 head tests + 8 base control tests + 1 wire-observed 408 control + 7 mutant-kill checks + 2 A/A gate checks = 32). Evidence images rendered by scripts/verify-capture.mjs from live re-runs of each arm.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
enabled auto-merge
August 11, 2026 09:35
qwen-code-ci-bot
approved these changes
Aug 11, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-review-bot
approved these changes
Aug 11, 2026
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. The current head (583ad52) correctly implements the advertised tradeoff:
- The
LocalNetworkstruct cleanly binds each IPv4 candidate to its verified netmask. - The same-subnet gate in the accept loop fires after
set_nonblocking(false), so the 403 is reliably delivered. - Enterprise addresses that match the OS-selected physical candidate are allowed; VPN/virtual interfaces remain excluded.
- Netmask validation rejects UNSPECIFIED, BROADCAST, and missing masks (fail-closed as documented).
- Dedup by address retains the narrowest mask.
- The test suite covers the enterprise path, off-subnet rejection, netmask validation edge cases, and duplicate-address resolution.
- Desktop Shell CI is green on the exact head.
Prior review rounds have been addressed in follow-up commits. No blocking issues found.
doudouOUC
requested changes
Aug 11, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed exact head 583ad52506d7a056bf17a400647981b345e54720. The prior review findings are fixed, and exact-head Desktop Shell CI is green. One newly identified routed-adapter boundary still needs to be addressed; details are inline.
| .find(|network| network.address == routed) | ||
| .copied() | ||
| }) { | ||
| return Ok(network); |
Collaborator
There was a problem hiding this comment.
[Critical] Do not let a routed virtual adapter bypass the physical-LAN boundary. At this return, physical has not actually been verified as physical: interface identity/type was discarded, and the preceding filter only checks !internal, a non-zero MAC, and a broadcast address. In the pinned network-interface 2.0.5 Windows backend, internal is true only for IF_TYPE_SOFTWARE_LOOPBACK, so Ethernet-style VPN/TAP/Hyper-V adapters can satisfy the filter.
Because this PR now returns a routed non-RFC1918 candidate before the private/link-local fallback filter, it newly accepts those virtual adapters. For example, with a default route through a virtual VPN at 100.64.0.10/10 and physical Wi-Fi at 192.168.1.20/24, the base code drops the VPN candidate and selects Wi-Fi, while this head selects the VPN. The generated URL is unreachable from the phone, and peers in the VPN prefix pass the subnet gate and can consume Local Control connection slots. This contradicts the PR's stated guarantee that VPN and virtual interfaces remain excluded.
Please retain interface identity/type through selection and allow the non-private routed exception only for a confirmed physical adapter, with a regression case covering a routed non-RFC1918 virtual adapter alongside a physical LAN.
Collaborator
|
Released in v0.21.10. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Allow Desktop Local Control to use the verified physical interface selected by the operating system as the default IPv4 route even when an enterprise network assigns a non-RFC1918 address. Incoming connections remain limited to the selected interface's subnet.
Why it's needed
Desktop currently removes every non-private IPv4 candidate before checking the default route. On managed enterprise Wi-Fi this can leave no candidate and show
Local Control could not find a usable IPv4 network.even though the active physical interface is valid. VPN and virtual interfaces remain excluded.Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: Local Control reports
Local Control could not find a usable IPv4 network.on the affected enterprise network.After: final packaged-app evidence is pending and will be posted separately before this PR is marked ready.
Tested on
Environment (optional)
macOS with an active physical enterprise interface and AliLang VPN interfaces present.
Risk & Scope
Linked Issues
Follow-up to #8806.
中文说明
这个 PR 做了什么
允许 Desktop Local Control 使用操作系统选中的、经过实体网卡校验的默认 IPv4 路由，即使企业网络分配的是非 RFC1918 地址。接入连接仍被限制在所选网卡的同一子网内。
为什么需要
Desktop 当前会在检查默认路由之前删除全部非私有 IPv4 候选。在受管企业 Wi-Fi 中，即使当前实体网卡有效，也可能因此没有候选地址并显示
Local Control could not find a usable IPv4 network.。VPN 和虚拟网卡仍然会被排除。Reviewer 测试计划
如何验证
证据（Before & After）
Before：在受影响的企业网络上，Local Control 报告
Local Control could not find a usable IPv4 network.。After：最终安装包证据仍待验证，在此 PR 标记 ready 前会单独提交。
测试平台
环境（可选）
macOS，活动企业实体网卡，并存在 AliLang VPN 网卡。
风险与范围
关联 Issue
#8806 的后续修复。