From 6eb6290001f451898f8e780ef481a5fcfcab1a6f Mon Sep 17 00:00:00 2001 From: Jonathan Haas Date: Mon, 3 Aug 2026 19:47:27 -0700 Subject: [PATCH] Fix query encoding, policy matching, scaffold deps; harden HTTP transports Correctness: - Query parameter values were passed through encodeURIComponent and then appended to URLSearchParams, which encodes again at serialization; values like "a b" reached the upstream API as "a%2520b". Values are now appended raw and encoded exactly once. allowReserved parameters bypass URLSearchParams so reserved characters survive as-is. - Tool allow/deny patterns did not escape "?" when building the regex, so a literal "?" in a pattern acted as a regex optional quantifier. Pattern matching moved to src/policy.ts with "*" as the only wildcard, and is now unit-tested. - The SSE /messages route fell back to "the only active session" when the sessionId did not match; the session id is now matched strictly. - Unknown CLI arguments now fail with an error instead of being silently ignored (a typo like --allow-host previously disabled the host allowlist without any signal). - Response size limits now measure UTF-8 bytes (Buffer.byteLength), not UTF-16 code units. - OAuth2 client-credentials env lookup now honors the --auth-scope env prefix; previously it always read MCP_OPENAPI_* even for scoped tags. - init/generate scaffolds depended on npm "mcp-openapi@latest", which is an unrelated third-party package; they now depend on github:evalops/mcp-openapi. Transport hardening: - Web transports bind to 127.0.0.1 by default; --host restores wider binding. - /mcp, /sse, and /messages validate the Origin header (DNS-rebinding protection). Localhost origins are always allowed; --allow-origins adds more. Non-browser clients without an Origin header are unaffected. - If MCP_OPENAPI_HTTP_AUTH_TOKEN is set, /mcp, /sse, and /messages require Authorization: Bearer , compared timing-safe. - CORS reflects only allowed origins instead of "*". - redactSecrets now recurses into arrays instead of converting them to objects. - Added --version; fixed literal "\n" in the SSE listen banner. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LqBsGC7xLvihtBxhCdWKz5 --- src/http.ts | 47 ++++++++------ src/policy.ts | 28 ++++++++ src/server.ts | 138 ++++++++++++++++++++++++++++------------ test/cli.test.ts | 35 ++++++++++ test/http.test.ts | 55 ++++++++++++++++ test/policy.test.ts | 61 ++++++++++++++++++ test/transports.test.ts | 75 +++++++++++++++++++++- 7 files changed, 377 insertions(+), 62 deletions(-) create mode 100644 src/policy.ts create mode 100644 test/policy.test.ts diff --git a/src/http.ts b/src/http.ts index 0805381..cd9d697 100644 --- a/src/http.ts +++ b/src/http.ts @@ -255,36 +255,41 @@ function applyQueryParams(url: URL, specs: ParameterSpec[], queryParams: Record< const spec = byName.get(name); const style = spec?.style ?? "form"; const explode = spec?.explode ?? true; - serializeQueryParam(url.searchParams, name, value, style, explode, Boolean(spec?.allowReserved)); + serializeQueryParam(url, name, value, style, explode, Boolean(spec?.allowReserved)); } } -function serializeQueryParam(searchParams: URLSearchParams, name: string, value: unknown, style: string, explode: boolean, allowReserved: boolean): void { - const encode = (v: string) => (allowReserved ? v : encodeURIComponent(v)); +// Values are appended raw; URLSearchParams percent-encodes once at serialization time. +// allowReserved params bypass URLSearchParams so reserved characters survive as-is. +function serializeQueryParam(url: URL, name: string, value: unknown, style: string, explode: boolean, allowReserved: boolean): void { + const append = (key: string, raw: string) => { + if (allowReserved) appendRawQueryPair(url, key, raw); + else url.searchParams.append(key, raw); + }; if (style === "deepObject" && isObject(value)) { for (const [k, v] of Object.entries(value as Record)) { - searchParams.append(`${name}[${k}]`, String(v)); + append(`${name}[${k}]`, String(v)); } return; } if (Array.isArray(value)) { if (style === "spaceDelimited") { - searchParams.append(name, value.map((v) => encode(String(v))).join(" ")); + append(name, value.map((v) => String(v)).join(" ")); return; } if (style === "pipeDelimited") { - searchParams.append(name, value.map((v) => encode(String(v))).join("|")); + append(name, value.map((v) => String(v)).join("|")); return; } if (explode) { for (const item of value) { - searchParams.append(name, encode(String(item))); + append(name, String(item)); } return; } - searchParams.append(name, value.map((v) => encode(String(v))).join(",")); + append(name, value.map((v) => String(v)).join(",")); return; } @@ -292,15 +297,21 @@ function serializeQueryParam(searchParams: URLSearchParams, name: string, value: const entries = Object.entries(value as Record); if (explode) { for (const [k, v] of entries) { - searchParams.append(k, encode(String(v))); + append(k, String(v)); } return; } - searchParams.append(name, entries.flatMap(([k, v]) => [k, String(v)]).map((x) => encode(x)).join(",")); + append(name, entries.flatMap(([k, v]) => [k, String(v)]).join(",")); return; } - searchParams.append(name, encode(String(value))); + append(name, String(value)); +} + +function appendRawQueryPair(url: URL, name: string, rawValue: string): void { + const existing = url.search.startsWith("?") ? url.search.slice(1) : url.search; + const pair = `${encodeURIComponent(name)}=${rawValue}`; + url.search = existing ? `${existing}&${pair}` : pair; } function applyHeaderParams(headers: Record, specs: ParameterSpec[], headerParams: Record): void { @@ -443,7 +454,7 @@ async function applyAuth(headers: Record, url: URL, cookieParams keyByName ?? process.env[`${envPrefix}OAUTH2_ACCESS_TOKEN`] ?? process.env[`${envPrefix}BEARER_TOKEN`] ?? - (await getOAuth2AccessToken(scheme)); + (await getOAuth2AccessToken(scheme, envPrefix)); if (token) headers.authorization = `Bearer ${token}`; continue; } @@ -455,16 +466,16 @@ async function applyAuth(headers: Record, url: URL, cookieParams } } -async function getOAuth2AccessToken(scheme: SecurityScheme): Promise { +async function getOAuth2AccessToken(scheme: SecurityScheme, envPrefix: string = "MCP_OPENAPI_"): Promise { if (!scheme.tokenUrl) return undefined; - const cacheKey = `${scheme.name}:${scheme.tokenUrl}`; + const cacheKey = `${envPrefix}:${scheme.name}:${scheme.tokenUrl}`; const cached = oauthTokenCache.get(cacheKey); if (cached && cached.expiresAtMs > Date.now() + 15_000) return cached.accessToken; - const clientId = process.env[`MCP_OPENAPI_${scheme.name.toUpperCase()}_CLIENT_ID`] ?? process.env.MCP_OPENAPI_OAUTH2_CLIENT_ID; + const clientId = process.env[`${envPrefix}${scheme.name.toUpperCase()}_CLIENT_ID`] ?? process.env[`${envPrefix}OAUTH2_CLIENT_ID`]; const clientSecret = - process.env[`MCP_OPENAPI_${scheme.name.toUpperCase()}_CLIENT_SECRET`] ?? process.env.MCP_OPENAPI_OAUTH2_CLIENT_SECRET; + process.env[`${envPrefix}${scheme.name.toUpperCase()}_CLIENT_SECRET`] ?? process.env[`${envPrefix}OAUTH2_CLIENT_SECRET`]; if (!clientId || !clientSecret) return undefined; const scope = Object.keys(scheme.scopes ?? {}).join(" "); @@ -628,7 +639,7 @@ async function parseResponseBody(response: Response, maxBytes: number): Promise< const contentType = response.headers.get("content-type") ?? ""; if (contentType.includes("application/json") || contentType.includes("+json")) { const text = await response.text(); - enforceSize(text.length, maxBytes); + enforceSize(Buffer.byteLength(text, "utf8"), maxBytes); return JSON.parse(text); } @@ -639,7 +650,7 @@ async function parseResponseBody(response: Response, maxBytes: number): Promise< } const text = await response.text(); - enforceSize(text.length, maxBytes); + enforceSize(Buffer.byteLength(text, "utf8"), maxBytes); return text; } diff --git a/src/policy.ts b/src/policy.ts new file mode 100644 index 0000000..4cacef4 --- /dev/null +++ b/src/policy.ts @@ -0,0 +1,28 @@ +import type { OperationModel, RuntimeOptions } from "./types.js"; + +export function isToolAllowed(operation: OperationModel, runtime: RuntimeOptions): boolean { + if (runtime.allowedMethods.length > 0 && !runtime.allowedMethods.includes(operation.method.toUpperCase())) { + return false; + } + + if (runtime.allowedPathPrefixes.length > 0 && !runtime.allowedPathPrefixes.some((prefix) => operation.pathTemplate.startsWith(prefix))) { + return false; + } + + if (runtime.allowToolPatterns.length > 0 && !runtime.allowToolPatterns.some((pattern) => matchToolPattern(operation.operationId, pattern))) { + return false; + } + + if (runtime.denyToolPatterns.some((pattern) => matchToolPattern(operation.operationId, pattern))) { + return false; + } + + return true; +} + +// `*` is the only wildcard; every other character matches literally. +export function matchToolPattern(value: string, pattern: string): boolean { + const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replaceAll("*", ".*"); + const re = new RegExp(`^${escaped}$`); + return re.test(value); +} diff --git a/src/server.ts b/src/server.ts index b2caf05..a5b3b75 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node import { basename, resolve } from "node:path"; +import { timingSafeEqual } from "node:crypto"; import { mkdir, readFile as readFileAsync, writeFile } from "node:fs/promises"; import { readFileSync } from "node:fs"; import { watch } from "node:fs"; @@ -35,6 +36,7 @@ import { observeLatency, observeStatus, renderPrometheus, metrics } from "./metr import type { CompileOptions, OperationModel, RuntimeOptions } from "./types.js"; import { zodFromJsonSchema } from "./zod-schema.js"; import { compileDocumentWithCache } from "./compile-cache.js"; +import { isToolAllowed } from "./policy.js"; import { lintOpenApiDocument } from "./lint.js"; import { loadOpenApiDocument } from "./openapi.js"; import yaml from "js-yaml"; @@ -70,9 +72,40 @@ interface CliOptions { watchSpec: boolean; transport: "stdio" | "streamable-http" | "sse"; port: number; + host: string; + allowedOrigins: string[]; runtime: RuntimeOptions; } +// Browser requests carry an Origin header; validating it prevents DNS-rebinding +// attacks against locally bound HTTP transports. Non-browser MCP clients send no +// Origin and are unaffected. Localhost origins are always accepted. +function isOriginAllowed(origin: string | undefined, allowedOrigins: string[]): boolean { + if (!origin) return true; + if (allowedOrigins.includes(origin)) return true; + try { + const parsed = new URL(origin); + return parsed.hostname === "localhost" || parsed.hostname === "127.0.0.1" || parsed.hostname === "[::1]" || parsed.hostname === "::1"; + } catch { + return false; + } +} + +function httpAuthToken(): string | undefined { + const token = process.env.MCP_OPENAPI_HTTP_AUTH_TOKEN; + return token && token.trim() ? token : undefined; +} + +function isAuthorized(authorizationHeader: string | undefined): boolean { + const required = httpAuthToken(); + if (!required) return true; + if (!authorizationHeader?.startsWith("Bearer ")) return false; + const presented = Buffer.from(authorizationHeader.slice("Bearer ".length)); + const expected = Buffer.from(required); + if (presented.length !== expected.length) return false; + return timingSafeEqual(presented, expected); +} + let inFlightCalls = 0; let responseTransform: ((ctx: { operation: OperationModel; response: { body: unknown; status: number } }) => unknown | Promise) | undefined; @@ -332,9 +365,9 @@ async function startWebServer(state: RuntimeState, cli: CliOptions, specPath: st app.use( "*", cors({ - origin: "*", + origin: (origin) => (isOriginAllowed(origin, cli.allowedOrigins) ? origin : ""), allowMethods: ["GET", "POST", "DELETE", "OPTIONS"], - allowHeaders: ["Content-Type", "mcp-session-id", "Last-Event-ID", "mcp-protocol-version"], + allowHeaders: ["Content-Type", "Authorization", "mcp-session-id", "Last-Event-ID", "mcp-protocol-version"], exposeHeaders: ["mcp-session-id", "mcp-protocol-version"] }) ); @@ -346,6 +379,12 @@ async function startWebServer(state: RuntimeState, cli: CliOptions, specPath: st app.get("/test/sse", (c) => c.html(SSE_TEST_HTML)); app.all("/mcp", async (c) => { + if (!isOriginAllowed(c.req.header("origin"), cli.allowedOrigins)) { + return c.json({ error: "Origin not allowed" }, 403); + } + if (!isAuthorized(c.req.header("authorization"))) { + return c.json({ error: "Unauthorized" }, 401); + } const transport = new WebStandardStreamableHTTPServerTransport(); const server = createMcpServer(state, cli); await server.connect(transport); @@ -358,13 +397,13 @@ async function startWebServer(state: RuntimeState, cli: CliOptions, specPath: st }); } - const server = serve({ fetch: app.fetch, port: cli.port }); + const server = serve({ fetch: app.fetch, port: cli.port, hostname: cli.host }); process.stderr.write( - `Listening on http://localhost:${cli.port} (${cli.transport})\n` + - `Health: http://localhost:${cli.port}/health\n` + - `Metrics: http://localhost:${cli.port}/metrics\n` + - `Streamable Test: http://localhost:${cli.port}/test/streamable\n` + - `SSE Test: http://localhost:${cli.port}/test/sse\n` + `Listening on http://${cli.host}:${cli.port} (${cli.transport})\n` + + `Health: http://${cli.host}:${cli.port}/health\n` + + `Metrics: http://${cli.host}:${cli.port}/metrics\n` + + `Streamable Test: http://${cli.host}:${cli.port}/test/streamable\n` + + `SSE Test: http://${cli.host}:${cli.port}/test/sse\n` ); wireGracefulShutdown(async () => { @@ -408,6 +447,20 @@ async function startSseServer(state: RuntimeState, cli: CliOptions, specPath: st return; } + if ((url.pathname === "/sse" || url.pathname === "/messages") && !isOriginAllowed(req.headers.origin, cli.allowedOrigins)) { + res.statusCode = 403; + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ error: "Origin not allowed" })); + return; + } + + if ((url.pathname === "/sse" || url.pathname === "/messages") && !isAuthorized(req.headers.authorization)) { + res.statusCode = 401; + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ error: "Unauthorized" })); + return; + } + if (req.method === "GET" && url.pathname === "/sse") { if (transports.size >= cli.runtime.sseMaxSessions) { res.statusCode = 503; @@ -435,7 +488,7 @@ async function startSseServer(state: RuntimeState, cli: CliOptions, specPath: st evictExpiredSseSessions(transports, cli.runtime.sseSessionTtlMs); - const stored = transports.get(sessionId) ?? (transports.size === 1 ? [...transports.values()][0] : undefined); + const stored = transports.get(sessionId); if (!stored) { res.statusCode = 404; res.setHeader("content-type", "application/json"); @@ -463,7 +516,7 @@ async function startSseServer(state: RuntimeState, cli: CliOptions, specPath: st wireSpecWatcher(specPath, cli, state, async () => {}); } - server.listen(cli.port); + server.listen(cli.port, cli.host); wireGracefulShutdown(async () => { for (const entry of transports.values()) { await entry.transport.close(); @@ -471,10 +524,10 @@ async function startSseServer(state: RuntimeState, cli: CliOptions, specPath: st server.close(); }); process.stderr.write( - `Listening on http://localhost:${cli.port} (sse)\\n` + - `Health: http://localhost:${cli.port}/health\\n` + - `Metrics: http://localhost:${cli.port}/metrics\\n` + - `SSE Test: http://localhost:${cli.port}/test/sse\\n` + `Listening on http://${cli.host}:${cli.port} (sse)\n` + + `Health: http://${cli.host}:${cli.port}/health\n` + + `Metrics: http://${cli.host}:${cli.port}/metrics\n` + + `SSE Test: http://${cli.host}:${cli.port}/test/sse\n` ); await new Promise(() => undefined); @@ -552,6 +605,7 @@ async function sendLog(server: Server, level: LoggingLevel, data: unknown, sessi } function redactSecrets(data: unknown): unknown { + if (Array.isArray(data)) return data.map((item) => redactSecrets(item)); if (!isObject(data)) return data; const out: Record = {}; for (const [key, value] of Object.entries(data as Record)) { @@ -584,7 +638,8 @@ async function scaffoldProject(targetDir: string): Promise { check: "tsc -p tsconfig.json --noEmit" }, dependencies: { - "mcp-openapi": "latest" + // The npm package named "mcp-openapi" is an unrelated third-party project; install from GitHub. + "mcp-openapi": "github:evalops/mcp-openapi" }, devDependencies: { "@types/node": "^22.13.4", @@ -684,32 +739,6 @@ function asObject(value: unknown): Record { return isObject(value) ? (value as Record) : { value }; } -function isToolAllowed(operation: OperationModel, runtime: RuntimeOptions): boolean { - if (runtime.allowedMethods.length > 0 && !runtime.allowedMethods.includes(operation.method.toUpperCase())) { - return false; - } - - if (runtime.allowedPathPrefixes.length > 0 && !runtime.allowedPathPrefixes.some((prefix) => operation.pathTemplate.startsWith(prefix))) { - return false; - } - - if (runtime.allowToolPatterns.length > 0 && !runtime.allowToolPatterns.some((pattern) => minimatch(operation.operationId, pattern))) { - return false; - } - - if (runtime.denyToolPatterns.some((pattern) => minimatch(operation.operationId, pattern))) { - return false; - } - - return true; -} - -function minimatch(value: string, pattern: string): boolean { - const escaped = pattern.replace(/[.+^${}()|[\]\\]/g, "\\$&").replaceAll("*", ".*"); - const re = new RegExp(`^${escaped}$`); - return re.test(value); -} - async function loadResponseTransform(modulePath: string): Promise<(ctx: { operation: OperationModel; response: { body: unknown; status: number } }) => unknown | Promise> { const loaded = (await import(resolve(modulePath))) as Record; const fn = (typeof loaded.default === "function" ? loaded.default : loaded.transform) as unknown; @@ -776,7 +805,7 @@ async function generateProjectFromSpec(targetDir: string, specPath: string, oper build: "tsc -p tsconfig.json", start: "node dist/server.js" }, - dependencies: { "mcp-openapi": "latest" }, + dependencies: { "mcp-openapi": "github:evalops/mcp-openapi" }, devDependencies: { typescript: "^5.7.3", tsx: "^4.20.3", "@types/node": "^22.13.4" } }, null, @@ -960,6 +989,8 @@ function parseArgs(argv: string[]): CliOptions { let toolNameSeparator: string | undefined; let transport: CliOptions["transport"] = "stdio"; let port = 3000; + let host = "127.0.0.1"; + let allowedOrigins: string[] = []; let timeoutMs = 20_000; let retries = 2; let retryDelayMs = 500; @@ -989,6 +1020,8 @@ function parseArgs(argv: string[]): CliOptions { watchSpec, transport, port, + host, + allowedOrigins, runtime: { timeoutMs, retries, @@ -1111,6 +1144,14 @@ function parseArgs(argv: string[]): CliOptions { port = parsePositiveInt(argv[++i], "--port"); continue; } + if (arg === "--host") { + host = argv[++i] ?? host; + continue; + } + if (arg === "--allow-origins") { + allowedOrigins = parseCsv(argv[++i]); + continue; + } if (arg === "--watch-spec") { watchSpec = true; continue; @@ -1150,6 +1191,11 @@ function parseArgs(argv: string[]): CliOptions { printHelp(); process.exit(0); } + if (arg === "--version" || arg === "-v") { + process.stdout.write(`${PKG_VERSION}\n`); + process.exit(0); + } + throw new Error(`Unknown argument: ${arg}. Run with --help for usage.`); } if (!specPath) { @@ -1170,6 +1216,8 @@ function parseArgs(argv: string[]): CliOptions { watchSpec, transport, port, + host, + allowedOrigins, runtime: { timeoutMs, retries, @@ -1231,6 +1279,8 @@ function printHelp(): void { " --validate-spec", " --transport stdio|streamable-http|sse", " --port ", + " --host
bind address for web transports (default: 127.0.0.1)", + " --allow-origins o1,o2 extra allowed Origin values for web transports (localhost always allowed)", " --watch-spec", " --timeout-ms ", " --retries ", @@ -1250,6 +1300,10 @@ function printHelp(): void { " --auth-scope tag=PREFIX pairs (e.g. governance=GOV,meter=METER)", " --policy-webhook URL for policy webhook (fail-closed)", " --tool-name-separator separator for tool names (default: _)", + " --version, -v print version and exit", + "", + "Transport env vars:", + " MCP_OPENAPI_HTTP_AUTH_TOKEN if set, /mcp, /sse, /messages require Authorization: Bearer ", "", "Auth env vars:", " MCP_OPENAPI_API_KEY", diff --git a/test/cli.test.ts b/test/cli.test.ts index 728164a..f1b556d 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -64,3 +64,38 @@ test("init command creates Gate scaffold files", async () => { assert.match(gateConnector, /endpoint_path: "\/mcp"/); assert.match(gateConnector, /dir: "\.\.\/\.data\/gate-mcp-recordings"/); }); + +test("unknown CLI arguments are rejected", async () => { + const result = spawnSync( + process.execPath, + [tsxCli, "src/server.ts", "--spec", "test/fixtures/sample-openapi.yaml", "--allow-host", "example.com"], + { cwd: process.cwd(), encoding: "utf8" } + ); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Unknown argument: --allow-host/); +}); + +test("--version prints the package version", async () => { + const result = spawnSync(process.execPath, [tsxCli, "src/server.ts", "--version"], { + cwd: process.cwd(), + encoding: "utf8" + }); + + assert.equal(result.status, 0); + assert.match(result.stdout.trim(), /^\d+\.\d+\.\d+$/); +}); + +test("scaffolds depend on the GitHub source, not the unrelated npm package", async () => { + const outDir = await mkdtemp(resolve(tmpdir(), "mcp-openapi-dep-")); + const result = spawnSync(process.execPath, [tsxCli, "src/server.ts", "init", outDir], { + cwd: process.cwd(), + encoding: "utf8" + }); + + assert.equal(result.status, 0); + const packageJson = JSON.parse(await readFile(resolve(outDir, "package.json"), "utf8")) as { + dependencies: Record; + }; + assert.equal(packageJson.dependencies["mcp-openapi"], "github:evalops/mcp-openapi"); +}); diff --git a/test/http.test.ts b/test/http.test.ts index 2442d86..8c602ee 100644 --- a/test/http.test.ts +++ b/test/http.test.ts @@ -372,3 +372,58 @@ test("executeOperation respects cancellation signal", async () => { await new Promise((resolve, reject) => server.close((err) => (err ? reject(err) : resolve()))); }); + +test("executeOperation encodes query values exactly once", async () => { + let rawUrl = ""; + let decodedValue: string | null = null; + const server = createServer((req, res) => { + rawUrl = req.url ?? ""; + decodedValue = new URL(rawUrl, "http://localhost").searchParams.get("q"); + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ ok: true })); + }); + + await new Promise((resolve) => server.listen(0, resolve)); + const address = server.address(); + assert.ok(address && typeof address === "object"); + const baseUrl = `http://127.0.0.1:${address.port}`; + + const parameters: ParameterSpec[] = [{ name: "q", in: "query", required: false }]; + const result = await executeOperation( + createOperation(baseUrl, { parameters }), + { query: { q: "a b&c=d" } }, + { timeoutMs: 5000, retries: 0, retryDelayMs: 5 } + ); + + await new Promise((resolve, reject) => server.close((err) => (err ? reject(err) : resolve()))); + + assert.equal(result.status, 200); + assert.equal(decodedValue, "a b&c=d"); + assert.ok(!rawUrl.includes("%25"), `expected no double-encoding, got ${rawUrl}`); +}); + +test("executeOperation preserves reserved characters for allowReserved params", async () => { + let rawUrl = ""; + const server = createServer((req, res) => { + rawUrl = req.url ?? ""; + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ ok: true })); + }); + + await new Promise((resolve) => server.listen(0, resolve)); + const address = server.address(); + assert.ok(address && typeof address === "object"); + const baseUrl = `http://127.0.0.1:${address.port}`; + + const parameters: ParameterSpec[] = [{ name: "filter", in: "query", required: false, allowReserved: true }]; + const result = await executeOperation( + createOperation(baseUrl, { parameters }), + { query: { filter: "a/b:c,d" } }, + { timeoutMs: 5000, retries: 0, retryDelayMs: 5 } + ); + + await new Promise((resolve, reject) => server.close((err) => (err ? reject(err) : resolve()))); + + assert.equal(result.status, 200); + assert.ok(rawUrl.includes("filter=a/b:c,d"), `expected raw reserved characters, got ${rawUrl}`); +}); diff --git a/test/policy.test.ts b/test/policy.test.ts new file mode 100644 index 0000000..00c0ea9 --- /dev/null +++ b/test/policy.test.ts @@ -0,0 +1,61 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { isToolAllowed, matchToolPattern } from "../src/policy.js"; +import type { OperationModel, RuntimeOptions } from "../src/types.js"; + +function runtime(overrides: Partial): RuntimeOptions { + return { + timeoutMs: 1000, + retries: 0, + retryDelayMs: 1, + maxResponseBytes: 1000, + allowedHosts: [], + maxConcurrency: 1, + allowToolPatterns: [], + denyToolPatterns: [], + allowedMethods: [], + allowedPathPrefixes: [], + sseMaxSessions: 1, + sseSessionTtlMs: 1000, + ...overrides + }; +} + +function operation(overrides: Partial): OperationModel { + return { + operationId: "getUsers", + method: "GET", + pathTemplate: "/users", + description: "d", + toolDescription: "d", + inputSchema: { type: "object" }, + parameters: [], + servers: ["https://api.example.com"], + authOptions: [], + ...overrides + }; +} + +test("matchToolPattern treats * as wildcard", () => { + assert.equal(matchToolPattern("getUsers", "get*"), true); + assert.equal(matchToolPattern("deleteUsers", "get*"), false); + assert.equal(matchToolPattern("anything", "*"), true); +}); + +test("matchToolPattern treats regex metacharacters as literals", () => { + assert.equal(matchToolPattern("op?name", "op?name"), true); + assert.equal(matchToolPattern("opname", "op?name"), false); + assert.equal(matchToolPattern("opXname", "op?name"), false); + assert.equal(matchToolPattern("a.b", "a.b"), true); + assert.equal(matchToolPattern("aXb", "a.b"), false); +}); + +test("isToolAllowed applies allow and deny patterns", () => { + const op = operation({}); + assert.equal(isToolAllowed(op, runtime({ allowToolPatterns: ["get*"] })), true); + assert.equal(isToolAllowed(op, runtime({ allowToolPatterns: ["post*"] })), false); + assert.equal(isToolAllowed(op, runtime({ denyToolPatterns: ["getUsers"] })), false); + assert.equal(isToolAllowed(op, runtime({ allowedMethods: ["POST"] })), false); + assert.equal(isToolAllowed(op, runtime({ allowedPathPrefixes: ["/admin"] })), false); + assert.equal(isToolAllowed(op, runtime({ allowedPathPrefixes: ["/users"] })), true); +}); diff --git a/test/transports.test.ts b/test/transports.test.ts index 6b4075d..e46e539 100644 --- a/test/transports.test.ts +++ b/test/transports.test.ts @@ -28,10 +28,17 @@ async function waitForHealth(port: number): Promise { throw new Error("Server did not become healthy"); } -function startWebServer(transport: "streamable-http" | "sse", specPath: string, apiBase: string, port: number): ChildProcessWithoutNullStreams { +function startWebServer( + transport: "streamable-http" | "sse", + specPath: string, + apiBase: string, + port: number, + env: Record = {} +): ChildProcessWithoutNullStreams { const child = spawn(process.execPath, ["dist/server.js", "--spec", specPath, "--server-url", apiBase, "--transport", transport, "--port", String(port)], { cwd: process.cwd(), - stdio: "pipe" + stdio: "pipe", + env: { ...process.env, ...env } }); return child; } @@ -129,3 +136,67 @@ test("sse transport integration", async () => { } }); }); + +test("streamable-http rejects non-local origins and enforces bearer auth when configured", async () => { + await withApiServer(async (apiBase) => { + const port = await getFreePort(); + const child = startWebServer("streamable-http", "test/fixtures/sample-openapi.yaml", apiBase, port, { + MCP_OPENAPI_HTTP_AUTH_TOKEN: "test-token" + }); + + try { + await waitForHealth(port); + const base = `http://127.0.0.1:${port}/mcp`; + const body = JSON.stringify({ jsonrpc: "2.0", id: 1, method: "ping" }); + const headers = { "content-type": "application/json", accept: "application/json, text/event-stream" }; + + const badOrigin = await fetch(base, { + method: "POST", + headers: { ...headers, origin: "http://evil.example", authorization: "Bearer test-token" }, + body + }); + assert.equal(badOrigin.status, 403); + + const noAuth = await fetch(base, { method: "POST", headers, body }); + assert.equal(noAuth.status, 401); + + const wrongAuth = await fetch(base, { method: "POST", headers: { ...headers, authorization: "Bearer wrong" }, body }); + assert.equal(wrongAuth.status, 401); + + const goodAuth = await fetch(base, { method: "POST", headers: { ...headers, authorization: "Bearer test-token" }, body }); + assert.notEqual(goodAuth.status, 401); + assert.notEqual(goodAuth.status, 403); + await goodAuth.body?.cancel(); + } finally { + child.kill("SIGTERM"); + await sleep(100); + } + }); +}); + +test("sse endpoints reject non-local origins", async () => { + await withApiServer(async (apiBase) => { + const port = await getFreePort(); + const child = startWebServer("sse", "test/fixtures/sample-openapi.yaml", apiBase, port); + + try { + await waitForHealth(port); + + const badOrigin = await fetch(`http://127.0.0.1:${port}/sse`, { + headers: { origin: "http://evil.example" } + }); + assert.equal(badOrigin.status, 403); + await badOrigin.body?.cancel(); + + const badMessages = await fetch(`http://127.0.0.1:${port}/messages?sessionId=none`, { + method: "POST", + headers: { origin: "http://evil.example", "content-type": "application/json" }, + body: "{}" + }); + assert.equal(badMessages.status, 403); + } finally { + child.kill("SIGTERM"); + await sleep(100); + } + }); +});