From c6938c24e674dbaac8f9c388ccfaa3fd417700d4 Mon Sep 17 00:00:00 2001 From: Den Delimarsky Date: Tue, 24 Mar 2026 03:07:04 +0000 Subject: [PATCH] slash-commands: add /lgtm force for core/lead maintainers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a /lgtm force variant gated to force-allow-teams (default: core-maintainers,lead-maintainers). Bypasses the self-approval guard and the CODEOWNERS path — only team membership grants it. The bot review body records that force was used, and the result output is lgtm-forced (or force-unauthorized on denial). /hold force and /stageblog force are rejected as noop rather than silently treated as the base command. --- slash-commands/README.md | 13 +++++++++++-- slash-commands/action.yml | 14 +++++++++++--- slash-commands/main.js | 34 ++++++++++++++++++++++++++++++---- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/slash-commands/README.md b/slash-commands/README.md index d75de24..b2ea502 100644 --- a/slash-commands/README.md +++ b/slash-commands/README.md @@ -13,6 +13,11 @@ Listens for PR comments and push events, then: review with body "Approved on behalf of @user via `/lgtm`." - **`/lgtm cancel`** — removes the `approved` label and dismisses the bot review. +- **`/lgtm force`** — same as `/lgtm`, but **only** members of + `force-allow-teams` (default: core + lead maintainers) may use it, and it + **bypasses the self-approval guard**. Intended as an escape hatch for + maintainers who need to approve their own PRs. The bot review body notes + the approval came via `/lgtm force`. - **`/hold`** — adds a `do-not-merge/hold` label (same authorization rule). - **`/hold cancel`** or **`/unhold`** — removes the hold label. - **`/stageblog`** — if the PR touches any blog paths (default `blog/**`), @@ -189,6 +194,7 @@ jobs: | `approved-label` | | `approved` | Label added by `/lgtm` | | `hold-label` | | `do-not-merge/hold` | Label added by `/hold` | | `always-allow-teams` | | `core-maintainers` | Comma-separated team slugs (in the repo's org) whose members can `/lgtm` or `/hold` any PR regardless of CODEOWNERS | +| `force-allow-teams` | | `core-maintainers,lead-maintainers` | Comma-separated team slugs whose members may use `/lgtm force` (bypasses self-approval guard). CODEOWNERS does **not** grant this — only team membership. | | `codeowners-path` | | `.github/CODEOWNERS` | Path to CODEOWNERS, fetched from the PR's **base** ref (never head — tamper-proof) | | `invalidate-on-push` | | `'true'` | Remove `approved` label + dismiss bot review + comment when new commits are pushed. Set `'false'` to keep approval across pushes. | | `submit-review` | | `'true'` | Submit a bot APPROVE review alongside the label on `/lgtm`. Set `'false'` to use label only. | @@ -200,7 +206,7 @@ jobs: | Output | Description | |---|---| -| `result` | One of: `lgtm-added`, `lgtm-removed`, `hold-added`, `hold-removed`, `invalidated`, `unauthorized`, `self-lgtm-blocked`, `stageblog-dispatched`, `stageblog-not-blog`, `stageblog-disabled`, `noop` | +| `result` | One of: `lgtm-added`, `lgtm-forced`, `lgtm-removed`, `hold-added`, `hold-removed`, `invalidated`, `unauthorized`, `force-unauthorized`, `self-lgtm-blocked`, `stageblog-dispatched`, `stageblog-not-blog`, `stageblog-disabled`, `noop` | | `actor` | Login of the commenter (empty for non-comment triggers) | ## `slash-commands/status` sub-action @@ -265,7 +271,10 @@ Last-match-wins per file, consistent with GitHub's native CODEOWNERS semantics. - **Workflow modification** — `issue_comment` and `pull_request_target` run from the default-branch workflow definition. The action does not check out PR code. -- **Self-approval** — explicitly blocked before the auth check. +- **Self-approval** — explicitly blocked before the auth check. The + `/lgtm force` escape hatch bypasses this, but is gated strictly to + `force-allow-teams` membership (CODEOWNERS does not grant it) and the + review body records that `force` was used. - **Manual label bypass** — the commit status (the actual merge gate) is only ever set to `success` by the `/lgtm` handler after the auth check passes. The `status` sub-action sets `pending`/`failure` but never `success`, and the diff --git a/slash-commands/action.yml b/slash-commands/action.yml index 6ea2cce..9ca92a5 100644 --- a/slash-commands/action.yml +++ b/slash-commands/action.yml @@ -38,6 +38,13 @@ inputs: /lgtm or /hold any PR regardless of CODEOWNERS required: false default: core-maintainers + force-allow-teams: + description: >- + Comma-separated team slugs (within the repo's org) whose members may + use /lgtm force. This bypasses the self-approval guard and CODEOWNERS — + only membership in one of these teams grants it. + required: false + default: core-maintainers,lead-maintainers codeowners-path: description: Path to the CODEOWNERS file (fetched from the PR base branch) required: false @@ -79,9 +86,9 @@ inputs: outputs: result: description: >- - One of: lgtm-added, lgtm-removed, hold-added, hold-removed, invalidated, - unauthorized, self-lgtm-blocked, stageblog-dispatched, stageblog-not-blog, - stageblog-disabled, noop + One of: lgtm-added, lgtm-forced, lgtm-removed, hold-added, hold-removed, + invalidated, unauthorized, force-unauthorized, self-lgtm-blocked, + stageblog-dispatched, stageblog-not-blog, stageblog-disabled, noop value: ${{ steps.handle.outputs.result }} actor: description: Login of the user who triggered the action (commenter) @@ -97,6 +104,7 @@ runs: APPROVED_LABEL: ${{ inputs.approved-label }} HOLD_LABEL: ${{ inputs.hold-label }} ALWAYS_ALLOW_TEAMS: ${{ inputs.always-allow-teams }} + FORCE_ALLOW_TEAMS: ${{ inputs.force-allow-teams }} CODEOWNERS_PATH: ${{ inputs.codeowners-path }} INVALIDATE_ON_PUSH: ${{ inputs.invalidate-on-push }} SUBMIT_REVIEW: ${{ inputs.submit-review }} diff --git a/slash-commands/main.js b/slash-commands/main.js index 4ba3237..a6b3530 100644 --- a/slash-commands/main.js +++ b/slash-commands/main.js @@ -8,6 +8,8 @@ module.exports = async ({ github, context, core }) => { const holdLabel = process.env.HOLD_LABEL; const alwaysAllowTeams = process.env.ALWAYS_ALLOW_TEAMS .split(',').map(s => s.trim()).filter(Boolean); + const forceAllowTeams = process.env.FORCE_ALLOW_TEAMS + .split(',').map(s => s.trim()).filter(Boolean); const codeownersPath = process.env.CODEOWNERS_PATH; const invalidateOnPush = process.env.INVALIDATE_ON_PUSH === 'true'; const submitReview = process.env.SUBMIT_REVIEW === 'true'; @@ -185,10 +187,11 @@ module.exports = async ({ github, context, core }) => { owner, repo, comment_id: commentId, content, }); } - async function submitApproval(prNumber, onBehalfOf) { + async function submitApproval(prNumber, onBehalfOf, forced) { + const via = forced ? '/lgtm force' : '/lgtm'; await github.rest.pulls.createReview({ owner, repo, pull_number: prNumber, event: 'APPROVE', - body: `${reviewMarker}\nApproved on behalf of @${onBehalfOf} via \`/lgtm\`.`, + body: `${reviewMarker}\nApproved on behalf of @${onBehalfOf} via \`${via}\`.`, }); } async function dismissBotApprovals(prNumber, reason) { @@ -257,11 +260,16 @@ module.exports = async ({ github, context, core }) => { const commentId = context.payload.comment.id; const actor = context.payload.comment.user.login; const firstLine = commentBody.split('\n')[0].trim(); - const m = firstLine.match(/^\/(lgtm|hold|unhold|stageblog)\b\s*(cancel)?\s*$/i); + const m = firstLine.match(/^\/(lgtm|hold|unhold|stageblog)\b\s*(cancel|force)?\s*$/i); if (!m) return setResult('noop', actor); const cmd = m[1].toLowerCase(); - const cancel = !!m[2] || cmd === 'unhold'; + const arg = (m[2] || '').toLowerCase(); + const cancel = arg === 'cancel' || cmd === 'unhold'; + // `force` is only meaningful for /lgtm — reject `/hold force` etc. as noop + // rather than silently treating it as the base command. + if (arg === 'force' && cmd !== 'lgtm') return setResult('noop', actor); + const force = arg === 'force'; const prNumber = context.payload.issue.number; const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); @@ -271,6 +279,24 @@ module.exports = async ({ github, context, core }) => { const hadApproved = preLabels.has(approvedLabel); const hadHold = preLabels.has(holdLabel); + // /lgtm force — restricted to force-allow-teams. Bypasses the self-approval + // guard and the CODEOWNERS path entirely; only team membership grants this. + if (cmd === 'lgtm' && force) { + let allowed = false; + for (const team of forceAllowTeams) { + if (await isTeamMember(actor, team)) { allowed = true; break; } + } + if (!allowed) { + await react(commentId, '-1'); + return setResult('force-unauthorized', actor); + } + await addLabel(prNumber, approvedLabel); + if (submitReview) await submitApproval(prNumber, actor, true); + await react(commentId, '+1'); + await setCommitStatus(pr.head.sha, true, hadHold); + return setResult('lgtm-forced', actor); + } + // Self-approval guard (only for /lgtm, not /hold) if (cmd === 'lgtm' && !cancel && actor.toLowerCase() === pr.user.login.toLowerCase()) { await react(commentId, '-1');