ci: require admin approval for large PR reviews

.github/scripts/pr-review/prepare.mjs

@@ -48,6 +48,10 @@ const headSha = required("PR_HEAD_SHA");
 const sessionKey = required("SESSION_KEY");
 const readinessContextSha256 = required("READINESS_CONTEXT_SHA256");
 const maxDiffBytes = positiveInteger("MAX_DIFF_BYTES");
+// Leave a small read margin so the explicit size check reports the limit.
+const diffReadBufferBytes = maxDiffBytes + 64 * 1024;
+const automaticMaxDiffBytes = positiveInteger("AUTOMATIC_MAX_DIFF_BYTES");
+const largeDiffApproved = required("LARGE_DIFF_APPROVED") === "true";
 const chunkTargetBytes = positiveInteger("CHUNK_TARGET_BYTES");
 const ledgerPath = path.join(stateDir, "review-ledger.json");
 const generationsDir = path.join(stateDir, "generations");
@@ -84,12 +88,18 @@ const diffArgs = (from, to, tripleDot = false) => [
 const mergeBase = String(git(repo, ["merge-base", baseTipSha, headSha])).trim();
 const fullDiff = Buffer.from(git(repo, diffArgs(mergeBase, headSha), {
   encoding: null,
+  maxBuffer: diffReadBufferBytes,
 }));
 if (fullDiff.length > maxDiffBytes) {
   throw new Error(
     `Pull-request diff is ${fullDiff.length} bytes; the configured total limit is ${maxDiffBytes} bytes.`,
   );
 }
+if (!largeDiffApproved && fullDiff.length > automaticMaxDiffBytes) {
+  throw new Error(
+    `Pull-request diff is ${fullDiff.length} bytes; automatic reviews are limited to ${automaticMaxDiffBytes} bytes. A repository admin must comment @codex review approve ${headSha} on this PR to review this head.`,
+  );
+}
 const effectiveDiffSha256 = sha256(fullDiff);
 
 const completed = ledger.generations
@@ -217,7 +227,7 @@ function filePatch(record, from, to, tripleDot) {
     ...diffArgs(from, to, tripleDot),
     "--",
     ...paths,
-  ]));
+  ], { maxBuffer: diffReadBufferBytes }));
 }
 
 function addedLines(patchText) {

.github/scripts/pr-review/test.mjs

@@ -150,6 +150,10 @@ assert.doesNotMatch(workflowSource, /\$\{\{ inputs\.(?:model|effort|max-diff-byt
 assert.match(workflowSource, /^  review:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_MODEL: gpt-6-sol$/m);
 assert.match(workflowSource, /^  review:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_EFFORT: low$/m);
 assert.match(workflowSource, /^  review:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_MAX_DIFF_BYTES: '100000000'$/m);
+assert.match(workflowSource, /^  review:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_AUTOMATIC_MAX_DIFF_BYTES: '5000000'$/m);
+assert.match(workflowSource, /AUTOMATIC_MAX_DIFF_BYTES: \$\{\{ env\.REVIEW_AUTOMATIC_MAX_DIFF_BYTES \}\}/);
+assert.match(workflowSource, /LARGE_DIFF_APPROVED: \$\{\{ needs\.resolve\.outputs\.large_diff_approved \}\}/);
+assert.match(workflowSource, /getCollaboratorPermissionLevel/);
 assert.match(workflowSource, /^  publish:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_MODEL: gpt-6-sol$/m);
 assert.match(workflowSource, /^  publish:\n(?:(?!^  \S)[\s\S])*?^      REVIEW_EFFORT: low$/m);
 assert.match(workflowSource, /MAX_DIFF_BYTES: \$\{\{ env\.REVIEW_MAX_DIFF_BYTES \}\}/);
@@ -537,11 +541,40 @@ try {
       PR_HEAD_SHA: head,
       SESSION_KEY: "repo:1:pr:2:v2",
       MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false",
       CHUNK_TARGET_BYTES: "600",
       READINESS_CONTEXT_SHA256: "context-v1",
     },
   });
   assert.equal(result.status, 0, result.stderr);
+  const prepareWithPolicy = (automaticMax, approved, hardMax = "1000000") =>
+    spawnSync(process.execPath, [
+      path.join(path.dirname(new URL(import.meta.url).pathname), "prepare.mjs"),
+    ], {
+      cwd: repo,
+      encoding: "utf8",
+      env: {
+        ...process.env,
+        REPOSITORY_DIR: repo,
+        PR_REVIEW_STATE_DIR: state,
+        PR_BASE_SHA: base,
+        PR_HEAD_SHA: head,
+        SESSION_KEY: "repo:1:pr:2:v2",
+        MAX_DIFF_BYTES: hardMax,
+        AUTOMATIC_MAX_DIFF_BYTES: automaticMax,
+        LARGE_DIFF_APPROVED: String(approved),
+        CHUNK_TARGET_BYTES: "600",
+        READINESS_CONTEXT_SHA256: "context-v1",
+      },
+    });
+  const blockedAutomatic = prepareWithPolicy("1000", false);
+  assert.notEqual(blockedAutomatic.status, 0);
+  assert.match(blockedAutomatic.stderr, /automatic reviews are limited to 1000 bytes/);
+  assert.equal(prepareWithPolicy("1000", true).status, 0);
+  const blockedHardCap = prepareWithPolicy("1000", true, "1000");
+  assert.notEqual(blockedHardCap.status, 0);
+  assert.match(blockedHardCap.stderr, /configured total limit is 1000 bytes/);
   const ledger = JSON.parse(fs.readFileSync(
     path.join(state, "review-ledger.json"),
     "utf8",
@@ -578,6 +611,8 @@ try {
       PR_HEAD_SHA: head,
       SESSION_KEY: "repo:1:pr:2:v2",
       MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false",
       CHUNK_TARGET_BYTES: "600",
       READINESS_CONTEXT_SHA256: "context-v1",
     },
@@ -613,6 +648,8 @@ try {
       PR_HEAD_SHA: nextHead,
       SESSION_KEY: "repo:1:pr:2:v2",
       MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false",
       CHUNK_TARGET_BYTES: "600",
       READINESS_CONTEXT_SHA256: "context-v1",
     },
@@ -673,6 +710,8 @@ try {
     PR_HEAD_SHA: mergedHead,
     SESSION_KEY: "repo:1:pr:2:v2",
     MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false",
     CHUNK_TARGET_BYTES: "600",
     READINESS_CONTEXT_SHA256: "context-v1",
   };
@@ -1019,7 +1058,9 @@ fs.writeFileSync(outputFile, JSON.stringify(result));
       cwd: repo, encoding: "utf8",
       env: { ...process.env, REPOSITORY_DIR: repo, PR_REVIEW_STATE_DIR: legacyState,
         PR_BASE_SHA: base, PR_HEAD_SHA: nextHead, SESSION_KEY: "repo:1:pr:2:v2",
-        MAX_DIFF_BYTES: "1000000", CHUNK_TARGET_BYTES: "600",
+        MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false", CHUNK_TARGET_BYTES: "600",
         READINESS_CONTEXT_SHA256: "context-v1", GITHUB_OUTPUT: output },
     });
     assert.equal(prepared.status, 0, prepared.stderr);
@@ -1369,6 +1410,8 @@ fs.writeFileSync(outputFile, JSON.stringify(result));
       PR_HEAD_SHA: mergedPrHead,
       SESSION_KEY: "repo:1:pr:2:v2",
       MAX_DIFF_BYTES: "1000000",
+      AUTOMATIC_MAX_DIFF_BYTES: "1000000",
+      LARGE_DIFF_APPROVED: "false",
       CHUNK_TARGET_BYTES: "600",
       READINESS_CONTEXT_SHA256: "context-v1",
     },

.github/scripts/review-request/common.mjs

@@ -5,6 +5,8 @@ export const REVIEW_REQUEST_SCHEMA_VERSION = 1;
 // spaces as an indented code block, so the trigger accepts at most three.
 export const REVIEW_REQUEST_COMMAND =
   /^ {0,3}@codex(?:[ \t]+review\b.*?)?[ \t]*$/im;
+const LARGE_DIFF_APPROVAL_COMMAND =
+  /^ {0,3}@codex[ \t]+review[ \t]+approve[ \t]+([0-9a-f]{40})[ \t]*$/im;
 
 const FENCE = /^ {0,3}(`{3,}|~{3,})/;
 const BLOCK_QUOTE = /^ {0,3}>/;
@@ -100,3 +102,15 @@ export function isReviewRequestComment(comment) {
   if (type === "bot" || /\[bot\]$/i.test(login)) return false;
   return REVIEW_REQUEST_COMMAND.test(stripQuotedText(comment?.body));
 }
+
+export function largeDiffApprovalSha(comment) {
+  const type = String(comment?.user_type ?? "").toLowerCase();
+  const login = String(comment?.user_login ?? "");
+  if (type === "bot" || /\[bot\]$/i.test(login)) return null;
+  return LARGE_DIFF_APPROVAL_COMMAND.exec(stripQuotedText(comment?.body))?.[1]?.toLowerCase() ?? null;
+}
+
+export function isAdminLargeDiffApproval({ comment, permission, headSha }) {
+  return permission === "admin"
+    && largeDiffApprovalSha(comment) === String(headSha).toLowerCase();
+}

.github/scripts/review-request/test.mjs

@@ -2,13 +2,48 @@
 
 import assert from "node:assert/strict";
 import {
+  isAdminLargeDiffApproval,
   isReviewRequestComment,
+  largeDiffApprovalSha,
   REVIEW_REQUEST_SCHEMA_VERSION,
   stripQuotedText,
 } from "./common.mjs";
 
 assert.equal(REVIEW_REQUEST_SCHEMA_VERSION, 1);
 
+const approvalHead = "a".repeat(40);
+const approvalComment = {
+  body: `@codex review approve ${approvalHead}`,
+  user_type: "User",
+  user_login: "admin-user",
+};
+assert.equal(isReviewRequestComment(approvalComment), true);
+assert.equal(largeDiffApprovalSha(approvalComment), approvalHead);
+assert.equal(isAdminLargeDiffApproval({
+  comment: approvalComment, permission: "admin", headSha: approvalHead,
+}), true);
+for (const permission of ["write", "maintain", "read", "none", undefined]) {
+  assert.equal(isAdminLargeDiffApproval({
+    comment: approvalComment, permission, headSha: approvalHead,
+  }), false);
+}
+assert.equal(isAdminLargeDiffApproval({
+  comment: approvalComment, permission: "admin", headSha: "b".repeat(40),
+}), false);
+for (const body of [
+  `> @codex review approve ${approvalHead}`,
+  `\`@codex review approve ${approvalHead}\``,
+  `\`\`\`\n@codex review approve ${approvalHead}\n\`\`\``,
+  `@codex review approve ${approvalHead.slice(1)}`,
+  `@codex review approve ${approvalHead} extra`,
+  `@codex review ${approvalHead}`,
+]) {
+  assert.equal(largeDiffApprovalSha({ ...approvalComment, body }), null, body);
+}
+assert.equal(largeDiffApprovalSha({
+  ...approvalComment, user_type: "Bot",
+}), null);
+
 const cases = [
   ["bare mention", "@codex", true],
   ["bare mention with surrounding whitespace", "  \n@codex  \n\n", true],

.github/workflows/codex-openai-review.yml

@@ -97,6 +97,7 @@ jobs:
       base_ref: ${{ steps.pr.outputs.base_ref }}
       head_sha: ${{ steps.pr.outputs.head_sha }}
       request_comment_id: ${{ steps.pr.outputs.request_comment_id }}
+      large_diff_approved: ${{ steps.pr.outputs.large_diff_approved }}
     steps:
       - name: Check out the exact comment-trigger implementation
         if: github.event_name == 'issue_comment'
@@ -158,7 +159,40 @@ jobs:
             const eligible = pr.state === 'open'
               && !pr.draft
               && (sameRepository || trustedBaseEvent);
+            let largeDiffApproved = false;
+            if (event === 'issue_comment' && process.env.COMMENT_REVIEW_REQUESTED === 'true') {
+              const { pathToFileURL } = require('node:url');
+              const path = require('node:path');
+              const source = pathToFileURL(path.join(
+                process.env.GITHUB_WORKSPACE,
+                '.openai-pr-review-trigger-source',
+                '.github/scripts/review-request/common.mjs',
+              )).href;
+              const { largeDiffApprovalSha, isAdminLargeDiffApproval } = await import(source);
+              const comment = {
+                body: context.payload.comment.body,
+                user_type: context.payload.comment.user.type,
+                user_login: context.payload.comment.user.login,
+              };
+              if (largeDiffApprovalSha(comment)) {
+                try {
+                  const { data: access } = await github.rest.repos.getCollaboratorPermissionLevel({
+                    owner: context.repo.owner,
+                    repo: context.repo.repo,
+                    username: comment.user_login,
+                  });
+                  largeDiffApproved = isAdminLargeDiffApproval({
+                    comment,
+                    permission: access.permission,
+                    headSha: pr.head.sha,
+                  });
+                } catch (error) {
+                  core.warning(`Could not verify large-diff approval permission: ${error.message}`);
+                }
+              }
+            }
             core.setOutput('eligible', String(eligible));
+            core.setOutput('large_diff_approved', String(largeDiffApproved));
             core.setOutput('number', String(pr.number));
             core.setOutput('base_sha', pr.base.sha);
             core.setOutput('base_ref', pr.base.ref);
@@ -320,6 +354,7 @@ jobs:
       REVIEW_MODEL: gpt-6-sol
       REVIEW_EFFORT: low
       REVIEW_MAX_DIFF_BYTES: '100000000'
+      REVIEW_AUTOMATIC_MAX_DIFF_BYTES: '5000000'
       PULL_REQUEST_NUMBER: ${{ needs.resolve.outputs.number }}
       PR_BASE_SHA: ${{ needs.resolve.outputs.base_sha }}
       PR_BASE_REF: ${{ needs.resolve.outputs.base_ref }}
@@ -844,6 +879,8 @@ jobs:
         env:
           REPOSITORY_DIR: ${{ env.PR_DIFF_REPOSITORY }}
           MAX_DIFF_BYTES: ${{ env.REVIEW_MAX_DIFF_BYTES }}
+          AUTOMATIC_MAX_DIFF_BYTES: ${{ env.REVIEW_AUTOMATIC_MAX_DIFF_BYTES }}
+          LARGE_DIFF_APPROVED: ${{ needs.resolve.outputs.large_diff_approved }}
           CHUNK_TARGET_BYTES: ${{ inputs.chunk-target-bytes }}
           READINESS_CONTEXT_SHA256: ${{ steps.prepare_readiness.outputs.context_sha256 }}
         run: node "$REVIEWER_SOURCE_DIR/.github/scripts/pr-review/prepare.mjs"

README.md

@@ -32,6 +32,20 @@ and `actions: write` so the reviewer can restore the latest per-PR Codex
 session artifact and delete superseded snapshots only after a replacement
 upload succeeds.
 
+Complete diffs up to 5,000,000 bytes are reviewed automatically. For a larger
+diff, an admin of the **calling repository** must post this line as a PR comment
+with the current full head commit SHA:
+
+```text
+@codex review approve <40-character-head-sha>
+```
+
+The shared reviewer checks the comment author's admin permission in the caller
+repository and the live head SHA before model work. A push requires a new
+approval. This uses the existing `issue_comment` trigger and works for public
+and private callers without a GitHub Environment or per-repository approver
+configuration. Diffs above 100,000,000 bytes remain rejected.
+
 ## Behavior
 
 - Reviews an open, non-draft PR when it is opened, reopened, edited, marked
@@ -138,8 +152,8 @@ upload succeeds.
 - Fork PRs run through the caller repository's trusted default-branch
   `pull_request_target` workflow and use the caller's explicitly forwarded
   secret. Secrets from the contributor's fork are not imported or used.
-- Automatic PR events intentionally permit an external contributor to consume
-  review requests when opening, editing, or pushing to an eligible PR. Use a
+- Automatic PR events permit an external contributor to consume review
+  requests up to the 5,000,000-byte automatic limit. Use a
   dedicated API project with appropriate usage limits and restrict the
   organization secret to selected repositories.
 - Complete diffs larger than 100,000,000 bytes fail before Codex runs. The