From 507dc70be26c191e372f09e58dc0965e73bf8f57 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:43:58 -0500 Subject: [PATCH] fix(evals): bind hosted controller admission to the first attempt (#17523) Bind hosted controller admission to the repository, GitHub run, first attempt, workflow revision and selected execution before credential reads or provider launch. Keep offline evidence replay independent of the current worker environment. Preserve the single-project boundary editor and refresh project-access data with agent-detail updates. Verified head: 1e588f75fea3d61ef84ff1217e58743818d307e1. Validation: 74 synthetic source/portable controller tests, 98 focused UI tests and token gates passed before the ancestry-only final sync; four hosted/portable regressions pass again with unchanged eval and UI repair trees. Final exact-head full CI passed (40387856225), and Greptile reviewed this head at 5/5 with zero unresolved findings. The optional secondary Paperclip Review is paused and not counted as completed. No paid work or schedule changes. Co-Authored-By: Paperclip --- .../docs/runner-protocol-live-evals.md | 21 +++++++ .../scripts/controlled-evals/archive.mjs | 5 +- .../controlled-evals/controller.test.mjs | 63 +++++++++++++++++++ .../scripts/controlled-evals/manifest.mjs | 30 ++++++++- .../controlled-evals/portable.test.mjs | 6 +- .../scripts/controlled-evals/run.mjs | 3 +- ui/src/components/TrustPresetSection.test.tsx | 19 +++++- ui/src/components/TrustPresetSection.tsx | 2 +- ui/src/context/LiveUpdatesProvider.test.ts | 18 ++++++ ui/src/lib/queryKeys.ts | 2 +- 10 files changed, 160 insertions(+), 9 deletions(-) diff --git a/packages/paperclip-runner/docs/runner-protocol-live-evals.md b/packages/paperclip-runner/docs/runner-protocol-live-evals.md index 14640e2baa5..521fa237041 100644 --- a/packages/paperclip-runner/docs/runner-protocol-live-evals.md +++ b/packages/paperclip-runner/docs/runner-protocol-live-evals.md @@ -316,6 +316,27 @@ The manifest schema is `paperclip.runner-eval.controller/v1`. It contains: `nativeLimits`, and a reviewed full-lifecycle `provisionalBound`. When absent, the controller starts no exporter or OTLP relay. +For GitHub workers, the manifest must also contain `hosted` with `repository`, +`runId` (a decimal string), `runAttempt: 1`, and `workflowRevision` (the run's +`GITHUB_SHA`, including the merge commit for a pull-request run). Its `campaignId` must be `gha--1`. Before creating +an admission record, reading provider credentials, or starting a proxy, both +`archive.mjs verify` and `run.mjs` compare this identity with the current +`GITHUB_ACTIONS`, `GITHUB_REPOSITORY`, `GITHUB_RUN_ID`, `GITHUB_RUN_ATTEMPT` and +`GITHUB_SHA`. Set `PAPERCLIP_RUNNER_EVAL_EXECUTION_ID` from the worker's selected +cell. It must equal the bound allocation's `executionId`. A reused preparation +job cannot authorize a later GitHub run attempt. Local manifests remain usable +outside a GitHub context. A hosted manifest cannot execute with missing context. + +This checks the trusted workflow's current environment; it is not a distributed +lock or signed GitHub attestation. The workflow must prepare one complete manifest, +distribute identical bytes and fan out each selected cell exactly once. The +private `prepare_controlled_campaign.py` command prepares requests with the +existing case compiler and rejects duplicate or missing matrix cells. Independent +budgets, a second workflow run, or duplicate first-attempt jobs are not authorized +by this guard. Pack and report commands remain usable offline after the run, so +expired or failed attempts can still be retained and reconciled. The current +live workflow has not yet been connected to this contract. + Use `requestBindingSha256` from `controlled-evals/manifest.mjs` to bind the complete compiled request. It excludes only the temporary working directory and native executable path; the executable digest remains bound and its path diff --git a/packages/paperclip-runner/scripts/controlled-evals/archive.mjs b/packages/paperclip-runner/scripts/controlled-evals/archive.mjs index ec246123490..58acc6b0a5a 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/archive.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/archive.mjs @@ -1,7 +1,7 @@ import { isEntrypoint } from './entrypoint.mjs'; import { lstat, readdir, writeFile } from 'node:fs/promises'; import { basename, join, resolve } from 'node:path'; -import { bindRequest, canonical, fields, readJson, requestBindingSha256, sha256, units, usd, validateManifest } from './manifest.mjs'; +import { assertHostedAdmission, 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'; @@ -174,7 +174,8 @@ if (await isEntrypoint(import.meta.url)) { const [command, ...args] = process.argv.slice(2); if (command === 'verify' && args.length === 2) { const { manifest } = validateManifest(await readJson(args[0])); - bindRequest(manifest, await readJson(args[1], 8 * MiB)); + const attempt = bindRequest(manifest, await readJson(args[1], 8 * MiB)); + assertHostedAdmission(manifest, attempt, process.env); } else if (command === 'pack' && args.length === 3) { const bundle = await packControlledAttempt({ manifest: await readJson(args[0]), attemptId: args[1] }); await writeFile(args[2], JSON.stringify(bundle) + '\n', { flag: 'wx', mode: 0o600 }); diff --git a/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs b/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs index b3362a8031b..0a23b4d036a 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/controller.test.mjs @@ -1,4 +1,6 @@ import assert from 'node:assert/strict'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; import { test } from 'node:test'; import { execFileSync } from 'node:child_process'; import { mkdtemp, mkdir, open, readFile, readdir, rm, stat, symlink, truncate, writeFile } from 'node:fs/promises'; @@ -706,6 +708,66 @@ test('cancellation during root export stops forwarding and cannot close the atte assert.equal(summary.committedUsd, '0.014000000'); assert.equal(summary.knownUsd, '0.000068000'); }); + +test('hosted workers reject reruns and identity drift before requests, while retained evidence replays offline', async t => { + const f = await fixture(t); + f.manifest.campaignId = 'gha-12345-1'; + f.manifest.hosted = { repository: 'paperclipai/paperclip', runId: '12345', runAttempt: 1, workflowRevision: 'c'.repeat(40) }; + await f.save(); + const context = { GITHUB_ACTIONS: 'true', GITHUB_REPOSITORY: 'paperclipai/paperclip', GITHUB_RUN_ID: '12345', + GITHUB_RUN_ATTEMPT: '1', GITHUB_SHA: 'c'.repeat(40), PAPERCLIP_RUNNER_EVAL_EXECUTION_ID: 'execution-0' }; + let calls = 0; + const transport = async () => { calls++; return success(); }; + for (const [key, value] of Object.entries(context)) { + for (const wrong of [undefined, key === 'GITHUB_RUN_ATTEMPT' ? '2' : 'wrong']) { + Object.assign(f.environment, context, { [key]: wrong }); + await assert.rejects(f.run(0, transport), /current first-attempt worker/); + assert.equal(calls, 0); + await assert.rejects(readdir(f.manifest.outputDirectory), { code: 'ENOENT' }); + } + f.environment[key] = value; + } + Object.assign(f.environment, context); + const command = [fileURLToPath(new URL('./archive.mjs', runtime)), 'verify', f.manifestPath, f.requests[0].requestPath]; + const verify = promisify(execFile); + await verify(process.execPath, command, { env: { PATH: process.env.PATH, ...context }, timeout: 15000 }); + await assert.rejects(verify(process.execPath, command, { + env: { PATH: process.env.PATH, ...context, GITHUB_RUN_ATTEMPT: '2' }, timeout: 15000, + }), /current first-attempt worker/); + assert.equal(calls, 0); + assert.equal((await f.run(0, transport)).code, 0); + assert.equal(calls, 2); + const bundle = await packControlledAttempt({ manifest: f.manifest, attemptId: 'attempt-0', environment: f.environment }); + const replay = await summarizeControlledEvidence([bundle], { environment: {} }); + assert.equal(replay.incompleteAttempts, 0); + assert.equal(replay.campaigns[0].campaignId, 'gha-12345-1'); +}); + +test('hosted context requires a manifest identity and rejects invalid or later-attempt manifests', async t => { + const f = await fixture(t); + let calls = 0; + const transport = async () => { calls++; return success(); }; + for (const key of ['GITHUB_ACTIONS', 'GITHUB_REPOSITORY', 'GITHUB_RUN_ID', 'GITHUB_RUN_ATTEMPT', 'GITHUB_SHA']) { + f.environment[key] = key === 'GITHUB_SHA' ? 'c'.repeat(40) : 'true'; + await assert.rejects(f.run(0, transport), /requires a hosted controller identity/); + delete f.environment[key]; + } + const hosted = { repository: 'paperclipai/paperclip', runId: '12345', runAttempt: 1, workflowRevision: 'c'.repeat(40) }; + f.manifest.campaignId = 'gha-12345-1'; + for (const change of [{ repository: '' }, { repository: 'paperclip' }, { runId: 12345 }, { runId: '012345' }, + { runAttempt: 2 }, { workflowRevision: 'master' }, { workflowRevision: ['c'.repeat(40)] }, { unexpected: true }]) { + f.manifest.hosted = { ...hosted, ...change }; + assert.throws(() => validateManifest(f.manifest), /hosted controller identity|controller fields/); + } + f.manifest.hosted = hosted; + await f.save(); + await assert.rejects(f.run(0, transport), /current first-attempt worker/); + assert.equal(calls, 0); + await assert.rejects(readdir(f.manifest.outputDirectory), { code: 'ENOENT' }); + f.manifest.campaignId = 'gha-12345-2'; + assert.throws(() => validateManifest(f.manifest), /hosted controller identity/); +}); + for (const name of ['observability.json', 'otlp-native.jsonl', 'otlp-controller.jsonl']) for (const kind of ['fifo', 'symlink', 'oversized']) { test(`native post-child ${name} rejects ${kind} and retains model cost plus the full hold`, { timeout: 20_000 }, async t => { const f = await fixture(t, { requests: 1, observability: true, invalidEvidence: name, invalidFileType: kind }); @@ -756,6 +818,7 @@ test('missing provider artifacts cannot bypass immutable result binding', async malformed.files['result.json'] = { sizeBytes: bytes.length, sha256: sha256(bytes), base64: bytes.toString('base64') }; await writeFile(join(directory, 'controller-evidence.json'), JSON.stringify(malformed)); await assert.rejects(readAttemptControlledEvidence(directory), /Malformed immutable controller result proof/); + }); diff --git a/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs b/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs index 60f184af35d..d9ad2da83d3 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/manifest.mjs @@ -32,7 +32,7 @@ export async function readJson(file, maximum = 1_048_576) { } export function validateManifest(value, now = Date.now(), { admission = true } = {}) { fields(value, ['schema', 'campaignId', 'maxUsd', 'deadlineAt', 'pricingVerifiedAt', 'pricingExpiresAt', 'overhead', - 'sourceRevision', 'definitionsRevision', 'runnerCli', 'runnerPackage', 'runnerd', 'outputDirectory', 'route', 'projectId', 'models', 'attempts', 'observability']); + 'sourceRevision', 'definitionsRevision', 'runnerCli', 'runnerPackage', 'runnerd', 'outputDirectory', 'route', 'projectId', 'models', 'attempts', 'observability', 'hosted']); const deadline = Date.parse(value.deadlineAt); const verified = Date.parse(value.pricingVerifiedAt), expires = Date.parse(value.pricingExpiresAt); if (value.schema !== 'paperclip.runner-eval.controller/v1' || !id(value.campaignId) @@ -46,6 +46,7 @@ export function validateManifest(value, now = Date.now(), { admission = true } = fields(artifact, ['path', 'sha256']); if (!isAbsolute(artifact.path ?? '') || !hash(artifact.sha256)) throw new Error('Invalid controller executable provenance'); } + validateHostedIdentity(value); const overhead = costRuntime.validateOperatingOverhead(value.overhead, value.maxUsd); if (admission && Date.parse(value.overhead.verifiedAt) > now || Date.parse(value.overhead.expiresAt) <= deadline || value.overhead.components.some(component => !component.provisionalBound)) throw new Error('Controller requires current full-lifecycle operating bounds'); @@ -82,6 +83,33 @@ export function validateManifest(value, now = Date.now(), { admission = true } = validateObservability(value); return { manifest: structuredClone(value), manifestSha256: sha256(canonical(value)), allocatedUsd: usd(allocated), overheadUsd: usd(overhead) }; } +function validateHostedIdentity(manifest) { + const hosted = manifest.hosted; + if (hosted === undefined) return; + fields(hosted, ['repository', 'runId', 'runAttempt', 'workflowRevision']); + if (typeof hosted.repository !== 'string' || !/^[A-Za-z0-9][A-Za-z0-9_.-]{0,99}\/[A-Za-z0-9][A-Za-z0-9_.-]{0,99}$/.test(hosted.repository) + || typeof hosted.runId !== 'string' || !/^[1-9][0-9]{0,19}$/.test(hosted.runId) + || hosted.runAttempt !== 1 || typeof hosted.workflowRevision !== 'string' || !/^[a-f0-9]{40}$/.test(hosted.workflowRevision) + || manifest.campaignId !== `gha-${hosted.runId}-1`) throw new Error('Invalid hosted controller identity'); +} + +export function assertHostedAdmission(manifest, attempt, environment) { + validateHostedIdentity(manifest); + const hosted = manifest.hosted; + if (!hosted) { + if (['GITHUB_ACTIONS', 'GITHUB_REPOSITORY', 'GITHUB_RUN_ID', 'GITHUB_RUN_ATTEMPT', 'GITHUB_SHA'].some(key => environment[key] !== undefined)) { + throw new Error('GitHub execution requires a hosted controller identity'); + } + return; + } + if (environment.GITHUB_ACTIONS !== 'true' || environment.GITHUB_REPOSITORY !== hosted.repository + || environment.GITHUB_RUN_ID !== hosted.runId || environment.GITHUB_RUN_ATTEMPT !== '1' + || environment.GITHUB_SHA !== hosted.workflowRevision + || environment.PAPERCLIP_RUNNER_EVAL_EXECUTION_ID !== attempt.executionId) { + throw new Error('Hosted controller admission does not match the current first-attempt worker'); + } +} + export function bindRequest(manifest, request) { const attempt = manifest.attempts.find(item => item.attemptId === request.attemptId); if (!attempt || requestBindingSha256(request) !== attempt.requestSha256 || request.provider !== attempt.provider diff --git a/packages/paperclip-runner/scripts/controlled-evals/portable.test.mjs b/packages/paperclip-runner/scripts/controlled-evals/portable.test.mjs index c2b970a36f0..39db05341e6 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/portable.test.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/portable.test.mjs @@ -75,11 +75,13 @@ test('copied controller runs without source dependencies, retains native evidenc console.log('stdin imports are side-effect free'); `); assert.match((await stdin).stdout, /stdin imports are side-effect free/); - const cases = 'real child requests|disjoint concurrent|controller owns native|cancellation during root export|native post-child otlp-native.jsonl rejects fifo|credential redaction replaces|missing provider artifacts|failed final OTLP receipt'; + const cases = 'real child requests|disjoint concurrent|controller owns native|cancellation during root export|native post-child otlp-native.jsonl rejects fifo|credential redaction replaces|missing provider artifacts|hosted workers reject reruns|hosted context requires a manifest|failed final OTLP receipt'; const behavioral = await exec(process.execPath, ['--test', '--test-reporter=tap', '--test-name-pattern', cases, join(source, 'controller.test.mjs')], { cwd: root, env: { ...environment, PAPERCLIP_CONTROLLER_TEST_RUNTIME: join(directory, 'controlled-evals') }, timeout: 80_000, maxBuffer: 1_048_576, }); - assert.match(behavioral.stdout, /# pass 9\b/); + assert.match(behavioral.stdout, /# pass 11\b/); + + const cli = await exec(process.execPath, [join(directory, 'controlled-evals/archive.mjs'), 'report-attempts', join(root, 'empty-attempts')], { cwd: root, env: environment, timeout: 10_000, }).then(value => value, error => error); diff --git a/packages/paperclip-runner/scripts/controlled-evals/run.mjs b/packages/paperclip-runner/scripts/controlled-evals/run.mjs index 5f303e093ee..8125e6594b8 100644 --- a/packages/paperclip-runner/scripts/controlled-evals/run.mjs +++ b/packages/paperclip-runner/scripts/controlled-evals/run.mjs @@ -7,7 +7,7 @@ import { pathToFileURL } from 'node:url'; import { parseArgs } from 'node:util'; import { startControlledObservability } from './observability.mjs'; import { limitFor, readRegularFile, replaceEvidenceFile } from './evidence-files.mjs'; -import { bindRequest, canonical, costRuntime, readJson, sha256, validateManifest } from './manifest.mjs'; +import { assertHostedAdmission, bindRequest, canonical, costRuntime, readJson, sha256, validateManifest } from './manifest.mjs'; async function atomicJson(file, value) { const temporary = `${file}.tmp`; @@ -69,6 +69,7 @@ export async function runControlledEval({ manifestPath, requestPath, outputPath, signal?.throwIfAborted(); const { manifest, manifestSha256 } = validateManifest(await readJson(manifestPath)); const request = await readJson(requestPath, 8_388_608), attempt = bindRequest(manifest, request); + assertHostedAdmission(manifest, attempt, environment); try { await lstat(outputPath); throw new Error('Controlled output already exists'); } catch (error) { if (error.code !== 'ENOENT') throw error; } if (request.session.workingDirectory) { diff --git a/ui/src/components/TrustPresetSection.test.tsx b/ui/src/components/TrustPresetSection.test.tsx index 9d6b57d29e7..7a8951015c9 100644 --- a/ui/src/components/TrustPresetSection.test.tsx +++ b/ui/src/components/TrustPresetSection.test.tsx @@ -11,7 +11,7 @@ import { TooltipProvider } from "@/components/ui/tooltip"; let root: Root | null = null; let container: HTMLDivElement | null = null; -function renderSection(permissions: Partial) { +function renderSection(permissions: Partial, withProjectEditor = false) { container = document.createElement("div"); document.body.appendChild(container); const onChange = vi.fn(); @@ -21,6 +21,7 @@ function renderSection(permissions: Partial) { createdRoot.render( Allowed projects : undefined} permissions={permissions} onChange={onChange} companyId="company-1" @@ -72,6 +73,22 @@ describe("TrustPresetSection", () => { expect(view.text()).not.toContain("Managed by EE/API"); }); + it("keeps the single-boundary controls when the allowed-project editor is present", () => { + const view = renderSection({ trustPreset: "low_trust_review", authorizationPolicy: { + trustPreset: "low_trust_review", trustBoundary: { + mode: "low_trust_review", companyId: "company-1", projectIds: ["project-1"], + }, + } }, true); + expect(view.text()).toContain("Allowed projects"); + expect(view.text()).toContain("Boundary type"); + const clear = [...container!.querySelectorAll("button")].find(button => button.textContent === "Clear boundary"); + expect(clear).toBeDefined(); + flushSync(() => clear!.click()); + const changed = view.onChange.mock.calls[0][0]; + expect(changed.trustPreset).toBe("low_trust_review"); + expect(changed.authorizationPolicy.trustBoundary.projectIds).toBeUndefined(); + }); + it("renders multi-boundary policies as read-only", () => { const view = renderSection({ canCreateAgents: false, diff --git a/ui/src/components/TrustPresetSection.tsx b/ui/src/components/TrustPresetSection.tsx index cae599bbdf6..383a24fd40e 100644 --- a/ui/src/components/TrustPresetSection.tsx +++ b/ui/src/components/TrustPresetSection.tsx @@ -150,7 +150,7 @@ export function TrustPresetSection({ : "This agent is set to low-trust review, but no project, root issue, or issue scope is set in the core policy. Add a scope before this agent can run without denial."}

- {projectAccessEditor && (boundary?.projectIds?.length ?? 0) > 0 ? null : boundaryEditable ? ( + {projectAccessEditor && (boundary?.projectIds?.length ?? 0) > 1 ? null : boundaryEditable ? (
diff --git a/ui/src/context/LiveUpdatesProvider.test.ts b/ui/src/context/LiveUpdatesProvider.test.ts index 3fc05666db8..539b3207d2a 100644 --- a/ui/src/context/LiveUpdatesProvider.test.ts +++ b/ui/src/context/LiveUpdatesProvider.test.ts @@ -16,6 +16,24 @@ import { __liveUpdatesTestUtils } from "./LiveUpdatesProvider"; import { queryKeys } from "../lib/queryKeys"; describe("LiveUpdatesProvider issue invalidation", () => { + it("refreshes saved project access with permission saves and live agent activity", async () => { + const client = new QueryClient(); + const target = queryKeys.agents.projectAccess("agent-1"); + const other = queryKeys.agents.projectAccess("agent-2"); + client.setQueryData(target, { projectIds: [] }); + client.setQueryData(other, { projectIds: ["other-project"] }); + await client.invalidateQueries({ queryKey: queryKeys.agents.detail("agent-1") }); + expect(client.getQueryState(target)?.isInvalidated).toBe(true); + expect(client.getQueryState(other)?.isInvalidated).toBe(false); + client.setQueryData(target, { projectIds: ["project-1"] }); + __liveUpdatesTestUtils.invalidateActivityQueries(client, "company-1", { + entityType: "agent", entityId: "agent-1", action: "agent.permissions_updated", + }, { userId: "user-1", agentId: null }); + expect(client.getQueryState(target)?.isInvalidated).toBe(true); + expect(client.getQueryState(other)?.isInvalidated).toBe(false); + client.clear(); + }); + it('refreshes sources, legacy repositories and installed skills after a background import', () => { const client = new QueryClient(); const invalidate = vi.spyOn(client, 'invalidateQueries'); diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index 26dafb622df..1cb6fcd79e1 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -222,7 +222,7 @@ export const queryKeys = { ["team-catalog", "installed", companyId] as const, }, agents: { - projectAccess: (id: string) => ["agents", "project-access", id] as const, + projectAccess: (id: string) => ["agents", "detail", id, "project-access"] as const, identity: (id: string) => ["agents", "identity", id] as const, list: (companyId: string) => ["agents", companyId] as const, detail: (id: string) => ["agents", "detail", id] as const,