Skip to content

fix(mcp): 保留网关标记间的 Codex 信任配置 - #1376

Open
Ginufet wants to merge 1 commit into
deepcoldy:masterfrom
Ginufet:fix/codex-hook-trust-preservation
Open

Ginufet wants to merge 1 commit into
deepcoldy:masterfrom
Ginufet:fix/codex-hook-trust-preservation

Conversation

@Ginufet

@Ginufet Ginufet commented Sep 12, 2026

Copy link
Copy Markdown

背景

Codex CLI 审批 Hook 后,会将 hooks.state 写入用户级 TOML。实测新表可能插入 BotMux MCP Gateway 结束注释之前。旧逻辑更新 Gateway 时按注释起止位置删除整个块,误删信任记录,导致 botmux start 后原生 Codex 和 Bot 会话再次要求审批。

Closes #1375

改动

  • 将 Gateway 标记清理改为仅过滤起止标记行,不删除标记之间的全部文本。
  • 继续使用现有 stripCodexBotmuxSections 删除 BotMux 自己的 MCP 表和子表。
  • 安装与移除路径共用这一修复。
  • 新增 LF/CRLF 回归用例,覆盖两条 Hook 信任记录、用户 MCP、项目配置、旧 Gateway 子表清理,以及重复安装/移除的幂等性。

影响范围

  • 仅改变 Codex TOML Gateway 标记处理;Claude JSON 路径不变,现有 Claude 回归用例通过。
  • 不修改沙箱策略、Hook 审批机制、CLI 启动目录或用户凭据。
  • PTY/Tmux、不同会话类型使用这条共享配置更新路径时均受益;没有修改后端特有逻辑。
  • LF/CRLF 已覆盖;实际构建和部署平台为 Linux x64,未执行 macOS 运行验证。
  • 本次不改写 legacy plugin block 清理器,也不宣称实现了完整 TOML 解析器。

实际验证

构建工具 Bun 1.4.2,依赖通过 bun install --frozen-lockfile 安装。

bun run test -- test/plugin-mcp-gateway-installer.test.ts test/plugin-manifest-store.test.ts test/plugin-mcp-sandbox.test.ts test/bypass-codex-hook-trust-config.test.ts
bun run build
BOTMUX_VERIFY_BAKED_VERSION=3.21.0-hooktrust.89eb378c bun run build:bun --target bun-linux-x64
node scripts/smoke-bun-binary.mjs dist-bin/botmux-linux-x64
git diff --check
  • 相关测试:37 passed、3 skipped;3 项真实沙箱集成测试因宿主条件不满足而跳过。未执行全量测试。
  • 完整构建、类型检查、构建资产审计通过。
  • 二进制隔离 smoke 全部通过,包括 CLI 图加载、版本、配置资源、插件服务、supervisor、Dashboard HTTP/前端和二进制完整性。
  • 实机部署:用构建产物替换本地单文件安装,并以安装路径执行 botmux restart;确认新安装的 SHA-256 与构建产物一致,daemon 和 Dashboard 均 online。
  • 重启后 hooks.state 的原有信任哈希保留,且位于 Gateway 标记块外。此检查没有关闭 Hook 审批,也没有重新授予信任。
  • 尚未声称验证了重启后所有飞书业务流程,或用户再次新开 Codex 的最终 UI 结果。

效果

当 Codex 在 Gateway 注释块中插入其他表时,下一次 Gateway 更新或移除不再把这些表一并删除。已丢失的信任记录不会凭空恢复,需要用户正常审批;仍存在的记录会被保留。

@Ginufet
Ginufet requested a review from deepcoldy as a code owner September 12, 2026 13:24
@deepcoldy

Copy link
Copy Markdown
Owner

你好 @Ginufet,这个 PR 的自动评审群已建好:https://applink.feishu.cn/client/chat/open?openChatId=oc_9b42a7f1c0598472f32e260319b56bb7

不过你目前还不在自动拉群名单里,暂时没法把你拉进评审群。麻烦把你的 GitHub 账号和飞书信息补进这个名单文档:https://bytedance.larkoffice.com/wiki/WJ1nwWbtxi89erkNGNbcgkt9nUe ,补好之后后续复审会自动把你拉进群。

这是自动流程发的消息,评审意见稍后会在群内同步,最终以维护者审阅为准。

@deepcoldy

Copy link
Copy Markdown
Owner

感谢这个修复,先说结论:问题定位准确、方向我认为是对的——"注释标记不是所有权边界"这个判断抓住了 #1375 的本质,hooks.state 被整块删掉确实是 stripCommentBlock 按区间删造成的。新增的 LF/CRLF 回归用例我也验证过是有效的:把 gateway-installer.ts 换回 master 的版本后,这两条用例精准转红,原有 4 条保持绿,说明它们真的咬住了这个 bug。

下面是一条建议在合入前处理的问题,以及两条可选项。

建议合入前修复:标记之后的内容会被误删

去掉围栏之后,"哪些内容属于 botmux" 就完全依赖 stripCodexBotmuxSections 按表名判断了。但这个函数识别表头用的是:

const section = line.match(/^\s*\[([^\]]+)]\s*(?:#.*)?$/);

[^\]]+ 遇到第一个 ] 就停,所以下面三类行不会被识别成表头skip 标志因此不复位,会继续删除后面的内容,直到遇到下一个能识别的表头(若其后没有,则一直删到文件末尾):

能识别吗
[hooks.state."/x/hooks.json:stop:0:0"]
[[skills.config]](array-of-tables)
[hooks.state."/tmp/we]ird:session_start:0:0"](键里含 ]
# 用户自己写的注释 ❌(会被当成 botmux 表的一部分删掉)

同一份输入在本 PR 与 master 上的对照(ensure 与 remove 两条路都复现):

场景(内容位于结束标记之后 master 本 PR
[[skills.config]] 保留 被删
键里含 ][hooks.state."…"] 保留 被删
标记后的用户注释 保留 被删
[projects."/tmp/work"]、普通 [hooks.state."…"] 保留 保留 ✅
#1375 原始场景(信任记录在标记之间 丢失 已修复

需要说明的是,常见形态是安全的:标记之后如果是 [projects."..."] 或键里不含 ] 的普通 [hooks.state."..."],都能让 skip 正常复位、内容完好。真正受影响的是两类:array-of-tables(例如在 Codex TUI 里禁用 skill 后追加的 [[skills.config]],症状是设置被静默重置),以及键里含 ] 的表头(较少见,需要路径本身含 ],症状与 #1375 相同)。

补充一点定性:这个 skip 过冲本身是 stripCodexBotmuxSections 既有的潜在缺陷,并非本 PR 写出来的——在 master 上它被"整块删除"的旧路径遮住了,很难触发。本 PR 把删除职责完全交给这个函数,等于让它从"被遮住"变成"常态可达"。所以这里不是推倒重来,而是建议顺带把根因一并修掉:本 PR 的前提正是"放弃标记区间、改用表名精准删",而承担这个职责的函数目前还不够健壮,前提就不牢靠。

验证过可行的修法:把表头识别换成括号感知的解析——引号内的 ] 不作为表头结束、[[...]] 识别为表头且永远不属于 botmux、注释与空行归属后一张表而不是前一张。实测 tsc --noEmit 通过、本 PR 自带的 6 条用例全绿(包括 #1375 那条,修复效果保持),上表中被误删的场景全部恢复保留。建议同时补一条覆盖"标记之后有 [[...]] / 含 ] 键"的回归用例。

两条可选项(不影响合入判断)

  1. isBotmuxMcpSection 不认单引号形式gateway-installer.ts:72)。正则只匹配裸 botmux"botmux",不匹配 'botmux'(TOML 的字面量字符串键)。后果比"残留"更麻烦一些:如果用户配置里存在手写的 [mcp_servers.'botmux'],它不会被清理,安装后文件里会同时出现 [mcp_servers.'botmux'][mcp_servers.botmux]——在 TOML 语义里这是同一张表重复定义,产物无法被解析(bunCannot redefine table 'botmux',Python tomllibCannot declare ('mcp_servers','botmux') twice)。这一条在 master 上表现完全相同,是既有问题、不是本 PR 引入的,所以不作为合入阻塞,但如果顺手,在正则里加上 'botmux' 成本很低。

  2. stripCodexNamedMcpSectionsgateway-installer.ts:91)有同样的表头过冲问题。它只在存在 materialized codex 名字时才执行(legacy 迁移路径),优先级较低,记为后续处理也可以。

另外一个纯命名问题:stripCommentBlock 现在是逐行过滤,start / end 两个参数已经没有"区间"语义了,叫 dropMarkerLines 之类可能更贴合实际行为。

我这边跑过的验证

  • 已在最新 origin/master(857c72492) 上 rebase(仅本地操作,未推送、未改动你的提交),零冲突,diff 与原 patch 逐字一致
  • tsc --noEmit 通过;bun run build 通过
  • 你在描述里列的 4 个测试文件:37 passed / 3 skipped,与你的结果一致
  • 反变异验证:源码还原为 master 后,新增的 2 条用例精准转红
  • 更大范围回归:*plugin* / *mcp* / *codex* 共 80 个文件,1075 passed / 9 failed;这 9 条在干净的 master 上同样失败且形态一致plugin-registry-sandbox-readworker-codex-app-turn-routing,属于既有的 bwrap / tmux 环境噪声),本 PR 无回归

以上是自动评审的初步意见,可能有理解偏差或遗漏,仅供参考,最终以维护者审阅结论为准。这个 PR 解决的是一个会实际影响用户(包括直接使用原生 Codex 的用户)的问题,很有价值,辛苦了 🙏

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.

fix(mcp): Gateway 配置更新误删 Codex Hook 信任记录,导致重复审批

2 participants