[security] fix(app): validate stored MCP tool URLs#6826
[security] fix(app): validate stored MCP tool URLs#6826c121914yu merged 1 commit intolabring:mainfrom
Conversation
|
|
c121914yu
left a comment
There was a problem hiding this comment.
PR Review: [security] fix(app): validate stored MCP tool URLs
需求理解
该 PR 目标是把 MCP URL 的内网地址校验从直接预览/调用入口扩展到 MCP 工具集创建、更新和工作流运行时,避免已保存的 MCP URL 绕过现有 SSRF 防护。
逻辑验证
- 创建 MCP 工具集时传入 localhost:会在持久化前被
assertMCPUrlNotInternal()拒绝。 - 更新 MCP 工具集时传入 localhost:会在写入 MongoApp/MongoAppVersion 前被拒绝。
- 工作流执行已保存的新旧 MCP 工具:构造
MCPClient前会重新校验 URL。 - 公网 MCP URL:按现有
isInternalAddress()策略允许。 - 公网 URL 30x 到内网:当前仍可绕过,见行级评论。
问题汇总
🔴 严重问题 1 个:MCP 实际连接阶段未防重定向到内网,安全边界仍可被公共跳转 URL 绕过。
测试情况
我在独立 worktree 中安装依赖后尝试运行 corepack pnpm@9.15.9 vitest run test/cases/service/core/app/mcp.test.ts,Vitest 在收集阶段因依赖 buffer-equal-constant-time 读取 Buffer.prototype 失败,测试文件未执行到;因此本地未能完成单测复跑。
审查结论
需修改:请先补齐 MCP transport 请求阶段的重定向防护,并增加 public URL redirect 到 localhost/metadata 的回归测试。
24dd880 to
f673cfd
Compare
Coverage Report
File CoverageNo changed files found. |
|
✅ Admin Preview Image Ready! |
|
✅ Build Successful - Preview code-sandbox Image for this PR: |
|
✅ Build Successful - Preview fastgpt Image for this PR: |
|
✅ Build Successful - Preview mcp_server Image for this PR: |
Summary
This PR hardens the MCP tool URL boundary so the same internal-address validation is enforced when MCP URLs are saved and when stored MCP tools are executed from workflows.
isInternalAddress()policy.Security issues covered
Before this PR
isInternalAddress(url)before connecting to a user-supplied MCP server.urlwithout the same internal-address validation.MCPClientinstances without re-checking the stored URL.After this PR
assertMCPUrlNotInternal()helper.Why this matters
MCP server URLs are server-side network destinations. If a stored MCP URL is allowed to point at localhost, metadata services, or other blocked internal destinations, a later workflow run can make the FastGPT backend connect across a network boundary that the direct MCP preview/run endpoints already attempted to protect.
This patch keeps the existing FastGPT SSRF policy consistent across both admission-time and runtime MCP paths.
How this differs from #6640
PR #6640 added SSRF checks to the direct MCP and HTTP tool run/preview API paths.
This PR addresses the remaining stored-configuration and workflow-runtime variant:
mcpTools/getTools.tsandmcpTools/runTool.ts.MCPClient.Both layers matter because saved tool configuration can outlive the original request path and can be executed later by workflow dispatch code.
Attack flow
Affected code
projects/app/src/pages/api/core/app/mcpTools/create.ts,projects/app/src/pages/api/core/app/mcpTools/update.ts,packages/service/core/workflow/dispatch/child/runTool.ts,packages/service/core/workflow/dispatch/ai/agent/sub/tool/index.tspackages/service/core/app/mcp.tstest/cases/service/core/app/mcp.test.tsRoot cause
Stored MCP URL validation drift:
CVSS assessment
CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:LRationale:
Safe reproduction steps
http://localhost:3000/mcpor another destination thatisInternalAddress()would reject.MCPClientconstruction for the stored URL.After this patch, the create/update request is rejected and workflow execution also re-checks stored MCP URLs before connecting.
Expected vulnerable behavior
MCPClient.Changes in this PR
assertMCPUrlNotInternal()in the MCP service module.Files changed
packages/service/core/app/mcp.tsassertMCPUrlNotInternal()using the existingisInternalAddress()/PRIVATE_URL_TEXTpolicyprojects/app/src/pages/api/core/app/mcpTools/getTools.ts,projects/app/src/pages/api/core/app/mcpTools/runTool.tsprojects/app/src/pages/api/core/app/mcpTools/create.ts,projects/app/src/pages/api/core/app/mcpTools/update.tspackages/service/core/workflow/dispatch/child/runTool.ts,packages/service/core/workflow/dispatch/ai/agent/sub/tool/index.tsMCPClienttest/cases/service/core/app/mcp.test.tsMaintainer impact
Fix rationale
The safest durable boundary is to validate MCP URLs both:
Centralizing this behind
assertMCPUrlNotInternal()keeps future MCP entrypoints aligned with the existing FastGPT internal-address policy.Type of change
Test plan
Executed with:
corepack pnpm@9.15.9 vitest run test/cases/service/core/app/mcp.test.tscorepack pnpm@9.15.9 prettier --config ./.prettierrc.js --check packages/service/core/app/mcp.ts projects/app/src/pages/api/core/app/mcpTools/getTools.ts projects/app/src/pages/api/core/app/mcpTools/runTool.ts projects/app/src/pages/api/core/app/mcpTools/create.ts projects/app/src/pages/api/core/app/mcpTools/update.ts packages/service/core/workflow/dispatch/child/runTool.ts packages/service/core/workflow/dispatch/ai/agent/sub/tool/index.ts test/cases/service/core/app/mcp.test.tscorepack pnpm@9.15.9 eslint --ignore-path .eslintignore packages/service/core/app/mcp.ts projects/app/src/pages/api/core/app/mcpTools/getTools.ts projects/app/src/pages/api/core/app/mcpTools/runTool.ts projects/app/src/pages/api/core/app/mcpTools/create.ts projects/app/src/pages/api/core/app/mcpTools/update.ts packages/service/core/workflow/dispatch/child/runTool.ts packages/service/core/workflow/dispatch/ai/agent/sub/tool/index.ts test/cases/service/core/app/mcp.test.tsToken usage
Disclosure notes