From af0e8f8d394a03349ab5049db7c3f073a19c2ead Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:03:53 +0100 Subject: [PATCH] Reclaim CI capacity when pull requests close Cancel queued and running validation workflows for a closed pull request head so obsolete matrices cannot hold the hosted-runner limit and delay required checks. Keep the privileged close hook bound to reviewed default-branch code and cover reopen, branch-reuse, identity, and API-race boundaries. Change-source: pulse-maintainer --- .../scripts/reclaim-closed-pr-capacity.cjs | 90 ++++++++++++++ .../reclaim-closed-pr-capacity.test.cjs | 110 ++++++++++++++++++ .../workflows/reclaim-closed-pr-capacity.yml | 33 ++++++ scripts/check_workflow_trust.py | 99 +++++++++++++++- .../tests/test-reclaim-closed-pr-capacity.sh | 5 + scripts/tests/test_workflow_trust.py | 48 +++++++- 6 files changed, 382 insertions(+), 3 deletions(-) create mode 100644 .github/scripts/reclaim-closed-pr-capacity.cjs create mode 100644 .github/scripts/reclaim-closed-pr-capacity.test.cjs create mode 100644 .github/workflows/reclaim-closed-pr-capacity.yml create mode 100755 scripts/tests/test-reclaim-closed-pr-capacity.sh diff --git a/.github/scripts/reclaim-closed-pr-capacity.cjs b/.github/scripts/reclaim-closed-pr-capacity.cjs new file mode 100644 index 000000000..5ce02e539 --- /dev/null +++ b/.github/scripts/reclaim-closed-pr-capacity.cjs @@ -0,0 +1,90 @@ +'use strict'; + +const ACTIVE_STATUSES = ['queued', 'in_progress']; + +async function cancelClosedPullRequestRuns({ github, context, core }) { + const pullRequest = context.payload.pull_request; + if (!pullRequest || !Number.isInteger(pullRequest.number)) { + core.setFailed('The close event has no valid pull request number.'); + return; + } + + const { owner, repo } = context.repo; + const current = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullRequest.number, + }); + if (current.data.state !== 'closed') { + core.info(`PR #${pullRequest.number} has reopened; leaving its runs alone.`); + return; + } + + const headRepository = pullRequest.head?.repo?.full_name; + const headOwner = pullRequest.head?.repo?.owner?.login; + const headBranch = pullRequest.head?.ref; + if (!headRepository || !headOwner || !headBranch) { + core.warning('The closed pull request has no durable head identity; no runs cancelled.'); + return; + } + + // A branch can be reused immediately after closure. An open PR for the same + // repository and branch takes precedence over this stale close event. + const openForHead = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + head: `${headOwner}:${headBranch}`, + per_page: 100, + }); + if (openForHead.length > 0) { + core.info('The head now belongs to an open pull request; no runs cancelled.'); + return; + } + + const candidates = new Map(); + for (const status of ACTIVE_STATUSES) { + const runs = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, { + owner, + repo, + event: 'pull_request', + status, + per_page: 100, + }); + for (const run of runs) { + if ( + run.head_branch === headBranch && + run.head_repository?.full_name === headRepository + ) { + candidates.set(run.id, run); + } + } + } + + let cancellationRequests = 0; + for (const run of candidates.values()) { + try { + await github.rest.actions.cancelWorkflowRun({ owner, repo, run_id: run.id }); + cancellationRequests += 1; + core.info(`Requested cancellation of ${run.name} run ${run.id} (${run.status}).`); + } catch (error) { + // Completion can race cancellation. Suppress only that proven terminal + // race; authentication and API failures stay visible. + const refreshed = await github.rest.actions.getWorkflowRun({ + owner, + repo, + run_id: run.id, + }); + if (refreshed.data.status !== 'completed') { + throw error; + } + core.info(`Run ${run.id} completed before cancellation.`); + } + } + core.info( + `Requested cancellation for ${cancellationRequests} of ${candidates.size} ` + + `unfinished run(s) for PR #${pullRequest.number}.`, + ); +} + +module.exports = { ACTIVE_STATUSES, cancelClosedPullRequestRuns }; diff --git a/.github/scripts/reclaim-closed-pr-capacity.test.cjs b/.github/scripts/reclaim-closed-pr-capacity.test.cjs new file mode 100644 index 000000000..a835b36a6 --- /dev/null +++ b/.github/scripts/reclaim-closed-pr-capacity.test.cjs @@ -0,0 +1,110 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const test = require('node:test'); + +const { + ACTIVE_STATUSES, + cancelClosedPullRequestRuns, +} = require('./reclaim-closed-pr-capacity.cjs'); + +function fixture({ state = 'closed', openForHead = [], runs = {}, cancelError, refreshed = 'completed' } = {}) { + const cancelled = []; + const messages = []; + const github = { + paginate: async (method, input) => method(input), + rest: { + pulls: { + get: async () => ({ data: { state } }), + list: async () => openForHead, + }, + actions: { + listWorkflowRunsForRepo: async ({ status }) => runs[status] || [], + cancelWorkflowRun: async ({ run_id: runId }) => { + if (cancelError) throw cancelError; + cancelled.push(runId); + }, + getWorkflowRun: async () => ({ data: { status: refreshed } }), + }, + }, + }; + const context = { + repo: { owner: 'rcourtman', repo: 'Pulse' }, + payload: { + pull_request: { + number: 1858, + head: { + ref: 'topic/old', + repo: { full_name: 'rcourtman/Pulse', owner: { login: 'rcourtman' } }, + }, + }, + }, + }; + const core = { + info: (message) => messages.push(message), + warning: (message) => messages.push(message), + setFailed: (message) => messages.push(message), + }; + return { github, context, core, cancelled, messages }; +} + +test('cancels only unfinished runs for the exact closed head', async () => { + const matching = { + id: 10, + name: 'Build and Test', + status: 'queued', + head_branch: 'topic/old', + head_repository: { full_name: 'rcourtman/Pulse' }, + }; + const duplicate = { ...matching, status: 'in_progress' }; + const otherBranch = { ...matching, id: 11, head_branch: 'topic/current' }; + const otherRepository = { + ...matching, + id: 12, + head_repository: { full_name: 'contributor/Pulse' }, + }; + const subject = fixture({ + runs: { queued: [matching, otherBranch, otherRepository], in_progress: [duplicate] }, + }); + + await cancelClosedPullRequestRuns(subject); + + assert.deepEqual(subject.cancelled, [10]); + assert.match(subject.messages.at(-1), /Requested cancellation for 1 of 1 unfinished run/); + assert.deepEqual(ACTIVE_STATUSES, ['queued', 'in_progress']); +}); + +test('does nothing when the pull request reopened', async () => { + const subject = fixture({ state: 'open' }); + await cancelClosedPullRequestRuns(subject); + assert.deepEqual(subject.cancelled, []); + assert.match(subject.messages[0], /has reopened/); +}); + +test('does nothing when an open pull request reused the head branch', async () => { + const subject = fixture({ openForHead: [{ number: 1900 }] }); + await cancelClosedPullRequestRuns(subject); + assert.deepEqual(subject.cancelled, []); + assert.match(subject.messages[0], /belongs to an open pull request/); +}); + +test('accepts only a proven completion race', async () => { + const run = { + id: 10, + name: 'Core E2E Tests', + status: 'in_progress', + head_branch: 'topic/old', + head_repository: { full_name: 'rcourtman/Pulse' }, + }; + const raced = fixture({ runs: { in_progress: [run] }, cancelError: new Error('409') }); + await cancelClosedPullRequestRuns(raced); + assert.match(raced.messages[0], /completed before cancellation/); + assert.match(raced.messages.at(-1), /Requested cancellation for 0 of 1/); + + const failed = fixture({ + runs: { in_progress: [run] }, + cancelError: new Error('authentication failed'), + refreshed: 'in_progress', + }); + await assert.rejects(cancelClosedPullRequestRuns(failed), /authentication failed/); +}); diff --git a/.github/workflows/reclaim-closed-pr-capacity.yml b/.github/workflows/reclaim-closed-pr-capacity.yml new file mode 100644 index 000000000..8b457c906 --- /dev/null +++ b/.github/workflows/reclaim-closed-pr-capacity.yml @@ -0,0 +1,33 @@ +name: Reclaim closed PR CI capacity + +# A closed pull request no longer needs its queued or running verdicts. Run in +# the base repository context so fork closures can release hosted capacity too; +# no pull-request code, artifact, cache, or secret enters this privileged job. +on: + pull_request_target: + types: [closed] + +permissions: + actions: write + contents: read + pull-requests: read + +jobs: + cancel: + name: Cancel obsolete pull request runs + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Check out the reviewed cancellation helper + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + sparse-checkout: .github/scripts/reclaim-closed-pr-capacity.cjs + sparse-checkout-cone-mode: false + + - name: Cancel unfinished runs for the closed head + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 + with: + script: | + const cleanup = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/reclaim-closed-pr-capacity.cjs`); + await cleanup.cancelClosedPullRequestRuns({ github, context, core }); diff --git a/scripts/check_workflow_trust.py b/scripts/check_workflow_trust.py index 3697353f3..9c20c12a4 100644 --- a/scripts/check_workflow_trust.py +++ b/scripts/check_workflow_trust.py @@ -155,6 +155,11 @@ GENERATED_CODE_ACTION_INPUTS = { "azure/cli@": frozenset({"inlinescript"}), "azure/powershell@": frozenset({"inlinescript"}), } +SAFE_PULL_REQUEST_TARGET_WORKFLOW = "reclaim-closed-pr-capacity.yml" +SAFE_PULL_REQUEST_TARGET_ACTIONS = ( + "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1", + "actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b", +) # v7.0.1 includes checkout's fail-closed fork-PR protection for privileged # pull_request_target and workflow_run events. Keep this exact-pin allowlist # reviewable: a dependency refresh must not silently discard that boundary. @@ -822,6 +827,95 @@ def _has_trigger(lines: list[str], event: str) -> bool: return False +def _is_hardened_closed_pr_cancellation(path: Path, lines: list[str]) -> bool: + """Recognise the one metadata-only privileged PR automation we permit.""" + if path.name != SAFE_PULL_REQUEST_TARGET_WORKFLOW: + return False + + significant = [ + line.rstrip() + for line in lines + if line.strip() and not line.lstrip().startswith("#") + ] + try: + trigger_index = significant.index("on:") + permissions_index = significant.index("permissions:") + except ValueError: + return False + if significant[trigger_index:permissions_index] != [ + "on:", + " pull_request_target:", + " types: [closed]", + ]: + return False + + try: + jobs_index = significant.index("jobs:") + except ValueError: + return False + if significant[permissions_index:jobs_index] != [ + "permissions:", + " actions: write", + " contents: read", + " pull-requests: read", + ]: + return False + + if any(RUN_RE.match(line.split("#", 1)[0]) for line in lines): + return False + dependencies = [ + match.group(1).strip("'\"") + for line in lines + if (match := USES_RE.match(line.split("#", 1)[0])) + ] + if dependencies != list(SAFE_PULL_REQUEST_TARGET_ACTIONS): + return False + if _has_confidential_secret_reference(lines): + return False + if any( + re.match(r"^\s*(?:container|services|defaults|env)\s*:", line) + for line in lines + ): + return False + + checkout_index = next( + index + for index, line in enumerate(lines) + if ( + (match := USES_RE.match(line.split("#", 1)[0])) + and match.group(1).strip("'\"") == SAFE_PULL_REQUEST_TARGET_ACTIONS[0] + ) + ) + checkout_block = _action_block(lines, checkout_index) + if any( + re.match( + rf"^\s*(?:{_yaml_key('repository')}|{_yaml_key('ref')}|" + rf"{_yaml_key('path')}|{_yaml_key('allow-unsafe-pr-checkout')})\s*:", + line, + ) + for _, line in checkout_block + ): + return False + + # The privileged generated program may only load the sparse, protected + # default-branch helper and invoke its metadata reconciliation entry point. + script_lines = [ + script_line.strip() + for index, line in enumerate(lines) + if ( + (match := USES_RE.match(line.split("#", 1)[0])) + and match.group(1).strip("'\"") == SAFE_PULL_REQUEST_TARGET_ACTIONS[1] + ) + for _, script_line in _action_generated_code_lines( + lines, index, frozenset({"script"}) + ) + ] + return script_lines == [ + "const cleanup = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/reclaim-closed-pr-capacity.cjs`);", + "await cleanup.cancelClosedPullRequestRuns({ github, context, core });", + ] + + def _static_yaml_list( lines: list[str], key_index: int, inline_value: str ) -> list[str] | None: @@ -1181,7 +1275,10 @@ def audit_workflow(path: Path) -> list[Finding]: findings.extend(_audit_workflow_run_trigger(path, lines)) has_workflow_run_trigger = _has_trigger(lines, "workflow_run") - if _has_trigger(lines, "pull_request_target"): + if ( + _has_trigger(lines, "pull_request_target") + and not _is_hardened_closed_pr_cancellation(path, lines) + ): findings.append( Finding( path, diff --git a/scripts/tests/test-reclaim-closed-pr-capacity.sh b/scripts/tests/test-reclaim-closed-pr-capacity.sh new file mode 100755 index 000000000..82a1cb524 --- /dev/null +++ b/scripts/tests/test-reclaim-closed-pr-capacity.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +set -euo pipefail + +root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +node --test "${root}/.github/scripts/reclaim-closed-pr-capacity.test.cjs" diff --git a/scripts/tests/test_workflow_trust.py b/scripts/tests/test_workflow_trust.py index 969c7a0c1..a45006836 100644 --- a/scripts/tests/test_workflow_trust.py +++ b/scripts/tests/test_workflow_trust.py @@ -24,9 +24,9 @@ CHECKOUT_PIN = next(iter(workflow_trust.PROTECTED_CHECKOUT_PINS)) class WorkflowTrustTest(unittest.TestCase): - def audit(self, content: str) -> list[str]: + def audit(self, content: str, name: str = "test.yml") -> list[str]: with tempfile.TemporaryDirectory() as temporary_directory: - path = Path(temporary_directory) / "test.yml" + path = Path(temporary_directory) / name path.write_text(content, encoding="utf-8") return [finding.message for finding in workflow_trust.audit_workflow(path)] @@ -362,6 +362,50 @@ steps: ) self.assertTrue(any("must not opt out" in finding for finding in findings)) + def test_only_allows_hardened_closed_pr_target_cancellation(self) -> None: + workflow = ( + REPO_ROOT + / ".github" + / "workflows" + / "reclaim-closed-pr-capacity.yml" + ).read_text() + findings = self.audit( + workflow, + workflow_trust.SAFE_PULL_REQUEST_TARGET_WORKFLOW, + ) + self.assertEqual(findings, []) + + unsafe = workflow.replace( + " await cleanup.cancelClosedPullRequestRuns({ github, context, core });", + " require('child_process').exec('git fetch origin pull/1/head');\n" + " await cleanup.cancelClosedPullRequestRuns({ github, context, core });", + ) + findings = self.audit( + unsafe, + workflow_trust.SAFE_PULL_REQUEST_TARGET_WORKFLOW, + ) + self.assertTrue( + any("pull_request_target is prohibited" in finding for finding in findings) + ) + + unsafe_checkout = workflow.replace( + " persist-credentials: false", + " persist-credentials: false\n" + " repository: ${{ github.event.pull_request.head.repo.full_name }}", + ) + findings = self.audit( + unsafe_checkout, + workflow_trust.SAFE_PULL_REQUEST_TARGET_WORKFLOW, + ) + self.assertTrue( + any("pull_request_target is prohibited" in finding for finding in findings) + ) + + findings = self.audit(workflow) + self.assertTrue( + any("pull_request_target is prohibited" in finding for finding in findings) + ) + def test_workflow_run_requires_canonical_upstream_code(self) -> None: missing_branch = self.audit( """on: