From 26ecf5b2cc26fb676fe4dbeaf48efceefeee5306 Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:19:32 -0700 Subject: [PATCH] Publish approved benchmark definitions Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- docs/README.md | 2 +- docs/results.md | 13 +++++--- scripts/publication.test.ts | 60 ++++++++++++++++++------------------- scripts/publish-ci.ts | 38 ++++------------------- 4 files changed, 46 insertions(+), 67 deletions(-) diff --git a/docs/README.md b/docs/README.md index 21397d3..bacf43e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -126,6 +126,6 @@ Benchmark checks ignore unrelated labels and title or body edits. * Benchmark Publication uses `contents: write` and `statuses: write`. It commits to `data` as `github-actions[bot]` and restricts write paths in code. * Pages handles PR metadata with `pull_request_target` and executes only `main` code. * Pages uses `pages: write` and `id-token: write`. Configure Pages for GitHub Actions and restrict the `github-pages` environment to `main`. -* Publishers validate PR artifacts as untrusted input. Validation and successful job topology do not prove measurements are genuine. Publishers do not execute artifact scripts or restore PR caches. +* Publishers validate PR artifacts as untrusted input. Artifact validation and a successful workflow do not prove measurements are genuine. Publishers do not execute artifact scripts or restore PR caches. * PRs can change workflows and check scripts. Inspect those changes before allowing execution or merging. Self-hosted runners must be disposable and isolated from credentials and sensitive networks. * Preview JavaScript shares production's origin and browser storage. Separate-origin hosting is required for browser isolation. \ No newline at end of file diff --git a/docs/results.md b/docs/results.md index 20a2d26..4b3ec46 100644 --- a/docs/results.md +++ b/docs/results.md @@ -235,12 +235,17 @@ The default policy comes from `shared/catalog.ts`. Use `--policy policy.json` when the measured revision requires another trusted policy. Never accept a publication policy supplied by untrusted PR code. +CI archival builds its policy from the approved artifact's catalog and benchmark +definitions plus the trusted runner SKUs from `main`. This permits approved +definition changes while retaining host validation. CI does not accept a +standalone policy from the artifact. + These are local consistency checks, not proof that a PR merged. A trusted CI job must obtain GitHub merge metadata, verify the approved PR revisions and -workflow attempt, verify required checks and skip eligibility, and obtain the -actual merged tree before invoking promotion. PR code must not control this -job's scripts or provenance inputs. `scripts/publish-ci.ts` performs these GitHub -checks from the trusted publication workflow. +workflow attempt, verify artifact completeness and skip eligibility, and obtain +the actual merged tree before invoking promotion. PR code must not control this +job's scripts or provenance inputs. `scripts/publish-ci.ts` performs these +GitHub checks from the trusted publication workflow. CI retains the trusted publication policy under `policies//.json` and a pending pointer under `pending/pr-.json`. Pending pointers do not diff --git a/scripts/publication.test.ts b/scripts/publication.test.ts index 94979f3..f83ce63 100644 --- a/scripts/publication.test.ts +++ b/scripts/publication.test.ts @@ -481,6 +481,15 @@ test('CI archival and promotion with simulated GitHub and Git', async context => } } writeJson(eventPath, notification, false) const bundle = fixture() + const sourceRuntime = bundle.catalog.runtimes[0]! + const addedRuntimeId = 'approved-runtime' + bundle.catalog.runtimes.push({ ...sourceRuntime, id: addedRuntimeId }) + bundle.benchmark.expectedConfigurations.push(...bundle.benchmark.expectedConfigurations + .filter(configuration => configuration.runtimeId === sourceRuntime.id) + .map(configuration => ({ ...configuration, runtimeId: addedRuntimeId }))) + bundle.measurements.push(...bundle.measurements + .filter(measurement => measurement.runtimeId === sourceRuntime.id) + .map(measurement => ({ ...measurement, id: `approved-${measurement.id}`, runtimeId: addedRuntimeId }))) const pr = { number: 7, base: { ref: 'main', repo: { full_name: repository }, sha: bundle.run.pullRequest!.base }, head: { sha: bundle.run.pullRequest!.head }, labels: [] as { name: string }[], changed_files: 1, @@ -488,15 +497,8 @@ test('CI archival and promotion with simulated GitHub and Git', async context => } let mergedTree = bundle.run.commit.tree let workflowHead = pr.head.sha - let failedJob = false let pushes = 0 let downloads = 0 - const jobs = ['workload / eligibility', 'workload / configure', 'workload / producer', - 'workload / collect', 'workload / Workload Status', 'Benchmark Status', - ...Array.from({ length: 2 }, (_, index) => `workload / prepare (${index})`), - ...Array.from({ length: 36 }, (_, index) => `workload / measure (${index})`), - ].map(name => ({ name, conclusion: 'success' })) - let retryJobs: typeof jobs = [] const statuses: any[] = [] context.mock.method(globalThis, 'fetch', async (url: string, options?: RequestInit) => { const parsed = new URL(url) @@ -510,12 +512,6 @@ test('CI archival and promotion with simulated GitHub and Git', async context => statuses.push(status) return Response.json(status) } - if (path.endsWith('/jobs')) { - const page = Number(parsed.searchParams.get('page')) - const response = structuredClone(path.includes('/attempts/2/') ? retryJobs : jobs) - if (failedJob && !path.includes('/attempts/2/')) response[0]!.conclusion = 'failure' - return Response.json({ jobs: response.slice((page - 1) * 100, page * 100), total_count: response.length }) - } const responses: Record = { 'pulls/7': pr, [`commits/${pr.merge_commit_sha}/pulls`]: [pr], @@ -554,7 +550,10 @@ test('CI archival and promotion with simulated GitHub and Git', async context => assert.equal(readFileSync(outputPath, 'utf8'), 'changed=true\n') assert.equal(downloads, 1) assert.deepEqual(readJson(resolve(temporary, 'data-store/index.json')), { schemaVersion: 1, runs: [] }) - assert.ok(existsSync(resolve(temporary, 'data-store/policies/12345/1.json'))) + const policy = readJson(resolve(temporary, 'data-store/policies/12345/1.json')) as any + assert.deepEqual(policy.catalog, bundle.catalog) + assert.deepEqual(policy.benchmark, bundle.benchmark) + assert.deepEqual(policy.expectedSkus, publicationPolicy.expectedSkus) assert.deepEqual(statuses.map(status => status.state), ['pending', 'success']) }) await context.test('main push promotes the archived candidate through commit association', async () => { @@ -591,26 +590,20 @@ test('CI archival and promotion with simulated GitHub and Git', async context => writeJson(eventPath, notification, false) } }) - await context.test('stale workflow and failed jobs cannot download artifacts', async () => { + await context.test('stale workflow cannot download artifacts', async () => { clearStore() workflowHead = 'e'.repeat(40) await assert.rejects(main(), /stale/) workflowHead = pr.head.sha - failedJob = true - await assert.rejects(main(), /successful eligibility/) assert.equal(statuses.at(-1).state, 'failure') - failedJob = false assert.equal(downloads, 0) assert.equal(pushes, 0) }) - await context.test('partial retry archives and promotes retained successful jobs', async () => { + await context.test('a successful retry archives its final artifact', async () => { clearStore() const originalUrl = bundle.run.workflow.url bundle.run.attempt = 2 bundle.run.workflow.url = `https://github.com/${repository}/actions/runs/12345/attempts/2` - retryJobs = jobs.filter(job => ['eligibility', 'measure (0)', 'collect', 'Workload Status', 'Benchmark Status'] - .includes(job.name.split(' / ').at(-1)!)) - failedJob = true writeJson(eventPath, { workflow_run: { ...notification.workflow_run, run_attempt: 2 } }, false) try { await main() @@ -619,21 +612,28 @@ test('CI archival and promotion with simulated GitHub and Git', async context => assert.deepEqual(readJson(resolve(temporary, 'data-store/index.json')), { schemaVersion: 1, runs: [{ id: '12345', attempt: 2 }] }) const stored = validateCompleteRun(readJson(resolve(temporary, 'data-store/runs/12345/2.json'))) assert.deepEqual(stored.measurements, bundle.measurements) - retryJobs = retryJobs.map(job => ({ ...job, conclusion: job.name.endsWith('measure (0)') ? 'failure' : 'success' })) - await assert.rejects(main(), /successful measure/) - assert.equal(downloads, 1, 'A failed retry must not use an earlier successful job') - retryJobs = retryJobs.filter(job => !job.name.endsWith('eligibility')) - await assert.rejects(main(), /successful eligibility/) - assert.equal(downloads, 1) } finally { bundle.run.attempt = 1 bundle.run.workflow.url = originalUrl - retryJobs = [] - failedJob = false writeJson(eventPath, notification, false) clearStore() } }) + await context.test('incomplete artifacts and untrusted runner SKUs cannot archive', async () => { + const measurement = bundle.measurements.pop()! + await assert.rejects(main(), /Missing successful configurations/) + bundle.measurements.push(measurement) + const runner = bundle.runners[0]! + const originalSku = runner.sku + const originalExpectedSku = runner.expectedSku + runner.sku = 'untrusted-sku' + runner.expectedSku = 'untrusted-sku' + await assert.rejects(main(), /Runner SKU differs from publication policy/) + runner.sku = originalSku + runner.expectedSku = originalExpectedSku + assert.equal(downloads, 2) + assert.equal(pushes, 0) + }) await context.test('label changes prevent archival and promotion', async () => { pr.labels = [{ name: 'benchmarks: skip' }] await main() diff --git a/scripts/publish-ci.ts b/scripts/publish-ci.ts index 8aed228..c116ba3 100644 --- a/scripts/publish-ci.ts +++ b/scripts/publish-ci.ts @@ -3,7 +3,7 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync } from 'node:fs' import { resolve } from 'node:path' import { pathToFileURL } from 'node:url' import { publicationPolicy } from '../shared/catalog.ts' -import { historyRunPath, validatePublication, validatePublishableRun } from '../shared/results.ts' +import { historyRunPath, validateCompleteRun, validatePublication, validatePublishableRun } from '../shared/results.ts' import { eligibility, github, pages } from './github-pr.ts' import { readJson, readOptionalJson, writeJson } from './result-store.ts' import { promoteRun, storeRun } from './publish-results.ts' @@ -45,34 +45,6 @@ async function verifiedRun(runId: number, attempt: number) { if (run.event !== 'pull_request' || run.conclusion !== 'success' || run.path !== '.github/workflows/benchmark-trigger.yml') { throw new Error('Expected a successful PR Benchmark workflow attempt') } - const latestJobs = new Map() - for (let currentAttempt = attempt; currentAttempt >= 1; currentAttempt--) { - let fetched = 0 - for (let page = 1; ; page++) { - const response = await github(`actions/runs/${runId}/attempts/${currentAttempt}/jobs?per_page=100&page=${page}`) - for (const job of response.jobs) { - if (!latestJobs.has(job.name)) latestJobs.set(job.name, job) - } - fetched += response.jobs.length - if (fetched >= response.total_count) break - if (!response.jobs.length) throw new Error('Incomplete workflow job list') - } - } - const jobs = [...latestJobs.values()] - const expected = new Map([ - ['eligibility', 1], ['configure', 1], ['producer', 1], ['prepare', 2], - ['measure', publicationPolicy.catalog.platforms.length * publicationPolicy.catalog.runtimes.length], - ['collect', 1], ['Workload Status', 1], ['Benchmark Status', 1], - ]) - for (const [name, count] of expected) { - const matching = jobs.filter(job => { - const leaf = job.name.split(' / ').at(-1) - return leaf === name || leaf?.startsWith(`${name} (`) - }) - if (matching.length !== count || matching.some(job => job.conclusion !== 'success')) { - throw new Error(`Run ${runId} through attempt ${attempt} requires ${count} successful ${name} jobs. Rerun the failed jobs.`) - } - } return run } @@ -178,7 +150,9 @@ async function archive(notification: any, number: number, { pr, decision }: Awai mkdirSync(incoming, { recursive: true }) execute('gh', ['run', 'download', String(run.id), '--repo', repository, '--name', `run-${run.id}-${notification.run_attempt}`, '--dir', incoming]) const input = resolve(incoming, 'run.json') - const bundle = validatePublishableRun(readJson(input), publicationPolicy) + const candidate = validateCompleteRun(readJson(input)) + const policy = { ...publicationPolicy, catalog: candidate.catalog, benchmark: candidate.benchmark } + const bundle = validatePublishableRun(candidate, policy) if (bundle.run.id !== String(run.id) || bundle.run.attempt !== notification.run_attempt) throw new Error('Artifact workflow identity differs') await verifyCandidate(bundle, pr) validatePublication(bundle, { @@ -186,8 +160,8 @@ async function archive(notification: any, number: number, { pr, decision }: Awai merge: { sha: bundle.run.commit.sha, tree: bundle.run.commit.tree, mergedAt: bundle.run.createdAt, message: bundle.run.commit.message }, }) openStore() - storeRun(directory, bundle) - writeJson(resolve(directory, 'policies', bundle.run.id, `${bundle.run.attempt}.json`), publicationPolicy, true) + storeRun(directory, bundle, policy) + writeJson(resolve(directory, 'policies', bundle.run.id, `${bundle.run.attempt}.json`), policy, true) const pointerPath = resolve(directory, 'pending', `pr-${number}.json`) const previous = readOptionalJson(pointerPath) as any const pointer = { id: bundle.run.id, attempt: bundle.run.attempt, createdAt: run.created_at }