From 13f31e2ec514708bdfe8410565bb46259dc072c2 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 26 Oct 2026 09:27:03 -0500 Subject: [PATCH] fix(evals): bound retained proof reads and credential exclusion Reject nonregular and oversized evidence without blocking. Share the safe reader across execution, archive and report. Exclude configured private keys and JSON config credentials, reject hidden nested tool definitions, and constrain retained attempt directory names. Co-Authored-By: Paperclip --- .../docs/runner-protocol-live-evals.md | 6 ++ .../scripts/controlled-evals/archive.mjs | 48 ++++++-------- .../controlled-evals/controller.test.mjs | 63 ++++++++++++++++++- .../controlled-evals/evidence-files.mjs | 28 +++++++++ .../scripts/controlled-evals/manifest.mjs | 2 +- .../scripts/controlled-evals/report.mjs | 8 ++- .../scripts/controlled-evals/run.mjs | 10 +-- tests/runner-e2e/eval-gateway.test.ts | 3 + tests/runner-e2e/eval-gateway.ts | 6 +- 9 files changed, 133 insertions(+), 41 deletions(-) create mode 100644 packages/paperclip-runner/scripts/controlled-evals/evidence-files.mjs diff --git a/packages/paperclip-runner/docs/runner-protocol-live-evals.md b/packages/paperclip-runner/docs/runner-protocol-live-evals.md index e73d4f28b8c..4c7733d1f6e 100644 --- a/packages/paperclip-runner/docs/runner-protocol-live-evals.md +++ b/packages/paperclip-runner/docs/runner-protocol-live-evals.md @@ -366,6 +366,12 @@ hashes. These detect corruption; they are not signatures or proof against a host that can rewrite all evidence. Known configured credential values are checked before encoding and after decoding. Keep bundles private. +Controller post-child reads, archive creation, and durable reports use the same +nonblocking, no-follow, byte-capped regular-file reader. Configured private/provider +keys and credentials inside JSON configuration contents are checked in both raw +and JSON-escaped form. Attempt IDs contain only letters, digits, `_`, `.`, and `-` +so each retained attempt has one directory name. + The reader accepts only bounded regular files and fixed evidence paths. A bundle is limited to 64 MiB of decoded content plus encoding overhead; this storage-format limit does not qualify a vendor charge or operating-cost bound. diff --git a/packages/paperclip-runner/scripts/controlled-evals/archive.mjs b/packages/paperclip-runner/scripts/controlled-evals/archive.mjs index fc328ee72a5..93dbcb017d1 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/archive.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/archive.mjs @@ -1,46 +1,38 @@ -import { constants } from 'node:fs'; -import { lstat, open, readdir, writeFile } from 'node:fs/promises'; +import { lstat, readdir, writeFile } from 'node:fs/promises'; import { basename, join, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; import { bindRequest, canonical, fields, readJson, requestBindingSha256, sha256, units, usd, validateManifest } from './manifest.mjs'; import { attemptDirectory, controlledResultArtifact } from './run.mjs'; import { summarizeControlledCampaign } from './report.mjs'; +import { FILE_LIMITS, RECEIPT, limitFor, readRegularFile } from './evidence-files.mjs'; const MiB = 1_048_576; const MAX_RAW_BYTES = 64 * MiB; const MAX_ARCHIVE_BYTES = 96 * MiB; -const FILE_LIMITS = new Map([ - ['admission.json', MiB], ['request.json', 8 * MiB], ['gateway.json', 8192], - ['ledger.jsonl', 16 * MiB], ['result.json', MiB], ['provider-artifact.json', 16 * MiB], - ['stdout.log', MiB], ['stderr.log', MiB], -]); -const RECEIPT = /^receipts\/[a-f0-9]{8}(?:-[a-f0-9]{4}){3}-[a-f0-9]{12}\.json$/; -const limitFor = name => FILE_LIMITS.get(name) ?? (RECEIPT.test(name) ? MiB : 0); function secretValues({ environment = process.env, secrets = [] } = {}) { - return [...new Set([...secrets, ...Object.entries(environment) - .filter(([name]) => /(?:API_KEY|TOKEN|SECRET|PASSWORD|ACCESS_KEY_ID)$/.test(name)) - .map(([, value]) => value)].filter(value => typeof value === 'string' && value.length > 0))]; + const configured = [...secrets, ...Object.entries(environment) + .filter(([name]) => /(?:KEY|TOKEN|SECRET|PASSWORD|ACCESS_KEY_ID|CONFIG_CONTENT)$/i.test(name)) + .map(([, value]) => value)].filter(value => typeof value === 'string' && value.length > 0); + for (const [name, value] of Object.entries(environment)) { + if (!/CONFIG_CONTENT$/i.test(name) || typeof value !== 'string') continue; + try { + const visit = item => { + if (!item || typeof item !== 'object') return; + for (const [key, entry] of Object.entries(item)) { + if (/(?:key|token|secret|password)$/i.test(key) && typeof entry === 'string' && entry) configured.push(entry); + else visit(entry); + } + }; + visit(JSON.parse(value)); + } catch {} + } + return [...new Set(configured.flatMap(value => [value, JSON.stringify(value).slice(1, -1)]))]; } function excludeSecrets(bytes, secrets) { if (secrets.some(secret => bytes.includes(Buffer.from(secret)))) throw new Error('Controller evidence contains a configured credential'); } -async function readRegularFile(path, maximum) { - const handle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); - try { - const stat = await handle.stat(); - if (!stat.isFile() || stat.size > maximum) throw new Error('Controller evidence is not a bounded regular file'); - const bytes = Buffer.alloc(stat.size + 1); - let length = 0; - while (length < bytes.length) { - const { bytesRead } = await handle.read(bytes, length, bytes.length - length, null); - if (!bytesRead) break; - length += bytesRead; - } - if (length !== stat.size) throw new Error('Controller evidence changed while being read'); - return bytes.subarray(0, length); - } finally { await handle.close(); } -} + async function realDirectory(path) { try { const stat = await lstat(path); diff --git a/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs b/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs index 820b24280d0..43dba81e415 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { mkdtemp, mkdir, readFile, readdir, rm, symlink, writeFile } from 'node:fs/promises'; +import { execFileSync } from 'node:child_process'; +import { mkdtemp, mkdir, readFile, readdir, rm, symlink, truncate, writeFile } from 'node:fs/promises'; import { writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -15,7 +16,9 @@ import assert from 'node:assert/strict'; import {createHash} from 'node:crypto'; import {spawn} from 'node:child_process'; import {once} from 'node:events'; -import {readFile,writeFile} from 'node:fs/promises'; +import {readFile,writeFile,rm,readdir,symlink,truncate} from 'node:fs/promises'; +import {dirname,join} from 'node:path'; +import {execFileSync} from 'node:child_process'; const args=new Map();for(let i=2;i { const f = await fixture(t, { count: 2 }); for (const mutate of [ + value => { value.attempts[0].attemptId = 'nested/attempt'; }, value => { value.maxUsd = '0.01'; }, value => { value.attempts[1].executionId = value.attempts[0].executionId; }, value => { value.attempts[0].provider = 'acpx'; value.attempts[0].acpxAgent = 'codex'; }, value => { delete value.overhead.components[0].provisionalBound; }, value => { value.models[0].secret = 'unexpected'; }, @@ -378,6 +392,51 @@ test('configured credentials are excluded before encoding and after archive deco await assert.rejects(readControlledEvidence(file, { environment }), /configured credential/); }); +for (const name of ['provider-artifact.json', 'receipt', 'ledger.jsonl']) for (const kind of ['fifo', 'symlink', 'oversized']) { + test(`post-child ${name} rejects ${kind} without blocking or releasing its allocation`, { timeout: 20_000 }, async t => { + const f = await fixture(t, { requests: 1, invalidEvidence: name, invalidFileType: kind }); + const result = await f.run(); + assert.equal(result.report.status, 'incomplete'); + assert.equal(result.report.accounting.knownUsd, '0.000068000'); + const summary = await summarizeControlledCampaign(f.manifest); + assert.equal(summary.incompleteAttempts, 1); + assert.equal(summary.committedUsd, '0.014000000'); + }); +} + +test('durable reports bound every evidence file and retain full holds for invalid proof', { timeout: 20_000 }, async t => { + const f = await fixture(t, { requests: 1 }); await f.run(); + const root = attemptDirectory(f.manifest, 'attempt-0'); + const receipt = `receipts/${(await readdir(join(root, 'receipts')))[0]}`; + for (const name of ['admission.json', 'request.json', 'result.json', 'provider-artifact.json', 'gateway.json', 'ledger.jsonl', receipt]) { + const path = join(root, name), original = await readFile(path); + for (const kind of ['fifo', 'symlink', 'oversized']) { + await rm(path); + if (kind === 'fifo') execFileSync('mkfifo', [path]); + else if (kind === 'symlink') await symlink('/dev/zero', path); + else { await writeFile(path, ''); await truncate(path, 16 * 1024 * 1024 + 1); } + const summary = await summarizeControlledCampaign(f.manifest); + assert.equal(summary.incompleteAttempts, 1, `${name}:${kind}`); + assert.equal(summary.committedUsd, '0.014000000', `${name}:${kind}`); + } + await rm(path); await writeFile(path, original); + } + assert.equal((await summarizeControlledCampaign(f.manifest)).incompleteAttempts, 0); +}); + +for (const name of ['PAPERCLIP_AGENT_PRIVATE_KEY', 'PAPERCLIP_AI_PROVIDER_KEY', 'AWS_SECRET_ACCESS_KEY', 'OPENCODE_CONFIG_CONTENT']) { + test(`archive excludes configured ${name} before and after encoding`, async t => { + const f = await fixture(t); await f.run(); + const secret = name === 'PAPERCLIP_AGENT_PRIVATE_KEY' ? '-----BEGIN PRIVATE KEY-----\nsynthetic-private-material\n-----END PRIVATE KEY-----' : 'synthetic-extra-credential'; + const environment = { [name]: name === 'OPENCODE_CONFIG_CONTENT' ? JSON.stringify({ provider: { apiKey: secret } }) : secret }; + const root = attemptDirectory(f.manifest, 'attempt-0'); + await writeFile(join(root, 'stderr.log'), JSON.stringify({ retained: secret })); + await assert.rejects(packControlledAttempt({ manifest: f.manifest, attemptId: 'attempt-0', environment }), /configured credential/); + const unchecked = await packControlledAttempt({ manifest: f.manifest, attemptId: 'attempt-0', environment: {} }); + assert.throws(() => decodeControlledEvidence(unchecked, { environment }), /configured credential/); + }); +} + test('retention annotations preserve an already-failed controller result and its known costs', async t => { const f = await fixture(t, { requests: 1, missingRoute: true }); await f.run(); const bundle = await packControlledAttempt({ manifest: f.manifest, attemptId: 'attempt-0' }); diff --git a/packages/paperclip-runner/scripts/controlled-evals/evidence-files.mjs b/packages/paperclip-runner/scripts/controlled-evals/evidence-files.mjs new file mode 100644 index 00000000000..b0aa4cfe485 --- /dev/null +++ b/packages/paperclip-runner/scripts/controlled-evals/evidence-files.mjs @@ -0,0 +1,28 @@ +import { constants } from 'node:fs'; +import { open } from 'node:fs/promises'; + +const MiB = 1_048_576; +export const FILE_LIMITS = new Map([ + ['admission.json', MiB], ['request.json', 8 * MiB], ['gateway.json', 8192], + ['ledger.jsonl', 16 * MiB], ['result.json', MiB], ['provider-artifact.json', 16 * MiB], + ['stdout.log', MiB], ['stderr.log', MiB], +]); +export const RECEIPT = /^receipts\/[a-f0-9]{8}(?:-[a-f0-9]{4}){3}-[a-f0-9]{12}\.json$/; +export const limitFor = name => FILE_LIMITS.get(name) ?? (RECEIPT.test(name) ? MiB : 0); + +export async function readRegularFile(path, maximum) { + const handle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + try { + const stat = await handle.stat(); + if (!stat.isFile() || stat.size > maximum) throw new Error('Controller evidence is not a bounded regular file'); + const bytes = Buffer.alloc(stat.size + 1); + let length = 0; + while (length < bytes.length) { + const { bytesRead } = await handle.read(bytes, length, bytes.length - length, null); + if (!bytesRead) break; + length += bytesRead; + } + if (length !== stat.size) throw new Error('Controller evidence changed while being read'); + return bytes.subarray(0, length); + } finally { await handle.close(); } +} diff --git a/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs b/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs index 0ca1bb655d0..ba1070a4b06 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs @@ -72,7 +72,7 @@ export function validateManifest(value, now = Date.now(), { admission = true } = const provider = attempt.provider === 'codex' && attempt.acpxAgent === undefined ? 'openai' : attempt.provider === 'acpx' && attempt.acpxAgent === 'claude' ? 'anthropic' : null; const reservation = prices.get(`${provider}:${attempt.model}`); - if (!provider || !id(attempt.executionId) || !id(attempt.attemptId) || !hash(attempt.requestSha256) + if (!provider || !id(attempt.executionId) || !id(attempt.attemptId) || !/^[A-Za-z0-9_.-]+$/.test(attempt.attemptId) || !hash(attempt.requestSha256) || attempts.has(attempt.attemptId) || executions.has(attempt.executionId) || reservation === undefined || !Number.isSafeInteger(attempt.timeoutMs) || attempt.timeoutMs < 1 || attempt.timeoutMs > 4 * 3_600_000 || !Number.isSafeInteger(attempt.maxRequests) || attempt.maxRequests < 1 || attempt.maxRequests > 1000 diff --git a/packages/paperclip-runner/scripts/controlled-evals/report.mjs b/packages/paperclip-runner/scripts/controlled-evals/report.mjs index 4dc48445cf4..c7cb1664347 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/report.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/report.mjs @@ -1,8 +1,8 @@ -import { readFile } from 'node:fs/promises'; import { join, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; import { canonical, costRuntime, readJson, requestBindingSha256, sha256, units, usd, validateManifest } from './manifest.mjs'; import { attemptDirectory } from './run.mjs'; +import { limitFor, readRegularFile } from './evidence-files.mjs'; export async function summarizeControlledCampaign(value, { filesByAttempt } = {}) { const { manifest, manifestSha256, allocatedUsd, overheadUsd } = validateManifest(value, Date.now(), { admission: false }); @@ -11,9 +11,11 @@ export async function summarizeControlledCampaign(value, { filesByAttempt } = {} for (const allocation of manifest.attempts) { const root = attemptDirectory(manifest, allocation.attemptId); const readEvidence = async name => { - if (filesByAttempt === undefined) return readFile(join(root, name)); + const limit = limitFor(name); + if (!limit) throw new Error('Unexpected controller evidence path'); + if (filesByAttempt === undefined) return readRegularFile(join(root, name), limit); const bytes = filesByAttempt.get(allocation.attemptId)?.get(name); - if (!Buffer.isBuffer(bytes)) throw new Error('Missing archived controller evidence'); + if (!Buffer.isBuffer(bytes) || bytes.length > limit) throw new Error('Missing archived controller evidence'); return bytes; }; const readDocument = async (name, maximum = 1_048_576) => { diff --git a/packages/paperclip-runner/scripts/controlled-evals/run.mjs b/packages/paperclip-runner/scripts/controlled-evals/run.mjs index cef1f272b0d..921896180c7 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/run.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/run.mjs @@ -4,6 +4,7 @@ import { lstat, mkdir, open, readFile, realpath, rename, rm, writeFile } from 'n import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; import { pathToFileURL } from 'node:url'; import { parseArgs } from 'node:util'; +import { limitFor, readRegularFile } from './evidence-files.mjs'; import { bindRequest, canonical, costRuntime, readJson, sha256, validateManifest } from './manifest.mjs'; async function atomicJson(file, value) { @@ -137,8 +138,7 @@ registerHooks({ resolve(specifier, context, nextResolve) { child = await runChild({ cli: stagedCli, loader, requestPath: retainedRequestPath, artifactPath, descriptorPath, cwd: request.session.workingDirectory ?? root, environment: childEnvironment, deadlineAt, signal, closeGateway: () => gateway.close() }); for (const stream of ['stdout', 'stderr']) await writeFile(join(root, `${stream}.log`), redact(child[stream]), { flag: 'wx', mode: 0o600 }); - const raw = await readFile(artifactPath, 'utf8'); - if (Buffer.byteLength(raw) > 16_777_216) throw new Error('Provider artifact exceeds its byte limit'); + const raw = (await readRegularFile(artifactPath, limitFor('provider-artifact.json'))).toString('utf8'); secretExcluded = secrets.every(secret => !raw.includes(secret)); if (!secretExcluded) await writeFile(artifactPath, redact(raw), { mode: 0o600 }); providerArtifactSha256 = sha256(redact(raw)); @@ -166,9 +166,11 @@ registerHooks({ resolve(specifier, context, nextResolve) { if (snapshot.overBudget || snapshot.conflictingRequests || snapshot.inflight || snapshot.unknownRequests !== snapshot.boundedUnresolvedRequests) failure ??= 'request_accounting_unbounded'; const receiptHashes = {}; try { - for (const request of snapshot.requests) receiptHashes[request.requestId] = sha256(await readFile(join(root, 'receipts', `${request.requestId}.json`))); + for (const request of snapshot.requests) receiptHashes[request.requestId] = sha256(await readRegularFile(join(root, 'receipts', `${request.requestId}.json`), limitFor(`receipts/${request.requestId}.json`))); } catch { failure ??= 'request_receipt_missing'; } - const ledgerSha256 = sha256(await readFile(ledgerPath)); + let ledgerSha256; + try { ledgerSha256 = sha256(await readRegularFile(ledgerPath, limitFor('ledger.jsonl'))); } + catch { failure ??= 'ledger_evidence_invalid'; } const report = { schema: 'paperclip.runner-eval.control-result/v1', campaignId: manifest.campaignId, manifestSha256, executionId: attempt.executionId, attemptId: attempt.attemptId, requestSha256: attempt.requestSha256, allocationUsd: attempt.maxUsd, limits, startedAt, finishedAt: new Date().toISOString(), status: failure ? 'incomplete' : 'closed', failure, diff --git a/tests/runner-e2e/eval-gateway.test.ts b/tests/runner-e2e/eval-gateway.test.ts index aefca15a9e3..c4c9e785b8d 100644 --- a/tests/runner-e2e/eval-gateway.test.ts +++ b/tests/runner-e2e/eval-gateway.test.ts @@ -139,6 +139,9 @@ it.each([ { type: "namespace", name: "local", tools: [{ type: "web_search" }] }, { type: "namespace", name: "local", tools: [{ type: "namespace", name: "nested", tools: [{ type: "web_search" }] }] }, { type: "namespace", name: "local" }, + { type: "namespace", name: "local", tools: [{ type: "custom", name: "x", tools: [{ type: "web_search" }] }] }, + { type: "namespace", name: "local", tools: [{ type: "function", name: "x", tools: [{ type: "web_search" }] }] }, + { type: "custom", name: "x", tools: [{ type: "web_search" }] }, ])("rejects unqualified tools inside namespaces before reservation", async tool => { const f = await fixture(), transport = vi.fn(); await expect(forwardEvalRequest({ ...f, payload: { ...f.payload, tools: [tool] }, transport })).rejects.toThrow(); diff --git a/tests/runner-e2e/eval-gateway.ts b/tests/runner-e2e/eval-gateway.ts index d7cc20ab0db..6d42ce680e5 100644 --- a/tests/runner-e2e/eval-gateway.ts +++ b/tests/runner-e2e/eval-gateway.ts @@ -72,11 +72,11 @@ function boundedPayload(value: unknown, route: EvalGatewayRoute) { if (body.speed !== undefined && body.speed !== "standard" || body.inference_geo !== undefined) throw new EvalRequestError("unqualified_billing_mode"); if (body.tools !== undefined && (!Array.isArray(body.tools) || body.tools.some(tool => { const definition = row(tool), type = definition.type; - if (route.provider !== "openai") return type !== undefined && type !== "custom"; + if (route.provider !== "openai") return type !== undefined && type !== "custom" || Object.hasOwn(definition, "tools"); if (type === "namespace") return typeof definition.name !== "string" || !definition.name || !Array.isArray(definition.tools) || !definition.tools.length - || definition.tools.some(nested => !["function", "custom"].includes(String(row(nested).type))); - return !["function", "custom"].includes(String(type)); + || definition.tools.some(nested => !["function", "custom"].includes(String(row(nested).type)) || Object.hasOwn(row(nested), "tools")); + return !["function", "custom"].includes(String(type)) || Object.hasOwn(definition, "tools"); }))) throw new EvalRequestError("unqualified_hosted_tool"); const inspect = (item: unknown): void => { if (Array.isArray(item)) { item.forEach(inspect); return; }