diff --git a/docs/MCP_SECURITY_BOUNDARY.md b/docs/MCP_SECURITY_BOUNDARY.md index 8858a8d..ed55620 100644 --- a/docs/MCP_SECURITY_BOUNDARY.md +++ b/docs/MCP_SECURITY_BOUNDARY.md @@ -2,7 +2,7 @@ Status: **source contract; deployed runtime must be verified separately** -This document records the security boundary introduced by cAPI commit `eb38524268cc3b4bcc767b4c8ce7794c91c777c9` so future changes do not accidentally restore ambient host execution or unauthenticated direct forwarding. +This document records the hosted MCP security boundary so future changes do not accidentally restore ambient host execution, unauthenticated direct forwarding, or arbitrary authenticated outbound network access. ## Responsibility @@ -16,6 +16,10 @@ cAPI is the governed connection/discovery/capability-negotiation layer. It is ** 4. Missing internal auth configuration fails closed; it must never silently make the proxy public. 5. cAPI internal authentication credentials are not forwarded to registered upstream services. 6. A spawned development MCP child must never inherit the complete cAPI service environment. +7. Authentication does not make an arbitrary remote URL safe. Remote MCP/OpenAPI destinations must pass the outbound egress policy before registration and immediately before cAPI-controlled outbound requests. +8. Production remote targets require an explicit server-controlled `CAPI_MCP_ALLOWED_HOSTS` allowlist. Loopback, private, link-local, metadata, reserved, and DNS-to-private destinations are rejected. +9. cAPI-controlled OpenAPI/proxy fetches must not follow an unvalidated redirect to a different destination. Redirects are disabled unless every redirect hop is independently revalidated. +10. Unsupported remote transport types fail closed rather than registering an endpoint that the execution driver cannot honor. ## Non-production local-process MCP @@ -28,12 +32,18 @@ When enabled, the child receives only a minimal runtime environment needed to st ## Remote MCP -Hosted cAPI should use remote MCP transports for actual service connections. Discovery/connection does not itself grant permission for a consequential operation. +Hosted cAPI may use remote MCP transports only under the outbound egress policy above. Discovery/connection does not itself grant permission for a consequential operation. + +The remote URL is not authority. It is untrusted input even when supplied by an authenticated administrator. A target must be canonicalized, matched against the server-controlled allowlist, resolved, and rejected if any resolved address is local/private/link-local/metadata/reserved. Client-visible errors must remain sanitized; transport/DNS details belong in server-side diagnostics. + +The current source validates the initial remote-SSE destination. Deployment verification must additionally prove that the SDK transport cannot redirect a validated public endpoint to a forbidden address. Until that redirect behavior is proven fail-closed (or remote SSE is constrained accordingly), the remote-SSE redirect boundary remains **NOT_VERIFIED**. ## Direct proxy rule The direct proxy is a transport helper, not an authority boundary. Because a direct call does not itself prove that CAPPO authorized the consequence, it is restricted to authenticated internal traffic and must not become a public alternate execution API. +A stored proxy destination is revalidated immediately before the outbound fetch. This is required because DNS and registry state can change after initial registration. + ## Required deployment verification After any deployment affecting these paths, verify at minimum: @@ -41,8 +51,13 @@ After any deployment affecting these paths, verify at minimum: - unauthenticated `GET /api/mcp/servers` is rejected; - unauthenticated `POST /api/mcp/servers` is rejected before any server start; - authenticated production registration of `local-process` is rejected; +- unsupported `remote-http` registration is rejected until a governed implementation exists; +- production remote registration fails closed when `CAPI_MCP_ALLOWED_HOSTS` is not configured; +- loopback/private/link-local/metadata targets and DNS-to-private targets are rejected; +- OpenAPI and direct-proxy redirects cannot pivot to forbidden destinations; - direct proxy requests without the internal key are rejected; - valid internal proxy calls do not forward the internal cAPI key upstream; +- client-visible registry/proxy errors do not disclose internal hostnames, ports, filesystem paths, or raw transport errors; - no alternate public route re-exposes MCP registration or direct forwarding. Do not infer deployed safety from the default branch alone. Record the exact deployed commit and negative-test results before marking the boundary verified live. diff --git a/src/app/api/mcp/servers/route.ts b/src/app/api/mcp/servers/route.ts index 300478e..957f12e 100644 --- a/src/app/api/mcp/servers/route.ts +++ b/src/app/api/mcp/servers/route.ts @@ -11,6 +11,7 @@ import { translateOpenApiToMcp } from "@/lib/covenant/dynamic-mcp"; import { toolRegistry } from "@/lib/covenant/tool-registry"; import { mcpOrchestrator } from "@/lib/mcp/orchestrator"; import type { McpServerDescriptor } from "@/lib/mcp/schema"; +import { OutboundTargetError, validateOutboundTarget } from "@/lib/security/outbound-target"; export const dynamic = "force-dynamic"; @@ -24,6 +25,20 @@ function localProcessAllowed(): boolean { return process.env.NODE_ENV !== "production" && process.env.CAPI_ALLOW_LOCAL_PROCESS_MCP === "true"; } +function safeRegistryError(error: unknown): NextResponse { + if (error instanceof OutboundTargetError) { + return NextResponse.json( + { ok: false, error: "Remote MCP target is not permitted", code: error.code }, + { status: 403 }, + ); + } + console.error("MCP registry operation failed", error); + return NextResponse.json( + { ok: false, error: "MCP registry operation failed" }, + { status: 502 }, + ); +} + export async function POST(req: NextRequest) { const authError = requireRegistryAuth(req); if (authError) return authError; @@ -49,6 +64,22 @@ export async function POST(req: NextRequest) { ); } + // The current driver implements SSE, not Streamable HTTP. Do not accept + // descriptors that are guaranteed to fail later in the execution path. + if (descriptor.type === "remote-http") { + return NextResponse.json( + { error: "remote-http MCP is not implemented; use a supported governed transport" }, + { status: 400 }, + ); + } + + if (descriptor.type === "remote-sse") { + if (!descriptor.serverUrl) { + return NextResponse.json({ error: "serverUrl is required for remote-sse MCP" }, { status: 400 }); + } + await validateOutboundTarget(descriptor.serverUrl); + } + const instance = await mcpOrchestrator.startServer(descriptor); return NextResponse.json({ @@ -57,7 +88,7 @@ export async function POST(req: NextRequest) { status: instance.status, tools_registered: instance.tools.length, tool_names: instance.tools.map((t) => t.name), - error: instance.error, + error: instance.status === "error" ? "MCP server failed to start" : undefined, }); } @@ -70,6 +101,8 @@ export async function POST(req: NextRequest) { ); } + await validateOutboundTarget(openapi_url); + await validateOutboundTarget(base_url); const tools = await translateOpenApiToMcp(server_id, openapi_url, base_url); return NextResponse.json({ @@ -79,8 +112,7 @@ export async function POST(req: NextRequest) { tool_names: tools.map((t) => t.name), }); } catch (err: unknown) { - const message = err instanceof Error ? err.message : String(err); - return NextResponse.json({ ok: false, error: message }, { status: 500 }); + return safeRegistryError(err); } } @@ -99,7 +131,7 @@ export async function GET(req: NextRequest) { type: inst.descriptor.type, status: inst.status, tool_count: inst.tools.length, - error: inst.error, + error: inst.status === "error" ? "MCP server unavailable" : undefined, })), total_tools: toolRegistry.getAllTools().length + nativeInstances.reduce((sum, inst) => sum + inst.tools.length, 0), }); diff --git a/src/app/api/proxy/[serverId]/[...path]/route.ts b/src/app/api/proxy/[serverId]/[...path]/route.ts index cf1ac22..d25d27d 100644 --- a/src/app/api/proxy/[serverId]/[...path]/route.ts +++ b/src/app/api/proxy/[serverId]/[...path]/route.ts @@ -8,6 +8,7 @@ import { timingSafeEqual } from "crypto"; import { NextRequest, NextResponse } from "next/server"; import { toolRegistry } from "@/lib/covenant/tool-registry"; +import { OutboundTargetError, validateOutboundTarget } from "@/lib/security/outbound-target"; export const dynamic = "force-dynamic"; @@ -56,20 +57,38 @@ async function handleProxy( } const path = `/${pathParts.join("/")}`; - const targetUrl = `${server.base_url}${path}${req.nextUrl.search}`; + let targetUrl: URL; + try { + const baseUrl = new URL(server.base_url); + const basePath = baseUrl.pathname.replace(/\/+$/, ""); + const requestPath = path.replace(/^\/+/, ""); + baseUrl.pathname = `${basePath}/${requestPath}`.replace(/\/{2,}/g, "/"); + baseUrl.search = req.nextUrl.search; + targetUrl = await validateOutboundTarget(baseUrl.toString()); + } catch (error) { + if (error instanceof OutboundTargetError) { + return NextResponse.json( + { error: "Registered upstream target is not permitted", code: error.code }, + { status: 403 }, + ); + } + console.error("Proxy target validation failed", error); + return NextResponse.json({ error: "Registered upstream target is unavailable" }, { status: 502 }); + } - // Forward caller-supplied upstream headers, but never leak the cAPI internal - // credential or reverse-proxy internals to the registered destination. + // Forward only headers that are explicitly safe for registered upstreams. + // Caller credentials, cookies, proxy credentials, internal keys, response-only + // headers, and hop-by-hop headers must never leave cAPI through this route. const forwardHeaders = new Headers(); - const blockedHeaders = new Set([ - "host", - "x-forwarded-host", - "x-forwarded-proto", - "x-api-key", - "x-covenant-admin-token", + const allowedRequestHeaders = new Set([ + "accept", + "content-type", + "if-match", + "if-none-match", + "range", ]); for (const [key, value] of req.headers.entries()) { - if (blockedHeaders.has(key.toLowerCase())) continue; + if (!allowedRequestHeaders.has(key.toLowerCase())) continue; forwardHeaders.set(key, value); } @@ -91,6 +110,9 @@ async function handleProxy( headers: forwardHeaders, body: body ?? null, signal: controller.signal, + // A previously validated public target must not be allowed to redirect + // the proxy to a private or metadata destination. + redirect: "error", }); clearTimeout(timer); @@ -111,8 +133,9 @@ async function handleProxy( clearTimeout(timer); const message = err instanceof Error ? err.message : String(err); const isTimeout = message.includes("abort") || message.includes("timeout"); + console.error("MCP proxy upstream request failed", err); return NextResponse.json( - { error: isTimeout ? "Upstream request timed out" : `Proxy error: ${message}` }, + { error: isTimeout ? "Upstream request timed out" : "Upstream request failed" }, { status: isTimeout ? 504 : 502 }, ); } diff --git a/src/lib/covenant/dynamic-mcp.ts b/src/lib/covenant/dynamic-mcp.ts index 22ac063..fd4ed41 100644 --- a/src/lib/covenant/dynamic-mcp.ts +++ b/src/lib/covenant/dynamic-mcp.ts @@ -16,6 +16,7 @@ import { randomUUID } from "crypto"; import { getEngine } from "./engine"; import { toolRegistry, type DynamicTool } from "./tool-registry"; import type { CapabilityIdentity } from "./types"; +import { validateOutboundTarget } from "@/lib/security/outbound-target"; // --------------------------------------------------------------------------- // Minimal OpenAPI types we care about @@ -96,12 +97,20 @@ export async function translateOpenApiToMcp( openapiUrl: string, baseUrl: string, ): Promise { - // Fetch the spec - const res = await fetch(openapiUrl, { headers: { Accept: "application/json" } }); - if (!res.ok) throw new Error(`Failed to fetch OpenAPI spec: ${res.status} ${openapiUrl}`); + // Validate both the specification source and the execution destination. + // Redirects are disabled for the spec fetch so a public allowlisted URL + // cannot redirect cAPI into a private or metadata address. + const validatedSpecUrl = await validateOutboundTarget(openapiUrl); + const validatedBaseUrl = await validateOutboundTarget(baseUrl); + + const res = await fetch(validatedSpecUrl, { + headers: { Accept: "application/json" }, + redirect: "error", + }); + if (!res.ok) throw new Error(`Failed to fetch OpenAPI spec: ${res.status}`); const spec = (await res.json()) as OAPISpec; - if (!spec.paths) throw new Error(`OpenAPI spec at ${openapiUrl} has no paths`); + if (!spec.paths) throw new Error("OpenAPI specification has no paths"); const engine = getEngine(); const tools: DynamicTool[] = []; @@ -126,7 +135,7 @@ export async function translateOpenApiToMcp( inputSchema: buildInputSchema(op), _meta: { server_id: serverId, - base_url: baseUrl, + base_url: validatedBaseUrl.toString(), path, method: rawMethod.toUpperCase() as DynamicTool["_meta"]["method"], capability_id: capabilityId, @@ -164,11 +173,11 @@ export async function translateOpenApiToMcp( } } - // Store in registry + // Store only canonicalized, policy-validated destinations in the registry. toolRegistry.set(serverId, tools, { server_id: serverId, - base_url: baseUrl, - openapi_url: openapiUrl, + base_url: validatedBaseUrl.toString(), + openapi_url: validatedSpecUrl.toString(), registered_at: new Date().toISOString(), }); diff --git a/src/lib/mcp/drivers/McpDriver.ts b/src/lib/mcp/drivers/McpDriver.ts index ecbb9cd..9f1cf13 100644 --- a/src/lib/mcp/drivers/McpDriver.ts +++ b/src/lib/mcp/drivers/McpDriver.ts @@ -2,6 +2,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; import type { McpServerDescriptor } from "../schema"; +import { validateOutboundTarget } from "@/lib/security/outbound-target"; const LOCAL_PROCESS_ENV_ALLOWLIST = [ "PATH", @@ -56,7 +57,8 @@ export class McpDriver { env: buildLocalProcessEnv(descriptor), }); } else if (descriptor.type === "remote-sse" && descriptor.serverUrl) { - transport = new SSEClientTransport(new URL(descriptor.serverUrl)); + const validatedUrl = await validateOutboundTarget(descriptor.serverUrl); + transport = new SSEClientTransport(validatedUrl); } else { throw new Error(`Unsupported or misconfigured MCP descriptor type: ${descriptor.type}`); } diff --git a/src/lib/security/outbound-target.test.ts b/src/lib/security/outbound-target.test.ts new file mode 100644 index 0000000..b82c231 --- /dev/null +++ b/src/lib/security/outbound-target.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { validateOutboundTarget } from "./outbound-target"; + +const publicResolver = async () => ["93.184.216.34"]; + +describe("validateOutboundTarget", () => { + it("accepts an explicitly allowlisted public HTTPS host", async () => { + const url = await validateOutboundTarget("https://api.example.com/v1", { + production: true, + allowedHosts: ["api.example.com"], + resolver: publicResolver, + }); + expect(url.hostname).toBe("api.example.com"); + }); + + it("fails closed in production when no host allowlist is configured", async () => { + await expect(validateOutboundTarget("https://api.example.com", { + production: true, + allowedHosts: [], + resolver: publicResolver, + })).rejects.toMatchObject({ code: "OUTBOUND_ALLOWLIST_UNCONFIGURED" }); + }); + + it.each([ + "http://127.0.0.1:8080", + "http://169.254.169.254/latest/meta-data", + "http://10.1.2.3", + "http://192.168.1.2", + "http://[::1]/", + ])("rejects local or private literal address %s", async (target) => { + await expect(validateOutboundTarget(target, { + production: false, + allowedHosts: [], + })).rejects.toMatchObject({ code: "OUTBOUND_ADDRESS_FORBIDDEN" }); + }); + + it("rejects a public-looking hostname that resolves to a private address", async () => { + await expect(validateOutboundTarget("https://api.example.com", { + production: true, + allowedHosts: ["api.example.com"], + resolver: async () => ["10.0.0.8"], + })).rejects.toMatchObject({ code: "OUTBOUND_ADDRESS_FORBIDDEN" }); + }); + + it("rejects non-allowlisted hosts", async () => { + await expect(validateOutboundTarget("https://other.example.com", { + production: true, + allowedHosts: ["api.example.com"], + resolver: publicResolver, + })).rejects.toMatchObject({ code: "OUTBOUND_HOST_NOT_ALLOWLISTED" }); + }); + + it.each([ + "file:///etc/passwd", + "https://user:pass@api.example.com/", + "https://api.example.com/#fragment", + "http://localhost:3000/", + ])("rejects unsafe URL form %s", async (target) => { + await expect(validateOutboundTarget(target, { + production: false, + allowedHosts: [], + resolver: publicResolver, + })).rejects.toBeTruthy(); + }); +}); diff --git a/src/lib/security/outbound-target.ts b/src/lib/security/outbound-target.ts new file mode 100644 index 0000000..28093af --- /dev/null +++ b/src/lib/security/outbound-target.ts @@ -0,0 +1,151 @@ +import { lookup } from "node:dns/promises"; +import { isIP } from "node:net"; + +export class OutboundTargetError extends Error { + constructor( + public readonly code: string, + message = "Outbound target is not allowed", + ) { + super(message); + this.name = "OutboundTargetError"; + } +} + +type Resolver = (hostname: string) => Promise; + +export interface OutboundTargetOptions { + allowedHosts?: string[]; + production?: boolean; + resolver?: Resolver; +} + +async function defaultResolver(hostname: string): Promise { + const records = await lookup(hostname, { all: true, verbatim: true }); + return records.map((record) => record.address); +} + +function configuredAllowedHosts(): string[] { + return (process.env.CAPI_MCP_ALLOWED_HOSTS ?? "") + .split(",") + .map((value) => value.trim().toLowerCase().replace(/\.$/, "")) + .filter(Boolean); +} + +function normalizeHostname(hostname: string): string { + return hostname + .trim() + .toLowerCase() + .replace(/^\[/, "") + .replace(/\]$/, "") + .replace(/\.$/, ""); +} + +function ipv4Octets(address: string): number[] | null { + if (isIP(address) !== 4) return null; + return address.split(".").map(Number); +} + +export function isUnsafeOutboundAddress(address: string): boolean { + const normalized = normalizeHostname(address); + const octets = ipv4Octets(normalized); + + if (octets) { + const [a, b] = octets; + return ( + a === 0 || + a === 10 || + a === 127 || + (a === 100 && b >= 64 && b <= 127) || + (a === 169 && b === 254) || + (a === 172 && b >= 16 && b <= 31) || + (a === 192 && b === 0) || + (a === 192 && b === 168) || + (a === 192 && b === 0 && octets[2] === 2) || + (a === 198 && (b === 18 || b === 19)) || + (a === 198 && b === 51 && octets[2] === 100) || + (a === 203 && b === 0 && octets[2] === 113) || + a >= 224 + ); + } + + if (isIP(normalized) === 6) { + const value = normalized.toLowerCase(); + return ( + value === "::" || + value === "::1" || + value.startsWith("fc") || + value.startsWith("fd") || + /^fe[89ab]/.test(value) || + value.startsWith("ff") || + value.startsWith("::ffff:") + ); + } + + return false; +} + +function isForbiddenHostname(hostname: string): boolean { + return ( + hostname === "localhost" || + hostname.endsWith(".localhost") || + hostname.endsWith(".local") || + hostname === "metadata.google.internal" + ); +} + +export async function validateOutboundTarget( + input: string, + options: OutboundTargetOptions = {}, +): Promise { + let url: URL; + try { + url = new URL(input); + } catch { + throw new OutboundTargetError("OUTBOUND_URL_INVALID"); + } + + if (url.protocol !== "https:" && url.protocol !== "http:") { + throw new OutboundTargetError("OUTBOUND_SCHEME_FORBIDDEN"); + } + if (url.username || url.password) { + throw new OutboundTargetError("OUTBOUND_CREDENTIALS_FORBIDDEN"); + } + if (url.hash) { + throw new OutboundTargetError("OUTBOUND_FRAGMENT_FORBIDDEN"); + } + + const hostname = normalizeHostname(url.hostname); + if (!hostname || isForbiddenHostname(hostname)) { + throw new OutboundTargetError("OUTBOUND_HOST_FORBIDDEN"); + } + + const production = options.production ?? process.env.NODE_ENV === "production"; + const allowedHosts = (options.allowedHosts ?? configuredAllowedHosts()).map(normalizeHostname); + if (production && allowedHosts.length === 0) { + throw new OutboundTargetError("OUTBOUND_ALLOWLIST_UNCONFIGURED"); + } + if (allowedHosts.length > 0 && !allowedHosts.includes(hostname)) { + throw new OutboundTargetError("OUTBOUND_HOST_NOT_ALLOWLISTED"); + } + + const resolver = options.resolver ?? defaultResolver; + let addresses: string[]; + if (isIP(hostname)) { + addresses = [hostname]; + } else { + try { + addresses = await resolver(hostname); + } catch { + throw new OutboundTargetError("OUTBOUND_DNS_UNAVAILABLE"); + } + } + + if (addresses.length === 0) { + throw new OutboundTargetError("OUTBOUND_DNS_EMPTY"); + } + if (addresses.some(isUnsafeOutboundAddress)) { + throw new OutboundTargetError("OUTBOUND_ADDRESS_FORBIDDEN"); + } + + return url; +}