From efce18fb8a9548bf6e57d0a46c195e8b310fcc52 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Fri, 14 Aug 2026 09:40:57 -0500 Subject: [PATCH] fix: gate automatic reviews by author trust Signed-off-by: Codex --- crates/github/src/lib.rs | 2 ++ crates/webhook/src/events.rs | 2 ++ crates/webhook/src/routes.rs | 27 +++++++++++++++++++ .../tests/fixtures/pull_request_opened.json | 1 + .../pull_request_ready_for_review.json | 1 + .../fixtures/pull_request_synchronize.json | 1 + crates/webhook/tests/parse_fixtures.rs | 1 + 7 files changed, 35 insertions(+) diff --git a/crates/github/src/lib.rs b/crates/github/src/lib.rs index 241fb57..e2e1d7d 100644 --- a/crates/github/src/lib.rs +++ b/crates/github/src/lib.rs @@ -110,6 +110,8 @@ pub struct PrChangedEvent { pub head_sha: String, pub base_ref: String, pub author_login: String, + /// GitHub's relationship between the PR author and the repository. + pub author_association: Option, pub draft: bool, } diff --git a/crates/webhook/src/events.rs b/crates/webhook/src/events.rs index 51e088f..63e0059 100644 --- a/crates/webhook/src/events.rs +++ b/crates/webhook/src/events.rs @@ -121,6 +121,7 @@ pub struct PullRequest { pub head: Option, pub base: Option, pub user: Option, + pub author_association: Option, } #[derive(Debug, Deserialize)] @@ -257,6 +258,7 @@ pub fn parse_event(event_type: &str, payload: &WebhookPayload) -> GitHubEvent { head_sha: head.sha.clone(), base_ref: base.git_ref.clone(), author_login: user.login.clone(), + author_association: pr.author_association.clone(), draft: pr.draft, }); } diff --git a/crates/webhook/src/routes.rs b/crates/webhook/src/routes.rs index 85af861..f415ab3 100644 --- a/crates/webhook/src/routes.rs +++ b/crates/webhook/src/routes.rs @@ -1090,6 +1090,17 @@ async fn event_to_task(state: &AppState, event: GitHubEvent) -> Option { let label = e.label_name.as_deref()?; scope.familiar_by_label(label)? } else { + // Automatic lifecycle reviews execute repository-controlled + // automation, so only authors GitHub identifies as having a + // trusted repository relationship may start one. Missing or + // external associations fail closed. A maintainer-applied + // review label remains the explicit opt-in path above. + if !matches!( + e.author_association.as_deref(), + Some("OWNER" | "MEMBER" | "COLLABORATOR") + ) { + return None; + } if !scope.reviews_enabled() { return None; } @@ -1782,6 +1793,7 @@ mod review_lane_tests { head_sha: "abc123".to_string(), base_ref: "main".to_string(), author_login: "octocat".to_string(), + author_association: Some("MEMBER".to_string()), draft: false, } } @@ -1830,6 +1842,20 @@ mod review_lane_tests { ); } + #[tokio::test] + async fn automatic_reviews_reject_untrusted_pr_authors() { + let state = app_state_with_review(review_on()); + for association in [None, Some("NONE"), Some("CONTRIBUTOR")] { + let mut event = pr_event("opened"); + event.author_association = association.map(str::to_string); + assert!( + event_to_task(&state, GitHubEvent::PullRequestChanged(event)) + .await + .is_none() + ); + } + } + #[tokio::test] async fn familiar_authored_prs_are_never_auto_reviewed() { // The adapter's own draft PRs must not re-trigger reviews (loop guard). @@ -1878,6 +1904,7 @@ mod review_lane_tests { let state = app_state(); let mut event = pr_event("labeled"); event.label_name = Some("coven:review".to_string()); + event.author_association = Some("NONE".to_string()); event.draft = true; let task = event_to_task(&state, GitHubEvent::PullRequestChanged(event)) .await diff --git a/crates/webhook/tests/fixtures/pull_request_opened.json b/crates/webhook/tests/fixtures/pull_request_opened.json index 5b28349..38f519b 100644 --- a/crates/webhook/tests/fixtures/pull_request_opened.json +++ b/crates/webhook/tests/fixtures/pull_request_opened.json @@ -11,6 +11,7 @@ "title": "Add spell compiler cache", "draft": false, "user": { "login": "octocat" }, + "author_association": "MEMBER", "head": { "ref": "feat/spell-cache", "sha": "abc123def4567890abc123def4567890abc123de" }, "base": { "ref": "main", "sha": "0001112223334445556667778889990001112223" } }, diff --git a/crates/webhook/tests/fixtures/pull_request_ready_for_review.json b/crates/webhook/tests/fixtures/pull_request_ready_for_review.json index 19876ef..186278b 100644 --- a/crates/webhook/tests/fixtures/pull_request_ready_for_review.json +++ b/crates/webhook/tests/fixtures/pull_request_ready_for_review.json @@ -11,6 +11,7 @@ "title": "Harden sigil parser", "draft": false, "user": { "login": "hexadecimal-cat" }, + "author_association": "MEMBER", "head": { "ref": "fix/sigil-parser", "sha": "beadfeedbeadfeedbeadfeedbeadfeedbeadfeed" }, "base": { "ref": "main", "sha": "0001112223334445556667778889990001112223" } }, diff --git a/crates/webhook/tests/fixtures/pull_request_synchronize.json b/crates/webhook/tests/fixtures/pull_request_synchronize.json index 27bd331..3dd63c7 100644 --- a/crates/webhook/tests/fixtures/pull_request_synchronize.json +++ b/crates/webhook/tests/fixtures/pull_request_synchronize.json @@ -13,6 +13,7 @@ "title": "Add spell compiler cache", "draft": true, "user": { "login": "octocat" }, + "author_association": "MEMBER", "head": { "ref": "feat/spell-cache", "sha": "f00dfacef00dfacef00dfacef00dfacef00dface" }, "base": { "ref": "main", "sha": "0001112223334445556667778889990001112223" } }, diff --git a/crates/webhook/tests/parse_fixtures.rs b/crates/webhook/tests/parse_fixtures.rs index 920f0fc..8f57454 100644 --- a/crates/webhook/tests/parse_fixtures.rs +++ b/crates/webhook/tests/parse_fixtures.rs @@ -141,6 +141,7 @@ fn pull_request_opened_fixture_parses() { assert_eq!(e.head_sha, "abc123def4567890abc123def4567890abc123de"); assert_eq!(e.base_ref, "main"); assert_eq!(e.author_login, "octocat"); + assert_eq!(e.author_association.as_deref(), Some("MEMBER")); assert!(!e.draft); } other => panic!("expected PullRequestChanged, got {other:?}"),