mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 18:28:15 +01:00
Require the Plain title check before merging, and keep long PR bodies readable (GST-1825) (#4152)
* Keep reading an older receipt at the top of a PR body, and leave quoted receipt examples in place Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Put a receipt posted on its own inside the PR body's Agent notes, so the new one is the one read Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Read the PR body from the event file so a very long body cannot break the title check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make the Plain title check required before gstack merges a PR Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the PR's opening paragraph when a posted receipt moves into the Agent notes Review-Finding: v1/adversarial-native#1 reader=adversarial-native Review-Finding: v1/outside-voice#1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say exactly when an older receipt at the top of a PR body is still read The older receipt is read only while it still opens the body. A receipt quoted in the readable part is never read only when the body has an Agent notes section. The example policy now lists the Plain title check. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Add the changelog entry for the required Plain title check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
118207f0f7
commit
d479c5bf1e
10 files changed
+113
-32
No files matched your search
@@ -54,14 +54,14 @@ jobs:
|
||||
env:
|
||||
# Attacker-controlled on fork PRs: env only, never inlined into run:.
|
||||
PR_TITLE: ${{ github.event.pull_request.title }}
|
||||
PR_BODY: ${{ github.event.pull_request.body }}
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
PR_NUM: ${{ github.event.pull_request.number }}
|
||||
REPO: ${{ github.repository }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
printf '%s' "$PR_TITLE" > "$RUNNER_TEMP/pr-title.txt"
|
||||
printf '%s' "$PR_BODY" > "$RUNNER_TEMP/pr-body.txt"
|
||||
# The body comes from the event file: a large body overflows one environment string (E2BIG).
|
||||
jq -j '.pull_request.body // ""' "$GITHUB_EVENT_PATH" > "$RUNNER_TEMP/pr-body.txt"
|
||||
# Commit subjects are a warning only: skip quietly when the API read fails.
|
||||
if ! gh api --paginate "repos/$REPO/pulls/$PR_NUM/commits" \
|
||||
--jq '.[] | select((.parents | length) < 2) | .commit.message | split("\n")[0]' \
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
"pr_count": { "default_pr_count": 1, "split_requires_owner": true },
|
||||
"maintainers": ["garrytan"],
|
||||
"train": { "gate_workflow": "evals.yml", "free_workflow": "free-tests.yml", "full_gate_check": "gstack/full-gate", "record_artifact": "gstack-train-record", "max_size": 5, "fill_hours": 1, "screen_selection": "gstack-paid", "trial_reservation_usd": 1.5 },
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-smoke", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint"] },
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-smoke", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint", "Plain title"] },
|
||||
"merge": "direct",
|
||||
"land": {
|
||||
"merged_checks": [
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
- **gstack now waits for the "Plain title" check to pass before it lands a pull request.** The check reads the pull request's description from the event file GitHub hands the workflow, so a very long description no longer breaks it. A pull request that has never had the check run gets one when its title or description is edited, or when new commits are pushed.
|
||||
- **A ship receipt posted on its own now goes into the pull request's Agent notes section and keeps the opening paragraph where it was.** The new receipt replaces the one in that section, so it is the one gstack reads.
|
||||
- **An older receipt at the very top of a pull request description is still read after an Agent notes section without a receipt is added below it.** Receipt examples quoted in the readable part of a description stay where they are.
|
||||
- **Correction to the earlier entry about plain titles:** a repository without `.gstack/plain-titles.json` gets no title checks at all, not warnings.
|
||||
+15
-9
@@ -113,7 +113,7 @@ hard-code:
|
||||
"changelog": "CHANGELOG.md",
|
||||
"history": "squash-ok",
|
||||
"stamp_sensitive_gates": ["tests"],
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-smoke", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint"] },
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-smoke", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint", "Plain title"] },
|
||||
"min_gstack_version": "1.91.340.0"
|
||||
}
|
||||
```
|
||||
@@ -1257,8 +1257,13 @@ which carry the workflow consequences the skill prose would otherwise spell out:
|
||||
| `ship: changelog_fragments <dir>: …` | Step 12 writes no version; Step 13 writes the entry as `<dir>/<branch-slug>.md` (entry text only, `bump:` front matter only when the change warrants more than the policy bump), never an `[Unreleased]` section, linted with `gstack-prose-lint <file> --gate`; `gstack-ship-queue land` assigns the version; see [changelog fragments](#changelog-fragments) |
|
||||
|
||||
The ship receipt (`gstack-ship-receipt write … --body`) goes in the PR body's closing Agent notes
|
||||
section, written by the pr-body section of `/ship` after the body is composed; a body from before that
|
||||
layout keeps its receipt at the top and is still read.
|
||||
section, written by the pr-body section of `/ship` after the body is composed. A body from before that
|
||||
layout keeps its receipt at the top. That receipt is still read while it opens the body, also after the
|
||||
body gains an Agent notes section with no receipt in it; once an edit or a post puts an opening paragraph
|
||||
above it, it is no longer read, and the pull request needs a fresh receipt. A receipt posted on its own
|
||||
with `gstack-post pr-body` to a body that has the section replaces the receipt inside it. In a body with
|
||||
the section, a receipt block quoted in the readable part stays where it is and is never read as the
|
||||
receipt; a body without the section is read whole, so there a quoted block can be read as the receipt.
|
||||
|
||||
<a id="ship-receipt"></a>
|
||||
## The ship receipt
|
||||
@@ -2768,9 +2773,10 @@ Codes and fixes: [troubleshooting.md](troubleshooting.md#plain-title-refused).
|
||||
|
||||
### Making it a required check
|
||||
|
||||
The rollout takes two merges. The first ships the checker, the writers and the
|
||||
workflow. Once every open PR has a "Plain title" run (an edit to its title or
|
||||
body starts one), a one-line follow-up adds `"Plain title"` to
|
||||
`changelog_fragments.required_checks`, and `gstack-ship-queue land` then
|
||||
refuses a PR whose check is red or missing. Adding it in the first merge would
|
||||
hold every open PR that has never had a run.
|
||||
The rollout took two merges. The first shipped the checker, the writers and the
|
||||
workflow. The second added `"Plain title"` to gstack's
|
||||
`changelog_fragments.required_checks`, so `gstack-ship-queue land` refuses a
|
||||
PR whose check is red or missing. A PR opened before the workflow existed has no
|
||||
run until its title or body is edited or a commit is pushed; do one of those
|
||||
before landing it. The workflow reads the PR body from the event file, not an
|
||||
environment variable, so a very long body still reaches the checker.
|
||||
+17
-2
@@ -31,7 +31,7 @@ import { scan, type Finding, type RepoVisibility } from "./redact-engine";
|
||||
import { CI_RUN_WORD_BEFORE } from "./redact-patterns";
|
||||
import { agentNotesLines } from "./agent-notes";
|
||||
import { checkTitle, loadPlainTitlePolicy, PLAIN_TITLE_POLICY_PATH } from "./plain-title";
|
||||
import { RECEIPT_FENCE, extractReceipt } from "./ship-receipt";
|
||||
import { RECEIPT_FENCE, RUNS_FENCE, extractReceipt, placeReceipt } from "./ship-receipt";
|
||||
import { canonicalRemote } from "./remote-identity";
|
||||
import { parseChangelog } from "./changelog-check";
|
||||
import { versionWidth } from "./version-source";
|
||||
@@ -271,11 +271,26 @@ function splitSections(body: string): Section[] {
|
||||
* The Agent notes section, the current one or the file's, always ends the
|
||||
* merged body, above a Capy badge that ends it. Any other file (`## `
|
||||
* sections with no receipt above them, or no `## ` section) merges as
|
||||
* described above, so the text under the receipt is kept too.
|
||||
* described above, so the text under the receipt is kept too. A receipt the
|
||||
* file brings outside an Agent notes section, posted to a body that has one,
|
||||
* goes into that section through placeReceipt, so it replaces the receipt
|
||||
* there instead of landing beside it while the old one still binds.
|
||||
*/
|
||||
export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } {
|
||||
const now = splitSections(current);
|
||||
const next = splitSections(incoming).filter(n => !(n.key.startsWith("text:") && n.text.trim() === ""));
|
||||
const lone = next.find(n => n.key === "receipt");
|
||||
const intoNotes = lone && now.some(s => s.key === NOTES_KEY) && !next.some(n => n.key === NOTES_KEY) ? extractReceipt(lone.text).receipt : undefined;
|
||||
if (!intoNotes) return mergeSplit(now, next, incoming);
|
||||
const after = next[next.indexOf(lone!) + 1];
|
||||
const rest = next.filter(n => n !== lone).map(n => n !== after || !n.key.startsWith("text:") ? n : { ...n, key: "text:after-receipt", text: n.text.replace(RUNS_BLOCK_RE, "") });
|
||||
const m = mergeSplit(now, rest.filter(n => !(n.key.startsWith("text:") && n.text.trim() === "")), incoming);
|
||||
return { ...m, body: placeReceipt(m.body, intoNotes), replaced: [...m.replaced, "ship receipt"] };
|
||||
}
|
||||
|
||||
const RUNS_BLOCK_RE = new RegExp("^```" + RUNS_FENCE + "[ \\t]*\\r?\\n[\\s\\S]*?\\r?\\n```[ \\t]*(?:\\r?\\n|$)", "m");
|
||||
|
||||
function mergeSplit(now: Section[], next: Section[], incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } {
|
||||
const ambiguous = next.filter(n => now.filter(s => s.key === n.key).length > 1).map(n => `"${n.label}"`);
|
||||
if (ambiguous.length) throw new UsageError(`the PR's current body holds ${[...new Set(ambiguous)].join(", ")} more than once, so the file's copy matches no single section; nothing was posted. Rerun with --replace and the whole body`);
|
||||
const preamble = (list: Section[]) => {
|
||||
|
||||
+21
-11
@@ -45,14 +45,23 @@ export function renderReceiptBlock(receipt: ShipReceipt): string {
|
||||
}
|
||||
|
||||
const BLOCK_RE = /```gstack-ship-receipt[ \t]*\r?\n([\s\S]*?)\r?\n```/;
|
||||
/** A legacy receipt that opens the body, with the runs block right after it, as /ship posted before the Agent notes section. */
|
||||
const LEGACY_TOP_RE = /^\s*```gstack-ship-receipt[ \t]*\r?\n[\s\S]*?\r?\n```[ \t]*(?:\r?\n|$)(?:\s*```gstack-receipt-runs[ \t]*\r?\n[\s\S]*?\r?\n```[ \t]*(?:\r?\n|$))?/;
|
||||
|
||||
/**
|
||||
* The receipt block a PR body carries, parsed; `found: false` when absent. A body with an Agent notes section is read
|
||||
* there only, so a receipt fence quoted in the readable part never binds; a body without one (posted before the
|
||||
* section existed) gives its first block anywhere.
|
||||
* Where the receipt and its runs are read: the Agent notes section when it holds a receipt, else a legacy receipt that
|
||||
* opens the body (a section added later without one never hides it), else the section; a body without a section
|
||||
* gives the whole body. A receipt fence quoted in the readable part never binds.
|
||||
*/
|
||||
function receiptSource(body: string): string {
|
||||
const notes = splitAgentNotes(body).notes;
|
||||
if (notes === null || BLOCK_RE.test(notes)) return agentNotesOrBody(body);
|
||||
return LEGACY_TOP_RE.exec(body)?.[0] ?? notes;
|
||||
}
|
||||
|
||||
/** The receipt block a PR body carries (receiptSource), parsed; `found: false` when absent. */
|
||||
export function extractReceipt(body: string): { found: boolean; receipt?: ShipReceipt; raw?: string; errors: ValidationError[] } {
|
||||
const m = BLOCK_RE.exec(agentNotesOrBody(body));
|
||||
const m = BLOCK_RE.exec(receiptSource(body));
|
||||
if (!m) return { found: false, errors: [] };
|
||||
try {
|
||||
const parsed = JSON.parse(m[1]!);
|
||||
@@ -110,7 +119,7 @@ export const RECEIPT_RUNS_KEPT = 30;
|
||||
const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n```[ \t]*(?:\r?\n|$)/;
|
||||
export type ReceiptRun = Pick<ShipReceipt, 'head' | 'version' | 'gstack_version' | 'run' | 'review_loop' | 'review_loop_exception'> & { depth?: string; carried_from?: string };
|
||||
export function receiptRuns(body: string): ReceiptRun[] {
|
||||
const m = RUNS_RE.exec(agentNotesOrBody(body));
|
||||
const m = RUNS_RE.exec(receiptSource(body));
|
||||
return m ? m[1]!.split('\n').flatMap(l => { try { const o = JSON.parse(l); return o && typeof o.head === 'string' ? [o as ReceiptRun] : []; } catch { return []; } }) : [];
|
||||
}
|
||||
export function receiptRun(r: ShipReceipt): ReceiptRun {
|
||||
@@ -118,13 +127,14 @@ export function receiptRun(r: ShipReceipt): ReceiptRun {
|
||||
return JSON.parse(JSON.stringify({ head, version, gstack_version, run, review_loop, review_loop_exception, depth: r.review_depth?.depth, carried_from: r.carry?.from }));
|
||||
}
|
||||
|
||||
/** Every receipt and runs block with the blank lines after it, wherever it sits. */
|
||||
/** Every receipt and runs block with the blank lines after it. */
|
||||
const STRIP_RE = /```gstack-(?:ship-receipt|receipt-runs)[ \t]*\r?\n[\s\S]*?\r?\n```[ \t]*(?:\r?\n|$)(?:[ \t]*\r?\n)*/g;
|
||||
|
||||
/**
|
||||
* The receipt and its runs block go at the end of the body's Agent notes section, which is created when the body has
|
||||
* none; the section's other content stays above them. Every receipt and runs block elsewhere in the body (a quoted
|
||||
* example, a legacy block) is removed, so one receipt remains. A replaced receipt joins the runs block.
|
||||
* none; the section's other content stays above them. The section's old receipt and runs blocks and a legacy block
|
||||
* that opens the body are removed, so one receipt remains; a fence quoted in the readable part stays, since it never
|
||||
* binds. A replaced receipt joins the runs block.
|
||||
* A legacy body (a receipt and no section, as /ship posted before the section existed) keeps its layout: the block is
|
||||
* replaced in place at the top, because its Plan and issue lines sit in the readable part and a new section holding
|
||||
* only the receipt would hide them from the readers that prefer the section (a carry on an open PR, GST-1825).
|
||||
@@ -140,9 +150,9 @@ export function placeReceipt(body: string, receipt: ShipReceipt): string {
|
||||
const rest = body.replace(BLOCK_RE, '').replace(RUNS_RE, '').replace(/^\s+/, '');
|
||||
return block + (rest ? '\n' + rest : '');
|
||||
}
|
||||
const stripped = body.replace(STRIP_RE, '');
|
||||
const notes = (splitAgentNotes(stripped).notes ?? '').trim();
|
||||
return withAgentNotes(stripped.replace(/^\s+/, ''), (notes ? notes + '\n\n' : '') + block);
|
||||
const rest = body.replace(LEGACY_TOP_RE, '');
|
||||
const notes = (splitAgentNotes(rest).notes ?? '').replace(STRIP_RE, '').trim();
|
||||
return withAgentNotes(rest.replace(/^\s+/, ''), (notes ? notes + '\n\n' : '') + block);
|
||||
}
|
||||
|
||||
/** The receipt's `issues` object from a gstack-issue-links --out or --json file (its other keys are preview-only). */
|
||||
|
||||
@@ -70,12 +70,13 @@ describe('placeReceipt: the receipt lives in the Agent notes section', () => {
|
||||
expect(placeReceipt(withLines, receipt(HEAD))).toBe(withLines);
|
||||
});
|
||||
|
||||
test('a new receipt replaces the one in the notes, the old one joins the runs block there, and a quoted example fence in the readable part is removed', () => {
|
||||
test('a new receipt replaces the one in the notes, the old one joins the runs block there, and a quoted example fence in the readable part stays', () => {
|
||||
const example = 'Here is what the record looks like:\n\n' + renderReceiptBlock(receipt(OTHER, 1)) + '\nMore prose.\n';
|
||||
const first = placeReceipt(example + '\n' + notes('Fixes #14'), receipt(HEAD));
|
||||
const second = placeReceipt(first, receipt(OTHER));
|
||||
expect(second.match(/```gstack-ship-receipt/g)).toHaveLength(1);
|
||||
expect(splitAgentNotes(second).readable).toBe('Here is what the record looks like:\n\nMore prose.\n\n');
|
||||
expect(second.match(/```gstack-ship-receipt/g)).toHaveLength(2);
|
||||
expect(splitAgentNotes(second).readable).toBe(example.trimEnd() + '\n\n');
|
||||
expect(splitAgentNotes(second).notes!.match(/```gstack-ship-receipt/g)).toHaveLength(1);
|
||||
expect(extractReceipt(second).receipt!.head).toBe(OTHER);
|
||||
expect(receiptRuns(second).map(r => r.head)).toEqual([HEAD]);
|
||||
expect(splitAgentNotes(second).notes).toStartWith('Fixes #14\n\n```gstack-ship-receipt');
|
||||
@@ -93,6 +94,18 @@ describe('placeReceipt: the receipt lives in the Agent notes section', () => {
|
||||
});
|
||||
|
||||
describe('which receipt binds', () => {
|
||||
test('a legacy receipt at the top still binds when the body gained an Agent notes section without one, and placing a new receipt moves it and its runs into the notes', () => {
|
||||
const legacy = renderReceiptBlock(receipt(OTHER)) + '\n```gstack-receipt-runs\n' + JSON.stringify({ head: 'c'.repeat(40) }) + '\n```\n\nOpening.\n\n## Summary\n\nx\n\n' + notes('Fixes #14');
|
||||
expect(bindReceipt(legacy, '7', [OTHER])).toMatchObject({ ok: true, head: OTHER });
|
||||
expect(receiptRuns(legacy).map(r => r.head)).toEqual(['c'.repeat(40)]);
|
||||
const placed = placeReceipt(legacy, receipt(HEAD));
|
||||
expect(placed).toStartWith('Opening.');
|
||||
expect(placed.match(/```gstack-ship-receipt/g)).toHaveLength(1);
|
||||
expect(extractReceipt(placed).receipt!.head).toBe(HEAD);
|
||||
expect(receiptRuns(placed).map(r => r.head)).toEqual(['c'.repeat(40), OTHER]);
|
||||
expect(splitAgentNotes(placed).notes).toStartWith('Fixes #14\n\n```gstack-ship-receipt');
|
||||
});
|
||||
|
||||
test('(a) a legacy receipt at the top of a body with no section binds', () => {
|
||||
const body = renderReceiptBlock(receipt(HEAD)) + '\nOpening.\n\n## Summary\n\nx\n';
|
||||
expect(bindReceipt(body, '7', [HEAD])).toMatchObject({ ok: true, head: HEAD });
|
||||
|
||||
@@ -11,6 +11,7 @@ import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { EXIT, attributionCallout, mergeBodySections, runPost, type PostEnv } from "../lib/gstack-post";
|
||||
import { extractReceipt, receiptRuns } from "../lib/ship-receipt";
|
||||
|
||||
interface Call { cmd: string; args: string[]; input?: string }
|
||||
|
||||
@@ -807,6 +808,32 @@ describe("gstack-post keeps the Agent notes section last", () => {
|
||||
// Value: protects=a PR posted before the notes section (receipt at the top) still merges: a partial post keeps the legacy receipt where it is, and a
|
||||
// full post with notes last replaces it, so one receipt remains and it is the notes' one; fails_when=the legacy receipt survives a full post (two
|
||||
// receipts, the stale one first) or a partial post drops it; why_new=the legacy layout meets the new one; seam=none; no_claim=the gh round trip
|
||||
// Value: protects=a receipt posted on its own to a body whose Agent notes hold one replaces that receipt, so the new one binds and the old one
|
||||
// joins the runs block; fails_when=the posted receipt lands outside the notes while the old one stays authoritative; why_new=review finding
|
||||
// gstack-post.ts:212; seam=none (mergeBodySections); no_claim=the gh round trip
|
||||
test("a receipt posted on its own replaces the one in the notes, so the new receipt binds", () => {
|
||||
const m = mergeBodySections(composed("aaaa", "old"), RECEIPT("bbbb"));
|
||||
expect(extractReceipt(m.body).receipt!.head).toBe("bbbb");
|
||||
expect(m.body.match(/```gstack-ship-receipt/g)).toHaveLength(1);
|
||||
expect(receiptRuns(m.body).map(r => r.head)).toEqual(["aaaa"]);
|
||||
expect(m.body).toStartWith("Opening sentence.\n\n## Summary\n\nold\n\n## Test plan\n\n- [x] bun test\n\n<!-- gstack:agent-notes -->");
|
||||
expect(m.replaced).toContain("ship receipt");
|
||||
});
|
||||
|
||||
// Value: protects=the PR's own opening paragraph and run history when a receipt posted with text or a runs block after it goes into the notes;
|
||||
// fails_when=the text after the receipt replaces the opening, or the file's runs block lands in the readable part; why_new=review finding
|
||||
// gstack-post.ts:283 (adversarial-native#1, outside-voice#1); seam=none (mergeBodySections); no_claim=the gh round trip
|
||||
test("a receipt posted with text and a runs block after it keeps the opening paragraph and the notes' run history", () => {
|
||||
const runs = "\`\`\`gstack-receipt-runs\n" + JSON.stringify({ head: "cccc" }) + "\n\`\`\`\n";
|
||||
const m = mergeBodySections(composed("aaaa", "old"), RECEIPT("bbbb") + runs + "\nNote under receipt.\n");
|
||||
expect(m.body).toStartWith("Opening sentence.\n\n");
|
||||
expect(m.replaced).not.toContain("leading text");
|
||||
expect(m.body).toContain("Note under receipt.");
|
||||
expect(extractReceipt(m.body).receipt!.head).toBe("bbbb");
|
||||
expect(m.body.match(/```gstack-receipt-runs/g)).toHaveLength(1);
|
||||
expect(receiptRuns(m.body).map(r => r.head)).toEqual(["aaaa"]);
|
||||
});
|
||||
|
||||
test("a legacy receipt-at-top body still merges: a partial post keeps it, a full post with notes last replaces it", () => {
|
||||
const legacy = RECEIPT("aaaa") + "\n## Summary\n\nold\n\n## Approvals\n\nPlan: exempt: docs-only\n";
|
||||
const partial = mergeBodySections(legacy, "## Summary\n\nnew\n");
|
||||
|
||||
@@ -183,7 +183,7 @@ describe("gstack's own .gstack/ship-policy.json", () => {
|
||||
for (const f of fs.readdirSync(path.join(root, '.github/workflows')).filter(f => f.endsWith('.yml'))) {
|
||||
const text = fs.readFileSync(path.join(root, '.github/workflows', f), 'utf8');
|
||||
const on = text.slice(text.search(/^on:/m), text.search(/^jobs:/m));
|
||||
if (!/^\s+pull_request:/m.test(on) || /^\s+paths(-ignore)?:/m.test(on)) continue;
|
||||
if (!/^\s+pull_request(?:_target)?:/m.test(on) || /^\s+paths(-ignore)?:/m.test(on)) continue;
|
||||
const body = text.slice(text.search(/^jobs:/m));
|
||||
for (const m of body.matchAll(/^ ([A-Za-z0-9_-]+):\n((?: .*\n|\s*\n)*)/gm)) {
|
||||
const name = /^ name:\s*(.+)$/m.exec(m[2]!)?.[1]?.trim();
|
||||
|
||||
@@ -57,16 +57,22 @@ describe('plain-title.yml pull_request_target safety', () => {
|
||||
expect(runBlockLines(planted).filter(l => /\$\{\{\s*github\.event\.pull_request/.test(l.text)).length).toBe(2);
|
||||
});
|
||||
|
||||
test('PR text reaches files through env: and printf, and is never echoed', () => {
|
||||
test('PR text reaches files through env: and printf, or straight from the event file, and is never echoed', () => {
|
||||
expect(content).toMatch(/PR_TITLE: \$\{\{ github\.event\.pull_request\.title \}\}/);
|
||||
expect(content).toMatch(/PR_BODY: \$\{\{ github\.event\.pull_request\.body \}\}/);
|
||||
expect(content).toMatch(/printf '%s' "\$PR_TITLE" >/);
|
||||
expect(content).toMatch(/printf '%s' "\$PR_BODY" >/);
|
||||
const run = runBlockLines(content).map(l => l.text).join('\n');
|
||||
expect(run).not.toMatch(/\becho\b[^\n]*\$\{?PR_/);
|
||||
expect(run).not.toMatch(/\bcat\b/);
|
||||
});
|
||||
|
||||
// Value: protects=a PR body larger than one environment string (about 128 KiB) still reaches the checker, so a good
|
||||
// title never fails on E2BIG; fails_when=the body travels through env:; why_new=review finding plain-title.yml:58;
|
||||
// seam=the workflow text; no_claim=the runner's exact limit
|
||||
test('the body is read from the event file, never through env:', () => {
|
||||
expect(content).not.toMatch(/github\.event\.pull_request\.body/);
|
||||
expect(content).toMatch(/jq -j '\.pull_request\.body \/\/ ""' "\$GITHUB_EVENT_PATH" > "\$RUNNER_TEMP\/pr-body\.txt"/);
|
||||
});
|
||||
|
||||
test('permissions are read-only', () => {
|
||||
const perms = content.match(/^permissions:\n((?:\s+.+\n)+)/m)?.[1] ?? '';
|
||||
expect(perms).toMatch(/contents: read/);
|
||||
|
||||
Reference in new issue
Block a user