From 6b1031df6292191494aee34518dbe7947dd92129 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Thu, 8 Oct 2026 09:16:02 +0200 Subject: [PATCH] fix(validator): gate on same-repo, not on the repo being a fork MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The org-required caller's validate job gated on `head.repo.fork == false` — "is the head repo itself a fork?". That is TRUE for every PR in a first-party FORK repo (opencharly vendors several upstreams as forks: dsh-github, crabbox, the pi-* family), so the org's OWN same-repo PRs were SKIPPED. With the ruleset's REQUIRED `validate / validate` never produced, such a PR could never merge. Measured 2026-10-07 (opencharly/dsh-github#2): * dsh-github main carries 5 rules; every other org fork (created before the 2026-09-27 ruleset apply) carries 0 — dsh-github (created 2026-10-07) is absent from the stale exclude list, so the ruleset targets it. * `gh pr merge 2` -> 'Required status check "validate / validate" is expected.' * the check run on the head is 'completed / skipped' (the job's if: excluded it). The fix gates on the correct predicate: `head.repo.full_name == github.repository` — the head repo is a DIFFERENT repository than the base. Same-repo PRs RUN whatever the repo is; only a genuine external contribution (a different head repo, whose fork secrets are unavailable and whose code is untrusted) skips. A null head repo (a deleted fork) resolves to '' != github.repository, so it skips — the safe default. Coverage that FAILS without the change: * .github/tests/validator-gate-harness.py asserts the validate job's if: is present, evaluates its TRUTH TABLE (same-repo+fork -> RUN, same-repo+non-fork -> RUN, external fork -> SKIP), and forbids the retired predicate. Positive control: 414 assertions pass. Negative control: reverting the predicate exits 1 with 3 failures. * a same_repo_gate row in scripts/governance-claims.tsv requires the new predicate and forbids an ACTIVE `if: ... head.repo.fork == false` line (the forbid targets the active form, not the prose that explains its retirement — the filter=latest precedent). Negative control: reverting exits 1. Gates run locally, green: validator-gate-harness.py (414 assertions), governance- reconcile.sh (+ self-test), org-ruleset_test.sh, retire-per-repo-dispatchers_test.sh, retire-per-repo-candy-gates_test.sh, strip-config-version_test.sh, bootstrap-repo-main_test.sh. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated) --- .github/tests/validator-gate-harness.py | 64 +++++++++++++++++++ .../org-wide-pr-validator-required.yml | 16 ++++- README.md | 31 +++++++++ scripts/governance-claims.tsv | 13 ++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/.github/tests/validator-gate-harness.py b/.github/tests/validator-gate-harness.py index a063f80..2651457 100644 --- a/.github/tests/validator-gate-harness.py +++ b/.github/tests/validator-gate-harness.py @@ -290,6 +290,32 @@ def workflow_permissions(path): return result +def job_if(path, job_name): + """Return a job's top-level `if:` expression (unquoted), or None when absent. + + A line-based read of the 2-space-indented `:` block under `jobs:`, then + the first 4-space-indented `if:` inside it — the harness's no-PyYAML technique. + """ + if not os.path.exists(path): + raise HarnessError("workflow not found: " + path) + with open(path, "r", encoding="utf-8") as fh: + lines = fh.read().splitlines() + in_job = False + for line in lines: + if not line.strip() or line.lstrip().startswith("#"): + continue + indent = len(line) - len(line.lstrip(" ")) + stripped = line.strip() + if indent == 2 and stripped == job_name + ":": + in_job = True + continue + if in_job and indent == 2 and stripped.endswith(":"): + break # the next job begins + if in_job and indent >= 4 and stripped.startswith("if:"): + return unquote(stripped.partition(":")[2].strip()) + return None + + def indent(text, pad): return NL.join(pad + line for line in text.splitlines()) @@ -1444,6 +1470,44 @@ def note(ok, message): "functional: the org-wide caller org-wide-pr-validator-required.yml ALSO grants " "`issues` (a called workflow holds only what its caller grants; asserting only the " "reusable would pass while the caller nullified it — .github#173)") + # SAME-REPO vs EXTERNAL-FORK (the dsh-github#2 block, 2026-10-07). The validate job must + # gate on `head.repo.full_name == github.repository` — same-repo PRs RUN even when the + # repo is a first-party FORK of an upstream, and only a genuine external contribution + # (a DIFFERENT head repo) skips. The retired `head.repo.fork == false` test is TRUE for + # every PR in a fork repo, so it SKIPPED the org's own PRs and the ruleset's REQUIRED + # `validate / validate` was never produced (measured: `dsh-github` main carries the + # ruleset; `gh pr merge` reports `Required status check "validate / validate" is + # expected`). Assert BOTH the predicate's presence and its evaluated truth table, so a + # silent revert to the fork-repo form — or the deletion of the `if:` — FAILS here. + caller_if = job_if(CALLER_WORKFLOW_PATH, "validate") + note(caller_if is not None, + "structural: the org-wide caller's validate job carries an `if:` (a job with no " + "`if:` would also run, but the predicate is the point — assert it is present)") + if caller_if is not None: + def caller_gate(head_fork, same_repo): + # A null head repo (a deleted fork) evaluates every `full_name` lookup to the + # empty string, which is != github.repository, so the job SKIPS — the safe + # default. Model `head_fork` explicitly so a revert to the fork-repo test is + # caught even where same_repo is true. + head_full = "o/r" if same_repo else "someoneelse/r" + ns = Ctx({ + "github": Ctx({"repository": "o/r", + "event": Ctx({"pull_request": Ctx({"head": Ctx({"repo": Ctx({ + "full_name": head_full, "fork": head_fork})})})})}), + }) + return eval_gh(caller_if, ns) + note(caller_gate(head_fork=True, same_repo=True) is True, + "functional: a SAME-REPO PR in a first-party FORK repo (head.fork=true, " + "head.full_name==github.repository) RUNS the validator (the exact dsh-github#2 " + "shape the fork-repo test wrongly skipped)") + note(caller_gate(head_fork=False, same_repo=True) is True, + "functional: a SAME-REPO PR in a non-fork repo RUNS the validator") + note(caller_gate(head_fork=True, same_repo=False) is False, + "functional: an EXTERNAL-FORK PR (head.full_name!=github.repository) SKIPS " + "(untrusted contribution + no fork secrets — the security intent preserved)") + note("head.repo.fork == false" not in caller_if, + "forbid: the retired fork-repo predicate `head.repo.fork == false` is GONE " + "(it is true for every PR in a fork repo, so it dead-locked first-party forks)") # FUNCTIONAL coverage (the review's R10 finding: the env entry and the engine-defective # classification shipped with NO assertion that fails without them). review_env = find_step(steps, "id", "review")["env"] diff --git a/.github/workflows/org-wide-pr-validator-required.yml b/.github/workflows/org-wide-pr-validator-required.yml index e2978b0..cb3bb5d 100644 --- a/.github/workflows/org-wide-pr-validator-required.yml +++ b/.github/workflows/org-wide-pr-validator-required.yml @@ -74,7 +74,21 @@ permissions: jobs: validate: name: validate - if: github.event.pull_request.head.repo.fork == false + # SAME-REPO vs EXTERNAL-FORK — measured 2026-10-07 (opencharly/dsh-github#2). The gate + # tests whether the head repo is a DIFFERENT repository than the base + # (`head.repo.full_name == github.repository`), NOT whether the head repo is itself a + # fork (`head.repo.fork == false`). The old fork-repo test is TRUE for every PR in a + # first-party FORK repo — opencharly vendors several upstreams as forks (dsh-github, + # crabbox, the pi-* family) — so it SKIPPED the org's OWN same-repo PRs: the ruleset's + # REQUIRED `validate / validate` was then never produced and the PR could never merge + # (measured: `dsh-github` main carries the ruleset while every pre-existing fork does + # not; `gh pr merge` reports `Required status check "validate / validate" is expected`). + # The new form skips ONLY a genuine external contribution — a PR whose head repo is a + # DIFFERENT repo than the base, whose fork secrets are unavailable and whose code is + # untrusted — while running for every SAME-REPO PR regardless of whether the repo is a + # fork of an upstream. A null head repo (a deleted fork) yields an absent `full_name` + # and skips, the safe default. + if: github.event.pull_request.head.repo.full_name == github.repository uses: opencharly/.github/.github/workflows/pr-validator.yml@main with: pr-number: ${{ github.event.pull_request.number }} diff --git a/README.md b/README.md index 7a372d2..ccdf2b5 100644 --- a/README.md +++ b/README.md @@ -195,6 +195,37 @@ $ scripts/org-ruleset.sh apply # then enable the org rulese $ scripts/org-ruleset.sh verify # assert the whole end state ``` +### The gate runs on SAME-REPO PRs, whatever the repo is + +The required workflow's `validate` job gates on +**`github.event.pull_request.head.repo.full_name == github.repository`** — the head +repo is a *different* repository than the base. It runs for **every same-repo PR**, +including PRs in a first-party **fork** repo (the org vendors several upstreams as +forks: `dsh-github`, `crabbox`, the `pi-*` family), and skips only a genuine +**external contribution** (a head repo that is a *different* repo — whose fork +secrets are unavailable and whose code is untrusted). A null head repo (a deleted +fork) resolves to the empty string, which is `!= github.repository`, so the job +skips — the safe default. + +**This predicate is load-bearing.** The retired `head.repo.fork == false` test asked +"is the head repo itself a fork?", which is true for **every** PR in a fork repo, so +it skipped the org's own PRs. Combined with the ruleset's REQUIRED +`validate / validate` check, that made such a PR un-mergeable — the required check was +never produced (`measured 2026-10-07, opencharly/dsh-github#2`: `dsh-github` main +carries the ruleset while every pre-existing fork does not, and `gh pr merge` reports +`Required status check "validate / validate" is expected`). The predicate and its +truth table are asserted by `.github/tests/validator-gate-harness.py` (which runs on +every PR to this repo) and by a `same_repo_gate` row in `scripts/governance-claims.tsv`. + +**Consequence for a new fork repo.** Forks are **excluded from the ruleset by +design** (`discover_excludes` filters on `isFork == true`), so they carry no required +check and their same-repo PRs merge without an org verdict. The hazard is a fork +created *after* the last `scripts/org-ruleset.sh apply`: it is absent from the stale +exclude list, so the ruleset's `~ALL`-minus-exclude scope still lists it and its +same-repo PRs get the REQUIRED check. With the retired predicate that check was never +produced (deadlock); with this predicate it runs and the PR merges on PASS. The next +`apply` re-derives the exclude set and drops the fork back out of the ruleset target. + ## New repos: bootstrap the initial `main` The org ruleset carries the `creation` rule on `refs/heads/main`, so a **brand-new diff --git a/scripts/governance-claims.tsv b/scripts/governance-claims.tsv index 9fbfeef..bf6a021 100644 --- a/scripts/governance-claims.tsv +++ b/scripts/governance-claims.tsv @@ -80,3 +80,16 @@ ci_gate_mechanical validator workflow dotgithub/.github/workflows/pr-validator.y ci_gate_mechanical validator rulebook dotgithub/charly.yml require BRANCH PROTECTION DOES NOT ENFORCE IT ci_gate_mechanical validator rulebook dotgithub/charly.yml require enforced by that first gate, NOT by branch ci_gate_mechanical validator rulebook dotgithub/charly.yml forbid enforced by branch protection +# SAME-REPO vs EXTERNAL-FORK (opencharly/.github dsh-github#2, 2026-10-07). The org-required +# caller must gate the validate job on `head.repo.full_name == github.repository` — the +# head repo is a DIFFERENT repository than the base — NOT on `head.repo.fork == false`, +# which is TRUE for every PR in a first-party FORK repo (opencharly vendors several upstreams +# as forks: dsh-github, crabbox, the pi-* family). The retired predicate skipped the org's OWN +# same-repo PRs, so the ruleset's REQUIRED `validate / validate` was never produced and the PR +# could never merge. The new form skips ONLY a genuine external contribution. +same_repo_gate org-wide required workflow dotgithub/.github/workflows/org-wide-pr-validator-required.yml require head.repo.full_name == github.repository +# The forbid pattern targets the ACTIVE form — an `if:` line using the retired predicate — +# not the prose that explains why it is retired (the same technique as the +# `check-runs...filter=latest` row above: the file names the retired form in a comment, so a +# bare-substring forbid would fail on its own explanation). +same_repo_gate org-wide required workflow dotgithub/.github/workflows/org-wide-pr-validator-required.yml forbid if: .*head\.repo\.fork == false