Merge develop into v4

[RetricSu]

No description provided.

[humble-little-bear]

[REVIEW-ORCHESTRATOR]

Review Report for #452 (Merge develop into v4)

Summary

• Review dimensions: Correctness, Maintainability, Performance, Security
• Specialists: 4 participated, all completed
• Challenger: local Review-Challenger agent completed; official Challenger issue RET-221 is in progress
• Findings after Challenger: CRITICAL: 4 | WARNING: 15 | SUGGESTION: 16
• Verdict: REQUEST CHANGES — 4 CRITICAL issues must be fixed before merge.

CRITICAL (must fix before merge)

1. [CORRECTNESS] 全局 --json 选项仅注册在顶层 program，放在子命令后会被 Commander 拒绝，破坏 JSON 输出稳定性

• Location: src/cli.ts:34
• Evidence: src/cli.ts:34 program.option('--json', 'Output logs in JSON format for agent/programmatic consumption');
• Fix suggestion: 把 --json 加到所有需要它的命令（node/transfer/balance/udt/status 等），或在 hook 中使用 actionCommand.optsWithGlobals() 并确保子命令继承该选项
• Challenger: KEEP — verified with code evidence.

2. [SECURITY] Prototype pollution via user-controlled settings.json in deepMerge

• Location: src/cfg/setting.ts:160
• Evidence: src/cfg/setting.ts:269-281 uses for (const key in source) and recursive target[key] = source[key]; a config value like {"__proto__":{"polluted":true}} causes deepMerge(Object.prototype, {polluted:true}) and pollutes every object
• Fix suggestion: Replace the loop with Object.keys(source).forEach((key) => { if (key === '__proto__' || key === 'constructor' || key === 'prototype') return; ... }) or use Object.hasOwn and structuredClone.
• Challenger: KEEP — verified with code evidence.

3. [SECURITY] ZipSlip / path traversal in ckb-tui archive extraction

• Location: src/tools/ckb-tui.ts:292
• Evidence: src/tools/ckb-tui.ts:1816-1823 calls zip.extractAllTo(extractDir, true) without sanitizing entry paths; a malicious release .zip can contain ../../../ entries and write outside the temp directory
• Fix suggestion: Iterate entries manually, resolve each with path.resolve(extractDir, entry.entryName), and verify resolved.startsWith(extractDir + path.sep) before extracting.
• Challenger: KEEP — verified with code evidence.

4. [SECURITY] Weak daemon identity verification enables arbitrary process termination via tampered PID file

• Location: src/cmd/node.ts:241
• Evidence: src/cmd/node.ts:704-720 verifyDaemonIdentity only checks command-line substrings (node, script name, offckb); a tampered daemon.pid can point to any process whose cmdline contains those strings, and stopNode will send SIGTERM/SIGKILL to it
• Fix suggestion: Store a cryptographically random nonce in the PID file and require the daemon child to expose it (e.g. via a dedicated env var or argv) before signaling; reject if the nonce does not match.
• Challenger: KEEP — verified with code evidence.

WARNING (should fix)

1. [CORRECTNESS] isProcessAlive 把 EPERM 误判为进程已死亡，导致跨用户场景下重复启动或无法停止守护进程

• Location: src/cmd/node.ts:183
• Evidence: src/cmd/node.ts:183 try { process.kill(pid, 0); return true; } catch { return false; }
• Fix suggestion: 区分 EPERM/ESRCH：catch (error) { const err = error as NodeJS.ErrnoException; return err.code === 'EPERM'; }
• Challenger: DOWNGRADE from CRITICAL to WARNING.

2. [CORRECTNESS] stopNode 中先 isProcessAlive 再 verifyDaemonIdentity 存在检查时序窗口，PID 被复用后可能终止错误进程

• Location: src/cmd/node.ts:241
• Evidence: src/cmd/node.ts:394-400 if (!isProcessAlive(pid)) { ... return; } const identityOk = await verifyDaemonIdentity(pid, metadata);
• Fix suggestion: 在 verifyDaemonIdentity 返回 true 后、发送信号前再次调用 isProcessAlive(pid) 确认，必要时把进程启动时间写入 PID 文件做辅助校验
• Challenger: DOWNGRADE from WARNING to WARNING.

3. [CORRECTNESS] startDaemon 的重复启动检查非原子，并发执行可能产生多个守护进程

• Location: src/cmd/node.ts:294
• Evidence: src/cmd/node.ts:294-302 const existing = readPidFile(pidFile); if (existing && isProcessAlive(existing.pid)) { ... }
• Fix suggestion: 在检查并写入 PID 文件期间使用文件锁（如 proper-lockfile 或对 log 目录加锁），或在 spawn 后重新读取 PID 文件确认未被覆盖
• Challenger: KEEP.

4. [CORRECTNESS] 打开日志文件异常时，已打开的文件描述符未关闭，造成 FD 泄漏

• Location: src/cmd/node.ts:306
• Evidence: src/cmd/node.ts:306-313 out = fs.openSync(logFile, 'a'); err = fs.openSync(logFile, 'a'); } catch (error) { logger.error(...); return; }
• Fix suggestion: catch 块内先调用 closeFileDescriptors(out, err); 再返回：} catch (error) { closeFileDescriptors(out, err); logger.error(...); return; }
• Challenger: KEEP.

5. [CORRECTNESS] udtDestroy 错误地拒绝销毁全部余额，但 UDT script 允许输出余额为 0

• Location: src/sdk/ckb.ts:477
• Evidence: src/sdk/ckb.ts:477-481 if (destroyAmount === totalBalance) { throw new Error('Destroying the entire UDT balance may be rejected by the UDT script.'); }
• Fix suggestion: 删除该 throw，或改为仅 warn 提示；让交易按正常 remaining = 0 构造
• Challenger: KEEP.

6. [CORRECTNESS] udtBalance 在检查 maxCells 前先 scanned++，导致实际扫描 maxCells + 1 个 cell

• Location: src/sdk/ckb.ts:345
• Evidence: src/sdk/ckb.ts:345-350 scanned++; if (scanned > maxCells) { ... break; }
• Fix suggestion: 调整顺序：if (scanned >= maxCells) { ... break; } scanned++; 或 if (scanned > maxCells) 改为 if (scanned >= maxCells) 并把自增放末尾
• Challenger: KEEP.

7. [CORRECTNESS] readSettings 在配置文件不存在或读取失败时直接返回共享的 defaultSettings，调用方修改会污染全局默认值

• Location: src/cfg/setting.ts:114
• Evidence: src/cfg/setting.ts:114 return defaultSettings; 与 src/cfg/setting.ts:118 return defaultSettings;
• Fix suggestion: 两个分支都返回深拷贝：return deepClone(defaultSettings);
• Challenger: KEEP.

8. [CORRECTNESS] validateSettings 不拒绝非对象的 tools，导致后续读取 tools.rootFolder 时 TypeError

• Location: src/cfg/setting.ts:181
• Evidence: src/cfg/setting.ts:181-185 if (obj.tools && typeof obj.tools === 'object') { ... }
• Fix suggestion: 先强制校验：if (obj.tools !== undefined && (!obj.tools || typeof obj.tools !== 'object' || Array.isArray(obj.tools))) throw new Error('tools must be an object');
• Challenger: KEEP.

9. [CORRECTNESS] validateSettings 不拒绝非对象的 tools.ckbTui，导致 settings.tools.ckbTui.version 访问异常

• Location: src/cfg/setting.ts:186
• Evidence: src/cfg/setting.ts:186-191 if (tools.ckbTui && typeof tools.ckbTui === 'object') { ... }
• Fix suggestion: 增加类型校验：if (tools.ckbTui !== undefined && (!tools.ckbTui || typeof tools.ckbTui !== 'object' || Array.isArray(tools.ckbTui))) throw new Error('tools.ckbTui must be an object');
• Challenger: KEEP.

10. [CORRECTNESS] status 在 RPC 端口未监听时只打印错误，未设置非零退出码，脚本无法感知失败

• Location: src/cmd/status.ts:35
• Evidence: src/cmd/status.ts:35-39 if (!isListening) { logger.error(...); return; }
• Fix suggestion: 设置退出码：if (!isListening) { logger.error(...); process.exitCode = 1; return; }
• Challenger: KEEP.

11. [MAINTAINABILITY] src/sdk/ckb.ts 的 CKB 类承担了 UDT 发行/转账/销毁/余额扫描等大量新职责，违反单一职责原则，类体膨胀至 600+ 行。

• Location: src/sdk/ckb.ts:260
• Evidence: 证据：src/sdk/ckb.ts:260-508 新增 detectUdtBalances、udtBalance、udtTransfer、udtIssue、udtDestroy 及 getUdtScriptInfo、buildUdtTypeScript 等辅助方法。
• Fix suggestion: 修复建议：新增 src/sdk/udt.ts 的 UdtService 类，将 UDT 方法迁移，CKB 类通过 this.udt = new UdtService(client, feeRate) 委托，保持每个类的职责单一。
• Challenger: KEEP.

12. [MAINTAINABILITY] src/tools/ckb-tui.ts 的 CKBTui 类混合二进制下载、校验、解压、安装、运行多种职责，且使用同步 I/O 阻塞事件循环。

• Location: src/tools/ckb-tui.ts:17
• Evidence: 证据：src/tools/ckb-tui.ts:17-302；installSync 在 line 120；ensureInstalled 在 line 38 直接调用同步安装；run 在 line 59 调用 ensureInstalled。
• Fix suggestion: 修复建议：拆分为 CkbTuiInstaller（负责异步下载/校验/解压）和 CkbTuiRunner（负责 spawnSync 运行），CKBTui 只暴露路径查询与运行入口。
• Challenger: KEEP.

13. [MAINTAINABILITY] src/cmd/transfer.ts 的 transfer 函数同时处理 CKB 转账和 UDT 转账，依赖 udtTypeArgs 是否存在切换分支，导致函数职责不单一。

• Location: src/cmd/transfer.ts:23
• Evidence: 证据：src/cmd/transfer.ts:23-38 为 UDT 分支，line 40-45 为 CKB 分支。
• Fix suggestion: 修复建议：拆分为 transferCkb(toAddress, amount, opt) 与 transferUdt(toAddress, amount, opt)，原 transfer 仅做路由判断。
• Challenger: KEEP.

14. [SECURITY] Best-effort checksum verification is skipped when unavailable

• Location: src/tools/ckb-tui.ts:219
• Evidence: src/tools/ckb-tui.ts:1735-1780 returns with only a logger.warn when the checksum file cannot be downloaded, then installs and executes the binary
• Fix suggestion: Fail closed: abort installation and throw an error if the checksum file is missing or the archive cannot be verified.
• Challenger: KEEP.

15. [SECURITY] Daemon PID and log files are created with default permissions

• Location: src/cmd/node.ts:157
• Evidence: src/cmd/node.ts:619-621 and src/cmd/node.ts:767-772 use fs.writeFileSync/fs.openSync without explicit mode, so files inherit the umask and may be readable or tamperable in shared environments
• Fix suggestion: Create them with restrictive mode: fs.writeFileSync(pidFile, ..., { mode: 0o600 }) and fs.openSync(logFile, 'a', 0o600).
• Challenger: KEEP.

SUGGESTION (optional improvements)

1. [MAINTAINABILITY] src/cmd/node.ts 的 daemon 生命周期与 node 启动逻辑高度耦合，单个文件同时负责 devnet 启动、进程守护、PID 管理、跨平台信号终止等多重职责，内聚性差。

• Location: src/cmd/node.ts:30
• Evidence: 证据：src/cmd/node.ts:30-441 同时包含 startNode、nodeDevnet、startDaemon、stopNode、resolveDaemonPaths、readPidFile、terminateProcess 等。
• Fix suggestion: 修复建议：提取 daemon 相关逻辑到 src/daemon/controller.ts，node.ts 仅保留命令入口，例如 const daemon = new DaemonController(settings.devnet.dataPath); daemon.start(); daemon.stop();。
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

2. [MAINTAINABILITY] src/tools/ckb-tui.ts 与 src/node/install.ts 存在重复的二进制安装模式但实现不一致（同步 vs 异步、curl vs Request、shell tar vs tar 库），后续维护容易出现偏差。

• Location: src/tools/ckb-tui.ts:143
• Evidence: 证据：src/tools/ckb-tui.ts:143 使用 spawnSync('curl', ...)，line 279 使用 spawnSync('tar', ...)；而 src/node/install.ts:74 使用 Request.send(...)，line 93-113 使用 tar.x(...)。
• Fix suggestion: 修复建议：在 src/util/installer.ts 抽象统一的 BinaryInstaller（包含 download/extract/checksum），ckb-tui.ts 与 install.ts 复用同一实现。
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

3. [MAINTAINABILITY] src/cmd/udt.ts 与 src/sdk/ckb.ts 存在同名接口 UdtIssueOption / UdtDestroyOption，容易造成导入混淆和维护时误用。

• Location: src/cmd/udt.ts:6
• Evidence: 证据：src/cmd/udt.ts:6-17 定义 UdtIssueOption/UdtDestroyOption；src/sdk/ckb.ts:55-68 也定义同名接口。
• Fix suggestion: 修复建议：将命令层接口重命名为 UdtIssueCliOptions / UdtDestroyCliOptions，SDK 层保留领域模型名称，避免命名冲突。
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

4. [MAINTAINABILITY] src/cmd/status.ts 通过 NETWORK_SETTINGS_KEY 记录将 Network 枚举映射到 settings 键，但键名与枚举值完全一致，映射属于过度设计。

• Location: src/cmd/status.ts:11
• Evidence: 证据：src/cmd/status.ts:11-17 定义 NetworkSettingsKey 与 NETWORK_SETTINGS_KEY。
• Fix suggestion: 修复建议：删除映射，直接使用 settings[network].rpcProxyPort，因为 Network 枚举值与 settings 的键相同。

5. [MAINTAINABILITY] src/cmd/node.ts 的 verifyDaemonIdentity 使用命令行字符串启发式匹配判断进程身份，规则脆弱且难以维护。

• Location: src/cmd/node.ts:241
• Evidence: 证据：src/cmd/node.ts:241-257 的 looksLikeNode、looksLikeOurScript、looksLikeOffckb 字符串包含判断。
• Fix suggestion: 修复建议：在 daemon 子进程启动时把会话 ID / 进程组 ID 写入 PID 文件，停止时直接比较进程组或会话 ID，避免对命令行字符串做启发式匹配。

6. [MAINTAINABILITY] src/cfg/setting.ts 手动实现 deepClone 并继续保留基于 any 的 deepMerge，与 Node >=20 环境不匹配，增加维护成本。

• Location: src/cfg/setting.ts:160
• Evidence: 证据：src/cfg/setting.ts:143-157 实现 deepClone；line 159-172 实现 deepMerge 并带有 // eslint-disable-next-line @typescript-eslint/no-explicit-any。
• Fix suggestion: 修复建议：使用原生 structuredClone(defaultSettings) 替换 deepClone，并将 deepMerge 改写为类型安全、不可变返回的合并函数（如基于 structuredClone + 递归合并）。

7. [MAINTAINABILITY] src/cfg/setting.ts 的 validateSettings 只校验了新增字段中的部分路径/版本，未覆盖 bins.defaultCKBVersion、devnet.rpcUrl 等，settings 校验与结构不同步。

• Location: src/cfg/setting.ts:174
• Evidence: 证据：src/cfg/setting.ts:174-199 仅校验 tools.rootFolder、tools.ckbTui.version 与 proxy。
• Fix suggestion: 修复建议：引入集中式 schema（如 zod/yup）或类型守卫，确保 Settings 接口新增字段时同步更新校验规则。

8. [MAINTAINABILITY] src/sdk/ckb.ts 中 UDT 相关硬编码常量（20 字节/32 字节 args、16 字节 data）散落在业务逻辑中，命名不足。

• Location: src/sdk/ckb.ts:416
• Evidence: 证据：src/sdk/ckb.ts:416 resolvedTypeArgs = ('0x' + issuerLockHash.slice(2, 42)) as HexString；line 364、line 393、line 428、line 496 等多次出现 ccc.numToBytes(amountBigInt, 16)。
• Fix suggestion: 修复建议：定义常量 SUDT_TYPE_ARGS_BYTE_LEN = 20、XUDT_TYPE_ARGS_BYTE_LEN = 32、UDT_DATA_BYTE_LEN = 16，并在 udtIssue/udtTransfer/udtDestroy 中复用。

9. [MAINTAINABILITY] src/util/link.ts 的 logTxSuccess 既负责生成 explorer 链接又负责打印日志，职责混合；且 action 参数通过字符串拼接生成句子，不利于复用。

• Location: src/util/link.ts:8
• Evidence: 证据：src/util/link.ts:8-14 同时调用 buildTestnetTxLink 与 logger.info(...)。
• Fix suggestion: 修复建议：新增 buildTxLink(network, txHash) 仅负责链接生成，命令层自行调用 logger.info(...) 输出成功信息。

10. [MAINTAINABILITY] .changeset/brave-falcons-dance.md 将新增用户可见的 --json 全局选项、node --daemon 与 node stop 命令标记为 patch，与 changeset 语义惯例（新增功能应为 minor）不一致。

• Location: .md:1
• Evidence: 证据：.changeset/brave-falcons-dance.md:1-2 为 patch bump，内容包含新增命令与全局选项。
• Fix suggestion: 修复建议：将该 changeset 的 bump 类型改为 minor，以准确反映功能新增。

11. [MAINTAINABILITY] tests/node-command.test.ts 通过 Object.defineProperty(process, 'platform', { value }) 修改全局 process.platform，可能影响并行测试稳定性且代码重复。

• Location: tests/node-command.test.ts:237
• Evidence: 证据：tests/node-command.test.ts:237-239 定义 setPlatform；line 253 在 beforeEach 中调用。
• Fix suggestion: 修复建议：将平台相关逻辑抽到 src/util/platform.ts 并在测试中 mock 该模块，或使用 jest.spyOn(process, 'platform', 'get') 进行局部替换。

12. [PERFORMANCE] ckb-tui 安装时 verifyChecksum 将整个二进制归档读入内存计算 SHA-256

• Location: src/tools/ckb-tui.ts:236
• Evidence: src/tools/ckb-tui.ts:1762 ；ckb-tui-with-node 归档通常数十至上百 MB，会产生同等大小内存尖峰
• Fix suggestion: 改为流式：
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

13. [PERFORMANCE] ckb-tui ZIP 解压将整个归档加载到内存

• Location: src/tools/ckb-tui.ts:292
• Evidence: src/tools/ckb-tui.ts:1818 ；AdmZip 会完整读取 zip，大归档时内存占用等于归档大小
• Fix suggestion: Windows zip 改用命令行 UnZip 6.00 of 20 April 2009, by Debian. Original by Info-ZIP.
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

14. [PERFORMANCE] daemon 模式使用单一 daemon.log 且无轮转/大小限制

• Location: src/cmd/node.ts:307
• Evidence: src/cmd/node.ts:771-772 仅追加不截断；CKB 节点持续输出日志会导致文件无限增长，长期运行占用磁盘
• Fix suggestion: 增加按大小/时间轮转（如单文件 10MB、保留 5 个历史文件），或在文档中提示用户手动清理
• Challenger: DOWNGRADE from WARNING to SUGGESTION.

15. [PERFORMANCE] detectUdtBalances 串行扫描 SUDT 和 xUDT

• Location: src/sdk/ckb.ts:324
• Evidence: src/sdk/ckb.ts:1332-1337 先 再 ；两次扫描相互独立，串行使延迟加倍
• Fix suggestion: 使用 并行， balances Map 可用独立 Map 后合并以避免并发写冲突

16. [PERFORMANCE] udtBalance / detectUdtBalances 的 maxCells 是软限制，迭代器可能多取一整批

• Location: src/sdk/ckb.ts:345
• Evidence: src/sdk/ckb.ts:1353 / 1299 在循环内 ，但异步生成器已向 RPC 请求当前批次；实际拉取数量可能超出 maxCells 一个批次
• Fix suggestion: 若 CCC client 的 findCells/findCellsByLock 支持 limit 参数则优先使用；否则在文档中说明 maxCells 为 soft cap

False-positive / dropped findings (from Challenger)

• --daemon exact-string filter nesting: DROP — Commander rejects --daemon=value for boolean flags.
• logger.serializeParam Error as {}: DROP — pre-existing behavior, not introduced by this PR.
• --no-udt double-negative mapping: DROP — idiomatic Commander negative option.
• OFFCKB_CLI_PATH arbitrary entry point: DROP — documented intentional escape hatch.
• child_process in package.json: DROP — pre-existing declaration.
• Unpatched elliptic dependency: DROP — known upstream gap already documented in changeset.
• udtDestroy unbounded findCellsByLock: DROP — maxInputCells is enforced inside the loop.

Fix checklist

• Fix CRITICAL #1
• Fix CRITICAL #2
• Fix CRITICAL #3
• Fix CRITICAL #4
• Fix WARNING #1
• Fix WARNING #2
• Fix WARNING #3
• Fix WARNING #4
• Fix WARNING #5
• Fix WARNING #6
• Fix WARNING #7
• Fix WARNING #8
• Fix WARNING #9
• Fix WARNING #10
• Fix WARNING #11
• Fix WARNING #12
• Fix WARNING #13
• Fix WARNING #14
• Fix WARNING #15

(See child issues: RET-214, RET-215, RET-217, RET-216, RET-221.)