Repository navigation
TB-127: 改用 ZIP 扩展导入并整合签名与连接安全修复 - #131
hehetoshang wants to merge 8 commits into
Conversation
| } | ||
| }; | ||
|
|
||
| let previous = store.grants.get(&manifest.name); |
There was a problem hiding this comment.
BUG / 设计缺口(中高):grant 是「只升不降、永不清理」的高水位线,卸载后重装更旧版本会被永久硬阻断,且 UI 没有任何恢复通道
store.grants.insert(第 255 行)是整个 src-tauri/src/ 中唯一修改 grants 的语句 —— 我 grep 过 grants.remove|retain|clear,全部为零。而本行 store.grants.get(&manifest.name) 只按扩展名查表,随后第 395–404 行对「降级」和「同版本摘要变化」直接写入 blocked_reason,authorize_activation 第 236–238 行随即返回 Err。
复现路径全部是正常用户操作:
- 安装
foo2.0.0 并确认 →grants = { foo: {version: 2.0.0, digest: D} } - 卸载
foo——ext_uninstall_extension(mod.rs:498-536)只做forget_desired+ 杀进程 +remove_dir_all,完全不动trust.json - 重新导入
foo1.0.0(发布者回退版本,或用户恢复自己的离线备份)→Ordering::Less→blocked_reason = "检测到版本降级/重放…"→ 导入被彻底阻断 - 或重新导入发布者重签过的
foo2.0.0(未提升版本号就重新签名)→Ordering::Equal且摘要不同 → 同样阻断
此时前端没有任何「清除信任记录 / 重置该扩展授权状态」的入口:我在 src/app/extensions/page.tsx、src/app/extensions/detail/page.tsx、src/lib/store/extensions.ts 中 grep 清除|重置|reset|revoke|forget 均为空。唯一解法是让用户手工去 AppData 删除 trust.json。而且第 403 行的报错文案「请由发布者提升版本后重新签名」在这里是误导 —— 发布者已经发布过更高版本了,用户只是想重装。
需要强调的是:不要在卸载时直接清空 grant。保留它是有正当安全价值的,否则「卸载 → 重装旧版」就成了绕过回滚保护的现成手法;docs/extension-trust-model.md:57 恰恰把「立即禁用或卸载受影响扩展」写成发布者被攻陷后的处置建议,这会把用户正好推进这个死胡同。
建议的修法:补一条显式、二次确认、可审计的恢复路径(例如「清除该扩展的本地信任记录」操作 + 在 UI/文档中写明它的安全含义),并区分「版本降级」与「同版本重新签名」两种文案。若希望更保守,可以把历史 grant 归档而不是删除,使其仍参与回滚判断。
说明:本次审查无法执行测试(本环境没有 node_modules,node --test 被权限拒绝),以上结论均来自静态阅读。
| fn trusted_key( | ||
| _store: &TrustStore, | ||
| manifest: &Manifest, |
There was a problem hiding this comment.
代码质量(中):trusted_key 完全忽略 store,使 TrustStore.publishers 成为死代码
fn trusted_key(_store: &TrustStore, manifest: &Manifest, signature: &VerifiedSignature) -> Result<bool, String>参数被命名为 _store,函数体内一次都没有使用。结合第 80 行 publishers: HashMap<String, TrustedPublisher> 与第 86 行 TrustedPublisher,可以判定:trust.json 里的 publishers 字段会被 serde 反序列化、会被原样写回,但永远不被任何逻辑读取(我在整个 crate 内 grep,除定义与序列化外无其它引用)。
在当前配置下不会产生错误结果 —— BUILTIN_TRUSTED_KEYS(第 34 行)为空,trusted 恒为 false,「未知发布者每次启用都需重新确认」的行为是正确的。但它留下两个隐患:
- 后续维护者看到
publishers字段会以为它生效,从而写出错误的信任推理; - 一旦有人往
BUILTIN_TRUSTED_KEYS加入锚点,trusted_key就成了唯一信任来源,而publishers仍然静默失效 —— 一个「看起来可配置、实际不可配置」的安全开关,正是最容易在审计中被误判的形态。
建议二选一:删除 publishers / TrustedPublisher / _store 参数,让类型签名本身表达「信任锚只能是编译进二进制的常量」(例如 fn trusted_key(manifest, signature));或者真正接上 store 并明确其优先级。docs/extension-trust-model.md:47 已说明「可写的 AppData 记录不会把未知密钥提升为官方信任」,代码应与该文档保持一致,而不是保留一个误导性字段。
| /// Re-evaluate immediately before activation and bind a matching explicit | ||
| /// approval to the current package. This rejects stale UI confirmations, but | ||
| /// does not protect against arbitrary same-user filesystem/process tampering. | ||
| pub fn authorize_activation( |
There was a problem hiding this comment.
性能(中低):激活路径每次都做全包 SHA-256,且一次升级会重复计算 4 次
authorize_activation → evaluate → package_digest(ext_dir)(第 286 行)会对整个已安装目录重新做一次完整 SHA-256。调用点是:
mod.rs的activate():每次「启用」扩展一次lifecycle.rs:313restore_runtime_state_inner:每次应用启动,对每个 desired-enabled 扩展各一次
也就是说,启动耗时随「已启用扩展数 × 包体积」线性增长,而 MAX_PACKAGE_BYTES 上限是 1 GiB。
同时,一次升级链路上的重复哈希相当可观:
| 阶段 | 位置 | 哈希对象 |
|---|---|---|
ext_prepare_import 审核 |
installer.rs evaluate |
staging 全量 |
ext_commit_import 复核 |
installer.rs 再次 evaluate |
staging 全量(第 362 行附近的 installed_digest 另计) |
| 旧版本记录 | installer.rs:362 |
已安装目录全量 |
| 提交后激活 | installer.rs:557 → authorize_activation |
新目录全量 |
即一个包在一次升级中被完整读取约 4 遍。列表页 (ext_list_extensions) 已经用 display_stamp 的 (size, mtime) 缓存规避了这个问题,DisplayCache 的取舍也写得很清楚 —— 但激活路径没有同样的缓存。
对常见的小体积扩展(几百 KB~几 MB)这不构成问题,因此我把它列为中低优先级而非缺陷。但如果预期会出现较大的扩展(例如内置语料、字典),建议把 DisplayCache 的 (size, mtime) 快速路径复用到激活路径,或在文档中明确写出激活/启动的哈希成本与 1 GiB 上限之间的取舍。
|
|
||
| #[test] | ||
| fn imports_real_cli_zip_when_supplied() { | ||
| let Ok(path) = std::env::var("MOKE_TEST_IMPORT_ZIP") else { |
There was a problem hiding this comment.
测试质量(中):这个用例在 CI 中必然「通过」,但实际上什么都没测
let Ok(path) = std::env::var("MOKE_TEST_IMPORT_ZIP") else { return; };未设置环境变量时直接 return,测试框架会把它记为通过。CI 不设置该变量,因此「真实 CLI 产出的 ZIP 能被宿主成功导入」这条端到端断言从未执行,却计入了绿灯。docs/TB-127-validation.md:147、:156 对此是诚实的(「含 1 条无外部 ZIP 输入时提前返回的可选用例」),但测试策略仍建议调整 —— 这条路径横跨 JS 打包器与 Rust 验证器两侧的契约,恰恰是最需要持续回归的地方。
建议:
- 最低成本:改为
#[ignore],并在文档中给出cargo test -- --ignored的运行方式 ——#[ignore]会在cargo test输出中明确显示为ignored,不会伪装成通过; - 更好:把一个小体积 fixture ZIP 直接提交进仓库(
tests/fixtures/signed-extension-v1/已有可用的签名夹具,据此生成一个确定性 ZIP 即可),让它成为默认执行的用例。
顺带确认:prepare/commit 的双次校验、.transaction 日志与 recover() 的回滚/归档分支、after_switch() 注入点、create_new(true) + 实际字节计数等设计我逐段读过,逻辑是自洽的;问题只在于验证它的测试没有真正运行。
| const EXTENSION_ACL_COMMANDS: &[&str] = &[ | ||
| "ext_list_extensions", | ||
| "ext_prepare_import", | ||
| "ext_commit_import", | ||
| "ext_cancel_import", | ||
| "ext_enable_extension", | ||
| "ext_disable_extension", | ||
| "ext_uninstall_extension", | ||
| "ext_get_api_port", | ||
| "ext_get_extensions_dir", | ||
| "ext_diagnostics", | ||
| ]; |
There was a problem hiding this comment.
流程/契约(中):EXTENSION_ACL_COMMANDS 没有与 invoke_handler! 做双向锁定,已经出现漂移
tests/reader-only-build.test.mjs:41-42 为 APP_ACL_COMMANDS 做了「必须是字面量列表」的契约校验,第 91 行对新代码只断言了语法:
assert.match(tauriBuild, /\[APP_ACL_COMMANDS, EXTENSION_ACL_COMMANDS\]\s*\.concat\(\)/);它校验的是 .concat() 怎么写,而不是列表内容与 handler 是否一致。后果是漂移不会被发现:
- 往
mod.rs的invoke_handler!添加一个ext_*命令、却忘了加进本列表 → 该命令在运行时拿不到 ACL 权限(前端调用直接失败),CI 全绿; - 反向:本列表里写了一个 handler 中不存在的命令 → 生成的 ACL 指向不存在的命令,同样无人发现。
而且漂移已经发生:mod.rs:846 的 ext_moke_list_offline_books 被注册进 invoke_handler!,但它既不在 EXTENSION_ACL_COMMANDS 也不在 APP_ACL_COMMANDS 中(我 grep 了 src/、capabilities/、permissions/,前端无任何调用点,也没有对应的 autogenerated permission 文件)。这是一个「注册了但永远无法被调用」的死命令。
建议参照仓库已有的 APP_ACL_COMMANDS ↔ readestlib::reader_invoke_handler 对比方式,把 EXTENSION_ACL_COMMANDS 与 mod.rs 的 invoke_handler! 列表也做一次集合比对(两个方向都比),并顺手决定 ext_moke_list_offline_books 的去留 —— 要么补进列表并接入前端,要么删除。这条命令会把离线书目暴露给调用方,属于应受 ACL 约束的能力,不适合以「未授权但已注册」的状态长期存在。
| if (manifest.entry?.backend) { | ||
| if (!manifest.entry.backend.targets?.length) throw new Error('Declare entry.backend.targets before packaging'); | ||
| if (!files.some(f => f.name === manifest.entry.backend.executable)) throw new Error('Backend executable is missing'); | ||
| const signature = JSON.parse(readFileSync(join(root, 'signature.json'), 'utf8')); | ||
| if (signature.package_sha256 !== computePackageDigest(root)) throw new Error('Package changed after signing; sign dist/ again'); | ||
| } |
There was a problem hiding this comment.
BUG(中):签名新鲜度校验被嵌在「有后端」分支内,纯 UI 扩展可以打包出过期签名
第 42–47 行的整套校验 —— 包括最关键的 signature.package_sha256 !== computePackageDigest(root) —— 全部位于 if (manifest.entry?.backend) 之内。而 signPackage(sign.js)只要求 manifest.publisher,并不要求声明后端。于是作者的典型流程会静默产出坏包:
moke-ext sign dist/→ 生成signature.json- 修改
ui.js(或任何不影响权限声明的文件) moke-ext package dist/ dist/x.zip→ 没有entry.backend,整段校验被跳过,ZIP 里带着一个已失效的signature.json出厂
宿主侧随后在 installer::evaluate → trust.rs 直接硬阻断,报「签名无效: 包内容摘要与签名记录不一致」。结果是:包能成功产出、能分发给用户,但用户导入时 100% 失败,且报错指向「签名无效」,会误导作者去怀疑签名算法,而不是「重新 sign」这一步。
更值得指出的是测试覆盖:tests/extension-zip.test.mjs 的用例名是 “stale signatures … are rejected”,但第 28 行的 fixture 恰好声明了 entry.backend,所以这个缺口完全没有测试覆盖,用例名夸大了实际覆盖面。
建议把新鲜度校验移出分支,只要 signature.json 存在就执行:
if (manifest.entry?.backend) {
if (!manifest.entry.backend.targets?.length) throw new Error('Declare entry.backend.targets before packaging');
if (!files.some(f => f.name === manifest.entry.backend.executable)) throw new Error('Backend executable is missing');
}
const signaturePath = join(root, 'signature.json');
if (existsSync(signaturePath)) {
let signature;
try { signature = JSON.parse(readFileSync(signaturePath, 'utf8')); }
catch { throw new Error('signature.json is unreadable; sign dist/ again'); }
if (signature.package_sha256 !== computePackageDigest(root)) throw new Error('Package changed after signing; sign dist/ again');
}(顺带解决第 45 行的问题:目前后端扩展遇到缺失或损坏的 signature.json 时,作者看到的是裸的 ENOENT / JSON.parse 报错,而不是「请先 sign dist/」的指引。)再补一条「纯 UI 扩展签名后修改文件」的用例即可。
| function TrustBadge({ ext }: { ext: ExtensionInfo }) { | ||
| const blocked = Boolean(ext.trust.blockedReason); | ||
| const Icon = blocked ? ShieldX : ext.trust.trusted ? ShieldCheck : TriangleAlert; | ||
| const className = blocked | ||
| ? 'bg-red-50 text-red-700' | ||
| : ext.trust.trusted | ||
| ? 'bg-green-50 text-green-700' | ||
| : 'bg-amber-50 text-amber-700'; | ||
| return ( | ||
| <span className={`inline-flex items-center gap-1 text-[10px] px-1.5 py-0.5 rounded-md ${className}`}> | ||
| <Icon className="w-3 h-3" /> | ||
| {trustLabel(ext)} |
There was a problem hiding this comment.
BUG(中):被阻断的扩展仍然显示「签名有效 / 发布者已确认」
trust.rs 在「版本降级/重放」和「同版本摘要变化」两种情况下只设置 blocked_reason,不修改 signature_status。因此一个签名确实有效、发布者也已确认的包,会同时满足 signatureStatus === 'trusted' 且 blockedReason != null。
此时本组件:图标判断是正确的(第 317–320 行用红色 ShieldX),但第 327 行的文案仍然调用 trustLabel(ext),显示 「签名有效 / 发布者已确认」。在安全 UI 里,「红色警告图标 + 安抚性文案」是最糟糕的组合 —— 用户通常只读文字,很可能据此认为包没问题。而且列表卡片(第 242 行)完全不渲染 blockedReason,唯一能看到阻断原因的地方是点进详情页,等于把关键信息藏在二级页面。
建议让文案也受阻断状态支配:
{blocked ? `已阻断:${ext.trust.blockedReason}` : trustLabel(ext)}并在列表卡片上以一行小字显示 blockedReason(或至少显示「已阻断」),不要让阻断原因需要点击才能看到。
另外,这里也涉及本项目 AGENTS.md 的墨水屏约定:bg-red-50 / bg-green-50 / bg-amber-50 在 src/app/globals.css 中没有任何 [data-eink='true'] 规则覆盖。现有 e-ink 规则会把 text-* 强制为黑色(可读性没问题),也对 bg-amber-50/40、bg-destructive/10 做了中性化,但纯 bg-{green,red,amber}-50 会保留淡彩底色,与「彩色底 → #E5E5E5 中性面」的约定不符;detail/page.tsx 中的 bg-amber-50 border-amber-200 风险提示框同理。建议补上对应的 e-ink 规则(注意用 :not([class*='hover:...']) 避开 AGENTS.md 里记录的贪婪子串匹配陷阱)。
| const cancelImport = async () => { | ||
| if (!preview || actionInProgress) return; | ||
| try { await cancelExtensionImport(preview.ticket); setPreview(null); } | ||
| catch (e) { setErrorMsg(String(e)); } | ||
| }; |
There was a problem hiding this comment.
BUG(中):取消导入失败时错误提示被模态对话框挡住;且取消操作没有并发保护
errorMsg 渲染在第 131–136 行,位置在 <dialog> 之前。而对话框是用 showModal() 打开的(进入 top layer),其 backdrop 会覆盖同文档内所有非 top-layer 内容。于是当 ext_cancel_import 失败时(第 47 行 catch (e) { setErrorMsg(String(e)) }):
preview未被清空 → 对话框继续开着- 错误信息被绘制在 backdrop 之后 → 用户完全看不到
用户看到的是一条「点了没反应」的取消按钮,唯一能做的动作就是再点一次取消(而它会重复失败)。这类「静默失败」在跨进程文件操作里尤其容易发生(文件被占用、目录权限、.host-lock 争用),不是罕见分支。
另外第 45 行的守卫用的是 actionInProgress,但 cancelImport 自己从不设置它(对比 commitImport 会设置),因此:
- 按 Esc(第 120 行
onCancel)与点击「取消」可以同时在途 → 两次ext_cancel_import - 快速双击「取消」同理
后端 ext_cancel_import 是幂等的,所以危害有限,但会产生重复 IPC 与 setPreview(null) 的竞态。
建议:把错误提示移到对话框内部(或在 top layer 内呈现),并给 cancelImport 加上与 commitImport 对称的在途标记,例如 setActionInProgress('cancel') / finally { setActionInProgress(null) }。
| useEffect(() => { | ||
| if (preview && dialog.current && !dialog.current.open) dialog.current.showModal(); | ||
| return () => { if (preview) void cancelExtensionImport(preview.ticket).catch(() => {}); }; | ||
| }, [preview]); |
There was a problem hiding this comment.
中(开发期):effect cleanup 把「预览变化/组件卸载」与「用户放弃导入」混为一谈,Fast Refresh 会误取消仍然有效的票据
第 33–36 行的 cleanup 在 preview 变化或组件卸载时都会调用 cancelExtensionImport(preview.ticket)。
生产环境下这条路径基本安全:preview 只会在 commit/cancel 之后变为 null,而提交开始时后端已经 take() 走了 pending 票据,多余的一次 cancel 是 no-op(installer.rs 的 ext_cancel_import 对未知 ticket 直接返回成功)。
但在 pnpm tauri dev 下(next.config.mjs 开启了 reactStrictMode),Fast Refresh 重挂载会重新执行 effect:cleanup 先对一张仍然有效的票据发出 cancel,把用户正盯着的审核对话框静默作废 —— 下次点「确认安装」会得到「导入已取消或过期」,而用户没有任何线索知道这是自己改代码触发的。挂载期的 StrictMode 双调用在这里无害(初次 preview 为 null),受影响的只有 HMR。
建议用 ref 记录「是否已提交或已主动取消」,把 cancel 收敛到真正的 unmount 上:
const previewRef = useRef(preview);
previewRef.current = preview;
useEffect(() => () => {
const current = previewRef.current;
if (current && !settledRef.current) void cancelExtensionImport(current.ticket).catch(() => {});
}, []);(dialog.showModal() 那段可以留在 [preview] 的 effect 里。)更彻底的做法是让后端区分「客户端断开」与「显式取消」,让票据只在显式取消或 10 分钟过期时失效。
| function trustLabel(ext: ExtensionInfo): string { | ||
| switch (ext.trust.signatureStatus) { | ||
| case 'trusted': return '签名有效 / 发布者已确认'; | ||
| case 'unknown_publisher': return '签名有效 / 未知发布者或新密钥'; | ||
| case 'invalid': return '签名无效'; | ||
| default: return '未签名'; | ||
| } | ||
| } | ||
|
|
||
| function buildApprovalMessage(ext: ExtensionInfo): string { | ||
| const lines = [ | ||
| `启用 ${ext.displayName} 前请核对:`, | ||
| '', | ||
| `签名:${trustLabel(ext)}`, | ||
| `发布者:${ext.trust.publisherName || ext.trust.publisherId || '未知'}`, | ||
| `来源:${ext.trust.source || '未声明'}`, | ||
| `版本:${ext.trust.upgradeFrom ? `${ext.trust.upgradeFrom} → ` : ''}${ext.version}`, | ||
| `当前权限:${ext.permissions.join('、') || '无'}`, | ||
| ]; | ||
| if (ext.trust.permissionsAdded.length) lines.push(`新增权限:${ext.trust.permissionsAdded.join('、')}`); |
There was a problem hiding this comment.
代码质量(中):trustLabel 与 buildApprovalMessage 在两个页面中逐字重复
这两个函数在 src/app/extensions/page.tsx(第 284–314 行)和本文件(第 222–241 行)中各存在一份完全相同的实现。这不只是「重复代码」的洁癖问题 —— 它们承载的是安全语义:
trustLabel的四种措辞(签名有效 / 发布者已确认、签名有效 / 未知发布者或新密钥、签名无效、未签名)必须与trust.rs中signature_status的取值集合严格对应;buildApprovalMessage是用户在授权前看到的唯一授权依据(签名、发布者、来源、版本变化、权限增删、风险提示、内容摘要),它的措辞直接决定用户能否做出正确判断。
两份副本意味着:将来调整信任语义(例如新增一种 signature_status,或按 §信任模型补充风险项)时只要漏改一处,就会出现「列表页说 A、详情页说 B」的分歧 —— 而这种分歧恰好发生在用户做安全决策的界面上,且没有任何测试会捕获它。
建议抽取到 src/lib/extension-trust.ts(与 src/lib/store/extensions.ts 中的 ExtensionTrust 类型放在一起,让类型与文案同源),两个页面共同引用。如果希望更稳,可以再补一个穷尽性检查(switch 加 default: assertNever(status) 或 satisfies 约束),让新增 signature_status 时编译期就报错。这与上一批评论中「被阻断的扩展仍显示安抚性文案」是同一处的两个缺陷,建议一并修掉。
| 开发者明确要求解决 #131 冲突,并将依赖 PR 的改动收拢到 #131 后关闭依赖 PR。本轮授权替代首次交付时“不关闭 #111/#115”的操作安排,目标仍为更新 #131 的工作分支。 | ||
|
|
||
| - 合入 main `05f5d145096b6fa1c015a87333d71ae89e5e1a44`。#111 `7530567aab7f1654986bbd93460a38b0c35302f6` 和 #115 `310c9b2177d16020904ebcbd74d09055d462bd42` 已经是原 #131 head 的祖先,保留完整提交历史,无需再次 cherry-pick。 | ||
| - 解决 Cargo、build.rs、主 WebView capability、生成 ACL 数据及 extensions API/events/mod 冲突。REST 复用 main 的有界 `http_transport`,保留 #131 的随机会话端口、Host/Origin/token 与权限快照检查;保留 main 的 POST/PUT Content-Length 和完整正文检查。连接上限采用 main 的 32,认证后支持合法 Expect: 100-continue,同步作者文档。 |
There was a problem hiding this comment.
可用性 / DoS(中高):文档记录了沿用 main 的 32 连接上限,但没有记录它与本 PR 新增的「32 路命令等待」预算相等这件事
我核对了这一条:http_transport.rs 确实不在本 PR 的改动文件中,连接上限、DeadlineStream、Content-Length 校验等都来自 main,文档的描述是准确的。但把三处常量放在一起看,会出现一个不该被漏掉的耦合:
| 常量 | 值 | 位置 |
|---|---|---|
MAX_CONNECTIONS |
32 | http_transport.rs:11(沿用 main) |
MAX_CONCURRENT_COMMAND_WAITS |
32 | api_server.rs:19 |
MAX_COMMAND_WAIT_MS |
30_000 | api_server.rs:17 |
同步命令等待是在**占用一个 HTTP worker(即一个连接槽)**的前提下阻塞在 rx.recv_timeout(...) 上的。因此一个已认证、拥有 reader.command.send 权限的扩展,只要发 32 个 wait_ms=30000 的长轮询 POST /reader/{label}/command,就能把全部 32 个 worker 占满 30 秒。这期间任何其它请求 —— /api/v1/info 健康检查、storage、其它扩展的请求 —— 都只能拿到 SERVER_BUSY(api_server.rs:773)。不需要漏洞,也不依赖传输层有任何缺陷。
这一点之所以值得写进本 PR 而不是留待以后:正是本 PR 第一次让第三方扩展能够触达这套 API,两类预算相等从「理论上的数字巧合」变成了「一个已签名扩展就能触发」。建议:
- 让
MAX_CONCURRENT_COMMAND_WAITS明显小于MAX_CONNECTIONS(例如 8),保证始终有 worker 能服务非阻塞端点;或把等待请求隔离到独立 worker 池;或让MAX_COMMAND_WAIT_MS远小于连接超时; - 在本文件的对应行补一句说明:连接上限沿用 main 是有意的,但其与命令等待预算的相等关系是已知的可用性取舍(或已被上述改动消除)。
顺带一提,同一节把 32 描述为「限流」是不准确的:它是一个全局并发上限,没有按来源的公平性或配额,超额时被拒的是新到的连接(http_transport.rs:196-203 回 503),占用槽位的一方不承受任何代价。措辞若改为「并发连接上限」会更贴合实现。
|
|
||
| - 合入 main `05f5d145096b6fa1c015a87333d71ae89e5e1a44`。#111 `7530567aab7f1654986bbd93460a38b0c35302f6` 和 #115 `310c9b2177d16020904ebcbd74d09055d462bd42` 已经是原 #131 head 的祖先,保留完整提交历史,无需再次 cherry-pick。 | ||
| - 解决 Cargo、build.rs、主 WebView capability、生成 ACL 数据及 extensions API/events/mod 冲突。REST 复用 main 的有界 `http_transport`,保留 #131 的随机会话端口、Host/Origin/token 与权限快照检查;保留 main 的 POST/PUT Content-Length 和完整正文检查。连接上限采用 main 的 32,认证后支持合法 Expect: 100-continue,同步作者文档。 | ||
| - WebSocket 共用 main 的 DeadlineStream,保留 #131 更严格的 8 个待认证连接、32 个客户端、128 KiB 握手预算和 Origin/订阅权限门禁。合并并适配 main 的慢 Upgrade、Ping/慢认证帧、容量回收与事件重放真实 socket 回归,补充 Host/Origin/权限联合回归。移除被统一传输替代的旧 transport 模块。 |
There was a problem hiding this comment.
安全一致性(中):这里完整记录了 WS 的握手期门禁,但没有记录「握手之后不再复核」这一撤销缺口
先确认记录准确的部分:8 个待认证连接、32 个客户端、128 KiB 握手预算、Origin 与 reader.events.subscribe 订阅权限校验,我逐处核对过 events.rs,与文档一致;authenticate_and_subscribe 要求 token 匹配且具备订阅权限才接受连接,broadcast_to_clients 只投递客户端订阅过的事件,这些设计是扎实的。
缺口在于撤销:HTTP 侧是刻意在读完正文后重新认证一次的(api_server.rs:628-638),目的正是捕捉「读取期间扩展被禁用/卸载」的场景。而 WS 侧只在 hello 帧认证一次(events.rs:381-469),客户端进入 clients 后,事件分发(events.rs:489-500)不再做任何重新校验,也不与 enabled 集合比对,代码里没有主动驱逐路径。
实际影响:禁用/卸载会杀掉扩展后端进程,从而关掉由后端建立的连接;但由前端页面(浏览器 UI)建立的长连接不会随之断开,会继续接收 reader:* 事件直到它自行断开。本 PR 的主题恰恰是收紧扩展的信任边界 —— 签名、摘要、Origin、订阅权限都做到了「每次重新判定」,唯独撤销只覆盖「建立连接那一刻」。
建议三选一:
- 在
ext_disable_extension/ext_uninstall_extension中主动关闭该扩展名对应的所有 WS 客户端(与 HTTP 的撤销语义对齐); - 在分发路径上周期性重新校验
enabled与 token; - 保持现状,但在本文件与
docs/extension-trust-model.md中显式写明「已建立的 WebSocket 连接在扩展被禁用后仍会延续,直到客户端主动断开」,避免读者从 HTTP 的重新认证行为错误地推断 WS 具备同等撤销保证。
我倾向 1 —— 它与 HTTP 的既有语义一致,改动量也最小(enabled 的移除路径已经在同一把锁下)。
| - reading-stats 静态路径按组件拒绝根路径、父路径、反斜杠、Windows 驱动器和控制字符;增加 `%2F`、编码遍历、UNC/驱动器及正常资源路径用例。注释明确 `/api/stats` 为严格 Host、无 CORS 的回环同源只读服务,同用户进程仍可读取;不向浏览器下发宿主 token。 | ||
| - 保留未知签名发布者每次启用确认策略,在 trust.rs 和作者文档说明空内置根、再次启用与重启待恢复行为。回归验证历史 grant 不能授权无确认启动,同一摘要再次显式确认仍可通过。 | ||
|
|
||
| 验证结果在本轮提交前记录;临时无 GUI 核心 harness 通过 path 引用生产 discovery/installer/lifecycle/trust/package_path/events/deadline_stream/security/http_transport/permissions/storage 模块,逐字提取 runtime 数据结构和 activate/start/stop/list helpers,以 url crate 提供 URL 类型、空 PendingCommands 替代 API 注册表。它验证核心 Rust 行为,但不编译 Tauri IPC commands/宏、完整 REST 路由或 Reader。可选实际 CLI ZIP 用例本轮没有 MOKE_TEST_IMPORT_ZIP 输入,提前返回,不能据此声称重做真实 CLI 包启用验收。 |
There was a problem hiding this comment.
验证覆盖(中):本文件已诚实说明 harness 不含 Tauri IPC/REST 层,但这一点的后果值得写得更具体 —— 有几类缺陷正好落在没有任何测试执行的那一层
先肯定:这一段(以及第 131、133 行)的自述是我在这个 PR 里见到的最诚实的验证记录之一 —— 明确写了「Tauri IPC 使用 mock」「不编译 Tauri IPC commands/宏、完整 REST 路由或 Reader」「完整 cargo check --lib 未完成」「本轮没有重新运行完整 ZIP 原生安装/回滚验收」,也明确「不能据此声称重做真实 CLI 包启用验收」。这不是文档问题,而是风险敞口需要被显式列出:
| 未被执行覆盖的层 | 恰好落在其中的问题 |
|---|---|
| Tauri command 宏 / IPC 参数反序列化 | ExtensionApproval(mod.rs:177)带 deny_unknown_fields 且字段名是 snake_case package_digest,而顶层参数经 Tauri 默认重命名为 camelCase;Playwright 用例 mock 了 __TAURI_INTERNALS__.invoke,所以这条真实反序列化路径从未运行过 |
ACL 列表 ↔ invoke_handler! 一致性 |
已存在漂移:mod.rs:846 的 ext_moke_list_offline_books 注册在 handler 中,却不在 EXTENSION_ACL_COMMANDS(build.rs:54-65)也不在 APP_ACL_COMMANDS,且前端无调用点 —— 现有测试(tests/reader-only-build.test.mjs:91)只断言 .concat() 的写法,不断言列表内容 |
api_server.rs(完整 REST 路由) |
上面两条评论提到的「32 路命令等待 × 32 连接」预算耦合、以及命令等待的并发行为,都在这个文件里,而 harness 的引用列表中没有它 |
| 原生安装/回滚 | .transaction 日志、recover() 归档、after_switch() 注入点、create_new(true) 写入等逻辑设计自洽,但本轮未跑原生验收 |
也就是说,最需要真实 IPC 才能暴露的问题,恰好都集中在 mock 覆盖的那一层。建议在本轮之后补两项成本不高的检查:
- 一个不依赖 mock 的 ACL 一致性测试:把
EXTENSION_ACL_COMMANDS与mod.rs的invoke_handler!做集合双向比对(与仓库已有的APP_ACL_COMMANDS ↔ readestlib::reader_invoke_handler对比同一模式); - 一个最小化的真实 Tauri command 冒烟(
tauri::test::mock_builder之类),至少覆盖ext_commit_import的嵌套approval参数与ext_enable_extension的确认绑定 —— 这正是tests/extension-import.spec.mjs想验证、但因 mock 而只验证了前端一半的契约。
桌面扩展改为本地 ZIP 导入:先校验并展示发布者、签名、版本和权限变化,再确认安装。首次安装保持禁用;已启用扩展升级后恢复运行,失败回滚旧目录与状态。CLI 与阅读统计示例生成可导入 ZIP,保留旧扩展数据及迁移说明。
关联 TB-127。完整承接 #111(
7530567aab7f1654986bbd93460a38b0c35302f6)和 #115(310c9b2177d16020904ebcbd74d09055d462bd42);两个 head 均为本 PR 祖先,两个被承接 PR 已按开发者要求关闭。最新 main05f5d145096b6fa1c015a87333d71ae89e5e1a44已合入,重新 fetch / merge 确认无新增提交或冲突。本轮 review 修复 head:1f07a680aa68591b26292e908d63fe90f7a481df。/api/stats有意采用回环 + 严格 Host + 无 CORS,同用户本地进程仍可读取;宿主会话 token 仅供后端 WS 使用,不向浏览器返回。pnpm test:e2e:extensions由独立 CI job 调用,专用 Tauri-mode Next 服务覆盖 ZIP 导入、漫画、详情操作。接入时发现窄屏短末页在长条模式恢复后回写上一页,已增加 Moke 宿主尾部滚动空间;固定第三方产物保持原样。当前内置可信根密钥为空。未知签名发布者每次启用(含禁用后再次启用)都需显式确认,重启保持待恢复;可编辑历史 grant 不能提升为发布者信任。未签名且无原生后端的旧 UI 包可复用相同内容确认,未签名原生后端始终阻断。该策略已补入 trust.rs、作者文档与回归。
本轮本地验证:Node 491 passed;typecheck 通过;lint 0 errors / 23 条既有 warnings;生产传输 Rust harness 20 passed;临时无 GUI 核心 Rust harness 59 passed;reading-stats 后端 5 passed;最终完整 Chromium 浏览器集 38 passed(Tauri IPC/HTTP mock);末页修复后漫画 Node 集 6 passed;CI YAML、diff-check 和依赖 ancestry 核对通过。命令、历史证据、失败修复与范围见
docs/TB-127-validation.md,作者流程见docs/extension-zip.md。完整原生 cargo check 因缺少 vendor/openharmony-ability 子模块停止。临时核心 harness 直接引用生产模块及提取的 host helpers,不编译 Tauri IPC 宏、完整 REST 路由或 Reader;59 项包含一条本轮无外部 ZIP 输入时提前返回的可选用例。未重做原生 GUI/OS 文件选择器、真实 CLI ZIP 后端启动、跨平台真机或全仓 DeepSec 扫描。首轮全仓扫描的既有发现和覆盖限制仍在验证文档,不能称全仓安全通过。Reader gitlink 沿用 main 的
4b3db91bd1d94e5e9063639964f95a407334e81f。原生后端不是沙箱;同用户进程仍可能在最终验签到 OS loader 打开文件间替换内容。旧目录保留会占磁盘,REST 每连接一次请求并拒绝 chunked。正式根密钥、在线撤销、token TTL 刷新及 TB-125 整体安全复测为后续范围。推送触发 CI,本轮不等待外部结果。