github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-17 05:29:44 +02:00
Code Issues Actions 14 Packages Projects Releases Wiki Activity Graph

ci: Enforce OWNERS required reviews on every base branch and fork PR (no-changelog) (#38713)

Browse Source 
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Charlie Kolb 2026-09-18 09:07:10 +00:00 committed by GitHub
parent 1b39c1d57d
commit 399da28fda
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 378 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
.github/WORKFLOWS.md vendored
View File

@ -889,14 +889,42 @@ PR changes and review events, and reports a commit status
named **Required Reviews** on the head SHA. A missing approval reports
`pending` ("Waiting for approval from: …"), not `failure`, so an unreviewed PR
does not show red CI; any non-success state blocks the merge equally. The
ruleset for `master` must list
that status as a required check for the block to take effect. Merge-queue runs
report success on the queue head without re-evaluating: a PR cannot enter the
queue unless the status is green on its head, and the queue does not change
approvals.
ruleset for a branch must list that status as a required check for the block
to take effect. Merge-queue runs report success on the queue head without
re-evaluating: a PR cannot enter the queue unless the status is green on its
head, and the queue does not change approvals.
The workflow reads OWNERS and its scripts from the base branch only, so a PR
cannot lift its own review requirement.
The status is evaluated for a PR into any base branch, from a same-repo head
or a fork. Both matter because the ruleset that gates a PR is not always the
one on its base branch: GitHub applies the ruleset of a stack's target branch
to every PR in the stack, so a stacked PR into a feature branch is gated by
the `master` ruleset. A required status that no run ever writes leaves the PR
blocked on "Expected". Routes that skip the evaluation are listed in
`REQUIRED_REVIEW_EXEMPTIONS` in
`required-reviews.mjs`. An entry is `<head> -> <base>` or just `<base>`
(any head); `*` matches any run of characters. An exempt PR reports
`success` with the route in the description. Only heads in this repository
can match, so a fork branch with a matching name is still evaluated. Add a
route only when every commit it carries was already reviewed elsewhere, as
with `sync/master-to-3x -> 3.x`: its commits landed on `master` first.
The workflow reads OWNERS, its scripts and the exemption routes from `master`
only, never from the base branch or the PR: any writable branch can be a base,
so only `master` is trusted input. A PR cannot lift its own review
requirement. A retarget re-evaluates the PR, so a verdict computed against the
old base does not carry over.
Every path that writes the status runs in the base repository context, because
a fork-context run has no secrets and a read-only token. PR changes arrive
through `pull_request_target`, which is safe here because no step checks out
or runs PR code. Review events on a same-repo PR arrive through
`pull_request_review`. Review events on a fork PR arrive through the
`workflow_run` of `ci-pull-request-review.yml`, which runs on every submitted
or dismissed review; the owners workflow looks the PR up from that run's head
and skips same-repo heads, which the direct event already covers. A first
contribution whose runs still wait for approval gets no review-event
re-evaluation until a maintainer approves the runs; `workflow_dispatch` with
the PR number is the manual fallback.
### Transition from CODEOWNERS

20
.github/scripts/github-helpers.mjs vendored
View File

@ -414,6 +414,26 @@ export async function getPullRequestById(pullRequestId) {
return pullRequest.data;
}
/**
* Open PRs whose head is `headOwner:headBranch`, newest first.
*
* @param { string } headOwner Owner of the head repository (a fork owner or this org).
* @param { string } headBranch
* @returns { Promise<any[]> }
*/
export async function listOpenPullRequestsByHead(headOwner, headBranch) {
const { octokit, owner, repo } = initGithub();
const pullRequests = await octokit.rest.pulls.list({
owner,
repo,
state: 'open',
head: `${headOwner}:${headBranch}`,
});
return pullRequests.data;
}
/**
* Returns the set of files changed in a PR, including previous filenames for renames.
*

96
.github/scripts/owners/required-reviews.mjs vendored
View File

@ -8,6 +8,9 @@
* runs do not reach this script: the workflow reports success on the queue
* head directly, because entering the queue already required a green status.
*
* Every base branch is enforced unless the PR matches a route in
* REQUIRED_REVIEW_EXEMPTIONS; an exempt PR reports success without evaluation.
*
* The job itself always succeeds when the evaluation runs; the commit status
* carries the verdict. The status is set to pending before the evaluation
* starts, so a crash (e.g. the team membership API is unavailable) cannot
@ -20,12 +23,30 @@ import {
getPrReviews,
getPullRequestById,
isTeamMember,
listOpenPullRequestsByHead,
setCommitStatus,
} from '../github-helpers.mjs';
import { parseOwnersFile, resolveRequiredTeams, teamHandleToSlug } from './owners.mjs';
export const STATUS_CONTEXT = 'Required Reviews';
const TARGET_BRANCH = 'master';
/**
* PR routes that skip the evaluation. Each entry is `<head> -> <base>` or
* just `<base>` (any head). `*` matches any run of characters, including
* `/`. Only heads in this repository qualify; a fork branch with a
* matching name does not.
*
* Add a route here only when every commit it carries was already reviewed
* elsewhere, like the master-to-3.x sync, whose commits landed on master.
*/
export const REQUIRED_REVIEW_EXEMPTIONS = ['sync/master-to-3x -> 3.x'];
/**
* @typedef ExemptRoute
* @property { string } head Glob for the head branch.
* @property { string } base Glob for the base branch.
* @property { string } source The entry as written.
*/
/**
* A PR review as returned by the reviews API.
@ -36,17 +57,68 @@ const TARGET_BRANCH = 'master';
* @property { string } [submitted_at]
*/
/**
* @param { string } entry `<head> -> <base>` or `<base>`
* @returns { ExemptRoute }
*/
export function parseExemption(entry) {
const parts = entry.split('->').map((part) => part.trim());
if (parts.length > 2 || parts.some((part) => part === '')) {
throw new Error(`Invalid exemption "${entry}": expected "<head> -> <base>" or "<base>"`);
}
const [head, base] = parts.length === 2 ? parts : ['*', parts[0]];
return { head, base, source: entry };
}
/**
* @param { string } ref Branch name.
* @param { string } pattern Glob where `*` matches any run of characters.
* @returns { boolean }
*/
export function matchesBranchPattern(ref, pattern) {
const escaped = pattern.split('*').map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'));
return new RegExp(`^${escaped.join('.*')}$`).test(ref);
}
/**
* @param { { head: { ref: string, repo: { full_name: string } | null }, base: { ref: string, repo: { full_name: string } } } } pullRequest
* @param { string[] } [exemptions]
* @returns { ExemptRoute | undefined } The first matching route.
*/
export function findExemption(pullRequest, exemptions = REQUIRED_REVIEW_EXEMPTIONS) {
// A fork can name its branch anything, so the head must live in this repo.
if (pullRequest.head.repo?.full_name !== pullRequest.base.repo.full_name) return undefined;
return exemptions
.map(parseExemption)
.find(
(route) =>
matchesBranchPattern(pullRequest.head.ref, route.head) &&
matchesBranchPattern(pullRequest.base.ref, route.base),
);
}
/**
* Resolve the PR number from the triggering event.
*
* A workflow_run event carries no PR: the PR is looked up from the
* triggering run's head. Only an open PR whose head SHA is the run's head
* SHA counts; several PRs can share a head branch.
*
* @param { string } eventName
* @param { any } event Parsed GITHUB_EVENT_PATH payload.
* @param { string | undefined } pullRequestNumberEnv PULL_REQUEST_NUMBER (workflow_dispatch input).
* @returns { number }
* @returns { Promise<number | undefined> } undefined when the event has no open PR to evaluate.
*/
export function resolvePullRequestNumber(eventName, event, pullRequestNumberEnv) {
export async function resolvePullRequestNumber(eventName, event, pullRequestNumberEnv) {
if (event?.pull_request?.number) return event.pull_request.number;
if (eventName === 'workflow_run' && event?.workflow_run) {
const { head_sha: sha, head_branch: branch, head_repository: headRepository } = event.workflow_run;
const candidates = await listOpenPullRequestsByHead(headRepository.owner.login, branch);
return candidates.find((pullRequest) => pullRequest.head.sha === sha)?.number;
}
const parsed = parseInt(pullRequestNumberEnv ?? '');
if (Number.isNaN(parsed)) {
throw new Error(`Cannot resolve a PR number for event "${eventName}"`);
@ -160,18 +232,16 @@ export async function run() {
const eventName = process.env.GITHUB_EVENT_NAME ?? 'workflow_dispatch';
const event = getEventFromGithubEventPath();
const pullRequestNumber = resolvePullRequestNumber(
const pullRequestNumber = await resolvePullRequestNumber(
eventName,
event,
process.env.PULL_REQUEST_NUMBER,
);
const pullRequest = await getPullRequestById(pullRequestNumber);
if (pullRequest.base.ref !== TARGET_BRANCH) {
console.log(`PR #${pullRequestNumber} targets "${pullRequest.base.ref}", not "${TARGET_BRANCH}"; skipping.`);
if (pullRequestNumber === undefined) {
console.log(`No open PR for the ${eventName} head; nothing to evaluate.`);
return;
}
const pullRequest = await getPullRequestById(pullRequestNumber);
const statusSha = pullRequest.head.sha;
const targetUrl = statusTargetUrl();
@ -187,7 +257,13 @@ export async function run() {
/** @type { { state: 'success' | 'pending', description: string } } */
let status;
try {
status = await evaluateRequiredReviews(pullRequestNumber);
const exemption = findExemption(pullRequest);
if (exemption) {
console.log(`PR #${pullRequestNumber} matches exempt route "${exemption.source}"; skipping the evaluation.`);
status = { state: 'success', description: `Exempt route: ${exemption.source}` };
} else {
status = await evaluateRequiredReviews(pullRequestNumber);
}
} catch (evaluationError) {
// Best effort: nicer than a stuck pending status. The pending status
// already blocks the merge if this write fails too.

187
.github/scripts/owners/required-reviews.test.mjs vendored
View File

@ -19,6 +19,8 @@ let getPrReviewsImpl = async () => [];
let isTeamMemberImpl = async () => false;
/** @type {(sha: string, status: any) => Promise<void>} */
let setCommitStatusImpl = async () => {};
/** @type {(headOwner: string, headBranch: string) => Promise<any[]>} */
let listOpenPullRequestsByHeadImpl = async () => [];
mock.module('../github-helpers.mjs', {
namedExports: {
@ -28,6 +30,8 @@ mock.module('../github-helpers.mjs', {
getPrReviews: (n) => getPrReviewsImpl(n),
isTeamMember: (slug, username) => isTeamMemberImpl(slug, username),
setCommitStatus: (sha, status) => setCommitStatusImpl(sha, status),
listOpenPullRequestsByHead: (headOwner, headBranch) =>
listOpenPullRequestsByHeadImpl(headOwner, headBranch),
},
});
@ -46,28 +50,150 @@ mock.module('./owners.mjs', {
});
const {
REQUIRED_REVIEW_EXEMPTIONS,
STATUS_CONTEXT,
buildStatus,
collectApprovers,
findExemption,
latestReviewStates,
matchesBranchPattern,
parseExemption,
resolvePullRequestNumber,
run,
} = await import('./required-reviews.mjs');
const REPO = 'n8n-io/n8n';
/** @param {string} head @param {string} base @param {string} [headRepo] */
function pullRequestFor(head, base, headRepo = REPO) {
return {
head: { ref: head, sha: 'head-sha', repo: { full_name: headRepo } },
base: { ref: base, repo: { full_name: REPO } },
};
}
describe('parseExemption', () => {
it('splits a "<head> -> <base>" route', () => {
assert.deepEqual(parseExemption('sync/master-to-3x -> 3.x'), {
head: 'sync/master-to-3x',
base: '3.x',
source: 'sync/master-to-3x -> 3.x',
});
});
it('treats a bare "<base>" as any head', () => {
assert.deepEqual(parseExemption('release/*'), { head: '*', base: 'release/*', source: 'release/*' });
});
it('rejects malformed routes', () => {
assert.throws(() => parseExemption('a -> b -> c'), /Invalid exemption/);
assert.throws(() => parseExemption(' -> 3.x'), /Invalid exemption/);
assert.throws(() => parseExemption('3.x -> '), /Invalid exemption/);
});
it('accepts every configured exemption', () => {
for (const entry of REQUIRED_REVIEW_EXEMPTIONS) parseExemption(entry);
});
});
describe('matchesBranchPattern', () => {
it('matches whole branch names', () => {
assert.equal(matchesBranchPattern('3.x', '3.x'), true);
assert.equal(matchesBranchPattern('3.xy', '3.x'), false);
assert.equal(matchesBranchPattern('v3.x', '3.x'), false);
});
it('treats "." as a literal', () => {
assert.equal(matchesBranchPattern('3ax', '3.x'), false);
});
it('expands "*" across path separators', () => {
assert.equal(matchesBranchPattern('release/2.39.5', 'release/*'), true);
assert.equal(matchesBranchPattern('release/2.39.5/hotfix', 'release/*'), true);
assert.equal(matchesBranchPattern('anything', '*'), true);
assert.equal(matchesBranchPattern('release-candidate/2.40.0', 'release/*'), false);
});
});
describe('findExemption', () => {
const exemptions = ['sync/master-to-3x -> 3.x', 'release/*'];
it('finds the route for a matching head and base', () => {
const route = findExemption(pullRequestFor('sync/master-to-3x', '3.x'), exemptions);
assert.equal(route?.source, 'sync/master-to-3x -> 3.x');
});
it('returns nothing when only the base matches', () => {
assert.equal(findExemption(pullRequestFor('feature/x', '3.x'), exemptions), undefined);
});
it('returns nothing when only the head matches', () => {
assert.equal(findExemption(pullRequestFor('sync/master-to-3x', 'master'), exemptions), undefined);
});
it('matches a bare base route for any head', () => {
const route = findExemption(pullRequestFor('feature/x', 'release/2.39.5'), exemptions);
assert.equal(route?.source, 'release/*');
});
it('ignores heads from another repository', () => {
const fromFork = pullRequestFor('sync/master-to-3x', '3.x', 'someone/n8n');
assert.equal(findExemption(fromFork, exemptions), undefined);
});
it('ignores heads whose repository is gone', () => {
const pullRequest = pullRequestFor('sync/master-to-3x', '3.x');
pullRequest.head.repo = null;
assert.equal(findExemption(pullRequest, exemptions), undefined);
});
});
describe('resolvePullRequestNumber', () => {
it('reads the PR number from pull_request payloads', () => {
/** A workflow_run payload for a run on `owner/branch` at `sha`. */
function workflowRunEvent(owner, branch, sha) {
return { workflow_run: { head_sha: sha, head_branch: branch, head_repository: { owner: { login: owner } } } };
}
beforeEach(() => {
listOpenPullRequestsByHeadImpl = async () => [];
});
it('reads the PR number from pull_request payloads', async () => {
assert.equal(
resolvePullRequestNumber('pull_request', { pull_request: { number: 42 } }, undefined),
await resolvePullRequestNumber('pull_request_target', { pull_request: { number: 42 } }, undefined),
42,
);
});
it('falls back to the PULL_REQUEST_NUMBER env value for workflow_dispatch', () => {
assert.equal(resolvePullRequestNumber('workflow_dispatch', {}, '7'), 7);
it('falls back to the PULL_REQUEST_NUMBER env value for workflow_dispatch', async () => {
assert.equal(await resolvePullRequestNumber('workflow_dispatch', {}, '7'), 7);
});
it('throws when no source yields a PR number', () => {
assert.throws(() => resolvePullRequestNumber('workflow_dispatch', {}, undefined), /Cannot resolve/);
it('rejects when no source yields a PR number', async () => {
await assert.rejects(resolvePullRequestNumber('workflow_dispatch', {}, undefined), /Cannot resolve/);
});
it('looks up the open PR for a workflow_run head', async () => {
const listOpenPullRequestsByHead = mock.fn(async () => [
{ number: 11, head: { sha: 'old-sha' } },
{ number: 12, head: { sha: 'run-sha' } },
]);
listOpenPullRequestsByHeadImpl = listOpenPullRequestsByHead;
assert.equal(
await resolvePullRequestNumber('workflow_run', workflowRunEvent('forker', 'fix/x', 'run-sha'), undefined),
12,
);
assert.deepEqual(listOpenPullRequestsByHead.mock.calls[0].arguments, ['forker', 'fix/x']);
});
it('returns undefined when no open PR has the workflow_run head SHA', async () => {
listOpenPullRequestsByHeadImpl = async () => [{ number: 11, head: { sha: 'old-sha' } }];
assert.equal(
await resolvePullRequestNumber('workflow_run', workflowRunEvent('forker', 'fix/x', 'run-sha'), undefined),
undefined,
);
});
});
@ -152,8 +278,7 @@ describe('run', () => {
eventImpl = () => ({ pull_request: { number: 42 } });
getPullRequestByIdImpl = async () => ({
base: { ref: 'master' },
head: { sha: 'head-sha' },
...pullRequestFor('feature/x', 'master'),
user: { login: 'author' },
});
getChangedFilesImpl = async () => new Set(['a.ts']);
@ -267,15 +392,53 @@ describe('run', () => {
assert.equal(status.state, 'pending');
});
it('skips PRs that do not target master without setting a status', async () => {
getPullRequestByIdImpl = async () => ({
base: { ref: 'release-candidate/2.9.x' },
head: { sha: 'head-sha' },
it('writes no status for a workflow_run head without an open PR', async () => {
process.env.GITHUB_EVENT_NAME = 'workflow_run';
eventImpl = () => ({
workflow_run: { head_sha: 'gone-sha', head_branch: 'fix/x', head_repository: { owner: { login: 'forker' } } },
});
listOpenPullRequestsByHeadImpl = async () => [];
await run();
assert.equal(setCommitStatus.mock.calls.length, 0);
});
it('evaluates PRs into branches other than master', async () => {
getPullRequestByIdImpl = async () => pullRequestFor('feature/x', 'release-candidate/2.9.x');
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
await run();
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
assert.equal(status.state, 'pending');
assert.match(status.description, /Waiting for approval from: qa-dx/);
});
it('reports success without evaluating a PR on an exempt route', async () => {
getPullRequestByIdImpl = async () => pullRequestFor('sync/master-to-3x', '3.x');
const getChangedFiles = mock.fn(async () => new Set(['a.ts']));
getChangedFilesImpl = getChangedFiles;
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
await run();
assert.equal(getChangedFiles.mock.calls.length, 0);
assert.equal(setCommitStatus.mock.calls.length, 2);
assert.equal(setCommitStatus.mock.calls[0].arguments[1].state, 'pending');
const [, status] = setCommitStatus.mock.calls[1].arguments;
assert.equal(status.state, 'success');
assert.equal(status.description, 'Exempt route: sync/master-to-3x -> 3.x');
});
it('does not exempt a fork branch that is named like an exempt route', async () => {
getPullRequestByIdImpl = async () => pullRequestFor('sync/master-to-3x', '3.x', 'someone/n8n');
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
await run();
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
assert.equal(status.state, 'pending');
});
});

114
.github/workflows/ci-owners-required-reviews.yml vendored
View File

@ -8,25 +8,44 @@ run-name: "${{ github.event_name == 'workflow_dispatch' && format('CI: Owners Re
# Replaces the GitHub-native CODEOWNERS enforcement, which could not express
# team ownership metadata beyond "request a review".
#
# Every PR is evaluated, whatever its base branch and whether or not its head
# is a fork. Two PR classes make this necessary: GitHub applies the ruleset of
# a stack's target branch to every PR in the stack, so a PR into a feature
# branch can be gated by the master ruleset; and a fork PR cannot write the
# status from a fork-context run. Every path that reports the status runs
# in the base repository context (secrets, write token). The routes that
# skip the evaluation (for example the master-to-3.x sync branch) live in
# REQUIRED_REVIEW_EXEMPTIONS in required-reviews.mjs; an exempt PR gets a
# success status.
#
# Triggers
# - pull_request (opened/synchronize/reopened/ready_for_review):
# re-evaluates whenever the changeset can change. Skipped for fork PRs
# (no secrets): their status stays "expected" — fail closed — until the
# first review event reports it.
# - pull_request_target (opened/synchronize/reopened/ready_for_review, and
# edited when the base branch changes): re-evaluates whenever the
# changeset can change, for same-repo and fork heads alike. A retargeted
# PR keeps its head SHA, so a verdict computed against the old base must
# not survive the retarget.
# - pull_request_review (submitted/dismissed): re-evaluates when approvals
# change. A privileged event (base repo context, secrets), so it covers
# fork PRs too — and approvals can only change through review events.
# change on a same-repo PR. A fork PR's review event runs without secrets
# and with a read-only token, so this path skips fork heads.
# Comment-only reviews are skipped: they cannot change approvals, and
# review bots submit one after every push.
# - workflow_run of "CI: Pull Request Review" (completed): the fork
# counterpart of pull_request_review. That workflow runs on every review
# event; its completion arrives here in the base repository context. The
# PR is looked up from the triggering run's head. Same-repo heads skip
# this path, which pull_request_review already covers.
# - merge_group: reports success on the queue head without re-evaluating.
# A PR cannot enter the queue unless this status is green on its head,
# and the queue does not change approvals.
# - workflow_dispatch: manual re-check (e.g. after a team membership change,
# or a fork PR push after an approval).
# - workflow_dispatch: manual re-check (e.g. after a team membership
# change).
#
# The checkout below always pins the base branch, never the PR merge ref: the
# scripts and the OWNERS file are trusted input, so a PR cannot lift its own
# review requirement or reach the status-writing token.
# The checkout below always pins master, never the base branch or the PR
# merge ref: the scripts, the OWNERS file and the exemption routes are trusted
# input, so a PR cannot lift its own review requirement or reach the
# status-writing token. Any branch can be a base, and anyone with push access
# can edit a branch, so only master is trusted. This is also what makes
# pull_request_target safe here: no step checks out or runs PR code.
#
# Output
# - A commit status named "Required Reviews" on the PR head SHA (or
@ -38,12 +57,13 @@ run-name: "${{ github.event_name == 'workflow_dispatch' && format('CI: Owners Re
# stale verdict in effect. The job itself only fails on errors.
on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
branches:
- master
pull_request_target:
types: [opened, synchronize, reopened, ready_for_review, edited]
pull_request_review:
types: [submitted, dismissed]
workflow_run:
workflows: ['CI: Pull Request Review']
types: [completed]
merge_group:
workflow_dispatch:
inputs:
@ -90,24 +110,29 @@ jobs:
required-reviews:
name: Check required reviews
# Fork PRs are skipped on pull_request (no secrets for the app token);
# pull_request_review covers them. pull_request_review has no `branches`
# filter; the script also skips non-master bases, the guard here keeps
# the early pending status off non-master heads.
# Of the edited events only a base change matters; title and body edits
# do not change the verdict. Review events split by head: same-repo
# heads come in through pull_request_review, fork heads through the
# workflow_run of the review workflow.
if: >-
github.event_name != 'merge_group' &&
(github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository) &&
(github.event_name != 'pull_request_target' ||
github.event.action != 'edited' || github.event.changes.base) &&
(github.event_name != 'pull_request_review' ||
(github.event.pull_request.base.ref == 'master' &&
github.event.review.state != 'commented'))
(github.event.pull_request.head.repo.full_name == github.repository &&
github.event.review.state != 'commented')) &&
(github.event_name != 'workflow_run' ||
(github.event.workflow_run.event == 'pull_request_review' &&
github.event.workflow_run.head_repository.full_name != github.repository))
runs-on: ubuntu-latest
timeout-minutes: 5
# Job-level, not workflow-level: a run whose job is skipped must not
# cancel an in-flight evaluation, or the status stays pending.
# cancel an in-flight evaluation, or the status stays pending. Keyed by
# head SHA, so the paths above share one group per PR head.
concurrency:
group: >-
owners-required-reviews-${{ github.event.pull_request.number
owners-required-reviews-${{ github.event.pull_request.head.sha
|| github.event.workflow_run.head_sha
|| inputs.pr_number
|| github.ref }}
cancel-in-progress: true
@ -135,7 +160,7 @@ jobs:
if: github.event_name != 'workflow_dispatch'
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
SHA: ${{ github.event.pull_request.head.sha }}
SHA: ${{ github.event.pull_request.head.sha || github.event.workflow_run.head_sha }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: |
gh api "repos/${GITHUB_REPOSITORY}/statuses/${SHA}" \
@ -144,44 +169,17 @@ jobs:
-f description='Evaluating required reviews' \
-f "target_url=${RUN_URL}"
# Always check out the base branch, never the PR merge ref: the OWNERS
# file and the scripts must be trusted input. The 'master' fallback
# covers workflow_dispatch, so a dispatch from another branch cannot
# run that branch's policy. The script reads only `.github/` and the
# root files, which cone mode always includes.
# Always check out master, never the base branch or the PR merge ref:
# the OWNERS file, the scripts and the exemption routes must be trusted
# input, and any writable branch can be a base. The script reads only
# `.github/` and the root files, which cone mode always includes.
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ github.event.pull_request.base.ref || 'master' }}
ref: master
persist-credentials: false
sparse-checkout: .github
# Bootstrap: the checkout is the trusted base branch. Until the action
# exists there, there is nothing to run. Report success so the PR that
# introduces it can merge; the check is enforced again from the next PR.
- name: Check for the enforcement action on the base branch
id: bootstrap
shell: bash
run: |
if [ -f .github/actions/run-workflow-script/action.yml ]; then
echo "ready=true" >> "$GITHUB_OUTPUT"
else
echo "ready=false" >> "$GITHUB_OUTPUT"
echo "No enforcement action on the base branch; nothing to enforce."
fi
- name: Report the bootstrap state
if: steps.bootstrap.outputs.ready != 'true' && github.event.pull_request.head.sha != ''
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
SHA: ${{ github.event.pull_request.head.sha }}
run: |
gh api "repos/${GITHUB_REPOSITORY}/statuses/${SHA}" \
-f state=success \
-f context='Required Reviews' \
-f description='Owners enforcement is not on the base branch yet'
- name: Evaluate required reviews
if: steps.bootstrap.outputs.ready == 'true'
uses: ./.github/actions/run-workflow-script
env:
PULL_REQUEST_NUMBER: ${{ inputs.pr_number || '' }}

5
.github/workflows/ci-pull-request-review.yml vendored
View File

@ -1,8 +1,11 @@
name: 'CI: Pull Request Review'
# "CI: Owners Required Reviews" listens to this workflow's completion
# (workflow_run) to re-evaluate fork PRs: a fork's review event runs without
# secrets, a workflow_run does not. Keep the trigger on every review event.
on:
pull_request_review:
types: [submitted]
types: [submitted, dismissed]
concurrency:
group: ci-review-${{ github.event.pull_request.number }}

3
packages/testing/code-health/src/index.ts
View File

@ -35,7 +35,8 @@ const defaultRuleSettings: RuleSettingsMap = {
'workflow-pr-target-safety': {
enabled: true,
severity: 'error',
options: { allowedWorkflows: ['ci-cla-check.yml'] },
// ci-owners-required-reviews.yml checks out master only, never PR code.
options: { allowedWorkflows: ['ci-cla-check.yml', 'ci-owners-required-reviews.yml'] },
},
'migration-timestamp': {
enabled: true,