From 53cbf764669f020c5dc65bea3858b7d362e97d49 Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Wed, 4 Nov 2026 16:19:32 +0100 Subject: [PATCH 1/9] chore(ci): validate pull request metadata for privileged workflows Public PR automation loses its default event allowance on 2 December. Fork and conflicted PRs still need metadata events, while existing backport callers need to retain their direct event behavior. A verified relay and Node preload bridge that gap without loading contributor code. --- .github/actions/resolve-pr-event/README.md | 69 ++++++ .github/actions/resolve-pr-event/action.yml | 47 ++++ .../resolve-pr-event/src/event-bootstrap.cjs | 20 ++ .../actions/resolve-pr-event/src/resolve.py | 216 ++++++++++++++++++ .../test/test_backport_contract.py | 24 ++ .../resolve-pr-event/test/test_resolve.py | 208 +++++++++++++++++ .github/workflows/backport.yaml | 57 +++-- .github/workflows/test-resolve-pr-event.yaml | 26 +++ Makefile | 5 + README.md | 11 + docs/workflows/backport.md | 1 + 11 files changed, 663 insertions(+), 21 deletions(-) create mode 100644 .github/actions/resolve-pr-event/README.md create mode 100644 .github/actions/resolve-pr-event/action.yml create mode 100644 .github/actions/resolve-pr-event/src/event-bootstrap.cjs create mode 100644 .github/actions/resolve-pr-event/src/resolve.py create mode 100644 .github/actions/resolve-pr-event/test/test_backport_contract.py create mode 100644 .github/actions/resolve-pr-event/test/test_resolve.py create mode 100644 .github/workflows/test-resolve-pr-event.yaml diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md new file mode 100644 index 00000000..1bcb8952 --- /dev/null +++ b/.github/actions/resolve-pr-event/README.md @@ -0,0 +1,69 @@ +# Resolve pull request event + +Resolve a completed `workflow_run` from the `pull_request_target` relay at +`.github/workflows/pull-request-events.yaml` into a synthetic event containing +the current pull request and repository from GitHub. +The caller supplies `source-workflow` and a token with `actions: read`, +`contents: read`, and `pull-requests: read`. The relay must upload an artifact +named `pull-request-event-`, containing only `event.json` with +`number`, `action`, and `label`. The label is an empty string unless the action +is `labeled`. + +Check `eligible == 'true'` before consuming outputs. Pass `event-json` through a +trusted workflow input to restore it in a later job. For each Node action that +needs the synthetic event, set `NODE_OPTIONS` to `node-options` and +`RESOLVED_PR_EVENT_PATH` to `event-path` in its step environment. The preload +validates the file and sets the Node process event context before the action +starts. The temporary file contains the current PR payload; avoid printing it. An unavailable artifact, +stale label, or failed provenance check produces a warning and an +ineligible result. API errors fail the action. + +Validation uses the exact workflow ID and path in the base repository and +GitHub's `pull_request_target` guarantee that the workflow comes from the base +repository default branch. The run head SHA belongs to the PR and +is not used as the workflow source SHA. The relay may only capture and upload metadata. It must not check out or execute +PR code or use business secrets. +See [GitHub's pull_request_target security guidance](https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target). + +## Event policy and rollout + +The relay still uses `pull_request_target` to preserve events from conflicted +PRs. It needs an explicit Actions event policy on each public caller repository. +Scope the policy to `.github/workflows/pull-request-events.yaml` and allow only +`pull_request_target`. The privileged consumers use `workflow_run` outside this allowance. +An administrator creates the policy separately from merging these changes. + +Merge the shared action and backport workflow changes before the caller PRs. +Publish `resolve-pr-event/v1` and update `backport/v1` to that merged commit before +activating callers. Verify the tags before merging callers. Test a branch PR, a +fork PR, a conflicted fork PR, a merged backport, and each Linear integration. +Local tests and lint do not prove these live events. + +## Inputs + + + +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|-----------------|--------|----------|-------------------------|-------------------------------------------------------------------| +| event-json | string | false | | Trusted payload passed between jobs through
workflow_call. | +| github-token | string | false | `"${{ github.token }}"` | Token with actions, contents, and pull
requests read access. | +| source-workflow | string | false | | Expected relay path, .github/workflows/pull-request-events.yaml. | + + + +## Outputs + + + +| OUTPUT | TYPE | DESCRIPTION | +|-----------------------|--------|------------------------------------------------------------------------------| +| action | string | Pull request action. | +| body-without-comments | string | PR body with HTML comments removed,
for follow-up descriptions. | +| eligible | string | Whether a validated event is available. | +| event-json | string | Synthetic event JSON for passing to
another job. | +| event-path | string | Temporary synthetic event file for RESOLVED_PR_EVENT_PATH. | +| label | string | Current label name for labeled events. | +| node-options | string | NODE_OPTIONS preload for Node actions that
consume the synthetic event. | +| pr-number | string | Pull request number. | + + diff --git a/.github/actions/resolve-pr-event/action.yml b/.github/actions/resolve-pr-event/action.yml new file mode 100644 index 00000000..791d32bc --- /dev/null +++ b/.github/actions/resolve-pr-event/action.yml @@ -0,0 +1,47 @@ +name: Resolve pull request event +description: Validate a completed pull request relay or restore a trusted event payload. +inputs: + github-token: + description: Token with actions, contents, and pull requests read access. + default: ${{ github.token }} + source-workflow: + description: Expected relay path, .github/workflows/pull-request-events.yaml. + required: false + event-json: + description: Trusted payload passed between jobs through workflow_call. + required: false +outputs: + eligible: + description: Whether a validated event is available. + value: ${{ steps.resolve.outputs.eligible }} + event-json: + description: Synthetic event JSON for passing to another job. + value: ${{ steps.resolve.outputs.event-json }} + event-path: + description: Temporary synthetic event file for RESOLVED_PR_EVENT_PATH. + value: ${{ steps.resolve.outputs.event-path }} + node-options: + description: NODE_OPTIONS preload for Node actions that consume the synthetic event. + value: ${{ steps.resolve.outputs.node-options }} + pr-number: + description: Pull request number. + value: ${{ steps.resolve.outputs.pr-number }} + action: + description: Pull request action. + value: ${{ steps.resolve.outputs.action }} + label: + description: Current label name for labeled events. + value: ${{ steps.resolve.outputs.label }} + body-without-comments: + description: PR body with HTML comments removed, for follow-up descriptions. + value: ${{ steps.resolve.outputs.body-without-comments }} +runs: + using: composite + steps: + - id: resolve + shell: bash + env: + INPUT_GITHUB_TOKEN: ${{ inputs.github-token }} + INPUT_SOURCE_WORKFLOW: ${{ inputs.source-workflow }} + INPUT_EVENT_JSON: ${{ inputs.event-json }} + run: python3 "${{ github.action_path }}/src/resolve.py" diff --git a/.github/actions/resolve-pr-event/src/event-bootstrap.cjs b/.github/actions/resolve-pr-event/src/event-bootstrap.cjs new file mode 100644 index 00000000..d034d659 --- /dev/null +++ b/.github/actions/resolve-pr-event/src/event-bootstrap.cjs @@ -0,0 +1,20 @@ +'use strict'; + +// Node actions initialize @actions/github before their entry point can adjust +// runner-provided event variables. Validate the resolver's file before loading. +const fs = require('node:fs'); +try { + const path = process.env.RESOLVED_PR_EVENT_PATH; + const repository = process.env.GITHUB_REPOSITORY; + if (!path || !repository) throw new Error(); + const event = JSON.parse(fs.readFileSync(path, 'utf8')); + const number = event?.pull_request?.number; + const eventRepository = event?.repository?.full_name; + if (!Number.isSafeInteger(number) || number <= 0 || + typeof eventRepository !== 'string' || + eventRepository.toLowerCase() !== repository.toLowerCase()) throw new Error(); + process.env.GITHUB_EVENT_PATH = path; + process.env.GITHUB_EVENT_NAME = 'pull_request_target'; +} catch (_) { + throw new Error('Unable to restore validated pull request event'); +} diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py new file mode 100644 index 00000000..389c47ba --- /dev/null +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""Resolve an untrusted workflow_run artifact into an authoritative PR event.""" +import io +import json +import os +import re +import sys +import tempfile +import urllib.error +import urllib.parse +import urllib.request +import uuid +import zipfile + +MAX_ARCHIVE = 128 * 1024 +MAX_JSON = 32 * 1024 +RELAY = '.github/workflows/pull-request-events.yaml' +ACTIONS = {'opened', 'edited', 'labeled', 'closed'} + +class Reject(Exception): + pass + +def unique_pairs(pairs): + result = {} + for key, value in pairs: + if key in result: + raise Reject('duplicate JSON key') + result[key] = value + return result + +def parse_json(data): + try: + return json.loads(data, object_pairs_hook=unique_pairs) + except (ValueError, UnicodeDecodeError, RecursionError) as exc: + raise Reject('invalid JSON') from exc + +def positive(value): + return type(value) is int and 0 < value <= 2**53 - 1 + +def decode_artifact(raw): + if len(raw) > MAX_ARCHIVE: + raise Reject('artifact archive too large') + try: + with zipfile.ZipFile(io.BytesIO(raw)) as archive: + entries = archive.infolist() + if len(entries) != 1 or entries[0].filename != 'event.json': + raise Reject('artifact must contain only event.json') + entry = entries[0] + if entry.is_dir() or entry.file_size > MAX_JSON or entry.flag_bits & 1: + raise Reject('invalid artifact member') + with archive.open(entry) as member: + data = member.read(MAX_JSON + 1) + if len(data) > MAX_JSON: + raise Reject('artifact member too large') + except (zipfile.BadZipFile, RuntimeError, OSError, EOFError) as exc: + raise Reject('invalid artifact archive') from exc + obj = parse_json(data) + if not isinstance(obj, dict) or set(obj) != {'number', 'action', 'label'}: + raise Reject('invalid artifact fields') + if not positive(obj['number']) or obj['action'] not in ACTIONS: + raise Reject('invalid artifact event') + label = obj['label'] + if not isinstance(label, str) or len(label) > 255 or any(ord(c) < 32 for c in label): + raise Reject('invalid artifact label') + if (obj['action'] == 'labeled') != bool(label): + raise Reject('invalid event label') + return obj + +class SafeRedirect(urllib.request.HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + redirected = super().redirect_request(req, fp, code, msg, headers, newurl) + if redirected and urllib.parse.urlsplit(newurl).netloc != urllib.parse.urlsplit(req.full_url).netloc: + redirected.remove_header('Authorization') + return redirected + +class API: + def __init__(self, token, repository): + self.root = os.environ.get('GITHUB_API_URL', 'https://api.github.com').rstrip('/') + self.repo = repository + self.token = token + + def get(self, path, binary=False, allow_missing=False): + request = urllib.request.Request(self.root + '/repos/' + self.repo + path, + headers={'Authorization': 'Bearer ' + self.token, 'Accept': 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28'}) + try: + with urllib.request.build_opener(SafeRedirect()).open(request, timeout=30) as response: + raw = response.read((MAX_ARCHIVE if binary else 4 * 1024 * 1024) + 1) + except urllib.error.HTTPError as exc: + if allow_missing and exc.code == 404: + return None + raise RuntimeError('GitHub API request failed (HTTP %d)' % exc.code) from exc + except urllib.error.URLError as exc: + raise RuntimeError('GitHub API request failed') from exc + if len(raw) > (MAX_ARCHIVE if binary else 4 * 1024 * 1024): + raise Reject('API response too large') + return raw if binary else parse_json(raw) + +def resolve(api, event, source): + if source != RELAY: + raise Reject('source-workflow must name the pull request relay') + if not isinstance(event, dict) or event.get('action') != 'completed': + raise Reject('not a completed workflow run') + payload_run = event.get('workflow_run') or {} + run_id, attempt = payload_run.get('id'), payload_run.get('run_attempt') + if not positive(run_id) or not positive(attempt): + raise Reject('invalid run identity') + run = api.get('/actions/runs/%d/attempts/%d' % (run_id, attempt)) + if (run.get('id') != run_id or run.get('run_attempt') != attempt or + run.get('event') != 'pull_request_target' or run.get('status') != 'completed' or + run.get('conclusion') != 'success' or run.get('path') != source): + raise Reject('source run is not an eligible relay') + workflow = api.get('/actions/workflows/' + urllib.parse.quote(source.rsplit('/', 1)[-1], safe='')) + run_repo = run.get('repository') or {} + if (not positive(run.get('workflow_id')) or workflow.get('id') != run.get('workflow_id') or + workflow.get('path') != source or not isinstance(run_repo.get('full_name'), str) or + run_repo['full_name'].lower() != api.repo.lower()): + raise Reject('source workflow or repository mismatch') + # pull_request_target executes the workflow from the base repository's + # default branch. The run head_sha is the PR head, not the workflow SHA. + repo = api.get('') + if repo.get('full_name', '').lower() != api.repo.lower(): + raise Reject('repository identity mismatch') + artifacts = [] + for page in range(1, 11): + result = api.get('/actions/runs/%d/artifacts?per_page=100&page=%d' % (run_id, page)) + batch = result.get('artifacts', []) + artifacts.extend(a for a in batch if a.get('name') == 'pull-request-event-%d' % attempt and not a.get('expired')) + if len(batch) < 100: + break + else: + raise Reject('too many artifacts') + if len(artifacts) != 1: + raise Reject('missing or ambiguous relay artifact; rerun the metadata relay') + artifact = artifacts[0] + if not positive(artifact.get('id')) or artifact.get('workflow_run', {}).get('id') != run_id: + raise Reject('artifact run mismatch') + artifact_repo_id = artifact.get('workflow_run', {}).get('repository_id') + if artifact_repo_id is not None and artifact_repo_id != repo.get('id'): + raise Reject('artifact repository mismatch') + seed = decode_artifact(api.get('/actions/artifacts/%d/zip' % artifact['id'], binary=True)) + pr = api.get('/pulls/%d' % seed['number'], allow_missing=True) + if not isinstance(pr, dict) or pr.get('number') != seed['number']: + raise Reject('pull request not found') + base_repo = ((pr.get('base') or {}).get('repo') or {}).get('full_name') + if not isinstance(base_repo, str) or base_repo.lower() != api.repo.lower(): + raise Reject('pull request repository mismatch') + label = None + if seed['action'] == 'labeled': + label = next((item for item in pr.get('labels', []) if isinstance(item, dict) and item.get('name') == seed['label']), None) + if label is None: + raise Reject('label no longer exists on pull request; restore the label and rerun the metadata relay') + result = {'action': seed['action'], 'number': seed['number'], 'pull_request': pr, 'repository': repo} + if label is not None: + result['label'] = label + return result + +def validate_restore(event, repository): + if not isinstance(event, dict) or not isinstance(event.get('pull_request'), dict): + raise Reject('invalid restored event') + number = event['pull_request'].get('number') + repo = event.get('repository') + if not positive(number) or not isinstance(repo, dict) or not isinstance(repo.get('full_name'), str) or repo['full_name'].lower() != repository.lower(): + raise Reject('restored event identity mismatch') + if event.get('action') not in ACTIONS or (event.get('number', number) != number): + raise Reject('invalid restored event action') + return event + +def write_outputs(event): + encoded = json.dumps(event, ensure_ascii=True, separators=(',', ':')) + with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', dir=os.environ['RUNNER_TEMP'], prefix='pr-event-', suffix='.json', delete=False) as f: + f.write(encoded) + path = f.name + label = event.get('label') or {} + bootstrap = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'event-bootstrap.cjs') + outputs = {'eligible': 'true', 'event-json': encoded, 'event-path': path, + 'node-options': '--require=' + json.dumps(bootstrap, ensure_ascii=False), + 'pr-number': str(event['pull_request']['number']), 'action': event['action'], + 'label': label.get('name', '') if isinstance(label, dict) else '', + 'body-without-comments': re.sub(r'', '', event['pull_request'].get('body') or '', flags=re.S)} + write_output_values(outputs) + +def write_output_values(outputs): + with open(os.environ['GITHUB_OUTPUT'], 'a', encoding='utf-8') as f: + for key, value in outputs.items(): + delimiter = 'ghadelimiter_' + uuid.uuid4().hex + while delimiter in value: + delimiter = 'ghadelimiter_' + uuid.uuid4().hex + f.write('%s<<%s\n%s\n%s\n' % (key, delimiter, value, delimiter)) + +def main(): + try: + repository = os.environ['GITHUB_REPOSITORY'] + restored = os.environ.get('INPUT_EVENT_JSON', '') + if restored: + if len(restored.encode('utf-8')) > 4 * 1024 * 1024: + raise Reject('restored event too large') + event = validate_restore(parse_json(restored), repository) + else: + if os.environ.get('GITHUB_EVENT_NAME') != 'workflow_run': + raise Reject('unsupported event') + with open(os.environ['GITHUB_EVENT_PATH'], 'rb') as f: + payload = parse_json(f.read(4 * 1024 * 1024 + 1)) + event = resolve(API(os.environ['INPUT_GITHUB_TOKEN'], repository), payload, + os.environ.get('INPUT_SOURCE_WORKFLOW', '')) + write_outputs(event) + except Reject as exc: + print('::warning::' + str(exc)) + write_output_values({'eligible': 'false'}) + except (RuntimeError, OSError, KeyError, TypeError) as exc: + print('::error::Unable to resolve pull request event: ' + type(exc).__name__, file=sys.stderr) + return 1 + return 0 + +if __name__ == '__main__': + sys.exit(main()) diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py new file mode 100644 index 00000000..a9dc882f --- /dev/null +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -0,0 +1,24 @@ +"""Guard compatibility between direct PR callers and workflow_run handoffs.""" +from pathlib import Path +import re + +WORKFLOW = Path(__file__).resolve().parents[3] / 'workflows' / 'backport.yaml' + + +def test_existing_callers_keep_the_original_event_as_fallback(): + text = WORKFLOW.read_text() + assert "default: ''" in text.split(' pr-event:', 1)[1].split(' legacy-split:', 1)[0] + assert 'fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true' in text + assert not re.search(r'github\.event\.(pull_request|action|label)', text) + + +def test_backport_action_receives_the_restored_payload_without_changing_other_steps(): + text = WORKFLOW.read_text() + assert "if: inputs.pr-event != ''" in text + assert 'event-json: ${{ inputs.pr-event }}' in text + assert 'NODE_OPTIONS: ${{ steps.pr-event.outputs.node-options }}' in text + assert 'RESOLVED_PR_EVENT_PATH: ${{ steps.pr-event.outputs.event-path }}' in text + assert 'GITHUB_EVENT_PATH:' not in text + assert 'cancel-in-progress: false' in text + assert 'needs: [backport, legacy-backport]' in text + assert 'continue-on-error: true' in text diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py new file mode 100644 index 00000000..90455e50 --- /dev/null +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -0,0 +1,208 @@ +import importlib.util +import io +import json +import os +from pathlib import Path +import zipfile + +import pytest + +spec = importlib.util.spec_from_file_location('resolve', Path(__file__).parents[1] / 'src/resolve.py') +r = importlib.util.module_from_spec(spec) +spec.loader.exec_module(r) +SHA = 'a' * 40 +DEFAULT = 'b' * 40 + +def archive(data=None, name='event.json', extra=None): + buf = io.BytesIO() + with zipfile.ZipFile(buf, 'w') as z: + z.writestr(name, data if data is not None else json.dumps({'number': 7, 'action': 'labeled', 'label': 'backport'})) + if extra: + z.writestr(extra, '{}') + return buf.getvalue() + +class FakeAPI: + repo = 'org/repo' + def __init__(self): + self.run = {'id': 123, 'run_attempt': 1, 'event': 'pull_request_target', 'workflow_id': 42, 'repository': {'full_name': 'org/repo'}, 'status': 'completed', 'conclusion': 'success', 'path': r.RELAY, 'head_sha': SHA, 'head_branch': 'feature', 'head_repository': {'full_name': 'fork/repo'}} + self.pr = {'number': 7, 'head': {'sha': SHA, 'ref': 'feature', 'repo': {'full_name': 'fork/repo'}}, 'base': {'repo': {'full_name': 'org/repo'}}, 'labels': [{'name': 'backport'}], 'merged': True, 'body': 'authoritative'} + self.data = archive() + self.items = [{'id': 9, 'name': 'pull-request-event-1', 'expired': False, 'workflow_run': {'id': 123}}] + def get(self, path, binary=False, allow_missing=False): + if path.startswith('/actions/runs/123/attempts/'): + return self.run + if path.startswith('/actions/workflows/'): + return {'id': 42, 'path': r.RELAY} + if path == '': + return {'full_name': self.repo, 'default_branch': 'main', 'id': 12} + if '/artifacts?' in path: + return {'artifacts': self.items} + if path.startswith('/actions/artifacts/'): + return self.data + if path.startswith('/pulls/'): + return self.pr + raise AssertionError(path) + +def event(attempt=1): + return {'action': 'completed', 'workflow_run': {'id': 123, 'run_attempt': attempt}} + +def test_fork_event_uses_authoritative_pr(): + api = FakeAPI() + result = r.resolve(api, event(), r.RELAY) + assert result['pull_request']['body'] == 'authoritative' + assert result['label']['name'] == 'backport' + assert result['pull_request']['merged'] is True + +def test_same_repo_branch(): + api = FakeAPI() + api.run['head_repository']['full_name'] = api.pr['head']['repo']['full_name'] = 'org/repo' + assert r.resolve(api, event(), r.RELAY)['number'] == 7 + +@pytest.mark.parametrize('field,value', [('event', 'pull_request'), ('event', 'push'), ('status', 'in_progress'), ('conclusion', 'failure'), ('path', 'wrong'), ('run_attempt', 2)]) +def test_invalid_run(field, value): + api = FakeAPI() + api.run[field] = value + with pytest.raises(r.Reject): + r.resolve(api, event(), r.RELAY) + +@pytest.mark.parametrize('change', ['number', 'base_repo', 'run_repo', 'workflow_id']) +def test_forged_pr_provenance(change): + api = FakeAPI() + if change == 'number': api.pr['number'] = 8 + if change == 'base_repo': api.pr['base']['repo']['full_name'] = 'other/repo' + if change == 'run_repo': api.run['repository']['full_name'] = 'other/repo' + if change == 'workflow_id': api.run['workflow_id'] = 99 + with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) + +def test_target_event_base_sha_and_deleted_fork(): + api = FakeAPI() + api.pr['head']['sha'] = DEFAULT + api.pr['head']['repo'] = None + api.pr['merged'] = True + assert r.resolve(api, event(), r.RELAY)['pull_request']['merged'] is True + +def test_fork_head_cannot_change_relay(): + # Fake API rejects any contents lookup: the fork head can omit or modify + # the relay, while pull_request_target executes the default branch workflow. + api = FakeAPI() + api.run['head_sha'] = 'c' * 40 + assert r.resolve(api, event(), r.RELAY)['number'] == 7 + +def test_stale_label(): + api = FakeAPI() + api.pr['labels'] = [] + with pytest.raises(r.Reject, match='restore the label'): r.resolve(api, event(), r.RELAY) + +def test_artifact_repository_mismatch(): + api = FakeAPI() + api.items[0]['workflow_run']['repository_id'] = 999 + with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) + api.items[0]['workflow_run']['repository_id'] = 12 + assert r.resolve(api, event(), r.RELAY)['number'] == 7 + +def test_workflow_path_and_id_must_match(): + api = FakeAPI() + api.run['workflow_id'] = 77 + with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) + api = FakeAPI() + with pytest.raises(r.Reject): r.resolve(api, event(), '.github/workflows/other.yaml') + +def test_attempt_specific_artifact(): + api = FakeAPI() + api.run['run_attempt'] = 2 + with pytest.raises(r.Reject): r.resolve(api, event(2), r.RELAY) + api.items[0]['name'] = 'pull-request-event-2' + assert r.resolve(api, event(2), r.RELAY)['number'] == 7 + api.items.append(api.items[0].copy()) + with pytest.raises(r.Reject): r.resolve(api, event(2), r.RELAY) + +@pytest.mark.parametrize('raw', [archive(name='../event.json'), archive(extra='second'), archive(data='{"number":7,"number":8,"action":"opened","label":""}'), archive(data='x' * (r.MAX_JSON + 1)), b'not a zip', b'x' * (r.MAX_ARCHIVE + 1), archive(data='{"number":7,"action":"opened","label":"","pull_request":{}}')]) +def test_untrusted_archives(raw): + with pytest.raises(r.Reject): r.decode_artifact(raw) + +def test_restore_and_output_injection(tmp_path, monkeypatch): + payload = {'action': 'opened', 'pull_request': {'number': 7, 'body': 'hello\nEOF\nother=evil'}, 'repository': {'full_name': 'org/repo'}} + assert r.validate_restore(payload, 'org/repo') == payload + with pytest.raises(r.Reject): r.validate_restore(payload, 'other/repo') + output = tmp_path / 'out' + monkeypatch.setenv('GITHUB_OUTPUT', str(output)) + monkeypatch.setenv('RUNNER_TEMP', str(tmp_path)) + r.write_outputs(payload) + lines = output.read_text().splitlines() + for index, line in enumerate(lines): + if '<<' in line: + delimiter = line.split('<<', 1)[1] + assert lines.count(delimiter) == 1 + eventfile = next(tmp_path.glob('pr-event-*.json')) + assert json.loads(eventfile.read_text()) == payload + +def test_restore_rejects_bad_numbers(): + with pytest.raises(r.Reject): r.validate_restore({'action': 'opened', 'pull_request': {'number': True}, 'repository': {'full_name': 'org/repo'}}, 'org/repo') + +@pytest.mark.parametrize('body,expected', [ + ('beforeafter\nEOF\nother=evil', 'beforeafter\nEOF\nother=evil'), + (None, ''), + ('keep normal text\nand an unfinished " body: ${{ steps.backport.outputs.comment-body }} - pr-number: ${{ github.event.pull_request.number }} + pr-number: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} github-token: ${{ secrets.gh-access-token }} expected-author: ${{ steps.pat.outputs.login }} @@ -282,9 +297,9 @@ jobs: if: | always() && !cancelled() && - github.event.pull_request.merged == true && - contains(join(github.event.pull_request.labels.*.name, ','), 'backport-to-') && - (github.event.action == 'closed' || startsWith(github.event.label.name, 'backport-to-')) + fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true && + contains(join(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name, ','), 'backport-to-') && + (fromJSON(inputs.pr-event || toJSON(github.event)).action == 'closed' || startsWith(fromJSON(inputs.pr-event || toJSON(github.event)).label.name, 'backport-to-')) runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -294,9 +309,9 @@ jobs: continue-on-error: true uses: loft-sh/github-actions/.github/actions/link-backport-prs@link-backport-prs/v1 # zizmor: ignore[unpinned-uses] -- internal loft-sh/github-actions ref; reusable workflows check out the CALLER repo, so a local ./.github/actions path is not present here. @link-backport-prs/v1 is the floating coordination tag kept aligned with backport/v1 (see DEVOPS-923 pin-drift class). with: - source-pr: ${{ github.event.pull_request.number }} + source-pr: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} repo-owner: ${{ github.repository_owner }} - repo-name: ${{ github.event.repository.name }} + repo-name: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).repository.name }} additional-repos: ${{ inputs.legacy-split && format('{0},{1}', inputs.oss-repo, inputs.pro-repo) || '' }} github-token: ${{ secrets.gh-access-token }} # zizmor: ignore[secrets-outside-env] -- PAT passed through workflow_call and required for cross-repo PR edits linear-token: ${{ secrets.linear-token }} # zizmor: ignore[secrets-outside-env] -- optional token passed through workflow_call diff --git a/.github/workflows/test-resolve-pr-event.yaml b/.github/workflows/test-resolve-pr-event.yaml new file mode 100644 index 00000000..c7c8fa7f --- /dev/null +++ b/.github/workflows/test-resolve-pr-event.yaml @@ -0,0 +1,26 @@ +name: Test resolve-pr-event +on: + push: + branches: [main] + paths: + - '.github/actions/resolve-pr-event/**' + - '.github/workflows/test-resolve-pr-event.yaml' + pull_request: + paths: + - '.github/actions/resolve-pr-event/**' + - '.github/workflows/test-resolve-pr-event.yaml' +permissions: + contents: read +jobs: + pytest: + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + - name: Run tests + run: uv run --with pytest==8.3.3 pytest .github/actions/resolve-pr-event/test/ -q diff --git a/Makefile b/Makefile index 82429828..6a9fb55c 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ ACTIONS_DIR := .github/actions .PHONY: test-check-action-pins +.PHONY: test-resolve-pr-event WORKFLOWS_DIR := .github/workflows SCRIPTS_DIR := .github/scripts @@ -100,6 +101,10 @@ test-retry-approved-pr-ci: ## run approved PR CI retry pytest tests uv run --with pytest==8.3.3 pytest $(ACTIONS_DIR)/retry-approved-pr-ci/test/ -q test: test-check-action-pins +test: test-resolve-pr-event + +test-resolve-pr-event: ## run pull request event handoff tests + uv run --with pytest==8.3.3 pytest $(ACTIONS_DIR)/resolve-pr-event/test/ -q test-ghcr-tag-guard: ## run GHCR publication guard tests uv run --with pytest==8.3.3 pytest $(ACTIONS_DIR)/ghcr-tag-guard/test/ -q diff --git a/README.md b/README.md index d5d98421..4bca64dd 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,17 @@ are written, tested, and structured. Join the testing tailnet from trusted GitHub-hosted Linux jobs using OIDC. See [testing-tailnet README](./.github/actions/testing-tailnet/README.md) for trust and cleanup requirements. +### Resolve pull request event + +Validates the identity of a read-only PR metadata relay and rebuilds its event +from the GitHub API for privileged `workflow_run` consumers. Supports forks +without checking out contributor code. + +Location: `.github/actions/resolve-pr-event` + +See [resolve-pr-event](./.github/actions/resolve-pr-event/README.md) for the trust +boundary, event policy, inputs, and outputs. + ### GHCR tag guard Checks every version tag before publication. Existing digests and ambiguous diff --git a/docs/workflows/backport.md b/docs/workflows/backport.md index 5ec94462..dc2099c1 100644 --- a/docs/workflows/backport.md +++ b/docs/workflows/backport.md @@ -12,6 +12,7 @@ Creates backport PRs when a merged PR is labeled with `backport/`. | fix-imports | boolean | false | `false` | After the cherry-picks, run backport-fix-imports on
each backport branch: rewrite the repository's
own Go module paths to the
target line's major, resolve import-only conflicts,
and push a fix commit. For
repositories whose release lines sit on
an older module major. | | legacy-split | boolean | false | `false` | Enable the pre-monorepo (<= v0.36) split/re-root backport
flow. When true this workflow empties
auto_backport_label_prefix so sorenlouv honors the caller's
.backportrc.json branchLabelMapping regex (which MUST be scoped to the monorepo era, >= v0.37), and a
companion job routes <= v0.36 targets
through backport-legacy-split. Requires subtree-prefix, oss-repo and
pro-repo. | | oss-repo | string | false | | OSS repo as owner/repo for legacy
OSS-side backport PRs. Required when legacy-split
is true. | +| pr-event | string | false | | Validated PR event JSON from resolve-pr-event.
Omit for direct PR event callers. | | pro-repo | string | false | | Pro repo as owner/repo for the
pro half of mixed legacy backports.
Required when legacy-split is true. | | subtree-prefix | string | false | | OSS subtree path in the monorepo,
e.g. staging/github.com/loft-sh/vcluster. Required when legacy-split is
true. | From d449b8f65f53d96775e0a7bb97dd3c3e47605d03 Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Thu, 5 Nov 2026 13:39:30 +0100 Subject: [PATCH 2/9] fix(ci): isolate privileged workflows from pr events public pr automation still required privileged trigger exceptions, and claude comment handlers could check out fork code with credentials. moving only pr metadata left that exposure and policy dependency in place. the new backport contract needs its own major tag so callers can migrate in order while existing consumers keep their current release. --- .github/actions/check-action-pins/README.md | 2 +- .github/actions/commitlint/README.md | 4 +- .../link-backport-prs/src/main_test.go | 4 +- .github/actions/resolve-pr-event/README.md | 139 ++++--- .github/actions/resolve-pr-event/action.yml | 19 +- .../resolve-pr-event/src/event-bootstrap.cjs | 2 +- .../actions/resolve-pr-event/src/resolve.py | 300 ++++++++------- .../test/test_backport_contract.py | 123 +++++- .../resolve-pr-event/test/test_resolve.py | 363 +++++++++--------- .github/workflows/backport.yaml | 103 +++-- .github/workflows/claude-code-review.yaml | 25 +- .github/workflows/claude.yaml | 32 +- .github/workflows/claude.yml | 32 +- .github/workflows/test-backport.yaml | 7 +- .../workflows/test-claude-code-review.yaml | 6 +- .github/workflows/test-claude.yaml | 3 + .github/workflows/test-link-backport-prs.yaml | 4 + .github/workflows/test-resolve-pr-event.yaml | 14 + README.md | 15 +- docs/workflows/backport.md | 51 ++- docs/workflows/claude-code-review.md | 5 +- docs/workflows/claude.md | 2 +- 22 files changed, 780 insertions(+), 475 deletions(-) diff --git a/.github/actions/check-action-pins/README.md b/.github/actions/check-action-pins/README.md index 29ae7d99..9dcb072a 100644 --- a/.github/actions/check-action-pins/README.md +++ b/.github/actions/check-action-pins/README.md @@ -34,7 +34,7 @@ jobs: ``` Fork pull requests need only `contents: read`. The scanner reads repository files; -it does not run their scripts. Do not switch to `pull_request_target` to run this +it does not run their scripts. Do not switch to a privileged PR event to run this check. For a required check, avoid workflow path filters that can leave it pending. ## Runner requirements diff --git a/.github/actions/commitlint/README.md b/.github/actions/commitlint/README.md index 03f370da..63797ee2 100644 --- a/.github/actions/commitlint/README.md +++ b/.github/actions/commitlint/README.md @@ -234,10 +234,10 @@ above, take expressions normally. `contents: read` is enough. The action makes no API calls and needs no token. -Run it on `pull_request`, not `pull_request_target`. On a fork pull request the +Run it on `pull_request`, not a privileged PR event. On a fork pull request the action checks out and installs from the fork's own manifest; `pull_request` grants no secrets and a read-only token, which is what makes that safe. -`pull_request_target` runs with the base repository's secrets and would turn +a privileged PR event runs with the base repository's secrets and would turn the same install into a credential-exfiltration path. ## Testing diff --git a/.github/actions/link-backport-prs/src/main_test.go b/.github/actions/link-backport-prs/src/main_test.go index 4547e93a..65dd8ab5 100644 --- a/.github/actions/link-backport-prs/src/main_test.go +++ b/.github/actions/link-backport-prs/src/main_test.go @@ -360,8 +360,8 @@ func TestWorkflowRunsLinkerAfterAllProducers(t *testing.T) { t.Fatal("link-backports job is missing") } linkJob := workflow[linkStart:] - if !strings.Contains(linkJob, "needs: [backport, legacy-backport]") { - t.Fatal("link-backports must wait for modern and legacy producers") + if !strings.Contains(linkJob, "needs: [resolve-event, backport, legacy-backport]") { + t.Fatal("link-backports must wait for validated event and modern and legacy producers") } if !strings.Contains(linkJob, "/link-backport-prs@") { t.Fatal("link-backports job does not invoke link-backport-prs") diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md index 1bcb8952..ab391658 100644 --- a/.github/actions/resolve-pr-event/README.md +++ b/.github/actions/resolve-pr-event/README.md @@ -1,53 +1,83 @@ -# Resolve pull request event - -Resolve a completed `workflow_run` from the `pull_request_target` relay at -`.github/workflows/pull-request-events.yaml` into a synthetic event containing -the current pull request and repository from GitHub. -The caller supplies `source-workflow` and a token with `actions: read`, -`contents: read`, and `pull-requests: read`. The relay must upload an artifact -named `pull-request-event-`, containing only `event.json` with -`number`, `action`, and `label`. The label is an empty string unless the action -is `labeled`. - -Check `eligible == 'true'` before consuming outputs. Pass `event-json` through a -trusted workflow input to restore it in a later job. For each Node action that -needs the synthetic event, set `NODE_OPTIONS` to `node-options` and -`RESOLVED_PR_EVENT_PATH` to `event-path` in its step environment. The preload -validates the file and sets the Node process event context before the action -starts. The temporary file contains the current PR payload; avoid printing it. An unavailable artifact, -stale label, or failed provenance check produces a warning and an -ineligible result. API errors fail the action. - -Validation uses the exact workflow ID and path in the base repository and -GitHub's `pull_request_target` guarantee that the workflow comes from the base -repository default branch. The run head SHA belongs to the PR and -is not used as the workflow source SHA. The relay may only capture and upload metadata. It must not check out or execute -PR code or use business secrets. -See [GitHub's pull_request_target security guidance](https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target). - -## Event policy and rollout - -The relay still uses `pull_request_target` to preserve events from conflicted -PRs. It needs an explicit Actions event policy on each public caller repository. -Scope the policy to `.github/workflows/pull-request-events.yaml` and allow only -`pull_request_target`. The privileged consumers use `workflow_run` outside this allowance. -An administrator creates the policy separately from merging these changes. - -Merge the shared action and backport workflow changes before the caller PRs. -Publish `resolve-pr-event/v1` and update `backport/v1` to that merged commit before -activating callers. Verify the tags before merging callers. Test a branch PR, a -fork PR, a conflicted fork PR, a merged backport, and each Linear integration. -Local tests and lint do not prove these live events. +# Resolve merged pull requests + +Resolve merged pull requests for a default-branch push or a completed read-only pull request signal workflow. The action fetches authoritative PR data from GitHub and only returns closed, merged PRs targeting the default branch. Fork PRs are included after merge. + +For a push, the action walks every commit in the before/after range and finds associated PRs whose merge SHA is in that range. A forced push, deleted branch, or new branch with a zero before SHA is skipped. Comparison and associated PR pagination are bounded to 1,000 entries each; incomplete results fail closed. + +For `workflow_run`, the action verifies the run ID, attempt, status, conclusion, source workflow identity, repository, event, head branch and SHA against the API. It uses PR associations from the run or, when empty, from the commit API. A re-fetched PR must match the signal head branch and either its head SHA or merge SHA; its merge commit must be an ancestor of the default branch. No signal artifact is read. Since a signal workflow's code can come from a PR, it must be read-only and carry no secrets. Its output is never treated as authoritative. + +The action returns compact identity envelopes in `events-json`, at most 256, for a matrix. When exactly one PR is eligible, `event-json` also contains current `backport-to-` labels. The full re-fetched event is written only to a runner-local file, which can be supplied through the `NODE_OPTIONS` and `RESOLVED_PR_EVENT_PATH` outputs for Node actions. The aggregate job outputs are capped below GitHub's job output limit. The consumer must restore each envelope with this action before using it; restoration verifies the identity and merge SHA again and fetches current labels. Both selector and consumer require `contents: read` and `pull-requests: read`; the signal consumer also needs `actions: read`. All consumers must run from the default branch. + +Example signal workflow, `.github/workflows/pull-request-events.yaml`: + +```yaml +name: Pull request events +on: + pull_request: + types: [closed, labeled] +permissions: {} +jobs: + signal: + runs-on: ubuntu-latest + permissions: {} + steps: + - run: ':' +``` + +Example selector and fanout for a repository whose default branch is `main`: + +```yaml +name: Backport on merge +on: + push: + branches: [main] + workflow_run: + workflows: [Pull request events] + types: [completed] +permissions: {} +jobs: + select: + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + pull-requests: read + outputs: + events: ${{ steps.resolve.outputs.events-json }} + steps: + - id: resolve + uses: loft-sh/github-actions/.github/actions/resolve-pr-event@resolve-pr-event/v1 + with: + source-workflow: .github/workflows/pull-request-events.yaml + backport: + needs: select + if: needs.select.outputs.events != '[]' && needs.select.outputs.events != '' + strategy: + matrix: + event: ${{ fromJSON(needs.select.outputs.events) }} + permissions: + actions: read + contents: read + pull-requests: read + uses: loft-sh/github-actions/.github/workflows/backport.yaml@backport/v2 + with: + pr-event: ${{ toJSON(matrix.event) }} + source-workflow: .github/workflows/pull-request-events.yaml + secrets: + gh-access-token: ${{ secrets.GH_ACCESS_TOKEN }} +``` + +The example uses a published action and does not check out PR code. Coordinate publication of the action and reusable workflow tags with caller rollout. ## Inputs -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|-----------------|--------|----------|-------------------------|-------------------------------------------------------------------| -| event-json | string | false | | Trusted payload passed between jobs through
workflow_call. | -| github-token | string | false | `"${{ github.token }}"` | Token with actions, contents, and pull
requests read access. | -| source-workflow | string | false | | Expected relay path, .github/workflows/pull-request-events.yaml. | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|-----------------|--------|----------|------------------------------------------------|----------------------------------------------------------------------------------------| +| event-json | string | false | | Trusted payload passed between jobs through
workflow_call. | +| github-token | string | false | `"${{ github.token }}"` | Token with contents and pull requests
read access; actions read for workflow_run. | +| source-workflow | string | false | `".github/workflows/pull-request-events.yaml"` | Expected read-only pull request signal workflow
path. | @@ -55,15 +85,14 @@ Local tests and lint do not prove these live events. -| OUTPUT | TYPE | DESCRIPTION | -|-----------------------|--------|------------------------------------------------------------------------------| -| action | string | Pull request action. | -| body-without-comments | string | PR body with HTML comments removed,
for follow-up descriptions. | -| eligible | string | Whether a validated event is available. | -| event-json | string | Synthetic event JSON for passing to
another job. | -| event-path | string | Temporary synthetic event file for RESOLVED_PR_EVENT_PATH. | -| label | string | Current label name for labeled events. | -| node-options | string | NODE_OPTIONS preload for Node actions that
consume the synthetic event. | -| pr-number | string | Pull request number. | +| OUTPUT | TYPE | DESCRIPTION | +|--------------|--------|-------------------------------------------------------------------------------| +| action | string | Pull request action. | +| eligible | string | Whether a validated event is available. | +| event-json | string | Compact PR identity and backport labels
when exactly one PR is eligible. | +| event-path | string | Temporary synthetic event file for RESOLVED_PR_EVENT_PATH. | +| events-json | string | Array of compact PR identity envelopes
for a job matrix. | +| node-options | string | NODE_OPTIONS preload for Node actions that
consume the synthetic event. | +| pr-number | string | Pull request number. | diff --git a/.github/actions/resolve-pr-event/action.yml b/.github/actions/resolve-pr-event/action.yml index 791d32bc..8b7076bb 100644 --- a/.github/actions/resolve-pr-event/action.yml +++ b/.github/actions/resolve-pr-event/action.yml @@ -1,12 +1,14 @@ name: Resolve pull request event -description: Validate a completed pull request relay or restore a trusted event payload. +description: Resolve merged pull requests from trusted default branch events. inputs: github-token: - description: Token with actions, contents, and pull requests read access. + required: false + description: Token with contents and pull requests read access; actions read for workflow_run. default: ${{ github.token }} source-workflow: - description: Expected relay path, .github/workflows/pull-request-events.yaml. required: false + description: Expected read-only pull request signal workflow path. + default: .github/workflows/pull-request-events.yaml event-json: description: Trusted payload passed between jobs through workflow_call. required: false @@ -14,8 +16,11 @@ outputs: eligible: description: Whether a validated event is available. value: ${{ steps.resolve.outputs.eligible }} + events-json: + description: Array of compact PR identity envelopes for a job matrix. + value: ${{ steps.resolve.outputs.events-json }} event-json: - description: Synthetic event JSON for passing to another job. + description: Compact PR identity and backport labels when exactly one PR is eligible. value: ${{ steps.resolve.outputs.event-json }} event-path: description: Temporary synthetic event file for RESOLVED_PR_EVENT_PATH. @@ -29,12 +34,6 @@ outputs: action: description: Pull request action. value: ${{ steps.resolve.outputs.action }} - label: - description: Current label name for labeled events. - value: ${{ steps.resolve.outputs.label }} - body-without-comments: - description: PR body with HTML comments removed, for follow-up descriptions. - value: ${{ steps.resolve.outputs.body-without-comments }} runs: using: composite steps: diff --git a/.github/actions/resolve-pr-event/src/event-bootstrap.cjs b/.github/actions/resolve-pr-event/src/event-bootstrap.cjs index d034d659..fcb3c9f7 100644 --- a/.github/actions/resolve-pr-event/src/event-bootstrap.cjs +++ b/.github/actions/resolve-pr-event/src/event-bootstrap.cjs @@ -14,7 +14,7 @@ try { typeof eventRepository !== 'string' || eventRepository.toLowerCase() !== repository.toLowerCase()) throw new Error(); process.env.GITHUB_EVENT_PATH = path; - process.env.GITHUB_EVENT_NAME = 'pull_request_target'; + process.env.GITHUB_EVENT_NAME = 'pull_request'; } catch (_) { throw new Error('Unable to restore validated pull request event'); } diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index 389c47ba..2235f097 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -1,6 +1,5 @@ #!/usr/bin/env python3 -"""Resolve an untrusted workflow_run artifact into an authoritative PR event.""" -import io +"""Resolve merged pull requests using trusted GitHub events and API data.""" import json import os import re @@ -10,12 +9,9 @@ import urllib.parse import urllib.request import uuid -import zipfile -MAX_ARCHIVE = 128 * 1024 -MAX_JSON = 32 * 1024 -RELAY = '.github/workflows/pull-request-events.yaml' -ACTIONS = {'opened', 'edited', 'labeled', 'closed'} +MAX_BYTES = 4 * 1024 * 1024 +SHA = re.compile(r'^[0-9a-f]{40}$') class Reject(Exception): pass @@ -37,34 +33,11 @@ def parse_json(data): def positive(value): return type(value) is int and 0 < value <= 2**53 - 1 -def decode_artifact(raw): - if len(raw) > MAX_ARCHIVE: - raise Reject('artifact archive too large') - try: - with zipfile.ZipFile(io.BytesIO(raw)) as archive: - entries = archive.infolist() - if len(entries) != 1 or entries[0].filename != 'event.json': - raise Reject('artifact must contain only event.json') - entry = entries[0] - if entry.is_dir() or entry.file_size > MAX_JSON or entry.flag_bits & 1: - raise Reject('invalid artifact member') - with archive.open(entry) as member: - data = member.read(MAX_JSON + 1) - if len(data) > MAX_JSON: - raise Reject('artifact member too large') - except (zipfile.BadZipFile, RuntimeError, OSError, EOFError) as exc: - raise Reject('invalid artifact archive') from exc - obj = parse_json(data) - if not isinstance(obj, dict) or set(obj) != {'number', 'action', 'label'}: - raise Reject('invalid artifact fields') - if not positive(obj['number']) or obj['action'] not in ACTIONS: - raise Reject('invalid artifact event') - label = obj['label'] - if not isinstance(label, str) or len(label) > 255 or any(ord(c) < 32 for c in label): - raise Reject('invalid artifact label') - if (obj['action'] == 'labeled') != bool(label): - raise Reject('invalid event label') - return obj +def sha(value): + return isinstance(value, str) and bool(SHA.fullmatch(value)) and value != '0' * 40 + +def same(left, right): + return isinstance(left, str) and isinstance(right, str) and left.lower() == right.lower() class SafeRedirect(urllib.request.HTTPRedirectHandler): def redirect_request(self, req, fp, code, msg, headers, newurl): @@ -79,106 +52,132 @@ def __init__(self, token, repository): self.repo = repository self.token = token - def get(self, path, binary=False, allow_missing=False): + def get(self, path): request = urllib.request.Request(self.root + '/repos/' + self.repo + path, headers={'Authorization': 'Bearer ' + self.token, 'Accept': 'application/vnd.github+json', 'X-GitHub-Api-Version': '2022-11-28'}) try: with urllib.request.build_opener(SafeRedirect()).open(request, timeout=30) as response: - raw = response.read((MAX_ARCHIVE if binary else 4 * 1024 * 1024) + 1) + raw = response.read(MAX_BYTES + 1) except urllib.error.HTTPError as exc: - if allow_missing and exc.code == 404: - return None raise RuntimeError('GitHub API request failed (HTTP %d)' % exc.code) from exc except urllib.error.URLError as exc: raise RuntimeError('GitHub API request failed') from exc - if len(raw) > (MAX_ARCHIVE if binary else 4 * 1024 * 1024): + if len(raw) > MAX_BYTES: raise Reject('API response too large') - return raw if binary else parse_json(raw) - -def resolve(api, event, source): - if source != RELAY: - raise Reject('source-workflow must name the pull request relay') - if not isinstance(event, dict) or event.get('action') != 'completed': - raise Reject('not a completed workflow run') - payload_run = event.get('workflow_run') or {} - run_id, attempt = payload_run.get('id'), payload_run.get('run_attempt') - if not positive(run_id) or not positive(attempt): - raise Reject('invalid run identity') - run = api.get('/actions/runs/%d/attempts/%d' % (run_id, attempt)) - if (run.get('id') != run_id or run.get('run_attempt') != attempt or - run.get('event') != 'pull_request_target' or run.get('status') != 'completed' or - run.get('conclusion') != 'success' or run.get('path') != source): - raise Reject('source run is not an eligible relay') - workflow = api.get('/actions/workflows/' + urllib.parse.quote(source.rsplit('/', 1)[-1], safe='')) - run_repo = run.get('repository') or {} - if (not positive(run.get('workflow_id')) or workflow.get('id') != run.get('workflow_id') or - workflow.get('path') != source or not isinstance(run_repo.get('full_name'), str) or - run_repo['full_name'].lower() != api.repo.lower()): - raise Reject('source workflow or repository mismatch') - # pull_request_target executes the workflow from the base repository's - # default branch. The run head_sha is the PR head, not the workflow SHA. - repo = api.get('') - if repo.get('full_name', '').lower() != api.repo.lower(): - raise Reject('repository identity mismatch') - artifacts = [] + return parse_json(raw) + +def paged(api, path, field=None): + items = [] for page in range(1, 11): - result = api.get('/actions/runs/%d/artifacts?per_page=100&page=%d' % (run_id, page)) - batch = result.get('artifacts', []) - artifacts.extend(a for a in batch if a.get('name') == 'pull-request-event-%d' % attempt and not a.get('expired')) + obj = api.get(path + ('&' if '?' in path else '?') + 'per_page=100&page=%d' % page) + batch = obj.get(field) if field and isinstance(obj, dict) else obj + if not isinstance(batch, list) or len(batch) > 100: + raise Reject('invalid paginated API response') + items.extend(batch) if len(batch) < 100: - break + return items + raise Reject('pagination limit exceeded') + +def trusted_context(api, payload, event_name): + repo = api.get('') + default = repo.get('default_branch') + if not same(repo.get('full_name'), api.repo) or not isinstance(default, str) or not default: + raise Reject('repository identity mismatch') + if not same((payload.get('repository') or {}).get('full_name'), api.repo): + raise Reject('event repository mismatch') + ref = 'refs/heads/' + default + if os.environ.get('GITHUB_REF') != ref: + raise Reject('workflow is not running on the default branch') + workflow_ref = os.environ.get('GITHUB_WORKFLOW_REF', '') + if not workflow_ref.startswith(api.repo + '/.github/workflows/') or not workflow_ref.endswith('@' + ref): + raise Reject('workflow revision is not trusted') + return repo, default, ref + +def merged_pr(pr, number, repo, default): + base = pr.get('base') or {} + return (positive(number) and pr.get('number') == number and pr.get('merged') is True and + pr.get('state') == 'closed' and sha(pr.get('merge_commit_sha')) and + same((base.get('repo') or {}).get('full_name'), repo) and base.get('ref') == default) + +def resolve(api, payload, event_name, source_workflow=''): + if not isinstance(payload, dict): + raise Reject('invalid event') + repo, default, ref = trusted_context(api, payload, event_name) + commits = [] + if event_name == 'push': + before, head = payload.get('before'), payload.get('after') + if payload.get('ref') != ref or payload.get('deleted') is not False or payload.get('forced') is not False or not sha(before) or not sha(head): + raise Reject('unsupported push ref, range, deletion, or force push') + commits = [item.get('sha') for item in paged(api, '/compare/%s...%s' % (before, head), 'commits') if isinstance(item, dict)] + if not commits or commits[-1] != head or any(not sha(item) for item in commits): + raise Reject('invalid or incomplete push range') + elif event_name == 'workflow_run': + signal = payload.get('workflow_run') or {} + run_id, attempt = signal.get('id'), signal.get('run_attempt') + if (payload.get('action') != 'completed' or not positive(run_id) or not positive(attempt) or + not re.fullmatch(r'\.github/workflows/[A-Za-z0-9_.-]+\.ya?ml', source_workflow)): + raise Reject('invalid signal identity') + run = api.get('/actions/runs/%d/attempts/%d' % (run_id, attempt)) + workflow = api.get('/actions/workflows/' + urllib.parse.quote(source_workflow.rsplit('/', 1)[-1], safe='')) + if (run.get('id') != run_id or run.get('run_attempt') != attempt or + run.get('event') != 'pull_request' or run.get('status') != 'completed' or run.get('conclusion') != 'success' or + run.get('path') != source_workflow or not positive(run.get('workflow_id')) or + workflow.get('id') != run.get('workflow_id') or workflow.get('path') != source_workflow or + not same((run.get('repository') or {}).get('full_name'), api.repo) or + not isinstance(run.get('head_branch'), str) or not run.get('head_branch') or + not sha(run.get('head_sha')) or signal.get('head_sha') != run.get('head_sha')): + raise Reject('signal run is not eligible') + candidates = run.get('pull_requests') + if not isinstance(candidates, list): + raise Reject('invalid signal pull request associations') + if not candidates: + candidates = paged(api, '/commits/%s/pulls' % run['head_sha']) + signal_sha = run['head_sha'] else: - raise Reject('too many artifacts') - if len(artifacts) != 1: - raise Reject('missing or ambiguous relay artifact; rerun the metadata relay') - artifact = artifacts[0] - if not positive(artifact.get('id')) or artifact.get('workflow_run', {}).get('id') != run_id: - raise Reject('artifact run mismatch') - artifact_repo_id = artifact.get('workflow_run', {}).get('repository_id') - if artifact_repo_id is not None and artifact_repo_id != repo.get('id'): - raise Reject('artifact repository mismatch') - seed = decode_artifact(api.get('/actions/artifacts/%d/zip' % artifact['id'], binary=True)) - pr = api.get('/pulls/%d' % seed['number'], allow_missing=True) - if not isinstance(pr, dict) or pr.get('number') != seed['number']: - raise Reject('pull request not found') - base_repo = ((pr.get('base') or {}).get('repo') or {}).get('full_name') - if not isinstance(base_repo, str) or base_repo.lower() != api.repo.lower(): - raise Reject('pull request repository mismatch') - label = None - if seed['action'] == 'labeled': - label = next((item for item in pr.get('labels', []) if isinstance(item, dict) and item.get('name') == seed['label']), None) - if label is None: - raise Reject('label no longer exists on pull request; restore the label and rerun the metadata relay') - result = {'action': seed['action'], 'number': seed['number'], 'pull_request': pr, 'repository': repo} - if label is not None: - result['label'] = label - return result + raise Reject('unsupported event') + numbers = set() + for commit in commits: + for candidate in paged(api, '/commits/%s/pulls' % commit): + number = candidate.get('number') if isinstance(candidate, dict) else None + if positive(number) and candidate.get('merge_commit_sha') == commit: + numbers.add(number) + if event_name == 'workflow_run': + for candidate in candidates: + number = candidate.get('number') if isinstance(candidate, dict) else None + if positive(number): + numbers.add(number) + events = [] + for number in sorted(numbers): + pr = api.get('/pulls/%d' % number) + if not isinstance(pr, dict) or not merged_pr(pr, number, api.repo, default): + continue + if commits and pr['merge_commit_sha'] not in commits: + continue + if event_name == 'workflow_run' and (signal_sha not in (pr['merge_commit_sha'], (pr.get('head') or {}).get('sha')) or + (pr.get('head') or {}).get('ref') != run['head_branch']): + continue + if not commits: + comparison = api.get('/compare/%s...%s' % (pr['merge_commit_sha'], urllib.parse.quote(default, safe=''))) + if comparison.get('status') not in ('ahead', 'identical'): + raise Reject('merged commit is not on the default branch') + events.append({'action': 'closed', 'number': number, 'pull_request': pr, 'repository': repo}) + if len(events) > 256: + raise RuntimeError('too many matched pull requests for a job matrix') + return events -def validate_restore(event, repository): - if not isinstance(event, dict) or not isinstance(event.get('pull_request'), dict): - raise Reject('invalid restored event') - number = event['pull_request'].get('number') - repo = event.get('repository') - if not positive(number) or not isinstance(repo, dict) or not isinstance(repo.get('full_name'), str) or repo['full_name'].lower() != repository.lower(): - raise Reject('restored event identity mismatch') - if event.get('action') not in ACTIONS or (event.get('number', number) != number): - raise Reject('invalid restored event action') - return event - -def write_outputs(event): - encoded = json.dumps(event, ensure_ascii=True, separators=(',', ':')) - with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', dir=os.environ['RUNNER_TEMP'], prefix='pr-event-', suffix='.json', delete=False) as f: - f.write(encoded) - path = f.name - label = event.get('label') or {} - bootstrap = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'event-bootstrap.cjs') - outputs = {'eligible': 'true', 'event-json': encoded, 'event-path': path, - 'node-options': '--require=' + json.dumps(bootstrap, ensure_ascii=False), - 'pr-number': str(event['pull_request']['number']), 'action': event['action'], - 'label': label.get('name', '') if isinstance(label, dict) else '', - 'body-without-comments': re.sub(r'', '', event['pull_request'].get('body') or '', flags=re.S)} - write_output_values(outputs) +def compact(event): + pr = event['pull_request'] + repo = event['repository'] + labels = [label for label in pr.get('labels', []) if isinstance(label, dict) and + isinstance(label.get('name'), str) and label['name'].startswith('backport-to-')] + return {'action': 'closed', 'number': event['number'], + 'repository': {'full_name': repo['full_name'], 'name': repo.get('name', repo['full_name'].split('/')[-1]), + 'default_branch': repo['default_branch']}, + 'pull_request': {'number': pr['number'], 'state': pr['state'], 'merged': pr['merged'], + 'merge_commit_sha': pr['merge_commit_sha'], 'base': {'ref': pr['base']['ref'], + 'repo': {'full_name': pr['base']['repo']['full_name']}}, + 'labels': [{'name': item['name']} for item in labels]}} def write_output_values(outputs): with open(os.environ['GITHUB_OUTPUT'], 'a', encoding='utf-8') as f: @@ -188,26 +187,61 @@ def write_output_values(outputs): delimiter = 'ghadelimiter_' + uuid.uuid4().hex f.write('%s<<%s\n%s\n%s\n' % (key, delimiter, value, delimiter)) +def write_outputs(events): + encoded_events = json.dumps([compact(event) for event in events], ensure_ascii=True, separators=(',', ':')) + outputs = {'eligible': 'true' if events else 'false', 'events-json': encoded_events} + if len(events) == 1: + event = events[0] + encoded = json.dumps(event, ensure_ascii=True, separators=(',', ':')) + compact_encoded = json.dumps(compact(event), ensure_ascii=True, separators=(',', ':')) + with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', dir=os.environ['RUNNER_TEMP'], prefix='pr-event-', suffix='.json', delete=False) as f: + f.write(encoded) + path = f.name + bootstrap = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'event-bootstrap.cjs') + outputs.update({'event-json': compact_encoded, 'event-path': path, + 'node-options': '--require=' + json.dumps(bootstrap, ensure_ascii=False), + 'pr-number': str(event['number']), 'action': event['action']}) + if sum(len(value.encode('utf-16-le')) for value in outputs.values()) > 700 * 1024: + raise RuntimeError('resolved outputs exceed size limit') + write_output_values(outputs) + def main(): try: repository = os.environ['GITHUB_REPOSITORY'] + with open(os.environ['GITHUB_EVENT_PATH'], 'rb') as f: + raw = f.read(MAX_BYTES + 1) + if len(raw) > MAX_BYTES: + raise Reject('event too large') + payload = parse_json(raw) restored = os.environ.get('INPUT_EVENT_JSON', '') + requested = None if restored: - if len(restored.encode('utf-8')) > 4 * 1024 * 1024: + if len(restored.encode('utf-8')) > MAX_BYTES: raise Reject('restored event too large') - event = validate_restore(parse_json(restored), repository) - else: - if os.environ.get('GITHUB_EVENT_NAME') != 'workflow_run': - raise Reject('unsupported event') - with open(os.environ['GITHUB_EVENT_PATH'], 'rb') as f: - payload = parse_json(f.read(4 * 1024 * 1024 + 1)) - event = resolve(API(os.environ['INPUT_GITHUB_TOKEN'], repository), payload, - os.environ.get('INPUT_SOURCE_WORKFLOW', '')) - write_outputs(event) + requested = parse_json(restored) + events = resolve(API(os.environ['INPUT_GITHUB_TOKEN'], repository), payload, + os.environ.get('GITHUB_EVENT_NAME', ''), os.environ.get('INPUT_SOURCE_WORKFLOW', '')) + if restored: + number = requested.get('number') if isinstance(requested, dict) else None + matches = [event for event in events if event['number'] == number] + supplied_pr = requested.get('pull_request') if isinstance(requested, dict) else None + if (len(matches) != 1 or not isinstance(supplied_pr, dict) or + requested.get('action') != 'closed' or supplied_pr.get('number') != number or + supplied_pr.get('merged') is not True or supplied_pr.get('state') != 'closed' or + supplied_pr.get('merge_commit_sha') != matches[0]['pull_request']['merge_commit_sha'] or + not same((requested.get('repository') or {}).get('full_name'), repository) or + not same(((supplied_pr.get('base') or {}).get('repo') or {}).get('full_name'), repository) or + (supplied_pr.get('base') or {}).get('ref') != matches[0]['repository']['default_branch']): + raise Reject('restored event does not match authoritative event') + events = matches + write_outputs(events) except Reject as exc: print('::warning::' + str(exc)) - write_output_values({'eligible': 'false'}) - except (RuntimeError, OSError, KeyError, TypeError) as exc: + write_output_values({'eligible': 'false', 'events-json': '[]'}) + except RuntimeError as exc: + print('::error::Unable to resolve pull request event: ' + str(exc), file=sys.stderr) + return 1 + except (OSError, KeyError, TypeError) as exc: print('::error::Unable to resolve pull request event: ' + type(exc).__name__, file=sys.stderr) return 1 return 0 diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index a9dc882f..6ab70470 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -1,24 +1,121 @@ -"""Guard compatibility between direct PR callers and workflow_run handoffs.""" +"""Enforce the trust boundary shared by callers, resolver and mutation jobs.""" from pathlib import Path +import os import re +import subprocess +import textwrap -WORKFLOW = Path(__file__).resolve().parents[3] / 'workflows' / 'backport.yaml' +import pytest +GITHUB = Path(__file__).resolve().parents[3] +WORKFLOWS = GITHUB / 'workflows' -def test_existing_callers_keep_the_original_event_as_fallback(): - text = WORKFLOW.read_text() - assert "default: ''" in text.split(' pr-event:', 1)[1].split(' legacy-split:', 1)[0] - assert 'fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true' in text - assert not re.search(r'github\.event\.(pull_request|action|label)', text) +def jobs(name): + text = (WORKFLOWS / name).read_text().split('\njobs:\n', 1)[1] + return dict(re.findall(r'^ ([\w-]+):\n(.*?)(?=^ [\w-]+:\n|\Z)', text, re.M | re.S)) -def test_backport_action_receives_the_restored_payload_without_changing_other_steps(): - text = WORKFLOW.read_text() - assert "if: inputs.pr-event != ''" in text - assert 'event-json: ${{ inputs.pr-event }}' in text + +def test_no_privileged_pr_trigger_remains_anywhere_in_github(): + forbidden = b'pull_request' + b'_target' + matches = [str(path.relative_to(GITHUB)) for path in GITHUB.rglob('*') + if path.is_file() and '__pycache__' not in path.parts + and forbidden in path.read_bytes()] + assert matches == [] + + +def test_backport_requires_trusted_default_branch_and_api_resolution(): + resolve = jobs('backport.yaml')['resolve-event'] + assert "inputs.pr-event != ''" in resolve + assert '"push","workflow_run"' in resolve + assert "github.ref == format('refs/heads/{0}', github.event.repository.default_branch)" in resolve + assert "endsWith(github.workflow_ref, format('@refs/heads/{0}', github.event.repository.default_branch))" in resolve + assert 'actions: read' in resolve + assert 'pull-requests: read' in resolve + assert 'contents: read' in resolve + assert 'event-json: ${{ inputs.pr-event }}' in resolve + + +def test_every_backport_job_depends_on_authoritative_event(): + entries = jobs('backport.yaml') + assert set(entries) == {'resolve-event', 'backport', 'fix-imports', + 'legacy-classify', 'legacy-backport', 'link-backports'} + for name, body in entries.items(): + assert 'permissions:\n contents: read' in body, name + assert 'contents: write' not in body, name + if name != 'resolve-event': + assert re.search(r'^ needs:.*resolve-event', body, re.M), name + assert 'github.event.pull_request' not in body, name + for name in ('backport', 'legacy-classify', 'link-backports'): + assert "needs.resolve-event.outputs.eligible == 'true'" in entries[name] + assert 'needs: [resolve-event, backport, legacy-backport]' in entries['link-backports'] + assert 'continue-on-error: true' in entries['link-backports'] + + +def test_node_adapter_is_step_scoped_and_checkouts_never_persist_tokens(): + text = (WORKFLOWS / 'backport.yaml').read_text() assert 'NODE_OPTIONS: ${{ steps.pr-event.outputs.node-options }}' in text assert 'RESOLVED_PR_EVENT_PATH: ${{ steps.pr-event.outputs.event-path }}' in text assert 'GITHUB_EVENT_PATH:' not in text + assert text.count('uses: actions/checkout@') == text.count('persist-credentials: false') assert 'cancel-in-progress: false' in text - assert 'needs: [backport, legacy-backport]' in text - assert 'continue-on-error: true' in text + body = jobs('backport.yaml')['backport'] + assert body.count("if: steps.pr-event.outputs.eligible == 'true'") == 3 + + +def test_review_rejects_forks_in_caller_and_reusable_before_secrets(): + for filename, job in [('claude-code-review.yaml', 'claude-review'), + ('test-claude-code-review.yaml', 'claude-review')]: + body = jobs(filename)[job] + assert 'github.event.pull_request.head.repo.full_name == github.repository' in body + assert body.index(' if:') < body.index(' permissions:') + assert 'contents: read' in body and 'pull-requests: write' in body + assert 'allow-unsafe-pr-checkout' not in body + assert 'Setup fork as origin' not in body + review = jobs('claude-code-review.yaml')['claude-review'] + assert "github.event_name == 'pull_request'" in review + assert 'persist-credentials: false' in review + + +def test_smoke_verifiers_need_no_token_permissions(): + for filename in ('test-backport.yaml', 'test-claude-code-review.yaml'): + assert 'permissions: {}' in jobs(filename)['verify'] + text = (WORKFLOWS / filename).read_text() + assert filename in text.split('permissions:', 1)[0] + + +@pytest.mark.parametrize('filename', ['claude.yaml', 'claude.yml']) +@pytest.mark.parametrize('association,is_pr,number,answer,exit_code,allowed,queried', [ + ('MEMBER', 'true', '17', 'true', 0, True, True), + ('OWNER', 'true', '17', 'false', 0, False, True), + ('COLLABORATOR', 'true', '17', 'true', 0, True, True), + ('CONTRIBUTOR', 'true', '17', 'true', 0, False, False), + ('MEMBER', 'false', '', 'true', 0, True, False), + ('NONE', 'false', '', 'true', 0, False, False), + ('MEMBER', 'true', '', 'true', 1, False, False), + ('MEMBER', 'true', '$(touch injected)', 'true', 1, False, False), + ('MEMBER', 'true', '17', '', 0, False, True), +]) +def test_comment_authorization_executes_before_any_secret_or_checkout( + tmp_path, filename, association, is_pr, number, answer, exit_code, allowed, queried): + entries = jobs(filename) + authorize, privileged = entries['authorize'], entries['claude'] + assert 'permissions:\n pull-requests: read' in authorize + assert 'secrets.' not in authorize and 'checkout@' not in authorize + assert 'needs: authorize' in privileged + assert "if: needs.authorize.outputs.allowed == 'true'" in privileged + script = textwrap.dedent(authorize.split(' run: |\n', 1)[1]) + mock = tmp_path / 'gh' + mock.write_text('#!/bin/sh\nprintf called > "$CALL_FILE"\nprintf "%s\\n" "$API_ANSWER"\n') + mock.chmod(0o755) + output, calls = tmp_path / 'out', tmp_path / 'calls' + env = {**os.environ, 'PATH': str(tmp_path) + os.pathsep + os.environ['PATH'], + 'ASSOCIATION': association, 'IS_PR': is_pr, 'PR_NUMBER': number, + 'REPOSITORY': 'org/repo', 'API_ANSWER': answer, + 'GITHUB_OUTPUT': str(output), 'CALL_FILE': str(calls)} + result = subprocess.run(['bash', '-euo', 'pipefail', '-c', script], env=env, + cwd=tmp_path, text=True, capture_output=True) + assert result.returncode == exit_code, result.stderr + assert (output.exists() and 'allowed=true' in output.read_text()) == allowed + assert calls.exists() == queried + assert not (tmp_path / 'injected').exists() diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index 90455e50..276fc095 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -1,208 +1,203 @@ import importlib.util -import io import json import os -from pathlib import Path -import zipfile +import pathlib +import subprocess import pytest -spec = importlib.util.spec_from_file_location('resolve', Path(__file__).parents[1] / 'src/resolve.py') +SRC = pathlib.Path(__file__).resolve().parents[1] / 'src' +spec = importlib.util.spec_from_file_location('resolver', SRC / 'resolve.py') r = importlib.util.module_from_spec(spec) spec.loader.exec_module(r) -SHA = 'a' * 40 -DEFAULT = 'b' * 40 +A, B, C = ('a' * 40, 'b' * 40, 'c' * 40) +REPO = {'full_name': 'org/repo', 'default_branch': 'main'} -def archive(data=None, name='event.json', extra=None): - buf = io.BytesIO() - with zipfile.ZipFile(buf, 'w') as z: - z.writestr(name, data if data is not None else json.dumps({'number': 7, 'action': 'labeled', 'label': 'backport'})) - if extra: - z.writestr(extra, '{}') - return buf.getvalue() +def pr(number, commit, fork=False): + return {'number': number, 'state': 'closed', 'merged': True, 'merge_commit_sha': commit, + 'base': {'ref': 'main', 'repo': {'full_name': 'org/repo'}}, + 'head': {'repo': {'full_name': 'fork/repo' if fork else 'org/repo'}, 'sha': B, 'ref': 'feature'}, + 'labels': [{'name': 'backport-to-v1'}], 'body': 'hello'} class FakeAPI: repo = 'org/repo' def __init__(self): - self.run = {'id': 123, 'run_attempt': 1, 'event': 'pull_request_target', 'workflow_id': 42, 'repository': {'full_name': 'org/repo'}, 'status': 'completed', 'conclusion': 'success', 'path': r.RELAY, 'head_sha': SHA, 'head_branch': 'feature', 'head_repository': {'full_name': 'fork/repo'}} - self.pr = {'number': 7, 'head': {'sha': SHA, 'ref': 'feature', 'repo': {'full_name': 'fork/repo'}}, 'base': {'repo': {'full_name': 'org/repo'}}, 'labels': [{'name': 'backport'}], 'merged': True, 'body': 'authoritative'} - self.data = archive() - self.items = [{'id': 9, 'name': 'pull-request-event-1', 'expired': False, 'workflow_run': {'id': 123}}] - def get(self, path, binary=False, allow_missing=False): - if path.startswith('/actions/runs/123/attempts/'): - return self.run - if path.startswith('/actions/workflows/'): - return {'id': 42, 'path': r.RELAY} - if path == '': - return {'full_name': self.repo, 'default_branch': 'main', 'id': 12} - if '/artifacts?' in path: - return {'artifacts': self.items} - if path.startswith('/actions/artifacts/'): - return self.data - if path.startswith('/pulls/'): - return self.pr + self.pull = {1: pr(1, B, True), 2: pr(2, C)} + self.calls = [] + def get(self, path): + self.calls.append(path) + if path == '': return REPO + if path.startswith('/compare/'): + return {'commits': [{'sha': B}, {'sha': C}], 'status': 'ahead'} + if path.startswith('/commits/'): + commit = path.split('/')[2] + return [self.pull[1]] if commit == B else [self.pull[2], self.pull[1]] + if path.startswith('/pulls/'): return self.pull[int(path.split('/')[-1])] raise AssertionError(path) -def event(attempt=1): - return {'action': 'completed', 'workflow_run': {'id': 123, 'run_attempt': attempt}} +@pytest.fixture(autouse=True) +def context(monkeypatch): + monkeypatch.setenv('GITHUB_REF', 'refs/heads/main') + monkeypatch.setenv('GITHUB_WORKFLOW_REF', 'org/repo/.github/workflows/backport.yml@refs/heads/main') -def test_fork_event_uses_authoritative_pr(): - api = FakeAPI() - result = r.resolve(api, event(), r.RELAY) - assert result['pull_request']['body'] == 'authoritative' - assert result['label']['name'] == 'backport' - assert result['pull_request']['merged'] is True +def push(): + return {'repository': REPO, 'ref': 'refs/heads/main', 'before': A, 'after': C, 'deleted': False, 'forced': False} -def test_same_repo_branch(): - api = FakeAPI() - api.run['head_repository']['full_name'] = api.pr['head']['repo']['full_name'] = 'org/repo' - assert r.resolve(api, event(), r.RELAY)['number'] == 7 - -@pytest.mark.parametrize('field,value', [('event', 'pull_request'), ('event', 'push'), ('status', 'in_progress'), ('conclusion', 'failure'), ('path', 'wrong'), ('run_attempt', 2)]) -def test_invalid_run(field, value): - api = FakeAPI() - api.run[field] = value - with pytest.raises(r.Reject): - r.resolve(api, event(), r.RELAY) - -@pytest.mark.parametrize('change', ['number', 'base_repo', 'run_repo', 'workflow_id']) -def test_forged_pr_provenance(change): - api = FakeAPI() - if change == 'number': api.pr['number'] = 8 - if change == 'base_repo': api.pr['base']['repo']['full_name'] = 'other/repo' - if change == 'run_repo': api.run['repository']['full_name'] = 'other/repo' - if change == 'workflow_id': api.run['workflow_id'] = 99 - with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) - -def test_target_event_base_sha_and_deleted_fork(): - api = FakeAPI() - api.pr['head']['sha'] = DEFAULT - api.pr['head']['repo'] = None - api.pr['merged'] = True - assert r.resolve(api, event(), r.RELAY)['pull_request']['merged'] is True - -def test_fork_head_cannot_change_relay(): - # Fake API rejects any contents lookup: the fork head can omit or modify - # the relay, while pull_request_target executes the default branch workflow. +def test_multi_commit_and_fork_merged_deduplicated(): api = FakeAPI() - api.run['head_sha'] = 'c' * 40 - assert r.resolve(api, event(), r.RELAY)['number'] == 7 + events = r.resolve(api, push(), 'push') + assert [x['number'] for x in events] == [1, 2] + assert all(x['action'] == 'closed' for x in events) + assert '/pulls/1' in api.calls and '/pulls/2' in api.calls -def test_stale_label(): +def test_nonmatching_and_unmerged_pr_skipped(): api = FakeAPI() - api.pr['labels'] = [] - with pytest.raises(r.Reject, match='restore the label'): r.resolve(api, event(), r.RELAY) - -def test_artifact_repository_mismatch(): - api = FakeAPI() - api.items[0]['workflow_run']['repository_id'] = 999 - with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) - api.items[0]['workflow_run']['repository_id'] = 12 - assert r.resolve(api, event(), r.RELAY)['number'] == 7 - -def test_workflow_path_and_id_must_match(): - api = FakeAPI() - api.run['workflow_id'] = 77 - with pytest.raises(r.Reject): r.resolve(api, event(), r.RELAY) + api.pull[1]['merged'] = False + api.pull[2]['base']['repo']['full_name'] = 'other/repo' + assert r.resolve(api, push(), 'push') == [] + +@pytest.mark.parametrize('field,value', [('ref', 'refs/heads/other'), ('forced', True), ('deleted', True), ('before', '0'*40)]) +def test_invalid_push_does_not_query_range(field, value): + api = FakeAPI(); payload = push(); payload[field] = value + with pytest.raises(r.Reject): r.resolve(api, payload, 'push') + assert len(api.calls) == 1 + +def test_untrusted_context_rejected(monkeypatch): + api = FakeAPI(); payload = push(); payload['repository'] = {'full_name': 'other/repo'} + with pytest.raises(r.Reject): r.resolve(api, payload, 'push') + monkeypatch.setenv('GITHUB_REF', 'refs/heads/branch') + with pytest.raises(r.Reject): r.resolve(api, push(), 'push') + monkeypatch.setenv('GITHUB_REF', 'refs/heads/main') + monkeypatch.setenv('GITHUB_WORKFLOW_REF', 'org/repo/.github/workflows/test.yml@refs/heads/branch') + with pytest.raises(r.Reject): r.resolve(api, push(), 'push') + +class SignalAPI(FakeAPI): + def __init__(self): + super().__init__() + self.signal = {'id': 12, 'run_attempt': 1, 'event': 'pull_request', 'status': 'completed', + 'conclusion': 'success', 'path': '.github/workflows/pull-request-events.yaml', + 'workflow_id': 99, 'repository': REPO, 'head_branch': 'feature', 'head_sha': B, + 'pull_requests': [{'number': 1}]} + def get(self, path): + if path.startswith('/actions/runs/'): return self.signal + if path.startswith('/actions/workflows/'): + return {'id': 99, 'path': '.github/workflows/pull-request-events.yaml'} + return super().get(path) + +def signal_payload(): + return {'repository': REPO, 'action': 'completed', 'workflow_run': {'id': 12, 'run_attempt': 1, 'head_sha': B}} + +SOURCE = '.github/workflows/pull-request-events.yaml' + +def test_signal_fork_pr_and_fallback(): + api = SignalAPI() + assert [e['number'] for e in r.resolve(api, signal_payload(), 'workflow_run', SOURCE)] == [1] + api.signal['pull_requests'] = [] + assert [e['number'] for e in r.resolve(api, signal_payload(), 'workflow_run', SOURCE)] == [1] + assert not any('artifacts' in call for call in api.calls) + +@pytest.mark.parametrize('field,value', [('conclusion', 'failure'), ('path', '.github/workflows/other.yaml'), + ('head_branch', 'other'), ('head_sha', C), ('event', 'push'), + ('repository', {'full_name': 'other/repo'})]) +def test_invalid_signal(field, value): + api = SignalAPI(); api.signal[field] = value + try: + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE) == [] + except r.Reject: + pass + +def test_unmerged_and_unrelated_signal_skipped(): + api = SignalAPI(); api.pull[1]['merged'] = False + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE) == [] + api.pull[1] = pr(1, C); api.pull[1]['head']['sha'] = C + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE) == [] + +def test_other_events_rejected(): + for name in ('pull_request', 'workflow_dispatch', 'schedule'): + with pytest.raises(r.Reject): r.resolve(FakeAPI(), {'repository': REPO}, name) + +def test_incomplete_range_fails(): api = FakeAPI() - with pytest.raises(r.Reject): r.resolve(api, event(), '.github/workflows/other.yaml') + payload = push(); payload['after'] = 'd'*40 + with pytest.raises(r.Reject): r.resolve(api, payload, 'push') -def test_attempt_specific_artifact(): - api = FakeAPI() - api.run['run_attempt'] = 2 - with pytest.raises(r.Reject): r.resolve(api, event(2), r.RELAY) - api.items[0]['name'] = 'pull-request-event-2' - assert r.resolve(api, event(2), r.RELAY)['number'] == 7 - api.items.append(api.items[0].copy()) - with pytest.raises(r.Reject): r.resolve(api, event(2), r.RELAY) - -@pytest.mark.parametrize('raw', [archive(name='../event.json'), archive(extra='second'), archive(data='{"number":7,"number":8,"action":"opened","label":""}'), archive(data='x' * (r.MAX_JSON + 1)), b'not a zip', b'x' * (r.MAX_ARCHIVE + 1), archive(data='{"number":7,"action":"opened","label":"","pull_request":{}}')]) -def test_untrusted_archives(raw): - with pytest.raises(r.Reject): r.decode_artifact(raw) - -def test_restore_and_output_injection(tmp_path, monkeypatch): - payload = {'action': 'opened', 'pull_request': {'number': 7, 'body': 'hello\nEOF\nother=evil'}, 'repository': {'full_name': 'org/repo'}} - assert r.validate_restore(payload, 'org/repo') == payload - with pytest.raises(r.Reject): r.validate_restore(payload, 'other/repo') - output = tmp_path / 'out' - monkeypatch.setenv('GITHUB_OUTPUT', str(output)) - monkeypatch.setenv('RUNNER_TEMP', str(tmp_path)) - r.write_outputs(payload) - lines = output.read_text().splitlines() - for index, line in enumerate(lines): - if '<<' in line: - delimiter = line.split('<<', 1)[1] - assert lines.count(delimiter) == 1 - eventfile = next(tmp_path.glob('pr-event-*.json')) - assert json.loads(eventfile.read_text()) == payload - -def test_restore_rejects_bad_numbers(): - with pytest.raises(r.Reject): r.validate_restore({'action': 'opened', 'pull_request': {'number': True}, 'repository': {'full_name': 'org/repo'}}, 'org/repo') - -@pytest.mark.parametrize('body,expected', [ - ('beforeafter\nEOF\nother=evil', 'beforeafter\nEOF\nother=evil'), - (None, ''), - ('keep normal text\nand an unfinished ' + r.write_outputs([event]) + output = (tmp_path / 'out').read_text() + assert 'events-json<- Enable the pre-monorepo (<= v0.36) split/re-root backport flow. When @@ -60,48 +65,80 @@ on: description: 'Linear API token for linking backport PRs to their matching Linear sub-issue. Optional; linking is skipped when not provided.' required: false +permissions: {} + # Serialize runs for the same source PR so two rapid label events can't both race # past the open-PR check and double-create the same deterministic backport branch # (the second gh pr create would fail "already exists"). Queue, don't cancel, so # an in-flight backport always finishes. concurrency: - group: backport-${{ github.repository }}-${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} + group: backport-${{ github.repository }}-${{ fromJSON(inputs.pr-event || '{}').pull_request.number || github.run_id }} cancel-in-progress: false jobs: + resolve-event: + name: Resolve merged PR on the trusted default branch + if: >- + inputs.pr-event != '' && + contains(fromJSON('["push","workflow_run"]'), github.event_name) && + github.ref == format('refs/heads/{0}', github.event.repository.default_branch) && + endsWith(github.workflow_ref, format('@refs/heads/{0}', github.event.repository.default_branch)) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + pull-requests: read + actions: read + outputs: + eligible: ${{ steps.event.outputs.eligible }} + pr-event: ${{ steps.event.outputs.event-json }} + steps: + - name: Re-read merged pull request from GitHub + id: event + uses: loft-sh/github-actions/.github/actions/resolve-pr-event@resolve-pr-event/v1 # zizmor: ignore[unpinned-uses] -- internal versioned action + with: + event-json: ${{ inputs.pr-event }} + source-workflow: ${{ inputs.source-workflow }} + backport: name: Backport PR + needs: resolve-event if: | - fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true && + needs.resolve-event.outputs.eligible == 'true' && + fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.merged == true && ( contains( - join(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name, ','), + join(fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.labels.*.name, ','), 'backport-to-' ) ) && - (fromJSON(inputs.pr-event || toJSON(github.event)).action == 'closed' || startsWith(fromJSON(inputs.pr-event || toJSON(github.event)).label.name, 'backport-to-')) + (fromJSON(needs.resolve-event.outputs.pr-event || '{}').action == 'closed' || startsWith(fromJSON(needs.resolve-event.outputs.pr-event || '{}').label.name, 'backport-to-')) runs-on: ubuntu-latest timeout-minutes: 20 permissions: contents: read + pull-requests: read + actions: read outputs: targets: ${{ steps.targets.outputs.targets }} steps: - - name: Restore validated pull request event + - name: Restore API-resolved pull request event id: pr-event - if: inputs.pr-event != '' uses: loft-sh/github-actions/.github/actions/resolve-pr-event@resolve-pr-event/v1 # zizmor: ignore[unpinned-uses] -- internal versioned action with: - event-json: ${{ inputs.pr-event }} + event-json: ${{ needs.resolve-event.outputs.pr-event }} + source-workflow: ${{ inputs.source-workflow }} # First, so fix-imports gets its targets even when a cherry-pick fails. - name: List backport targets + if: steps.pr-event.outputs.eligible == 'true' id: targets env: - LABELS: ${{ toJSON(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name) }} + LABELS: ${{ toJSON(fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.labels.*.name) }} run: echo "targets=$(jq -c '[.[] | select(startswith("backport-to-")) | ltrimstr("backport-to-")]' <<< "$LABELS")" >> "$GITHUB_OUTPUT" - name: Checkout repository + if: steps.pr-event.outputs.eligible == 'true' uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -113,6 +150,7 @@ jobs: # or a job container would need it installed explicitly. - name: Backport Action + if: steps.pr-event.outputs.eligible == 'true' env: NODE_OPTIONS: ${{ steps.pr-event.outputs.node-options }} RESOLVED_PR_EVENT_PATH: ${{ steps.pr-event.outputs.event-path }} @@ -128,7 +166,7 @@ jobs: auto_backport_label_prefix: ${{ inputs.legacy-split == false && 'backport-to-' || '' }} - name: Info log - if: ${{ success() }} + if: ${{ success() && steps.pr-event.outputs.eligible == 'true' }} run: cat ~/.backport/backport.info.log - name: Debug log @@ -140,7 +178,7 @@ jobs: # filtering here. fix-imports: name: Fix imports for ${{ matrix.target }} - needs: backport + needs: [resolve-event, backport] if: inputs.fix-imports && !cancelled() && needs.backport.outputs.targets != '' && needs.backport.outputs.targets != '[]' runs-on: ubuntu-latest timeout-minutes: 30 @@ -157,12 +195,12 @@ jobs: fetch-depth: 0 persist-credentials: false - - name: Fix imports on backport/${{ matrix.target }}/pr-${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} + - name: Fix imports on backport/${{ matrix.target }}/pr-${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.number }} uses: loft-sh/github-actions/.github/actions/backport-fix-imports@backport-fix-imports/v1 # zizmor: ignore[unpinned-uses] -- internal loft-sh/github-actions ref; reusable workflows check out the CALLER repo, so a local ./.github/actions path is not present here. @backport-fix-imports/v1 is the floating coordination tag kept aligned with backport/v1 (see DEVOPS-923 pin-drift class). with: target-branch: ${{ matrix.target }} - backport-branch: backport/${{ matrix.target }}/pr-${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} - source-branch: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.base.ref }} + backport-branch: backport/${{ matrix.target }}/pr-${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.number }} + source-branch: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.base.ref }} github-token: ${{ secrets.gh-access-token }} # zizmor: ignore[secrets-outside-env] -- PAT passed through workflow_call; pushes the fix commit so the backport PR's CI runs on it # --- pre-monorepo (<= v0.36) split/re-root flow --------------------------- @@ -171,11 +209,13 @@ jobs: # staging/ tree; they are re-rooted and PR'd by backport-legacy-split instead. legacy-classify: name: Classify legacy targets + needs: resolve-event if: | inputs.legacy-split && - fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true && - contains(join(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name, ','), 'backport-to-') && - (fromJSON(inputs.pr-event || toJSON(github.event)).action == 'closed' || startsWith(fromJSON(inputs.pr-event || toJSON(github.event)).label.name, 'backport-to-')) + needs.resolve-event.outputs.eligible == 'true' && + fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.merged == true && + contains(join(fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.labels.*.name, ','), 'backport-to-') && + (fromJSON(needs.resolve-event.outputs.pr-event || '{}').action == 'closed' || startsWith(fromJSON(needs.resolve-event.outputs.pr-event || '{}').label.name, 'backport-to-')) runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -204,12 +244,12 @@ jobs: id: allowlist uses: loft-sh/github-actions/.github/actions/backport-legacy-allowlist@backport-legacy-allowlist/v1 # zizmor: ignore[unpinned-uses] -- internal loft-sh/github-actions ref; reusable workflows check out the CALLER repo, so a local ./.github/actions path is not present here. @backport-legacy-allowlist/v1 is the floating coordination tag kept aligned with backport/v1 (see DEVOPS-923 pin-drift class). with: - labels: ${{ toJSON(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name) }} + labels: ${{ toJSON(fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.labels.*.name) }} max-minor: '36' legacy-backport: name: Backport ${{ matrix.target }} (legacy) - needs: legacy-classify + needs: [resolve-event, legacy-classify] if: needs.legacy-classify.outputs.has-targets == 'true' runs-on: ubuntu-latest # Bound clone + apply + push so a GitHub service stall can't strand the @@ -226,9 +266,9 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: # Pin to the merge commit so it AND its parent (the diff base) are - # present locally; on a pull_request_target closed event the default - # checkout ref is the pre-merge base SHA, which would not contain it. - ref: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merge_commit_sha }} + # present locally. The resolver has verified this merge is in the + # trusted default-branch history. + ref: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.merge_commit_sha }} fetch-depth: 0 persist-credentials: false @@ -246,11 +286,11 @@ jobs: oss-repo: ${{ inputs.oss-repo }} pro-repo: ${{ inputs.pro-repo }} github-token: ${{ secrets.gh-access-token }} - commit: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merge_commit_sha }} + commit: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.merge_commit_sha }} # PR number so the action diffs the PR head (refs/pull/N/head) from its # merge base -- GitHub's "Files changed" -- capturing every commit of a # rebase-merged PR, not just the merge tip. - pr-number: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} + pr-number: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.number }} create-pr: 'true' docs-url: ${{ inputs.docs-url }} @@ -284,7 +324,7 @@ jobs: with: marker: "" body: ${{ steps.backport.outputs.comment-body }} - pr-number: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} + pr-number: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.number }} github-token: ${{ secrets.gh-access-token }} expected-author: ${{ steps.pat.outputs.login }} @@ -293,13 +333,14 @@ jobs: # successful targets link even when another target conflicts or is skipped. link-backports: name: Link backport PRs to Linear sub-issues - needs: [backport, legacy-backport] + needs: [resolve-event, backport, legacy-backport] if: | always() && !cancelled() && - fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.merged == true && - contains(join(fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.labels.*.name, ','), 'backport-to-') && - (fromJSON(inputs.pr-event || toJSON(github.event)).action == 'closed' || startsWith(fromJSON(inputs.pr-event || toJSON(github.event)).label.name, 'backport-to-')) + needs.resolve-event.outputs.eligible == 'true' && + fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.merged == true && + contains(join(fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.labels.*.name, ','), 'backport-to-') && + (fromJSON(needs.resolve-event.outputs.pr-event || '{}').action == 'closed' || startsWith(fromJSON(needs.resolve-event.outputs.pr-event || '{}').label.name, 'backport-to-')) runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -309,9 +350,9 @@ jobs: continue-on-error: true uses: loft-sh/github-actions/.github/actions/link-backport-prs@link-backport-prs/v1 # zizmor: ignore[unpinned-uses] -- internal loft-sh/github-actions ref; reusable workflows check out the CALLER repo, so a local ./.github/actions path is not present here. @link-backport-prs/v1 is the floating coordination tag kept aligned with backport/v1 (see DEVOPS-923 pin-drift class). with: - source-pr: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).pull_request.number }} + source-pr: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').pull_request.number }} repo-owner: ${{ github.repository_owner }} - repo-name: ${{ fromJSON(inputs.pr-event || toJSON(github.event)).repository.name }} + repo-name: ${{ fromJSON(needs.resolve-event.outputs.pr-event || '{}').repository.name }} additional-repos: ${{ inputs.legacy-split && format('{0},{1}', inputs.oss-repo, inputs.pro-repo) || '' }} github-token: ${{ secrets.gh-access-token }} # zizmor: ignore[secrets-outside-env] -- PAT passed through workflow_call and required for cross-repo PR edits linear-token: ${{ secrets.linear-token }} # zizmor: ignore[secrets-outside-env] -- optional token passed through workflow_call diff --git a/.github/workflows/claude-code-review.yaml b/.github/workflows/claude-code-review.yaml index 4d36d772..bee54187 100644 --- a/.github/workflows/claude-code-review.yaml +++ b/.github/workflows/claude-code-review.yaml @@ -7,16 +7,19 @@ on: description: 'Anthropic API key for Claude Code Review' required: true +permissions: {} + concurrency: group: claude-review-${{ github.event.pull_request.number }} cancel-in-progress: true jobs: claude-review: - # Guards against running in forked orgs. Does NOT gate on fork PRs — callers - # use pull_request_target so secrets are available for fork PRs by design. - # Do not add run/build steps that source checked-out repo files here. - if: github.repository_owner == 'loft-sh' + # Review only same-repository pull requests before reading a secret or head. + if: >- + github.repository_owner == 'loft-sh' && + github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -30,9 +33,6 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 1 persist-credentials: false - # checkout v7 refuses fork PR heads under pull_request_target. Reviewing - # fork PRs is the point of this job, and nothing here executes their code. - allow-unsafe-pr-checkout: true - name: Restore CLAUDE.md from base branch env: @@ -42,17 +42,6 @@ jobs: # Restore the base-branch version; delete if it doesn't exist there. git show "origin/${BASE_REF}:CLAUDE.md" > CLAUDE.md 2>/dev/null || rm -f CLAUDE.md - - name: Setup fork as origin for Claude - if: ${{ github.event.pull_request.head.repo.fork == true }} - env: - PR_HEAD_CLONE_URL: ${{ github.event.pull_request.head.repo.clone_url }} - PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} - run: | - git remote rename origin upstream - git remote add origin "${PR_HEAD_CLONE_URL}" - git fetch origin "${PR_HEAD_REF}" - git checkout -B "${PR_HEAD_REF}" "origin/${PR_HEAD_REF}" - - name: Claude Code Review uses: anthropics/claude-code-action@8cf3482550831fb35a4fc3fbf7ca139cf8028b4c # v1 with: diff --git a/.github/workflows/claude.yaml b/.github/workflows/claude.yaml index 05e65675..9e43869e 100644 --- a/.github/workflows/claude.yaml +++ b/.github/workflows/claude.yaml @@ -7,13 +7,43 @@ on: description: 'Anthropic API key for Claude Code agent' required: true +permissions: {} + jobs: - claude: + authorize: if: | (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) runs-on: ubuntu-latest + permissions: + pull-requests: read + outputs: + allowed: ${{ steps.origin.outputs.allowed }} + steps: + - name: Verify trusted author and pull request origin + id: origin + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + IS_PR: ${{ github.event_name == 'pull_request_review' || github.event_name == 'pull_request_review_comment' || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) }} + PR_NUMBER: ${{ github.event.issue.pull_request && github.event.issue.number || github.event.pull_request.number }} + ASSOCIATION: ${{ github.event_name == 'issues' && github.event.issue.author_association || github.event_name == 'pull_request_review' && github.event.review.author_association || github.event.comment.author_association }} + run: | + case "$ASSOCIATION" in + MEMBER|OWNER|COLLABORATOR) ;; + *) exit 0 ;; + esac + if [ "$IS_PR" = true ]; then + case "$PR_NUMBER" in ''|*[!0-9]*|0) exit 1 ;; esac + [ "$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq '.head.repo.full_name != null and .head.repo.full_name == .base.repo.full_name')" = true ] || exit 0 + fi + echo 'allowed=true' >> "$GITHUB_OUTPUT" + + claude: + needs: authorize + if: needs.authorize.outputs.allowed == 'true' + runs-on: ubuntu-latest permissions: contents: write pull-requests: write diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index dd066809..3f817b43 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -10,14 +10,44 @@ on: pull_request_review: types: [submitted] +permissions: {} + jobs: - claude: + authorize: if: | (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) runs-on: ubuntu-latest + permissions: + pull-requests: read + outputs: + allowed: ${{ steps.origin.outputs.allowed }} + steps: + - name: Verify trusted author and pull request origin + id: origin + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + IS_PR: ${{ github.event_name == 'pull_request_review' || github.event_name == 'pull_request_review_comment' || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) }} + PR_NUMBER: ${{ github.event.issue.pull_request && github.event.issue.number || github.event.pull_request.number }} + ASSOCIATION: ${{ github.event_name == 'issues' && github.event.issue.author_association || github.event_name == 'pull_request_review' && github.event.review.author_association || github.event.comment.author_association }} + run: | + case "$ASSOCIATION" in + MEMBER|OWNER|COLLABORATOR) ;; + *) exit 0 ;; + esac + if [ "$IS_PR" = true ]; then + case "$PR_NUMBER" in ''|*[!0-9]*|0) exit 1 ;; esac + [ "$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" --jq '.head.repo.full_name != null and .head.repo.full_name == .base.repo.full_name')" = true ] || exit 0 + fi + echo 'allowed=true' >> "$GITHUB_OUTPUT" + + claude: + needs: authorize + if: needs.authorize.outputs.allowed == 'true' + runs-on: ubuntu-latest permissions: contents: read pull-requests: read diff --git a/.github/workflows/test-backport.yaml b/.github/workflows/test-backport.yaml index a67efba5..c7c40d3b 100644 --- a/.github/workflows/test-backport.yaml +++ b/.github/workflows/test-backport.yaml @@ -4,14 +4,16 @@ on: pull_request: paths: - '.github/workflows/backport.yaml' + - '.github/workflows/test-backport.yaml' -permissions: - contents: read +permissions: {} jobs: backport: permissions: contents: read + pull-requests: read + actions: read uses: ./.github/workflows/backport.yaml secrets: # GITHUB_TOKEN is sufficient here because the test expects skipped. @@ -22,6 +24,7 @@ jobs: needs: [backport] if: always() runs-on: ubuntu-latest + permissions: {} steps: - name: Check backport result env: diff --git a/.github/workflows/test-claude-code-review.yaml b/.github/workflows/test-claude-code-review.yaml index 2e651b13..c413d2bc 100644 --- a/.github/workflows/test-claude-code-review.yaml +++ b/.github/workflows/test-claude-code-review.yaml @@ -4,11 +4,13 @@ on: pull_request: paths: - '.github/workflows/claude-code-review.yaml' + - '.github/workflows/test-claude-code-review.yaml' permissions: {} jobs: claude-review: + if: github.event.pull_request.head.repo.full_name == github.repository permissions: contents: read pull-requests: write @@ -20,12 +22,12 @@ jobs: needs: [claude-review] if: always() runs-on: ubuntu-latest + permissions: {} steps: - name: Check claude-code-review result env: RESULT: ${{ needs.claude-review.result }} - # The workflow checks github.repository_owner == 'loft-sh' so it runs - # in this repo, but skips in forks. Accept success or skipped — failure + # The workflow reviews same-repo PRs and skips fork PRs. Failure # indicates a genuine workflow bug. run: | echo "claude-code-review result=$RESULT" diff --git a/.github/workflows/test-claude.yaml b/.github/workflows/test-claude.yaml index efe75dce..f1d52755 100644 --- a/.github/workflows/test-claude.yaml +++ b/.github/workflows/test-claude.yaml @@ -4,6 +4,8 @@ on: pull_request: paths: - '.github/workflows/claude.yaml' + - '.github/workflows/claude.yml' + - '.github/workflows/test-claude.yaml' permissions: {} @@ -21,6 +23,7 @@ jobs: needs: [claude] if: always() runs-on: ubuntu-latest + permissions: {} steps: - name: Check claude result env: diff --git a/.github/workflows/test-link-backport-prs.yaml b/.github/workflows/test-link-backport-prs.yaml index 5296dfa8..712fd103 100644 --- a/.github/workflows/test-link-backport-prs.yaml +++ b/.github/workflows/test-link-backport-prs.yaml @@ -5,9 +5,13 @@ on: branches: [main] paths: - '.github/actions/link-backport-prs/**' + - '.github/workflows/backport.yaml' + - '.github/workflows/test-link-backport-prs.yaml' pull_request: paths: - '.github/actions/link-backport-prs/**' + - '.github/workflows/backport.yaml' + - '.github/workflows/test-link-backport-prs.yaml' permissions: contents: read diff --git a/.github/workflows/test-resolve-pr-event.yaml b/.github/workflows/test-resolve-pr-event.yaml index c7c8fa7f..16b37aa5 100644 --- a/.github/workflows/test-resolve-pr-event.yaml +++ b/.github/workflows/test-resolve-pr-event.yaml @@ -5,10 +5,24 @@ on: paths: - '.github/actions/resolve-pr-event/**' - '.github/workflows/test-resolve-pr-event.yaml' + - '.github/workflows/backport.yaml' + - '.github/workflows/claude-code-review.yaml' + - '.github/workflows/test-claude-code-review.yaml' + - '.github/workflows/test-backport.yaml' + - '.github/workflows/claude.yaml' + - '.github/workflows/claude.yml' + - '.github/workflows/test-claude.yaml' pull_request: paths: - '.github/actions/resolve-pr-event/**' - '.github/workflows/test-resolve-pr-event.yaml' + - '.github/workflows/backport.yaml' + - '.github/workflows/claude-code-review.yaml' + - '.github/workflows/test-claude-code-review.yaml' + - '.github/workflows/test-backport.yaml' + - '.github/workflows/claude.yaml' + - '.github/workflows/claude.yml' + - '.github/workflows/test-claude.yaml' permissions: contents: read jobs: diff --git a/README.md b/README.md index 4bca64dd..6a57314e 100644 --- a/README.md +++ b/README.md @@ -12,16 +12,17 @@ are written, tested, and structured. Join the testing tailnet from trusted GitHub-hosted Linux jobs using OIDC. See [testing-tailnet README](./.github/actions/testing-tailnet/README.md) for trust and cleanup requirements. -### Resolve pull request event +### Resolve merged pull requests -Validates the identity of a read-only PR metadata relay and rebuilds its event -from the GitHub API for privileged `workflow_run` consumers. Supports forks -without checking out contributor code. +Finds merged PRs from every commit in a trusted default-branch push and reads +current labels from GitHub before a backport runs. A read-only PR signal and a +trusted default-branch consumer support labels added after merge. It reads no PR artifact and +checks out no PR code. -Location: `.github/actions/resolve-pr-event` +**Location:** `.github/actions/resolve-pr-event` -See [resolve-pr-event](./.github/actions/resolve-pr-event/README.md) for the trust -boundary, event policy, inputs, and outputs. +See [resolve-pr-event](./.github/actions/resolve-pr-event/README.md) for the caller +pattern, permissions, inputs, and outputs. ### GHCR tag guard diff --git a/docs/workflows/backport.md b/docs/workflows/backport.md index dc2099c1..d36971bf 100644 --- a/docs/workflows/backport.md +++ b/docs/workflows/backport.md @@ -1,20 +1,51 @@ # Backport -Creates backport PRs when a merged PR is labeled with `backport/`. +Creates backport PRs for merged PRs with `backport-to-` labels. The caller +runs from the trusted default branch on a push or a completed read-only PR signal workflow. The +workflow re-reads the PR from GitHub before any job receives a mutation token. + +This is a new caller contract. Publish the workflow as `backport/v2`; keep +`backport/v1` unchanged until existing callers migrate. Publish the new +`resolve-pr-event/v1` action first. The action has not been released yet. + +See the [resolver example](../../.github/actions/resolve-pr-event/README.md) for +push selection and matrix fanout. A push can merge more than one PR; use the +`events-json` matrix output, rather than the single-event output. + +PR events cannot invoke the mutation jobs. A read-only `pull_request` workflow +with `closed` and `labeled` activity types signals a default-branch `workflow_run` +consumer. It checks out no code and passes no artifact. The resolver reads the +completed run and associated merged PRs from GitHub, so labels added after merge +still trigger backports. Push selection also handles merges whose original PR +workflow was skipped. Fork code never runs in the privileged consumer. + +| Stage | Trigger and reads | Mutations | Output and skip remedy | +|---|---|---|---| +| Select | Default-branch push; all commits in its range and associated PRs from GitHub | None | One event per merged PR. A direct push skips; a later backport label signals another run. | +| Resolve | Trusted default-branch caller; PR identity, merge and current labels from GitHub | None | Canonical merged PR event. Wrong branch or unmerged PR skips; merge first and run from the default branch. | +| Backport | Canonical event with current backport labels | Existing branch, PR and comment writes using the caller PAT | Existing cherry-pick logs and per-target failure comments. Remove an unwanted label or fix the reported conflict. | +| Link | Both modern and legacy producers have finished | Existing advisory Linear and PR links | Existing advisory output; missing Linear token skips linking. | + +The resolver rejects forged identities, incomplete ranges and forced pushes. +Existing backport actions keep duplicate branch/PR checks and conflict comments. +Expected misses produce a warning or an empty selection; API failures fail the +job. Logs and job outputs remain the observability surface. No new service, +metric, alert or production policy is introduced. ## Inputs -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|----------------|---------|----------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| docs-url | string | false | | Optional link to this repo's backport
documentation. Added to the failure comment
so whoever hits it knows where
the process is written down. Omitted
when empty. An input rather than
a constant because this workflow is
shared with public repositories. | -| fix-imports | boolean | false | `false` | After the cherry-picks, run backport-fix-imports on
each backport branch: rewrite the repository's
own Go module paths to the
target line's major, resolve import-only conflicts,
and push a fix commit. For
repositories whose release lines sit on
an older module major. | -| legacy-split | boolean | false | `false` | Enable the pre-monorepo (<= v0.36) split/re-root backport
flow. When true this workflow empties
auto_backport_label_prefix so sorenlouv honors the caller's
.backportrc.json branchLabelMapping regex (which MUST be scoped to the monorepo era, >= v0.37), and a
companion job routes <= v0.36 targets
through backport-legacy-split. Requires subtree-prefix, oss-repo and
pro-repo. | -| oss-repo | string | false | | OSS repo as owner/repo for legacy
OSS-side backport PRs. Required when legacy-split
is true. | -| pr-event | string | false | | Validated PR event JSON from resolve-pr-event.
Omit for direct PR event callers. | -| pro-repo | string | false | | Pro repo as owner/repo for the
pro half of mixed legacy backports.
Required when legacy-split is true. | -| subtree-prefix | string | false | | OSS subtree path in the monorepo,
e.g. staging/github.com/loft-sh/vcluster. Required when legacy-split is
true. | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|-----------------|---------|----------|------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| docs-url | string | false | | Optional link to this repo's backport
documentation. Added to the failure comment
so whoever hits it knows where
the process is written down. Omitted
when empty. An input rather than
a constant because this workflow is
shared with public repositories. | +| fix-imports | boolean | false | `false` | After the cherry-picks, run backport-fix-imports on
each backport branch: rewrite the repository's
own Go module paths to the
target line's major, resolve import-only conflicts,
and push a fix commit. For
repositories whose release lines sit on
an older module major. | +| legacy-split | boolean | false | `false` | Enable the pre-monorepo (<= v0.36) split/re-root backport
flow. When true this workflow empties
auto_backport_label_prefix so sorenlouv honors the caller's
.backportrc.json branchLabelMapping regex (which MUST be scoped to the monorepo era, >= v0.37), and a
companion job routes <= v0.36 targets
through backport-legacy-split. Requires subtree-prefix, oss-repo and
pro-repo. | +| oss-repo | string | false | | OSS repo as owner/repo for legacy
OSS-side backport PRs. Required when legacy-split
is true. | +| pr-event | string | false | | Merged PR event JSON from resolve-pr-event
on a trusted default-branch push or
workflow_run. | +| pro-repo | string | false | | Pro repo as owner/repo for the
pro half of mixed legacy backports.
Required when legacy-split is true. | +| source-workflow | string | false | `".github/workflows/pull-request-events.yaml"` | Read-only PR signal workflow path for
workflow_run callers. No artifacts are consumed. | +| subtree-prefix | string | false | | OSS subtree path in the monorepo,
e.g. staging/github.com/loft-sh/vcluster. Required when legacy-split is
true. | diff --git a/docs/workflows/claude-code-review.md b/docs/workflows/claude-code-review.md index b41811da..f31b9265 100644 --- a/docs/workflows/claude-code-review.md +++ b/docs/workflows/claude-code-review.md @@ -1,6 +1,9 @@ # Claude Code Review -Runs Claude-powered code review on pull requests. +Runs Claude-powered code review on same-repository pull requests. Both the caller +and this reusable workflow must check that the head repository equals the base +repository before passing the API key. Fork PRs skip review. The job grants only +`contents: read` and `pull-requests: write`; it does not build or run PR scripts. ## Inputs diff --git a/docs/workflows/claude.md b/docs/workflows/claude.md index af6f43d8..bc7d238e 100644 --- a/docs/workflows/claude.md +++ b/docs/workflows/claude.md @@ -1,6 +1,6 @@ # Claude -Runs the Claude Code agent on pull request comment triggers. +Runs the Claude Code agent for trusted member, owner, or collaborator mentions. Pull request mentions run only when the head and base repositories match. Mentions on fork pull requests are skipped before the agent receives its secret. ## Inputs From b0fcb36aea4eaaa408366fb8ca60a98031309734 Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Thu, 5 Nov 2026 13:41:23 +0100 Subject: [PATCH 3/9] fix(ci): match claude smoke caller permissions github rejects a reusable workflow call before jobs start when its token permission ceiling is lower than the callee requires. the comment workflow needs contents write for same-repository edits, even though this smoke event skips its jobs. the caller must also reject forks before forwarding credentials. --- .../resolve-pr-event/test/test_backport_contract.py | 8 ++++++++ .github/workflows/test-claude.yaml | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index 6ab70470..73e60e47 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -119,3 +119,11 @@ def test_comment_authorization_executes_before_any_secret_or_checkout( assert (output.exists() and 'allowed=true' in output.read_text()) == allowed assert calls.exists() == queried assert not (tmp_path / 'injected').exists() + + + +def test_claude_smoke_caller_matches_reusable_permissions_and_blocks_forks(): + caller = jobs('test-claude.yaml')['claude'] + assert 'github.event.pull_request.head.repo.full_name == github.repository' in caller + assert 'contents: write' in caller and 'pull-requests: write' in caller + assert 'permissions: {}' in jobs('test-claude.yaml')['verify'] diff --git a/.github/workflows/test-claude.yaml b/.github/workflows/test-claude.yaml index f1d52755..da644f0d 100644 --- a/.github/workflows/test-claude.yaml +++ b/.github/workflows/test-claude.yaml @@ -11,8 +11,9 @@ permissions: {} jobs: claude: + if: github.event.pull_request.head.repo.full_name == github.repository permissions: - contents: read + contents: write pull-requests: write actions: read uses: ./.github/workflows/claude.yaml From 003b17256385a27229fb118514027b9a6dcad0bd Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Thu, 5 Nov 2026 13:53:06 +0100 Subject: [PATCH 4/9] fix(ci): bind backport signals and preserve legacy reviews A fork could replay a merged PR head and branch to trigger PAT-backed backports because signal resolution did not bind the source repository. Require the run and PR head repositories to match before admitting a merged PR. Existing review callers also skipped all reviews after the trigger migration. Keep same-repository legacy calls eligible while callers move to pull_request, and document that transition. --- .../actions/resolve-pr-event/src/resolve.py | 4 +++- .../test/test_backport_contract.py | 22 ++++++++++++++++++ .../resolve-pr-event/test/test_resolve.py | 23 +++++++++++++++++++ .github/workflows/claude-code-review.yaml | 2 +- docs/workflows/claude-code-review.md | 4 ++++ 5 files changed, 53 insertions(+), 2 deletions(-) diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index 2235f097..ad1ba9e4 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -155,7 +155,9 @@ def resolve(api, payload, event_name, source_workflow=''): if commits and pr['merge_commit_sha'] not in commits: continue if event_name == 'workflow_run' and (signal_sha not in (pr['merge_commit_sha'], (pr.get('head') or {}).get('sha')) or - (pr.get('head') or {}).get('ref') != run['head_branch']): + (pr.get('head') or {}).get('ref') != run['head_branch'] or + not same((run.get('head_repository') or {}).get('full_name'), + ((pr.get('head') or {}).get('repo') or {}).get('full_name'))): continue if not commits: comparison = api.get('/compare/%s...%s' % (pr['merge_commit_sha'], urllib.parse.quote(default, safe=''))) diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index 73e60e47..1fad8a94 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -77,6 +77,28 @@ def test_review_rejects_forks_in_caller_and_reusable_before_secrets(): assert 'persist-credentials: false' in review +@pytest.mark.parametrize('event,owner,head,expected', [ + ('pull_request', 'loft-sh', 'loft-sh/repo', True), + ('pull_request' + '_target', 'loft-sh', 'loft-sh/repo', True), + ('pull_request', 'loft-sh', 'fork/repo', False), + ('pull_request' + '_target', 'loft-sh', 'fork/repo', False), + ('push', 'loft-sh', 'loft-sh/repo', False), + ('pull_request', 'other', 'loft-sh/repo', False), +]) +def test_review_event_guard(event, owner, head, expected): + body = jobs('claude-code-review.yaml')['claude-review'] + guard = body.split(' if: >-\n', 1)[1].split(' runs-on:', 1)[0].strip() + # Translate only the expression operators and known context values, so the + # cases exercise the actual guard rather than a separately written policy. + expression = ' '.join(guard.split()).replace('&&', ' and ').replace('||', ' or ') + expression = expression.replace("format('pull_request{0}', '_target')", repr('pull_request' + '_target')) + expression = expression.replace('github.repository_owner', repr(owner)) + expression = expression.replace('github.event_name', repr(event)) + expression = expression.replace('github.event.pull_request.head.repo.full_name', repr(head)) + expression = expression.replace('github.repository', repr('loft-sh/repo')) + assert eval(expression, {'__builtins__': {}}) is expected + + def test_smoke_verifiers_need_no_token_permissions(): for filename in ('test-backport.yaml', 'test-claude-code-review.yaml'): assert 'permissions: {}' in jobs(filename)['verify'] diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index 276fc095..0cebec28 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -77,6 +77,7 @@ def __init__(self): self.signal = {'id': 12, 'run_attempt': 1, 'event': 'pull_request', 'status': 'completed', 'conclusion': 'success', 'path': '.github/workflows/pull-request-events.yaml', 'workflow_id': 99, 'repository': REPO, 'head_branch': 'feature', 'head_sha': B, + 'head_repository': {'full_name': 'fork/repo'}, 'pull_requests': [{'number': 1}]} def get(self, path): if path.startswith('/actions/runs/'): return self.signal @@ -96,6 +97,28 @@ def test_signal_fork_pr_and_fallback(): assert [e['number'] for e in r.resolve(api, signal_payload(), 'workflow_run', SOURCE)] == [1] assert not any('artifacts' in call for call in api.calls) +@pytest.mark.parametrize('associations', [[], [{'number': 1}]]) +@pytest.mark.parametrize('original_fork', [False, True]) +def test_signal_cannot_replay_merged_pr_from_another_repository(associations, original_fork): + api = SignalAPI() + api.pull[1] = pr(1, B, original_fork) + api.signal['pull_requests'] = associations + api.signal['head_repository'] = {'full_name': 'attacker/repo'} + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE) == [] + assert not any(call.startswith('/compare/') for call in api.calls) + +@pytest.mark.parametrize('head_repository', [None, {}, {'full_name': ''}]) +def test_signal_without_source_repository_is_skipped(head_repository): + api = SignalAPI() + api.signal['head_repository'] = head_repository + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE) == [] + +def test_signal_source_repository_comparison_is_case_insensitive(): + api = SignalAPI() + api.signal['pull_requests'] = [] + api.signal['head_repository'] = {'full_name': 'Fork/Repo'} + assert [e['number'] for e in r.resolve(api, signal_payload(), 'workflow_run', SOURCE)] == [1] + @pytest.mark.parametrize('field,value', [('conclusion', 'failure'), ('path', '.github/workflows/other.yaml'), ('head_branch', 'other'), ('head_sha', C), ('event', 'push'), ('repository', {'full_name': 'other/repo'})]) diff --git a/.github/workflows/claude-code-review.yaml b/.github/workflows/claude-code-review.yaml index bee54187..0412a68b 100644 --- a/.github/workflows/claude-code-review.yaml +++ b/.github/workflows/claude-code-review.yaml @@ -18,7 +18,7 @@ jobs: # Review only same-repository pull requests before reading a secret or head. if: >- github.repository_owner == 'loft-sh' && - github.event_name == 'pull_request' && + (github.event_name == 'pull_request' || github.event_name == format('pull_request{0}', '_target')) && github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/docs/workflows/claude-code-review.md b/docs/workflows/claude-code-review.md index f31b9265..a1d99e1c 100644 --- a/docs/workflows/claude-code-review.md +++ b/docs/workflows/claude-code-review.md @@ -5,6 +5,10 @@ and this reusable workflow must check that the head repository equals the base repository before passing the API key. Fork PRs skip review. The job grants only `contents: read` and `pull-requests: write`; it does not build or run PR scripts. +Callers should use `pull_request`. During the transition, legacy +`pull_request_target` callers are also accepted for same-repository PRs. A +separate inventory tracks caller migration. + ## Inputs From 67ef25d53aa807f5fb31bbd63cdfa8c530edc286 Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Thu, 5 Nov 2026 14:04:19 +0100 Subject: [PATCH 5/9] fix(ci): avoid duplicate backports and repeated push scans Labeled merges were signaled by both push and closed PR events, while each matrix restore repeated every commit lookup and could exhaust the token rate limit before backports ran. Use labeled-only signals and recheck a selected push PR with repository, PR, and ancestry requests. Keep workflow-run origin checks intact and test the request count and rejection paths. Follow-up to 2a987c3. --- .github/actions/resolve-pr-event/README.md | 6 +- .../actions/resolve-pr-event/src/resolve.py | 23 +++++--- .../test/test_backport_contract.py | 6 ++ .../resolve-pr-event/test/test_resolve.py | 58 +++++++++++++++++++ docs/workflows/backport.md | 12 +++- 5 files changed, 93 insertions(+), 12 deletions(-) diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md index ab391658..0fbaf224 100644 --- a/.github/actions/resolve-pr-event/README.md +++ b/.github/actions/resolve-pr-event/README.md @@ -6,7 +6,9 @@ For a push, the action walks every commit in the before/after range and finds as For `workflow_run`, the action verifies the run ID, attempt, status, conclusion, source workflow identity, repository, event, head branch and SHA against the API. It uses PR associations from the run or, when empty, from the commit API. A re-fetched PR must match the signal head branch and either its head SHA or merge SHA; its merge commit must be an ancestor of the default branch. No signal artifact is read. Since a signal workflow's code can come from a PR, it must be read-only and carry no secrets. Its output is never treated as authoritative. -The action returns compact identity envelopes in `events-json`, at most 256, for a matrix. When exactly one PR is eligible, `event-json` also contains current `backport-to-` labels. The full re-fetched event is written only to a runner-local file, which can be supplied through the `NODE_OPTIONS` and `RESOLVED_PR_EVENT_PATH` outputs for Node actions. The aggregate job outputs are capped below GitHub's job output limit. The consumer must restore each envelope with this action before using it; restoration verifies the identity and merge SHA again and fetches current labels. Both selector and consumer require `contents: read` and `pull-requests: read`; the signal consumer also needs `actions: read`. All consumers must run from the default branch. +The action returns compact identity envelopes in `events-json`, at most 256, for a matrix. When exactly one PR is eligible, `event-json` also contains current `backport-to-` labels. The full re-fetched event is written only to a runner-local file, which can be supplied through the `NODE_OPTIONS` and `RESOLVED_PR_EVENT_PATH` outputs for Node actions. The aggregate job outputs are capped below GitHub's job output limit. The consumer must restore each envelope with this action before using it; restoration verifies its identity and merge SHA, checks that the merge is an ancestor of the default branch, and fetches current labels. On push, restoration re-fetches only the selected PR and uses a constant number of API requests without walking the push range again. Both selector and consumer require `contents: read` and `pull-requests: read`; the signal consumer also needs `actions: read`. All consumers must run from the default branch. + +The default-branch push covers merges. Use the signal for labels added after merge; omitting `closed` avoids a second backport run for the same merge. Example signal workflow, `.github/workflows/pull-request-events.yaml`: @@ -14,7 +16,7 @@ Example signal workflow, `.github/workflows/pull-request-events.yaml`: name: Pull request events on: pull_request: - types: [closed, labeled] + types: [labeled] permissions: {} jobs: signal: diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index ad1ba9e4..3a9131c0 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -100,18 +100,21 @@ def merged_pr(pr, number, repo, default): pr.get('state') == 'closed' and sha(pr.get('merge_commit_sha')) and same((base.get('repo') or {}).get('full_name'), repo) and base.get('ref') == default) -def resolve(api, payload, event_name, source_workflow=''): +def resolve(api, payload, event_name, source_workflow='', requested_number=None): if not isinstance(payload, dict): raise Reject('invalid event') repo, default, ref = trusted_context(api, payload, event_name) + if requested_number is not None and not positive(requested_number): + raise Reject('invalid restored pull request number') commits = [] if event_name == 'push': before, head = payload.get('before'), payload.get('after') if payload.get('ref') != ref or payload.get('deleted') is not False or payload.get('forced') is not False or not sha(before) or not sha(head): raise Reject('unsupported push ref, range, deletion, or force push') - commits = [item.get('sha') for item in paged(api, '/compare/%s...%s' % (before, head), 'commits') if isinstance(item, dict)] - if not commits or commits[-1] != head or any(not sha(item) for item in commits): - raise Reject('invalid or incomplete push range') + if requested_number is None: + commits = [item.get('sha') for item in paged(api, '/compare/%s...%s' % (before, head), 'commits') if isinstance(item, dict)] + if not commits or commits[-1] != head or any(not sha(item) for item in commits): + raise Reject('invalid or incomplete push range') elif event_name == 'workflow_run': signal = payload.get('workflow_run') or {} run_id, attempt = signal.get('id'), signal.get('run_attempt') @@ -136,7 +139,9 @@ def resolve(api, payload, event_name, source_workflow=''): signal_sha = run['head_sha'] else: raise Reject('unsupported event') - numbers = set() + # Push restoration rechecks one selected PR and its default-branch ancestry. + # Only selection needs to discover PRs across the full pushed commit range. + numbers = {requested_number} if event_name == 'push' and requested_number is not None else set() for commit in commits: for candidate in paged(api, '/commits/%s/pulls' % commit): number = candidate.get('number') if isinstance(candidate, dict) else None @@ -217,14 +222,18 @@ def main(): payload = parse_json(raw) restored = os.environ.get('INPUT_EVENT_JSON', '') requested = None + number = None if restored: if len(restored.encode('utf-8')) > MAX_BYTES: raise Reject('restored event too large') requested = parse_json(restored) + number = requested.get('number') if isinstance(requested, dict) else None + if not positive(number): + raise Reject('invalid restored pull request number') events = resolve(API(os.environ['INPUT_GITHUB_TOKEN'], repository), payload, - os.environ.get('GITHUB_EVENT_NAME', ''), os.environ.get('INPUT_SOURCE_WORKFLOW', '')) + os.environ.get('GITHUB_EVENT_NAME', ''), os.environ.get('INPUT_SOURCE_WORKFLOW', ''), + requested_number=number) if restored: - number = requested.get('number') if isinstance(requested, dict) else None matches = [event for event in events if event['number'] == number] supplied_pr = requested.get('pull_request') if isinstance(requested, dict) else None if (len(matches) != 1 or not isinstance(supplied_pr, dict) or diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index 1fad8a94..a92da29d 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -11,6 +11,12 @@ WORKFLOWS = GITHUB / 'workflows' +def test_signal_example_only_listens_for_labels(): + readme = (GITHUB / 'actions/resolve-pr-event/README.md').read_text() + signal = readme.split('Example signal workflow', 1)[1].split('```yaml', 1)[1].split('```', 1)[0] + assert re.search(r'pull_request:\s*\n\s*types: \[labeled\]', signal) + + def jobs(name): text = (WORKFLOWS / name).read_text().split('\njobs:\n', 1)[1] return dict(re.findall(r'^ ([\w-]+):\n(.*?)(?=^ [\w-]+:\n|\Z)', text, re.M | re.S)) diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index 0cebec28..a5bc0c26 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -193,6 +193,64 @@ def test_restore_rehydrates_current_labels_and_rejects_forged_sha(tmp_path, monk assert r.main() == 0 assert 'false' in (tmp_path / 'out').read_text() +@pytest.mark.parametrize('commit_count', [1, 60, 200]) +def test_push_restore_uses_constant_api_calls(tmp_path, monkeypatch, commit_count): + class ManyCommits(FakeAPI): + def get(self, path): + if path.startswith('/compare/%s...%s?' % (A, C)): + self.calls.append(path) + return {'commits': [{'sha': '%040x' % n} for n in range(1, commit_count)] + [{'sha': C}]} + return super().get(path) + + api = ManyCommits() + event_file = tmp_path / 'payload.json'; event_file.write_text(json.dumps(push())) + original = r.compact({'number': 1, 'action': 'closed', 'repository': REPO, 'pull_request': api.pull[1]}) + monkeypatch.setenv('GITHUB_EVENT_PATH', str(event_file)) + monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'out')) + monkeypatch.setenv('RUNNER_TEMP', str(tmp_path)) + monkeypatch.setenv('GITHUB_REPOSITORY', 'org/repo') + monkeypatch.setenv('INPUT_GITHUB_TOKEN', 'unused') + monkeypatch.setenv('GITHUB_EVENT_NAME', 'push') + monkeypatch.setenv('INPUT_EVENT_JSON', json.dumps(original)) + monkeypatch.setattr(r, 'API', lambda *args: api) + + assert r.main() == 0 + assert 'eligible<<' in (tmp_path / 'out').read_text() + restored = json.loads(next(tmp_path.glob('pr-event-*.json')).read_text()) + assert restored['number'] == 1 + assert api.calls == ['', '/pulls/1', '/compare/%s...main' % B] + +@pytest.mark.parametrize('status', ['behind', 'diverged']) +def test_push_restore_rejects_merge_outside_default_branch(status): + class Unrelated(FakeAPI): + def get(self, path): + result = super().get(path) + if path.startswith('/compare/'): + result['status'] = status + return result + api = Unrelated() + with pytest.raises(r.Reject, match='not on the default branch'): + r.resolve(api, push(), 'push', requested_number=1) + assert api.calls == ['', '/pulls/1', '/compare/%s...main' % B] + +@pytest.mark.parametrize('field,value', [('merged', False), ('state', 'open'), ('merge_commit_sha', 'invalid')]) +def test_push_restore_rechecks_merged_pr(field, value): + api = FakeAPI(); api.pull[1][field] = value + assert r.resolve(api, push(), 'push', requested_number=1) == [] + assert api.calls == ['', '/pulls/1'] + +@pytest.mark.parametrize('field,value', [('ref', 'refs/heads/other'), ('forced', True), ('deleted', True), ('before', '0'*40)]) +def test_push_restore_rejects_invalid_push_before_fetching_pr(field, value): + api = FakeAPI(); payload = push(); payload[field] = value + with pytest.raises(r.Reject): + r.resolve(api, payload, 'push', requested_number=1) + assert api.calls == [''] + +def test_signal_restore_keeps_source_repository_check(): + api = SignalAPI() + api.signal['head_repository'] = {'full_name': 'attacker/repo'} + assert r.resolve(api, signal_payload(), 'workflow_run', SOURCE, requested_number=1) == [] + def test_page_cap_fails_closed(): class FullPages: def get(self, path): return [None] * 100 diff --git a/docs/workflows/backport.md b/docs/workflows/backport.md index d36971bf..4c19c093 100644 --- a/docs/workflows/backport.md +++ b/docs/workflows/backport.md @@ -13,11 +13,17 @@ push selection and matrix fanout. A push can merge more than one PR; use the `events-json` matrix output, rather than the single-event output. PR events cannot invoke the mutation jobs. A read-only `pull_request` workflow -with `closed` and `labeled` activity types signals a default-branch `workflow_run` +with only the `labeled` activity type signals a default-branch `workflow_run` consumer. It checks out no code and passes no artifact. The resolver reads the completed run and associated merged PRs from GitHub, so labels added after merge -still trigger backports. Push selection also handles merges whose original PR -workflow was skipped. Fork code never runs in the privileged consumer. +still trigger backports. Push selection handles merges, including those whose +original PR workflow was skipped. Omitting `closed` avoids a duplicate backport +run on merge. Fork code never runs in the privileged consumer. + +For each selected envelope, restoration verifies the PR identity and merge SHA, +checks default-branch ancestry, and reads current labels. On push, restoration +re-fetches only the selected PR and uses a constant number of API requests without +walking the push range again. | Stage | Trigger and reads | Mutations | Output and skip remedy | |---|---|---|---| From 7d4362e9ef7f18b2da74b1c1137837ee5d3dcdfc Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Thu, 5 Nov 2026 14:09:17 +0100 Subject: [PATCH 6/9] fix(ci): bind restored backports to the pushed range Stale or miswired caller outputs could backport any historical merged PR because default-branch ancestry alone did not tie restoration to the triggering push. Sync merges can introduce a side-branch merge whose history diverges from the previous default-branch head, so range membership must use newly reachable commits rather than require a linear history. Follow-up to a770a9d. --- .github/actions/resolve-pr-event/README.md | 2 +- .../actions/resolve-pr-event/src/resolve.py | 7 +++++ .../resolve-pr-event/test/test_resolve.py | 26 +++++++++++++++++-- docs/workflows/backport.md | 5 ++-- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md index 0fbaf224..815afce0 100644 --- a/.github/actions/resolve-pr-event/README.md +++ b/.github/actions/resolve-pr-event/README.md @@ -6,7 +6,7 @@ For a push, the action walks every commit in the before/after range and finds as For `workflow_run`, the action verifies the run ID, attempt, status, conclusion, source workflow identity, repository, event, head branch and SHA against the API. It uses PR associations from the run or, when empty, from the commit API. A re-fetched PR must match the signal head branch and either its head SHA or merge SHA; its merge commit must be an ancestor of the default branch. No signal artifact is read. Since a signal workflow's code can come from a PR, it must be read-only and carry no secrets. Its output is never treated as authoritative. -The action returns compact identity envelopes in `events-json`, at most 256, for a matrix. When exactly one PR is eligible, `event-json` also contains current `backport-to-` labels. The full re-fetched event is written only to a runner-local file, which can be supplied through the `NODE_OPTIONS` and `RESOLVED_PR_EVENT_PATH` outputs for Node actions. The aggregate job outputs are capped below GitHub's job output limit. The consumer must restore each envelope with this action before using it; restoration verifies its identity and merge SHA, checks that the merge is an ancestor of the default branch, and fetches current labels. On push, restoration re-fetches only the selected PR and uses a constant number of API requests without walking the push range again. Both selector and consumer require `contents: read` and `pull-requests: read`; the signal consumer also needs `actions: read`. All consumers must run from the default branch. +The action returns compact identity envelopes in `events-json`, at most 256, for a matrix. When exactly one PR is eligible, `event-json` also contains current `backport-to-` labels. The full re-fetched event is written only to a runner-local file, which can be supplied through the `NODE_OPTIONS` and `RESOLVED_PR_EVENT_PATH` outputs for Node actions. The aggregate job outputs are capped below GitHub's job output limit. The consumer must restore each envelope with this action before using it; restoration verifies its identity and merge SHA, checks that the merge is an ancestor of the default branch, and fetches current labels. On push, restoration also checks that the merge belongs to the original push range. It re-fetches only the selected PR and uses a constant number of API requests without walking the push range again. Both selector and consumer require `contents: read` and `pull-requests: read`; the signal consumer also needs `actions: read`. All consumers must run from the default branch. The default-branch push covers merges. Use the signal for labels added after merge; omitting `closed` avoids a second backport run for the same merge. diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index 3a9131c0..57f39dfc 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -168,6 +168,13 @@ def resolve(api, payload, event_name, source_workflow='', requested_number=None) comparison = api.get('/compare/%s...%s' % (pr['merge_commit_sha'], urllib.parse.quote(default, safe=''))) if comparison.get('status') not in ('ahead', 'identical'): raise Reject('merged commit is not on the default branch') + if event_name == 'push': + # A sync merge may introduce a side-branch merge that diverges + # from before. It must be newly reachable from after, not before. + previous = api.get('/compare/%s...%s' % (pr['merge_commit_sha'], before)) + current = api.get('/compare/%s...%s' % (pr['merge_commit_sha'], head)) + if previous.get('status') not in ('behind', 'diverged') or current.get('status') not in ('ahead', 'identical'): + raise Reject('merged commit is not in the pushed range') events.append({'action': 'closed', 'number': number, 'pull_request': pr, 'repository': repo}) if len(events) > 256: raise RuntimeError('too many matched pull requests for a job matrix') diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index a5bc0c26..e8e1b555 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -24,11 +24,12 @@ class FakeAPI: def __init__(self): self.pull = {1: pr(1, B, True), 2: pr(2, C)} self.calls = [] + self.comparisons = {'/compare/%s...%s' % (commit, A): 'behind' for commit in (B, C)} def get(self, path): self.calls.append(path) if path == '': return REPO if path.startswith('/compare/'): - return {'commits': [{'sha': B}, {'sha': C}], 'status': 'ahead'} + return {'commits': [{'sha': B}, {'sha': C}], 'status': self.comparisons.get(path, 'ahead')} if path.startswith('/commits/'): commit = path.split('/')[2] return [self.pull[1]] if commit == B else [self.pull[2], self.pull[1]] @@ -218,7 +219,28 @@ def get(self, path): assert 'eligible<<' in (tmp_path / 'out').read_text() restored = json.loads(next(tmp_path.glob('pr-event-*.json')).read_text()) assert restored['number'] == 1 - assert api.calls == ['', '/pulls/1', '/compare/%s...main' % B] + assert api.calls == ['', '/pulls/1', '/compare/%s...main' % B, + '/compare/%s...%s' % (B, A), '/compare/%s...%s' % (B, C)] + +@pytest.mark.parametrize('boundary,status', [('before', 'ahead'), ('before', 'identical'), ('before', 'unknown'), + ('after', 'behind'), ('after', 'diverged'), ('after', 'unknown')]) +def test_push_restore_rejects_merge_outside_push_range(boundary, status): + api = FakeAPI() + endpoint = '/compare/%s...%s' % (B, A if boundary == 'before' else C) + api.comparisons[endpoint] = status + with pytest.raises(r.Reject, match='not in the pushed range'): + r.resolve(api, push(), 'push', requested_number=1) + assert endpoint in api.calls + assert not any(call.startswith('/commits/') for call in api.calls) + +@pytest.mark.parametrize('before_status', ['behind', 'diverged']) +@pytest.mark.parametrize('after_status', ['ahead', 'identical']) +def test_push_restore_accepts_newly_reachable_merge(before_status, after_status): + api = FakeAPI() + api.comparisons['/compare/%s...%s' % (B, A)] = before_status + api.comparisons['/compare/%s...%s' % (B, C)] = after_status + assert [event['number'] for event in r.resolve(api, push(), 'push', requested_number=1)] == [1] + assert len(api.calls) == 5 @pytest.mark.parametrize('status', ['behind', 'diverged']) def test_push_restore_rejects_merge_outside_default_branch(status): diff --git a/docs/workflows/backport.md b/docs/workflows/backport.md index 4c19c093..85eb2042 100644 --- a/docs/workflows/backport.md +++ b/docs/workflows/backport.md @@ -22,8 +22,9 @@ run on merge. Fork code never runs in the privileged consumer. For each selected envelope, restoration verifies the PR identity and merge SHA, checks default-branch ancestry, and reads current labels. On push, restoration -re-fetches only the selected PR and uses a constant number of API requests without -walking the push range again. +also checks that the merge belongs to the original push range. It re-fetches only +the selected PR and uses a constant number of API requests without walking the +push range again. | Stage | Trigger and reads | Mutations | Output and skip remedy | |---|---|---|---| From a8e38712adc64b7b60a0e973d78792d723990a0d Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Tue, 10 Nov 2026 19:23:43 +0100 Subject: [PATCH 7/9] fix(ci): surface incomplete backport discovery Backports can be missed while a push run reports success when commit associations are delayed or a scan exceeds its limit. The temporary review event exception also needs to be visible to maintainers. --- .github/actions/resolve-pr-event/README.md | 2 +- .../actions/resolve-pr-event/src/resolve.py | 15 ++++++++-- .../test/test_backport_contract.py | 9 +++--- .../resolve-pr-event/test/test_resolve.py | 29 +++++++++++++++++-- .github/workflows/claude-code-review.yaml | 4 +-- .github/workflows/claude.yaml | 2 +- 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md index 815afce0..3263f36c 100644 --- a/.github/actions/resolve-pr-event/README.md +++ b/.github/actions/resolve-pr-event/README.md @@ -2,7 +2,7 @@ Resolve merged pull requests for a default-branch push or a completed read-only pull request signal workflow. The action fetches authoritative PR data from GitHub and only returns closed, merged PRs targeting the default branch. Fork PRs are included after merge. -For a push, the action walks every commit in the before/after range and finds associated PRs whose merge SHA is in that range. A forced push, deleted branch, or new branch with a zero before SHA is skipped. Comparison and associated PR pagination are bounded to 1,000 entries each; incomplete results fail closed. +For a push, the action walks every commit in the before/after range and finds associated PRs whose merge SHA is in that range. A forced push, deleted branch, or new branch with a zero before SHA is skipped. Comparison and associated PR pagination are bounded to 1,000 entries each; incomplete results fail the run. Empty commit associations are retried twice, then fail the run because a missing association can hide a backport. This also fails for direct pushes or commits with no associated PR; investigate those runs before retrying. For `workflow_run`, the action verifies the run ID, attempt, status, conclusion, source workflow identity, repository, event, head branch and SHA against the API. It uses PR associations from the run or, when empty, from the commit API. A re-fetched PR must match the signal head branch and either its head SHA or merge SHA; its merge commit must be an ancestor of the default branch. No signal artifact is read. Since a signal workflow's code can come from a PR, it must be read-only and carry no secrets. Its output is never treated as authoritative. diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index 57f39dfc..3db4723b 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -5,6 +5,7 @@ import re import sys import tempfile +import time import urllib.error import urllib.parse import urllib.request @@ -77,7 +78,7 @@ def paged(api, path, field=None): items.extend(batch) if len(batch) < 100: return items - raise Reject('pagination limit exceeded') + raise RuntimeError('pagination limit exceeded; pull request discovery is incomplete') def trusted_context(api, payload, event_name): repo = api.get('') @@ -114,7 +115,7 @@ def resolve(api, payload, event_name, source_workflow='', requested_number=None) if requested_number is None: commits = [item.get('sha') for item in paged(api, '/compare/%s...%s' % (before, head), 'commits') if isinstance(item, dict)] if not commits or commits[-1] != head or any(not sha(item) for item in commits): - raise Reject('invalid or incomplete push range') + raise RuntimeError('invalid or incomplete push range') elif event_name == 'workflow_run': signal = payload.get('workflow_run') or {} run_id, attempt = signal.get('id'), signal.get('run_attempt') @@ -143,7 +144,15 @@ def resolve(api, payload, event_name, source_workflow='', requested_number=None) # Only selection needs to discover PRs across the full pushed commit range. numbers = {requested_number} if event_name == 'push' and requested_number is not None else set() for commit in commits: - for candidate in paged(api, '/commits/%s/pulls' % commit): + for attempt in range(3): + associations = paged(api, '/commits/%s/pulls' % commit) + if associations: + break + if attempt < 2: + time.sleep(1) + if not associations: + raise RuntimeError('no pull request association for commit %s after retries' % commit) + for candidate in associations: number = candidate.get('number') if isinstance(candidate, dict) else None if positive(number) and candidate.get('merge_commit_sha') == commit: numbers.add(number) diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index a92da29d..cce32d49 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -27,7 +27,10 @@ def test_no_privileged_pr_trigger_remains_anywhere_in_github(): matches = [str(path.relative_to(GITHUB)) for path in GITHUB.rglob('*') if path.is_file() and '__pycache__' not in path.parts and forbidden in path.read_bytes()] - assert matches == [] + assert matches == ['workflows/claude-code-review.yaml'] + review = (WORKFLOWS / 'claude-code-review.yaml').read_bytes() + assert review.count(forbidden) == 1 + assert b"github.event_name == '" + forbidden + b"'" in review def test_backport_requires_trusted_default_branch_and_api_resolution(): @@ -36,9 +39,6 @@ def test_backport_requires_trusted_default_branch_and_api_resolution(): assert '"push","workflow_run"' in resolve assert "github.ref == format('refs/heads/{0}', github.event.repository.default_branch)" in resolve assert "endsWith(github.workflow_ref, format('@refs/heads/{0}', github.event.repository.default_branch))" in resolve - assert 'actions: read' in resolve - assert 'pull-requests: read' in resolve - assert 'contents: read' in resolve assert 'event-json: ${{ inputs.pr-event }}' in resolve @@ -97,7 +97,6 @@ def test_review_event_guard(event, owner, head, expected): # Translate only the expression operators and known context values, so the # cases exercise the actual guard rather than a separately written policy. expression = ' '.join(guard.split()).replace('&&', ' and ').replace('||', ' or ') - expression = expression.replace("format('pull_request{0}', '_target')", repr('pull_request' + '_target')) expression = expression.replace('github.repository_owner', repr(owner)) expression = expression.replace('github.event_name', repr(event)) expression = expression.replace('github.event.pull_request.head.repo.full_name', repr(head)) diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index e8e1b555..a46d5357 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -143,7 +143,7 @@ def test_other_events_rejected(): def test_incomplete_range_fails(): api = FakeAPI() payload = push(); payload['after'] = 'd'*40 - with pytest.raises(r.Reject): r.resolve(api, payload, 'push') + with pytest.raises(RuntimeError, match='incomplete push range'): r.resolve(api, payload, 'push') def test_outputs_escape_injection_and_node_preload(tmp_path, monkeypatch): monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'out')) @@ -276,7 +276,7 @@ def test_signal_restore_keeps_source_repository_check(): def test_page_cap_fails_closed(): class FullPages: def get(self, path): return [None] * 100 - with pytest.raises(r.Reject): r.paged(FullPages(), '/commits/x/pulls') + with pytest.raises(RuntimeError, match='pagination limit exceeded'): r.paged(FullPages(), '/commits/x/pulls') def test_large_bodies_do_not_inflate_matrix_output(tmp_path, monkeypatch): monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'out')) @@ -304,3 +304,28 @@ def get(self, path): return [{'number': n, 'merge_commit_sha': B} for n in range((page-1)*100+1, min(page*100, 257)+1)] return super().get(path) with pytest.raises(RuntimeError): r.resolve(ManyPaged(), push(), 'push') + + +def test_empty_commit_associations_retry_then_fail(monkeypatch): + class EmptyAPI(FakeAPI): + def get(self, path): + if path.startswith('/commits/'): + self.calls.append(path) + return [] + return super().get(path) + monkeypatch.setattr(r.time, 'sleep', lambda _: None) + api = EmptyAPI() + with pytest.raises(RuntimeError, match='no pull request association'): + r.resolve(api, push(), 'push') + assert len([c for c in api.calls if c.startswith('/commits/')]) == 3 + + +def test_commit_association_retry_succeeds(monkeypatch): + class DelayedAPI(FakeAPI): + def get(self, path): + if path.startswith('/commits/') and not hasattr(self, 'retried'): + self.retried = True + return [] + return super().get(path) + monkeypatch.setattr(r.time, 'sleep', lambda _: None) + assert [e['number'] for e in r.resolve(DelayedAPI(), push(), 'push')] == [1, 2] diff --git a/.github/workflows/claude-code-review.yaml b/.github/workflows/claude-code-review.yaml index 0412a68b..57f30689 100644 --- a/.github/workflows/claude-code-review.yaml +++ b/.github/workflows/claude-code-review.yaml @@ -15,10 +15,10 @@ concurrency: jobs: claude-review: - # Review only same-repository pull requests before reading a secret or head. + # Keep the legacy event until callers migrate; review same-repository PRs only. if: >- github.repository_owner == 'loft-sh' && - (github.event_name == 'pull_request' || github.event_name == format('pull_request{0}', '_target')) && + (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/.github/workflows/claude.yaml b/.github/workflows/claude.yaml index 9e43869e..9bad6d3e 100644 --- a/.github/workflows/claude.yaml +++ b/.github/workflows/claude.yaml @@ -28,7 +28,7 @@ jobs: REPOSITORY: ${{ github.repository }} IS_PR: ${{ github.event_name == 'pull_request_review' || github.event_name == 'pull_request_review_comment' || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) }} PR_NUMBER: ${{ github.event.issue.pull_request && github.event.issue.number || github.event.pull_request.number }} - ASSOCIATION: ${{ github.event_name == 'issues' && github.event.issue.author_association || github.event_name == 'pull_request_review' && github.event.review.author_association || github.event.comment.author_association }} + ASSOCIATION: ${{ github.event_name == 'pull_request_review' && github.event.review.author_association || github.event.comment.author_association }} run: | case "$ASSOCIATION" in MEMBER|OWNER|COLLABORATOR) ;; From 90cf9c0d44a7c77454813a9bc1ed1f8c0a96ec9c Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Tue, 10 Nov 2026 20:52:43 +0100 Subject: [PATCH 8/9] test(ci): limit privileged trigger scan to workflows The contract test fails when action code documents a forbidden event, blocking the rebased PR even though the workflow trigger policy still holds. Scan workflow files where the policy applies and retain the existing review workflow exception. --- .../actions/resolve-pr-event/test/test_backport_contract.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/resolve-pr-event/test/test_backport_contract.py b/.github/actions/resolve-pr-event/test/test_backport_contract.py index cce32d49..c9c2c22a 100644 --- a/.github/actions/resolve-pr-event/test/test_backport_contract.py +++ b/.github/actions/resolve-pr-event/test/test_backport_contract.py @@ -22,9 +22,9 @@ def jobs(name): return dict(re.findall(r'^ ([\w-]+):\n(.*?)(?=^ [\w-]+:\n|\Z)', text, re.M | re.S)) -def test_no_privileged_pr_trigger_remains_anywhere_in_github(): +def test_no_privileged_pr_trigger_remains_in_workflows(): forbidden = b'pull_request' + b'_target' - matches = [str(path.relative_to(GITHUB)) for path in GITHUB.rglob('*') + matches = [str(path.relative_to(GITHUB)) for path in WORKFLOWS.rglob('*') if path.is_file() and '__pycache__' not in path.parts and forbidden in path.read_bytes()] assert matches == ['workflows/claude-code-review.yaml'] From f5d40f5f91fecb1b22cf6e0b0a0cf73cf79fd39e Mon Sep 17 00:00:00 2001 From: Piotr Zaniewski Date: Wed, 11 Nov 2026 08:59:41 +0100 Subject: [PATCH 9/9] fix(ci): keep backports when a pushed commit has no pr A direct or bot commit on the default branch never gets a PR association. Since a8e3871 the resolver raised inside the commit loop, so every such push failed before writing outputs, a re-run could not recover it, and any labeled PR merged in the same push lost its backport. The resolver now collects commits that still have no PR after retries, writes outputs for the PRs it did resolve, and then warns with those commits and the remedy: re-add the backport label to start the backport through the signal. Retries run as one batch after 2 and 4 seconds, so a push with many direct commits waits 6 seconds in total rather than per commit. The pagination cap and an incomplete compare range still fail the run. Docs now match: a direct push skips with a warning, and PRs merged in the same push still run. References DEVOPS-1803 --- .github/actions/resolve-pr-event/README.md | 2 +- .../actions/resolve-pr-event/src/resolve.py | 60 ++++++++++++++----- .../resolve-pr-event/test/test_resolve.py | 54 +++++++++++++++-- docs/workflows/backport.md | 2 +- 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/.github/actions/resolve-pr-event/README.md b/.github/actions/resolve-pr-event/README.md index 3263f36c..cb0ecc0b 100644 --- a/.github/actions/resolve-pr-event/README.md +++ b/.github/actions/resolve-pr-event/README.md @@ -2,7 +2,7 @@ Resolve merged pull requests for a default-branch push or a completed read-only pull request signal workflow. The action fetches authoritative PR data from GitHub and only returns closed, merged PRs targeting the default branch. Fork PRs are included after merge. -For a push, the action walks every commit in the before/after range and finds associated PRs whose merge SHA is in that range. A forced push, deleted branch, or new branch with a zero before SHA is skipped. Comparison and associated PR pagination are bounded to 1,000 entries each; incomplete results fail the run. Empty commit associations are retried twice, then fail the run because a missing association can hide a backport. This also fails for direct pushes or commits with no associated PR; investigate those runs before retrying. +For a push, the action walks every commit in the before/after range and finds associated PRs whose merge SHA is in that range. A forced push, deleted branch, or new branch with a zero before SHA is skipped. Comparison and associated PR pagination are bounded to 1,000 entries each; incomplete results fail the run. Commits with no associated PR are retried twice as one batch, after 2 and 4 seconds, because GitHub can list a merge commit's PR shortly after the push. Commits still without a PR, such as direct or bot pushes, do not stop the run: the PRs found in the same push are still returned, and a warning lists those commits after the outputs are written. If a listed commit merged a PR, remove and re-add its `backport-to-` label to start the backport through the signal. For `workflow_run`, the action verifies the run ID, attempt, status, conclusion, source workflow identity, repository, event, head branch and SHA against the API. It uses PR associations from the run or, when empty, from the commit API. A re-fetched PR must match the signal head branch and either its head SHA or merge SHA; its merge commit must be an ancestor of the default branch. No signal artifact is read. Since a signal workflow's code can come from a PR, it must be read-only and carry no secrets. Its output is never treated as authoritative. diff --git a/.github/actions/resolve-pr-event/src/resolve.py b/.github/actions/resolve-pr-event/src/resolve.py index 3db4723b..38d182b6 100644 --- a/.github/actions/resolve-pr-event/src/resolve.py +++ b/.github/actions/resolve-pr-event/src/resolve.py @@ -13,6 +13,8 @@ MAX_BYTES = 4 * 1024 * 1024 SHA = re.compile(r'^[0-9a-f]{40}$') +# Seconds to wait before each retry of the commits that had no PR association. +RETRY_DELAYS = (2, 4) class Reject(Exception): pass @@ -101,7 +103,38 @@ def merged_pr(pr, number, repo, default): pr.get('state') == 'closed' and sha(pr.get('merge_commit_sha')) and same((base.get('repo') or {}).get('full_name'), repo) and base.get('ref') == default) -def resolve(api, payload, event_name, source_workflow='', requested_number=None): +def associated_numbers(api, commits): + """Return PRs merged by pushed commits, and the commits GitHub lists no PR for. + + GitHub can list a merge commit's PR a few seconds after the push. Empty + lookups are retried as one batch, so the wait does not grow with the number + of direct commits in a push. + """ + numbers, pending = set(), list(commits) + for delay in (0,) + RETRY_DELAYS: + if delay: + time.sleep(delay) + missing = [] + for commit in pending: + associations = paged(api, '/commits/%s/pulls' % commit) + if not associations: + missing.append(commit) + for candidate in associations: + number = candidate.get('number') if isinstance(candidate, dict) else None + if positive(number) and candidate.get('merge_commit_sha') == commit: + numbers.add(number) + pending = missing + if not pending: + break + return numbers, pending + +def unassociated_warning(commits): + shown = ', '.join(commits[:10]) + (', and %d more' % (len(commits) - 10) if len(commits) > 10 else '') + return ('No pull request is associated with %d pushed commit(s) after retries: %s. A direct push needs no backport. ' + 'If one of these commits merged a pull request, remove and re-add its backport-to- label to start the backport.' + % (len(commits), shown)) + +def resolve(api, payload, event_name, source_workflow='', requested_number=None, unassociated=None): if not isinstance(payload, dict): raise Reject('invalid event') repo, default, ref = trusted_context(api, payload, event_name) @@ -143,19 +176,13 @@ def resolve(api, payload, event_name, source_workflow='', requested_number=None) # Push restoration rechecks one selected PR and its default-branch ancestry. # Only selection needs to discover PRs across the full pushed commit range. numbers = {requested_number} if event_name == 'push' and requested_number is not None else set() - for commit in commits: - for attempt in range(3): - associations = paged(api, '/commits/%s/pulls' % commit) - if associations: - break - if attempt < 2: - time.sleep(1) - if not associations: - raise RuntimeError('no pull request association for commit %s after retries' % commit) - for candidate in associations: - number = candidate.get('number') if isinstance(candidate, dict) else None - if positive(number) and candidate.get('merge_commit_sha') == commit: - numbers.add(number) + if commits: + # A commit with no PR, such as a direct or bot push, must not drop the + # PRs merged in the same push. The caller reports it after the outputs. + found, missing = associated_numbers(api, commits) + numbers |= found + if unassociated is not None: + unassociated.extend(missing) if event_name == 'workflow_run': for candidate in candidates: number = candidate.get('number') if isinstance(candidate, dict) else None @@ -246,9 +273,10 @@ def main(): number = requested.get('number') if isinstance(requested, dict) else None if not positive(number): raise Reject('invalid restored pull request number') + unassociated = [] events = resolve(API(os.environ['INPUT_GITHUB_TOKEN'], repository), payload, os.environ.get('GITHUB_EVENT_NAME', ''), os.environ.get('INPUT_SOURCE_WORKFLOW', ''), - requested_number=number) + requested_number=number, unassociated=unassociated) if restored: matches = [event for event in events if event['number'] == number] supplied_pr = requested.get('pull_request') if isinstance(requested, dict) else None @@ -262,6 +290,8 @@ def main(): raise Reject('restored event does not match authoritative event') events = matches write_outputs(events) + if unassociated: + print('::warning::' + unassociated_warning(unassociated)) except Reject as exc: print('::warning::' + str(exc)) write_output_values({'eligible': 'false', 'events-json': '[]'}) diff --git a/.github/actions/resolve-pr-event/test/test_resolve.py b/.github/actions/resolve-pr-event/test/test_resolve.py index a46d5357..aa3abebe 100644 --- a/.github/actions/resolve-pr-event/test/test_resolve.py +++ b/.github/actions/resolve-pr-event/test/test_resolve.py @@ -306,18 +306,62 @@ def get(self, path): with pytest.raises(RuntimeError): r.resolve(ManyPaged(), push(), 'push') -def test_empty_commit_associations_retry_then_fail(monkeypatch): +class DirectCommitAPI(FakeAPI): + """Commit B is a direct push with no PR; commit C merges PR 2.""" + def get(self, path): + if path.startswith('/commits/%s/' % B): + self.calls.append(path) + return [] + return super().get(path) + +def test_unassociated_commits_retry_as_one_batch_then_skip(monkeypatch): class EmptyAPI(FakeAPI): def get(self, path): if path.startswith('/commits/'): self.calls.append(path) return [] return super().get(path) + sleeps = [] + monkeypatch.setattr(r.time, 'sleep', sleeps.append) + api, unassociated = EmptyAPI(), [] + assert r.resolve(api, push(), 'push', unassociated=unassociated) == [] + assert unassociated == [B, C] + assert len([c for c in api.calls if c.startswith('/commits/')]) == 6 + # One wait per retry round, not per commit. + assert sleeps == list(r.RETRY_DELAYS) + +def test_direct_commit_keeps_pr_merged_in_same_push(monkeypatch): + monkeypatch.setattr(r.time, 'sleep', lambda _: None) + api, unassociated = DirectCommitAPI(), [] + assert [e['number'] for e in r.resolve(api, push(), 'push', unassociated=unassociated)] == [2] + assert unassociated == [B] + # Only the unassociated commit is retried. + assert len([c for c in api.calls if c.startswith('/commits/%s/' % C)]) == 1 + +def test_direct_commit_push_writes_outputs_then_warns(tmp_path, monkeypatch, capsys): + event_file = tmp_path / 'payload.json'; event_file.write_text(json.dumps(push())) + monkeypatch.setenv('GITHUB_EVENT_PATH', str(event_file)) + monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'out')) + monkeypatch.setenv('RUNNER_TEMP', str(tmp_path)) + monkeypatch.setenv('GITHUB_REPOSITORY', 'org/repo') + monkeypatch.setenv('INPUT_GITHUB_TOKEN', 'unused') + monkeypatch.setenv('GITHUB_EVENT_NAME', 'push') + monkeypatch.delenv('INPUT_EVENT_JSON', raising=False) monkeypatch.setattr(r.time, 'sleep', lambda _: None) - api = EmptyAPI() - with pytest.raises(RuntimeError, match='no pull request association'): - r.resolve(api, push(), 'push') - assert len([c for c in api.calls if c.startswith('/commits/')]) == 3 + monkeypatch.setattr(r, 'API', lambda *args: DirectCommitAPI()) + assert r.main() == 0 + output = (tmp_path / 'out').read_text() + events = json.loads(output.split('events-json<<', 1)[1].split('\n')[1]) + assert [e['number'] for e in events] == [2] + assert 'eligible<<' in output and '\ntrue\n' in output + out = capsys.readouterr().out + assert out.startswith('::warning::') and B in out and C not in out and 'backport-to-' in out + +def test_unassociated_warning_bounds_listed_commits(): + commits = ['%040x' % n for n in range(1, 13)] + message = r.unassociated_warning(commits) + assert '12 pushed commit(s)' in message and commits[9] in message and commits[10] not in message + assert 'and 2 more' in message and '\n' not in message def test_commit_association_retry_succeeds(monkeypatch): diff --git a/docs/workflows/backport.md b/docs/workflows/backport.md index 85eb2042..58a2af1f 100644 --- a/docs/workflows/backport.md +++ b/docs/workflows/backport.md @@ -28,7 +28,7 @@ push range again. | Stage | Trigger and reads | Mutations | Output and skip remedy | |---|---|---|---| -| Select | Default-branch push; all commits in its range and associated PRs from GitHub | None | One event per merged PR. A direct push skips; a later backport label signals another run. | +| Select | Default-branch push; all commits in its range and associated PRs from GitHub | None | One event per merged PR. A commit with no PR, such as a direct push, is skipped with a warning that lists it; PRs merged in the same push still run. If a listed commit merged a PR, re-add its backport label to signal another run. | | Resolve | Trusted default-branch caller; PR identity, merge and current labels from GitHub | None | Canonical merged PR event. Wrong branch or unmerged PR skips; merge first and run from the default branch. | | Backport | Canonical event with current backport labels | Existing branch, PR and comment writes using the caller PAT | Existing cherry-pick logs and per-target failure comments. Remove an unwanted label or fix the reported conflict. | | Link | Both modern and legacy producers have finished | Existing advisory Linear and PR links | Existing advisory output; missing Linear token skips linking. |