Skip to content

Commit 0eeb376

Browse files
authored
Read PR author association with the kernel-internal app token (#27)
Co-authored-by: tnsardesai <18272584+tnsardesai@users.noreply.github.com>
1 parent bc63679 commit 0eeb376

3 files changed

Lines changed: 48 additions & 32 deletions

File tree

‎.github/workflows/pr-risk.yml‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@ name: PR Risk
1010
# rewritten so it never shows a stale "yes".
1111
#
1212
# Credentials are split by step. The model runs with no tools and no GitHub token, reading
13-
# only the PR data gathered with the caller's read-only token. The kernel-internal token is
14-
# minted after the model has finished, scoped to this repo and to pull-request writes.
13+
# only the PR data gathered with a read-only kernel-internal token. The token that can comment
14+
# is minted after the model has finished, scoped to this repo and to pull-request writes.
15+
#
16+
# Gather uses the app rather than the workflow token because both the workflow token and the
17+
# pull_request event payload report private org members as CONTRIBUTOR, and every Kernel
18+
# member is private. An org-installed app's token sees the real MEMBER/OWNER association.
1519

1620
on:
1721
workflow_call:
@@ -61,12 +65,23 @@ jobs:
6165
curl -fsSL "$base/scripts/pr_risk.py" -o /tmp/pr_risk.py
6266
curl -fsSL "$base/.github/workflows/pr-risk/classify-prompt.md" -o /tmp/pr-risk-prompt.md
6367
68+
- name: Generate read token
69+
id: read-token
70+
# Pinned to a commit because this step is handed the app's private key. d72941d is v1.12.0.
71+
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547
72+
with:
73+
app-id: ${{ secrets.ADMIN_APP_ID }}
74+
private-key: ${{ secrets.ADMIN_APP_PRIVATE_KEY }}
75+
owner: ${{ github.repository_owner }}
76+
repositories: ${{ github.event.repository.name }}
77+
permission-contents: read
78+
permission-pull-requests: read
79+
6480
- name: Gather
6581
id: gather
6682
env:
67-
GH_TOKEN: ${{ github.token }}
83+
GH_TOKEN: ${{ steps.read-token.outputs.token }}
6884
DENY_PATHS: ${{ inputs.deny-paths }}
69-
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }}
7085
run: python3 /tmp/pr_risk.py gather --repo "$REPO" --number "$NUMBER" --dir "$WORKDIR"
7186

7287
- name: Install Claude Code CLI

‎scripts/pr_risk.py‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,16 @@ def deny_patterns(extra: str) -> list[str]:
160160
return list(DEFAULT_DENY_PATHS) + [line for line in lines if line and not line.startswith("#")]
161161

162162

163-
def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str],
164-
association: str) -> str | None:
163+
def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str]) -> str | None:
165164
if pr.get("draft"):
166165
return "draft PR"
167166
head_repo = (pr["head"].get("repo") or {}).get("full_name")
168167
if head_repo != pr["base"]["repo"]["full_name"]:
169168
return "PR comes from a fork"
170169
# Only org members' own changes are candidates. Bots are never members, so their PRs are
171170
# skipped too: no person wrote them, and approving them unread is a separate decision.
172-
# The association comes from the event payload: the API answers for the caller's token,
173-
# and the workflow token cannot see private org membership, so it reports CONTRIBUTOR.
174-
if association not in ("OWNER", "MEMBER"):
171+
# This is only right when read with the app token; see the note in pr-risk.yml.
172+
if pr.get("author_association") not in ("OWNER", "MEMBER"):
175173
return "author is not an org member"
176174
changed = pr["additions"] + pr["deletions"]
177175
if changed > MAX_CHANGED_LINES:
@@ -283,12 +281,12 @@ def previous_data(body: str) -> dict[str, Any] | None:
283281
return None
284282

285283

286-
def gather(gh: GitHub, number: int, extra_deny: str, association: str, out: Path) -> dict[str, Any]:
284+
def gather(gh: GitHub, number: int, extra_deny: str, out: Path) -> dict[str, Any]:
287285
out.mkdir(parents=True, exist_ok=True)
288286
pr = gh.pull(number)
289287
files = gh.files(number)
290288
existing = find_comment(gh.comments(number))
291-
reason = ineligible_reason(pr, files, deny_patterns(extra_deny), association)
289+
reason = ineligible_reason(pr, files, deny_patterns(extra_deny))
292290
gate = {
293291
"sha": pr["head"]["sha"],
294292
"eligible": reason is None,
@@ -439,7 +437,7 @@ def main() -> int:
439437
return 0
440438
gh = GitHub(args.repo)
441439
if args.cmd == "gather":
442-
gather(gh, args.number, os.environ.get("DENY_PATHS", ""), os.environ["AUTHOR_ASSOCIATION"], args.dir)
440+
gather(gh, args.number, os.environ.get("DENY_PATHS", ""), args.dir)
443441
return 0
444442
return comment(gh, args.number, args.dir, os.environ.get("RUN_URL", ""))
445443

‎scripts/test_pr_risk.py‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@
3838
PATCH = "@@ -10,3 +10,4 @@ flags:\n - a\n - b\n+ - c\n - d"
3939

4040

41-
def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def"):
41+
def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def", association="MEMBER"):
4242
return {
4343
"number": 7, "draft": draft, "title": "Add an org to a flag", "body": "Adds one org.",
44-
"user": {"login": "someone"}, "author_association": "CONTRIBUTOR",
44+
"user": {"login": "someone"}, "author_association": association,
4545
"additions": additions, "deletions": deletions,
4646
"head": {"sha": sha, "repo": {"full_name": "external/infra" if fork else "kernel/infra"}},
4747
"base": {"sha": "base000", "ref": "main", "repo": {"full_name": "kernel/infra"}},
@@ -97,10 +97,10 @@ def visible_words(body):
9797

9898

9999
class PRRiskTests(unittest.TestCase):
100-
def run_pr(self, gh, verdict=VERDICT, result=None, extra_deny="", association="MEMBER"):
100+
def run_pr(self, gh, verdict=VERDICT, result=None, extra_deny=""):
101101
with tempfile.TemporaryDirectory() as tmp:
102102
work = Path(tmp)
103-
gate = pr_risk.gather(gh, 7, extra_deny, association, work)
103+
gate = pr_risk.gather(gh, 7, extra_deny, work)
104104
if gate["eligible"]:
105105
(work / "result.json").write_text(json.dumps(result or {"is_error": False, "structured_output": verdict}))
106106
code = pr_risk.comment(gh, 7, work, "https://example.com/run")
@@ -130,22 +130,21 @@ def test_ineligible_reasons(self):
130130
renamed = file("b.yaml", status="renamed", previous_filename="a.yaml", additions=0, changes=0)
131131
del renamed["patch"]
132132
cases = {
133-
"draft": (pr(draft=True), [file()], "MEMBER"),
134-
"fork": (pr(fork=True), [file()], "MEMBER"),
135-
"not an org member": (pr(), [file()], "CONTRIBUTOR"),
136-
"author is not an org member": (pr(), [file()], "NONE"),
137-
"51 changed lines": (pr(additions=30, deletions=21), [file()], "MEMBER"),
138-
"deny list (`identity/**`)": (pr(), [file("identity/access.go")], "MEMBER"),
139-
"deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")], "MEMBER"),
140-
"`bin/helper` has no reviewable text diff": (pr(additions=0), [binary], "MEMBER"),
141-
"`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed], "MEMBER"),
133+
"draft": (pr(draft=True), [file()]),
134+
"fork": (pr(fork=True), [file()]),
135+
"not an org member": (pr(association="CONTRIBUTOR"), [file()]),
136+
"author is not an org member": (pr(association="NONE"), [file()]),
137+
"51 changed lines": (pr(additions=30, deletions=21), [file()]),
138+
"deny list (`identity/**`)": (pr(), [file("identity/access.go")]),
139+
"deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")]),
140+
"`bin/helper` has no reviewable text diff": (pr(additions=0), [binary]),
141+
"`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed]),
142142
}
143-
for expected, (p, files, association) in cases.items():
143+
for expected, (p, files) in cases.items():
144144
with self.subTest(expected=expected):
145-
self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns, association))
146-
# The API reports CONTRIBUTOR for private members; only the event payload's value counts.
147-
self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns, "MEMBER"))
148-
self.assertIsNone(pr_risk.ineligible_reason(pr(), [file()], patterns, "OWNER"))
145+
self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns))
146+
self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns))
147+
self.assertIsNone(pr_risk.ineligible_reason(pr(association="OWNER"), [file()], patterns))
149148

150149
def test_ineligible_without_comment_is_silent(self):
151150
gh = FakeGitHub(pr(additions=200), [file()])
@@ -281,8 +280,12 @@ def test_workflow_isolates_the_model(self):
281280
names = list(steps)
282281
self.assertLess(names.index("Classify"), names.index("Generate app token"))
283282
self.assertEqual(steps["Generate app token"]["with"]["permission-pull-requests"], "write")
284-
self.assertEqual(steps["Gather"]["env"]["AUTHOR_ASSOCIATION"],
285-
"${{ github.event.pull_request.author_association }}")
283+
# The workflow token and event payload both report private members as CONTRIBUTOR.
284+
self.assertEqual(steps["Gather"]["env"]["GH_TOKEN"], "${{ steps.read-token.outputs.token }}")
285+
read = steps["Generate read token"]["with"]
286+
self.assertEqual({k: v for k, v in read.items() if k.startswith("permission-")},
287+
{"permission-contents": "read", "permission-pull-requests": "read"})
288+
self.assertLess(names.index("Generate read token"), names.index("Gather"))
286289
triggers = workflow[True]["workflow_call"]
287290
self.assertEqual(set(triggers["secrets"]), {"ADMIN_APP_ID", "ADMIN_APP_PRIVATE_KEY", "ANTHROPIC_API_KEY"})
288291

0 commit comments

Comments
 (0)