Repository navigation
[Market] MCP 市场原型:多源可配 + 官方注册表实时目录 + 一键安装(#278) - #285
Conversation
|
按当前 main c992d4e 复核了 head 2e385cd。市场聚合和复用现有 mcp.upsert 的方向清楚,但当前版本仍不能合并。
我做了确定性的 URL guard 输入检查,并检查了两个 aggregator 的请求配置;这证明了上述边界缺口,但没有声称实际网络请求已经访问内网。
PR 描述还明确记录 E2E 未运行。修复上述问题后,请补齐相关市场安装/网络边界 E2E。当前 head 保持不合并。 |
|
收到,两个 P1 都确认属实,马上修复,逐点回复: 1. URL 边界缺口(localhost./、IPv4-mapped、ULA、link-local + redirect 跟随) — 复现了 guard 输入判定:
2. Registry 语义丢失 — 确认: E2E:修复落地后补市场安装与网络边界场景,按 AGENTS 规则记录运行结果。修完推送后在 PR 里逐点贴证据再请求复审。 |
|
两个 P1 已修复并推送:f8f9cb4 前后(见最新 head)——①shared guard 重写(尾点/IPv4-mapped/ULA/link-local/组播/文档地址分类,isPublicHostname + isPublicIpLiteral 供 main 复用),main fetch 改 redirect:manual 逐跳校验 + 命名主机 DNS 解析逐 IP 分类;②Registry 语义保留(named 参数 name/value、positional value、env isRequired→optional、value/default→defaultValue),补 named/env 语义回归测试。网络边界 E2E 场景正在补,先请复审代码修复本身~ |
|
按要求补齐 E2E 并实测通过——新增
Summary 3/3。场景已注册进 e2e 测试计划文档(en/zh 成对)。两个 P1 的代码修复在上一个提交,请复审~ |
Template entries that resolve to plain McpServerInput via the existing
upsert path: ${VAR} placeholders backed by requiredEnv declarations,
lenient catalog validation, and a 15-entry builtin catalog that keeps
the market usable offline.
Browse the builtin catalog from the MCP settings page and install an
entry through the regular upsert path. The install sheet shows the
exact command/url that will be saved and collects ${VAR} values
declared by requiredEnv before writing anything.
Pin the guarantees that matter: install goes through the existing upsert path with no side channel, the sheet shows the exact command and collects requiredEnv values, installed state matches server ids, and the market strings exist in every locale.
Card grid with hover lift and staggered entrance, category-tinted glyphs, animated chips and search focus ring, spring-in install sheet, and a view-level fade — all on ds tokens with a reduced-motion opt-out.
The market now searches registry.modelcontextprotocol.io from the main process (renderer CSP only allows localhost, same split as models.dev) and merges mapped records under the built-in picks. npm packages map to npx templates, pypi to uvx, https remotes to http entries; records without a runnable form are dropped. Built-in stays as the offline floor and registry outages degrade to it with a status note.
Market sources move out of the code and into a managed list: the official registry ships as the built-in default, and users add their own sources — registry-protocol endpoints or static catalog JSON in the market schema. Sources are queried in parallel, tagged per source, deduped, and persisted in localStorage; unsafe (non-https / private) source URLs are rejected on both sides of the IPC boundary.
The settings shell and the market root both carry transforms, which turn the overlay's position:fixed into shell-relative positioning — the sheet rendered near the bottom of the scroll content instead of the viewport. Portal the install and sources sheets to document.body and refresh the PR screenshots taken with the fix in place.
… deeper browse - Market glyphs now follow the entry's tool family (code/globe/book/ database/list-checks) instead of transport only - Registry records carry no taxonomy, so the mapper infers a category from name/title/description keywords — remote entries join the category filters instead of living in 'all' only - Registry browse follows 4 cursor pages (≈400 servers) instead of 2
The settings pane rendered every matching card at once — 400+ entries made the page heavy and the bottom unreachable. Cards now page 24 at a time with prev/next controls and a page/total indicator; search and category changes reset to page 1.
Prev/next alone could not navigate 17 pages. The pager now shows numbered pages with ellipsis collapse (1 2 … 14 15 16 17), a type-a-page jump box, and a last-page shortcut.
Typing a page number and pressing Enter already covers both, so the two buttons only added noise.
The registry holds thousands of servers, so browsing now loads two pages to paint fast and a 'load more' button extends the window by ten pages per click from the remembered cursor. Searches were and remain server-side, so they always see the full catalog; empty-search browsing reports exhaustion so the button hides when a source runs dry.
Headless protocol-level suite (pnpm test:e2e:mcp-market) covering the review asks: guard bypass forms rejected, registry record semantics preserved end-to-end, and a builtin entry installed through the real host binary into an isolated HOME. Scenarios registered in the e2e test plan (en/zh).
90550eb to
26b1999
Compare
vastsa
left a comment
There was a problem hiding this comment.
Review: Request changes
PR 的总体方向(Electron Main 聚合市场源、Renderer 复用现有 mcp.upsert、内置目录离线兜底)是合理的,但当前 head 不能合并,原因如下:
阻塞问题
-
当前与
main有真实合并冲突。 GitHub 当前状态是DIRTY / CONFLICTING;git merge-tree确认冲突涉及apps/desktop/src/lib/api.ts、apps/desktop/src/styles/settings.css、docs/spec/06-delivery/04-e2e-test-plan.md、其中文档、根package.json和packages/shared/src/index.ts。请基于最新mainrebase/解决后再请求复审,不要 force-push 无关分支。 -
P0:DNS 预检与实际连接之间存在 SSRF/DNS-rebinding TOCTOU。
apps/desktop/electron/main/mcp-registry-catalog.ts:51-74先用lookup()判断所有地址为公网,随后让fetch()重新解析主机;攻击者控制的 DNS 可以让预检返回公网地址、实际连接返回 loopback/内网地址。重定向逐跳复核 URL 仍不能消除这个竞态。需要把解析结果固定到实际连接(或使用能验证实际 peer address 的连接层),并为每一跳补 deterministic 测试。 -
P0/P1:公网地址分类不完整。
packages/shared/src/mcp-registry.ts:294-337会接受192.0.0.1、198.18.0.1、文档/保留 IPv4 段以及fec0::1等 special-use 地址;测试只覆盖了少数 RFC1918/loopback 形态。请使用完整的 global-unicast 判定或成熟 IP 解析器,覆盖 IPv4、IPv6、mapped/compatible 和全部 special-use 范围。 -
P1:Registry package 的精确
version被丢弃。packages/shared/src/mcp-registry.ts:162-195只使用identifier,npm/PyPI 记录带version时仍安装 mutable latest。这会使安装结果与 Registry 元数据不一致并引入供应链漂移;请保留并安全编码版本,并添加回归测试。 -
P1:搜索结果被固定截断为一页。
apps/desktop/electron/main/mcp-registry-catalog.ts:148-158固定limit=100却忽略metadata.nextCursor;McpMarketPanel.tsx:650又在有搜索词时隐藏“加载更多”。匹配超过 100 条时用户永远看不到后续结果,与 PR 描述的“命中全库”不符。请实现搜索游标分页或明确收敛产品契约,并补测试。 -
P1:外部响应没有字节上限。
apps/desktop/electron/main/mcp-registry-catalog.ts:70-85直接response.json()/response.text(),对自定义源或被攻陷源的 chunked 超大响应会把主进程内存打满。请在响应头和流式读取两层实施上限,并在失败路径取消 body。
非阻塞但应跟进
- Registry 的 PyPI 映射仍丢失
runtimeArguments/packageArguments; - 市场远程条目只要求 HTTPS,安装后进入 user MCP runtime,未继承市场公网/逐跳策略;
loadMore没有 source/query generation 门控,旧请求可能覆盖新搜索结果;validateMcpCatalogFile对categories等字段缺少运行时类型校验;- 新增市场协议/网络边界需要补充对应 ADR/产品安全规范同步。
验证记录
- PR head
26b1999d的 GitHub CI:JS build/typecheck/lint/architecture/test、Rust、Docs locale 均通过;Vercel 因授权失败,不是代码检查通过。 - PR worktree 定向验证:shared 市场测试 33/33 通过;host-core 369/369 通过;市场面板契约测试 6/6 通过;新增
pnpm test:e2e:mcp-market3/3 通过(这些测试不覆盖上面列出的 DNS 实际连接、完整 special-use 地址、version 映射和搜索游标问题)。
请先解决上述合并冲突和阻塞问题,完成 rebase 后再请求复审。
|
Thank you for the review — the DNS-rebinding TOCTOU call-out (F2) is exactly the kind of attack-surface rigor this surface needs, and the special-use address gaps are real. We'll take every item into the follow-up branch:
Will rebase onto current main and request re-review when the follow-up is green. |
MCP market hardening follows PR #285. Public-network, Registry mapping, pagination, bounded responses, redirect credentials, and bilingual specs/tests are included.
|
Thank you for resolving the conflicts and landing it, and for following up with #330 so quickly — the shared public-network module closes the review cleanly. Appreciated as always. |
Follow-up to the discussion in #278 — 按维护者建议的 "models.dev 模式" 实现了 MCP 市场原型:运行时聚合外部目录源,内置精选仅作离线兜底,零目录维护成本。
功能
1. 多源可配的市场源
/v0/servers)的端点2. 实时目录 + 搜索
浏览视图以内置 15 条精选打底,官方注册表目录按需流式加载(初始 200,点「加载更多」每次追加约 1000,实测已到 1200+,全库数千条);搜索框输入即实时查询注册表服务端(命中全库,不受已加载窗口限制),远程条目带来源徽标:
3. 一键安装(写配置,零新写入路径)
安装 = 目录条目映射成
McpServerInput走现有mcp.upsert,不新增任何配置写入路径。安装前完整展示将要执行的命令:npx -y <pkg>;PyPI 包 →uvx <pkg>;https 远程 → 直连requiredEnv,支持默认值),缺必填值会被拦截:4. 离线兜底
内置 15 条精选(含零配置项)随应用分发,注册表不可达时市场照常可用:
实现说明
packages/shared:目录 schema、注册表记录 → 模板的纯映射器、内置目录、${VAR}/requiredEnv解析(无 I/O,renderer/main 共用)electron/main/mcp-registry-catalog.ts:注册表/目录源聚合器(渲染层 CSP 只允许 localhost,网络层与 models.dev 同层)apps/desktop:市场面板(复用现有 Capability*/ext-sheet 组件与 ds 令牌)、源管理弹层;安装动作复用现有mcp.upsert,host-core 零改动验证
~/.agents/servers/<id>.json→ 列表出现 → 测试连接发现 2 个工具后续(如方向认可)
.mcp.json解析,复用现有粘贴导入解析器)