From bf799a1628263e2c1682e2697dc9c31cc1bc3ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20Bu=C4=9Fra=20Yi=C4=9Fiter?= Date: Thu, 13 Aug 2026 17:36:07 +0300 Subject: [PATCH] fix(github): allow CONTRIBUTOR association by default GitHub labels some write-access teammates as CONTRIBUTOR instead of MEMBER, so the payload gate skipped reviews before the live write check. --- action.yml | 2 +- specs/project/functional_spec.md | 2 +- specs/project/html/functional_spec.html | 5 ++++- src/github-action/event-gate.ts | 11 ++++++++++- tests/github-action.test.ts | 16 +++++++++++++++- 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/action.yml b/action.yml index e0939f2..d4975d7 100644 --- a/action.yml +++ b/action.yml @@ -21,7 +21,7 @@ inputs: default: "true" allowed-associations: description: "Comma-separated author associations allowed to trigger (payload check; a live write-permission check also applies)." - default: "OWNER,MEMBER,COLLABORATOR" + default: "OWNER,MEMBER,COLLABORATOR,CONTRIBUTOR,FIRST_TIME_CONTRIBUTOR" allowed-users: description: "Comma-separated logins allowed to trigger regardless of association. Widening this is a deliberate security decision." default: "" diff --git a/specs/project/functional_spec.md b/specs/project/functional_spec.md index 64a12e4..256029c 100644 --- a/specs/project/functional_spec.md +++ b/specs/project/functional_spec.md @@ -132,7 +132,7 @@ Behavior: `codegenie github-action` is the entrypoint the bundled composite action (`action.yml`) invokes inside GitHub Actions runners; it is not intended for interactive use. It reads the standard Actions environment (`GITHUB_EVENT_NAME`, `GITHUB_EVENT_PATH`, `GITHUB_REPOSITORY`, `GITHUB_RUN_ID`), decides whether the event should trigger a review, and when it should, runs the ordinary `--pr` review with a live status comment on the PR (see `components/repository_and_github.md`, GitHub Action Adapter). With `preflight-only: true`, it performs the same trigger and live-permission checks without claiming a comment or starting a review, and returns `should-run`/`pr-number` Action outputs. - Trigger lanes: `pull_request` (opened/synchronize/ready_for_review; drafts and fork heads skip) and `issue_comment` (created, on an open PR; the trimmed comment must equal the configured trigger phrase, default `codegenie review`, or start with it followed by whitespace — trailing text is ignored and never parsed into options). -- Authorization: payload author association must be in the allowlist (default OWNER/MEMBER/COLLABORATOR) and a live collaborator-permission check must report write or admin; `allowed-users` bypasses both explicitly. Bot actors are ignored unless allowlisted. Authorization logic lives only in the binary — payload-only YAML approximations are forbidden. Workflow templates are single-job with per-PR `concurrency` and uniform `cancel-in-progress: true` (newest event wins); the accepted residual is that a comment can supersede its own PR's in-flight run before the skip decision. `preflight-only: true` remains available for a stricter two-job pattern: it runs the same gate without claiming a comment or reviewing, returning `should-run`/`pr-number` outputs. +- Authorization: payload author association must be in the allowlist (default OWNER/MEMBER/COLLABORATOR/CONTRIBUTOR/FIRST_TIME_CONTRIBUTOR) and a live collaborator-permission check must report write or admin; `allowed-users` bypasses both explicitly. Bot actors are ignored unless allowlisted. CONTRIBUTOR and FIRST_TIME_CONTRIBUTOR are included because GitHub uses those labels for some write-access org teammates (private membership, and some org-repo permission shapes) instead of MEMBER. Authorization logic lives only in the binary — payload-only YAML approximations are forbidden. Workflow templates are single-job with per-PR `concurrency` and uniform `cancel-in-progress: true` (newest event wins); the accepted residual is that a comment can supersede its own PR's in-flight run before the skip decision. `preflight-only: true` remains available for a stricter two-job pattern: it runs the same gate without claiming a comment or reviewing, returning `should-run`/`pr-number` outputs. - Comment-author identity resolves as `bot-login` input → `/user` lookup → `github-actions[bot]`; status-comment reclaim requires an exact case-insensitive author match against it. - A non-triggering event is a skip: exit `0`, one-line stdout reason, nothing posted. Review and posting failures keep their existing nonzero exit semantics. - Inline posting maps to the existing `--post-github-comments` flag; the status comment is enabled only by this entrypoint. Repo-resident config can enable neither. diff --git a/specs/project/html/functional_spec.html b/specs/project/html/functional_spec.html index 075ffaf..7f6a3db 100644 --- a/specs/project/html/functional_spec.html +++ b/specs/project/html/functional_spec.html @@ -415,9 +415,12 @@

GitHub Action Mode

codegenie review, or start with it followed by whitespace — trailing text is ignored and never parsed into options).
  • Authorization: payload author association must be in the allowlist -(default OWNER/MEMBER/COLLABORATOR) and a live collaborator-permission +(default OWNER/MEMBER/COLLABORATOR/CONTRIBUTOR/FIRST_TIME_CONTRIBUTOR) and a live collaborator-permission check must report write or admin; allowed-users bypasses both explicitly. Bot actors are ignored unless allowlisted. +CONTRIBUTOR and FIRST_TIME_CONTRIBUTOR are included because GitHub +uses those labels for some write-access org teammates (private +membership, and some org-repo permission shapes) instead of MEMBER. Authorization logic lives only in the binary — payload-only YAML approximations are forbidden. Workflow templates are single-job with per-PR concurrency and uniform diff --git a/src/github-action/event-gate.ts b/src/github-action/event-gate.ts index 655ca80..7269b24 100644 --- a/src/github-action/event-gate.ts +++ b/src/github-action/event-gate.ts @@ -5,7 +5,16 @@ // review knobs come exclusively from workflow inputs. export const DEFAULT_TRIGGER_PHRASE = "codegenie review"; -export const DEFAULT_ALLOWED_ASSOCIATIONS = ["OWNER", "MEMBER", "COLLABORATOR"]; +// GitHub reports some write-access teammates as CONTRIBUTOR / +// FIRST_TIME_CONTRIBUTOR instead of MEMBER (private org membership, and +// some org-repo permission shapes). The live write check is still required. +export const DEFAULT_ALLOWED_ASSOCIATIONS = [ + "OWNER", + "MEMBER", + "COLLABORATOR", + "CONTRIBUTOR", + "FIRST_TIME_CONTRIBUTOR" +]; // ready_for_review completes the draft story: drafts skip, so the moment a // draft is marked ready must itself trigger the review. const PULL_REQUEST_ACTIONS = new Set(["opened", "synchronize", "ready_for_review"]); diff --git a/tests/github-action.test.ts b/tests/github-action.test.ts index 79a38f4..de61225 100644 --- a/tests/github-action.test.ts +++ b/tests/github-action.test.ts @@ -109,6 +109,19 @@ describe("github-action event gate", () => { }); }); + it("allows CONTRIBUTOR and FIRST_TIME_CONTRIBUTOR by default", () => { + expect( + decideTrigger("pull_request", pullRequestPayload({ author_association: "CONTRIBUTOR" }), RULES) + ).toMatchObject({ run: true, association: "CONTRIBUTOR" }); + expect( + decideTrigger( + "pull_request", + pullRequestPayload({ author_association: "FIRST_TIME_CONTRIBUTOR" }), + RULES + ) + ).toMatchObject({ run: true, association: "FIRST_TIME_CONTRIBUTOR" }); + }); + it("gates by association with an allowed-users override that skips the live check", () => { const outsider = pullRequestPayload({ author_association: "NONE", user: { login: "mallory", type: "User" } }); expect(decideTrigger("pull_request", outsider, RULES)).toMatchObject({ run: false }); @@ -1153,10 +1166,11 @@ describe("GitHub Action and workflow contracts", () => { it("forwards preflight and bot identity inputs through the composite action", () => { const raw = readFileSync(path.resolve("action.yml"), "utf8"); const action = parseYaml(raw) as { - inputs: Record; + inputs: Record; outputs: Record; runs: { steps: WorkflowStep[] }; }; + expect(action.inputs["allowed-associations"]?.default).toBe(DEFAULT_ALLOWED_ASSOCIATIONS.join(",")); expect(action.inputs["on-pull-request"]?.description).toContain("ready_for_review"); expect(action.inputs["bot-login"]).toBeDefined(); expect(action.inputs["preflight-only"]).toBeDefined();