mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
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
This commit is contained in:
@@ -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 };
|
||||
@@ -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/);
|
||||
});
|
||||
@@ -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 });
|
||||
@@ -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,
|
||||
|
||||
+5
@@ -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"
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user