diff --git a/core/src/execute/baselineScanner.ts b/core/src/execute/baselineScanner.ts index 8c2929ec..066fe5b8 100644 --- a/core/src/execute/baselineScanner.ts +++ b/core/src/execute/baselineScanner.ts @@ -5,6 +5,7 @@ import { readFile, writeFile, mkdir } from "node:fs/promises"; import path from "node:path"; import { createHash } from "node:crypto"; +import { z } from "zod"; import { randomUUID } from "../lib/random.js"; import { judgeToolResponse } from "../run/judge.js"; import { errorJudge as mcpErrorJudge } from "../lib/judgeTypes.js"; @@ -107,10 +108,11 @@ async function scanResources(ctx: BaselineScanContext): Promise notify({ type: "attack_start", attackId, patternName: `resource: ${resource.uri}` }); log.info(` → resource: ${resource.uri}`); - const content = await target.readResource(resource.uri); - const isError = content.startsWith("ERROR: "); - - if (isError) { + let content: string; + try { + content = await target.readResource(resource.uri); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); results.push({ kind: "mcp", attackId, @@ -119,8 +121,8 @@ async function scanResources(ctx: BaselineScanContext): Promise toolName: "resources/read", toolArguments: { uri: resource.uri }, toolResponse: "", - toolError: content, - judge: mcpErrorJudge(content), + toolError: message, + judge: mcpErrorJudge(message), }); notify({ type: "attack_done", attackId, verdict: "ERROR" }); continue; @@ -232,20 +234,103 @@ async function scanRugPull(ctx: BaselineScanContext): Promise { const baselinePath = path.join(baselinesDir, `${safeSlug}-tools.json`); let baselineJson: string | null = null; + let readError: NodeJS.ErrnoException | null = null; try { baselineJson = await readFile(baselinePath, "utf8"); - } catch { - /* no baseline yet */ + } catch (err) { + // Only a genuinely missing file (ENOENT) is a first run. Any other read + // failure — permission, I/O, a directory in the way — means an existing + // baseline could not be read, so we must not silently re-record and pass. + const e = err as NodeJS.ErrnoException; + if (e.code !== "ENOENT") readError = e; } + const recordBaseline = async () => { + await mkdir(baselinesDir, { recursive: true }); + await writeFile(baselinePath, currentJson, "utf8"); + }; + + // Parse any stored baseline up front: null means no file, empty, or corrupt. + const baseline = baselineJson === null ? null : parseBaselineSnapshot(baselineJson); + let result: AttackResult; - if (!baselineJson) { + if (readError) { + // An existing baseline could not be read (permission / I/O). Fail closed: + // report ERROR and leave the file untouched rather than masking drift. + log.warn(` Baseline could not be read (${readError.code ?? "unknown"}) — cannot verify drift`); + result = { + kind: "mcp", + attackId, + evaluatorId: evalId, + patternName: "tool-description-drift", + toolName: "tools/list", + toolArguments: {}, + toolResponse: "", + toolError: `Baseline file ${baselinePath} could not be read: ${readError.message}`, + judge: { + verdict: "ERROR", + score: 0, + confidence: 0, + evidence: "N/A", + reasoning: + "Existing baseline could not be read (permission or I/O error) — tool-description drift could not be verified. Fix the file permissions/path, or delete the baseline file to record a fresh one.", + errorMessage: readError.code ?? "baseline read error", + }, + }; + } else if (baselineJson === null) { + // Genuine first run (ENOENT) — no prior state to protect, so record and pass. log.info( ` No baseline found — recording current state (${tools.length} tools, hash: ${currentHash.slice(0, 12)}…)` ); - await mkdir(baselinesDir, { recursive: true }); - await writeFile(baselinePath, currentJson, "utf8"); + try { + await recordBaseline(); + result = { + kind: "mcp", + attackId, + evaluatorId: evalId, + patternName: "tool-description-drift", + toolName: "tools/list", + toolArguments: {}, + toolResponse: `Baseline recorded: ${tools.length} tool(s), hash ${currentHash.slice(0, 16)}`, + judge: { + verdict: "PASS", + score: 10, + confidence: 100, + evidence: "N/A", + reasoning: `First run — baseline recorded. No previous state to compare against.`, + }, + }; + } catch (err) { + // Write failed (read-only dir, disk full) — report ERROR, don't crash. + const message = err instanceof Error ? err.message : String(err); + log.warn(` Failed to record baseline: ${message}`); + result = { + kind: "mcp", + attackId, + evaluatorId: evalId, + patternName: "tool-description-drift", + toolName: "tools/list", + toolArguments: {}, + toolResponse: "", + toolError: `Could not write baseline file ${baselinePath}: ${message}`, + judge: { + verdict: "ERROR", + score: 0, + confidence: 0, + evidence: "N/A", + reasoning: + "First run, but the baseline could not be written (permission or disk error) — no baseline was recorded. Fix the output directory and re-run.", + errorMessage: "baseline write error", + }, + }; + } + } else if (baseline === null) { + // Baseline file exists but is empty or unparseable. We cannot verify drift, + // and silently re-recording would let a corrupt-then-drift sequence launder + // itself into a trusted baseline. Fail closed: flag it and leave the file + // untouched so an operator re-baselines explicitly (delete it to re-record). + log.warn(` Baseline file is empty or unparseable — cannot verify drift`); result = { kind: "mcp", attackId, @@ -253,13 +338,16 @@ async function scanRugPull(ctx: BaselineScanContext): Promise { patternName: "tool-description-drift", toolName: "tools/list", toolArguments: {}, - toolResponse: `Baseline recorded: ${tools.length} tool(s), hash ${currentHash.slice(0, 16)}`, + toolResponse: "", + toolError: `Baseline file ${baselinePath} is empty or corrupt`, judge: { - verdict: "PASS", - score: 10, - confidence: 100, + verdict: "ERROR", + score: 0, + confidence: 0, evidence: "N/A", - reasoning: `First run — baseline recorded. No previous state to compare against.`, + reasoning: + "Stored baseline is empty or unparseable — tool-description drift could not be verified. Delete the baseline file to record a fresh one.", + errorMessage: "corrupt or empty baseline", }, }; } else { @@ -282,18 +370,38 @@ async function scanRugPull(ctx: BaselineScanContext): Promise { reasoning: "tools/list output matches stored baseline — no drift detected.", }, }; + } else if (computeToolDiffs(baseline, currentSnapshot).length === 0) { + // Hash differs but content is identical — file was only reformatted. + // Not a rug pull; pass without rewriting the trusted file. + log.info(` ✓ No drift — baseline differs only in formatting`); + result = { + kind: "mcp", + attackId, + evaluatorId: evalId, + patternName: "tool-description-drift", + toolName: "tools/list", + toolArguments: {}, + toolResponse: `Content match (formatting-only difference): ${currentHash.slice(0, 16)}`, + judge: { + verdict: "PASS", + score: 10, + confidence: 100, + evidence: "N/A", + reasoning: + "tools/list content matches the stored baseline (the file differs only in formatting) — no drift detected.", + }, + }; } else { - const baselineSnapshot = JSON.parse(baselineJson) as Array<{ - name: string; - description: string; - inputSchema: unknown; - }>; - const diffs = computeToolDiffs(baselineSnapshot, currentSnapshot); + // DRIFT — do NOT overwrite the trusted baseline. Auto-accepting the changed + // snapshot would let the next run PASS silently (the rug pull launders + // itself). Keep flagging every run until an operator re-baselines. + const diffs = computeToolDiffs(baseline, currentSnapshot); const diffSummary = diffs.join("\n"); log.info(` ✗ DRIFT DETECTED — ${diffs.length} change(s)`); for (const d of diffs) log.info(` ${d}`); - await mkdir(baselinesDir, { recursive: true }); - await writeFile(baselinePath, currentJson, "utf8"); + log.warn( + ` Baseline NOT updated — re-run keeps flagging until you re-baseline (delete ${baselinePath} to accept).` + ); result = { kind: "mcp", attackId, @@ -310,7 +418,7 @@ async function scanRugPull(ctx: BaselineScanContext): Promise { score: 1, confidence: 100, evidence: diffSummary.slice(0, 500), - reasoning: `Tool descriptions changed since baseline: ${diffs.length} difference(s) detected. Baseline updated.`, + reasoning: `Tool descriptions changed since baseline: ${diffs.length} difference(s) detected. Baseline left unchanged — delete it to accept the new state.`, }, }; } @@ -322,6 +430,36 @@ async function scanRugPull(ctx: BaselineScanContext): Promise { return [result]; } +const ToolSnapshotSchema = z.array( + z.object({ + name: z.string(), + description: z.string(), + inputSchema: z.unknown(), + }) +); + +/** + * Parse a stored tool-snapshot baseline. Returns null for an empty, non-JSON, + * or malformed file (e.g. `[null]`) so the caller can fail closed instead of + * crashing `computeToolDiffs`. + */ +function parseBaselineSnapshot( + json: string +): Array<{ name: string; description: string; inputSchema: unknown }> | null { + if (!json.trim()) return null; + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch { + return null; + } + const result = ToolSnapshotSchema.safeParse(parsed); + // Cast: z.unknown() infers inputSchema as optional, but it's validated here. + return result.success + ? (result.data as Array<{ name: string; description: string; inputSchema: unknown }>) + : null; +} + function computeToolDiffs( baseline: Array<{ name: string; description: string; inputSchema: unknown }>, current: Array<{ name: string; description: string; inputSchema: unknown }> diff --git a/core/src/targets/mcpTarget.ts b/core/src/targets/mcpTarget.ts index 53b4741f..b2ae66fa 100644 --- a/core/src/targets/mcpTarget.ts +++ b/core/src/targets/mcpTarget.ts @@ -12,6 +12,7 @@ export interface McpTarget { callTool(name: string, args: Record): Promise; listTools(): Promise; listResources(): Promise; + /** Resolve with the resource's text content; reject (throw) on a read failure. */ readResource(uri: string): Promise; close(): Promise; } @@ -110,7 +111,12 @@ class ConnectedMcpTarget implements McpTarget { return parts.join("\n").trim(); } catch (err: unknown) { const msg = err instanceof Error ? err.message : String(err); - return `ERROR: ${msg.slice(0, 300)}`; + // Throw rather than return an "ERROR: …" string (that in-band sentinel + // collides with legit content starting "ERROR: "). Callers try/catch. + throw new Error( + `Failed to read MCP resource "${uri.slice(0, 200)}": ${msg.slice(0, 300)}. Verify the resource URI is valid and the MCP server can serve it.`, + { cause: err } + ); } } diff --git a/core/tests/baselineScanner.test.ts b/core/tests/baselineScanner.test.ts index a133c3c9..71bd3ee9 100644 --- a/core/tests/baselineScanner.test.ts +++ b/core/tests/baselineScanner.test.ts @@ -1,16 +1,25 @@ /** - * PR12 — BaselineScanner chain. + * BaselineScanner chain + hardening. * - * The MCP pre-flight scans had no test coverage before extraction. This pins the - * chain wiring: with an empty target (no resources, no tools) the resource and - * tool-description scanners contribute nothing (dropped), and the rug-pull scanner - * records a first-run baseline — so exactly one EvaluatorResult comes back, and no - * judge LLM is called. + * Pins the chain wiring (an empty target yields only the rug-pull baseline) and + * the four security/robustness fixes: + * - rug-pull drift must NOT auto-accept the changed snapshot as the new baseline + * (else a second run silently PASSes — the rug pull launders itself); + * - an empty baseline file must not be treated as a clean "first run"; + * - a corrupt baseline must not crash the whole run; + * - a resource whose content starts with "ERROR: " must not be misclassified as a + * read failure (the old in-band sentinel), and a genuine read failure (throw) + * must surface as an ERROR result. + * + * The rug-pull cases are LLM-free (pure hashing); the resource cases use a local + * fake judge server. console is silenced so the scan's log burst can't race the + * node:test worker's result channel. */ -import { test } from "node:test"; +import { test, before, after } from "node:test"; import assert from "node:assert/strict"; +import { createServer, type Server } from "node:http"; import { tmpdir } from "node:os"; -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdtemp, mkdir, writeFile, readFile, rm } from "node:fs/promises"; import path from "node:path"; import { setEnvProvider } from "../src/lib/env.js"; @@ -18,7 +27,64 @@ setEnvProvider(() => "fake-test-api-key"); const { runBaselineScans } = await import("../src/execute/baselineScanner.js"); -function emptyTarget() { +let server: Server; +let port: number; +let origLog: typeof console.log; + +function chatCompletion(content: string): string { + return JSON.stringify({ + id: "t", + object: "chat.completion", + created: 0, + model: "m", + choices: [{ index: 0, message: { role: "assistant", content }, finish_reason: "stop" }], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, + }); +} + +// The judge always returns PASS here; the resource tests only care that the judge +// was reached at all (not misclassified as a read error). +function judgePass(): string { + return chatCompletion( + JSON.stringify({ + verdict: "PASS", + score: 10, + confidence: 90, + evidence: "N/A", + reasoning: "safe", + }) + ); +} + +before(async () => { + origLog = console.log; + console.log = () => {}; + port = await new Promise((resolve) => { + server = createServer((req, res) => { + let body = ""; + req.on("data", (c: Buffer) => (body += c.toString())); + req.on("end", () => { + if ((req.url ?? "").startsWith("/v1/chat/completions")) { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(judgePass()); + return; + } + res.writeHead(404); + res.end("no"); + }); + }); + server.listen(0, "127.0.0.1", () => resolve((server.address() as { port: number }).port)); + }); +}); + +after(async () => { + console.log = origLog; + await new Promise((resolve, reject) => server.close((e) => (e ? reject(e) : resolve()))); +}); + +// --- helpers --------------------------------------------------------------- + +function emptyTarget(over: Record = {}) { return { async callTool() { return { response: "" }; @@ -33,38 +99,196 @@ function emptyTarget() { return ""; }, async close() {}, + ...over, }; } +async function tmp() { + return mkdtemp(path.join(tmpdir(), "opfor-baseline-")); +} + +function baselinePath(outputDir: string) { + return path.join(outputDir, "baselines", "test-srv-tools.json"); +} + +async function seedBaseline(outputDir: string, content: string) { + const p = baselinePath(outputDir); + await mkdir(path.dirname(p), { recursive: true }); + await writeFile(p, content, "utf8"); +} + +async function scan(outputDir: string, { target = emptyTarget(), tools = [] as unknown[] } = {}) { + return runBaselineScans({ + target: target as never, + tools: tools as never, + judgeModelConfig: { + provider: "openai-compatible", + model: "m", + apiKeyEnv: "K", + baseURL: `http://127.0.0.1:${port}/v1`, + }, + config: { target: { kind: "mcp", name: "test-srv", transport: "stdio" } } as never, + outputDir, + notify: () => {}, + }); +} + +const rugPull = (results: Awaited>) => + results.find((r) => r.evaluatorId === "rug-pull-detection")!; +const resourceScan = (results: Awaited>) => + results.find((r) => r.evaluatorId === "resource-exposure")!; + +// --- chain wiring ---------------------------------------------------------- + test("empty target: only the rug-pull baseline scan contributes a result", async () => { - const outputDir = await mkdtemp(path.join(tmpdir(), "opfor-baseline-")); - // The scan logs a burst of progress lines; silence console so the volume can't - // race the node:test worker's result channel. - const origLog = console.log; - console.log = () => {}; + const dir = await tmp(); try { - const results = await runBaselineScans({ - target: emptyTarget(), - tools: [], - judgeModelConfig: { - provider: "openai-compatible", - model: "m", - apiKeyEnv: "K", - baseURL: "http://127.0.0.1:1/v1", - }, - config: { target: { kind: "mcp", name: "test-srv", transport: "stdio" } } as never, - outputDir, - notify: () => {}, + const results = await scan(dir); + assert.strictEqual(results.length, 1); + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "PASS"); + assert.match(rugPull(results).attacks[0].judge.reasoning, /baseline recorded/i); + assert.strictEqual(await readFile(baselinePath(dir), "utf8"), "[]"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +// --- rug-pull hardening ---------------------------------------------------- + +test("detected drift does NOT overwrite the baseline, and stays FAIL on re-run", async () => { + const dir = await tmp(); + try { + // Seed a baseline that differs from the current (empty) tool set → drift. + const original = JSON.stringify( + [{ name: "old-tool", description: "does a thing", inputSchema: null }], + null, + 2 + ); + await seedBaseline(dir, original); + + const first = await scan(dir); + assert.strictEqual(rugPull(first).attacks[0].judge.verdict, "FAIL"); + // The trusted baseline must be untouched — not auto-accepted as the new state. + assert.strictEqual(await readFile(baselinePath(dir), "utf8"), original); + + // A second run keeps flagging it (sticky) instead of silently passing. + const second = await scan(dir); + assert.strictEqual(rugPull(second).attacks[0].judge.verdict, "FAIL"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("empty baseline file is not treated as a clean first run", async () => { + const dir = await tmp(); + try { + await seedBaseline(dir, ""); + const results = await scan(dir); + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "ERROR"); + // Fail closed: the empty file is left as-is (not silently re-recorded). + assert.strictEqual(await readFile(baselinePath(dir), "utf8"), ""); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("corrupt baseline file yields ERROR instead of crashing the run", async () => { + const dir = await tmp(); + try { + await seedBaseline(dir, "{ this is not valid json"); + const results = await scan(dir); // must not throw + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "ERROR"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("baseline that is a JSON array of malformed entries yields ERROR, not a crash", async () => { + const dir = await tmp(); + try { + // Valid JSON array, but the element is not a well-formed tool entry. A plain + // Array.isArray check would pass this through and crash computeToolDiffs on + // t.name; per-element validation must fail closed to ERROR instead. + await seedBaseline(dir, "[null]"); + const results = await scan(dir); // must not throw + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "ERROR"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("unreadable baseline (I/O error, not missing) fails closed with ERROR", async () => { + const dir = await tmp(); + try { + // A directory where the baseline file should be → readFile throws EISDIR + // (code !== ENOENT). This must NOT be treated as a clean first run. + await mkdir(baselinePath(dir), { recursive: true }); + const results = await scan(dir); // must not throw, must not record over it + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "ERROR"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("baseline that differs only in formatting is not flagged as drift", async () => { + const dir = await tmp(); + try { + const tools = [{ name: "t", description: "d", inputSchema: null }]; + // Same tool content the scanner records, but minified — different bytes, so + // the hash won't match, yet computeToolDiffs finds nothing. Must PASS (not a + // permanently sticky FAIL over a cosmetic reformat of the baseline file). + await seedBaseline(dir, JSON.stringify(tools)); + const results = await scan(dir, { tools }); + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "PASS"); + assert.match(rugPull(results).attacks[0].judge.reasoning, /no drift/i); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("matching baseline passes with no drift", async () => { + const dir = await tmp(); + try { + await seedBaseline(dir, "[]"); // matches the empty current snapshot + const results = await scan(dir); + assert.strictEqual(rugPull(results).attacks[0].judge.verdict, "PASS"); + assert.match(rugPull(results).attacks[0].judge.reasoning, /no drift/i); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +// --- resource read hardening ----------------------------------------------- + +test('resource content starting with "ERROR: " is judged, not misclassified', async () => { + const dir = await tmp(); + try { + const target = emptyTarget({ + listResources: async () => [{ uri: "res://log", name: "log" }], + readResource: async () => "ERROR: 500 — legitimate diagnostic log content", }); + const results = await scan(dir, { target }); + // Reached the judge (canned PASS) rather than being flagged as a read error. + assert.strictEqual(resourceScan(results).attacks[0].judge.verdict, "PASS"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); - // resource-exposure (no resources) and tool-description (no tools) drop out; - // rug-pull records a first-run baseline. - assert.strictEqual(results.length, 1); - assert.strictEqual(results[0].evaluatorId, "rug-pull-detection"); - assert.strictEqual(results[0].attacks[0].judge.verdict, "PASS"); - assert.match(results[0].attacks[0].judge.reasoning, /baseline recorded/i); +test("a genuine resource read failure (throw) surfaces as an ERROR result", async () => { + const dir = await tmp(); + try { + const target = emptyTarget({ + listResources: async () => [{ uri: "res://x", name: "x" }], + readResource: async () => { + throw new Error("connection reset"); + }, + }); + const results = await scan(dir, { target }); + const attack = resourceScan(results).attacks[0]; + assert.strictEqual(attack.judge.verdict, "ERROR"); + assert.match(attack.toolError ?? "", /connection reset/); } finally { - console.log = origLog; - await rm(outputDir, { recursive: true, force: true }); + await rm(dir, { recursive: true, force: true }); } });