Feat/mcp registry - #170
Conversation
491f0d4 to
73b3ee7
Compare
cfcf60a to
4ce0416
Compare
yujiezhang-ops
left a comment
There was a problem hiding this comment.
P0-1 移除守卫的字符串对不上,永远不生效
internal/mcp/adapter.go:126
if spec == nil && strings.Contains(err.Error(), "does not exist") {
hujson 实际返回的是 value not found。我单独建了个最小工程验证:
err = hujson: patch operation 0: value not found
contains "does not exist"? false
所以"移除一个本来不存在的服务"不是 no-op,而是报错。写测试证实:
BUG CONFIRMED: patch MCP server "absent": hujson: patch operation 0: value not found
空配置 {} 同样触发。这条在 ApplyMCP 里可达:mcpTargetAgents(app/mcp.go:240)取"已关联 agent ∪ 本次选中 agent"的并集,然后对未选中的 agent 置 spec = nil 去移除。当一个服务已经写进 agent A,用户改为只勾选 B 时,对 A 之外那些从未配置过的 agent 就会走到这个分支。
后果分两层:用户看到本不需要改动的 agent 报 Cannot apply MCP configuration;更麻烦的是若 change.Delete 为真,deleteFailed 被置位(mcp.go:434),注册表条目就不会被删除(mcp.go:486),注册表和磁盘从此不一致。
好消息是不会写出截断文件 —— Apply 在 v.Pack() 之前返回,out 为空且错误已短路 AtomicWrite。
TOML 和 YAML 适配器用的是 delete(),本来就是安全的 no-op,只有 JSON 这条路受影响。
P0-2 Codex TOML 丢注释、改写引号、丢未管理的键
internal/mcp/adapter.go:193-221 把整个文件 unmarshal 成 map[string]any 再 marshal 回去。经过 Go map 一轮,注释、键序、原始引号全部不可能保留。
实测输入输出:
输入
Managed by hand -- do not reorder
model = "gpt-5"
[mcp_servers.mine]
command = "x"
startup_timeout_sec = 45
输出
model = 'gpt-5' ← 注释没了,双引号变单引号
[mcp_servers]
[mcp_servers.mine]
...
这个仓库已经专门解决过同一个文件的同一个问题。 internal/config/write.go:521 的 mergeManagedTOML 是逐行实现的,正是为了让未管理的行(含注释)逐字节透传,managedTOMLShape 明确声明哪些键和表属于自己。MCP 这条路绕开了那套机制,手写了第二个 TOML writer。
另外当同一个 ID 被重写时,该条目下未管理的键会整块丢失:
输入: [mcp_servers.managed] 带 startup_timeout_sec = 45, tool_timeout_sec = 90
输出: 只剩 command 和 type
LOST: startup_timeout_sec dropped
LOST: tool_timeout_sec dropped
Codex 认这些键,所以用户设的 45 秒启动超时会静默回到默认值。兄弟条目(未被管理的其他 server)是安全的。
Hermes 的 YAML 适配器(adapter.go:239-267)有同样的结构问题,同样地 write.go:386 的 WriteHermes 已经用 yaml.Node 就地修改来避免它。YAML 这条只丢格式不丢数据,算 P2。
P1 OpenCode 的 env 双向丢失,且与 hasSecrets 自相矛盾
internal/mcp/opencode.go:14-38 解码只认 type/command/url/headers,编码(:46)只输出 type/command/enabled。OpenCode 原生 local server 的环境变量在 environment 字段。
实测写入:
输入 spec: Env{GITHUB_TOKEN: ghp_x}
实际写出: {"command":["gh-mcp"],"enabled":true,"type":"local"}
hasSecrets 报告: true
两个后果叠在一起:token 没写进去,服务启动后因缺少凭证而失败,而 OneAgent 不报任何错;同时 hasSecrets 仍返回 true,于是文件按"含密钥"的权限写入,并告知用户密钥已处理 —— 而实际上没有。设计文档第 221 行把 environment 列为敏感路径,说明字段是知道的,只是适配器没映射。
P1 明文导出的"确认"是渲染层自证,后端无法校验
internal/mcp/transfer.go:62 靠 options.ConfirmPlaintext 拦截明文导出,但这个布尔值来自渲染层(binding/mcp.go:13 → TransferPage.tsx:145 里 keys === "plain" 时置 true)。
它看起来像安全联锁,实际是自我声明。任意 XSS 或恶意 bridge 调用直接 Export({mode:"plaintext", confirm_plaintext:true}),就能拿到所有 Authorization 头和 API key 明文,不弹任何对话框。考虑到这个仓库已有"明文 key 进 WebView"的既有问题(#150),这里是同类风险的扩大。
相关但较轻(P2):GetMCP(app/mcp.go:367)把未脱敏的 Spec.Env/Spec.Headers 直接返回给渲染层,MCPPage.tsx:112 再 JSON.stringify 塞进 textarea,于是活 token 进入 DOM。mcp.RedactSpec(model.go:135)已经存在,但只在 omit 模式导出时用。列表接口用 HasSecrets 布尔值的做法(app/mcp.go:50)是对的,编辑器可以照 provider 那套 keep_existing_key 的哨兵值模式来做。
为什么 CI 没抓到
adapter_test.go:37 的 TestStructuredAdaptersRoundTrip 覆盖了 Codex 和 Hermes,但断言只有 strings.Contains(out, "printf") —— 只验证新内容写进去了,从不验证旧内容是否还在。它的 Codex fixture 是 model = "x",不含任何注释,所以注释丢失在构造上就不可见。
TestOpenCodeUsesNativeMCPShape(:88)用的 spec 没有 Env,所以抓不到丢失。
唯一真正的保留性测试是 adapter_test.go:13,只覆盖 JSON。
确认无问题的部分
这些我逐条追过,是对的,不用改:
原子写:全部走 securefs.AtomicWrite,temp file + rename,写前有备份,writeMu 覆盖整个 apply。写不出截断文件。
幂等:同一 spec 应用两次输出字节相同。EqualNormalized(model.go:79)按排序 JSON 归一化后比较。
删除不碰未管理条目:每次删除都绑定具体 change.ID;ScanMCP 在读不到 agent 配置时恢复此前事实(mcp.go:307)而不是当成"没有 server",这一点很容易做错,做对了。
JSON 路径的兄弟条目保留:逐 ID 下 JSON Patch,没有整块替换 mcpServers。带注释和尾逗号的未管理条目实测存活。
Codex headers 映射正确:headers → http_headers,解码两种拼写都认。Normalized 会按传输类型清掉不兼容字段。
crypto 正确:每次加密都新生成 16 字节 salt 和 GCM nonce,长度校验在 Open 之前,解密失败只返回一个通用错误。攻击者篡改 payload 里的 Iterations 不可利用 —— 我实测改小会直接触发认证失败,因为 GCM tag 覆盖了用该迭代数派生的密钥。500000 上限实测 159ms,不构成 DoS;更重要的 10000 下限是存在的。
无日志泄露:internal/mcp/ 和 app/mcp.go 里没有任何日志调用,错误信息只插值 server ID,没有 %v 整个 struct。第三方解析器的报错我实测过也不回显内容。
手写 PBKDF2 我对三个 RFC 6070 向量验证过(c=1/2/4096)全部正确,不是 bug。但它比标准库慢 4.4 倍(63ms vs 14.5ms),而 go.mod 已声明 go 1.26.5,crypto/pbkdf2 从 Go 1.24 就进标准库了 —— 换过去是净删 20 行、零新依赖。顺带说,标准库的速度让同样耗时下把迭代数提到约 600k 成为可能。
建议处置
P0-1 和 P0-2 我认为应该在合并前修:一个让正常操作报错并使注册表失同步,一个会破坏用户手写的 Codex 配置,而仓库里已有正确做法可以复用。P1 的 OpenCode env 也值得一并修,因为它是"静默失败 + 谎报已处理密钥"的组合。
Summary
Implement mcp registry, allows users to create, manage and sync different mcp servers across agents.
Related issue: #154
Verification
Change checklist
frontend/bindings,frontend/src/backend/wails.ts, and handwritten API types were synchronized.README.mdandREADME_ZH.mdwere updated together.NOTICEwas updated.AGENTS.mdanddocs/internal/remain in Chinese.