diff --git a/.changes/fork-pr-reviewer-router-identity.md b/.changes/fork-pr-reviewer-router-identity.md new file mode 100644 index 00000000..ae91d481 --- /dev/null +++ b/.changes/fork-pr-reviewer-router-identity.md @@ -0,0 +1,5 @@ +--- +category: Fixed +--- + +- **Fork pull-request admission** — keeps the read-only Reviewer Router identity check fail-closed while allowing external contributors' CI to use the reviewed public App slug when GitHub withholds repository variables. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31c4a2ee..f9216835 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -938,7 +938,15 @@ jobs: const pullNumber = context.payload.pull_request.number; const eventHeadSha = context.payload.pull_request.head.sha; const eventBaseSha = context.payload.pull_request.base.sha; - const appSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim(); + const configuredAppSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim(); + const reviewedForkAppSlug = 'dingtalk-dws-reviewer-router'; + const pullHeadRepository = + context.payload.pull_request.head.repo.full_name?.toLowerCase(); + const baseRepository = `${owner}/${repo}`.toLowerCase(); + const isForkPull = + Boolean(pullHeadRepository) && pullHeadRepository !== baseRepository; + const appSlug = + configuredAppSlug || (isForkPull ? reviewedForkAppSlug : ''); if ( !appSlug || appSlug !== appSlug.toLowerCase() || @@ -949,6 +957,11 @@ jobs: ); return; } + if (!configuredAppSlug) { + core.info( + `Fork pull request cannot read the repository App slug variable; using the reviewed public slug ${reviewedForkAppSlug}.`, + ); + } const expectedAppOwner = `${appSlug}[bot]`; const writerRulesetName = 'main-merge-writers'; const skipWorkflowPattern = diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 2730c99e..20500e12 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -184,8 +184,16 @@ ruleset before it can enable, reconcile, or synchronously merge. The read-only `Test` token may receive a repository projection with both merge-default properties omitted; it accepts only that complete omission or exact `MERGE_MESSAGE` plus -`PR_TITLE`/`BLANK`, while partial or malformed projections fail closed. The -minted App's `Contents: write` token must observe the exact reviewed defaults +`PR_TITLE`/`BLANK`, while partial or malformed projections fail closed. +The same unprivileged `pull_request` job may receive an empty repository-variable +projection for an external fork. Only when the event head repository differs +from the base repository does it substitute the exact reviewed public slug +`dingtalk-dws-reviewer-router` for identity comparison. An empty variable on a +same-repository PR and every malformed non-empty value still fail closed. This +fallback neither mints a token nor grants merge authority; the base-owned +Router continues to require its minted App slug to equal the repository +variable before any mutation. The minted App's `Contents: write` token must +observe the exact reviewed defaults before either mutation path proceeds. GitHub hides the complete `bypass_actors` list from low-privilege callers, so the rollout audit must still keep the writer list at exactly the Reviewer App, `haofeng0705` (ID diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go index 6a47d788..c2918544 100644 --- a/test/scripts/reviewer_router_workflow_test.go +++ b/test/scripts/reviewer_router_workflow_test.go @@ -724,7 +724,13 @@ func TestCodeAdmissionEnforcesReviewerRouterWriterBoundary(t *testing.T) { "name: Verify auto-merge identity", "if: github.event_name == 'pull_request' && github.event.pull_request.draft == false", "REVIEWER_ROUTER_APP_SLUG: ${{ vars.REVIEWER_ROUTER_APP_SLUG }}", - "process.env.REVIEWER_ROUTER_APP_SLUG?.trim()", + "const configuredAppSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim()", + "const reviewedForkAppSlug = 'dingtalk-dws-reviewer-router'", + "context.payload.pull_request.head.repo.full_name?.toLowerCase()", + "const baseRepository = `${owner}/${repo}`.toLowerCase()", + "Boolean(pullHeadRepository) && pullHeadRepository !== baseRepository", + "configuredAppSlug || (isForkPull ? reviewedForkAppSlug : '')", + "Fork pull request cannot read the repository App slug variable; using the reviewed public slug", "const expectedAppOwner = `${appSlug}[bot]`", "github.rest.pulls.get", "github.rest.repos.get",