fix: forward noble BLE write options through desktop preload - #12928
fix: forward noble BLE write options through desktop preload#12928originalix wants to merge 3 commits into
Conversation
|
@codex review |
|
@codex security review |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Added the SDK bump to 1.2.0-alpha.147 on this branch — published from OneKeyHQ/hardware-js-sdk#888 (combined desktop BLE hardening: stale peripheral cache clear on disconnect, process-lifetime state listener, desktop-scoped 60s acquire deadline + immediate cancel, queue task leak fix). One branch now carries the preload fix + the SDK for desktop self-testing: switch to it, |
|
Coordination note: #12917 (about to merge) bumps SDK pins to 1.2.0-alpha.150, which does NOT contain the desktop BLE fixes in this PR's alpha.147 (npm next line is content-forked between hardware-js-sdk#888 and #886 — no published version has both). After #12917 merges: this PR will be rebased to keep only the two code files (preload.ts + desktop.ts, which #12917 doesn't touch — zero code conflicts), and the pin should move to alpha.151 once hardware-js-sdk#888 is rebased onto #886 and republished. Do NOT resolve pins toward 147 (#12917's cancel-ownership requires SDK ≥148); resolving to 150 silently drops the desktop fixes — 151 is the correct target. |
620df13 to
e99ef2b
Compare
|
Rebased onto hotfix/v6.5.2 post-#12917 and bumped SDK pins to 1.2.0-alpha.151 — the first published version containing BOTH hardware-js-sdk#888 (desktop BLE hardening, rebased onto #886 and republished) and #886. Verified from installed artifacts: transport stale-cache/state-listener fixes present, hd-core contains both raceBleAcquire (deadline) and beginConnectionAttempt (#886). agent:check commit profile passes. |
|
@cursoragent review |
PR #12928 代码审查报告审查的是 fix: forward noble BLE write options through desktop preload。preload 转发本身是对的，可以合；当前分支不能直接合，因为已经和 审查概要
评分
PR 评论分析
评论误报分析
发现的问题[🔴 高] [🔵 High] 与 hotfix 冲突，SDK pin 落后于已合入的 162文件: [🟢 低] [🟠 Medium] PR 描述过期文件: PR body [🟢 低] [⚪ Low] preload 内联了
|
| 优先级 | 置信度 | 文件 | 类型 | 描述 | Auto-fix |
|---|---|---|---|---|---|
| 🔴 高 | 🔵 High | package.json / yarn.lock | 合入 | rebase，pin ≥162，勿回退到 161 | — |
| 🟢 低 | 🟠 Medium | PR body | 规范 | 去掉过期的 alpha.142 | — |
| 🟢 低 | ⚪ Low | preload.ts:296 | 规范 | 复用 NobleBleWriteOptions |
— |
代码路径核对（通过）
V2 传输在 hd-transport-web-device 的 writeOnce() 里传 { pacingDelayMs: 0 }；V1 仍走两参数 write()，继续用默认 5ms。main 的 NOBLE_BLE_WRITE 从 150 起就收第三参，并 Number.isFinite + floor + clamp 到 [0, 1000]。preload 补上转发后，这条链才通。旧的两参数 handler 会忽略多余 IPC 参数，版本错位最多回到今天的慢写，不会炸。
未发现密钥/PII 泄漏、鉴权绕过、或 import hierarchy 破坏。options 多出来的字段 main 不会读；恶意渲染进程本来就能反复 write，节流参数不新增能力。DApp webview 共用 preload 是既有问题，这次没有加重。
测试建议
- Desktop BLE + Pro2 / V2：固件或资源传输，确认不再是每包 5ms。
- Desktop BLE + Classic / V1：签名、小包通信，确认默认 pacing 还在。
- rebase 之后回归 fix: detect hardware disconnect on desktop USB and BLE (OK-60486) #12942 的 USB/BLE 断连检测，以及
onDeviceDisconnected.reason。 - 不要把「IPC handler 存在」当成通过；要看实际传输耗时和设备是否仍稳定。
Affects: Desktop（preload）；rebase 前的 SDK bump 还会碰到所有平台。
Does NOT affect: 其它平台上的这条 write-options 路径。
本次是只读审查，没有改代码，也没有往 PR 上留 comment。需要的话我可以 rebase 到 hotfix（pin ≥162，只留两个代码文件）并更新 PR 描述。
eb9bce4 to
849d35c
Compare
849d35c to
3a94b42
Compare
2 participants
Problem
desktopApi.nobleBle.writein the preload only forwards(uuid, data)to theNOBLE_BLE_WRITEIPC, silently dropping the thirdoptionsargument. The 1.2.x renderer transport (hd-transport-web-deviceelectron-ble-transport.ts) passes{ pacingDelayMs: 0 }to suppress per-packet write pacing for V2 high-throughput transfers; because the preload drops it, the main handler falls back to the default 5 ms delay on every packet, slowing V2 firmware/resource transfers.Change
apps/desktop/app/preload.ts:writeaccepts and forwards the optional{ pacingDelayMs }argument.packages/shared/types/desktop.ts: widenINobleBleApi.writeaccordingly (inline type, no new imports).The main-process handler (
@onekeyfe/hd-transport-electron≥1.2.x, pinned at 1.2.0-alpha.142 on this branch) already acceptsoptions?: NobleBleWriteOptions; older 2-arg handlers ignore extra IPC args, so version skew is safe in both directions (worst case is falling back to today's behavior).Verification
yarn agent:check --profile commitpasses (lint-worktree-ts / lint-staged / tsc-staged).Related: OneKeyHQ/hardware-js-sdk#888, OneKeyHQ/hardware-js-sdk#889 (desktop BLE hardening from the same investigation).