From bbe68873027ec8ed13b182d109175f39593568e0 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:50:21 +0200 Subject: [PATCH 1/8] chore: add contribution firewall --- .github/PULL_REQUEST_TEMPLATE.md | 34 ++- .github/scripts/pr-admission.cjs | 110 ++++++++ .github/scripts/pr-admission.test.cjs | 126 ++++++++++ .github/workflows/pr-admission.yml | 237 ++++++++++++++++++ .github/workflows/stale-author-prs.yml | 40 +++ .../plans/2026-08-02-contribution-firewall.md | 67 +++++ ...2026-08-02-contribution-firewall-design.md | 63 +++++ 7 files changed, 671 insertions(+), 6 deletions(-) create mode 100644 .github/scripts/pr-admission.cjs create mode 100644 .github/scripts/pr-admission.test.cjs create mode 100644 .github/workflows/pr-admission.yml create mode 100644 .github/workflows/stale-author-prs.yml create mode 100644 docs/superpowers/plans/2026-08-02-contribution-firewall.md create mode 100644 docs/superpowers/specs/2026-08-02-contribution-firewall-design.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e740a4d996..e64900fa01 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,13 +1,35 @@ ## Summary -- Explain the user-visible or maintainer-facing change. +Explain the user-visible or maintainer-facing change and why this approach is appropriate. + +## Linked issue + +Closes # + +Implementation pull requests must reference an issue labeled `approved-for-work`. Documentation-only and maintainer-owned integration changes are exempt. ## Verification -- List the commands or checks you ran. +List the exact commands or checks you ran and their results. Do not write only "tested" or "CI". + +```text +bun run typecheck +bun run test +``` + +## Regression coverage + +Name the test that fails without this change and passes with it. If automated coverage is genuinely impossible, explain why and describe the manual evidence. + +## Screenshots or recordings + +Required for user-visible dashboard changes. Remove this section when it does not apply. -## Checklist +## Author responsibility -- [ ] Scope stays focused and avoids unrelated cleanup. -- [ ] Docs or release notes were updated when needed. -- [ ] Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. +- [ ] I reviewed every changed line and can explain the implementation. +- [ ] I ran the validation commands listed above. +- [ ] Behavior changes include focused regression coverage, or I explained why automated coverage is impossible. +- [ ] The pull request contains no unrelated cleanup, generated churn, or accidental lockfile changes. +- [ ] I checked automated-review findings critically instead of applying them blindly. +- [ ] I will remain available to resolve CI failures and review feedback. diff --git a/.github/scripts/pr-admission.cjs b/.github/scripts/pr-admission.cjs new file mode 100644 index 0000000000..090d5a8e39 --- /dev/null +++ b/.github/scripts/pr-admission.cjs @@ -0,0 +1,110 @@ +"use strict"; + +const IMPLEMENTATION_PREFIXES = [ + "src/", + "gui/", + "scripts/", + "tests/", + "bin/", + "packages/", +]; + +const IMPLEMENTATION_FILES = new Set([ + "package.json", + "bun.lock", + "tsconfig.json", +]); + +const REQUIRED_ATTESTATIONS = [ + "I reviewed every changed line and can explain the implementation.", + "I ran the validation commands listed above.", + "Behavior changes include focused regression coverage, or I explained why automated coverage is impossible.", + "The pull request contains no unrelated cleanup, generated churn, or accidental lockfile changes.", + "I checked automated-review findings critically instead of applying them blindly.", + "I will remain available to resolve CI failures and review feedback.", +]; + +function normalizeCheckboxLabel(value) { + return value.trim().replace(/\s+/g, " "); +} + +function checkedAttestations(body) { + const checked = new Set(); + const text = typeof body === "string" ? body : ""; + for (const match of text.matchAll(/^\s*[-*+]\s+\[[xX]\]\s+(.+?)\s*$/gm)) { + checked.add(normalizeCheckboxLabel(match[1])); + } + return checked; +} + +function missingAttestations(body) { + const checked = checkedAttestations(body); + return REQUIRED_ATTESTATIONS.filter((label) => !checked.has(label)); +} + +function extractLinkedIssueNumbers(body) { + const text = typeof body === "string" ? body : ""; + const numbers = new Set(); + + for (const match of text.matchAll( + /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?|issue)\s*:?\s*#(\d+)\b/gi, + )) { + numbers.add(Number(match[1])); + } + + return [...numbers]; +} + +function isImplementationPath(path) { + if (IMPLEMENTATION_FILES.has(path)) return true; + return IMPLEMENTATION_PREFIXES.some((prefix) => path.startsWith(prefix)); +} + +function needsApprovedIssue(changedFiles) { + return changedFiles.some(isImplementationPath); +} + +function issueIsApproved(issue) { + return issue.labels.some((label) => { + const name = typeof label === "string" ? label : label?.name; + return name === "approved-for-work"; + }); +} + +function assessAdmission({ + body, + changedFiles, + linkedIssues, + authorHasPushPermission = false, +}) { + const failures = []; + const missing = missingAttestations(body); + + if (missing.length > 0) { + failures.push({ code: "missing_attestations", missing }); + } + + if (needsApprovedIssue(changedFiles) && !authorHasPushPermission) { + if (linkedIssues.length === 0) { + failures.push({ code: "missing_issue" }); + } else if (!linkedIssues.some(issueIsApproved)) { + failures.push({ + code: "issue_not_approved", + issues: linkedIssues.map((issue) => issue.number), + }); + } + } + + return failures; +} + +module.exports = { + REQUIRED_ATTESTATIONS, + assessAdmission, + checkedAttestations, + extractLinkedIssueNumbers, + isImplementationPath, + issueIsApproved, + missingAttestations, + needsApprovedIssue, +}; diff --git a/.github/scripts/pr-admission.test.cjs b/.github/scripts/pr-admission.test.cjs new file mode 100644 index 0000000000..da9093b966 --- /dev/null +++ b/.github/scripts/pr-admission.test.cjs @@ -0,0 +1,126 @@ +"use strict"; + +const { describe, it } = require("node:test"); +const assert = require("node:assert/strict"); +const { + REQUIRED_ATTESTATIONS, + assessAdmission, + extractLinkedIssueNumbers, + missingAttestations, + needsApprovedIssue, +} = require("./pr-admission.cjs"); + +function completeBody(extra = "") { + return [ + "## Summary", + "A complete explanation of the change and why it is needed.", + "", + "## Linked issue", + "Closes #123", + "", + "## Author responsibility", + ...REQUIRED_ATTESTATIONS.map((label) => `- [x] ${label}`), + "", + extra, + ].join("\n"); +} + +describe("missingAttestations", () => { + it("rejects unchecked and missing author responsibility items", () => { + const body = [ + `- [x] ${REQUIRED_ATTESTATIONS[0]}`, + `- [ ] ${REQUIRED_ATTESTATIONS[1]}`, + ].join("\n"); + + assert.deepEqual( + missingAttestations(body), + REQUIRED_ATTESTATIONS.slice(1), + ); + }); + + it("accepts every required checked item", () => { + assert.deepEqual(missingAttestations(completeBody()), []); + }); +}); + +describe("extractLinkedIssueNumbers", () => { + it("recognizes closing and reference syntax without duplicates", () => { + assert.deepEqual( + extractLinkedIssueNumbers("Closes #12\nRefs: #34\nFixes #12"), + [12, 34], + ); + }); +}); + +describe("needsApprovedIssue", () => { + it("requires an approved issue for implementation paths", () => { + assert.equal(needsApprovedIssue(["src/router.ts"]), true); + assert.equal(needsApprovedIssue(["gui/src/App.tsx"]), true); + assert.equal(needsApprovedIssue(["package.json"]), true); + }); + + it("does not require one for documentation-only changes", () => { + assert.equal( + needsApprovedIssue(["README.md", "docs-site/src/content/docs/foo.md"]), + false, + ); + }); +}); + +describe("assessAdmission", () => { + it("rejects implementation PRs with no linked issue", () => { + const failures = assessAdmission({ + body: completeBody(), + changedFiles: ["src/router.ts"], + linkedIssues: [], + }); + + assert.deepEqual(failures, [{ code: "missing_issue" }]); + }); + + it("rejects linked issues that are not approved for work", () => { + const failures = assessAdmission({ + body: completeBody(), + changedFiles: ["src/router.ts"], + linkedIssues: [{ number: 123, labels: ["bug"] }], + }); + + assert.deepEqual(failures, [ + { code: "issue_not_approved", issues: [123] }, + ]); + }); + + it("accepts an approved implementation issue", () => { + const failures = assessAdmission({ + body: completeBody(), + changedFiles: ["src/router.ts"], + linkedIssues: [ + { number: 123, labels: [{ name: "approved-for-work" }] }, + ], + }); + + assert.deepEqual(failures, []); + }); + + it("allows maintainers to perform integration work without an issue", () => { + const failures = assessAdmission({ + body: completeBody(), + changedFiles: ["src/router.ts"], + linkedIssues: [], + authorHasPushPermission: true, + }); + + assert.deepEqual(failures, []); + }); + + it("still requires maintainer attestations", () => { + const failures = assessAdmission({ + body: "", + changedFiles: ["src/router.ts"], + linkedIssues: [], + authorHasPushPermission: true, + }); + + assert.equal(failures[0].code, "missing_attestations"); + }); +}); diff --git a/.github/workflows/pr-admission.yml b/.github/workflows/pr-admission.yml new file mode 100644 index 0000000000..36c2468ec0 --- /dev/null +++ b/.github/workflows/pr-admission.yml @@ -0,0 +1,237 @@ +name: PR admission + +on: + pull_request_target: + types: [opened, reopened, edited, synchronize, ready_for_review] + +# This workflow runs trusted default-branch code only. It never checks out or +# executes the pull request head. +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: pr-admission-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + admission: + runs-on: ubuntu-latest + steps: + - name: Checkout trusted admission script + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + sparse-checkout: .github/scripts + + - name: Validate review readiness + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 + with: + script: | + const path = require("node:path"); + const { + assessAdmission, + extractLinkedIssueNumbers, + } = require( + path.join(process.cwd(), ".github", "scripts", "pr-admission.cjs"), + ); + + const { owner, repo } = context.repo; + const pull_number = context.payload.pull_request.number; + const marker = ""; + + const { data: pr } = await github.rest.pulls.get({ + owner, + repo, + pull_number, + }); + + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, + repo, + pull_number, + per_page: 100, + }); + + let permission = "read"; + try { + const response = + await github.rest.repos.getCollaboratorPermissionLevel({ + owner, + repo, + username: pr.user.login, + }); + permission = response.data.permission; + } catch (error) { + core.warning(`Permission lookup failed: ${error.message}`); + } + + const issueNumbers = extractLinkedIssueNumbers(pr.body); + const linkedIssues = []; + for (const issue_number of issueNumbers) { + try { + const { data: issue } = await github.rest.issues.get({ + owner, + repo, + issue_number, + }); + if (!issue.pull_request) { + linkedIssues.push({ + number: issue.number, + labels: issue.labels, + }); + } + } catch (error) { + core.warning( + `Could not load linked issue #${issue_number}: ${error.message}`, + ); + } + } + + const failures = assessAdmission({ + body: pr.body, + changedFiles: files.map((file) => file.filename), + linkedIssues, + authorHasPushPermission: + permission === "admin" || + permission === "maintain" || + permission === "write", + }); + + async function ensureLabel(name, color, description) { + try { + await github.rest.issues.getLabel({ owner, repo, name }); + } catch (error) { + if (error.status !== 404) throw error; + try { + await github.rest.issues.createLabel({ + owner, + repo, + name, + color, + description, + }); + } catch (createError) { + if (createError.status !== 422) throw createError; + } + } + } + + await ensureLabel( + "awaiting-author", + "d93f0b", + "Author action is required before maintainer review", + ); + await ensureLabel( + "intake: admitted", + "0e8a16", + "PR passed the automated contribution intake gate", + ); + + const currentLabels = new Set( + pr.labels.map((label) => label.name), + ); + + async function addLabel(name) { + if (currentLabels.has(name)) return; + await github.rest.issues.addLabels({ + owner, + repo, + issue_number: pull_number, + labels: [name], + }); + currentLabels.add(name); + } + + async function removeLabel(name) { + if (!currentLabels.has(name)) return; + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: pull_number, + name, + }); + currentLabels.delete(name); + } + + const comments = await github.paginate( + github.rest.issues.listComments, + { owner, repo, issue_number: pull_number, per_page: 100 }, + ); + const existing = comments.find( + (comment) => + comment.user?.login === "github-actions[bot]" && + comment.body?.includes(marker), + ); + + async function upsert(body) { + if (existing) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner, + repo, + issue_number: pull_number, + body, + }); + } + } + + if (failures.length === 0) { + await removeLabel("awaiting-author"); + await addLabel("intake: admitted"); + if (existing) { + await upsert( + `${marker}\n\n✅ **Contribution intake passed.**\n\n` + + "The PR is eligible for CI and maintainer review. Passing intake is not approval.", + ); + } + return; + } + + await removeLabel("intake: admitted"); + await addLabel("awaiting-author"); + + const sections = failures.map((failure) => { + if (failure.code === "missing_attestations") { + return [ + "### Author responsibility", + "Check every required item in the PR template after actually completing it:", + ...failure.missing.map((item) => `- [ ] ${item}`), + ].join("\n"); + } + if (failure.code === "missing_issue") { + return [ + "### Approved issue required", + "This implementation PR does not reference an issue.", + "Link one with `Closes #123` or `Refs #123`. The issue must carry `approved-for-work` before external implementation begins.", + ].join("\n"); + } + return [ + "### Linked issue is not approved", + `None of the linked issues (${failure.issues.map((n) => `#${n}`).join(", ")}) carries \`approved-for-work\`.`, + "Ask a maintainer to confirm the scope before continuing implementation.", + ].join("\n"); + }); + + await upsert( + [ + marker, + "", + "⚠️ **This PR is not ready for maintainer review.**", + "", + ...sections.flatMap((section) => [section, ""]), + "The `awaiting-author` label remains until these requirements pass. PRs left in that state for five inactive days may be closed automatically.", + ].join("\n"), + ); + + core.setFailed( + `PR admission failed: ${failures.map((f) => f.code).join(", ")}`, + ); diff --git a/.github/workflows/stale-author-prs.yml b/.github/workflows/stale-author-prs.yml new file mode 100644 index 0000000000..10e74ed31f --- /dev/null +++ b/.github/workflows/stale-author-prs.yml @@ -0,0 +1,40 @@ +name: Close abandoned author-action PRs + +# Scheduled workflows run from the repository default branch. Landing this on +# dev does not activate it until the workflow is promoted to main. +on: + schedule: + - cron: "35 6 * * *" + +permissions: + issues: write + pull-requests: write + +concurrency: + group: stale-author-prs + cancel-in-progress: false + +jobs: + stale: + runs-on: ubuntu-latest + steps: + - name: Warn and close inactive author-action PRs + uses: actions/stale@1e223db275d687790206a7acac4d1a11bd6fe629 # v10.4.0 + with: + only-pr-labels: awaiting-author + days-before-issue-stale: -1 + days-before-issue-close: -1 + days-before-pr-stale: 3 + days-before-pr-close: 2 + stale-pr-label: stale + remove-pr-stale-when-updated: true + ascending: true + operations-per-run: 60 + stale-pr-message: | + This pull request has been waiting on author action for three inactive days. + + Resolve the failing admission or CI checks, answer review feedback, and push the required fixes. It will close after two more inactive days. Any meaningful update resets the timer. + close-pr-message: | + Closing after five inactive days in `awaiting-author`. + + Reopen this PR, or open a clean replacement, after the blocking checks and feedback are resolved. Maintainers are not expected to repair contributor branches. diff --git a/docs/superpowers/plans/2026-08-02-contribution-firewall.md b/docs/superpowers/plans/2026-08-02-contribution-firewall.md new file mode 100644 index 0000000000..36afe04747 --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-contribution-firewall.md @@ -0,0 +1,67 @@ +# Contribution Firewall Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Reject implementation pull requests that lack agreed scope or explicit author ownership, and close abandoned author-action PRs after five inactive days. + +**Architecture:** A pure CommonJS validator classifies changed paths, parses linked issues, and validates required author attestations. A trusted `pull_request_target` workflow performs GitHub API lookups and synchronizes admission labels and one bot comment. A separate default-branch scheduled workflow handles inactivity. + +**Tech Stack:** GitHub Actions, `actions/github-script`, Node CommonJS, `node:test`. + +## Global Constraints + +- Never check out or execute pull-request head code from `pull_request_target`. +- Pin third-party actions to immutable full SHAs. +- External implementation PRs require an `approved-for-work` issue. +- Maintainers bypass only the approved-issue requirement, not attestations. +- Warn after three inactive days in `awaiting-author`; close after two more. +- Passing intake is not approval. + +--- + +### Task 1: Pure admission validator + +**Files:** +- Create: `.github/scripts/pr-admission.cjs` +- Create: `.github/scripts/pr-admission.test.cjs` + +- [x] Write tests for checked attestations, linked-issue parsing, implementation-path classification, approved issue behavior, and maintainer bypass. +- [x] Implement the smallest pure validator that passes those tests. +- [x] Run `node --test .github/scripts/pr-admission.test.cjs`. + +### Task 2: Trusted PR intake workflow + +**Files:** +- Create: `.github/workflows/pr-admission.yml` + +- [x] Check out only trusted default-branch scripts. +- [x] Read live PR files, body, author permission, and linked issue labels through GitHub APIs. +- [x] Apply `awaiting-author` on failure and `intake: admitted` on success. +- [x] Upsert one actionable bot comment. +- [x] Fail the `admission` job when requirements are not met. + +### Task 3: Author-action staleness + +**Files:** +- Create: `.github/workflows/stale-author-prs.yml` + +- [x] Limit processing to PRs labeled `awaiting-author`. +- [x] Warn after three inactive days. +- [x] Close after two additional inactive days. +- [x] Leave maintainer-blocked PRs untouched. + +### Task 4: Submission contract and design record + +**Files:** +- Modify: `.github/PULL_REQUEST_TEMPLATE.md` +- Create: `docs/superpowers/specs/2026-08-02-contribution-firewall-design.md` +- Create: `docs/superpowers/plans/2026-08-02-contribution-firewall.md` + +- [x] Add linked-issue, verification, regression-evidence, UI-evidence, and author-responsibility sections. +- [x] Record security boundaries, exemptions, non-goals, and staged rollout. + +### Verification + +- [x] `node --test .github/scripts/pr-admission.test.cjs` +- [ ] GitHub Actions for the exact PR commit. +- [ ] Synthetic external fork PR after workflow promotion to the default branch. diff --git a/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md b/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md new file mode 100644 index 0000000000..9d6285700b --- /dev/null +++ b/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md @@ -0,0 +1,63 @@ +# Contribution firewall — Design + +**Date:** 2026-08-02 +**Status:** Draft for maintainer review +**Target branch:** `dev` + +## Problem + +Low-effort implementation pull requests currently transfer the cost of validation, debugging, and repeated automated-review repair to maintainers. Opening a PR is cheap; proving it is reviewable is expensive. The repository already rejects wrong targets, suspicious ancestry, and empty or malformed descriptions, but it does not yet require authors to demonstrate ownership of the implementation or begin from an agreed scope. + +## Decision + +Add a second intake layer that evaluates outcomes rather than attempting to detect AI-generated code. + +1. Implementation PRs from contributors without repository push permission must reference an issue labeled `approved-for-work`. +2. Every PR author must check a concrete responsibility attestation. +3. Failed intake applies `awaiting-author`; passed intake applies `intake: admitted`. +4. A scheduled workflow warns after three inactive days in `awaiting-author` and closes after two more. +5. Passing intake means only that the PR may consume CI and maintainer attention. It is not approval and does not replace review. +6. Maintainers may perform integration or urgent repair work without an approved issue, but they do not bypass the author attestation. + +## Scope classification + +An approved issue is required when any changed path is under `src/`, `gui/`, `scripts/`, `tests/`, `bin/`, or `packages/`, or when the PR changes `package.json`, `bun.lock`, or `tsconfig.json`. + +Documentation-only and repository-policy changes remain exempt so typo fixes and governance work do not require ceremonial issues. + +## Security model + +`pr-admission.yml` uses `pull_request_target`, checks out only `.github/scripts` from the repository default branch, and never executes the PR head. It reads file metadata, PR text, linked issues, labels, and collaborator permission through GitHub APIs. It receives only the minimum write permissions needed to maintain PR labels and one bot comment. + +The scheduled stale workflow is default-branch-only and uses an immutable action SHA. + +## Author responsibility contract + +The PR template requires the author to attest that they: + +- reviewed and understand every changed line; +- ran the listed validation; +- supplied regression coverage or explained why it is impossible; +- removed unrelated cleanup and accidental generated churn; +- evaluated automated-review findings critically; +- will remain available for CI and review feedback. + +The gate verifies that the checkboxes are checked. It cannot prove the claims are true; false attestation is a review and trust signal, not something automation can solve. + +## Deliberate non-goals + +- AI-origin detection. +- Automatically deciding whether tests are semantically adequate. +- Counting commits as a quality metric. +- Automatically marking a PR `awaiting-maintainer` before all repository CI is green. +- Enabling branch rulesets in this PR. +- Closing PRs that are waiting on maintainers rather than authors. + +## Rollout + +1. Review this draft and tune the scope paths, labels, copy, and inactivity window. +2. Merge to `dev`. +3. Promote the trusted workflows and scripts to the default branch. +4. Create or confirm the `approved-for-work` label. +5. Add `PR admission / admission` to required checks only after a synthetic fork PR proves the full failure and recovery path. +6. Measure intake failures, reopen rate, review rounds, and maintainer time for two weeks before tightening further. From ed09e079679805fd4f112c45b3c70c7ec038ebec Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:01:54 +0200 Subject: [PATCH 2/8] refactor(ci): remove unused test helper parameter --- .github/scripts/pr-admission.test.cjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/scripts/pr-admission.test.cjs b/.github/scripts/pr-admission.test.cjs index da9093b966..eca4824f0e 100644 --- a/.github/scripts/pr-admission.test.cjs +++ b/.github/scripts/pr-admission.test.cjs @@ -10,7 +10,7 @@ const { needsApprovedIssue, } = require("./pr-admission.cjs"); -function completeBody(extra = "") { +function completeBody() { return [ "## Summary", "A complete explanation of the change and why it is needed.", @@ -21,7 +21,6 @@ function completeBody(extra = "") { "## Author responsibility", ...REQUIRED_ATTESTATIONS.map((label) => `- [x] ${label}`), "", - extra, ].join("\n"); } From 4a59642dc55443d189205d342232fccdef0f46b4 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:02:10 +0200 Subject: [PATCH 3/8] refactor(ci): reuse shared push-permission helper in admission gate --- .github/workflows/pr-admission.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-admission.yml b/.github/workflows/pr-admission.yml index 36c2468ec0..cebba3d52b 100644 --- a/.github/workflows/pr-admission.yml +++ b/.github/workflows/pr-admission.yml @@ -37,6 +37,9 @@ jobs: } = require( path.join(process.cwd(), ".github", "scripts", "pr-admission.cjs"), ); + const { authorHasPushPermission } = require( + path.join(process.cwd(), ".github", "scripts", "pr-quality.cjs"), + ); const { owner, repo } = context.repo; const pull_number = context.payload.pull_request.number; @@ -94,10 +97,7 @@ jobs: body: pr.body, changedFiles: files.map((file) => file.filename), linkedIssues, - authorHasPushPermission: - permission === "admin" || - permission === "maintain" || - permission === "write", + authorHasPushPermission: authorHasPushPermission(permission), }); async function ensureLabel(name, color, description) { From 4d8a9d047a8327b5b71e2e7887897bd2f692ba27 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:09:25 +0200 Subject: [PATCH 4/8] fix(ci): reject closed approved issues and classify renamed and bunfig files --- .github/scripts/pr-admission.cjs | 2 ++ .github/scripts/pr-admission.test.cjs | 41 +++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/.github/scripts/pr-admission.cjs b/.github/scripts/pr-admission.cjs index 090d5a8e39..4f0f78b6c9 100644 --- a/.github/scripts/pr-admission.cjs +++ b/.github/scripts/pr-admission.cjs @@ -12,6 +12,7 @@ const IMPLEMENTATION_PREFIXES = [ const IMPLEMENTATION_FILES = new Set([ "package.json", "bun.lock", + "bunfig.toml", "tsconfig.json", ]); @@ -65,6 +66,7 @@ function needsApprovedIssue(changedFiles) { } function issueIsApproved(issue) { + if (issue.state !== "open") return false; return issue.labels.some((label) => { const name = typeof label === "string" ? label : label?.name; return name === "approved-for-work"; diff --git a/.github/scripts/pr-admission.test.cjs b/.github/scripts/pr-admission.test.cjs index eca4824f0e..748f60c8f4 100644 --- a/.github/scripts/pr-admission.test.cjs +++ b/.github/scripts/pr-admission.test.cjs @@ -1,5 +1,7 @@ "use strict"; +const fs = require("node:fs"); +const path = require("node:path"); const { describe, it } = require("node:test"); const assert = require("node:assert/strict"); const { @@ -10,6 +12,10 @@ const { needsApprovedIssue, } = require("./pr-admission.cjs"); +function escapeRegExp(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + function completeBody() { return [ "## Summary", @@ -56,6 +62,7 @@ describe("needsApprovedIssue", () => { assert.equal(needsApprovedIssue(["src/router.ts"]), true); assert.equal(needsApprovedIssue(["gui/src/App.tsx"]), true); assert.equal(needsApprovedIssue(["package.json"]), true); + assert.equal(needsApprovedIssue(["bunfig.toml"]), true); }); it("does not require one for documentation-only changes", () => { @@ -81,7 +88,7 @@ describe("assessAdmission", () => { const failures = assessAdmission({ body: completeBody(), changedFiles: ["src/router.ts"], - linkedIssues: [{ number: 123, labels: ["bug"] }], + linkedIssues: [{ number: 123, labels: ["bug"], state: "open" }], }); assert.deepEqual(failures, [ @@ -94,13 +101,27 @@ describe("assessAdmission", () => { body: completeBody(), changedFiles: ["src/router.ts"], linkedIssues: [ - { number: 123, labels: [{ name: "approved-for-work" }] }, + { number: 123, labels: [{ name: "approved-for-work" }], state: "open" }, ], }); assert.deepEqual(failures, []); }); + it("rejects closed issues even when they carry the approval label", () => { + const failures = assessAdmission({ + body: completeBody(), + changedFiles: ["src/router.ts"], + linkedIssues: [ + { number: 123, labels: [{ name: "approved-for-work" }], state: "closed" }, + ], + }); + + assert.deepEqual(failures, [ + { code: "issue_not_approved", issues: [123] }, + ]); + }); + it("allows maintainers to perform integration work without an issue", () => { const failures = assessAdmission({ body: completeBody(), @@ -123,3 +144,19 @@ describe("assessAdmission", () => { assert.equal(failures[0].code, "missing_attestations"); }); }); + +describe("template parity", () => { + it("keeps the PR template attestations in sync with REQUIRED_ATTESTATIONS", () => { + const template = fs.readFileSync( + path.join(__dirname, "..", "PULL_REQUEST_TEMPLATE.md"), + "utf8", + ); + for (const label of REQUIRED_ATTESTATIONS) { + const pattern = new RegExp( + `^\\s*[-*+]\\s+\\[[ xX]\\]\\s+${escapeRegExp(label)}\\s*$`, + "m", + ); + assert.match(template, pattern); + } + }); +}); From 9e9db374d7fd538ad4ef0fe96314b33392fa157a Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:09:26 +0200 Subject: [PATCH 5/8] fix(ci): bound issue lookups, fail closed on lookup errors, support manual re-run --- .github/workflows/pr-admission.yml | 68 ++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-admission.yml b/.github/workflows/pr-admission.yml index cebba3d52b..55e95115b8 100644 --- a/.github/workflows/pr-admission.yml +++ b/.github/workflows/pr-admission.yml @@ -3,21 +3,29 @@ name: PR admission on: pull_request_target: types: [opened, reopened, edited, synchronize, ready_for_review] + workflow_dispatch: + inputs: + pull_request_number: + description: "PR number to re-evaluate" + required: true # This workflow runs trusted default-branch code only. It never checks out or # executes the pull request head. -permissions: - contents: read - issues: write - pull-requests: write +# Least privilege: no default permissions; the admission job grants only what it needs. +permissions: {} concurrency: - group: pr-admission-${{ github.event.pull_request.number }} + group: pr-admission-${{ github.event.pull_request.number || github.event.inputs.pull_request_number }} cancel-in-progress: true jobs: admission: runs-on: ubuntu-latest + # Needed to read PR metadata/files and to maintain intake labels plus one bot comment. + permissions: + contents: read + issues: write + pull-requests: write steps: - name: Checkout trusted admission script uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 @@ -40,11 +48,37 @@ jobs: const { authorHasPushPermission } = require( path.join(process.cwd(), ".github", "scripts", "pr-quality.cjs"), ); + const { rejectsWorkflowDispatchNonDefaultBranch } = require( + path.join(process.cwd(), ".github", "scripts", "issue-quality.cjs"), + ); const { owner, repo } = context.repo; - const pull_number = context.payload.pull_request.number; const marker = ""; + const nonDefaultBranchFailure = rejectsWorkflowDispatchNonDefaultBranch( + context.eventName, + context.ref, + context.payload.repository?.default_branch, + ); + if (nonDefaultBranchFailure) { + core.setFailed(nonDefaultBranchFailure); + return; + } + + let pull_number; + if (context.eventName === "workflow_dispatch") { + const parsed = Number(context.payload.inputs?.pull_request_number); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + core.setFailed( + `Invalid workflow_dispatch pull_request_number: ${context.payload.inputs?.pull_request_number}`, + ); + return; + } + pull_number = parsed; + } else { + pull_number = context.payload.pull_request.number; + } + const { data: pr } = await github.rest.pulls.get({ owner, repo, @@ -71,9 +105,15 @@ jobs: core.warning(`Permission lookup failed: ${error.message}`); } + const MAX_LINKED_ISSUE_LOOKUPS = 10; const issueNumbers = extractLinkedIssueNumbers(pr.body); + if (issueNumbers.length > MAX_LINKED_ISSUE_LOOKUPS) { + core.warning( + `Truncating linked-issue lookups to the first ${MAX_LINKED_ISSUE_LOOKUPS} of ${issueNumbers.length} references.`, + ); + } const linkedIssues = []; - for (const issue_number of issueNumbers) { + for (const issue_number of issueNumbers.slice(0, MAX_LINKED_ISSUE_LOOKUPS)) { try { const { data: issue } = await github.rest.issues.get({ owner, @@ -84,18 +124,28 @@ jobs: linkedIssues.push({ number: issue.number, labels: issue.labels, + state: issue.state, }); } } catch (error) { - core.warning( + if (error.status === 404) { + core.warning( + `Linked issue #${issue_number} does not exist; skipping.`, + ); + continue; + } + core.setFailed( `Could not load linked issue #${issue_number}: ${error.message}`, ); + return; } } const failures = assessAdmission({ body: pr.body, - changedFiles: files.map((file) => file.filename), + changedFiles: files.flatMap((file) => + [file.filename, file.previous_filename].filter(Boolean), + ), linkedIssues, authorHasPushPermission: authorHasPushPermission(permission), }); From 78817c560ed316b6cb6d572b3366722c40077027 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:09:27 +0200 Subject: [PATCH 6/8] fix(ci): isolate stale label and state for author-action PRs --- .github/workflows/stale-author-prs.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/stale-author-prs.yml b/.github/workflows/stale-author-prs.yml index 10e74ed31f..6b036c76d5 100644 --- a/.github/workflows/stale-author-prs.yml +++ b/.github/workflows/stale-author-prs.yml @@ -7,6 +7,9 @@ on: - cron: "35 6 * * *" permissions: + # actions/stale persists processed state through the Actions cache, and PR + # labels/comments/closure go through the shared issues API on PR numbers. + actions: write issues: write pull-requests: write @@ -26,7 +29,10 @@ jobs: days-before-issue-close: -1 days-before-pr-stale: 3 days-before-pr-close: 2 - stale-pr-label: stale + stale-pr-label: awaiting-author-stale + # Never un-stale issues: this workflow owns PR labels only, and the + # issue stale workflow manages the shared `stale` label for issues. + remove-issue-stale-when-updated: false remove-pr-stale-when-updated: true ascending: true operations-per-run: 60 From 7684973f9c9841d30bfba80b23fbd84500fe82ef Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:09:28 +0200 Subject: [PATCH 7/8] docs: sync admission design record with hardened gate --- .../specs/2026-08-02-contribution-firewall-design.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md b/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md index 9d6285700b..f565b23401 100644 --- a/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md +++ b/docs/superpowers/specs/2026-08-02-contribution-firewall-design.md @@ -12,7 +12,7 @@ Low-effort implementation pull requests currently transfer the cost of validatio Add a second intake layer that evaluates outcomes rather than attempting to detect AI-generated code. -1. Implementation PRs from contributors without repository push permission must reference an issue labeled `approved-for-work`. +1. Implementation PRs from contributors without repository push permission must reference an open issue labeled `approved-for-work`. 2. Every PR author must check a concrete responsibility attestation. 3. Failed intake applies `awaiting-author`; passed intake applies `intake: admitted`. 4. A scheduled workflow warns after three inactive days in `awaiting-author` and closes after two more. @@ -21,7 +21,7 @@ Add a second intake layer that evaluates outcomes rather than attempting to dete ## Scope classification -An approved issue is required when any changed path is under `src/`, `gui/`, `scripts/`, `tests/`, `bin/`, or `packages/`, or when the PR changes `package.json`, `bun.lock`, or `tsconfig.json`. +An approved issue is required when any changed path is under `src/`, `gui/`, `scripts/`, `tests/`, `bin/`, or `packages/`, or when the PR changes `package.json`, `bun.lock`, `bunfig.toml`, or `tsconfig.json`. Documentation-only and repository-policy changes remain exempt so typo fixes and governance work do not require ceremonial issues. @@ -31,6 +31,8 @@ Documentation-only and repository-policy changes remain exempt so typo fixes and The scheduled stale workflow is default-branch-only and uses an immutable action SHA. +Linked-issue lookups are capped so untrusted PR text cannot exhaust the API allowance, and transient lookup failures abort before admission labels are mutated. A manual `workflow_dispatch` re-run lets a maintainer re-evaluate a PR after its linked issue gains `approved-for-work`; dispatch is restricted to the repository default branch. + ## Author responsibility contract The PR template requires the author to attest that they: From df2b93470076afb9e63c5f76e906bb405d75db24 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 03:20:29 +0200 Subject: [PATCH 8/8] fix(ci): scope stale workflow permissions to the stale job --- .github/workflows/stale-author-prs.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/stale-author-prs.yml b/.github/workflows/stale-author-prs.yml index 6b036c76d5..6b6418e6de 100644 --- a/.github/workflows/stale-author-prs.yml +++ b/.github/workflows/stale-author-prs.yml @@ -6,12 +6,8 @@ on: schedule: - cron: "35 6 * * *" -permissions: - # actions/stale persists processed state through the Actions cache, and PR - # labels/comments/closure go through the shared issues API on PR numbers. - actions: write - issues: write - pull-requests: write +# Least privilege: no default permissions; the stale job grants only what it needs. +permissions: {} concurrency: group: stale-author-prs @@ -20,6 +16,12 @@ concurrency: jobs: stale: runs-on: ubuntu-latest + # actions/stale persists processed state through the Actions cache, and PR + # labels/comments/closure go through the shared issues API on PR numbers. + permissions: + actions: write + issues: write + pull-requests: write steps: - name: Warn and close inactive author-action PRs uses: actions/stale@1e223db275d687790206a7acac4d1a11bd6fe629 # v10.4.0