From 9fe46588142fcb123d36e5d8d966d565d67a7c24 Mon Sep 17 00:00:00 2001 From: Pigbibi <199175422+Pigbibi@users.noreply.github.com> Date: Sat, 7 Nov 2026 22:24:41 +0800 Subject: [PATCH] feat: resolve the reviewer identity from protected variables Repositories whose content rules keep account identifiers out of public source can configure the reviewer through RISK_GATE_REVIEWER_APP_ID, RISK_GATE_REVIEWER_USER_ID and RISK_GATE_REVIEWER_LOGIN, matching the owner override. A partial set fails closed instead of falling back to the committed identity, and a policy may omit reviewer when the variables are set. --- pr-governance/README.md | 8 ++++++-- pr-governance/scripts/main.cjs | 19 +++++++++++++++++++ pr-governance/scripts/test-main.cjs | 24 ++++++++++++++++++++++++ pr-governance/templates/risk-gate.yml | 5 +++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/pr-governance/README.md b/pr-governance/README.md index b21cae3..fea5198 100644 --- a/pr-governance/README.md +++ b/pr-governance/README.md @@ -64,7 +64,10 @@ organization community-health files. `reviewer` and repository-specific `r2_paths`. Leave `owner` out: account identities belong in protected configuration, so the action reads `RISK_GATE_OWNER_ID` and `RISK_GATE_OWNER` from repository or organization - variables and replaces `owner` when they are set. + variables and replaces `owner` when they are set. A repository whose content + rules keep accounts out of public source may also leave `reviewer` out and + set `RISK_GATE_REVIEWER_APP_ID`, `RISK_GATE_REVIEWER_USER_ID` and + `RISK_GATE_REVIEWER_LOGIN` instead; a partial set fails closed. 2. Copy `templates/risk-gate.yml`, `templates/codex-nudge.yml`, `templates/gitleaks.yml` and `templates/weekly-secret-scan.yml` into `.github/workflows/`, replacing `__GOVERNANCE_REPO__` with the @@ -82,7 +85,8 @@ organization community-health files. The owner identity comes from the `RISK_GATE_OWNER_ID` and `RISK_GATE_OWNER` variables. A policy that still carries `owner` keeps working until the -variables are set, but no account identity is required in the file. +variables are set, but no account identity is required in the file. The +reviewer follows the same rule through the `RISK_GATE_REVIEWER_*` variables. ## Trust model diff --git a/pr-governance/scripts/main.cjs b/pr-governance/scripts/main.cjs index 22c3e1b..f1ba8ce 100644 --- a/pr-governance/scripts/main.cjs +++ b/pr-governance/scripts/main.cjs @@ -19,6 +19,23 @@ function configuredOwner() { return { user_id: id, login }; } +// The reviewer is the same kind of account identity, so a repository that keeps +// accounts out of public source can configure it the same way. A partial set +// fails closed instead of falling back to a committed identity. +function configuredReviewer() { + const appId = process.env.GOVERNANCE_REVIEWER_APP_ID; + const userId = process.env.GOVERNANCE_REVIEWER_USER_ID; + const login = process.env.GOVERNANCE_REVIEWER_LOGIN; + if (!appId && !userId && !login) return null; + const app = Number(appId); + const user = Number(userId); + if (!Number.isSafeInteger(app) || app < 1 || !Number.isSafeInteger(user) || user < 1 + || typeof login !== 'string' || !login) { + throw new Error('Incomplete reviewer configuration'); + } + return { provider: process.env.GOVERNANCE_REVIEWER_PROVIDER || 'codex', app_id: app, user_id: user, login }; +} + async function defaultBranch(github, context) { const known = context.payload?.repository?.default_branch || context.payload?.pull_request?.base?.ref; @@ -45,6 +62,8 @@ async function loadPolicy(github, context, branch) { } const owner = configuredOwner(); if (owner) policy.owner = owner; + const reviewer = configuredReviewer(); + if (reviewer) policy.reviewer = reviewer; gate.validatePolicy(policy); // Without an owner nobody can ever satisfy owner-approved, so a missing // configuration must fail the check instead of silently downgrading it. diff --git a/pr-governance/scripts/test-main.cjs b/pr-governance/scripts/test-main.cjs index ef16177..f71db46 100644 --- a/pr-governance/scripts/test-main.cjs +++ b/pr-governance/scripts/test-main.cjs @@ -36,6 +36,9 @@ async function load(policy, environment) { } finally { delete process.env.GOVERNANCE_OWNER_ID; delete process.env.GOVERNANCE_OWNER_LOGIN; + delete process.env.GOVERNANCE_REVIEWER_APP_ID; + delete process.env.GOVERNANCE_REVIEWER_USER_ID; + delete process.env.GOVERNANCE_REVIEWER_LOGIN; Object.assign(process.env, saved); } } @@ -57,6 +60,27 @@ test('a policy without owner and without variables fails closed', async () => { await assert.rejects(load(POLICY, {})); }); +const OWNER_ENV = { GOVERNANCE_OWNER_ID: '42', GOVERNANCE_OWNER_LOGIN: 'owner' }; +const REVIEWER_ENV = { ...OWNER_ENV, GOVERNANCE_REVIEWER_APP_ID: '7', + GOVERNANCE_REVIEWER_USER_ID: '8', GOVERNANCE_REVIEWER_LOGIN: 'reviewer[bot]' }; + +test('reviewer configuration replaces a committed reviewer', async () => { + const { policy } = await load(POLICY, REVIEWER_ENV); + assert.deepEqual(policy.reviewer, { provider: 'codex', app_id: 7, user_id: 8, login: 'reviewer[bot]' }); +}); + +test('a policy without reviewer is valid when the variables are set', async () => { + const { reviewer, ...rest } = POLICY; + const { policy } = await load(rest, REVIEWER_ENV); + assert.deepEqual(policy.reviewer, { provider: 'codex', app_id: 7, user_id: 8, login: 'reviewer[bot]' }); +}); + +test('an incomplete reviewer configuration fails closed', async () => { + await assert.rejects(load(POLICY, { GOVERNANCE_REVIEWER_APP_ID: '7' }), /reviewer configuration/); + await assert.rejects(load(POLICY, { ...REVIEWER_ENV, GOVERNANCE_REVIEWER_USER_ID: '0' }), + /reviewer configuration/); +}); + test('an incomplete owner configuration fails closed', async () => { await assert.rejects(load(POLICY, { GOVERNANCE_OWNER_ID: '42' }), /owner configuration/); await assert.rejects(load(POLICY, diff --git a/pr-governance/templates/risk-gate.yml b/pr-governance/templates/risk-gate.yml index 1c68191..8a1a7af 100644 --- a/pr-governance/templates/risk-gate.yml +++ b/pr-governance/templates/risk-gate.yml @@ -88,6 +88,11 @@ jobs: env: GOVERNANCE_OWNER_ID: ${{ vars.RISK_GATE_OWNER_ID }} GOVERNANCE_OWNER_LOGIN: ${{ vars.RISK_GATE_OWNER }} + # Optional: a repository that keeps account identifiers out of public + # source configures the reviewer here instead of in policy.json. + GOVERNANCE_REVIEWER_APP_ID: ${{ vars.RISK_GATE_REVIEWER_APP_ID }} + GOVERNANCE_REVIEWER_USER_ID: ${{ vars.RISK_GATE_REVIEWER_USER_ID }} + GOVERNANCE_REVIEWER_LOGIN: ${{ vars.RISK_GATE_REVIEWER_LOGIN }} with: mode: gate record-label: 'false'