From 97ae1be91b815fdd85aaf1da3ef691d91acd30b2 Mon Sep 17 00:00:00 2001 From: Nik <84yk8btb9f@privaterelay.appleid.com> Date: Sat, 3 Oct 2026 17:06:38 +0300 Subject: [PATCH] Fix scanner deadlines, catalog integrity and report safety --- README.md | 43 ++++++-- package-lock.json | 76 ++++++++------ package.json | 4 +- src/baseline.ts | 59 ++++++++--- src/checks/index.ts | 25 ++--- src/connect.ts | 179 +++++++++++++++++++++++++++++---- src/index.ts | 26 +++-- src/report.ts | 31 ++++-- test/baseline.test.mjs | 50 +++++++++ test/cli.test.mjs | 51 ++++++++++ test/connect.test.mjs | 146 +++++++++++++++++++++++++++ test/fixtures/check-error.mjs | 8 ++ test/fixtures/http-server.mjs | 75 ++++++++++++++ test/fixtures/stdio-server.mjs | 32 ++++++ test/reliability.test.mjs | 47 +++++++++ 15 files changed, 741 insertions(+), 111 deletions(-) create mode 100644 test/baseline.test.mjs create mode 100644 test/cli.test.mjs create mode 100644 test/connect.test.mjs create mode 100644 test/fixtures/check-error.mjs create mode 100644 test/fixtures/http-server.mjs create mode 100644 test/fixtures/stdio-server.mjs create mode 100644 test/reliability.test.mjs diff --git a/README.md b/README.md index a7d98f9..614bfff 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![npm version](https://img.shields.io/npm/v/toolfence.svg?style=flat-square&color=0B0B0D&labelColor=0B0B0D)](https://www.npmjs.com/package/toolfence) [![CI](https://github.com/nikolas-sapa/toolfence/actions/workflows/ci.yml/badge.svg)](https://github.com/nikolas-sapa/toolfence/actions/workflows/ci.yml) [![License: Apache 2.0](https://img.shields.io/badge/license-Apache%202.0-0B0B0D?style=flat-square&labelColor=0B0B0D)](./LICENSE) -[![Node](https://img.shields.io/badge/node-%3E%3D20-0B0B0D?style=flat-square&labelColor=0B0B0D)](./package.json) +[![Node](https://img.shields.io/badge/node-%3E%3D20.3-0B0B0D?style=flat-square&labelColor=0B0B0D)](./package.json) [![PRs welcome](https://img.shields.io/badge/PRs-welcome-0B0B0D?style=flat-square&labelColor=0B0B0D)](./CONTRIBUTING.md) **Security scanner for MCP servers.** Point it at any [Model Context Protocol](https://modelcontextprotocol.io) server and get a severity-ranked report of the risks your agents inherit by connecting to it. @@ -22,7 +22,7 @@ npx toolfence https://your-server.example.com/mcp Tool "fetch_doc" contains language matching: instruction override. Tool descriptions are fed verbatim into the agent's context — this is a tool-poisoning vector. - HIGH No authentication required + HIGH No credentials required for tool discovery HIGH Tool definition changed since baseline [search] MED Large tool catalog ... @@ -35,7 +35,7 @@ Exit code is non-zero when high/critical findings exist, so it drops straight in ## Why -OAuth authenticated your agents. It didn't make them **safe**. The MCP spec settled authentication (OAuth 2.1 for HTTP transports) — but that's roughly 20% of the threat surface. The other 80% lives in what the tools *do* and what their definitions *say*: +Authentication alone does not establish tool safety. Tool definitions can expose additional risks: - **Tool poisoning** — a malicious server ships a tool description that instructs the agent to read your secrets or ignore prior instructions. The agent reads it and complies. - **Indirect prompt injection** — adversarial text smuggled into tool definitions or schemas. @@ -71,12 +71,19 @@ toolfence https://example.com/mcp --bearer "$TOKEN" # Local stdio server (everything after --stdio is the command) toolfence --stdio npx -y @modelcontextprotocol/server-everything +toolfence --json --no-baseline --stdio node server.js "argument with spaces" # Machine-readable output for CI / dashboards toolfence https://example.com/mcp --json toolfence https://example.com/mcp --markdown -o report.md ``` +Requires Node 20.3 or newer for [combined abort signals](https://nodejs.org/download/release/v20.20.1/docs/api/globals.html#static-method-abortsignalanysignals). +Place scanner options before `--stdio`. Every subsequent argument goes unchanged +to the child server, including `--help`. Connection, handshake and catalog paging +share a 30-second deadline; the initial HTTP header probe is capped at 5 seconds. +Stdio commands run with your user permissions; the scanner does not sandbox them. + ### Options | Flag | Description | @@ -95,7 +102,7 @@ toolfence https://example.com/mcp --markdown -o report.md |------|---------| | `0` | No high/critical findings | | `1` | At least one high or critical finding | -| `2` | Scan could not run (connection / usage error) | +| `2` | Scan incomplete (connection, usage, baseline or check error) | ## What it checks (12) @@ -107,7 +114,7 @@ toolfence https://example.com/mcp --markdown -o report.md | **Known-bad signatures** | Documented MCP abuse patterns (secret-file reads, rug-pulls, cross-tool shadowing, exfiltration-to-external, obfuscated payloads) | | **Tool integrity / drift** | Tool definitions that changed since the last scan | | **Context cost** | Tool catalogs large enough to inflate every agent turn | -| **Rate-limit posture** | No server-side ceiling on call volume | +| **Rate-limit posture** | Missing rate-limit headers on the initial HTTP probe | | **Naming hygiene** | Duplicate or collision-prone generic tool names | | **Sensitive capability** | Tools that reach the filesystem, execute code, or touch the network | | **Schema strength** | Missing, untyped, or unsealed input schemas | @@ -116,13 +123,29 @@ toolfence https://example.com/mcp --markdown -o report.md The [known-bad signature set](./src/signatures.ts) is community-extensible — see [CONTRIBUTING](./CONTRIBUTING.md). +Checks inspect server metadata and tool definitions; they do not invoke tools. +Anonymous catalog discovery does not prove anonymous tool execution. Missing +rate-limit headers do not prove absent enforcement. Findings are heuristics, +not a guarantee that a server is safe or unsafe. + ### Drift detection -The first scan of a target records a fingerprint of every tool definition under -`~/.toolfence/baselines/`. Subsequent scans of the same target compare against it -and flag any tool whose definition changed, appeared, or disappeared — the core -signal for catching a server that turns malicious *after* you trusted it. Run it -in CI to fail the build on unexpected drift. +Each completed scan records tool fingerprints under `~/.toolfence/baselines/`. +The next scan compares names, descriptions, input schemas and safety annotations +against that latest observation. Changes, additions and removals produce findings. +Completed scans replace the baseline even when high/critical findings exist; +this is not an immutable record of approved tools. Incomplete discovery or failed +checks exit 2 without replacing it. Malformed or unreadable baselines fail the scan. + +Writes use a private temporary file and atomic replacement. Reports and baselines +retain the target URL or command arguments: avoid embedding credentials in them. +`--bearer` avoids storing the token in the target, but command-line arguments may +still be visible to local process inspection. Use `--no-baseline` to skip caching. + +Upgrading from earlier versions can flag one-time drift for annotated tools. +Stdio targets now use JSON-quoted argument tokens as their cache identity, so +their first scan establishes a new baseline. Annotation-free HTTP fingerprints +retain their previous format. ## Roadmap diff --git a/package-lock.json b/package-lock.json index 30c9a59..dd48d43 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,32 +1,32 @@ { - "name": "mcpguard", - "version": "0.1.0", + "name": "toolfence", + "version": "0.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { - "name": "mcpguard", - "version": "0.1.0", + "name": "toolfence", + "version": "0.3.0", "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", "gpt-tokenizer": "^3.4.0" }, "bin": { - "mcpguard": "dist/index.js" + "toolfence": "dist/index.js" }, "devDependencies": { "@types/node": "^22.0.0", "typescript": "^5.6.0" }, "engines": { - "node": ">=20" + "node": ">=20.3" } }, "node_modules/@hono/node-server": { - "version": "1.19.14", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.14.tgz", - "integrity": "sha512-GwtvgtXxnWsucXvbQXkRgqksiH2Qed37H9xHZocE5sA3N8O8O8/8FA3uclQXxXVzc9XBZuEOMK7+r02FmSpHtw==", + "version": "1.19.17", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.17.tgz", + "integrity": "sha512-dSneS5qhiauZWGDCeK4o695Xd9nUNjviSZCMQrj10eetr8Uln1ucn6bbphOM6UynAMMtNIzZNSpL9vnASJwrPQ==", "license": "MIT", "engines": { "node": ">=18.14.1" @@ -132,20 +132,20 @@ } }, "node_modules/body-parser": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", - "integrity": "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.3.0.tgz", + "integrity": "sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==", "license": "MIT", "dependencies": { "bytes": "^3.1.2", - "content-type": "^1.0.5", + "content-type": "^2.0.0", "debug": "^4.4.3", - "http-errors": "^2.0.0", - "iconv-lite": "^0.7.0", + "http-errors": "^2.0.1", + "iconv-lite": "^0.7.2", "on-finished": "^2.4.1", - "qs": "^6.14.1", - "raw-body": "^3.0.1", - "type-is": "^2.0.1" + "qs": "^6.15.2", + "raw-body": "^3.0.2", + "type-is": "^2.1.0" }, "engines": { "node": ">=18" @@ -155,6 +155,19 @@ "url": "https://opencollective.com/express" } }, + "node_modules/body-parser/node_modules/content-type": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.1.0.tgz", + "integrity": "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/bytes": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", @@ -453,9 +466,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", - "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "version": "3.1.8", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.8.tgz", + "integrity": "sha512-GZMtZUTNRpOVIECoXwLNZS5xUGE+mVNbTB8h/7Rwh2TFWcBQiPzTgyZi05BF9UMZKkLJv8XBRJTlU7zg8+ZfMg==", "funding": [ { "type": "github", @@ -596,9 +609,9 @@ } }, "node_modules/hono": { - "version": "4.12.24", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.24.tgz", - "integrity": "sha512-I36D1s+HgQc55KbhEr4iybfxv/9o1zdpw+XEM6dJa91LqQD0HCoSGdxpRJCZE+aavs87j4V3Ls2OJzq8C/U4iw==", + "version": "4.13.12", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.12.tgz", + "integrity": "sha512-6E2QDAc9Ick9Sq77ZrGS/dk2WUYni91aufTw6LJKpV7w8kW5/GxVUc650FOADOmlwg3K+f7Pun6XlV+pYmW6gw==", "license": "MIT", "engines": { "node": ">=16.9.0" @@ -647,9 +660,9 @@ "license": "ISC" }, "node_modules/ip-address": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", - "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", + "version": "10.7.2", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.7.2.tgz", + "integrity": "sha512-7H/2gFSIitxc0hG3nOI1glS8QLo/EHBFFLk8vEUjXY/xu0AdL8jZ9U1IzO2PUm0d2D/ofQcAifb0g6OBkt8U7w==", "license": "MIT", "engines": { "node": ">= 12" @@ -860,12 +873,13 @@ } }, "node_modules/qs": { - "version": "6.15.2", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.2.tgz", - "integrity": "sha512-Rzq0KEyX/w/tEybncDgdkZrJgVUsUMk3xjh3t5bv3S1HTAtg+uOYt72+ZfwiQwKdysThkTBdL/rTi6HDmX9Ddw==", + "version": "6.16.0", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.16.0.tgz", + "integrity": "sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==", "license": "BSD-3-Clause", "dependencies": { - "side-channel": "^1.1.0" + "es-define-property": "^1.0.1", + "side-channel": "^1.1.1" }, "engines": { "node": ">=0.6" diff --git a/package.json b/package.json index 52f8632..c7bf8e5 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "build": "tsc -p tsconfig.json", "dev": "tsc -p tsconfig.json --watch", "start": "node dist/index.js", - "test": "npm run build && node --test test/checks.test.mjs", + "test": "npm run build && node --test test/*.test.mjs", "prepublishOnly": "npm run build" }, "keywords": [ @@ -29,7 +29,7 @@ ], "license": "Apache-2.0", "engines": { - "node": ">=20" + "node": ">=20.3" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", diff --git a/src/baseline.ts b/src/baseline.ts index 881bd5b..3467394 100644 --- a/src/baseline.ts +++ b/src/baseline.ts @@ -1,9 +1,6 @@ -// Baseline persistence for drift detection. Stored per-target under -// ~/.toolfence/baselines/.json so re-running the same target compares -// against the last seen tool fingerprints. - -import { createHash } from "node:crypto"; -import { mkdir, readFile, writeFile } from "node:fs/promises"; +// Baselines record the latest completed scan, not an immutable trust decision. +import { createHash, randomUUID } from "node:crypto"; +import { mkdir, readFile, open, rename, rm } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; import type { Baseline, ToolInfo } from "./types.js"; @@ -13,33 +10,63 @@ function baselineDir(): string { return join(homedir(), ".toolfence", "baselines"); } -function baselinePath(target: string): string { +function baselinePath(target: string, directory: string): string { const h = createHash("sha256").update(target).digest("hex").slice(0, 24); - return join(baselineDir(), `${h}.json`); + return join(directory, `${h}.json`); } export async function loadBaseline( target: string, + directory = baselineDir(), ): Promise { + let raw: string; + try { + raw = await readFile(baselinePath(target, directory), "utf8"); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return undefined; + throw err; + } + let parsed: unknown; try { - const raw = await readFile(baselinePath(target), "utf8"); - return JSON.parse(raw) as Baseline; + parsed = JSON.parse(raw); } catch { - return undefined; + throw new Error("Invalid drift baseline JSON; repair it before scanning."); + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error("Invalid drift baseline structure."); } + const b = parsed as Partial; + if (typeof b.target !== "string" || typeof b.createdAt !== "string" || + !b.toolFingerprints || typeof b.toolFingerprints !== "object" || + Array.isArray(b.toolFingerprints) || + !Object.values(b.toolFingerprints).every(hash => typeof hash === "string")) { + throw new Error("Invalid drift baseline structure."); + } + return b as Baseline; } export async function saveBaseline( target: string, tools: ToolInfo[], + directory = baselineDir(), ): Promise { const baseline: Baseline = { createdAt: new Date().toISOString(), target, - toolFingerprints: Object.fromEntries( - tools.map((t) => [t.name, fingerprint(t)]), - ), + toolFingerprints: Object.fromEntries(tools.map(t => [t.name, fingerprint(t)])), }; - await mkdir(baselineDir(), { recursive: true }); - await writeFile(baselinePath(target), JSON.stringify(baseline, null, 2)); + await mkdir(directory, { recursive: true, mode: 0o700 }); + const file = baselinePath(target, directory); + const tmp = `${file}.${randomUUID()}.tmp`; + const handle = await open(tmp, "wx", 0o600); + try { + try { + await handle.writeFile(JSON.stringify(baseline, null, 2)); + } finally { + await handle.close(); + } + await rename(tmp, file); + } finally { + await rm(tmp, { force: true }); + } } diff --git a/src/checks/index.ts b/src/checks/index.ts index c092896..f0abb1b 100644 --- a/src/checks/index.ts +++ b/src/checks/index.ts @@ -14,7 +14,10 @@ function toolText(t: ToolInfo): string { } export function fingerprint(t: ToolInfo): string { - return createHash("sha256").update(toolText(t)).digest("hex").slice(0, 16); + const definition = t.annotations === undefined + ? toolText(t) + : `${toolText(t)}\n${JSON.stringify(t.annotations)}`; + return createHash("sha256").update(definition).digest("hex").slice(0, 16); } // ── 1. Auth posture ───────────────────────────────────────────────────────── @@ -39,9 +42,9 @@ const authPosture: Check = { f.push({ checkId: this.id, severity: "high", - title: "No authentication required", + title: "No credentials required for tool discovery", detail: - "The server accepted an MCP session and listed tools without any credentials. Anyone who can reach this URL can invoke its tools.", + "The server accepted an MCP session and listed tools without any credentials. Tool execution was not tested; listing access does not prove anonymous execution access.", remediation: "Require OAuth 2.1 (per the 2025 MCP spec for HTTP transports) or at minimum a bearer token. Disable anonymous Dynamic Client Registration.", }); @@ -70,7 +73,7 @@ const toolIntegrity: Check = { const now = new Map(ctx.tools.map((t) => [t.name, fingerprint(t)])); for (const [name, hash] of now) { - if (!(name in prev)) { + if (!Object.hasOwn(prev, name)) { f.push({ checkId: this.id, severity: "medium", @@ -83,7 +86,7 @@ const toolIntegrity: Check = { checkId: this.id, severity: "high", title: "Tool definition changed since baseline", - detail: `Tool "${name}" has a different fingerprint than the baseline (description or input schema changed). Silent tool redefinition is a poisoning vector.`, + detail: `Tool "${name}" has a different fingerprint than the baseline (description, input schema or safety annotations changed). Silent tool redefinition is a poisoning vector.`, tool: name, remediation: "Re-review the tool definition. Pin trusted server versions and alert on drift in CI.", @@ -163,13 +166,7 @@ const contextCost: Check = { inputSchema: t.inputSchema, })), ); - let tokens: number; - try { - tokens = encode(catalog).length; - } catch { - // Fallback heuristic if tokenizer chokes on the payload. - tokens = Math.ceil(catalog.length / 4); - } + const tokens = encode(catalog, { disallowedSpecial: new Set() }).length; const sev = tokens >= COST_HIGH ? "high" : tokens >= COST_WARN ? "medium" : "info"; if (sev !== "info") { @@ -216,7 +213,7 @@ const rateLimit: Check = { severity: "low", title: "No rate-limit headers advertised", detail: - "The endpoint exposed no standard rate-limit headers. An agent in a loop can hammer this server with no server-side ceiling, driving runaway cost.", + "The endpoint exposed no standard rate-limit headers. Server-side enforcement was not tested; absent headers do not establish absent call-volume limits.", remediation: "Advertise RateLimit headers (RFC 9239 draft) and enforce per-client quotas. Govern call volume at the gateway layer.", }); @@ -388,7 +385,7 @@ const schemaStrength: Check = { .additionalProperties; const sealed = additional === false; - if (!hasProps) { + if (!hasProps && !sealed) { f.push({ checkId: this.id, severity: "low", diff --git a/src/connect.ts b/src/connect.ts index c1f57e1..4b8dd7c 100644 --- a/src/connect.ts +++ b/src/connect.ts @@ -5,6 +5,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import type { ChildProcess } from "node:child_process"; import type { ConnectionContext, ToolInfo } from "./types.js"; export interface ConnectResult { @@ -19,10 +20,28 @@ export interface ConnectOptions { bearer?: string; // Treat the target as a stdio command instead of a URL. stdio?: boolean; + // Pre-tokenized command and arguments, as supplied by a CLI after --stdio. + stdioArgs?: string[]; + // Deadline for the entire probe, handshake and tool catalog operation. + timeoutMs?: number; } const CLIENT_INFO = { name: "toolfence", version: "0.3.0" }; +function spawnedChild(transport: StdioClientTransport): ChildProcess | undefined { + // The SDK exposes only pid publicly. Its current child handle lets cleanup + // check exit state before signaling, including while SDK close awaits pipes. + const child = (transport as unknown as { _process?: ChildProcess })._process; + return child && child.pid === transport.pid && typeof child.kill === "function" + ? child : undefined; +} + +function terminateChild(child: ChildProcess | undefined): void { + if (child && child.exitCode === null && child.signalCode === null) { + try { child.kill("SIGKILL"); } catch { /* already exited */ } + } +} + function isUrl(target: string): boolean { return /^https?:\/\//i.test(target); } @@ -32,7 +51,10 @@ function isUrl(target: string): boolean { async function probeHeaders( url: URL, bearer?: string, + signal?: AbortSignal, ): Promise | undefined> { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), 5_000); try { const res = await fetch(url, { method: "GET", @@ -42,6 +64,9 @@ async function probeHeaders( }, // Don't follow into auth redirects silently; we want the first response. redirect: "manual", + signal: signal + ? AbortSignal.any([signal, controller.signal]) + : controller.signal, }); const headers: Record = {}; res.headers.forEach((v, k) => { @@ -56,27 +81,48 @@ async function probeHeaders( return headers; } catch { return undefined; + } finally { + clearTimeout(timer); } } async function connectHttp( url: URL, opts: ConnectOptions, + signal: AbortSignal, + register: (client: Client) => void, ): Promise<{ client: Client; usedSse: boolean }> { const requestInit = opts.bearer ? { headers: { Authorization: `Bearer ${opts.bearer}` } } : undefined; + const fetchWithDeadline: typeof fetch = (input, init) => + fetch(input, { + ...init, + signal: init?.signal + ? AbortSignal.any([signal, init.signal]) + : signal, + }); // Try Streamable HTTP first (current spec), fall back to legacy SSE. const client = new Client(CLIENT_INFO, { capabilities: {} }); + register(client); try { - const transport = new StreamableHTTPClientTransport(url, { requestInit }); - await client.connect(transport); + const transport = new StreamableHTTPClientTransport(url, { + requestInit, + fetch: fetchWithDeadline, + }); + await client.connect(transport, { signal }); return { client, usedSse: false }; } catch { + await client.close().catch(() => {}); + signal.throwIfAborted(); const sseClient = new Client(CLIENT_INFO, { capabilities: {} }); - const sseTransport = new SSEClientTransport(url, { requestInit }); - await sseClient.connect(sseTransport); + register(sseClient); + const sseTransport = new SSEClientTransport(url, { + requestInit, + fetch: fetchWithDeadline, + }); + await sseClient.connect(sseTransport, { signal }); return { client: sseClient, usedSse: true }; } } @@ -92,16 +138,94 @@ export async function connect( target: string, opts: ConnectOptions = {}, ): Promise { - const useStdio = opts.stdio || !isUrl(target); + const timeoutMs = opts.timeoutMs ?? 30_000; + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { + throw new Error("Connection timeout must be a positive finite number."); + } + const controller = new AbortController(); + const clients: Client[] = []; + // The SDK can close itself on initialization failure. Share that in-flight + // close promise so cleanup also waits for stdio child termination. + const register = (client: Client) => { + const close = client.close.bind(client); + let closing: Promise | undefined; + client.close = () => (closing ??= close()); + clients.push(client); + }; + let timer: ReturnType; + const deadline = new Promise((_, reject) => { + timer = setTimeout(() => { + const error = new Error(`Connection timed out after ${timeoutMs}ms.`); + error.name = "TimeoutError"; + controller.abort(error); + reject(error); + }, timeoutMs); + }); + const operation = connectTarget(target, opts, controller.signal, register, Date.now() + timeoutMs); + try { + const result = await Promise.race([operation, deadline]); + return { + ...result, + close: async () => { + controller.abort(); + await Promise.all(clients.map((client) => client.close().catch(() => {}))); + }, + }; + } catch (error) { + controller.abort(error); + await Promise.all(clients.map((client) => client.close().catch(() => {}))); + throw error; + } finally { + clearTimeout(timer!); + } +} + +async function connectTarget( + target: string, + opts: ConnectOptions, + signal: AbortSignal, + register: (client: Client) => void, + deadlineAt: number, +): Promise { + const useStdio = opts.stdio || opts.stdioArgs !== undefined || !isUrl(target); if (useStdio) { - const { command, args } = splitCommand(target); + const { command, args } = opts.stdioArgs + ? { command: opts.stdioArgs[0] ?? "", args: opts.stdioArgs.slice(1) } + : splitCommand(target); if (!command) throw new Error("Empty stdio command."); const client = new Client(CLIENT_INFO, { capabilities: {} }); + register(client); const transport = new StdioClientTransport({ command, args }); - await client.connect(transport); - - const tools = await listTools(client); + const closeTransport = transport.close.bind(transport); + transport.close = async () => { + // SDK close clears its public pid before waiting for child termination. + // Retain this owned child so cleanup shares the original deadline even + // when a catalog error has already aborted the connection signal. + const child = spawnedChild(transport); + const cleanupTimer = setTimeout(() => { + terminateChild(child); + }, Math.max(0, deadlineAt - Date.now())); + try { await closeTransport(); } finally { clearTimeout(cleanupTimer); } + }; + const stopTimedOutChild = () => { + // SDK close gives unresponsive children two grace periods. Once our + // deadline expires, terminate only the process this transport spawned. + if ( + signal.reason instanceof Error && + signal.reason.name === "TimeoutError" + ) { + terminateChild(spawnedChild(transport)); + } + }; + signal.addEventListener("abort", stopTimedOutChild, { once: true }); + let tools: ToolInfo[]; + try { + await client.connect(transport, { signal }); + tools = await listTools(client, signal); + } finally { + signal.removeEventListener("abort", stopTimedOutChild); + } const sv = client.getServerVersion(); const connection: ConnectionContext = { transport: "stdio", @@ -119,9 +243,10 @@ export async function connect( } const url = new URL(target); - const headers = await probeHeaders(url, opts.bearer); - const { client } = await connectHttp(url, opts); - const tools = await listTools(client); + const headers = await probeHeaders(url, opts.bearer, signal); + signal.throwIfAborted(); + const { client } = await connectHttp(url, opts, signal, register); + const tools = await listTools(client, signal); const sv = client.getServerVersion(); const connection: ConnectionContext = { @@ -141,17 +266,29 @@ export async function connect( }; } -async function listTools(client: Client): Promise { - try { - const res = await client.listTools(); - return (res.tools ?? []).map((t) => ({ +async function listTools(client: Client, signal: AbortSignal): Promise { + const tools: ToolInfo[] = []; + const cursors = new Set(); + let cursor: string | undefined; + do { + signal.throwIfAborted(); + const res = await client.listTools( + cursor === undefined ? undefined : { cursor }, + { signal }, + ); + tools.push(...res.tools.map((t) => ({ name: t.name, description: t.description, inputSchema: t.inputSchema, annotations: t.annotations as ToolInfo["annotations"], - })); - } catch { - // Server may not expose tools; treat as empty catalog. - return []; - } + }))); + cursor = res.nextCursor; + if (cursor !== undefined) { + if (cursors.has(cursor)) { + throw new Error("Server repeated a tools/list pagination cursor."); + } + cursors.add(cursor); + } + } while (cursor !== undefined); + return tools; } diff --git a/src/index.ts b/src/index.ts index 92c10cf..b18c012 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ import { renderJsonSafe, renderMarkdown, renderTerminal, + terminalText, worstSeverity, } from "./report.js"; import type { Finding, ScanContext } from "./types.js"; @@ -17,6 +18,7 @@ import type { Finding, ScanContext } from "./types.js"; interface Args { target?: string; stdio: boolean; + stdioArgs?: string[]; bearer?: string; format: "terminal" | "json" | "markdown"; out?: string; @@ -43,7 +45,11 @@ function parseArgs(argv: string[]): Args { break; case "--stdio": a.stdio = true; - break; + a.stdioArgs = argv.slice(i + 1); + // Preserve the original token boundaries for transport startup and + // include all arguments in the baseline/report identity. + a.target = a.stdioArgs.map((part) => JSON.stringify(part)).join(" "); + return a; case "--bearer": a.bearer = argv[++i]; break; @@ -68,8 +74,7 @@ function parseArgs(argv: string[]): Args { rest.push(arg); } } - // With --stdio, everything non-flag is the command + its args. - a.target = a.stdio ? rest.join(" ") : rest[0]; + a.target = rest[0]; return a; } @@ -90,6 +95,9 @@ OPTIONS --no-color Disable ANSI colors -h, --help Show this help +Place scanner options before --stdio. Every argument after --stdio is passed +unchanged to the server, including --help and other option-looking arguments. + EXIT CODES 0 no high/critical findings 1 at least one high or critical finding @@ -111,11 +119,12 @@ async function main(): Promise { try { result = await connect(args.target, { stdio: args.stdio, + stdioArgs: args.stdioArgs, bearer: args.bearer, }); } catch (err) { process.stderr.write( - `toolfence: failed to connect to "${args.target}": ${(err as Error).message}\n`, + `toolfence: failed to connect to "${terminalText(args.target)}": ${terminalText(String((err as Error).message))}\n`, ); return 2; } @@ -132,10 +141,12 @@ async function main(): Promise { }; const findings: Finding[] = []; + let checkFailed = false; for (const check of CHECKS) { try { findings.push(...(await check.run(ctx))); } catch (err) { + checkFailed = true; findings.push({ checkId: check.id, severity: "info", @@ -145,7 +156,7 @@ async function main(): Promise { } } - if (args.baseline) { + if (args.baseline && !checkFailed) { await saveBaseline(args.target, result.tools); } @@ -167,12 +178,13 @@ async function main(): Promise { if (args.out) { await writeFile(args.out, rendered); - process.stdout.write(`toolfence: report written to ${args.out}\n`); + process.stdout.write(`toolfence: report written to ${terminalText(args.out)}\n`); } else { process.stdout.write(rendered + "\n"); } const worst = worstSeverity(report); + if (checkFailed) return 2; return worst === "critical" || worst === "high" ? 1 : 0; } finally { await result.close().catch(() => {}); @@ -182,6 +194,6 @@ async function main(): Promise { main() .then((code) => process.exit(code)) .catch((err) => { - process.stderr.write(`toolfence: fatal: ${err?.stack ?? err}\n`); + process.stderr.write(`toolfence: fatal: ${terminalText(String(err?.stack ?? err))}\n`); process.exit(2); }); diff --git a/src/report.ts b/src/report.ts index 0d1664d..3975047 100644 --- a/src/report.ts +++ b/src/report.ts @@ -65,6 +65,17 @@ export function worstSeverity(r: ScanReport): Severity | null { return null; } +export function terminalText(value: string): string { + return value.replace(/[\u0000-\u001f\u007f-\u009f\u200b-\u200f\u202a-\u202e\u2060\u2066-\u2069\ufeff]/g, + char => `\\u${char.charCodeAt(0).toString(16).padStart(4, "0")}`); +} + +function markdownText(value: string): string { + return terminalText(value) + .replace(/&/g, "&").replace(//g, ">") + .replace(/[\\`*_{}[\]()#+.!|~-]/g, "\\$&"); +} + export function renderTerminal(r: ScanReport, color: boolean): string { const c = (s: Severity, text: string) => color ? `${COLOR[s]}${text}${RESET}` : text; @@ -73,9 +84,9 @@ export function renderTerminal(r: ScanReport, color: boolean): string { const lines: string[] = []; lines.push(""); lines.push(b(` toolfence — scan report`)); - lines.push(` target: ${r.target}`); + lines.push(` target: ${terminalText(r.target)}`); lines.push( - ` server: ${r.serverName ?? "unknown"}${r.serverVersion ? ` v${r.serverVersion}` : ""} (${r.transport})`, + ` server: ${terminalText(r.serverName ?? "unknown")}${r.serverVersion ? ` v${terminalText(r.serverVersion)}` : ""} (${r.transport})`, ); lines.push(` tools: ${r.toolCount}`); lines.push(""); @@ -88,10 +99,10 @@ export function renderTerminal(r: ScanReport, color: boolean): string { for (const f of r.findings) { lines.push( - ` ${c(f.severity, LABEL[f.severity])} ${b(f.title)}${f.tool ? ` ${c("info", `[${f.tool}]`)}` : ""}`, + ` ${c(f.severity, LABEL[f.severity])} ${b(terminalText(f.title))}${f.tool ? ` ${c("info", `[${terminalText(f.tool)}]`)}` : ""}`, ); - lines.push(` ${f.detail}`); - if (f.remediation) lines.push(` ${c("info", "→ " + f.remediation)}`); + lines.push(` ${terminalText(f.detail)}`); + if (f.remediation) lines.push(` ${c("info", "→ " + terminalText(f.remediation))}`); lines.push(""); } @@ -111,11 +122,11 @@ export function renderMarkdown(r: ScanReport): string { const lines: string[] = []; lines.push(`# toolfence scan report`); lines.push(""); - lines.push(`- **Target:** \`${r.target}\``); + lines.push(`- **Target:** ${markdownText(r.target)}`); lines.push( - `- **Server:** ${r.serverName ?? "unknown"}${r.serverVersion ? ` v${r.serverVersion}` : ""}`, + `- **Server:** ${markdownText(r.serverName ?? "unknown")}${r.serverVersion ? ` v${markdownText(r.serverVersion)}` : ""}`, ); - lines.push(`- **Transport:** ${r.transport}`); + lines.push(`- **Transport:** ${markdownText(r.transport)}`); lines.push(`- **Tools:** ${r.toolCount}`); lines.push(`- **Scanned:** ${r.scannedAt}`); lines.push(""); @@ -128,9 +139,9 @@ export function renderMarkdown(r: ScanReport): string { lines.push(`| Severity | Check | Tool | Finding |`); lines.push(`|---|---|---|---|`); for (const f of r.findings) { - const detail = f.detail.replace(/\|/g, "\\|"); + const detail = markdownText(f.detail); lines.push( - `| ${f.severity.toUpperCase()} | ${f.checkId} | ${f.tool ?? "—"} | ${detail} |`, + `| ${f.severity.toUpperCase()} | ${markdownText(f.checkId)} | ${markdownText(f.tool ?? "—")} | ${detail} |`, ); } lines.push(""); diff --git a/test/baseline.test.mjs b/test/baseline.test.mjs new file mode 100644 index 0000000..8b9975f --- /dev/null +++ b/test/baseline.test.mjs @@ -0,0 +1,50 @@ +import test, { after } from 'node:test'; +import os from 'node:os'; +import { syncBuiltinESMExports } from 'node:module'; +import assert from 'node:assert/strict'; +import { mkdtemp, readFile, writeFile, readdir, unlink, symlink, stat, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createHash } from 'node:crypto'; +import { loadBaseline, saveBaseline } from '../dist/baseline.js'; +const originalHomedir = os.homedir; +const fixtureHome = await mkdtemp(join(tmpdir(), 'toolfence-home-')); +os.homedir = () => fixtureHome; +syncBuiltinESMExports(); +after(async () => { + os.homedir = originalHomedir; + syncBuiltinESMExports(); + await rm(fixtureHome, { recursive: true, force: true }); +}); +const target = 'fixture:controlled'; +const filename = createHash('sha256').update(target).digest('hex').slice(0, 24) + '.json'; +test('baseline I/O uses explicit fixture directory, private atomic files, no symlink target writes', async () => { + const dir = await mkdtemp(join(tmpdir(), 'toolfence-baseline-')); + try { + await saveBaseline(target, [{ name: 'safe' }], dir); + assert.deepEqual(await readdir(dir), [filename]); + const file = join(dir, filename); + const original = await loadBaseline(target, dir); + assert.equal(original.target, target); + if (process.platform !== 'win32') assert.equal((await stat(file)).mode & 0o777, 0o600); + const marker = join(dir, 'marker'); + await writeFile(marker, 'keep original'); + await unlink(file); + await symlink(marker, file); + await saveBaseline(target, [{ name: 'new' }], dir); + assert.equal(await readFile(marker, 'utf8'), 'keep original'); + assert.ok(Object.hasOwn((await loadBaseline(target, dir)).toolFingerprints, 'new')); + } finally { await rm(dir, { recursive: true, force: true }); } +}); +test('corrupt or wrong-shaped baselines fail closed, missing baseline remains absent', async () => { + const dir = await mkdtemp(join(tmpdir(), 'toolfence-baseline-')); + try { + assert.equal(await loadBaseline(target, dir), undefined); + const file = join(dir, filename); + for (const data of ['{ broken', 'null', '{"createdAt":"x","target":"fixture","toolFingerprints":[]}']) { + await writeFile(file, data); + await assert.rejects(loadBaseline(target, dir)); + assert.equal(await readFile(file, 'utf8'), data); + } + } finally { await rm(dir, { recursive: true, force: true }); } +}); diff --git a/test/cli.test.mjs b/test/cli.test.mjs new file mode 100644 index 0000000..13038ca --- /dev/null +++ b/test/cli.test.mjs @@ -0,0 +1,51 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { mkdtemp, readFile, readdir } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const cli = fileURLToPath(new URL("../dist/index.js", import.meta.url)); +const fixture = fileURLToPath(new URL("./fixtures/stdio-server.mjs", import.meta.url)); +const preload = fileURLToPath(new URL("./fixtures/check-error.mjs", import.meta.url)); +async function run(args, options = {}) { + const child = spawn(process.execPath, args, { ...options, stdio: ["ignore", "pipe", "pipe"] }); + let stdout = "", stderr = ""; + child.stdout.on("data", (chunk) => stdout += chunk); + child.stderr.on("data", (chunk) => stderr += chunk); + const timeout = setTimeout(() => child.kill("SIGKILL"), 5000); + try { + const code = await new Promise((resolve, reject) => { child.once("error", reject); child.once("close", resolve); }); + return { code, stdout, stderr }; + } finally { clearTimeout(timeout); } +} + +test("CLI forwards every stdio argv exactly, including whitespace and scanner-like flags", async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-argv-")); + const output = join(scratch, "argv.json"); + const args = ["one two", "", "--help", "--json", "--bearer", "literal", "x'y\"z"]; + const result = await run([cli, "--no-baseline", "--json", "--stdio", process.execPath, fixture, "ok", output, ...args]); + assert.equal(result.code, 0, result.stderr); + assert.equal(JSON.parse(result.stdout).toolCount, 1); + assert.deepEqual(JSON.parse(await readFile(output, "utf8")), args); +}); + +test("CLI preserves partial report on unexpected check failure, exits 2, writes no baseline", async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-check-error-")); + const result = await run(["--import", preload, cli, "--json", "--stdio", process.execPath, fixture, "ok", "-"], { + env: { ...process.env, TOOLFENCE_TEST_BASELINE_DIR: scratch }, + }); + assert.equal(result.code, 2, result.stderr); + const report = JSON.parse(result.stdout); + assert.equal(report.toolCount, 1); + assert.ok(report.findings.some((finding) => finding.checkId === "fixture-error" && finding.detail === "fixture check failed")); + assert.deepEqual(await readdir(scratch), []); +}); + +test("CLI escapes remote control characters in discovery errors", async () => { + const result = await run([cli, "--no-baseline", "--stdio", process.execPath, fixture, "control-error", "-"]); + assert.equal(result.code, 2); + assert.equal(result.stderr.includes("\u001b"), false); + assert.ok(result.stderr.includes("\\u001b[2J")); +}); diff --git a/test/connect.test.mjs b/test/connect.test.mjs new file mode 100644 index 0000000..42a0aa6 --- /dev/null +++ b/test/connect.test.mjs @@ -0,0 +1,146 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { setTimeout as delay } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; +import { mkdtemp, readFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { connect } from "../dist/connect.js"; +import { serve } from "./fixtures/http-server.mjs"; + +const stdioFixture = fileURLToPath(new URL("./fixtures/stdio-server.mjs", import.meta.url)); + +test("HTTP catalog includes every page, exactly two list requests", async () => { + const server = await serve(); + try { + const result = await connect(server.url); + try { + assert.deepEqual(result.tools.map((tool) => tool.name), ["first_tool", "second_tool"]); + assert.equal(server.requests.filter((request) => request.rpc?.method === "tools/list").length, 2); + } finally { await result.close(); } + } finally { await server.close(); } +}); + +test("legacy SSE fallback completes handshake and pagination, closes stream", async () => { + const server = await serve({ mode: "sse" }); + try { + const result = await connect(server.url, { timeoutMs: 1000 }); + assert.deepEqual(result.tools.map((tool) => tool.name), ["first_tool", "second_tool"]); + await result.close(); + await delay(100); + assert.equal(server.activeResponses.size, 0); + assert.equal(server.requests.filter((request) => request.method === "POST" && request.url === "/mcp").length, 1); + } finally { await server.close(); } +}); + +test("valid empty catalog remains a successful scan", async () => { + const server = await serve({ mode: "empty" }); + try { + const result = await connect(server.url, { timeoutMs: 1000 }); + try { assert.deepEqual(result.tools, []); } + finally { await result.close(); } + } finally { await server.close(); } +}); + +for (const timeoutMs of [0, -1, NaN, Infinity]) { + test(`invalid timeout ${timeoutMs} fails before any network operation`, async () => { + const server = await serve(); + try { + await assert.rejects(connect(server.url, { timeoutMs }), /positive finite/); + assert.equal(server.requests.length, 0); + } finally { await server.close(); } + }); +} + +for (const mode of ["error", "invalid", "cycle"]) { + test(`HTTP ${mode} catalog fails closed and closes active transport sockets`, async () => { + const server = await serve({ mode }); + try { + await assert.rejects(connect(server.url, { timeoutMs: 500 })); + await delay(100); + assert.equal(server.activeResponses.size, 0, "no active response after catalog failure"); + const before = server.requests.length; + await delay(100); + assert.equal(server.requests.length, before, "no fallback or further requests after catalog failure"); + if (mode === "cycle") assert.equal(server.requests.filter((request) => request.rpc?.method === "tools/list").length, 2); + } finally { await server.close(); } + }); +} + +for (const mode of ["probe-hang", "init-hang", "sse-hang", "list-hang"]) { + test(`whole deadline cancels ${mode} under 1200ms, no further requests`, { timeout: 3000 }, async () => { + const server = await serve({ mode }); + try { + const started = performance.now(); + await assert.rejects(connect(server.url, { timeoutMs: 200 }), /timed out|timeout/i); + assert.ok(performance.now() - started < 1200); + await delay(100); + const before = server.requests.length; + await delay(200); + assert.equal(server.requests.length, before); + // Global fetch owns the idle keepalive pool. Only active transport + // requests must close, including the socket carrying the stalled body. + assert.equal(server.activeResponses.size, 0, "all hanging transport responses closed"); + } finally { await server.close(); } + }); +} + +test("deadline covers cumulative probe, initialize and paginated list time", async () => { + const server = await serve({ delay: 90 }); + try { await assert.rejects(connect(server.url, { timeoutMs: 200 }), /timed out|timeout/i); } + finally { await server.close(); } +}); + +for (const mode of ["error", "invalid"]) { + test(`stdio ${mode} catalog fails instead of returning empty success`, async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-stdio-")); + const output = join(scratch, "pid.json"); + await assert.rejects(connect("fixture", { stdioArgs: [process.execPath, stdioFixture, mode, output, "--record-pid"], timeoutMs: 500 })); + const { pid } = JSON.parse(await readFile(output, "utf8")); + assert.throws(() => process.kill(pid, 0), { code: "ESRCH" }); + }); +} + +test("stdio initialize timeout closes child process", async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-stdio-")); + const output = join(scratch, "pid.json"); + const started = performance.now(); + await assert.rejects(connect("fixture", { stdioArgs: [process.execPath, stdioFixture, "hang", output, "--record-pid"], timeoutMs: 600 }), /timed out|timeout/i); + assert.ok(performance.now() - started < 1200); + const { pid } = JSON.parse(await readFile(output, "utf8")); + assert.throws(() => process.kill(pid, 0), { code: "ESRCH" }); +}); + +test("deadline terminates owned child ignoring stdin and SIGTERM under 1200ms", async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-stdio-")); + const output = join(scratch, "pid.json"); + const started = performance.now(); + await assert.rejects(connect("fixture", { stdioArgs: [process.execPath, stdioFixture, "stubborn", output, "--record-pid"], timeoutMs: 600 }), /timed out|timeout/i); + assert.ok(performance.now() - started < 1200); + const { pid } = JSON.parse(await readFile(output, "utf8")); + assert.throws(() => process.kill(pid, 0), { code: "ESRCH" }); +}); + +test("deadline bounds cleanup after catalog failure from stubborn child under 1200ms", async () => { + const scratch = await mkdtemp(join(tmpdir(), "toolfence-stdio-")); + const output = join(scratch, "pid.json"); + const started = performance.now(); + await assert.rejects(connect("fixture", { stdioArgs: [process.execPath, stdioFixture, "stubborn-error", output, "--record-pid"], timeoutMs: 600 }), /failed|timeout/i); + assert.ok(performance.now() - started < 1200); + const { pid } = JSON.parse(await readFile(output, "utf8")); + assert.throws(() => process.kill(pid, 0), { code: "ESRCH" }); +}); + +test("header probe stops after five seconds and handshake uses remaining deadline", { timeout: 8000 }, async () => { + const server = await serve({ mode: "probe-hang" }); + try { + const started = performance.now(); + const result = await connect(server.url, { timeoutMs: 6500 }); + try { + assert.ok(performance.now() - started >= 4900); + assert.ok(performance.now() - started < 6500); + assert.equal(result.tools.length, 2); + assert.equal(result.connection.httpHeaders, undefined); + } finally { await result.close(); } + } finally { await server.close(); } +}); diff --git a/test/fixtures/check-error.mjs b/test/fixtures/check-error.mjs new file mode 100644 index 0000000..946006c --- /dev/null +++ b/test/fixtures/check-error.mjs @@ -0,0 +1,8 @@ +import os from "node:os"; +import { syncBuiltinESMExports } from "node:module"; +import { CHECKS } from "../../dist/checks/index.js"; + +// Only the fixture redirects storage. Production still uses its normal path. +os.homedir = () => process.env.TOOLFENCE_TEST_BASELINE_DIR; +syncBuiltinESMExports(); +CHECKS.push({ id: "fixture-error", title: "Fixture error", run() { throw new Error("fixture check failed"); } }); diff --git a/test/fixtures/http-server.mjs b/test/fixtures/http-server.mjs new file mode 100644 index 0000000..0934472 --- /dev/null +++ b/test/fixtures/http-server.mjs @@ -0,0 +1,75 @@ +import { createServer } from "node:http"; + +export async function serve({ mode = "pages", delay = 0 } = {}) { + const requests = []; + const sockets = new Set(); + const activeResponses = new Set(); + let sseResponse; + const server = createServer(async (req, res) => { + activeResponses.add(res); + const done = () => activeResponses.delete(res); + res.once("close", done); + res.once("finish", done); + requests.push({ method: req.method, url: req.url }); + if (req.method === "GET") { + if (mode === "probe-hang") return; + if (mode === "sse-hang") { + res.writeHead(200, { "Content-Type": "text/event-stream" }); + res.write(": fixture open\n\n"); + return; + } + if (mode === "sse") { + res.writeHead(200, { "Content-Type": "text/event-stream" }); + res.write("event: endpoint\ndata: /messages\n\n"); + sseResponse = res; + return; + } + res.writeHead(405).end(); + return; + } + let raw = ""; + for await (const chunk of req) raw += chunk; + const request = JSON.parse(raw); + requests.at(-1).rpc = request; + if (mode === "sse-hang" || (mode === "sse" && req.url === "/mcp")) { + res.writeHead(405).end(); + return; + } + if (request.id === undefined) { + res.writeHead(202).end(); + return; + } + if (mode === "init-hang" || (mode === "list-hang" && request.method === "tools/list")) return; + if (delay) await new Promise((resolve) => setTimeout(resolve, delay)); + let result; + let error; + if (request.method === "initialize") { + result = { protocolVersion: request.params.protocolVersion, capabilities: { tools: {} }, serverInfo: { name: "local-fixture", version: "1" } }; + } else if (request.method === "tools/list") { + if (mode === "error") error = { code: -32603, message: "fixture list failed" }; + else if (mode === "invalid") result = { tools: [{ name: 123 }] }; + else if (mode === "empty") result = { tools: [] }; + else if (mode === "cycle") result = { tools: [], nextCursor: "repeat" }; + else result = request.params?.cursor === "second" + ? { tools: [{ name: "second_tool", inputSchema: { type: "object" } }] } + : { tools: [{ name: "first_tool", inputSchema: { type: "object" } }], nextCursor: "second" }; + } + const response = JSON.stringify({ jsonrpc: "2.0", id: request.id, ...(error ? { error } : { result }) }); + if (mode === "sse") { + res.writeHead(202).end(); + sseResponse.write(`event: message\ndata: ${response}\n\n`); + } else { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(response); + } + }); + server.on("connection", (socket) => { + sockets.add(socket); + socket.once("close", () => sockets.delete(socket)); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + return { + url: `http://127.0.0.1:${server.address().port}/mcp`, requests, sockets, activeResponses, + close: () => new Promise((resolve) => { server.close(resolve); server.closeAllConnections(); }), + }; +} diff --git a/test/fixtures/stdio-server.mjs b/test/fixtures/stdio-server.mjs new file mode 100644 index 0000000..9a5b4c6 --- /dev/null +++ b/test/fixtures/stdio-server.mjs @@ -0,0 +1,32 @@ +import { createInterface } from "node:readline"; +import { writeFileSync } from "node:fs"; + +const [mode, output, ...args] = process.argv.slice(2); +if (output && output !== "-") writeFileSync(output, JSON.stringify(args[0] === "--record-pid" ? { pid: process.pid } : args)); +const lines = createInterface({ input: process.stdin }); +if (mode === "stubborn" || mode === "stubborn-error") { + process.on("SIGTERM", () => {}); + setInterval(() => {}, 1000); +} else lines.on("close", () => process.exit(0)); +lines.on("line", (line) => { + const request = JSON.parse(line); + if (request.id === undefined || mode === "hang" || mode === "stubborn") return; + let result; + if (request.method === "initialize") { + result = { + protocolVersion: request.params.protocolVersion, + capabilities: { tools: {} }, + serverInfo: { name: "local-fixture", version: "1" }, + }; + } else if (request.method === "tools/list") { + if (["error", "stubborn-error", "control-error"].includes(mode)) { + const message = mode === "control-error" ? "fixture\u001b[2J list failed" : "fixture list failed"; + process.stdout.write(JSON.stringify({ jsonrpc: "2.0", id: request.id, error: { code: -32603, message } }) + "\n"); + return; + } + result = mode === "invalid" ? { tools: [{ name: 123 }] } : { + tools: [{ name: "fixture_tool", inputSchema: { type: "object" } }], + }; + } + process.stdout.write(JSON.stringify({ jsonrpc: "2.0", id: request.id, result }) + "\n"); +}); diff --git a/test/reliability.test.mjs b/test/reliability.test.mjs new file mode 100644 index 0000000..aa36142 --- /dev/null +++ b/test/reliability.test.mjs @@ -0,0 +1,47 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { encode } from 'gpt-tokenizer'; +import { CHECKS, fingerprint } from '../dist/checks/index.js'; +import { buildReport, renderTerminal, renderMarkdown, renderJsonSafe } from '../dist/report.js'; +const context = tools => ({ connection: { transport: 'stdio', target: 'fixture', authProvided: false }, tools }); +const check = id => CHECKS.find(c => c.id === id); +test('literal special-token markers cannot bypass high catalog-cost findings', async () => { + const text = Array.from({ length: 60000 }, (_, i) => String.fromCharCode(0x4e00 + i % 2000)).join('') + '<|endoftext|>'; + const tools = [{ name: 'fixture', description: text }]; + const expected = encode(JSON.stringify(tools), { disallowedSpecial: new Set() }).length; + assert.ok(expected > 50000); + const findings = await check('context-cost').run(context(tools)); + assert.equal(findings[0].severity, 'high'); + assert.ok(findings[0].detail.includes(expected.toLocaleString())); +}); +test('safety annotation changes alter tool fingerprint', async () => { + const before = { name: 'fixture', annotations: { readOnlyHint: true } }; + const after = { name: 'fixture', annotations: { readOnlyHint: false, destructiveHint: true } }; + assert.notEqual(fingerprint(before), fingerprint(after)); + const findings = await check('tool-integrity').run({ ...context([after]), baseline: { target: 'fixture', createdAt: 'fixture', toolFingerprints: { fixture: fingerprint(before) } } }); + assert.ok(findings.some(f => f.severity === 'high')); +}); +test('inherited baseline properties are treated as newly observed tools', async () => { + const findings = await check('tool-integrity').run({ ...context([{ name: 'toString' }]), baseline: { target: 'fixture', createdAt: 'fixture', toolFingerprints: {} } }); + assert.ok(findings.some(f => /new tool/i.test(f.title))); + assert.ok(!findings.some(f => /changed/i.test(f.title))); +}); +test('sealed empty object schema does not claim arbitrary input acceptance', async () => { + const findings = await check('schema-strength').run(context([{ name: 'ping', inputSchema: { type: 'object', properties: {}, additionalProperties: false } }])); + assert.ok(!findings.some(f => /any input shape/i.test(f.detail))); +}); +test('report rendering escapes remote terminal controls and Markdown markup', () => { + const hostile = '\x1b[2J\u2060\n| forged | row |`'; + const report = buildReport(hostile, 'stdio', 1, [{ checkId: hostile, severity: 'high', title: hostile, tool: hostile, detail: hostile, remediation: hostile }], hostile, hostile); + for (const color of [true, false]) { + const rendered = renderTerminal(report, color); + assert.ok(!rendered.includes('\x1b[2J')); + assert.ok(!rendered.includes('\u2060')); + assert.ok(!rendered.includes('\n| forged')); + } + const markdown = renderMarkdown(report); + assert.ok(!markdown.includes('