Add slash-commands composite action for /lgtm and /hold

Prow-style PR approval via comment commands, gated by org team membership
or CODEOWNERS of the PR's changed files.

- /lgtm adds 'approved' label + bot APPROVE review; /lgtm cancel undoes both
- /hold adds 'do-not-merge/hold'; /hold cancel or /unhold removes it
- Self-approval is blocked; unauthorized attempts get a thumbs-down reaction only
- New commits pushed to a PR invalidate approval (label removed, review dismissed)
- CODEOWNERS is fetched from the PR base ref to prevent tampering

Requires a PAT or App token with read:org — default GITHUB_TOKEN cannot
check team membership.

Closes modelcontextprotocol/modelcontextprotocol#2199
This commit is contained in:
Den Delimarsky 2026-03-11 16:40:07 +00:00
commit d5df36515c
No known key found for this signature in database
3 changed files with 513 additions and 0 deletions

View file

@ -8,3 +8,4 @@ GitHub Actions relevant to the management of MCP repositories.
|---|---|
| [`cloudflare-pages-preview/deploy`](cloudflare-pages-preview/README.md) | Deploy a static HTML directory to Cloudflare Pages under a per-PR branch alias, inject noindex headers, and post a sticky PR comment with preview URLs |
| [`cloudflare-pages-preview/cleanup`](cloudflare-pages-preview/README.md) | Delete Cloudflare Pages deployments for a closed PR's branch alias and update the sticky comment |
| [`slash-commands`](slash-commands/README.md) | Handle `/lgtm` and `/hold` PR slash commands gated by team membership + CODEOWNERS; invalidate approval on push |

181
slash-commands/README.md Normal file
View file

@ -0,0 +1,181 @@
# slash-commands
Reusable composite action that implements Prow-style `/lgtm` and `/hold` slash
commands on pull requests, gated by team membership and CODEOWNERS.
## What it does
Listens for PR comments and push events, then:
- **`/lgtm`** — if the commenter is a core maintainer _or_ a CODEOWNER of the
PR's changed files, adds an `approved` label and submits a bot **APPROVE**
review with body "Approved on behalf of @user via `/lgtm`."
- **`/lgtm cancel`** — removes the `approved` label and dismisses the bot
review.
- **`/hold`** — adds a `do-not-merge/hold` label (same authorization rule).
- **`/hold cancel`** or **`/unhold`** — removes the hold label.
- **New commits pushed** — removes the `approved` label, dismisses the bot
review, and posts a brief comment asking for re-approval.
Commands are parsed from the **first line** of the comment (case-insensitive,
must match `^/cmd\b`). Unauthorized attempts, or a PR author trying to `/lgtm`
their own PR, receive a 👎 reaction with no further noise. Successful commands
receive a 👍 reaction.
The caller workflow adds a separate `status` job that reports a required check
which passes only when `approved` is present and `do-not-merge/hold` is absent
— making the labels an actual merge gate via branch protection.
## Prerequisites
- The `approved` and `do-not-merge/hold` labels **must already exist** in the
caller repo (the action does not auto-create them). Suggested colors:
`approved` → `#0e8a16`, `do-not-merge/hold` → `#d93f0b`.
- A CODEOWNERS file at `.github/CODEOWNERS` (or pass a different path via
`codeowners-path`). Without one, only `always-allow-teams` members can act.
## Required secrets
Create this secret in the caller repo: GitHub → repo → **Settings → Secrets
and variables → Actions → New repository secret**.
| Repo secret name | Action input | Value |
|---|---|---|
| `SLASH_COMMANDS_TOKEN` | `github-token` | Fine-grained PAT (or GitHub App installation token) with **Organization → Members: read** plus **Repository → Contents: read, Pull requests: write, Issues: write** |
> **The default `${{ github.token }}` will NOT work.** Team-membership checks
> (`GET /orgs/{org}/teams/{team}/memberships/{user}`) require `read:org` scope,
> which the automatic `GITHUB_TOKEN` never has — even with a `permissions:`
> block. You _must_ provide a PAT or App token.
### `SLASH_COMMANDS_TOKEN`
**Option A — fine-grained PAT (recommended for single-repo setups):**
GitHub → **Settings → Developer settings → Personal access tokens → Fine-grained
tokens → Generate new token**. Configure:
| Section | Setting |
|---|---|
| Resource owner | `modelcontextprotocol` (or your org) |
| Repository access | **Only select repositories** → pick the caller repo |
| Repository permissions → Contents | Read-only |
| Repository permissions → Pull requests | Read and write |
| Repository permissions → Issues | Read and write |
| Organization permissions → Members | Read-only |
Do not grant any write permissions beyond Pull requests + Issues. **Contents**
must remain read-only — the action never writes code.
**Option B — GitHub App installation token:**
If your org already uses a bot App, generate an installation token with the
same scopes and store it as `SLASH_COMMANDS_TOKEN`. The App must be installed
on the caller repo _and_ have org-level **Members: read**.
## Caller workflow requirements
| | |
|---|---|
| **Triggers** | `issue_comment` (types: `created`) + `pull_request_target` (types: `synchronize`, `opened`, `reopened`, `labeled`, `unlabeled`) |
| **Permissions** | `pull-requests: write`, `issues: write`, `contents: read` |
| **Fork-PR safety** | No special guard needed — `issue_comment` and `pull_request_target` both run in the **base** repo's context with the base workflow definition, so fork authors cannot modify the logic. CODEOWNERS is also fetched from the PR's **base** ref, never the head. |
| **No checkout** | The action calls GitHub API only. Do not `actions/checkout` PR code. |
## Usage
```yaml
name: Slash Commands
on:
issue_comment:
types: [created]
pull_request_target:
types: [synchronize, opened, reopened, labeled, unlabeled]
permissions:
pull-requests: write
issues: write
contents: read
jobs:
handle:
if: >-
(github.event_name == 'issue_comment' && github.event.issue.pull_request) ||
(github.event_name == 'pull_request_target' && github.event.action == 'synchronize')
runs-on: ubuntu-latest
steps:
- uses: modelcontextprotocol/actions/slash-commands@main
with:
github-token: ${{ secrets.SLASH_COMMANDS_TOKEN }}
always-allow-teams: core-maintainers,lead-maintainers
# Merge-gate status check — make this a required check in branch protection.
status:
if: github.event_name == 'pull_request_target'
runs-on: ubuntu-latest
steps:
- name: Require approval, block on hold
env:
LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
run: |
echo "Labels: $LABELS"
if ! jq -e 'contains(["approved"])' <<<"$LABELS" > /dev/null; then
echo "::error::Missing 'approved' label — comment /lgtm"
exit 1
fi
if jq -e 'contains(["do-not-merge/hold"])' <<<"$LABELS" > /dev/null; then
echo "::error::'do-not-merge/hold' present — resolve, then /unhold"
exit 1
fi
echo "approved and not held"
```
## Inputs
| Input | Required | Default | Description |
|---|---|---|---|
| `github-token` | ✅ | — | PAT or App token with `read:org` + repo `pull-requests: write` + `issues: write` + `contents: read`. **Default `GITHUB_TOKEN` will not work** for team checks. |
| `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 |
| `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. |
## Outputs
| Output | Description |
|---|---|
| `result` | One of: `lgtm-added`, `lgtm-removed`, `hold-added`, `hold-removed`, `invalidated`, `unauthorized`, `self-lgtm-blocked`, `noop` |
| `actor` | Login of the commenter (empty for non-comment triggers) |
## CODEOWNERS pattern support
The inline CODEOWNERS parser handles the patterns currently used in MCP repos:
| Pattern | Supported | Meaning |
|---|---|---|
| `/path/` | ✅ | Anchored directory prefix (matches anything under `path/`) |
| `/path/to/file.ext` | ✅ | Exact anchored path |
| `*.ext` | ✅ | Any file with extension `.ext` at any depth |
| `/path/**/*.ext` | ✅ | Glob at any depth under `/path/` |
| `**/file` | ✅ | File named `file` at any depth |
| `@user` / `@org/team` | ✅ | Owner: individual user or team (team org must match repo org) |
| `!negation` | ❌ | Not supported (not in use in MCP repos) |
| Escaped spaces (`\ `) | ❌ | Not supported |
Last-match-wins per file, consistent with GitHub's native CODEOWNERS semantics.
## Security notes
- **Comment body injection** — command parsing is a fixed regex on the
first line in JS; comment text is never interpolated into a shell.
- **CODEOWNERS tampering** — fetched from `pr.base.ref`, never the PR head.
An attacker cannot add themselves as CODEOWNER in the PR under review.
- **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.
- **Review provenance** — the bot review body always names the real approver
("on behalf of @login"), even though GitHub shows the review as bot-authored.

331
slash-commands/action.yml Normal file
View file

@ -0,0 +1,331 @@
name: PR Slash Commands
description: >-
Handles /lgtm and /hold slash commands on PRs. Authorized users
(core maintainers or CODEOWNERS of the PR's changed files) can approve via
/lgtm (adds a label + bot APPROVE review) or block via /hold. Approval is
invalidated when new commits are pushed.
inputs:
github-token:
description: >-
Token for API calls. MUST have read:org scope to check team membership —
the default GITHUB_TOKEN cannot do this. Use a fine-grained PAT with
Organization Members:read + Repository Contents:read + Pull requests:write
+ Issues:write, or a GitHub App installation token with equivalent perms.
required: true
approved-label:
description: Label added by /lgtm and used by the status job as the merge gate
required: false
default: approved
hold-label:
description: Label added by /hold to block merging
required: false
default: do-not-merge/hold
always-allow-teams:
description: >-
Comma-separated team slugs (within the repo's org) whose members may
/lgtm or /hold any PR regardless of CODEOWNERS
required: false
default: core-maintainers
codeowners-path:
description: Path to the CODEOWNERS file (fetched from the PR base branch)
required: false
default: .github/CODEOWNERS
invalidate-on-push:
description: >-
Remove the approved label and dismiss the bot review when new commits
are pushed to the PR. Set to 'false' to keep approval across pushes.
required: false
default: 'true'
submit-review:
description: >-
Whether to submit a bot APPROVE review alongside the label on /lgtm.
Set to 'false' to rely on the label alone.
required: false
default: 'true'
outputs:
result:
description: >-
One of: lgtm-added, lgtm-removed, hold-added, hold-removed, invalidated,
unauthorized, self-lgtm-blocked, noop
value: ${{ steps.handle.outputs.result }}
actor:
description: Login of the user who triggered the action (commenter)
value: ${{ steps.handle.outputs.actor }}
runs:
using: composite
steps:
- name: Handle slash command
id: handle
uses: actions/github-script@v8
env:
APPROVED_LABEL: ${{ inputs.approved-label }}
HOLD_LABEL: ${{ inputs.hold-label }}
ALWAYS_ALLOW_TEAMS: ${{ inputs.always-allow-teams }}
CODEOWNERS_PATH: ${{ inputs.codeowners-path }}
INVALIDATE_ON_PUSH: ${{ inputs.invalidate-on-push }}
SUBMIT_REVIEW: ${{ inputs.submit-review }}
with:
github-token: ${{ inputs.github-token }}
script: |
const { owner, repo } = context.repo;
const approvedLabel = process.env.APPROVED_LABEL;
const holdLabel = process.env.HOLD_LABEL;
const alwaysAllowTeams = process.env.ALWAYS_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';
const reviewMarker = '<!-- slash-commands-lgtm -->';
function setResult(result, actor) {
core.setOutput('result', result);
core.setOutput('actor', actor || '');
core.info(`Result: ${result}${actor ? ` (actor: ${actor})` : ''}`);
}
// --- CODEOWNERS pattern → regex ---------------------------------
// Supports: leading-/ anchoring, trailing-/ directory match,
// `**` (any depth), `*` (single segment), `?`. No `!` negation.
function patternToRegex(pattern) {
let p = pattern;
const anchored = p.startsWith('/');
if (anchored) p = p.slice(1);
const dirMatch = p.endsWith('/');
if (dirMatch) p = p.slice(0, -1);
// Escape regex meta chars except our glob tokens * and ?
let re = p.replace(/[.+^${}()|[\]\\]/g, '\\$&');
// Handle globs. Use placeholders so single-* replacement doesn't
// catch ** sequences. `**/` = zero or more directories.
re = re.replace(/\*\*\//g, '\u0001');
re = re.replace(/\*\*/g, '\u0000');
re = re.replace(/\*/g, '[^/]*');
re = re.replace(/\?/g, '[^/]');
re = re.replace(/\u0001/g, '(?:.*/)?');
re = re.replace(/\u0000/g, '.*');
// Anchoring: leading `/` → match from repo root; otherwise
// match at any path depth (start or after a `/`).
const prefix = anchored ? '^' : '(^|/)';
// Trailing `/` → directory prefix (match anything under it).
// Otherwise must match to end of string.
const suffix = dirMatch ? '/.*$' : '$';
return new RegExp(prefix + re + suffix);
}
function parseCodeowners(content) {
const rules = [];
for (const raw of content.split('\n')) {
const line = raw.replace(/#.*$/, '').trim();
if (!line) continue;
const parts = line.split(/\s+/);
const pattern = parts[0];
const owners = parts.slice(1);
if (!owners.length) continue;
rules.push({ regex: patternToRegex(pattern), owners });
}
return rules;
}
function ownersForFile(rules, file) {
// Last match wins.
let owners = null;
for (const r of rules) {
if (r.regex.test(file)) owners = r.owners;
}
return owners || [];
}
// --- Auth helpers -----------------------------------------------
const teamMembershipCache = new Map();
async function isTeamMember(user, teamSlug) {
const key = `${teamSlug}:${user}`;
if (teamMembershipCache.has(key)) return teamMembershipCache.get(key);
try {
const { data } = await github.rest.teams.getMembershipForUserInOrg({
org: owner, team_slug: teamSlug, username: user,
});
const ok = data.state === 'active';
teamMembershipCache.set(key, ok);
return ok;
} catch (e) {
if (e.status === 404) {
teamMembershipCache.set(key, false);
return false;
}
// 403 here almost always means the token lacks read:org.
if (e.status === 403) {
core.warning(`Team membership check for @${owner}/${teamSlug} returned 403 — the provided github-token likely lacks read:org (Organization Members:read) scope. Treating as not-a-member.`);
teamMembershipCache.set(key, false);
return false;
}
throw e;
}
}
async function isAuthorized(user, pr) {
// 1. always-allow teams
for (const team of alwaysAllowTeams) {
if (await isTeamMember(user, team)) {
core.info(`@${user} authorized via always-allow team @${owner}/${team}`);
return true;
}
}
// 2. CODEOWNERS of the PR's changed files (from BASE ref)
let content;
try {
const { data } = await github.rest.repos.getContent({
owner, repo, path: codeownersPath, ref: pr.base.ref,
});
content = Buffer.from(data.content, data.encoding).toString('utf8');
} catch (e) {
if (e.status === 404) {
core.info(`No CODEOWNERS found at ${codeownersPath} on ${pr.base.ref}`);
return false;
}
throw e;
}
const rules = parseCodeowners(content);
const files = await github.paginate(github.rest.pulls.listFiles, {
owner, repo, pull_number: pr.number, per_page: 100,
});
const ownerSet = new Set();
for (const f of files) {
for (const o of ownersForFile(rules, f.filename)) ownerSet.add(o);
}
core.info(`CODEOWNERS for this PR's files: ${[...ownerSet].join(', ') || '(none)'}`);
for (const o of ownerSet) {
if (o.includes('/')) {
// @org/team — we require the org to match our repo's org
const [oOrg, oTeam] = o.replace(/^@/, '').split('/');
if (oOrg.toLowerCase() !== owner.toLowerCase()) continue;
if (await isTeamMember(user, oTeam)) {
core.info(`@${user} authorized via CODEOWNER team ${o}`);
return true;
}
} else {
// @user
if (o.replace(/^@/, '').toLowerCase() === user.toLowerCase()) {
core.info(`@${user} authorized via CODEOWNER user entry`);
return true;
}
}
}
return false;
}
// --- Label & review helpers -------------------------------------
async function addLabel(prNumber, label) {
await github.rest.issues.addLabels({
owner, repo, issue_number: prNumber, labels: [label],
});
}
async function removeLabel(prNumber, label) {
try {
await github.rest.issues.removeLabel({
owner, repo, issue_number: prNumber, name: label,
});
} catch (e) {
if (e.status !== 404) throw e;
}
}
async function react(commentId, content) {
await github.rest.reactions.createForIssueComment({
owner, repo, comment_id: commentId, content,
});
}
async function submitApproval(prNumber, onBehalfOf) {
await github.rest.pulls.createReview({
owner, repo, pull_number: prNumber, event: 'APPROVE',
body: `${reviewMarker}\nApproved on behalf of @${onBehalfOf} via \`/lgtm\`.`,
});
}
async function dismissBotApprovals(prNumber, reason) {
const reviews = await github.paginate(github.rest.pulls.listReviews, {
owner, repo, pull_number: prNumber, per_page: 100,
});
for (const r of reviews) {
if (r.state !== 'APPROVED') continue;
if (!r.body || !r.body.includes(reviewMarker)) continue;
try {
await github.rest.pulls.dismissReview({
owner, repo, pull_number: prNumber, review_id: r.id,
message: reason,
});
} catch (e) {
core.warning(`Failed to dismiss review ${r.id}: ${e.message}`);
}
}
}
// --- Dispatch ---------------------------------------------------
const ev = context.eventName;
// Invalidation on push
if (ev === 'pull_request_target' && context.payload.action === 'synchronize') {
if (!invalidateOnPush) return setResult('noop');
const pr = context.payload.pull_request;
const hasApproved = pr.labels.some(l => l.name === approvedLabel);
if (!hasApproved) return setResult('noop');
await removeLabel(pr.number, approvedLabel);
await dismissBotApprovals(pr.number, 'New commits pushed — approval invalidated.');
await github.rest.issues.createComment({
owner, repo, issue_number: pr.number,
body: `New commits were pushed — removed the \`${approvedLabel}\` label. Re-approve with \`/lgtm\`.`,
});
return setResult('invalidated');
}
// Comment commands
if (ev !== 'issue_comment') return setResult('noop');
if (!context.payload.issue.pull_request) return setResult('noop');
const commentBody = context.payload.comment.body || '';
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)\b\s*(cancel)?\s*$/i);
if (!m) return setResult('noop', actor);
const cmd = m[1].toLowerCase();
const cancel = !!m[2] || cmd === 'unhold';
const prNumber = context.payload.issue.number;
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
if (pr.state !== 'open') return setResult('noop', actor);
// Self-approval guard (only for /lgtm, not /hold)
if (cmd === 'lgtm' && !cancel && actor.toLowerCase() === pr.user.login.toLowerCase()) {
await react(commentId, '-1');
return setResult('self-lgtm-blocked', actor);
}
// Auth check (same rule for all commands)
if (!(await isAuthorized(actor, pr))) {
await react(commentId, '-1');
return setResult('unauthorized', actor);
}
// Execute
if (cmd === 'lgtm' && !cancel) {
await addLabel(prNumber, approvedLabel);
if (submitReview) await submitApproval(prNumber, actor);
await react(commentId, '+1');
return setResult('lgtm-added', actor);
}
if (cmd === 'lgtm' && cancel) {
await removeLabel(prNumber, approvedLabel);
await dismissBotApprovals(prNumber, `@${actor} cancelled approval via /lgtm cancel.`);
await react(commentId, '+1');
return setResult('lgtm-removed', actor);
}
if ((cmd === 'hold' && !cancel)) {
await addLabel(prNumber, holdLabel);
await react(commentId, '+1');
return setResult('hold-added', actor);
}
// /hold cancel or /unhold
await removeLabel(prNumber, holdLabel);
await react(commentId, '+1');
return setResult('hold-removed', actor);