Skip to content

Commit 02d7c6f

Browse files
authored
Use Actions approval for artifact publication (#31)
## Summary * rely on GitHub Actions approval for external contributors instead of workflow-file equality * retain trusted publisher validation for run identity, artifacts, job completion, and write destinations * reuse PR eligibility during archival to avoid a duplicate API request ## Validation * `npm run build` * `git diff --check` Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 42e73af commit 02d7c6f

6 files changed

Lines changed: 9 additions & 81 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ perf_*.json
2222
mem_*.json
2323

2424
# Editor directories and files
25+
.copilot-reviews/
2526
.vscode/*
2627
!.vscode/extensions.json
2728
.idea

‎docs/README.md‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ The preview URL then returns 404.
110110
* Each of the 36 measurement jobs runs all three strategies sequentially with a fresh server process for each. Retrying a job repeats its three strategies.
111111
* Preview builds run independently of benchmarks. Draft PRs have no preview.
112112
* Production uses published history. Previews include matching pending results when available.
113-
* Benchmark trigger, workload, and preview workflow YAML must match `main` before publishers accept their artifacts.
114113
* Merge requires `Benchmark Policy`, `Benchmark Status`, an up-to-date branch, and squash merging. Required PR review count is zero. Preview and publication success do not block merge.
115114

116115
Publication calls Pages after a main push or a data change. Pages also runs for
@@ -122,11 +121,11 @@ Benchmark checks ignore unrelated labels and title or body edits.
122121

123122
## Security
124123

125-
* Outside contributors require **Approve and run**. Collaborators run automatically. Allowing preview execution also permits serving its JavaScript to visitors.
124+
* GitHub Actions must use **Require approval for all external contributors**. **Approve and run** authorizes pending benchmark archival and serving preview JavaScript from that revision. Collaborators run automatically.
126125
* PR jobs use read-only repository permissions. The separate `workflow_run` publishers execute `main` code with their own tokens.
127126
* Benchmark Publication uses `contents: write` and `statuses: write`. It commits to `data` as `github-actions[bot]` and restricts write paths in code.
128127
* Pages handles PR metadata with `pull_request_target` and executes only `main` code.
129128
* Pages uses `pages: write` and `id-token: write`. Configure Pages for GitHub Actions and restrict the `github-pages` environment to `main`.
130-
* Publishers validate PR artifacts as untrusted input. They do not execute artifact scripts or restore PR caches. Workflow matching cannot prove measurements are genuine.
129+
* 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.
131130
* 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.
132131
* Preview JavaScript shares production's origin and browser storage. Separate-origin hosting is required for browser isolation.

‎scripts/github-pr.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { z } from 'zod'
2-
import { readFileSync } from 'node:fs'
32

43
const repository = process.env.GITHUB_REPOSITORY!
54
if (!/^[\w.-]+\/[\w.-]+$/.test(repository)) throw new Error('GITHUB_REPOSITORY is required')
@@ -28,12 +27,6 @@ export async function pages(path: string): Promise<any[]> {
2827
}
2928
}
3029

31-
export async function workflowTrusted(path: string, head: string): Promise<boolean> {
32-
const workflow = await github(`contents/${path}?ref=${head}`)
33-
if (workflow.encoding !== 'base64') return false
34-
return Buffer.from(workflow.content, 'base64').toString('utf8') === readFileSync(path, 'utf8')
35-
}
36-
3730
const commit = z.string().regex(/^[a-f0-9]{40}$/)
3831

3932
export const decisionSchema = z.object({

‎scripts/publication.test.ts‎

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -397,35 +397,6 @@ test('label-only eligibility with simulated GitHub responses', async context =>
397397
})
398398
})
399399

400-
test('workflows must match trusted main', async context => {
401-
process.env.GITHUB_REPOSITORY = repository
402-
process.env.GH_TOKEN = 'fixture-token'
403-
const { workflowTrusted } = await import('./github-pr.ts')
404-
const temporary = mkdtempSync(resolve(tmpdir(), 'benchmark-workflow-trust-'))
405-
const previousDirectory = process.cwd()
406-
context.after(() => {
407-
process.chdir(previousDirectory)
408-
rmSync(temporary, { recursive: true, force: true })
409-
})
410-
process.chdir(temporary)
411-
mkdirSync('.github/workflows', { recursive: true })
412-
const head = 'a'.repeat(40)
413-
let content = 'trusted workflow'
414-
context.mock.method(globalThis, 'fetch', async (url: string) => {
415-
const path = new URL(url).pathname.replace(`/repos/${repository}/`, '')
416-
assert.ok(path.startsWith('contents/'), `Unexpected GitHub request: ${path}`)
417-
return Response.json({ encoding: 'base64', content: Buffer.from(content).toString('base64') })
418-
})
419-
for (const workflow of ['benchmark', 'benchmark-trigger', 'preview']) {
420-
const path = `.github/workflows/${workflow}.yml`
421-
writeFileSync(path, 'trusted workflow')
422-
content = 'trusted workflow'
423-
assert.equal(await workflowTrusted(path, head), true)
424-
content = 'changed workflow'
425-
assert.equal(await workflowTrusted(path, head), false)
426-
}
427-
})
428-
429400
test('Pages verifies previews with revision and build jobs and excludes draft PRs', async context => {
430401
const temporary = mkdtempSync(resolve(tmpdir(), 'benchmark-preview-policy-'))
431402
const previousDirectory = process.cwd()
@@ -449,9 +420,6 @@ test('Pages verifies previews with revision and build jobs and excludes draft PR
449420
head: { sha: 'a'.repeat(40), repo: { full_name: repository } },
450421
base: { sha: 'b'.repeat(40), ref: 'main', repo: { full_name: repository } },
451422
}
452-
const workflow = readFileSync(resolve(root, '.github/workflows/preview.yml'), 'utf8')
453-
mkdirSync('.github/workflows', { recursive: true })
454-
writeFileSync('.github/workflows/preview.yml', workflow)
455423
const run = {
456424
id: 123, run_attempt: 1, event: 'pull_request', path: '.github/workflows/preview.yml',
457425
conclusion: 'success', head_sha: pr.head.sha, head_repository: pr.head.repo,
@@ -479,7 +447,6 @@ test('Pages verifies previews with revision and build jobs and excludes draft PR
479447
'actions/runs/123/attempts/1/jobs': { jobs: [
480448
{ name: 'revision', conclusion: 'success' }, { name: 'build', conclusion: 'success' },
481449
] },
482-
'contents/.github/workflows/preview.yml': { encoding: 'base64', content: Buffer.from(workflow).toString('base64') },
483450
}
484451
assert.ok(path in responses, `Unexpected GitHub request: ${path}`)
485452
return Response.json(responses[path])
@@ -513,9 +480,6 @@ test('CI archival and promotion with simulated GitHub and Git', async context =>
513480
id: 12345, run_attempt: 1, event: 'pull_request', conclusion: 'success', head_sha: 'a'.repeat(40), pull_requests: [{ number: 7 }],
514481
} }
515482
writeJson(eventPath, notification, false)
516-
const triggerWorkflow = readFileSync(resolve(root, '.github/workflows/benchmark-trigger.yml'), 'utf8')
517-
let triggerWorkflowContent = triggerWorkflow
518-
const workloadWorkflow = readFileSync(resolve(root, '.github/workflows/benchmark.yml'), 'utf8')
519483
const bundle = fixture()
520484
const pr = {
521485
number: 7, base: { ref: 'main', repo: { full_name: repository }, sha: bundle.run.pullRequest!.base },
@@ -558,8 +522,6 @@ test('CI archival and promotion with simulated GitHub and Git', async context =>
558522
[`commits/${'d'.repeat(40)}/pulls`]: [pr],
559523
'actions/runs/12345/attempts/1': { id: 12345, event: 'pull_request', conclusion: 'success', path: '.github/workflows/benchmark-trigger.yml', head_sha: workflowHead, created_at: bundle.run.createdAt },
560524
'actions/runs/12345/attempts/2': { id: 12345, event: 'pull_request', conclusion: 'success', path: '.github/workflows/benchmark-trigger.yml', head_sha: workflowHead, created_at: bundle.run.createdAt },
561-
'contents/.github/workflows/benchmark-trigger.yml': { encoding: 'base64', content: Buffer.from(triggerWorkflowContent).toString('base64') },
562-
'contents/.github/workflows/benchmark.yml': { encoding: 'base64', content: Buffer.from(workloadWorkflow).toString('base64') },
563525
[`git/commits/${bundle.run.commit.sha}`]: { tree: { sha: bundle.run.commit.tree }, parents: [{ sha: bundle.run.pullRequest!.base }, { sha: bundle.run.pullRequest!.head }] },
564526
[`git/commits/${pr.merge_commit_sha}`]: { tree: { sha: mergedTree }, message: 'Benchmark results (#7)' },
565527
}
@@ -579,9 +541,6 @@ test('CI archival and promotion with simulated GitHub and Git', async context =>
579541
return { status: 0, stdout: command === 'git' && args[0] === 'diff' ? 'fixture changes' : '', stderr: '' }
580542
})
581543
syncBuiltinESMExports()
582-
mkdirSync(resolve(temporary, '.github/workflows'), { recursive: true })
583-
writeFileSync(resolve(temporary, '.github/workflows/benchmark-trigger.yml'), triggerWorkflow)
584-
writeFileSync(resolve(temporary, '.github/workflows/benchmark.yml'), workloadWorkflow)
585544
const { main } = await import('./publish-ci.ts')
586545
const clearStore = () => {
587546
rmSync(resolve(temporary, 'data-store'), { recursive: true, force: true })
@@ -705,23 +664,6 @@ test('CI archival and promotion with simulated GitHub and Git', async context =>
705664
writeJson(eventPath, notification, false)
706665
}
707666
})
708-
await context.test('changed workflows cannot archive before matching main', async () => {
709-
clearStore()
710-
triggerWorkflowContent = `${triggerWorkflow}\n`
711-
pr.merged = false
712-
pr.state = 'open'
713-
try {
714-
await assert.rejects(main(), /workflow files must match current main/)
715-
assert.equal(downloads, 0)
716-
assert.equal(pushes, 0)
717-
assert.equal(statuses.at(-1).state, 'failure')
718-
} finally {
719-
triggerWorkflowContent = triggerWorkflow
720-
pr.merged = true
721-
pr.state = 'closed'
722-
clearStore()
723-
}
724-
})
725667
await context.test('merge with no archive reports failure and requires manual archival', async () => {
726668
clearStore()
727669
process.env.GITHUB_EVENT_NAME = 'push'

‎scripts/publish-ci.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { resolve } from 'node:path'
44
import { pathToFileURL } from 'node:url'
55
import { publicationPolicy } from '../shared/catalog.ts'
66
import { historyRunPath, validatePublication, validatePublishableRun } from '../shared/results.ts'
7-
import { eligibility, github, pages, workflowTrusted } from './github-pr.ts'
7+
import { eligibility, github, pages } from './github-pr.ts'
88
import { readJson, readOptionalJson, writeJson } from './result-store.ts'
99
import { promoteRun, storeRun } from './publish-results.ts'
1010

@@ -45,11 +45,6 @@ async function verifiedRun(runId: number, attempt: number) {
4545
if (run.event !== 'pull_request' || run.conclusion !== 'success' || run.path !== '.github/workflows/benchmark-trigger.yml') {
4646
throw new Error('Expected a successful PR Benchmark workflow attempt')
4747
}
48-
for (const path of ['.github/workflows/benchmark-trigger.yml', '.github/workflows/benchmark.yml']) {
49-
if (!await workflowTrusted(path, run.head_sha)) {
50-
throw new Error('Benchmark workflow files must match current main. After merge, rerun this publication workflow.')
51-
}
52-
}
5348
const latestJobs = new Map<string, any>()
5449
for (let currentAttempt = attempt; currentAttempt >= 1; currentAttempt--) {
5550
let fetched = 0
@@ -167,13 +162,12 @@ export async function main() {
167162
if (notification.event !== 'pull_request' || notification.conclusion !== 'success') return
168163
if (notification.pull_requests.length !== 1) throw new Error('Workflow must identify one PR')
169164
const number = notification.pull_requests[0].number
170-
const current = await github(`pulls/${number}`)
171-
if (current.head.sha !== notification.head_sha) throw new Error('Workflow is stale. A fresh benchmark run is required.')
172-
await withPublicationStatus(notification.head_sha, () => archive(notification, number))
165+
const eligibilityResult = await eligibility(number)
166+
if (eligibilityResult.pr.head.sha !== notification.head_sha) throw new Error('Workflow is stale. A fresh benchmark run is required.')
167+
await withPublicationStatus(notification.head_sha, () => archive(notification, number, eligibilityResult))
173168
}
174169

175-
async function archive(notification: any, number: number) {
176-
const { pr, decision } = await eligibility(number)
170+
async function archive(notification: any, number: number, { pr, decision }: Awaited<ReturnType<typeof eligibility>>) {
177171
if (decision.mode === 'skip') {
178172
console.log(`PR ${number}: approved skip. No results archived.`)
179173
return

‎scripts/site-ci.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { spawnSync } from 'node:child_process'
22
import { existsSync, mkdirSync, rmSync } from 'node:fs'
33
import { resolve } from 'node:path'
44
import { assembleSite, previewBuildSchema } from './assemble-site.ts'
5-
import { eligibility, github, pages, workflowTrusted } from './github-pr.ts'
5+
import { eligibility, github, pages } from './github-pr.ts'
66
import { canonicalJson } from '../shared/results.ts'
77
import { readJson, writeJson } from './result-store.ts'
88

@@ -39,7 +39,6 @@ async function verifiedBuild(run: any, pr: any, artifact: any): Promise<boolean>
3939
|| !run.pull_requests.some((entry: { number: number }) => entry.number === pr.number)) return false
4040
if (!artifact || artifact.expired || artifact.name !== `preview-${pr.number}-${pr.head.sha}-${run.run_attempt}`) return false
4141
if (artifact.size_in_bytes > 100 * 1024 ** 2) throw new Error(`Preview artifact is too large: ${artifact.name}`)
42-
if (!await workflowTrusted('.github/workflows/preview.yml', run.head_sha)) return false
4342
const jobs = await github(`actions/runs/${run.id}/attempts/${run.run_attempt}/jobs?per_page=100`)
4443
for (const jobName of ['revision', 'build']) {
4544
const matches = jobs.jobs.filter((job: { name: string }) => job.name === jobName)

0 commit comments

Comments
 (0)