mirror of
https://github.com/block/buzz.git
synced 2026-10-30 05:51:46 +00:00
fix: authorize team security reviews automatically
Signed-off-by: Jordan Mecom <jm@squareup.com>
This commit is contained in:
44
.github/scripts/codex-security-review.js
vendored
44
.github/scripts/codex-security-review.js
vendored
@@ -28,8 +28,8 @@ const completedMarker = (baseSha, headSha) =>
|
||||
|
||||
const reviewCommand = (headSha) => `${REVIEW_COMMAND} ${headSha}`;
|
||||
|
||||
const isOrganizationMember = (association) =>
|
||||
association === "MEMBER" || association === "OWNER";
|
||||
const hasWritePermission = (permission) =>
|
||||
permission === "write" || permission === "admin";
|
||||
|
||||
const hasCurrentReviewLabel = (pullRequest) =>
|
||||
pullRequest.labels?.some(
|
||||
@@ -234,6 +234,28 @@ async function getPullRequest({ github, context, prNumber }) {
|
||||
return pullRequest;
|
||||
}
|
||||
|
||||
async function pullRequestAuthorCanWrite({ github, context, pullRequest }) {
|
||||
const username = pullRequest.user?.login;
|
||||
if (typeof username !== "string" || username.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
const { data: access } =
|
||||
await github.rest.repos.getCollaboratorPermissionLevel({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
username,
|
||||
});
|
||||
return hasWritePermission(access.permission);
|
||||
} catch (error) {
|
||||
if (error?.status === 404) {
|
||||
return false;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function getLiveMainSha({ github, context }) {
|
||||
const { data: mainRef } = await github.rest.git.getRef({
|
||||
owner: context.repo.owner,
|
||||
@@ -364,10 +386,12 @@ async function prepare({ github, context, core }) {
|
||||
}
|
||||
if (
|
||||
context.eventName === "pull_request_target" &&
|
||||
!isOrganizationMember(pullRequest.author_association)
|
||||
!(await pullRequestAuthorCanWrite({ github, context, pullRequest }))
|
||||
) {
|
||||
const author = pullRequest.user?.login || "unknown author";
|
||||
core.info(
|
||||
`Pull request #${prNumber} requires authorization from a Block organization member.`,
|
||||
`Pull request #${prNumber} author ${author} does not have write access ` +
|
||||
`to ${context.repo.owner}/${context.repo.repo}; manual authorization is required.`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
@@ -479,7 +503,7 @@ async function invalidatePullRequestUpdate({ github, context, core }) {
|
||||
github,
|
||||
context,
|
||||
core,
|
||||
existingOnlyForOrganizationMembers: true,
|
||||
existingOnlyForTrustedAuthors: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -489,7 +513,7 @@ async function invalidate({
|
||||
core,
|
||||
prNumber: requestedPrNumber,
|
||||
existingOnly = false,
|
||||
existingOnlyForOrganizationMembers = false,
|
||||
existingOnlyForTrustedAuthors = false,
|
||||
}) {
|
||||
const prNumber = Number(
|
||||
requestedPrNumber ?? context.payload.pull_request?.number,
|
||||
@@ -506,10 +530,12 @@ async function invalidate({
|
||||
}
|
||||
|
||||
const existing = await findReviewComment({ github, context, prNumber });
|
||||
const trustedUnreviewedAuthor =
|
||||
!existing &&
|
||||
existingOnlyForTrustedAuthors &&
|
||||
(await pullRequestAuthorCanWrite({ github, context, pullRequest }));
|
||||
const shouldOnlyUpdateExisting =
|
||||
existingOnly ||
|
||||
(existingOnlyForOrganizationMembers &&
|
||||
isOrganizationMember(pullRequest.author_association));
|
||||
existingOnly || trustedUnreviewedAuthor;
|
||||
if (!existing && shouldOnlyUpdateExisting) {
|
||||
if (existingOnly || hasCurrentReviewLabel(pullRequest)) {
|
||||
await clearCurrentReview({ github, context, prNumber });
|
||||
|
||||
53
.github/scripts/codex-security-review.test.js
vendored
53
.github/scripts/codex-security-review.test.js
vendored
@@ -24,12 +24,14 @@ const CURRENT_REVIEW_LABEL = "codex-security-review-current";
|
||||
|
||||
function pullRequest({
|
||||
authorAssociation = "CONTRIBUTOR",
|
||||
authorLogin = "pr-author",
|
||||
baseSha = OLD_BASE_SHA,
|
||||
headSha = HEAD_SHA,
|
||||
labels = [],
|
||||
} = {}) {
|
||||
return {
|
||||
author_association: authorAssociation,
|
||||
user: { login: authorLogin },
|
||||
state: "open",
|
||||
base: {
|
||||
ref: "main",
|
||||
@@ -46,6 +48,7 @@ function pullRequest({
|
||||
}
|
||||
|
||||
function harness({
|
||||
authorPermission = "read",
|
||||
pull = pullRequest(),
|
||||
comments = [],
|
||||
files = [],
|
||||
@@ -63,6 +66,7 @@ function harness({
|
||||
const notices = [];
|
||||
const info = [];
|
||||
const warnings = [];
|
||||
const permissionChecks = [];
|
||||
const listComments = async () => storedComments;
|
||||
const listFiles = async () => files;
|
||||
let labelExists = false;
|
||||
@@ -122,6 +126,12 @@ function harness({
|
||||
get: async () => ({ data: pull }),
|
||||
listFiles,
|
||||
},
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async (input) => {
|
||||
permissionChecks.push(input);
|
||||
return { data: { permission: authorPermission } };
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
const core = {
|
||||
@@ -150,6 +160,7 @@ function harness({
|
||||
info,
|
||||
notices,
|
||||
outputs,
|
||||
permissionChecks,
|
||||
removeLabelCalls,
|
||||
removedLabels,
|
||||
storedComments,
|
||||
@@ -231,24 +242,31 @@ test("prepare binds a member command to the named head SHA", async () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("pull request authorization uses the live author association", async () => {
|
||||
const member = harness({
|
||||
pull: pullRequest({ authorAssociation: "MEMBER" }),
|
||||
});
|
||||
member.context.eventName = "pull_request_target";
|
||||
member.context.payload.pull_request = {
|
||||
number: 6816,
|
||||
head: { sha: HEAD_SHA },
|
||||
author_association: "CONTRIBUTOR",
|
||||
};
|
||||
test("pull request authorization uses live repository write access", async () => {
|
||||
for (const authorPermission of ["write", "admin"]) {
|
||||
const trusted = harness({
|
||||
authorPermission,
|
||||
pull: pullRequest({ authorAssociation: "CONTRIBUTOR" }),
|
||||
});
|
||||
trusted.context.eventName = "pull_request_target";
|
||||
trusted.context.payload.pull_request = {
|
||||
number: 6816,
|
||||
head: { sha: HEAD_SHA },
|
||||
author_association: "CONTRIBUTOR",
|
||||
};
|
||||
|
||||
await prepare(member);
|
||||
await prepare(trusted);
|
||||
|
||||
assert.equal(member.outputs.get("authorized"), "true");
|
||||
assert.deepEqual(member.failures, []);
|
||||
assert.equal(trusted.outputs.get("authorized"), "true");
|
||||
assert.deepEqual(trusted.failures, []);
|
||||
assert.deepEqual(trusted.permissionChecks, [
|
||||
{ owner: "block", repo: "buzz", username: "pr-author" },
|
||||
]);
|
||||
}
|
||||
|
||||
const external = harness({
|
||||
pull: pullRequest({ authorAssociation: "CONTRIBUTOR" }),
|
||||
authorPermission: "read",
|
||||
pull: pullRequest({ authorAssociation: "MEMBER" }),
|
||||
});
|
||||
external.context.eventName = "pull_request_target";
|
||||
external.context.payload.pull_request = {
|
||||
@@ -261,7 +279,7 @@ test("pull request authorization uses the live author association", async () =>
|
||||
|
||||
assert.equal(external.outputs.get("authorized"), undefined);
|
||||
assert.deepEqual(external.failures, []);
|
||||
assert.match(external.info.at(-1), /requires authorization/);
|
||||
assert.match(external.info.at(-1), /does not have write access/);
|
||||
});
|
||||
|
||||
test("PR mutation jobs use pull request write permission", () => {
|
||||
@@ -498,8 +516,9 @@ test("base reconciliation does not create comments on unreviewed PRs", async ()
|
||||
assert.equal(state.updated.length, 0);
|
||||
});
|
||||
|
||||
test("pull request updates invalidate member reviews without adding placeholders", async () => {
|
||||
test("pull request updates invalidate trusted reviews without adding placeholders", async () => {
|
||||
const reviewed = harness({
|
||||
authorPermission: "write",
|
||||
pull: pullRequest({
|
||||
authorAssociation: "MEMBER",
|
||||
headSha: OTHER_HEAD_SHA,
|
||||
@@ -529,6 +548,7 @@ test("pull request updates invalidate member reviews without adding placeholders
|
||||
assert.equal(reviewed.removedLabels.length, 1);
|
||||
|
||||
const unreviewed = harness({
|
||||
authorPermission: "write",
|
||||
pull: pullRequest({ authorAssociation: "OWNER" }),
|
||||
});
|
||||
unreviewed.context.eventName = "pull_request_target";
|
||||
@@ -544,6 +564,7 @@ test("pull request updates invalidate member reviews without adding placeholders
|
||||
assert.equal(unreviewed.removeLabelCalls.length, 0);
|
||||
|
||||
const external = harness({
|
||||
authorPermission: "read",
|
||||
pull: pullRequest({ authorAssociation: "CONTRIBUTOR" }),
|
||||
});
|
||||
external.context.eventName = "pull_request_target";
|
||||
|
||||
11
.github/workflows/codex-security-review.yml
vendored
11
.github/workflows/codex-security-review.yml
vendored
@@ -16,10 +16,11 @@ jobs:
|
||||
name: Authorize Security Review
|
||||
# This workflow posts an advisory review; its skipped jobs are not a merge
|
||||
# gate and must not be configured as required status checks.
|
||||
# MEMBER and OWNER are GitHub's associations for members of the `block`
|
||||
# organization. The trusted prepare step checks the live PR instead of the
|
||||
# event snapshot. Outside contributors require this exact command from one
|
||||
# of those members: @buzz-security-review <full-head-sha>
|
||||
# Authors with live write or admin access to `block/buzz` are reviewed
|
||||
# automatically. This calculated repository permission includes direct,
|
||||
# team, organization, and enterprise grants. Outside contributors require
|
||||
# this exact command from a Block organization member:
|
||||
# @buzz-security-review <full-head-sha>
|
||||
if: >-
|
||||
github.repository == 'block/buzz' && (
|
||||
(
|
||||
@@ -301,7 +302,7 @@ jobs:
|
||||
codex-version: '0.149.0'
|
||||
model: ${{ env.CODEX_MODEL }}
|
||||
codex-args: '["-c","model_reasoning_effort=${{ env.CODEX_REASONING_EFFORT }}"]'
|
||||
# The trusted authorization job already enforced repository membership.
|
||||
# The trusted authorization job already enforced repository write access.
|
||||
allow-users: '*'
|
||||
safety-strategy: drop-sudo
|
||||
permission-profile: ':read-only'
|
||||
|
||||
Reference in New Issue
Block a user