Repository navigation
fix(auth): 隔离 MCP 请求上下文并按当前调用核验授权 - #7068
Conversation
Signed-off-by: ficowang <fico@xd.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
[Medium impact] 本 PR 看起来可以合并,未发现由本次改动引入的实际问题。
|
MagicLizi
left a comment
There was a problem hiding this comment.
PR #7068 独立代码审查报告(standard 档)
- Head:
c0025a2caf877d4e648c6e6d7372e416517123a2(1 个 commit) - Base:
4af715d21fbe5a031f6d4d37310a784327527967(main) - diffHash:
ba6f3b447262b3fc8bb9638f03bb7f0fa8c60dc5963a0af6e301d43ced1f4abf - 改动面:
apps/desktop/src/main/maker-ipc/(taskActionAuthorization.ts新增请求级隔离原语、taskActionHost.ts用其包裹统一工具授权入口)、apps/desktop/src/main/mcp-integrations/codexHttpBridge.ts(HTTP 请求入口隔离)、两个既有测试文件补回归、docs/product-rules/agent-user-parity.md补 2026-10-24 修订说明。
Findings
没有 P0/P1。逐项核验结论:
taskActionAuthorization.ts:199-218新增runTaskActionRequest(actionContext.exit(operation))与isolateTaskActionAuthorizer:授权判定在无环境证据的上下文中执行,decision.run也在exit内执行、由admitTaskAction自己的actionContext.run建立本次调用的 admission。实测 NodeAsyncLocalStorage.exit语义(独立小脚本):exit 回调内 store 为undefined、exit 结束后外层 store 恢复、exit 内嵌套run()能正常建立新 store——三点分别对应「裸放行不继承」「不破坏外层 admission」「admitted 调用内部证据照常可用」。对ok:false决定原样透传,withToolCallAuthority的错误包装语义不变。taskActionHost.ts:216,473-476:统一授权入口整体包进isolateTaskActionAuthorizer,逐次判定与执行互不串证据。核对入口语义未变:非保护工具与TEAMMATE_STANDING_GRANT_TOOLS/TEAMMATE_SCHEDULER_WRITE_TOOLS裸放行(routine_*/schedule_*管理类,不排输入队列)不再挂旧 evidence;capture/confirm/admitTaskAction的一次性使用、指纹校验、assertCurrent/assertDeliveryCurrent/assertCurrentSync复查链全部保留。当前调用内部的证据读取(bindTaskActionInput、captureTaskActionDeliveryGuard、botDelegationService.ts:2497的approvedTaskDelegation委托快照)都在decision.run建立的自身上下文内执行,行为不变——新增测试也锁了「内层隔离不清除外层 admission」。codexHttpBridge.ts:241-243,423:整个 HTTP 请求回调包进runTaskActionRequest,惰性启动桥接的行动上下文不再粘到后续请求(PR 描述的TASK_ACTION_AUTHORIZATION_EXPIRED误报根因);只清行动授权这一个AsyncLocalStorage,MCP 路由上下文(session-context.ts的withLiziMcpSessionContext,另一实例)不受影响——回归测试在请求内读getLiziMcpSessionContext()?.sessionId并按 thread 路由成功即为实证。请求内 try/catch 错误收口、403 localhost 防御、token 鉴权、SSE 长连接设置均未改动。- 行为收紧而非放松:裸放行不再能读到他人 admission(
hasTaskActionAuthorization/approvedTaskDelegation/taskActionPrincipal在无 store 时返回空),当前调用的拒绝(NEEDS_OWNER_REQUEST/TASK_ACTION_AUTHORIZATION_REQUIRED)、撤销后过期(TASK_ACTION_AUTHORIZATION_EXPIRED)、一次性消费与延迟交付检查全部保持;无权限档改动、无自动重发、无新增依赖、无网络/落盘新增。安全扫描 0 hard/0 soft 命中(security.scanned=true)。 - 测试:
codexHttpBridge.test.ts新回归经真实 loopback HTTP + 真实 SDK 复现「admitted 内惰性建桥 → 撤销创建者 → 新请求仍按当前调用判定」的最小失败输入,覆盖当前拒绝、当前 admission、当前撤销三态;taskActionAuthorization.test.ts新回归覆盖裸放行不继承/不强制确认、并发 admitted 调用互不串权、外层有效授权不能借给被拒的当前调用。两个测试均先红后绿成立(旧实现下第一组断言会因继承旧 store 抛revoked/hasTaskActionAuthorization为真而失败)。影响面测试(sendToSession*、sessionHandoffIpc/Remote、sessionControlService、remoteAuthorization)共 247 项复跑全绿。 - 共享符号读方检查:
withToolCallAuthority的全部生产调用点(lizi_xdtHelperMcpServer.ts:346,1105、cindy_schedulerMcpServer.ts:174、orca/server.ts:403)均为单次工具入口,不存在「一次 admitted 操作内再走一次授权」的嵌套依赖;approvedTaskDelegation/bindTaskActionInput/captureTaskAction*的读方都在调用操作内部或显式捕获,与隔离边界一致。mcp-integrations → maker-ipc的 import 方向与既有mcp-providers.ts一致,main 进程仍是顶层静态 import。
Rule coverage — 读取的规则文件及逐项结论
AGENTS.md、docs/dev-rules/development-workflow.md、.github/PULL_REQUEST_TEMPLATE.md:三大段齐全,变更类型 fix、风险分类(权限/安全 + 跨平台)与 diff 一致;「怎么验证的」的命令与结果与本次复跑一致,未执行项(整仓 CI、Windows 实机、完整端到端)如实声明;无凭证/红线路径;DCO 单 commit 已签。docs/dev-rules/electron-security-and-process-boundaries.md§8 Review 清单:1) 无 Renderer 特权变化;2/3) 不涉及窗口/preload;4) MCP 请求入口鉴权与 per-tool 授权链保持且新增隔离(fail closed 方向:少了继承证据只会更严);5) 不涉及导航/协议;6) 无 CSP/Fuses 放宽;7) 新增安全边界配了能阻止回退的自动化回归(HTTP + 进程内各一组)。通过。docs/dev-rules/architecture-invariants.mdReview 清单:1) 无新增 package 反向依赖;2) 无运行时动态import();3/4) 未触碰布局树。通过。docs/dev-rules/credentials-and-local-storage.md:授权/委托证据只存在于进程内AsyncLocalStorage与既有 DB 快照,无凭证、落盘或用户数据路径改动。通过。docs/dev-rules/engineering-conventions.md/docs/dev-rules/desktop-development.md:main 进程逻辑改动带真实入口回归与错误路径检查,测试无外网(loopback 临时端口)、无固定端口、无共享临时目录;无 UI/i18n。通过。docs/product-rules/agent-user-parity.md(本 PR 同步的权威文档):新增 2026-10-24 修订与既有「只认运行时任务的实时权限、不读数据库旧值、放行失效」原则一致,明确保留委托/撤销/延迟交付检查、不恢复过期授权、不自动重发,无权限放宽。- 维护者确认门(rules/arch 触发)语义 fallback 逐文件判读:
taskActionAuthorization.ts/taskActionHost.ts/codexHttpBridge.ts及两处测试均为缺陷修复与回归(bugfix);docs/product-rules/agent-user-parity.md的 7 行为该 bugfix 的行为说明同步(沿用该文档既有「YYYY-MM-DD 修订」记录体例),只把修复后的语义写清,不改判据适用范围、不新增裁决、不放松任何检查,判为「不改变规则含义」,rules/arch 两类按语义 fallback 放行(不创建 hold、按既有 release 脚本清理标签),fallback.action=review。
Verification — 实际运行的命令、结果、未执行项及原因
均在本 PR worktree(HEAD=任务 head)执行,完整日志落盘,保留命令自身退出码:
| 命令 | 退出码 | 结果 |
|---|---|---|
pnpm install --prefer-offline |
0 | 依赖就绪(12.9s) |
pnpm --filter desktop exec vitest run(9 个 PR 声明的测试文件 --maxWorkers=2) |
0 | 9 文件 / 247 项全部通过(taskActionAuthorization 92、codexHttpBridge 26、sessionControlService 22、sendToSessionLazyResumePermission 28、sendToSessionExecutionConfig 18、sendToSessionLock 14、remoteAuthorization 14、sessionHandoffIpc 15、sessionHandoffRemote 18) |
pnpm --filter desktop run --if-present typecheck |
0 | tsc --noEmit 通过 |
node scripts/check-test-hygiene.mjs |
0 | PASS(361 files within baseline) |
node --test scripts/__tests__/dev-docs-contract.test.mjs |
0 | 通过 |
node scripts/check-dco.mjs |
0 | 1 commit signed off(range 4af715d2..c0025a2c) |
git diff --check |
0 | 无空白错误 |
Node AsyncLocalStorage.exit 语义小脚本(独立、不运行 PR 代码) |
0 | exit 内 store 清空、嵌套 run() 生效、exit 后恢复 |
未执行项:整仓 pnpm test:unit / test:all、Windows 实机、完整 Desktop 端到端(隔离登录/跨电脑)未复跑,属 PR 已如实声明的未执行验证;本改动为 Node AsyncLocalStorage 平台无关实现,风险由上述定向测试 + 双源全绿 CI(gate.ci.status=pass:client-ci、pr-design-basis)覆盖。security-scan.mjs 为 heavy 档专用,本 PR 为 standard 档未跑,凭证/隐私门由 security.scanned=true、0 命中覆盖。
Intent alignment
- 描述来源:PR body(2026-10-24 当前版本,单 commit 无编辑改写痕迹需要对照的多轮修复);原目标:修复「共享 MCP HTTP 桥接在任务行动调用内惰性启动 → 后续普通调用误读启动者旧授权 → 合法调用误报
TASK_ACTION_AUTHORIZATION_EXPIRED」的跨任务投递缺陷,边界为「只做 HTTP 请求隔离 + 进程内工具授权隔离 + 回归测试 + 同权规则说明;不改权限档、不放宽授权、不自动重发、不做用户数据迁移与热补丁」。 - 初始实现 SHA/范围:本 PR 仅 1 个 commit
c0025a2caf877d4e648c6e6d7372e416517123a2(父即 base4af715d21fbe5a031f6d4d37310a784327527967),初始实现即待合并实现,历史未被改写,累计 diff 与该 commit 一致(history.commits仅此一条,git log核对父链无多父/带入)。 - 最终行为对照表:
| 原目标/初始行为 | 最终行为与影响 | 代码/提交证据 | 变化依据或待确认问题 |
|---|---|---|---|
| 独立 MCP 请求不应继承惰性启动桥接者的行动授权 | HTTP 请求入口整体 runTaskActionRequest 隔离,后续请求按自身 token/调用方/per-tool 授权判定 |
c0025a2c apps/desktop/src/main/mcp-integrations/codexHttpBridge.ts:241-243,423 |
直接服务原目标,无偏离 |
| 进程内普通调用不应读到旧撤销检查/旧委托证据 | 统一授权器逐次隔离判定与执行,裸放行不继承、不额外确认、不强制排队 | c0025a2c apps/desktop/src/main/maker-ipc/taskActionAuthorization.ts:199-218、taskActionHost.ts:216 |
直接服务原目标;当前调用的委托/撤销/延迟交付检查保留,无放宽 |
| 明确不包含:权限档/授权放宽/自动重发/迁移/热补丁 | 最终 diff 无权限档、无授权放宽、无重发、无 schema/协议/持久化变化 | 6 文件全量 diff 核对 | 一致 |
- 多轮意见核对:无 review thread(
history.reviewThreads空)、无 Request Changes;Codex code/security review 因用量限额未完成(非 finding),Greptile 结论为「未发现由本次改动引入的实际问题」,均无待落实要求;signoff.discussionIssue为空,无讨论 issue 修改要求需要逐条核对。结论:一致,无偏离,无待裁定事项。
Overall
pass —— 没有 P0/P1。
备注(不阻断):维护者确认门的 rules/arch 触发按语义 fallback 放行(逐条理由见 Rule coverage 末条),未创建确认 issue;touches:core / touches:rules 展示标签按 context desired 同步;Codex 安全审查因账号用量限额未产出结论,其意见可能在合并后到达,由既有 post-merge 迟到意见清扫流程处理。
这次改了什么
摘要
共享 MCP HTTP 桥接在任务行动调用内懒启动时,Node 异步上下文可能带上启动者的授权。启动者的输入或权限失效后,后续普通任务虽然通过自己的权限判定,发送仍会读到旧授权并报
TASK_ACTION_AUTHORIZATION_EXPIRED。本次在独立 HTTP 请求入口隔离行动上下文;Desktop 工具授权器也隔离每次判定与执行,只使用当前调用返回的 admission。普通任务的裸放行不继承其它调用的委托证据,不额外确认或强制排队。当前调用的拒绝、撤销、一次性使用及延迟交付检查保持有效。
变更类型
fix缺陷修复范围
UI 变化
不涉及。引用的设计规范:不涉及。
怎么验证的
自动验证
taskActionAuthorization、codexHttpBridge、sendToSessionExecutionConfig、sendToSessionLock、sendToSessionLazyResumePermission、sessionHandoffIpc、sessionControlService、sessionHandoffRemote、remoteAuthorization(pnpm --filter desktop exec vitest run … --maxWorkers=2,9.12s)。pnpm --filter desktop run --if-present typecheck:通过。node scripts/check-test-hygiene.mjs:通过。node --test scripts/__tests__/dev-docs-contract.test.mjs:10 项通过。git diff --check、node scripts/check-dco.mjs:通过。测试价值:
codexHttpBridge.test.ts:经生产桥接、真实 SDK 和临时 loopback HTTP 端口,验证创建者过期后普通调用成功、当前拒绝仍生效、当前 admission 可用且撤销仍拒绝。旧覆盖均在空行动上下文中启动桥接;最小失败输入是“授权内启动桥接 → 撤销创建者 → 新调用”。unit tier,复用现有 HTTP fixture 和 shutdown。单独运行 Vitest 5.11s,命令墙钟 5.54s;收集 4.79s,测试 111ms,略超 5s 目标来自既有大模块导入,未新建测试文件或真实等待。taskActionAuthorization.test.ts:生产 Desktop 授权器和行动守卫验证进程内普通调用不继承旧授权、外层撤销仍有效、并发任务互不串权,以及当前调用拒绝不能借用外层有效授权。旧覆盖仅从顶层调用授权器;最小输入是撤销后的外层调用里进入新普通调用。unit tier,内存 Host facts。单独运行 Vitest 5.53s,墙钟 5.95s;收集 5.29s,测试 27ms,沿用现有测试文件。手工验证
macOS arm64 本地源码测试;未操作真实任务、设备或真实凭证作为测试 fixture。
未执行的验证
未跑整仓 CI、Windows 实机及完整隔离 Desktop 登录/跨电脑端到端。正在运行的安装版 0.1.115 尚未加载本提交;应用更新查询返回 no_installable_update,不能把源码测试视为现场恢复。
风险
风险分类
影响与回滚
提交前检查