Skip to content

fix(event-dispatcher): 权限可观测性——无权限不再静默、解析失败可观测、跨 app ou_ 校验、改名防抖 - #1006

Open
ITcathyh wants to merge 4 commits into
deepcoldy:masterfrom
ITcathyh:sprint-permission
Open

fix(event-dispatcher): 权限可观测性——无权限不再静默、解析失败可观测、跨 app ou_ 校验、改名防抖#1006
ITcathyh wants to merge 4 commits into
deepcoldy:masterfrom
ITcathyh:sprint-permission

Conversation

@ITcathyh

Copy link
Copy Markdown
Contributor

PR 2: sprint-permission(权限可观测性 4 项)

改了什么

来自「Botmux 交流群」近 7 天用户反馈的权限专项,4 个修复:

  1. fix(event-dispatcher): 无权限消息不再静默丢弃

    • 私聊(p2p)无权限消息此前硬静默丢弃,用户毫无感知(本周 4+ 人反馈「机器人不理我」)。现在:p2p 被挡时给发送者发中性提示(不泄露 owner 信息)+ owner DM(24h 节流);群聊授权卡被节流/禁用时给发送者简短提示(10min 节流)。
  2. fix(allowed-users): 确定性解析失败可观测

    • allowedUsers 中邮箱/手机号确定性解析失败(不存在/不可见)此前完全静默(连 WARN 日志都没有),配置「看起来正确」但功能失效。现在 resolveAllowedUsersWithMap 返回 definitiveMisses,启动时 DM owner 列出无法解析的条目,setBotAllowedUsers 返回 warnings(不拒绝写入,可能是临时不可见)。
  3. fix(allowed-users): 运行时写入校验跨 app ou_

    • setBotAllowedUsers 写入前用 detectUnusableOwnerEntries 校验 ou_/on_ 条目,跨 app ou_(app-scoped,对目标 Bot 无效)确定性不可用时拒绝写入并返回明确错误;inconclusive(网络/scope 错误)不拒绝(fail-closed 原则保持)。启动期巡检跨 app ou_ 并 DM owner 警告。
  4. fix(dashboard-ipc): chat-rename user_explicit 豁免防抖须持 turn origin 凭证

    • Agent 未经要求自动改群名(本周用户反馈「干着活突然改群名」):chat rename 缺省 proactive=false 被记为 user_explicit 完全跳过 10 分钟防抖。翻转信任模型:声明 user_explicit 须持有效 turn origin 凭证,否则强制 proactive 防抖。

影响面

  • fail-closed 原则保持:只有目标 app 明确判定不可用才拒绝写入;网络/scope/无 secret 一律 inconclusive 放行
  • 跨 CLI:改动在 im/lark 和 services 层,不碰适配器/worker/backend,20+ CLI 零影响
  • 跨会话类型:覆盖 p2p、群聊、bot-to-bot 路径
  • 安全红线:未触碰 BOTMUX_OWNER_OPEN_ID 注入冻结语义;p2p 提示文案不含 owner 任何身份信息
  • 瞬态路径:errored/transient 分类、缓存兜底、重试调度一行未动

测试验证

  • pnpm build 通过
  • 13 个目标测试文件 473/473 全绿(含新增 allowed-users-runtime-guard、grant-pending 节流、event-dispatcher 通知、ipc-chat-rename-route 凭证)
  • pnpm test 全量:18173 passed;3 个失败(bwrap/dsh 沙箱环境问题)已在干净基线复现确认预存

huangyuhang.edu and others added 4 commits August 25, 2026 19:26
p2p 私聊被挡时硬静默丢弃(注释自认待修);群聊授权申请卡被节流/
禁用/无 owner 抑制时发送者也无感知。本次修复:

- p2p 被挡:给发送者发通用无权限提示(per-(bot,sender) 10min 节流,
  文案中性不泄露 owner 身份),同时把授权申请发到 owner DM
  (per-(bot,owner) 24h 节流,复用 dmAdmin + getOwnerOpenId)
- 群聊节流窗口内:授权卡被抑制时给发送者发「已提交/冷却中」提示
  (per-(bot,chat,sender) 10min 节流)
- autoGrantRequestCards=false:发通用提示但不发授权卡
- maybeSendGrantRequestCard 返回判别结果(sent/throttled/disabled/
  no_owner/no_requester),3 处调用点据此补发提示
- grant-pending 新增通用轻量节流 shouldThrottleNotice(独立表 +
  定期 prune,不会无限增长)

所有提示/DM 均 fire-and-forget,失败只 debug 日志,不影响消息主流程。

Co-Authored-By: Claude <noreply@anthropic.com>
B-2 确定性解析失败静默:email/mobile code-0 批量响应缺失条目只标
definitive 丢弃,连 WARN 都没有;启动路径只在瞬态 failed 时通知。
- resolveAllowedUsersWithMap 返回值加 definitiveMisses(从 entryStatus
  派生,ou_ 字面量恒为 resolved 不出现,transient 不列入)
- email/mobile 确定性 miss 处补 WARN 日志
- 启动路径:definitiveMisses 非空时 DM owner 列出无法解析的条目
  (不拒绝启动,可能是临时不可见)
- setBotAllowedUsers:definitiveMisses 非空时返回 warnings 但仍写入

B-3 运行时写入不校验跨 app ou_:Dashboard onboarding 和 scripted
setup 已用 detectUnusableOwnerEntries,但运行时 setBotAllowedUsers
无校验,粘贴别的 Bot 的 ou_ 被接受后静默失效。
- setBotAllowedUsers 写入前用本 bot 凭证跑 detectUnusableOwnerEntries,
  确定性不可用(99992361/41012/40001/code-0 无 user)拒绝写入并返回
  unusable_owner_entries + entries;inconclusive(网络/scope/无 secret)
  不拒绝,fail-closed 仍由 resolver 瞬态路径兜底
- 启动路径:detectUnusableOwnerEntries 巡检 ou_/on_ 条目,跨 app ou_
  DM owner 警告(不拒绝启动,fire-and-forget 不阻塞启动)

瞬态路径(缓存兜底/重试/DM)一行未动。

Co-Authored-By: Claude <noreply@anthropic.com>
proactive 标志由 CLI 自报,缺省 false → trigger=user_explicit 完全跳过
10 分钟防抖。Agent 误判「用户要求改名」时省略 --proactive,服务端无核验。

翻转信任模型:
- 声明 proactive=false(user_explicit)时,要求请求携带与
  ds.managedTurnOrigin 匹配的当前 turn origin 凭证
  (authorizeSessionScopedIpc,固定 trustedHost=false——本机 HMAC
  签名只证明「本机进程」,不绑定具体用户 turn)
- 无凭证或不匹配 → 强制 proactive=true 走防抖(fail-closed:
  宁防抖勿绕过)
- 有有效 turn origin → user_explicit 不防抖(保留合法路径)
- 显式 --proactive 仍正常走防抖,行为不变

Co-Authored-By: Claude <noreply@anthropic.com>
contact API 挂起时不得阻塞消息分发关键路径——超时后按无名处理,
与 daemon.ts notifyAllowedUsersResolveFailure 的 untimed SDK 纪律一致。

Co-Authored-By: Claude <noreply@anthropic.com>
@ITcathyh
ITcathyh requested a review from deepcoldy as a code owner August 25, 2026 12:21

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcfec65987

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1741 to +1745
if (!effectiveProactive) {
const originDecision = proveCurrentTurnOrigin(ds, params.sessionId, body);
if (!originDecision.ok) {
effectiveProactive = true;
trigger = 'ai_proactive';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Attach the turn proof on trusted-host rename requests

For normal non-relayed CLI sessions, postSessionCliIpc loads the host secret and only reads originCapability when that secret is absent (src/cli.ts:6722-6737). Consequently, an ordinary botmux chat rename issued during a valid user turn reaches this branch without a claim and is reclassified as proactive; after one rename, a second user-requested correction within ten minutes receives rate_limited, despite the skill promising direct execution for explicit requests. The CLI needs to attach the available turn proof even when it also authenticates with HMAC.

Useful? React with 👍 / 👎.

Comment on lines +1710 to +1714
sessionId,
liveOrigin: ds?.managedTurnOrigin,
claimedCapability: typeof body?.originCapability === 'string' ? body.originCapability : undefined,
claimedTurnId: typeof body?.originTurnId === 'string' ? body.originTurnId : undefined,
claimedDispatchAttempt: claimedAttempt,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not treat a turn capability as proof of explicit rename intent

In read-isolated sessions, every action performed during an active user-triggered turn can present this same managed-origin capability, including an AI-initiated rename that should have used --proactive (the rename skill explicitly permits such proactive decisions). Therefore an agent that omits the flag still passes this check and bypasses the cooldown—the credential proves which current turn/process made the call, not that the user explicitly requested a rename. The exemption needs an intent-specific signal, or it should not be granted solely from the generic turn capability.

Useful? React with 👍 / 👎.

Comment on lines +416 to +418
return warnings.length > 0
? { ok: true, raw: rawEntries, resolved, warnings }
: { ok: true, raw: rawEntries, resolved };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface allowed-user diagnostics through the command handler

When /config allow includes a valid owner plus an unresolvable email or phone, this function returns success with warnings, but its sole production caller in src/core/command-handler.ts:997-1004 always replies with the generic success count and discards those warnings. That leaves the operator unaware that an entry was persisted but did not take effect; the new entries detail on unusable_owner_entries is similarly reduced to a generic reason. Propagate these result details into the command reply so the runtime mutation path actually gains the intended observability.

Useful? React with 👍 / 👎.

@ITcathyh

Copy link
Copy Markdown
Contributor Author

CI 失败的 test/mojo-close-failclosed.test.ts 是 flaky 测试:期望 mojo_local_child_termination_unproven 实际得到 mojo_local_termination_unscannable,是进程终止检测的时序竞态。本地(同 commit)跑 7/7 通过,且本 PR 只改权限相关文件(event-dispatcher/grant-pending/client/allowed-users-apply/bot-config-store/daemon/dashboard-ipc-server),与 mojo 完全无关。master 上 0517705 的 CI 也是成功的。麻烦 maintainer 重跑一下 CI。

@deepcoldy

Copy link
Copy Markdown
Owner

感谢这个 PR,权限可观测性的四个方向都切中了真实痛点,B-1/B-2/B-3 的核心机制、owner 身份边界与 fail-closed 处理经核对都没有问题。以下是一条自动评审的初步意见,仅供参考,最终以维护者审阅为准;提出一个建议合入前处理的问题和几个次要建议。

建议合入前处理

1.(较重要)改名信任翻转对「默认非隔离部署」构成回归

proveCurrentTurnOrigin 固定 trustedHost: false 并要求请求携带与 managedTurnOrigin 匹配的 originCapability。但这个 capability 只在会话具备 origin-channel 传输时才存在:

  • willReadIsolate 仅在 process.platform === 'darwin' && sandboxRequested 时为真(worker.ts),Linux 生产部署恒为 false;BOTMUX_ORIGIN_CHANNEL_ID 也只在 read-isolation / 设备凭证隔离(opt-in)时才由 worker 注入。
  • 因此默认部署(无 sandbox、无设备凭证)的普通 PTY 会话postSessionCliIpc 能读到 host secret → 走 HMAC 路 → body 不带 originCapabilityproveCurrentTurnOrigin 必然失败 → 被强制按 ai_proactive 走 10 分钟防抖。

后果:master 上 user_explicit(未带 --proactive)完全不受防抖,而本 PR 后,默认部署下真人显式改名在 10 分钟内的第二次会收到 429。这是一个行为回归(首次改名不受影响,属有界)。

补充一点:新增的 forces proactive cooldown on a user_explicit claim without a turn-origin credential 用例确实走了真实的 trusted-host/HMAC 路径并断言被强制防抖——也就是说它把上面的回归行为编码成了「预期」;而豁免路径只在手工注入 capability 的用例里覆盖,于是整套测试全绿,但默认路径下"真人显式改名应豁免防抖"这一场景并没有被覆盖到。

建议修法:把信任翻转的前提改为「会话确实具备 origin-capability 传输」——即 managedTurnOrigin 有 capability 且会话有 channel 时才做凭证核验;没有该传输通道的默认非隔离会话回退到原先的 flag 信任模型。这样隔离会话仍能达成本 PR 目标,默认部署不回归。

同一个门校验的是「传输/当前轮」而非「用户意图」,所以隔离会话里若 AI 自主改名且漏传 --proactivepostSessionCliIpc 在无 host secret 时仍会无条件附上当前轮 capability → 豁免照样成立。也就是说 commit message 里想堵的「漏传 --proactive 绕过防抖」在隔离路径上并没有真正堵住,建议一并考虑。

次要建议(可同 PR 处理)

  1. /botconfig set allowedUsers 的成功分支只回了 count/totalcommand-handler.ts),setBotAllowedUsers 新返回的 warnings 被丢弃。这样 B-2 想要的「确定性解析失败可见」在这个主要用户入口其实没达成——被丢弃的条目只在服务端日志里能看到。建议把 warnings 拼进回复。

  2. 新的失败原因 unusable_owner_entries 没有单独处理,会落到通用的 write_failed 分支,直接把英文枚举名渲染进 ⚠️ 写入失败:{reason}(无 i18n),且没有带出 entries 列表。建议加一条本地化文案并列出被拒条目。

  3. on_(union_id)与 ou_ 一起走硬拒、而 email/mobile 只做告警,这个不对称可能值得再斟酌:ou_ 是字面透传(永不丢弃)才有静默锁死风险;而 on_ 解析失败本身会被 resolver 丢弃、并不会锁死任何人。把 on_ou_ 同组硬拒,可能误伤「同租户但本应用通讯录不可见」的合法 on_ 协作者(返回 code-0 无 user → 整条拒写),而这正是推荐使用的跨应用稳定形。建议要么把 on_ 降为告警(与 email 同口径),要么硬拒只限 ou_

关于 CI

当前 CI 失败在 mojo-close-failclosed.test.tsrefuses the close when the local child cannot be proven dead),本 PR 并未改动任何 mojo 相关文件,该用例在本地可稳定通过(7/7)。它是一个对进程扫描 liveness 敏感、易受 CI 并发影响的用例,建议重跑一次 CI 确认是否为偶发。


再次感谢贡献。以上是自动评审的初步意见,可能有误,最终以维护者审阅为准

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants