From 3628915c5dab671446c8df8d25ae5f7e6d38cb99 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 20 Sep 2026 02:10:23 -0400 Subject: [PATCH] Make build-approved release pending PR workflows --- .github/VOUCHED.td | 5 +- .github/scripts/approve-pr-workflows.cjs | 78 ++++++++ .github/workflows/approve-pr.yml | 46 +++++ .github/workflows/build-pr.yml | 26 ++- .github/workflows/test.yml | 3 + README.md | 13 ++ tests/pr-workflow-approval.cjs | 242 +++++++++++++++++++++++ 7 files changed, 405 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/approve-pr-workflows.cjs create mode 100644 .github/workflows/approve-pr.yml create mode 100644 tests/pr-workflow-approval.cjs diff --git a/.github/VOUCHED.td b/.github/VOUCHED.td index 753f35a..1707f1f 100644 --- a/.github/VOUCHED.td +++ b/.github/VOUCHED.td @@ -4,7 +4,10 @@ # its author is trusted: repository collaborators are trusted automatically # and do not need listing; external contributors listed here are trusted # too. Anyone else gets the plan only, until a maintainer either adds them -# here or applies the "build-approved" label to that one PR. +# here or applies the "build-approved" label to that one PR. The label also +# releases GitHub's approval hold for that PR's build and test workflows. +# It remains effective while attached, without vouching for the author's +# other PRs. An explicit denouncement cannot be overridden by the label. # # Syntax: # github:username diff --git a/.github/scripts/approve-pr-workflows.cjs b/.github/scripts/approve-pr-workflows.cjs new file mode 100644 index 0000000..0ee32b3 --- /dev/null +++ b/.github/scripts/approve-pr-workflows.cjs @@ -0,0 +1,78 @@ +const BUILD = '.github/workflows/build-pr.yml'; +const TESTS = '.github/workflows/test.yml'; + +module.exports = async function approve({ github, context, core, vouchStatus, + sleep = ms => new Promise(resolve => setTimeout(resolve, ms)), attempts = 36 }) { + // Missing/failed vouch lookups must not become approval. Denouncements + // remain absolute, just as they are in the package build gate. + if (!['unknown', 'bot', 'collaborator', 'vouched'].includes(vouchStatus)) { + throw new Error(`Cannot approve workflows: vouch status is ${vouchStatus || 'missing'}.`); + } + + const expected = context.payload.pull_request; + const eventTime = Date.parse(expected.updated_at); + if (!Number.isFinite(eventTime)) throw new Error('Missing PR event timestamp.'); + const approved = new Set(); + let precedingBuild; + + const stillApproved = async () => { + const { data: pr } = await github.rest.pulls.get({ + ...context.repo, pull_number: expected.number, + }); + return pr.state === 'open' && pr.head.sha === expected.head.sha && + pr.labels.some(label => label.name === 'build-approved'); + }; + + // The label and PR-run events arrive independently. Wait for the build + // belonging to this event, rather than returning after approving an older + // run and leaving the new label-triggered run stuck behind GitHub's gate. + for (let attempt = 0; attempt < attempts; attempt++) { + if (attempt) await sleep(5000); + if (!await stillApproved()) { + core.info('PR closed, head changed, or build-approved removed; stopping.'); + return; + } + + const all = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, { + ...context.repo, event: 'pull_request', head_sha: expected.head.sha, per_page: 100, + }); + const runs = all.filter(run => + run.event === 'pull_request' && run.head_sha === expected.head.sha && + run.head_repository?.id === expected.head.repo.id && run.head_branch === expected.head.ref && + [BUILD, TESTS].includes(run.path) && + // Fork runs awaiting approval often have no pull_requests entries. + (!run.pull_requests?.length || run.pull_requests.some(pr => pr.number === expected.number)) + ).sort((a, b) => a.id - b.id); + + const newestBuild = runs.findLast(run => run.path === BUILD); + if (!newestBuild || !(Date.parse(newestBuild.created_at) >= eventTime) || + !runs.some(run => run.path === TESTS && + (context.payload.action === 'labeled' || Date.parse(run.created_at) >= eventTime))) continue; + + if (precedingBuild) { + const { data: run } = await github.rest.actions.getWorkflowRun({ + ...context.repo, run_id: precedingBuild, + }); + // Approve older builds first, and let them acquire concurrency before + // releasing a newer build. Otherwise an older queued run could start + // last and cancel the label-triggered build that carries approval. + if (!['in_progress', 'completed'].includes(run.status) || run.conclusion === 'action_required') continue; + precedingBuild = undefined; + } + + const pending = runs.filter(run => run.conclusion === 'action_required' && !approved.has(run.id) && + // If the newest build already runs (e.g. a maintainer approved it), + // don't resurrect an obsolete hold that could cancel that newer run. + (run.path !== BUILD || run.id === newestBuild.id || newestBuild.conclusion === 'action_required')); + if (!pending.length) return; + const run = pending[0]; + // Recheck after the API reads, immediately before exercising write access. + if (!await stillApproved()) return; + await github.rest.actions.approveWorkflowRun({ ...context.repo, run_id: run.id }); + approved.add(run.id); + core.info(`Approved ${run.path} run ${run.id} for PR #${expected.number}.`); + if (run.path === BUILD) precedingBuild = run.id; + if (pending.length === 1) return; + } + throw new Error('Timed out waiting for PR workflows. Remove and reapply build-approved to retry.'); +}; diff --git a/.github/workflows/approve-pr.yml b/.github/workflows/approve-pr.yml new file mode 100644 index 0000000..996fdc9 --- /dev/null +++ b/.github/workflows/approve-pr.yml @@ -0,0 +1,46 @@ +name: Approve PR workflows + +# A pull_request workflow cannot approve itself: GitHub can hold it before +# any job starts. This workflow only runs trusted default-branch code and +# releases the ordinary, unprivileged PR workflows after build approval. +on: + pull_request_target: + types: [opened, synchronize, reopened, labeled] + +permissions: + contents: read + pull-requests: read + actions: write + +concurrency: + group: approve-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + approve: + # Match build-pr.yml's events, including other labels applied while this + # PR still carries build-approved: each labeled event creates a build. + if: contains(github.event.pull_request.labels.*.name, 'build-approved') + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # Never check out the PR head or its merge ref with this write token. + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - id: vouch + uses: mitchellh/vouch/action/check-user@f23dbb5e745334f97414ec70463ce7301071a661 # v1 + with: + user: ${{ github.event.pull_request.user.login }} + allow-fail: true + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Approve this PR's pending build and test runs + uses: actions/github-script@v7 + env: + VOUCH_STATUS: ${{ steps.vouch.outputs.status }} + with: + script: | + const approve = require('./.github/scripts/approve-pr-workflows.cjs'); + await approve({ github, context, core, vouchStatus: process.env.VOUCH_STATUS }); diff --git a/.github/workflows/build-pr.yml b/.github/workflows/build-pr.yml index c80927a..764ef90 100644 --- a/.github/workflows/build-pr.yml +++ b/.github/workflows/build-pr.yml @@ -28,8 +28,7 @@ jobs: # collaborators, anyone in .github/VOUCHED.td (read from the default # branch, so a PR cannot vouch for itself), or a PR a maintainer has # labelled "build-approved". Everyone else gets this job's plan output - # and a passing `result`, which is enough for a maintainer to review - # before deciding to spend the compute. + # and a failing `result` until a maintainer approves the build. changes: runs-on: ubuntu-latest outputs: @@ -64,6 +63,19 @@ jobs: allow-fail: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - id: approval + if: github.event_name == 'pull_request' + uses: actions/github-script@v7 + with: + script: | + const { data: pr } = await github.rest.pulls.get({ + ...context.repo, pull_number: context.payload.pull_request.number, + }); + // Approving or rerunning a held run keeps its original event, + // which may predate the label. Read the current approval instead. + core.setOutput('approved', pr.state === 'open' && + pr.head.sha === context.payload.pull_request.head.sha && + pr.labels.some(label => label.name === 'build-approved')); # One matrix entry per package per architecture. Every package builds # once, against edge; the channels it ships to on merge are carried # along for information. A filename means one set of bytes. @@ -92,16 +104,17 @@ jobs: fi - id: gate env: - STATUS: ${{ steps.vouch.outputs.status || 'dispatch' }} + STATUS: ${{ github.event_name == 'workflow_dispatch' && 'dispatch' || steps.vouch.outputs.status }} AUTHOR: ${{ github.event.pull_request.user.login }} - APPROVED: ${{ contains(github.event.pull_request.labels.*.name, 'build-approved') }} + APPROVED: ${{ steps.approval.outputs.approved || 'false' }} PLANNED: ${{ steps.list.outputs.planned }} run: | case "$STATUS" in bot|collaborator|vouched|dispatch) trusted=true ;; # A denouncement is absolute: the label cannot override it. denounced) trusted=false ;; - *) trusted=$APPROVED ;; + unknown) trusted=$APPROVED ;; + *) trusted=false ;; esac echo "trusted=$trusted" >> "$GITHUB_OUTPUT" if [[ $trusted == true ]]; then @@ -176,8 +189,7 @@ jobs: steps: - run: | echo "trusted=${{ needs.changes.outputs.trusted }} build=${{ needs.build.result }}" - # An untrusted author's PR is held, not failed: the required check - # stays pending until a maintainer vouches or labels it. + # An untrusted author's PR fails until a maintainer vouches or labels it. if [[ "${{ needs.changes.outputs.trusted }}" != "true" ]]; then echo "::error::Builds were not run: author is not vouched. Add to .github/VOUCHED.td or apply the 'build-approved' label." exit 1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9ced3ea..acf3704 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,6 +32,9 @@ jobs: with: persist-credentials: false + - name: Test PR workflow approval + run: node --test tests/pr-workflow-approval.cjs + # An Arch container for vercmp: version ordering has to be decided by # the same comparator pacman uses on users' machines. - name: Run self-tests diff --git a/README.md b/README.md index 5ff0edc..24168ba 100644 --- a/README.md +++ b/README.md @@ -844,6 +844,19 @@ The repository includes GitHub workflows and systemd services for automated rele 1. **sync-upstream.yml** (Every 6 hours): Watches direct upstream feeds and updates owned recipes. Successful package updates reach a PR even if another feed fails; failed recipes stay untouched and the workflow remains red. 2. **sync-rebuilds.yml** (Every 6 hours): Bumps pkgrel for packages whose `rebuild_on` dependencies have moved in the official repositories and opens a PR. +To approve builds for an unvouched contributor's PR, apply **`build-approved`**. +This triggers a package build and automatically releases GitHub's pending +build and test workflows for that PR's current commit. The approval workflow +runs only trusted default-branch code; package builds and tests stay in the +ordinary PR workflows. It may take a few minutes for GitHub to register and +release all the runs. + +The label stays effective for that PR while attached, including later commits; +it does not vouch for the author's other PRs. Removing it stops further label +approvals, but does not cancel runs already released. An explicit denouncement +in `.github/VOUCHED.td` still blocks builds. If the approval workflow times out, +remove and reapply the label to retry. + #### Systemd Services All four units run **every 5 minutes**, staggered by a minute each, so a push diff --git a/tests/pr-workflow-approval.cjs b/tests/pr-workflow-approval.cjs new file mode 100644 index 0000000..d2b74f8 --- /dev/null +++ b/tests/pr-workflow-approval.cjs @@ -0,0 +1,242 @@ +const assert = require('node:assert/strict'); +const { readFileSync } = require('node:fs'); +const { join } = require('node:path'); +const { test } = require('node:test'); +const { execFileSync } = require('node:child_process'); +const approve = require('../.github/scripts/approve-pr-workflows.cjs'); + +const BUILD = '.github/workflows/build-pr.yml'; +const TESTS = '.github/workflows/test.yml'; +const time = '2026-09-19T02:47:52Z'; +const earlier = '2026-09-19T02:36:04Z'; +const pr = { + number: 390, state: 'open', updated_at: time, + head: { sha: 'reviewed-sha', ref: 'ghost', repo: { id: 42 } }, + labels: [{ name: 'build-approved' }], +}; +const clone = value => structuredClone(value); +const run = (id, path, overrides = {}) => ({ + id, path, event: 'pull_request', head_sha: pr.head.sha, + head_repository: { id: 42 }, head_branch: 'ghost', pull_requests: [], + status: 'completed', conclusion: 'action_required', created_at: time, + ...overrides, +}); + +function fixture(initial = [run(1, TESTS, { created_at: earlier }), run(2, BUILD)], options = {}) { + const state = { pr: clone(pr), runs: clone(initial), approved: [], reads: 0, tick: 0, transitions: [] }; + const repo = { owner: 'omacom', repo: 'omarchy-pkgs' }; + const github = { + rest: { + pulls: { get: async args => { + assert.deepEqual(args, { ...repo, pull_number: 390 }); + state.reads++; + options.onRead?.(state); + return { data: clone(state.pr) }; + } }, + actions: { + listWorkflowRunsForRepo() {}, + getWorkflowRun: async ({ run_id }) => { + const current = state.runs.find(run => run.id === run_id); + state.transitions.push([run_id, current.status]); + return { data: clone(current) }; + }, + approveWorkflowRun: async args => { + assert.deepEqual(args, { ...repo, run_id: args.run_id }); + options.onApprove?.(state, args.run_id); + const current = state.runs.find(run => run.id === args.run_id); + assert.equal(current.conclusion, 'action_required'); + state.approved.push(current.id); + current.status = 'queued'; + current.conclusion = null; + }, + }, + }, + paginate: async (method, args) => { + assert.equal(method, github.rest.actions.listWorkflowRunsForRepo); + assert.deepEqual(args, { ...repo, event: 'pull_request', head_sha: pr.head.sha, per_page: 100 }); + return clone(state.runs).reverse(); // GitHub returns newest first. + }, + }; + const invoke = overrides => approve({ + github, context: { repo, payload: { action: 'labeled', pull_request: clone(pr) } }, + core: { info() {} }, vouchStatus: 'unknown', attempts: 6, + sleep: async () => { + state.tick++; + for (const current of state.runs) { + if (current.status === 'queued' && state.tick >= (options.queueUntil ?? 1)) current.status = 'in_progress'; + } + options.onSleep?.(state); + }, + ...overrides, + }); + return { state, invoke, github }; +} + +test('an unvouched, labeled fork PR releases both required workflows', async () => { + const { state, invoke } = fixture(); + await invoke(); + assert.deepEqual(state.approved, [1, 2]); +}); + +test('waits for the label-triggered build instead of stopping at the old build', async () => { + const { state, invoke } = fixture([ + run(1, BUILD, { created_at: earlier }), run(2, TESTS, { created_at: earlier }), + ], { + onSleep(state) { + if (state.tick === 2) { + assert.deepEqual(state.approved, []); + state.runs.push(run(3, BUILD)); + } + }, + queueUntil: 4, + onApprove(state, id) { + if (id === 3) assert.equal(state.runs.find(run => run.id === 1).status, 'in_progress'); + }, + }); + await invoke({ attempts: 10 }); + assert.deepEqual(state.approved, [1, 2, 3]); + assert.ok(state.transitions.some(([id, status]) => id === 1 && status === 'queued')); +}); + +test('approves only the two known workflows for this fork, branch, PR and SHA', async () => { + const unrelated = [ + { path: '.github/workflows/publish.yml' }, { event: 'push' }, + { head_sha: 'other-sha' }, { head_repository: { id: 99 } }, + { head_branch: 'other-branch' }, { pull_requests: [{ number: 391 }] }, + ].map((overrides, i) => run(10 + i, BUILD, overrides)); + const { state, invoke } = fixture([run(1, TESTS), run(2, BUILD), ...unrelated]); + await invoke(); + assert.deepEqual(state.approved, [1, 2]); +}); + +test('accepts a run explicitly associated with this PR', async () => { + const { state, invoke } = fixture([ + run(1, TESTS), run(2, BUILD, { pull_requests: [{ number: 390 }] }), + ]); + await invoke(); + assert.deepEqual(state.approved, [1, 2]); +}); + +test('does not restart running or completed workflows', async () => { + const { state, invoke } = fixture([ + run(1, TESTS, { conclusion: 'success' }), + run(2, BUILD, { status: 'in_progress', conclusion: null }), + ]); + await invoke(); + assert.deepEqual(state.approved, []); +}); + +test('an obsolete build hold cannot cancel a newer build that was already released', async () => { + for (const current of [ + { status: 'queued', conclusion: null }, { status: 'in_progress', conclusion: null }, + { status: 'completed', conclusion: 'success' }, { status: 'completed', conclusion: 'failure' }, + ]) { + const { state, invoke } = fixture([ + run(1, BUILD, { created_at: earlier }), run(2, TESTS), run(3, BUILD, current), + ]); + await invoke(); + assert.deepEqual(state.approved, [2]); + } +}); + +for (const status of ['denounced', '', undefined, 'unexpected']) { + test(`vouch status ${String(status)} fails closed`, async () => { + const { state, invoke } = fixture(); + await assert.rejects(invoke({ vouchStatus: status }), /Cannot approve workflows/); + assert.deepEqual(state.approved, []); + }); +} + +for (const status of ['bot', 'collaborator', 'vouched']) { + test(`a labeled ${status} can also clear GitHub's approval gate`, async () => { + const { state, invoke } = fixture(); + await invoke({ vouchStatus: status }); + assert.deepEqual(state.approved, [1, 2]); + }); +} + +for (const [name, change] of [ + ['removed label', pr => { pr.labels = []; }], + ['changed head', pr => { pr.head.sha = 'new-sha'; }], + ['closed PR', pr => { pr.state = 'closed'; }], +]) { + test(`${name} stops approval, including changes immediately before a write`, async () => { + for (const read of [1, 2]) { + const { state, invoke } = fixture(undefined, { onRead(state) { + if (state.reads === read) change(state.pr); + } }); + await invoke(); + assert.deepEqual(state.approved, []); + } + }); +} + +test('revocation between approvals prevents releasing further workflows', async () => { + const { state, invoke } = fixture(undefined, { onSleep(state) { state.pr.labels = []; } }); + await invoke(); + assert.deepEqual(state.approved, [1]); +}); + +test('a delayed tests workflow is also awaited', async () => { + const { state, invoke } = fixture([run(2, BUILD)], { + onSleep(state) { if (state.tick === 2) state.runs.push(run(1, TESTS)); }, + }); + await invoke(); + assert.deepEqual(state.approved, [1, 2]); +}); + +test('reopening a labeled PR waits for its new tests, even if old tests passed at the same SHA', async () => { + const { state, invoke } = fixture([ + run(1, TESTS, { created_at: earlier, conclusion: 'success' }), run(2, BUILD), + ], { onSleep(state) { if (state.tick === 2) state.runs.push(run(3, TESTS)); } }); + await invoke({ context: { repo: { owner: 'omacom', repo: 'omarchy-pkgs' }, + payload: { action: 'reopened', pull_request: clone(pr) } } }); + assert.deepEqual(state.approved, [2, 3]); +}); + +test('missing current runs time out without approving stale builds', async () => { + const { state, invoke } = fixture([run(1, TESTS), run(2, BUILD, { created_at: earlier })]); + await assert.rejects(invoke(), /Timed out/); + assert.deepEqual(state.approved, []); +}); + +test('API failure is reported rather than silently treated as approval', async () => { + const { state, invoke } = fixture(undefined, { onApprove() { throw new Error('Forbidden'); } }); + await assert.rejects(invoke(), /Forbidden/); + assert.deepEqual(state.approved, []); +}); + +// Execute the actual build workflow's approval script and shell gate. This +// covers the stale event payload that originally accompanied held PR runs. +const workflow = readFileSync(join(__dirname, '../.github/workflows/build-pr.yml'), 'utf8'); +const approvalScript = workflow.match(/- id: approval[\s\S]*?script: \|\n([\s\S]*?)(?= # One matrix)/)[1] + .split('\n').map(line => line.replace(/^ /, '')).join('\n'); +const gateScript = workflow.match(/ case "\$STATUS" in[\s\S]*? esac/)[0] + '\nprintf "%s" "$trusted"'; + +test('the build reads the live label rather than its pre-label event payload', async () => { + const execute = new (Object.getPrototypeOf(async function () {}).constructor)('github', 'context', 'core', approvalScript); + for (const [current, approved] of [ + [pr, true], [{ ...pr, labels: [] }, false], + [{ ...pr, head: { ...pr.head, sha: 'new-sha' } }, false], + [{ ...pr, state: 'closed' }, false], + ]) { + const outputs = {}; + await execute({ rest: { pulls: { get: async () => ({ data: current }) } } }, + { repo: {}, payload: { pull_request: { ...pr, labels: [] } } }, + { setOutput: (key, value) => { outputs[key] = value; } }); + assert.equal(outputs.approved, approved); + } +}); + +test('the build gate permits a missing vouch only with approval, never a denouncement or lookup failure', () => { + for (const [status, approved, expected] of [ + ['unknown', 'true', 'true'], ['unknown', 'false', 'false'], + ['denounced', 'true', 'false'], ['', 'true', 'false'], ['unexpected', 'true', 'false'], + ['vouched', 'false', 'true'], ['collaborator', 'false', 'true'], + ['bot', 'false', 'true'], ['dispatch', 'false', 'true'], + ]) { + assert.equal(execFileSync('bash', ['-c', gateScript], { + env: { ...process.env, STATUS: status, APPROVED: approved }, encoding: 'utf8', + }), expected); + } +});