Compare commits

...

3 Commits

Author SHA1 Message Date
Gauvino
935cacff81 fix(pr-validation): paginate issue comments + guard unreadable body file
Addresses review: github.rest.issues.listComments only returns the first page,
so the sticky-comment marker could be missed on busy PRs — use github.paginate.
And guard readFileSync so a missing/unreadable body file exits 2 (per the doc)
instead of crashing without JSON.
2026-06-01 20:22:28 +02:00
Gauvino
5f59dce0c7 fix(pr-validation): run under pull_request_target + drop DoS-prone comment loop
Security audit fixes:
- The jobs gated on github.event_name == 'pull_request' but the trigger is
  pull_request_target, so they never ran (validation was silently disabled).
  Gate on 'pull_request_target'.
- Replace the loop-until-stable HTML-comment strip with a single linear pass
  (+ trailing-unterminated strip): still leaves no <!-- (CodeQL-clean) but
  removes the quadratic re-scan a crafted nested-comment body could abuse.
2026-06-01 20:14:24 +02:00
Gauvino
3de9b65b7d ci(pr-validation): validate PR title + body against the template
New .github/workflows/pr-validation.yml (pull_request_target, like seerr, so it
works on fork PRs without checking out fork code): moves the Conventional-Commits
title check out of the quality gate and adds a PR template check
(scripts/check-pr-template.mjs) — Description/Ticket/Testing filled, contribution
+ AI-disclosure boxes ticked (maintainers bypass AI), and Screenshots required
when the PR changes UI (.tsx under app/ or components/). Posts a sticky comment +
'blocked: template' label on failure, clears on success; skips bots + synchronize.
Robust comment stripping (CodeQL-safe). Inspired by seerr's pr-validation.
2026-06-01 17:24:03 +02:00
3 changed files with 255 additions and 32 deletions

View File

@@ -12,38 +12,6 @@ permissions:
contents: read
jobs:
validate_pr_title:
name: "📝 Validate PR Title"
if: github.event_name == 'pull_request'
runs-on: ubuntu-24.04
permissions:
pull-requests: write
contents: read
steps:
- uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6.1.1
id: lint_pr_title
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4
if: always() && (steps.lint_pr_title.outputs.error_message != null)
with:
header: pr-title-lint-error
message: |
Hey there and thank you for opening this pull request! 👋🏼
We require pull request titles to follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/).
**Error details:**
```
${{ steps.lint_pr_title.outputs.error_message }}
```
- if: ${{ steps.lint_pr_title.outputs.error_message == null }}
uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4
with:
header: pr-title-lint-error
delete: true
dependency-review:
name: 🔍 Vulnerable Dependencies
runs-on: ubuntu-24.04

136
.github/workflows/pr-validation.yml vendored Normal file
View File

@@ -0,0 +1,136 @@
name: 🚦 PR Validation
# Uses pull_request_target so the jobs get a write token even on fork PRs (to comment
# and label) — same as seerr. SECURITY: never check out or run the PR head's code here;
# we only read the title/body from the event payload and run our own scripts from the base.
on:
pull_request_target:
types: [opened, edited, synchronize, reopened]
workflow_dispatch:
permissions: {}
concurrency:
group: pr-validation-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
jobs:
validate_pr_title:
name: "📝 Validate PR Title"
if: github.event_name == 'pull_request_target'
runs-on: ubuntu-24.04
permissions:
pull-requests: write
contents: read
steps:
- uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6.1.1
id: lint_pr_title
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4
if: always() && (steps.lint_pr_title.outputs.error_message != null)
with:
header: pr-title-lint-error
message: |
Hey there and thank you for opening this pull request! 👋🏼
We require pull request titles to follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/).
**Error details:**
```
${{ steps.lint_pr_title.outputs.error_message }}
```
- if: ${{ steps.lint_pr_title.outputs.error_message == null }}
uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4
with:
header: pr-title-lint-error
delete: true
validate_pr_template:
name: "📋 Validate PR Template"
# Skip pushes to an existing PR (the body rarely changes) and bot-authored PRs.
if: >-
github.event_name == 'pull_request_target' &&
github.event.action != 'synchronize' &&
github.actor != 'renovate[bot]' &&
github.actor != 'github-actions[bot]'
runs-on: ubuntu-24.04
permissions:
pull-requests: write
issues: write
contents: read
steps:
- name: "📥 Checkout"
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: "🍞 Setup Bun"
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
with:
bun-version: latest
- name: "📝 Write PR body to file"
env:
PR_BODY: ${{ github.event.pull_request.body }}
run: printf '%s' "$PR_BODY" > /tmp/pr-body.txt
- name: "📂 List changed files"
env:
GH_TOKEN: ${{ github.token }}
run: |
gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" \
--paginate --jq '.[].filename' > /tmp/pr-files.txt
- name: "🔎 Validate body against template"
id: check
env:
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }}
PR_FILES: /tmp/pr-files.txt
run: |
set +e
bun scripts/check-pr-template.mjs /tmp/pr-body.txt > /tmp/pr-issues.json
echo "code=$?" >> "$GITHUB_OUTPUT"
- name: "💬 Report problems"
if: steps.check.outputs.code != '0'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v8.0.0
with:
script: |
const fs = require('fs');
let issues;
try { issues = JSON.parse(fs.readFileSync('/tmp/pr-issues.json', 'utf8')); }
catch { issues = ["The PR template check could not parse the description. Please make sure it follows the template."]; }
if (!Array.isArray(issues) || issues.length === 0) issues = ["The PR description does not follow the template."];
const body = [
"👋 Thanks for the PR! A few things in the description need attention before review:",
"",
...issues.map((i) => `- ${i}`),
"",
"Please update the PR description ([template](https://github.com/${{ github.repository }}/blob/develop/.github/pull_request_template.md)). This check re-runs when you edit it.",
].join("\n");
const { owner, repo } = context.repo;
const issue_number = context.payload.pull_request.number;
const marker = "<!-- pr-template-check -->";
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number });
const existing = comments.find((c) => c.body?.includes(marker));
const payload = `${marker}\n${body}`;
if (existing) await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: payload });
else await github.rest.issues.createComment({ owner, repo, issue_number, body: payload });
const label = "blocked: template";
try { await github.rest.issues.getLabel({ owner, repo, name: label }); }
catch { await github.rest.issues.createLabel({ owner, repo, name: label, color: "d93f0b", description: "PR description does not follow the template" }); }
await github.rest.issues.addLabels({ owner, repo, issue_number, labels: [label] });
core.setFailed(`PR template check failed:\n- ${issues.join("\n- ")}`);
- name: "✅ Clear problems on success"
if: steps.check.outputs.code == '0'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v8.0.0
with:
script: |
const { owner, repo } = context.repo;
const issue_number = context.payload.pull_request.number;
const marker = "<!-- pr-template-check -->";
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number });
const existing = comments.find((c) => c.body?.includes(marker));
if (existing) await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
try { await github.rest.issues.removeLabel({ owner, repo, issue_number, name: "blocked: template" }); } catch {}

View File

@@ -0,0 +1,119 @@
#!/usr/bin/env bun
/**
* Validates that a pull request body follows .github/pull_request_template.md:
* required sections are filled in and the key checklist items are ticked.
*
* Usage: bun scripts/check-pr-template.mjs <path-to-pr-body.txt>
* Output: a JSON array of human-readable problems (empty array = all good).
* Exit: 0 = ok, 1 = one or more problems, 2 = no body file given.
*
* Env: AUTHOR_ASSOCIATION — when OWNER/MEMBER/COLLABORATOR, the AI-disclosure
* check is skipped (maintainers self-police).
*/
import { existsSync, readFileSync } from "node:fs";
const bodyFile = process.argv[2];
if (!bodyFile) {
console.error("usage: bun scripts/check-pr-template.mjs <pr-body-file>");
process.exit(2);
}
let body;
try {
body = readFileSync(bodyFile, "utf8").replace(/\r\n/g, "\n");
} catch (e) {
console.error(`cannot read body file ${bodyFile}: ${e.message}`);
process.exit(2);
}
const association = (process.env.AUTHOR_ASSOCIATION || "").toUpperCase();
const isMaintainer = ["OWNER", "MEMBER", "COLLABORATOR"].includes(association);
// Strip HTML comments in a single linear pass: remove complete `<!-- … -->`
// blocks, then drop any leftover unterminated `<!-- …` to end-of-string. This
// leaves no `<!--` behind (satisfies CodeQL) without the quadratic re-scan loop
// a malicious deeply-nested body could abuse for CPU-DoS.
const stripComments = (s) =>
s
.replace(/<!--[\s\S]*?-->/g, "")
.replace(/<!--[\s\S]*$/, "")
.trim();
// Grab the text under a heading whose title contains `keyword`, up to the next heading
// or the end of the body.
const section = (keyword) => {
const re = new RegExp(
`(?:^|\\n)#{1,4}\\s*[^\\n]*${keyword}[^\\n]*\\n([\\s\\S]*?)(?=\\n#{1,4}\\s|$)`,
"i",
);
const m = body.match(re);
return m ? m[1] : null;
};
const isFilled = (content) => {
if (content == null) return false;
// Template guidance lives in HTML comments; once stripped, a real answer remains.
return stripComments(content).length > 0;
};
const issues = [];
if (section("Description") === null)
issues.push("The **Description** section is missing.");
else if (!isFilled(section("Description")))
issues.push(
"The **Description** section is empty — describe what changed and why.",
);
if (section("Ticket") === null)
issues.push("The **Ticket / Issue** section is missing.");
else if (!isFilled(section("Ticket")))
issues.push(
"The **Ticket / Issue** section is empty — link an issue or write `N/A`.",
);
if (section("Testing Instructions") === null)
issues.push("The **Testing Instructions** section is missing.");
else if (!isFilled(section("Testing Instructions")))
issues.push(
"The **Testing Instructions** section is empty — tell reviewers how to test this, or write `N/A`.",
);
const checklist = section("Checklist");
if (checklist === null) {
issues.push("The **Checklist** section is missing.");
} else {
if (!/- \[x\][^\n]*contribution guidelines/i.test(checklist))
issues.push(
"Please read and tick the **contribution guidelines** checklist item.",
);
if (!isMaintainer && !/- \[x\][^\n]*declared if AI/i.test(checklist))
issues.push(
"Please tick the **AI disclosure** checklist item (declare whether AI was used).",
);
}
// Require the Screenshots section when the PR changes UI (.tsx under app/ or components/).
// PR_FILES points to a newline list of changed paths (provided by the workflow).
const filesPath = process.env.PR_FILES;
if (filesPath && existsSync(filesPath)) {
const changed = readFileSync(filesPath, "utf8").split("\n").filter(Boolean);
const touchesUI = changed.some(
(f) =>
/^(app|components)\/.*\.tsx$/.test(f) && !/\.(test|spec)\.tsx$/.test(f),
);
if (touchesUI) {
const shots = section("Screenshots");
if (shots === null)
issues.push(
"This PR changes UI (`.tsx`) — add the **Screenshots / GIFs** section with before/after media.",
);
else if (!isFilled(shots))
issues.push(
"This PR changes UI — the **Screenshots / GIFs** section is empty; add screenshots (or write `N/A` if it's genuinely not visual).",
);
}
}
console.log(JSON.stringify(issues));
process.exit(issues.length ? 1 : 0);