Check the PR author's repo permission instead of org association

.github/workflows/pr-risk.yml

@@ -3,19 +3,16 @@ name: PR Risk
 # Shadow-mode risk check for small pull requests. Comments whether the PR would have been
 # auto-approved and why; it never approves, so a human review is still what merges a PR.
 #
-# A PR is skipped, with no comment, when it is a draft, comes from a fork, was not opened by an
-# org member (bots included), changes more than 50 lines, or touches a deny-listed path
-# (defaults in scripts/pr_risk.py plus the caller's `deny-paths`). Otherwise Claude classifies the diff and may deny it on its own judgment.
+# A PR is skipped, with no comment, when it is a draft, comes from a fork, was opened by someone
+# without write access (bots included), changes more than 50 lines, or touches a deny-listed
+# path (defaults in scripts/pr_risk.py plus the caller's `deny-paths`). Otherwise Claude
+# classifies the diff and may deny it on its own judgment.
 # If a PR that already has a comment becomes ineligible on a later push, the comment is
 # rewritten so it never shows a stale "yes".
 #
 # Credentials are split by step. The model runs with no tools and no GitHub token, reading
-# only the PR data gathered with a read-only kernel-internal token. The token that can comment
-# is minted after the model has finished, scoped to this repo and to pull-request writes.
-#
-# Gather uses the app rather than the workflow token because both the workflow token and the
-# pull_request event payload report private org members as CONTRIBUTOR, and every Kernel
-# member is private. An org-installed app's token sees the real MEMBER/OWNER association.
+# only the PR data gathered with the caller's read-only token. The kernel-internal token is
+# minted after the model has finished, scoped to this repo and to pull-request writes.
 
 on:
   workflow_call:
@@ -65,22 +62,10 @@ jobs:
           curl -fsSL "$base/scripts/pr_risk.py" -o /tmp/pr_risk.py
           curl -fsSL "$base/.github/workflows/pr-risk/classify-prompt.md" -o /tmp/pr-risk-prompt.md
 
-      - name: Generate read token
-        id: read-token
-        # Pinned to a commit because this step is handed the app's private key. d72941d is v1.12.0.
-        uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547
-        with:
-          app-id: ${{ secrets.ADMIN_APP_ID }}
-          private-key: ${{ secrets.ADMIN_APP_PRIVATE_KEY }}
-          owner: ${{ github.repository_owner }}
-          repositories: ${{ github.event.repository.name }}
-          permission-contents: read
-          permission-pull-requests: read
-
       - name: Gather
         id: gather
         env:
-          GH_TOKEN: ${{ steps.read-token.outputs.token }}
+          GH_TOKEN: ${{ github.token }}
           DENY_PATHS: ${{ inputs.deny-paths }}
         run: python3 /tmp/pr_risk.py gather --repo "$REPO" --number "$NUMBER" --dir "$WORKDIR"
 

README.md

@@ -71,9 +71,10 @@ Shadow-mode risk check for small PRs. When a PR is opened or marked ready, and o
 it leaves one comment saying whether the PR would have been auto-approved and why. It never
 approves.
 
-These PRs are skipped with no comment: drafts, fork PRs, PRs not opened by an org member
-(including bots), PRs over 50 changed lines, and PRs that touch a deny-listed path. The defaults live in `scripts/pr_risk.py`, and callers can add
-more through `deny-paths`. Claude classifies everything else and can deny a PR on its own
+These PRs are skipped with no comment: drafts, fork PRs, PRs whose author lacks write
+access to the repo (including bots), PRs over 50 changed lines, and PRs that touch a
+deny-listed path. The defaults live in `scripts/pr_risk.py`, and callers can add more
+through `deny-paths`. Claude classifies everything else and can deny a PR on its own
 judgment. The comment stores its verdict in a hidden `pr-risk:v1` JSON marker for later analysis.
 
 ```yaml

scripts/pr_risk.py

@@ -117,6 +117,10 @@ def files(self, number: int) -> list[dict[str, Any]]:
     def comments(self, number: int) -> list[dict[str, Any]]:
         return self.paginate(f"/repos/{self.repo}/issues/{number}/comments?per_page=100")
 
+    def permission(self, login: str) -> str:
+        data = json.loads(self.api(f"/repos/{self.repo}/collaborators/{urllib.parse.quote(login)}/permission"))
+        return data["permission"]
+
     def file_at(self, path: str, ref: str) -> str | None:
         try:
             return self.api("-H", "Accept: application/vnd.github.raw",
@@ -160,17 +164,19 @@ def deny_patterns(extra: str) -> list[str]:
     return list(DEFAULT_DENY_PATHS) + [line for line in lines if line and not line.startswith("#")]
 
 
-def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str]) -> str | None:
+def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str],
+                      author_permission: str) -> str | None:
     if pr.get("draft"):
         return "draft PR"
     head_repo = (pr["head"].get("repo") or {}).get("full_name")
     if head_repo != pr["base"]["repo"]["full_name"]:
         return "PR comes from a fork"
-    # Only org members' own changes are candidates. Bots are never members, so their PRs are
-    # skipped too: no person wrote them, and approving them unread is a separate decision.
-    # This is only right when read with the app token; see the note in pr-risk.yml.
-    if pr.get("author_association") not in ("OWNER", "MEMBER"):
-        return "author is not an org member"
+    # Only changes by people who can already push here are candidates. Bots have no repo
+    # permission, so their PRs are skipped too: no person wrote them, and approving them unread
+    # is a separate decision. author_association can't be used: every Kernel member is private,
+    # and both the workflow token and the event payload report private members as CONTRIBUTOR.
+    if author_permission not in ("admin", "write"):
+        return "author does not have write access"
     changed = pr["additions"] + pr["deletions"]
     if changed > MAX_CHANGED_LINES:
         return f"{changed} changed lines (limit {MAX_CHANGED_LINES})"
@@ -286,7 +292,7 @@ def gather(gh: GitHub, number: int, extra_deny: str, out: Path) -> dict[str, Any
     pr = gh.pull(number)
     files = gh.files(number)
     existing = find_comment(gh.comments(number))
-    reason = ineligible_reason(pr, files, deny_patterns(extra_deny))
+    reason = ineligible_reason(pr, files, deny_patterns(extra_deny), gh.permission(pr["user"]["login"]))
     gate = {
         "sha": pr["head"]["sha"],
         "eligible": reason is None,

scripts/test_pr_risk.py

@@ -29,6 +29,8 @@
     sys.exit(1)
 if "/contents/" in path:
     print("raw file", end="")
+elif path.endswith("/permission"):
+    print(json.dumps({"permission": "none", "role_name": ""}))
 elif "--slurp" in sys.argv:
     print(json.dumps([[{"n": 1}, {"n": 2}], [{"n": 3}]]))
 else:
@@ -38,10 +40,10 @@
 PATCH = "@@ -10,3 +10,4 @@ flags:\n   - a\n   - b\n+  - c\n   - d"
 
 
-def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def", association="MEMBER"):
+def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def"):
     return {
         "number": 7, "draft": draft, "title": "Add an org to a flag", "body": "Adds one org.",
-        "user": {"login": "someone"}, "author_association": association,
+        "user": {"login": "someone"}, "author_association": "CONTRIBUTOR",
         "additions": additions, "deletions": deletions,
         "head": {"sha": sha, "repo": {"full_name": "external/infra" if fork else "kernel/infra"}},
         "base": {"sha": "base000", "ref": "main", "repo": {"full_name": "kernel/infra"}},
@@ -54,12 +56,13 @@ def file(name="posthog/flags/prod.yaml", patch=PATCH, **extra):
 
 
 class FakeGitHub:
-    def __init__(self, pull, files, comments=(), contents=None, head_after=None):
+    def __init__(self, pull, files, comments=(), contents=None, head_after=None, permission="write"):
         self._pull = pull
         self._files = list(files)
         self._comments = list(comments)
         self._contents = contents or {}
         self.head_after = head_after
+        self._permission = permission
         self.created = []
         self.updated = []
         self.pull_reads = 0
@@ -76,6 +79,9 @@ def files(self, number):
     def comments(self, number):
         return self._comments
 
+    def permission(self, login):
+        return self._permission
+
     def file_at(self, path, ref):
         return self._contents.get((path, ref))
 
@@ -130,21 +136,29 @@ def test_ineligible_reasons(self):
         renamed = file("b.yaml", status="renamed", previous_filename="a.yaml", additions=0, changes=0)
         del renamed["patch"]
         cases = {
-            "draft": (pr(draft=True), [file()]),
-            "fork": (pr(fork=True), [file()]),
-            "not an org member": (pr(association="CONTRIBUTOR"), [file()]),
-            "author is not an org member": (pr(association="NONE"), [file()]),
-            "51 changed lines": (pr(additions=30, deletions=21), [file()]),
-            "deny list (`identity/**`)": (pr(), [file("identity/access.go")]),
-            "deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")]),
-            "`bin/helper` has no reviewable text diff": (pr(additions=0), [binary]),
-            "`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed]),
+            "draft": (pr(draft=True), [file()], "write"),
+            "fork": (pr(fork=True), [file()], "write"),
+            "author does not have write access": (pr(), [file()], "read"),
+            "does not have write access": (pr(), [file()], "none"),
+            "51 changed lines": (pr(additions=30, deletions=21), [file()], "write"),
+            "deny list (`identity/**`)": (pr(), [file("identity/access.go")], "write"),
+            "deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")], "write"),
+            "`bin/helper` has no reviewable text diff": (pr(additions=0), [binary], "write"),
+            "`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed], "write"),
         }
-        for expected, (p, files) in cases.items():
+        for expected, (p, files, permission) in cases.items():
             with self.subTest(expected=expected):
-                self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns))
-        self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns))
-        self.assertIsNone(pr_risk.ineligible_reason(pr(association="OWNER"), [file()], patterns))
+                self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns, permission))
+        # pr() reports CONTRIBUTOR, as the API does for every private member; it must not matter.
+        self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns, "write"))
+        self.assertIsNone(pr_risk.ineligible_reason(pr(), [file()], patterns, "admin"))
+
+    def test_bot_author_is_silent(self):
+        gh = FakeGitHub(pr(), [file()], permission="none")
+        gate, code, model_input = self.run_pr(gh)
+        self.assertEqual(gate["reason"], "author does not have write access")
+        self.assertIsNone(model_input)
+        self.assertEqual((gh.created, gh.updated), ([], []))
 
     def test_ineligible_without_comment_is_silent(self):
         gh = FakeGitHub(pr(additions=200), [file()])
@@ -262,12 +276,14 @@ def test_github_client_calls_gh(self):
                 self.assertEqual(gh.file_at("dir/a b.yaml", "abc"), "raw file")
                 self.assertIsNone(gh.file_at("missing.yaml", "abc"))
                 gh.update_comment(9, "hello")
+                self.assertEqual(gh.permission("kernel-internal[bot]"), "none")
             calls = json.loads((root / "log.json").read_text())
         self.assertEqual(calls[0]["args"], ["api", "--paginate", "--slurp", "/repos/kernel/infra/issues/7/comments?per_page=100"])
         self.assertIn("/repos/kernel/infra/contents/dir/a%20b.yaml?ref=abc", calls[1]["args"])
         self.assertIn("Accept: application/vnd.github.raw", calls[1]["args"])
         self.assertEqual(calls[3]["args"][:4], ["api", "-X", "PATCH", "/repos/kernel/infra/issues/comments/9"])
         self.assertEqual(json.loads(calls[3]["stdin"]), {"body": "hello"})
+        self.assertEqual(calls[4]["args"], ["api", "/repos/kernel/infra/collaborators/kernel-internal%5Bbot%5D/permission"])
 
     def test_workflow_isolates_the_model(self):
         workflow = yaml.safe_load(WORKFLOW.read_text())
@@ -280,12 +296,7 @@ def test_workflow_isolates_the_model(self):
         names = list(steps)
         self.assertLess(names.index("Classify"), names.index("Generate app token"))
         self.assertEqual(steps["Generate app token"]["with"]["permission-pull-requests"], "write")
-        # The workflow token and event payload both report private members as CONTRIBUTOR.
-        self.assertEqual(steps["Gather"]["env"]["GH_TOKEN"], "${{ steps.read-token.outputs.token }}")
-        read = steps["Generate read token"]["with"]
-        self.assertEqual({k: v for k, v in read.items() if k.startswith("permission-")},
-                         {"permission-contents": "read", "permission-pull-requests": "read"})
-        self.assertLess(names.index("Generate read token"), names.index("Gather"))
+        self.assertEqual(steps["Gather"]["env"]["GH_TOKEN"], "${{ github.token }}")
         triggers = workflow[True]["workflow_call"]
         self.assertEqual(set(triggers["secrets"]), {"ADMIN_APP_ID", "ADMIN_APP_PRIVATE_KEY", "ANTHROPIC_API_KEY"})