From 35344f34fef574339be0944803ca3c8c3f4d0c16 Mon Sep 17 00:00:00 2001 From: bluebox timmy Date: Mon, 27 Jul 2026 02:33:38 -0400 Subject: [PATCH 1/3] ci: unblock docs-only PRs without weakening the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci.yml` carries `paths-ignore: ['**/*.md', '.myrobots/**', 'LICENSE']` so a prose-only change doesn't burn a ~25-minute pipeline. But ruleset 16042163 REQUIRES `typecheck + unit + ART + E2E` and `vrt-strict (visual regression — strict subset)`, and a path-skipped workflow reports NOTHING — GitHub treats a never-reported required check as pending forever. A docs-only PR therefore sits at mergeStateStatus=BLOCKED / mergeable=MERGEABLE with zero failures and can never auto-merge (#1184, armed since 06:06Z, permanently stuck). Adds `.github/workflows/docs-only-gate.yml`: fires on the EXACT INVERSE path filter (same list P — ci.yml `paths-ignore: P`, this `paths: P`) and posts the two required contexts as commit statuses, but only when TWO independent guards agree — every changed file is a doc (G1) AND GitHub started no ci.yml run for this head SHA (G2). A PR touching docs AND code fires both workflows; the bypass posts nothing and the real suite gates it. Posts statuses rather than naming jobs after the required contexts: a job-level `if:` skip reports as SUCCESS to branch protection, which would satisfy the gate on exactly the mixed docs+code case where it must not. `scripts/docs-only-gate.test.ts` (unit lane, 52 tests) pins the complement invariant, the context strings against ci.yml's job names, and the negative controls — no changeset containing a source file can ever be bypassed. ci.yml is untouched, so code PRs' wall time does not move. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/docs-only-gate.yml | 176 ++++++++++++ CLAUDE.md | 26 ++ scripts/docs-only-gate.mjs | 242 +++++++++++++++++ scripts/docs-only-gate.test.ts | 387 +++++++++++++++++++++++++++ 4 files changed, 831 insertions(+) create mode 100644 .github/workflows/docs-only-gate.yml create mode 100644 scripts/docs-only-gate.mjs create mode 100644 scripts/docs-only-gate.test.ts diff --git a/.github/workflows/docs-only-gate.yml b/.github/workflows/docs-only-gate.yml new file mode 100644 index 000000000..69abf9c4c --- /dev/null +++ b/.github/workflows/docs-only-gate.yml @@ -0,0 +1,176 @@ +name: Docs-only gate + +# Unblocks DOCS-ONLY pull requests WITHOUT weakening the gate. +# +# ci.yml carries `paths-ignore: ['**/*.md', '.myrobots/**', 'LICENSE']` so a +# prose-only change doesn't burn a ~25-minute pipeline. But ruleset 16042163 +# REQUIRES two contexts — `typecheck + unit + ART + E2E` and +# `vrt-strict (visual regression — strict subset)` — and a path-skipped workflow +# reports NOTHING, which GitHub treats as pending forever. A docs-only PR then +# sits BLOCKED with zero failures and can never auto-merge (live case: #1184). +# +# This workflow fires on the EXACT INVERSE path filter (same list P: ci.yml uses +# `paths-ignore: P`, this uses `paths: P`) and posts those two contexts as +# COMMIT STATUSES — but only after TWO independent guards agree. The full +# rationale, including why it posts statuses instead of naming its jobs after +# the required contexts, lives in scripts/docs-only-gate.mjs; the invariants are +# ENFORCED by scripts/docs-only-gate.test.ts in the `unit` lane (the path list, +# the context strings, and the negative controls all fail the build on drift). +# +# The one thing to keep in mind when editing: this workflow DOES fire on a PR +# that touches BOTH docs and code (path filters are per-file ANY-match — they +# cannot express "every file is a doc"). It must post NOTHING in that case, and +# it doesn't: guard G1 sees the non-doc file, and guard G2 sees the real ci.yml +# run that GitHub started for the same SHA. Either one alone is sufficient. +# +# Cost: one ~1-minute ubuntu job, and only on PRs that touch a doc path. Code +# PRs are completely unaffected — ci.yml is untouched by this change, so their +# wall time does not move. + +on: + pull_request: + types: [opened, synchronize, reopened] + branches: [main] + # INVERSE of ci.yml's `paths-ignore`. Keep byte-identical to it — the + # complement property is the whole safety argument, and + # scripts/docs-only-gate.test.ts fails if the two lists ever drift. + paths: + - '**/*.md' + - '.myrobots/**' + - 'LICENSE' + +permissions: + contents: read + actions: read # read: does a ci.yml run exist for this head SHA? (guard G2) + pull-requests: read # read: the PR's changed-file list (guard G1) + statuses: write # write: the two required contexts, ONLY when both guards pass + +concurrency: + group: docs-only-gate-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + # NOTE: this job's `name:` is deliberately NOT one of the required contexts. + # A job named after a required context creates a check run the instant it + # starts and cannot withdraw it — and a job-level `if:` skip reports as + # SUCCESS to branch protection, which would satisfy the gate on precisely the + # mixed docs+code PRs where it must not. The contexts are only ever created by + # the explicitly guarded API call in the last step. + docs-only-gate: + name: docs-only gate (bypass check, not a required context) + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + # --------------------------------------------------------------------- + # Guard G1 input — the FULL PR diff (merge-base ... head), which is the + # same file set GitHub's own `paths:`/`paths-ignore` evaluation uses for + # pull_request events. `--paginate` so a large docs sweep (#1175 touched + # 62 files) is not truncated at one page. + # --------------------------------------------------------------------- + - name: Resolve the PR's changed files + id: files + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + FILES=$(gh api "repos/${{ github.repository }}/pulls/${PR}/files" \ + --paginate --jq '.[].filename') + echo "resolved $(printf '%s' "$FILES" | grep -c . || true) changed file(s)" + { + echo 'files<> "$GITHUB_OUTPUT" + + # --------------------------------------------------------------------- + # Guard G2 input — ask GitHub whether IT started the real suite for this + # head SHA. Exact by construction: no re-derivation of glob semantics. + # + # Both workflow runs are created from the same webhook event, so a ci.yml + # run (if the filters admitted one) already exists by the time this job + # has booted a runner. The poll is pure belt-and-braces against a slow + # run-object materialisation; we must observe ABSENCE for the whole + # window before considering a bypass. + # --------------------------------------------------------------------- + - name: Look for a real CI run on this head SHA (guard G2) + id: cirun + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + EXISTS=false + for attempt in $(seq 1 12); do + COUNT=$(gh api \ + "repos/${{ github.repository }}/actions/workflows/ci.yml/runs?head_sha=${HEAD_SHA}&per_page=1" \ + --jq '.total_count' 2>/dev/null || echo "unknown") + if [ "$COUNT" = "unknown" ]; then + echo "::warning::could not query ci.yml runs (attempt ${attempt}) — treating as PRESENT (fail safe)" + EXISTS=true + break + fi + if [ "$COUNT" != "0" ]; then + echo "attempt ${attempt}: ci.yml run(s) present for ${HEAD_SHA} (total_count=${COUNT})" + EXISTS=true + break + fi + echo "attempt ${attempt}/12: no ci.yml run for ${HEAD_SHA} yet" + [ "$attempt" -lt 12 ] && sleep 5 + done + echo "ci_run_exists=${EXISTS}" >> "$GITHUB_OUTPUT" + + # --------------------------------------------------------------------- + # The decision — a pure function with unit-tested negative controls + # (scripts/docs-only-gate.test.ts). Posts nothing by itself. + # --------------------------------------------------------------------- + - name: Decide whether the docs-only bypass applies + id: decide + env: + CHANGED_FILES: ${{ steps.files.outputs.files }} + CI_RUN_EXISTS: ${{ steps.cirun.outputs.ci_run_exists }} + SAME_REPO: ${{ github.event.pull_request.head.repo.full_name == github.repository }} + run: node scripts/docs-only-gate.mjs decide + + - name: Real CI is authoritative — no bypass + if: steps.decide.outputs.post != 'true' + run: | + echo "::notice::No docs-only bypass. ${{ steps.decide.outputs.reason }}" + + # --------------------------------------------------------------------- + # The ONLY place a required context is ever produced by this workflow. + # Guarded on the decision above; contexts come from the same module the + # unit test pins against ci.yml's job names. + # --------------------------------------------------------------------- + - name: Post the required contexts (docs-only bypass) + if: steps.decide.outputs.post == 'true' + uses: actions/github-script@v7 + with: + script: | + const { REQUIRED_CONTEXTS } = await import( + `${process.env.GITHUB_WORKSPACE}/scripts/docs-only-gate.mjs` + ); + const sha = context.payload.pull_request.head.sha; + const target_url = + `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}` + + `/actions/runs/${context.runId}`; + for (const ctx of REQUIRED_CONTEXTS) { + await github.rest.repos.createCommitStatus({ + owner: context.repo.owner, + repo: context.repo.repo, + sha, + state: 'success', + context: ctx, + target_url, + description: 'docs-only change — CI is path-skipped by design', + }); + core.info(`posted success status "${ctx}" on ${sha}`); + } + core.notice( + `Docs-only bypass: satisfied ${REQUIRED_CONTEXTS.length} required ` + + `context(s) on ${sha.slice(0, 9)} without running the suite.`, + ); diff --git a/CLAUDE.md b/CLAUDE.md index 1f577a2e1..e48dbe629 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -319,6 +319,32 @@ a just-started green run; it broke main CI and spawned #699/#701/#702 the same day. See also `feedback_never_merge_on_red_collab_is_doom_gate` and `feedback_no_flake_tolerance`.) +## Docs-only PRs: the path lists in ci.yml and docs-only-gate.yml move TOGETHER + +`ci.yml` skips a prose-only change (`paths-ignore: ['**/*.md', '.myrobots/**', +'LICENSE']`) so a typo fix doesn't burn ~25 min. But ruleset 16042163 REQUIRES +`typecheck + unit + ART + E2E` and `vrt-strict (visual regression — strict +subset)`, and a path-skipped workflow reports NOTHING — GitHub treats a +never-reported required check as pending FOREVER, so the PR sits `BLOCKED` with +zero failures and can never auto-merge (#1184). + +`.github/workflows/docs-only-gate.yml` breaks that: it fires on the **exact +inverse** filter (`paths:` with the SAME list) and posts those two contexts as +commit statuses — but only when **both** guards agree: every changed file is a +doc **and** GitHub started no `ci.yml` run for that head SHA. A PR touching docs +**and** code fires both workflows; the bypass posts nothing and the real suite +gates it. + +- **Editing `ci.yml`'s `paths-ignore` means editing `docs-only-gate.yml`'s + `paths` in the SAME commit** — the complement is the whole safety argument. + `scripts/docs-only-gate.test.ts` (unit lane) fails on drift, on a context + rename, and on any changeset containing a source file. +- **Never** satisfy a required context by naming a job after it: a job-level + `if:` skip reports as SUCCESS to branch protection, which would green-light + the mixed docs+code case. Statuses are posted by an explicit guarded call. +- Renaming the `ci` or `vrt-strict` job still needs a coordinated ruleset PUT — + now plus `REQUIRED_CONTEXTS` in `scripts/docs-only-gate.mjs`. + ## Poly/MIDI modules: e2e the REAL source chain Any **poly or MIDI module** must ship an e2e that **wires the REAL default-mode diff --git a/scripts/docs-only-gate.mjs b/scripts/docs-only-gate.mjs new file mode 100644 index 000000000..03b511884 --- /dev/null +++ b/scripts/docs-only-gate.mjs @@ -0,0 +1,242 @@ +// scripts/docs-only-gate.mjs +// +// The decision logic behind `.github/workflows/docs-only-gate.yml` — the tiny +// companion workflow that unblocks DOCS-ONLY pull requests without weakening +// the gate. +// +// ── The deadlock this exists to break ────────────────────────────────────── +// +// `.github/workflows/ci.yml` carries `paths-ignore: ['**/*.md', '.myrobots/**', +// 'LICENSE']` on both `push` and `pull_request` — a deliberate optimisation so a +// prose-only change doesn't burn a ~25-minute pipeline. But ruleset 16042163 +// ("main: green PRs only") REQUIRES two status contexts: +// +// typecheck + unit + ART + E2E (ci.yml job `ci`) +// vrt-strict (visual regression — strict subset) (ci.yml job `vrt-strict`) +// +// When CI is path-skipped those contexts NEVER REPORT, and GitHub treats a +// never-reported required check as PENDING FOREVER: the PR sits at +// mergeStateStatus=BLOCKED / mergeable=MERGEABLE with zero failures and can +// never auto-merge. (Live case: #1184, docs-only, 0 failed, 0 running, +// auto-merge armed and permanently stuck. #1175 — the .myrobots corpus PR — +// merged fine because it ALSO touched `.gitignore`, which is not in +// paths-ignore, so CI actually ran.) +// +// ── Why this is SAFE (the two independent guards) ────────────────────────── +// +// GitHub path filters are per-file ANY-match, so for a non-empty changeset: +// +// ci.yml `paths-ignore: P` fires ⟺ ∃ file ∉ P +// this one `paths: P` fires ⟺ ∃ file ∈ P +// +// with the SAME list P (enforced byte-for-byte by docs-only-gate.test.ts): +// +// docs-only → ONLY this workflow fires → it posts the contexts +// code-only → ONLY ci.yml fires → the real suite gates +// BOTH → BOTH fire → see below +// +// The both-touched case is the one that must not go wrong: a bypass that fired +// there could satisfy a required context while the real CI run is still in +// flight (or red). Path filters CANNOT express "every changed file is a doc" +// (they are ANY-match, and `!` negation inside `paths:` only re-expresses +// paths-ignore), so this workflow DOES fire on a mixed PR. It is stopped by +// TWO independent guards, and BOTH must agree before a single status is posted: +// +// G1 (predicate) every changed file in the PR diff matches P, computed +// locally from `gh api pulls/N/files`. +// G2 (oracle) NO run of `.github/workflows/ci.yml` exists for this head +// SHA — i.e. GitHub's OWN filter evaluation declined to start +// the real suite. This is exact by construction: it asks +// GitHub what it did rather than re-deriving it. +// +// G2 alone already closes the both-touched case (a mixed PR always produces a +// CI run). G1 alone already closes it under our reading of GitHub's glob +// semantics. Requiring both means a disagreement between this matcher and +// GitHub's — e.g. whether `**/*.md` matches a ROOT-level `README.md`, which +// GitHub's filter engine and minimatch read differently — degrades to "no +// bypass, real CI gates", never to "bypass while code went ungated". +// +// The bypass posts COMMIT STATUSES (statuses API) rather than naming its jobs +// after the required contexts. That is deliberate: a check run is created the +// moment a job starts and cannot be withdrawn, and a JOB-level `if:` skip +// reports as SUCCESS to branch protection — so a name-matched job would satisfy +// the gate on mixed PRs exactly when it must not. A status is only created by an +// explicit, guarded API call, so on a mixed PR this workflow emits NOTHING. +// +// It also never creates a run of `ci.yml`, so daily-prod-deploy.yml's +// `find-green` scan (which looks up `actions/workflows/ci.yml/runs?head_sha=` +// and additionally asserts the umbrella job is present) cannot be fooled into +// treating a docs-only commit as a fully-green deploy candidate. +// +// Scope note: `**/*.md` / `.myrobots/**` / `LICENSE` feed nothing in the build +// or the test suites (verified by grep — every reference is a source comment). +// The one exception, `docs/testing/test-ledger.generated.md`, is a GENERATED +// golden asserted by scripts/test-ledger.test.ts; a hand-edit to it alone is +// ungated — but that is a PRE-EXISTING consequence of ci.yml's paths-ignore, +// unchanged by this workflow, and the next code PR's `unit` lane catches it. + +/** + * The doc path-set. MUST stay byte-identical to ci.yml's `paths-ignore` on both + * `push` and `pull_request` (docs-only-gate.test.ts fails the build otherwise) — + * that identity is what makes the two workflows exact complements. + */ +export const DOCS_PATTERNS = ['**/*.md', '.myrobots/**', 'LICENSE']; + +/** + * The status contexts required by ruleset 16042163. Byte-identical to the + * `name:` of ci.yml's `ci` and `vrt-strict` jobs (asserted by the test) — GitHub + * matches required checks by context string, literally, em-dash included. + */ +export const REQUIRED_CONTEXTS = [ + 'typecheck + unit + ART + E2E', + 'vrt-strict (visual regression — strict subset)', +]; + +/** Escape a literal character for use inside a RegExp. */ +function escapeChar(c) { + return /[.*+?^${}()|[\]\\]/.test(c) ? `\\${c}` : c; +} + +/** + * Compile a GitHub-Actions-style path filter glob to a RegExp. + * + * `**` matches any run of characters including `/`; `**\/` may additionally + * match ZERO path segments (the permissive, minimatch-ish reading, so a + * root-level `README.md` matches `**\/*.md`). `*` and `?` never cross `/`. + * + * Deliberately permissive: G2 (the CI-run oracle) is what makes the bypass + * safe, so this matcher erring wide can only cost a bypass, never grant one. + */ +export function globToRegExp(pattern) { + let re = ''; + for (let i = 0; i < pattern.length; i++) { + const c = pattern[i]; + if (c === '*') { + if (pattern[i + 1] === '*') { + if (pattern[i + 2] === '/') { + re += '(?:.*/)?'; // `**/` — zero or more leading segments + i += 2; + } else { + re += '.*'; // bare `**` — anything, including `/` + i += 1; + } + } else { + re += '[^/]*'; // `*` — anything within one segment + } + } else if (c === '?') { + re += '[^/]'; + } else { + re += escapeChar(c); + } + } + return new RegExp(`^${re}$`); +} + +/** True iff a repo-relative path matches the glob. */ +export function matchesGlob(file, pattern) { + return globToRegExp(pattern).test(file); +} + +/** The changed files that are NOT docs — i.e. the ones that demand real CI. */ +export function nonDocFiles(files, patterns = DOCS_PATTERNS) { + return files.filter((f) => !patterns.some((p) => matchesGlob(f, p))); +} + +/** True iff the changeset is non-empty and EVERY file is a doc. */ +export function isDocsOnly(files, patterns = DOCS_PATTERNS) { + return files.length > 0 && nonDocFiles(files, patterns).length === 0; +} + +/** + * The whole decision, in one pure function so the negative controls are unit + * tests rather than a CI experiment. + * + * Posts ONLY when every guard agrees. Any doubt → no status → the PR stays in + * exactly the state it is in today (blocked), which is the safe failure mode. + * + * @param {object} input + * @param {string[]} input.changedFiles full PR diff, repo-relative POSIX paths + * @param {boolean} input.ciRunExists a ci.yml run exists for this head SHA + * @param {boolean} input.sameRepo head repo === base repo (fork tokens + * are read-only and cannot write statuses) + * @returns {{ post: boolean, reason: string }} + */ +export function decideBypass({ changedFiles, ciRunExists, sameRepo }) { + if (!sameRepo) { + return { + post: false, + reason: 'fork PR — GITHUB_TOKEN is read-only, cannot write commit statuses', + }; + } + if (!Array.isArray(changedFiles) || changedFiles.length === 0) { + return { + post: false, + reason: 'could not resolve the PR file list (or it is empty) — refusing to bypass', + }; + } + const offenders = nonDocFiles(changedFiles); + if (offenders.length > 0) { + const shown = offenders.slice(0, 10).join(', '); + const more = offenders.length > 10 ? ` (+${offenders.length - 10} more)` : ''; + return { + post: false, + reason: `G1 FAILED — ${offenders.length} non-doc file(s) changed: ${shown}${more}. The real CI suite gates this PR.`, + }; + } + if (ciRunExists) { + return { + post: false, + reason: + 'G2 FAILED — a ci.yml run exists for this head SHA, so the real suite is the authority. ' + + 'Posting here would duplicate a required context and could green-light an in-flight or red run.', + }; + } + return { + post: true, + reason: `docs-only change (${changedFiles.length} file(s), all matching ${DOCS_PATTERNS.join(' | ')}) and GitHub started no ci.yml run for this SHA`, + }; +} + +// --------------------------------------------------------------------------- +// CLI — `node scripts/docs-only-gate.mjs decide` +// +// Reads CHANGED_FILES (newline-separated), CI_RUN_EXISTS and SAME_REPO from the +// environment; writes `post=` / `reason=` to $GITHUB_OUTPUT (and stdout). +// --------------------------------------------------------------------------- + +/** @internal exported for the test; parses the env into decideBypass() input. */ +export function inputFromEnv(env) { + return { + changedFiles: (env.CHANGED_FILES ?? '') + .split('\n') + .map((s) => s.trim()) + .filter(Boolean), + ciRunExists: env.CI_RUN_EXISTS === 'true', + sameRepo: env.SAME_REPO !== 'false', + }; +} + +const isMain = + typeof process !== 'undefined' && + process.argv[1] && + process.argv[1].endsWith('docs-only-gate.mjs'); + +if (isMain && process.argv[2] === 'decide') { + const { appendFileSync } = await import('node:fs'); + const input = inputFromEnv(process.env); + const { post, reason } = decideBypass(input); + + console.log(`changed files (${input.changedFiles.length}):`); + for (const f of input.changedFiles) console.log(` · ${f}`); + console.log(`ci run exists for head sha: ${input.ciRunExists}`); + console.log(`same-repo PR: ${input.sameRepo}`); + console.log(`\ndecision: post=${post}\nreason: ${reason}`); + + if (process.env.GITHUB_OUTPUT) { + appendFileSync(process.env.GITHUB_OUTPUT, `post=${post}\n`); + appendFileSync(process.env.GITHUB_OUTPUT, `reason=${reason.replace(/\n/g, ' ')}\n`); + } + if (post) { + console.log(`\nwill post: ${REQUIRED_CONTEXTS.map((c) => `"${c}"`).join(', ')}`); + } +} diff --git a/scripts/docs-only-gate.test.ts b/scripts/docs-only-gate.test.ts new file mode 100644 index 000000000..6a2e150f0 --- /dev/null +++ b/scripts/docs-only-gate.test.ts @@ -0,0 +1,387 @@ +// scripts/docs-only-gate.test.ts +// +// Gate for the DOCS-ONLY BYPASS (.github/workflows/docs-only-gate.yml + +// scripts/docs-only-gate.mjs). Pure-unit, zero-flake, runs in the `unit` lane +// via `task test` → `task test:scripts`. +// +// The bypass posts the two REQUIRED status contexts for a prose-only PR so it +// isn't blocked forever by a path-skipped ci.yml (the #1184 deadlock). That is +// only safe while three things hold, so all three are asserted here and the +// build goes red the moment any of them drifts: +// +// 1. COMPLEMENT — the bypass's `paths:` list is byte-identical to ci.yml's +// `paths-ignore:` list on BOTH triggers. Path filters are per-file +// ANY-match, so with the same list P exactly one workflow fires for a +// homogeneous changeset, and BOTH fire for a mixed one. Drift here would +// open a window where NEITHER fires (deadlock returns) or where the bypass +// fires for a file class the real CI also skips silently. +// +// 2. CONTEXT IDENTITY — the strings the bypass posts are byte-identical to +// the `name:` of ci.yml's `ci` and `vrt-strict` jobs (which is what ruleset +// 16042163 requires; GitHub matches contexts literally, em-dash included). +// A rename on one side only would either re-deadlock docs PRs or leave a +// stale context lying around. +// +// 3. NEGATIVE CONTROL — a changeset containing ANY real source file, or any +// SHA for which GitHub started a real ci.yml run, can NEVER be bypassed. +// This is the "prove it cannot green-light a code change" requirement, and +// it is proven at the decision function rather than by CI experiment. + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import * as gate from './docs-only-gate.mjs'; + +const { + DOCS_PATTERNS, + REQUIRED_CONTEXTS, + matchesGlob, + isDocsOnly, + nonDocFiles, + decideBypass, + inputFromEnv, +} = gate as unknown as { + DOCS_PATTERNS: string[]; + REQUIRED_CONTEXTS: string[]; + matchesGlob: (file: string, pattern: string) => boolean; + isDocsOnly: (files: string[], patterns?: string[]) => boolean; + nonDocFiles: (files: string[], patterns?: string[]) => string[]; + decideBypass: (i: { + changedFiles: string[]; + ciRunExists: boolean; + sameRepo: boolean; + }) => { post: boolean; reason: string }; + inputFromEnv: (env: Record) => { + changedFiles: string[]; + ciRunExists: boolean; + sameRepo: boolean; + }; +}; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); +const CI_YML = readFileSync(join(ROOT, '.github/workflows/ci.yml'), 'utf8'); +const BYPASS_YML = readFileSync(join(ROOT, '.github/workflows/docs-only-gate.yml'), 'utf8'); + +/** Every single-quoted YAML list that immediately follows a `:` line. */ +function listsAfterKey(src: string, key: string): string[][] { + const lines = src.split('\n'); + const found: string[][] = []; + const header = new RegExp(`^\\s*${key}:\\s*$`); + for (let i = 0; i < lines.length; i++) { + if (!header.test(lines[i])) continue; + const items: string[] = []; + for (let j = i + 1; j < lines.length; j++) { + const m = lines[j].match(/^\s*-\s*'([^']*)'\s*$/); + if (!m) break; + items.push(m[1]); + } + found.push(items); + } + return found; +} + +/** The `name:` of a top-level job, read out of the raw workflow text. */ +function jobName(src: string, jobId: string): string | undefined { + const lines = src.split('\n'); + const start = lines.indexOf(` ${jobId}:`); + if (start < 0) return undefined; + for (let j = start + 1; j < lines.length; j++) { + if (/^ {2}\S/.test(lines[j])) break; // next top-level job + const m = lines[j].match(/^ {4}name:\s*(.+?)\s*$/); + if (m) return m[1]; + } + return undefined; +} + +// --------------------------------------------------------------------------- +// 1. COMPLEMENT — the two workflows must filter on the SAME path list. +// --------------------------------------------------------------------------- + +describe('docs-only bypass: the path filters are exact complements', () => { + it('ci.yml declares paths-ignore on BOTH triggers, identical to DOCS_PATTERNS', () => { + const ignores = listsAfterKey(CI_YML, 'paths-ignore'); + // push + pull_request + expect(ignores).toHaveLength(2); + for (const list of ignores) { + expect(list).toEqual(DOCS_PATTERNS); + } + }); + + it('docs-only-gate.yml filters on the SAME list via `paths:` (the inverse filter)', () => { + const paths = listsAfterKey(BYPASS_YML, 'paths'); + expect(paths).toHaveLength(1); + expect(paths[0]).toEqual(DOCS_PATTERNS); + }); + + it('the bypass workflow never uses paths-ignore (that would break the complement)', () => { + expect(BYPASS_YML).not.toMatch(/^\s*paths-ignore:/m); + }); + + it('ci.yml is NOT modified to always-run — the wall-time optimisation is intact', () => { + // If someone "fixes" the deadlock by deleting paths-ignore instead, this + // bypass becomes dead weight that must be deleted with it. + expect(CI_YML).toMatch(/paths-ignore:/); + }); +}); + +// --------------------------------------------------------------------------- +// 2. CONTEXT IDENTITY — what we post === what the ruleset requires. +// --------------------------------------------------------------------------- + +describe('docs-only bypass: the posted contexts match the required checks', () => { + it('posts exactly the two contexts ruleset 16042163 requires', () => { + expect(REQUIRED_CONTEXTS).toHaveLength(2); + }); + + it('context 1 is byte-identical to the ci.yml `ci` umbrella job name', () => { + expect(jobName(CI_YML, 'ci')).toBe(REQUIRED_CONTEXTS[0]); + expect(REQUIRED_CONTEXTS[0]).toBe('typecheck + unit + ART + E2E'); + }); + + it('context 2 is byte-identical to the ci.yml `vrt-strict` job name (em-dash included)', () => { + expect(jobName(CI_YML, 'vrt-strict')).toBe(REQUIRED_CONTEXTS[1]); + // U+2014 EM DASH — the ruleset stores it literally; an en-dash would make + // the posted context a different check and re-deadlock every docs PR. + expect(REQUIRED_CONTEXTS[1]).toContain('—'); + expect(REQUIRED_CONTEXTS[1]).toBe('vrt-strict (visual regression — strict subset)'); + }); + + it('the bypass workflow declares no JOB named after a required context', () => { + // A job named after a required context creates a check run the moment it + // starts and cannot withdraw it — and a job-level `if:` skip reports as + // SUCCESS to branch protection, which would satisfy the gate on exactly the + // mixed docs+code PRs where it must not. Contexts must only ever come from + // the explicitly guarded statuses API call. + for (const ctx of REQUIRED_CONTEXTS) { + expect(BYPASS_YML).not.toContain(`name: ${ctx}`); + } + }); + + it('the bypass never impersonates the CI workflow itself', () => { + // daily-prod-deploy.yml's find-green scan looks up + // actions/workflows/ci.yml/runs?head_sha=… — a docs-only commit must stay + // invisible to it, so this must never become a second `name: CI` workflow. + expect(BYPASS_YML).not.toMatch(/^name:\s*CI\s*$/m); + expect(BYPASS_YML).toMatch(/^name:\s*Docs-only gate\s*$/m); + }); + + it('the status-posting step is guarded on the decision output', () => { + expect(BYPASS_YML).toMatch(/if:\s*steps\.decide\.outputs\.post == 'true'/); + // ...and the statuses permission exists, or the post would 403. + expect(BYPASS_YML).toMatch(/statuses:\s*write/); + }); +}); + +// --------------------------------------------------------------------------- +// 3. NEGATIVE CONTROL — a code change can never be bypassed. +// --------------------------------------------------------------------------- + +const CODE_FILES = [ + 'packages/web/src/lib/audio/modules/adsr.ts', + 'packages/dsp/src/cube.ts', + 'packages/server/src/relay.ts', + 'e2e/tests/ai-smoke.spec.ts', + 'e2e/vrt/vrt-exemptions.ts', + '.github/workflows/ci.yml', + '.github/workflows/docs-only-gate.yml', + 'scripts/docs-only-gate.mjs', + 'package.json', + 'package-lock.json', + 'Taskfile.yml', + '.gitignore', + 'packages/web/src/lib/docs/contract-lock.txt', + 'db/schema/001_init.sql', + 'e2e/vrt/__screenshots__/darwin/adsr.png', + 'art/baselines/moog911.f32', + 'CLAUDE.md.ts', // adversarial: .md is a substring, not the extension + 'docs.md/thing.ts', // adversarial: .md as a directory name +]; + +const DOC_FILES = [ + '.myrobots/26-07-22-roundup.md', + '.myrobots/plans/dx7-and-polyphony.md', + '.myrobots/previews/cellshade/input-wheel.png', // any file under .myrobots/** + '.myrobots/FABLE_PERF_PLAN', + 'README.md', + 'CLAUDE.md', + 'docs/testing/README.md', + 'packages/web/README.md', + 'LICENSE', +]; + +describe('docs-only bypass: negative control — code changes are never bypassed', () => { + it.each(CODE_FILES)('a PR touching %s is NOT docs-only', (file) => { + expect(isDocsOnly([file])).toBe(false); + expect(decideBypass({ changedFiles: [file], ciRunExists: false, sameRepo: true }).post).toBe( + false, + ); + }); + + it.each(DOC_FILES)('a PR touching only %s IS docs-only', (file) => { + expect(isDocsOnly([file])).toBe(true); + }); + + it('BOTH-TOUCHED: docs + one source file → no bypass, even before CI reports', () => { + const mixed = [...DOC_FILES, 'packages/web/src/lib/audio/modules/adsr.ts']; + // G1 alone stops it: the bypass workflow DOES fire on a mixed PR (path + // filters are ANY-match), so this is the case that must not leak. + const d = decideBypass({ changedFiles: mixed, ciRunExists: false, sameRepo: true }); + expect(d.post).toBe(false); + expect(d.reason).toMatch(/G1 FAILED/); + expect(nonDocFiles(mixed)).toEqual(['packages/web/src/lib/audio/modules/adsr.ts']); + }); + + it('BOTH-TOUCHED: G2 alone also stops it, independently of G1', () => { + // Belt-and-braces: even if the matcher were wrong and G1 waved a mixed + // changeset through, the presence of a real ci.yml run for the SHA blocks + // the post. Both guards must pass; either one suffices to refuse. + const d = decideBypass({ + changedFiles: ['.myrobots/plan.md'], + ciRunExists: true, + sameRepo: true, + }); + expect(d.post).toBe(false); + expect(d.reason).toMatch(/G2 FAILED/); + }); + + it('an empty / unresolvable file list is never bypassed', () => { + expect(decideBypass({ changedFiles: [], ciRunExists: false, sameRepo: true }).post).toBe(false); + expect( + decideBypass({ + changedFiles: undefined as unknown as string[], + ciRunExists: false, + sameRepo: true, + }).post, + ).toBe(false); + }); + + it('a fork PR is never bypassed (its GITHUB_TOKEN cannot write statuses anyway)', () => { + const d = decideBypass({ + changedFiles: ['.myrobots/plan.md'], + ciRunExists: false, + sameRepo: false, + }); + expect(d.post).toBe(false); + expect(d.reason).toMatch(/fork/); + }); + + it('the only path to post=true is docs-only AND no CI run AND same-repo', () => { + for (const sameRepo of [true, false]) { + for (const ciRunExists of [true, false]) { + for (const files of [ + ['.myrobots/a.md'], + ['.myrobots/a.md', 'packages/dsp/src/cube.ts'], + [], + ]) { + const { post } = decideBypass({ changedFiles: files, ciRunExists, sameRepo }); + expect(post).toBe(sameRepo && !ciRunExists && isDocsOnly(files)); + } + } + } + }); +}); + +// --------------------------------------------------------------------------- +// Real historical changesets — the two PRs that motivated this. +// --------------------------------------------------------------------------- + +describe('docs-only bypass: the live cases', () => { + it('#1184 (only .myrobots/26-07-22-roundup.md) → bypass applies', () => { + const d = decideBypass({ + changedFiles: ['.myrobots/26-07-22-roundup.md'], + ciRunExists: false, + sameRepo: true, + }); + expect(d.post).toBe(true); + }); + + it('#1175 (the .myrobots corpus + .gitignore) → NO bypass, exactly as it behaved', () => { + // #1175 merged fine precisely because .gitignore is not in paths-ignore, so + // the real CI ran. The bypass must reproduce that: G1 rejects on .gitignore + // and G2 rejects on the run GitHub actually started. + const files = [ + '.gitignore', + '.myrobots/26-07-22-roundup.md', + '.myrobots/plans/mobile-view-2026-07-02.md', + '.myrobots/previews/cellshade-rebuild-2026-07-11/input-wheel.png', + ]; + expect(decideBypass({ changedFiles: files, ciRunExists: false, sameRepo: true }).post).toBe( + false, + ); + expect(decideBypass({ changedFiles: files, ciRunExists: true, sameRepo: true }).post).toBe( + false, + ); + }); + + it('THIS PR (touches .github/** and scripts/**) → NO bypass, full suite runs', () => { + const files = [ + '.github/workflows/docs-only-gate.yml', + 'scripts/docs-only-gate.mjs', + 'scripts/docs-only-gate.test.ts', + ]; + expect(isDocsOnly(files)).toBe(false); + expect(decideBypass({ changedFiles: files, ciRunExists: false, sameRepo: true }).post).toBe( + false, + ); + }); +}); + +// --------------------------------------------------------------------------- +// The glob matcher + the env plumbing. +// --------------------------------------------------------------------------- + +describe('docs-only bypass: glob semantics', () => { + it('`**/*.md` matches nested AND root markdown', () => { + expect(matchesGlob('README.md', '**/*.md')).toBe(true); + expect(matchesGlob('docs/testing/README.md', '**/*.md')).toBe(true); + expect(matchesGlob('a/b/c/d.md', '**/*.md')).toBe(true); + }); + + it('`**/*.md` does not match a non-markdown file', () => { + expect(matchesGlob('src/a.mdx', '**/*.md')).toBe(false); + expect(matchesGlob('src/md', '**/*.md')).toBe(false); + expect(matchesGlob('CLAUDE.md.ts', '**/*.md')).toBe(false); + }); + + it('`*` does not cross a path separator', () => { + expect(matchesGlob('a/b.md', '*.md')).toBe(false); + expect(matchesGlob('b.md', '*.md')).toBe(true); + }); + + it('`.myrobots/**` matches everything under the dir and nothing outside it', () => { + expect(matchesGlob('.myrobots/a.md', '.myrobots/**')).toBe(true); + expect(matchesGlob('.myrobots/plans/deep/a.png', '.myrobots/**')).toBe(true); + expect(matchesGlob('myrobots/a.md', '.myrobots/**')).toBe(false); + expect(matchesGlob('x/.myrobots/a.md', '.myrobots/**')).toBe(false); + }); + + it('`LICENSE` is exact, not a prefix', () => { + expect(matchesGlob('LICENSE', 'LICENSE')).toBe(true); + expect(matchesGlob('LICENSES/mit.txt', 'LICENSE')).toBe(false); + expect(matchesGlob('packages/web/LICENSE', 'LICENSE')).toBe(false); + }); +}); + +describe('docs-only bypass: env plumbing', () => { + it('parses the workflow env into the decision input', () => { + expect( + inputFromEnv({ + CHANGED_FILES: '.myrobots/a.md\n\n .myrobots/b.md \n', + CI_RUN_EXISTS: 'false', + SAME_REPO: 'true', + }), + ).toEqual({ + changedFiles: ['.myrobots/a.md', '.myrobots/b.md'], + ciRunExists: false, + sameRepo: true, + }); + }); + + it('treats any non-"true" CI_RUN_EXISTS as absent but a literal "false" SAME_REPO as a fork', () => { + expect(inputFromEnv({ CI_RUN_EXISTS: 'unknown' }).ciRunExists).toBe(false); + expect(inputFromEnv({ CI_RUN_EXISTS: 'true' }).ciRunExists).toBe(true); + expect(inputFromEnv({ SAME_REPO: 'false' }).sameRepo).toBe(false); + expect(inputFromEnv({}).sameRepo).toBe(true); + }); +}); From cf8f3fb129c829ba2291e311f07c32e15d9da672 Mon Sep 17 00:00:00 2001 From: bluebox timmy Date: Mon, 27 Jul 2026 02:40:36 -0400 Subject: [PATCH 2/3] ci(docs-gate): pass the contexts as a step output, not a dynamic import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `actions/github-script` evaluates `script:` inside a `new AsyncFunction(...)` body, where dynamic `import()` has no reliable module referrer — the one code path the live both-touched run on this PR could not exercise, because the posting step is (correctly) guarded off. The decide step now emits `contexts=` from REQUIRED_CONTEXTS and the posting step does `JSON.parse(process.env.CONTEXTS)`. docs-only-gate.mjs stays the single source of truth, still pinned to ci.yml's job names by the unit test, with no ESM-in-vm dependency. Adds 4 tests: the workflow wiring (`CONTEXTS` env in, `JSON.parse` out, no `await import(`), plus three real `node scripts/docs-only-gate.mjs decide` invocations asserting the exact $GITHUB_OUTPUT the workflow reads — including the JSON round-trip back to REQUIRED_CONTEXTS. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/docs-only-gate.yml | 10 +++-- scripts/docs-only-gate.mjs | 5 +++ scripts/docs-only-gate.test.ts | 67 +++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docs-only-gate.yml b/.github/workflows/docs-only-gate.yml index 69abf9c4c..3dce618cb 100644 --- a/.github/workflows/docs-only-gate.yml +++ b/.github/workflows/docs-only-gate.yml @@ -149,11 +149,15 @@ jobs: - name: Post the required contexts (docs-only bypass) if: steps.decide.outputs.post == 'true' uses: actions/github-script@v7 + env: + # Emitted by the decide step from REQUIRED_CONTEXTS — that module stays + # the single source of truth (and the unit test pins it against ci.yml's + # job names) without this step needing a dynamic `import()`, which has + # no reliable module referrer inside github-script's AsyncFunction body. + CONTEXTS: ${{ steps.decide.outputs.contexts }} with: script: | - const { REQUIRED_CONTEXTS } = await import( - `${process.env.GITHUB_WORKSPACE}/scripts/docs-only-gate.mjs` - ); + const REQUIRED_CONTEXTS = JSON.parse(process.env.CONTEXTS); const sha = context.payload.pull_request.head.sha; const target_url = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}` + diff --git a/scripts/docs-only-gate.mjs b/scripts/docs-only-gate.mjs index 03b511884..b3b3fc0a8 100644 --- a/scripts/docs-only-gate.mjs +++ b/scripts/docs-only-gate.mjs @@ -235,6 +235,11 @@ if (isMain && process.argv[2] === 'decide') { if (process.env.GITHUB_OUTPUT) { appendFileSync(process.env.GITHUB_OUTPUT, `post=${post}\n`); appendFileSync(process.env.GITHUB_OUTPUT, `reason=${reason.replace(/\n/g, ' ')}\n`); + // The contexts travel as an output rather than being imported by the + // posting step: dynamic `import()` inside actions/github-script's + // `new AsyncFunction(...)` body has no reliable module referrer, so this + // module stays the single source of truth WITHOUT an ESM-in-vm dependency. + appendFileSync(process.env.GITHUB_OUTPUT, `contexts=${JSON.stringify(REQUIRED_CONTEXTS)}\n`); } if (post) { console.log(`\nwill post: ${REQUIRED_CONTEXTS.map((c) => `"${c}"`).join(', ')}`); diff --git a/scripts/docs-only-gate.test.ts b/scripts/docs-only-gate.test.ts index 6a2e150f0..211d8c602 100644 --- a/scripts/docs-only-gate.test.ts +++ b/scripts/docs-only-gate.test.ts @@ -28,7 +28,9 @@ // it is proven at the decision function rather than by CI experiment. import { describe, it, expect } from 'vitest'; -import { readFileSync } from 'node:fs'; +import { readFileSync, writeFileSync, mkdtempSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import * as gate from './docs-only-gate.mjs'; @@ -171,6 +173,69 @@ describe('docs-only bypass: the posted contexts match the required checks', () = // ...and the statuses permission exists, or the post would 403. expect(BYPASS_YML).toMatch(/statuses:\s*write/); }); + + it('the posting step takes its contexts from the decide step, not a dynamic import', () => { + // github-script evaluates `script:` inside a `new AsyncFunction(...)` body, + // where dynamic `import()` has no reliable module referrer. The contexts + // therefore travel as a step output — docs-only-gate.mjs stays the single + // source of truth (pinned to ci.yml's job names by the tests above) without + // that fragile ESM-in-vm dependency. + expect(BYPASS_YML).toMatch(/CONTEXTS: \$\{\{ steps\.decide\.outputs\.contexts \}\}/); + expect(BYPASS_YML).toContain('JSON.parse(process.env.CONTEXTS)'); + expect(BYPASS_YML).not.toMatch(/await import\(/); + }); +}); + +// --------------------------------------------------------------------------- +// The CLI seam the workflow actually invokes — run it for real. +// --------------------------------------------------------------------------- + +describe('docs-only bypass: the `decide` CLI writes the outputs the workflow reads', () => { + function runDecide(env: Record) { + const out = join(mkdtempSync(join(tmpdir(), 'docs-only-gate-')), 'GITHUB_OUTPUT'); + writeFileSync(out, ''); + execFileSync(process.execPath, [join(ROOT, 'scripts/docs-only-gate.mjs'), 'decide'], { + env: { ...process.env, ...env, GITHUB_OUTPUT: out }, + encoding: 'utf8', + }); + return Object.fromEntries( + readFileSync(out, 'utf8') + .split('\n') + .filter(Boolean) + .map((l) => [l.slice(0, l.indexOf('=')), l.slice(l.indexOf('=') + 1)]), + ); + } + + it('docs-only → post=true and contexts parse back to REQUIRED_CONTEXTS', () => { + const o = runDecide({ + CHANGED_FILES: '.myrobots/26-07-22-roundup.md', + CI_RUN_EXISTS: 'false', + SAME_REPO: 'true', + }); + expect(o.post).toBe('true'); + // The exact round-trip the workflow performs: JSON.parse(process.env.CONTEXTS). + expect(JSON.parse(o.contexts)).toEqual(REQUIRED_CONTEXTS); + }); + + it('a code file → post=false (the workflow step never fires)', () => { + const o = runDecide({ + CHANGED_FILES: '.myrobots/a.md\npackages/dsp/src/cube.ts', + CI_RUN_EXISTS: 'false', + SAME_REPO: 'true', + }); + expect(o.post).toBe('false'); + expect(o.reason).toContain('G1 FAILED'); + }); + + it('a real CI run → post=false', () => { + const o = runDecide({ + CHANGED_FILES: '.myrobots/a.md', + CI_RUN_EXISTS: 'true', + SAME_REPO: 'true', + }); + expect(o.post).toBe('false'); + expect(o.reason).toContain('G2 FAILED'); + }); }); // --------------------------------------------------------------------------- From 7ac27e25d129f40e03461cdc3bcce02b3787fe49 Mon Sep 17 00:00:00 2001 From: bluebox timmy Date: Mon, 27 Jul 2026 02:49:41 -0400 Subject: [PATCH 3/3] ci(docs-gate): close the untrusted-filename injection surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial pass over the bypass workflow. Filenames from a PR diff are attacker-controllable (a branch can carry a path containing quotes, backticks or even a newline — git allows it), and they flow into three places: 1. `run:` bodies — the "no bypass" step interpolated the decision reason, which embeds those filenames, directly into the shell via `${{ }}`. Now passed via `env:`, like every other step output. Guarded by a test that walks all four `run:` bodies plus the github-script `script:` body and rejects any `${{ }}` interpolation of `steps.*` / `github.event.*`. 2. `$GITHUB_OUTPUT` — an unflattened reason carrying CR/LF would inject a second `key=value` line, and the last value wins, so a crafted path could forge `post=true` past the guarded posting step. Reason is now flattened on `\r` AND `\n`; negative-control test included. 3. The changed-file heredoc — a fixed `CHANGED_FILES_EOF` delimiter appearing in the payload would TRUNCATE the file list, the one truncation that could make a mixed PR look docs-only to G1. Delimiter is now randomised per run (GitHub's documented practice). G2 would still have refused; not relying on the second guard for something this cheap. Also converts the G2 poll's trailing `[ … ] && sleep` to an explicit `if` — verified it does NOT trip `set -e` today (the failing test is a non-final command of an AND-OR list, so it is exempt), but the exhaustion path is the docs-only path, and it should not depend on that subtlety. 60 tests, 3× clean; actionlint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/docs-only-gate.yml | 34 ++++++++++++++----- scripts/docs-only-gate.mjs | 5 ++- scripts/docs-only-gate.test.ts | 51 ++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/.github/workflows/docs-only-gate.yml b/.github/workflows/docs-only-gate.yml index 3dce618cb..0bc5a7c79 100644 --- a/.github/workflows/docs-only-gate.yml +++ b/.github/workflows/docs-only-gate.yml @@ -75,16 +75,24 @@ jobs: id: files env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} PR: ${{ github.event.pull_request.number }} run: | set -euo pipefail - FILES=$(gh api "repos/${{ github.repository }}/pulls/${PR}/files" \ + FILES=$(gh api "repos/${REPO}/pulls/${PR}/files" \ --paginate --jq '.[].filename') echo "resolved $(printf '%s' "$FILES" | grep -c . || true) changed file(s)" + # Random heredoc delimiter (GitHub's documented practice): a git path + # may legally contain a newline, and a fixed delimiter appearing inside + # the payload would TRUNCATE the file list — which, if it dropped the + # source file from a mixed PR, is the one truncation that could turn a + # code change into an apparently docs-only one. G2 would still refuse, + # but do not rely on the second guard for something this cheap to fix. + DELIM="CHANGED_FILES_EOF_$(openssl rand -hex 16)" { - echo 'files<> "$GITHUB_OUTPUT" # --------------------------------------------------------------------- @@ -101,13 +109,15 @@ jobs: id: cirun env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail EXISTS=false - for attempt in $(seq 1 12); do + ATTEMPTS=12 + for attempt in $(seq 1 "$ATTEMPTS"); do COUNT=$(gh api \ - "repos/${{ github.repository }}/actions/workflows/ci.yml/runs?head_sha=${HEAD_SHA}&per_page=1" \ + "repos/${REPO}/actions/workflows/ci.yml/runs?head_sha=${HEAD_SHA}&per_page=1" \ --jq '.total_count' 2>/dev/null || echo "unknown") if [ "$COUNT" = "unknown" ]; then echo "::warning::could not query ci.yml runs (attempt ${attempt}) — treating as PRESENT (fail safe)" @@ -119,8 +129,10 @@ jobs: EXISTS=true break fi - echo "attempt ${attempt}/12: no ci.yml run for ${HEAD_SHA} yet" - [ "$attempt" -lt 12 ] && sleep 5 + echo "attempt ${attempt}/${ATTEMPTS}: no ci.yml run for ${HEAD_SHA} yet" + if [ "$attempt" -lt "$ATTEMPTS" ]; then + sleep 5 + fi done echo "ci_run_exists=${EXISTS}" >> "$GITHUB_OUTPUT" @@ -138,8 +150,14 @@ jobs: - name: Real CI is authoritative — no bypass if: steps.decide.outputs.post != 'true' + env: + # Via env, never `${{ }}` inside the script body: the reason embeds + # FILENAMES from the PR diff, which are attacker-controllable (a branch + # can carry a file whose name contains quotes or backticks) and would + # otherwise be spliced into the shell before it runs. + REASON: ${{ steps.decide.outputs.reason }} run: | - echo "::notice::No docs-only bypass. ${{ steps.decide.outputs.reason }}" + echo "::notice::No docs-only bypass. ${REASON}" # --------------------------------------------------------------------- # The ONLY place a required context is ever produced by this workflow. diff --git a/scripts/docs-only-gate.mjs b/scripts/docs-only-gate.mjs index b3b3fc0a8..418fae200 100644 --- a/scripts/docs-only-gate.mjs +++ b/scripts/docs-only-gate.mjs @@ -234,7 +234,10 @@ if (isMain && process.argv[2] === 'decide') { if (process.env.GITHUB_OUTPUT) { appendFileSync(process.env.GITHUB_OUTPUT, `post=${post}\n`); - appendFileSync(process.env.GITHUB_OUTPUT, `reason=${reason.replace(/\n/g, ' ')}\n`); + // `reason` embeds FILENAMES from the PR diff, and a git path may legally + // contain CR/LF — which would inject an extra `key=value` line into + // $GITHUB_OUTPUT and let a crafted branch forge `post=true`. Flatten first. + appendFileSync(process.env.GITHUB_OUTPUT, `reason=${reason.replace(/[\r\n]+/g, ' ')}\n`); // The contexts travel as an output rather than being imported by the // posting step: dynamic `import()` inside actions/github-script's // `new AsyncFunction(...)` body has no reliable module referrer, so this diff --git a/scripts/docs-only-gate.test.ts b/scripts/docs-only-gate.test.ts index 211d8c602..e50396544 100644 --- a/scripts/docs-only-gate.test.ts +++ b/scripts/docs-only-gate.test.ts @@ -236,6 +236,57 @@ describe('docs-only bypass: the `decide` CLI writes the outputs the workflow rea expect(o.post).toBe('false'); expect(o.reason).toContain('G2 FAILED'); }); + + it('a filename carrying CRLF cannot forge a second `post=true` output line', () => { + // git paths may legally contain CR/LF. The reason string embeds filenames, + // so an unflattened reason would inject an extra key=value line into + // $GITHUB_OUTPUT — and the LAST value wins, letting a crafted branch flip + // the guarded posting step on. The filename is a source file, so the real + // decision is (and must stay) post=false. + const evil = 'packages/dsp/src/cube.ts\npost=true\r\npost=true'; + const o = runDecide({ + CHANGED_FILES: `.myrobots/a.md\n${evil.replace(/\n/g, '')}`, + CI_RUN_EXISTS: 'false', + SAME_REPO: 'true', + }); + expect(o.post).toBe('false'); + // ...and directly at the seam: the reason is single-line, always. + const d = decideBypass({ changedFiles: [evil], ciRunExists: false, sameRepo: true }); + expect(d.post).toBe(false); + expect(d.reason.replace(/[\r\n]+/g, ' ')).not.toMatch(/[\r\n]/); + }); +}); + +describe('docs-only bypass: the workflow never splices untrusted text into a shell', () => { + it('no `${{ steps.* }}` interpolation inside a run: body', () => { + // The decision reason embeds attacker-controllable FILENAMES; interpolating + // it into `run:` would splice them into the shell before it executes. Every + // step output reaches the shell via `env:` instead. + const runBodies = BYPASS_YML.split(/^ {6}- name: /m) + .filter((s) => /^ {8}run: /m.test(s)) + .map((s) => s.slice(s.search(/^ {8}run: /m))); + // Every `run:` step in the file, block-scalar or single-line. + expect(runBodies).toHaveLength(4); + for (const body of runBodies) { + expect(body).not.toMatch(/\$\{\{\s*steps\./); + expect(body).not.toMatch(/\$\{\{\s*github\.event\./); + } + }); + + it('the github-script body interpolates nothing either', () => { + // Same hazard, JS instead of shell: `${{ }}` inside `script:` is spliced in + // before Node parses it. The contexts arrive via process.env.CONTEXTS. + const script = BYPASS_YML.slice(BYPASS_YML.indexOf('script: |')); + expect(script).not.toMatch(/\$\{\{/); + expect(script).toContain('JSON.parse(process.env.CONTEXTS)'); + }); + + it('the changed-file heredoc delimiter is randomised', () => { + // A fixed delimiter appearing inside the payload (a path may contain a + // newline) would truncate the file list — the one truncation that could + // make a mixed PR look docs-only to G1. + expect(BYPASS_YML).toMatch(/DELIM="CHANGED_FILES_EOF_\$\(openssl rand -hex 16\)"/); + }); }); // ---------------------------------------------------------------------------