jianf/MailUI4Agents
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
c8fcc6ad85fe9de6552ace2b6c66f1bc152131db
MailUI4Agents/docs/reviews/push-and-gui-review.md
JianFeeeee 3b8204f356 docs(审查): 补上 pi 建议的第二格判据（pi 又改了一次它自己的标注） 
上一提交只写进了一格判据，pi 随后把它的建议补上 —— 报告里现在是两格：

  · `TestHMSAccessTokenFailureDoesNotBurnQuota`          钉内部计数器 dayCount==0
  · `TestHMSQuotaSurvivesTokenFailureWithLimitOne`       钉用户看得见的行为

两层都要钉的理由：**计数器对而行为错是可能的** ——
运维会收到「达到每日推送上限」这种**误导性文案**，
而真实原因只是上一次网络抖动。这正是它建议单独加一格的原因。
2026-09-28 09:53:12 +08:00

19 KiB
Raw Blame History

系统推送链 与 GUI 审查报告

审查对象：

  • 推送链（端到端）：client/harmony 推送侧（PushService.ets / PushContract.ts / EntryAbility.ets） ＋ server/internal/push/*（push.go / hms.go / config.go）＋ handler/push.go ＋ repo/push_tokens.go ＋ notify/mail.go
  • GUI（Electron/React）：client/electron/src/（约 14,100 行，10 个 store + 30 个组件）

基线：node test/run-all.mjs → 577 判据 / 565 通过 / 3 红（与本报告无关，见上份报告） Go 侧未能执行 go test（本机无 GOMODCACHE/GOPATH），推送逻辑以读码 + 对照既有测试验证。

第一部分：系统推送

一、先说结论：推送链整体是健康的

设计上有几处明确比一般实现高明的地方，先列出来避免后续误改：

  • 不写死华为：Notifier 接口 + Factory 注册表 + push.json 配置式接入， RegisterType 一个函数就能加厂商。
  • 单项配错不拖垮服务：config.go:133-159 逐项 continue 并打明确日志， 网关照常启动 —— 推送是锦上添花，不能变成单点故障。
  • 推送是可选旁路且零开销：shouldDispatch 抽成纯函数（push.go:115-117）， 且注释诚实记录了变异验证曾经失败的原因（goroutine 竞态让错误理由也能绿）。
  • 所有失败静默：客户端 PushService 每一步 catch 都不打扰用户、不阻塞登录。
  • 有效 token 自愈：华为回 80300007 / illegal_tokens 时自动删表（hms.go:244-254）， 因为测试额度是项目级的，死 token 会白吃额度。
  • 动态 IN (...) 的方言安全性我已验证：repo/push_tokens.go 手拼 $1,$2… 占位符， 而 db.go:7 明确「SQLite 也支持 $1/$2，无需改写」⇒ 两种方言都安全，不是 SQL 注入 （占位符位置由 i+1 生成，值一律走参数绑定）。

二、推送链的真实缺陷

1.【HIGH】每日额度按 token 数扣，且在实际投递之前扣

hms.go:192-194：

if !h.reserveDaily(len(tokens)) {
    return fmt.Errorf("达到每日推送上限 %d 条（HMS_DAILY_LIMIT）", h.DailyLimit)
}
tok, err := h.accessToken(ctx)   // ← 额度已经扣了
...
// 网络请求在下面，失败也只记日志

两个问题叠在一起：

  • 计数单位错：reserveDaily(len(tokens)) 记的是 token 个数，而注释与变量名 （dayCount、达到每日推送上限 N 条）都说是「条」。华为的测试消息额度是 按条消息限的（一次 messages:send 一条消息，无论带几个 token）。 ⇒ 3 个设备 1 封邮件就吃掉 3 条额度，实际发出去 1 条。 多设备自部署用户会以约 1/设备数 的速度提前耗尽 1000 条/天。
  • 扣在投递前：accessToken 失败、HTTP 失败、华为返回非成功码 —— 这些一条都没发出去， 但额度已经扣掉了。而失败只 log.Printf（push.go:174-176），用户侧完全无感。 ⇒ 一次网络抖动会静默烧掉配额。

修法：reserveDaily(1)（按批次计），并把预留挪到确认拿到 access_token 之后、 或失败时归还。

修复状态（2026-09-28）：① 已修（044a664，reserveDaily(1)，有判据 TestHMSDailyLimitCountsMessagesNotTokens）。 ② 曾被声称已修但没落地 —— 044a664 的 commit message 承诺「挪到确认能发之后」， diff 里却只改了传参，调用位置仍在 accessToken 之前，注释与代码自相矛盾。 之所以没被当场发现，是因为当时那批判据造不出「accessToken 失败」这条路。 现已真正落地（预留移到 accessToken 成功之后、发请求之前）， 并补上两格判据：

  • TestHMSAccessTokenFailureDoesNotBurnQuota —— 钉内部计数器（dayCount == 0）
  • TestHMSQuotaSurvivesTokenFailureWithLimitOne —— 钉用户看得见的行为 （DailyLimit=1 时一次 accessToken 失败后第二次仍然要能发出去）

两层都要钉：计数器对而行为错是可能的 —— 那会让运维收到「达到每日推送上限」这种 误导性文案，真实原因却是上一次网络抖动。 变异验证：把预留挪回 accessToken 之前，两格同时红。 抓住这处不一致的是 pi。

2.【HIGH】HMS 通知载荷缺 click_action，锁屏点击可能带不出 data

hms.go:206-217：

"message": map[string]any{
    "token": tokens,
    "notification": map[string]any{ "title": ..., "body": ... },
    "data": string(data),          // ← data 带了，但没有任何 click 行为声明
}

客户端完全依赖 data 里的 mail_id 做跳转： EntryAbility.onCreate / onNewWant → PushService.routeFromWant → routeFromWant(params, ...) 读 parameters['data']（EntryAbility.ets:84-86、120-122）。

而载荷里没有 click_action、没有 intent、没有 backgroundMode。 HMS v1 在「普通通知」形态下，点击行为由 click_action 决定（1=打开应用、2=打开页面、 3=触发 Action）；缺省时不同形态/系统版本对 data 是否透传到 want.parameters 的行为并不一致。 项目里 PushContract.ts:9-10 自己写着「服务端推送是至多一次、无幂等键」， EntryAbility.ets:78-82 也记录了 2026-09-15 为此专门加过冷启处理 —— 说明这条链此前已经因「跳转目标丢失」返工过一次，而根因（载荷形态）没有变。

这正是「App 弹到前台、停在列表」的另一种表现形态。

修法：显式声明点击行为（HMS v1 click_action 或按 Action 形态补 intent）， 并用真机 token 实测一次确认 data 能进 want.parameters —— hms.go:50 自己写着「我无法在本机验证成功路径（需要一台真机产出的 token）」， 这个未知项至今没有关闭。

3.【MEDIUM】通知标题把完整邮件主题写进锁屏

push.go:36-38 的设计声明：

「字段刻意少：推送只负责『通知栏那一行 + 点进去看哪封信』。 正文不进通知，否则锁屏上就会露出邮件内容。」

但 hms.go:211 实际是：

"title": "新邮件：" + truncate(n.Subject, 40),   // ← 主题全文前 40 字
"body":  n.From,

Subject 是邮件主题本身，标题里就带「报销 3 万走哪个账户」这类内容。 声明说"正文不进通知"，实现却把主题放进去了 —— 这在锁屏上是可见的。 （truncate 限了 40 字，但 40 字足够泄漏一个机密项目名。）

修法：要么按声明改成固定文案（如「新邮件」+ 发件人）， 要么把 push.go 的注释改成与实现一致 —— 现状是注释在骗人，这比任一选择都糟。

4.【MEDIUM】客户端写死 provider: 'hms'，与"配置式多厂商"自相矛盾

PushContract.ts:13 + PushService.ets:348,381 无条件用 PROVIDER_HMS。

服务端明确支持一实例多厂商、且 name 可覆盖（config.go:44-47）。 但客户端把 hms 硬编码，PushService.ts:360-361 又只是把 resp.providers 打进日志 就丢掉，从不据此选择。⇒ 服务端配了小米通道，鸿蒙客户端永远不会往它登记。

讽刺的是 PushContract.ts:203-207 的注释正在讲"不要硬编码只有 hms 合法"这个原则， 而调用处恰恰硬编码了。

5.【MEDIUM】NotificationLedger 只在 want 层去重，SSE 侧无跨通道去重

PushContract.ts:9-10 说「重复保护只能落在客户端」，EntryAbility 用 NotificationLedger(50) 按 mail_id 去重（EntryAbility.ets:42）。

但这只覆盖「点通知」这一条路径。用户实际会看到两次：

  1. App 在前台 → SSE new_mail → MainPage.ets:2911 showToast('新邮件：…')
  2. HMS 通知同时下发 → 通知栏一条

两条路径各自独立、无共享去重台账 ⇒ 前台每次都"toast + 通知栏"双份。 main.go 端也没有按在线状态抑制推送的逻辑（shouldDispatch 只看 Enabled() 和收件人非空）。

6.【MEDIUM】ledger 是 EntryAbility 实例字段，50 条就会开始漏

EntryAbility.ets:42 private ledger: NotificationLedger = new NotificationLedger(50)。 台账按 mail_id 去重且有界（50）。高流量场景（Agent 批量发信）下， 第 51 封之后旧 mail_id 被挤出 ⇒ 同一封邮件若被系统重放 want（HMS 在网络重试时确实会） 会再次触发跳转。属于可接受但需知情的取舍，limit=50 是否够值得确认。

7.【LOW】client 侧 PushService 在 aboutToAppear 之前就上报（已知，非新问题）

EntryAbility.onCreate:106 用 ApiClient.getInstance(...) 上报，而该实例尚未 init() （init() 只在 LoginPage.ets:101 与 MainPage.ets:3128 调）⇒ 此刻无 token，服务端回 401。 LoginPage.ets:168-192 的 reportPushToken 已在登录后补报，已修复。 仅记录：EntryAbility.ets:101 那句注释仍写「PushService.isEnabled，默认关」， 而 PushContract.DEFAULT_PUSH_ENABLED = true —— 注释与实现相反，会误导后续维护。

8.【LOW】SessionApi 同类问题：null 违反本仓约定

api/SessionApi.ets:48 proposal: RenameProposal | null = null。 本仓 ArkTS 约定偏向 undefined。服务端用 null 表示"没有待处理建议"， 客户端需要原样承载 —— 属于契约强制，但与 PushContract.ts 的 | undefined 风格不一致。

第二部分：GUI（Electron/React）

一、先说结论：GUI 的 XSS 面是干净的

我独立复核并确认：

  • 全仓零 dangerouslySetInnerHTML / innerHTML / insertAdjacentHTML
  • 三个 Markdown 渲染点（MailView.tsx:146、:690、ComposePage.tsx:305）统一用 <Markdown remarkPlugins={[remarkGfm]}>，没有 rehype-raw ⇒ 原始 HTML 被转义
  • 唯一的动态 href 是 Attachments.tsx:22 的 api.attachmentURL(a.attachment_id) （服务端签发的 id + encodeURIComponent 过的 token），唯一的动态 src 是本地生成的 JPEG data URL
  • electron/main.cjs:47-48：contextIsolation: true / nodeIntegration: false ✔
  • main.cjs:47-48 有 preload，未见 openExternal / will-navigate 缺口

这条链上 test/markdown-xss.test.mjs 的不变量没有被绕过。

二、GUI 的真实缺陷

1.【CRITICAL】切账号不清 store ⇒ A 账号的邮件显示在 B 账号下

AccountSwitcher.tsx:56-62：

const pick = async (id: string) => {
  setOpen(false);
  await setActive(id);
  await fetchInbox('all');   // ← 只重取了收件箱
};

setActive（accountStore.ts:186-197）会 syncAuth(next, id)，把 api/config 单例的 API_BASE 与 bearer 翻到新账号。此后：

  • useMailStore.sent、currentMail
  • useSessionStore.sessions、currentSession、currentSessionMails
  • useContactStore.contacts / archivedContacts

一个都没复位。而后续单账号请求（api.getSent()、api.getMail()、api.archiveContact()） 会带上 B 的凭证。

后果：A 账号下打开发件箱 → 切到 B → 发件箱仍是 A 的邮件； MailView.tsx:67 仍按 A 的会话渲染全文；从这些陈旧视图点转发/归档/批准，改的是 B。

grep 确认：只有 MailList.pick / ContactPanel.open / PermissionList.pick 会调 clearSession() / clearCurrentMail()，账号切换路径一个都没有。

修法：setActive 之后除 fetchInbox 外，还要 clearCurrentMail() / clearSession() / 清 sent / 清 contacts。

这与鸿蒙侧 MailStore.clear() 零调用（上份报告第 3 条）是同一个根因的两端： 两个客户端都在"切换身份"时只刷新了主列表，没清其余状态。

2.【CRITICAL】lastUploadedImage 是模块级单例、不按账号分 ⇒ 第二个账号的壁纸永不上传

appearanceSync.ts:40：

let lastUploadedImage = '';      // 模块级，不带账号维度

pull() 第 87 行写它（从服务端图回填时），push() 第 115 行用它做跳过判据：

if (ok && bg.kind === 'image' && bg.imageDataUrl && bg.imageDataUrl !== lastUploadedImage) {

路径：A 账号把壁纸设为 X（上传成功 ⇒ 标记 = X）→ 切到 B 账号，B 的服务端记录是 saved:false ⇒ pull() 走 appearanceSync.ts:69-76 的"把本地这份推上去"分支 （:74 调 push()）⇒ 但跳过判据被 A 留下的标记满足 ⇒ B 的壁纸永远不传，而 :120 仍把 status 置为 'synced'。

⇒ B 的壁纸在其其它所有设备上都缺失，而界面上显示"已同步"。

讽刺的是该文件头明确把 localStorage 键按账号做了隔离， 却漏了这个变量 —— 它同样需要 accountId 维度。

修法：lastUploadedImage 改成 Map<accountId, string>，或直接存进按账号分区的 localStorage（与现有 key 策略一致）。

3.【HIGH】权限审批吞掉错误 ⇒ 失败的"同意"看起来像成功了

MailView.tsx:746-761：

} catch (err) {
  console.error(err);        // ← 只进控制台
} finally { setBusy(false); }

setDecided(...)（:753）在 await api.decidePermission 之后。 ⇒ 请求失败时 decided 仍是 ''，界面回到可点状态，用户看不到任何提示， 只会以为"点了没反应"而反复点，而 Agent 那边一直阻塞。

同文件其它提交路径（ForwardBar.submit:401、ReplyBar.send:1046、BudgetEditor.commit:252） 都正确地 setError 了 —— 只有这一处是例外。

修法：catch 里 setError(...) 并把错误渲染出来。

4.【HIGH】转发有双发窗口，且"关窗"依赖可能失败的刷新

MailView.tsx:390-408：

if (!to.trim() || busy) return;
setBusy(true);
...
await api.forwardMail(...);
await Promise.all([fetchInbox('all'), fetchSent(), fetchSessions(), fetchContacts()]);
onClose();
  • busy 守卫要等 React 提交 setBusy(true) 之后才生效 ⇒ 快速双击会发出两个 forwardMail（真转发，不是本地乐观更新）
  • 四个刷新里任一 reject ⇒ onClose() 被跳过，尽管转发已经成功 ⇒ 用户看到报错、于是再转一次

修法：setBusy 之前用同步 ref 置位；onClose() 挪进 finally 或改成 Promise.allSettled。

5.【HIGH】ICS 导出的 object URL 同步 revoke ⇒ 文件可能 0 字节

CalendarView.tsx:262-267：

const url = URL.createObjectURL(new Blob([ics], ...));
const a = document.createElement('a');
a.href = url; a.download = `...ics`;
a.click();
URL.revokeObjectURL(url);      // ← 同一同步块内立即撤销

a 也从未插入 document。Firefox / WebKit 的下载是异步取 blob 的 ⇒ 同步撤销会导致静默失败或 0 字节 .ics（Chromium 通常容忍，故容易漏测）。

修法：setTimeout(() => URL.revokeObjectURL(url), 0)，并把 a 挂到 document 后再 remove()。

6.【MEDIUM】日历加载无请求代次守卫 ⇒ 快速翻月时"标题与内容错配"

CalendarView.tsx:101-106：

useEffect(() => { setLoading(true); load(); }, [range.from, range.to]);

load() 里无条件 setEvents(r.events ?? [])。 快速点「下一页/上一页」⇒ 两个请求同时在飞，后落地的覆盖先落地的 ⇒ 标题显示 11 月而网格是 10 月的事件。

对比：ThreadView.tsx:66-85 用 gen.current / myGen 做对了， CalendarView 没有对应实现。

7.【MEDIUM】三个 setTimeout 无卸载清理

ComposePage.tsx:160-163（900ms 后清提示并关窗）、AdminUsersPage.tsx:93-96、 KeyPanel.tsx:37-40。ComposePage 那条可达（清空按钮在 sending 期间未禁用）。

8.【MEDIUM】预设缩略图全部渲染同一张自定义图

BackgroundPicker.tsx:120-122：

className={`bg-preset-${p.id} absolute inset-0 block`}
style={{ backgroundImage: 'var(--bg-image)', backgroundSize: 'cover' }}

当 kind==='image' 时 --bg-image 有值 ⇒ 六个预设格子都显示同一张照片， 而用户正在这一步里挑选预设。

9.【LOW】api.attachmentURL 把 token 放进 query

api/client.ts:385-387 + :118-122。服务端有意支持 （main.go:299 用 middleware.UserAuthAllowQueryToken，handler/events.go:17 注释说明 这是浏览器 EventSource 专用回退）⇒ 不是缺陷。 但代价是 token 会进浏览器历史/服务端访问日志，是已知的取舍，记录备查。 （鸿蒙侧 ApiClient.getBytes 的注释明确写了"不用 ?token=，那会把密钥写进日志" —— 两端做法不一致，值得统一口径。）

三、本轮明确排除的疑点

  • 推送 SQL 注入：repo/push_tokens.go 的 IN (...) 占位符由序号生成、值全走绑定， 且两种方言都支持 $N ⇒ 安全。
  • Go 推送测试：23 个测试覆盖了形状、分批、限额、无效 token 清理、配置容错 —— 覆盖面是好的；TestHMSDailyLimitStopsSending 用例也说明"按 token 计"是已固化的行为， 但没人质疑这个计数单位（见缺陷 1）。
  • resetUI() 登出不清数据 store：单独看不可达（无账号时 AccountSwitcher 渲染 null）， 真正的泄露面是切账号路径（缺陷 1）。不算独立缺陷。
  • Electron MonthGrid key={i}：父级 key={${anchor.getTime()}-${scale}} 使其 每次导航都重挂载，索引不会跨数据变化 ⇒ 安全。
  • ContactPanel 双按钮：非嵌套可交互元素，无点击吞没。
  • GUI 监听器/定时器清理：LoginPage.tsx:43-55、AddressInput.tsx:195-199 （{capture:true} 成对）、backgroundStore.ts:379-393（两条路径都 revoke）⇒ 正确。

四、修复顺序建议

序 项 位置 理由
1 切账号清 store AccountSwitcher.tsx:56-62 唯一的跨账号数据泄露，改动最小
2 壁纸按账号分 appearanceSync.ts:40 静默丢数据 + 谎报"已同步"
3 审批错误提示 MailView.tsx:756-757 权限决策失败却无反馈，Agent 一直阻塞
4 每日额度计数单位 hms.go:192 用户可感知的功能失效（提前耗尽配额）
5 HMS click_action + 真机实测 hms.go:206-217 需真机 token 才能关闭这个未知项
6 ICS revoke 时机 CalendarView.tsx:267 导出功能在部分内核上失效
7 转发双发 / 关窗 MailView.tsx:390-408 数据重复
8 锁屏文案 hms.go:211 + push.go:36-38 注释与实现不一致（二选一，别留现状）
9 日历代次守卫 CalendarView.tsx:101 照抄 ThreadView.tsx:66-85 即可
10 provider 硬编码 PushContract.ts:13 影响"配置式多厂商"这个卖点