fix(ci): exempt issue-gate writers via repo permission (#1081)
author_association on the webhook is CONTRIBUTOR when org membership is private, so maintainers with write (e.g. ajspig) were labelled needs-approved-issue. Skip on admin/maintain/write from getCollaboratorPermissionLevel instead; 404 stays gated.
This commit is contained in:
parent
d04f622317
commit
370232e139
|
|
@ -33,22 +33,36 @@ const isBot = (account) => Boolean(account) && account.type === 'Bot';
|
|||
* Why this pull request is exempt from the gate, or null if it is not.
|
||||
*
|
||||
* Single source of truth: every caller that acts on a pull request runs this.
|
||||
* The stale-draft sweep previously re-listed these checks and silently lost the
|
||||
* bot case.
|
||||
*/
|
||||
const exemptReason = (pr) => {
|
||||
const exemptReason = async ({ github, owner, repo, pr }) => {
|
||||
if (isBot(pr.user)) return 'author is a bot';
|
||||
if (WRITE_ACCESS.includes(pr.author_association)) {
|
||||
return `author_association is ${pr.author_association}`;
|
||||
}
|
||||
if (hasLabel(pr, EXEMPT_LABEL)) return `carries the ${EXEMPT_LABEL} label`;
|
||||
|
||||
const username = pr.user && pr.user.login;
|
||||
if (!username) return null;
|
||||
|
||||
const permission = await repoPermission({ github, owner, repo, username });
|
||||
if (WRITE_PERMISSIONS.includes(permission)) {
|
||||
return `author has ${permission} permission`;
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
// Write access to the repository. CONTRIBUTOR is deliberately absent: GitHub uses
|
||||
// it for "has previously committed to the repository", which describes every
|
||||
// returning outside contributor, not a maintainer. Do not add it.
|
||||
const WRITE_ACCESS = ['OWNER', 'MEMBER', 'COLLABORATOR'];
|
||||
// Repo roles that skip the gate. `read` / `triage` do not.
|
||||
const WRITE_PERMISSIONS = ['admin', 'maintain', 'write'];
|
||||
|
||||
/** Highest repo permission for `username`, or null if they are not a collaborator. */
|
||||
async function repoPermission({ github, owner, repo, username }) {
|
||||
try {
|
||||
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
|
||||
owner, repo, username,
|
||||
});
|
||||
return data.permission;
|
||||
} catch (err) {
|
||||
if (err && err.status === 404) return null;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
const CLOSING_ISSUES = `
|
||||
query($owner: String!, $repo: String!, $number: Int!) {
|
||||
|
|
@ -78,7 +92,7 @@ const CLOSING_ISSUES = `
|
|||
async function checkGate({ github, owner, repo, pr }) {
|
||||
if (pr.state !== 'open') return { passed: true, skipped: 'pull request is not open' };
|
||||
if (pr.draft) return { passed: true, skipped: 'pull request is a draft' };
|
||||
const exempt = exemptReason(pr);
|
||||
const exempt = await exemptReason({ github, owner, repo, pr });
|
||||
if (exempt) return { passed: true, skipped: exempt };
|
||||
|
||||
const data = await github.graphql(CLOSING_ISSUES, { owner, repo, number: pr.number });
|
||||
|
|
@ -251,7 +265,7 @@ async function runSweep({ github, core, context, dryRun }) {
|
|||
// Stale drafts. The gate skips drafts entirely, so they never carry the label;
|
||||
// this pass keys off inactivity and applies the shared exemptions itself.
|
||||
for (const pr of prs.filter((p) => p.draft)) {
|
||||
const exempt = exemptReason(pr);
|
||||
const exempt = await exemptReason({ github, owner, repo, pr });
|
||||
if (exempt) {
|
||||
core.info(`#${pr.number}: leaving stale draft alone — ${exempt}`);
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -12,12 +12,18 @@ const {
|
|||
|
||||
const pull = (over = {}) => ({
|
||||
number: 1, state: 'open', draft: false,
|
||||
user: { type: 'User' }, author_association: 'NONE', labels: [],
|
||||
user: { type: 'User', login: 'alice' }, labels: [],
|
||||
...over,
|
||||
});
|
||||
|
||||
const notCollaborator = () => {
|
||||
const err = new Error('Not Found');
|
||||
err.status = 404;
|
||||
throw err;
|
||||
};
|
||||
|
||||
// `linked` is the list of issues GitHub resolves as closing references.
|
||||
const stub = (linked) => ({
|
||||
const stub = (linked, permission) => ({
|
||||
graphql: async () => ({
|
||||
repository: { pullRequest: { closingIssuesReferences: {
|
||||
nodes: linked.map((i) => ({
|
||||
|
|
@ -26,10 +32,18 @@ const stub = (linked) => ({
|
|||
})),
|
||||
} } },
|
||||
}),
|
||||
rest: {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
if (!permission) return notCollaborator();
|
||||
return { data: { permission } };
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const run = (linked, over) =>
|
||||
checkGate({ github: stub(linked), owner: 'o', repo: 'r', pr: pull(over) });
|
||||
const run = (linked, over, permission) =>
|
||||
checkGate({ github: stub(linked, permission), owner: 'o', repo: 'r', pr: pull(over) });
|
||||
|
||||
const cases = [
|
||||
['no linked issue fails', () => run([]), (r) => r.passed === false],
|
||||
|
|
@ -45,17 +59,19 @@ const cases = [
|
|||
(r) => r.passed === true && r.issue === 8],
|
||||
|
||||
// Exemptions.
|
||||
['maintainer skips', () => run([], { author_association: 'MEMBER' }), (r) => r.passed === true],
|
||||
['collaborator skips', () => run([], { author_association: 'COLLABORATOR' }), (r) => r.passed === true],
|
||||
['write permission skips', () => run([], {}, 'write'), (r) => r.passed === true],
|
||||
['maintain permission skips', () => run([], {}, 'maintain'), (r) => r.passed === true],
|
||||
['bot skips', () => run([], { user: { type: 'Bot' } }), (r) => r.passed === true],
|
||||
['draft skips', () => run([], { draft: true }), (r) => r.passed === true],
|
||||
[`${EXEMPT_LABEL} skips`, () => run([], { labels: [{ name: EXEMPT_LABEL }] }), (r) => r.passed === true],
|
||||
|
||||
// Regression guard: GitHub hands CONTRIBUTOR to anyone who has previously
|
||||
// committed, i.e. every returning outside contributor. It must stay gated.
|
||||
['CONTRIBUTOR is still gated',
|
||||
() => run([], { author_association: 'CONTRIBUTOR' }),
|
||||
['triage permission is still gated', () => run([], {}, 'triage'), (r) => r.passed === false],
|
||||
['MEMBER association without write is still gated',
|
||||
() => run([], { author_association: 'MEMBER' }),
|
||||
(r) => r.passed === false],
|
||||
['CONTRIBUTOR with write skips',
|
||||
() => run([], { author_association: 'CONTRIBUTOR' }, 'write'),
|
||||
(r) => r.passed === true],
|
||||
];
|
||||
|
||||
// --- findNotices: only the bot's own notices count -------------------------
|
||||
|
|
@ -81,12 +97,12 @@ const noticeCases = [
|
|||
// --- runSweep: the stale-draft pass must honour every exemption ------------
|
||||
const draft = (over) => ({
|
||||
number: 9, draft: true, state: 'open', labels: [],
|
||||
user: { type: 'User' }, author_association: 'NONE',
|
||||
user: { type: 'User', login: 'alice' },
|
||||
updated_at: new Date(Date.now() - 400 * 86400_000).toISOString(),
|
||||
...over,
|
||||
});
|
||||
|
||||
async function sweepClosed(pr) {
|
||||
async function sweepClosed(pr, permission) {
|
||||
const closed = [];
|
||||
const github = {
|
||||
paginate: async (route) => (route === 'pulls' ? [pr] : []),
|
||||
|
|
@ -96,6 +112,12 @@ async function sweepClosed(pr) {
|
|||
update: async ({ pull_number }) => closed.push(pull_number),
|
||||
},
|
||||
issues: { listComments: 'comments', createComment: async () => {} },
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
if (!permission) return notCollaborator();
|
||||
return { data: { permission } };
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
await runSweep({
|
||||
|
|
@ -108,7 +130,7 @@ async function sweepClosed(pr) {
|
|||
const sweepCases = [
|
||||
['stale draft from an outside author closes', draft({}), 1],
|
||||
['stale draft from a bot is left alone', draft({ user: { type: 'Bot' } }), 0],
|
||||
['stale draft from a maintainer is left alone', draft({ author_association: 'MEMBER' }), 0],
|
||||
['stale draft from a writer is left alone', draft({}), 0, 'write'],
|
||||
[`stale draft with ${EXEMPT_LABEL} is left alone`, draft({ labels: [{ name: EXEMPT_LABEL }] }), 0],
|
||||
['recent draft is left alone', draft({ updated_at: new Date().toISOString() }), 0],
|
||||
];
|
||||
|
|
@ -120,8 +142,8 @@ const sweepCases = [
|
|||
if (got === want) console.log(` ok ${name}`);
|
||||
else { failed++; console.log(` FAIL ${name} -> ${got} notices, wanted ${want}`); }
|
||||
}
|
||||
for (const [name, pr, want] of sweepCases) {
|
||||
const got = (await sweepClosed(pr)).length;
|
||||
for (const [name, pr, want, permission] of sweepCases) {
|
||||
const got = (await sweepClosed(pr, permission)).length;
|
||||
if (got === want) console.log(` ok ${name}`);
|
||||
else { failed++; console.log(` FAIL ${name} -> closed ${got}, wanted ${want}`); }
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue