From 73aade50fd9dccf3f3b3e8ced04c32f7d1ea552f Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:36:08 -0700 Subject: [PATCH] The ruleset profile argued from a fourteen-commit repository that now has sixty-six .github/rulesets/README.md recommends required_signatures and argues against merge commits from measurements taken on 2026-08-15 and never re-read: "all fourteen commits on main" and "the three merge commits already on main". At d4f533f there are 66 and 12. Both conclusions survive; the evidence for them did not. The counts are now stated against the commit they were taken at, so main advancing does not falsify them. Re-measuring turned up a worse problem. A local git log --format='%G?' reports N, "no signature", for thirteen commits on main. They are not unsigned: they are SSH-signed with the owner's key, and git 2.55 classifies an SSH signature as N when no gpg.ssh.allowedSignersFile is configured. Supply any allowed-signers file and the same commit reports U. The other fifty-three are PGP-signed by GitHub's web-flow key and report E. Zero commits on main carry no signature, all 66 report verification.verified: true from the API, and that API verification is what GitHub's own required_signatures rule enforces. An audit that read N as unsigned would report thirteen unsigned commits that do not exist. tests/test_ruleset_evidence.py holds both halves without becoming a counter that jams the queue. The commit and merge counts are re-measured from the SHA the prose pins, so history advancing changes nothing, while a number edited without moving the pin, a pin moved without re-measuring, or a checkout too shallow to see that commit all fail. Every required status check context in main.json is derived from the workflow files, so a renamed job fails here rather than silently emptying the profile on the day it is applied. Each gate was run against the fault it exists to catch, with the sabotage asserted present in the file before the result was read. The ruleset is still not applied; re-confirmed today that rulesets is [] and main reports protected: false. Applying it is the owner's action and stays open as #15. --- .github/rulesets/README.md | 41 +++++- CHANGELOG.md | 32 +++++ tests/test_ruleset_evidence.py | 249 +++++++++++++++++++++++++++++++++ 3 files changed, 316 insertions(+), 6 deletions(-) create mode 100644 tests/test_ruleset_evidence.py diff --git a/.github/rulesets/README.md b/.github/rulesets/README.md index ee1d983..4698f0a 100644 --- a/.github/rulesets/README.md +++ b/.github/rulesets/README.md @@ -8,7 +8,7 @@ setting, which is the owner's call, not a pull request's. ## What is true today **No ruleset is applied on this repository.** Measured 2026-08-15, re-read -2026-08-28 and again 2026-08-29, every time with the same answer: +2026-08-28, 2026-08-29 and again 2026-09-06, every time with the same answer: | Question | Answer | |---|---| @@ -93,14 +93,43 @@ a required context that matches nothing is a gate that has silently gone away. possible today and should not be. **`required_signatures`.** Checked before recommending it, because enabling it -with an unsigned history locks the owner out: all fourteen commits on `main` -report `verification.verified: true` from the GitHub API, across both -`ChelseaKR` and `dependabot[bot]`, so nothing is locked out by turning it on. +with an unsigned history locks the owner out. Re-measured 2026-09-06 at +`d4f533f`, the tip of `main`: **66 commits**, and all 66 report +`verification.verified: true` from the GitHub API, across `ChelseaKR` and +`dependabot[bot]`. Nothing is locked out by turning it on. This paragraph said +"all fourteen commits" until 2026-09-06, which was the count on 2026-08-15 and +had not been re-read since; the conclusion survived the re-measurement, the +evidence for it did not. + +Measure it the way this paragraph does, with one call, and not from a local +checkout: + +```sh +gh api 'repos/ChelseaKR/perimeter/commits?sha=main&per_page=100' \ + --jq '[.[] | select(.commit.verification.verified | not)] | length' +``` + +**`git log --format='%G?'` is the wrong instrument here and reports the opposite +answer.** Run locally against this repository it returns `N` — "no signature" — +for thirteen commits, and `E` for the rest. Both readings are artifacts of the +local checkout, not facts about the commits. The thirteen are SSH-signed with +the owner's own key, and git 2.55 reports an SSH signature as `N` when no +`gpg.ssh.allowedSignersFile` is configured; supply any allowed-signers file and +the same commit reports `U`, a good signature from an untrusted key. The other +fifty-three are PGP-signed by GitHub's web-flow key, and `E` means the key is +not in the local keyring. Zero commits on `main` carry no signature. An audit +that read `N` as "unsigned" would report thirteen unsigned commits that do not +exist, which is this portfolio's most common defect — a read that failed +published as a measurement — committed by the tool that was checking for it. +GitHub's own enforcement of `required_signatures` uses the verification the API +reports, so the API is also the instrument that matches the rule. **`required_linear_history`** with `allowed_merge_methods: ["squash", "rebase"]`. The repository currently also allows merge commits; linear history and a merge -commit cannot both be had, and the three merge commits already on `main` are -unaffected, since a ruleset governs new pushes rather than existing history. +commit cannot both be had, and the **12** merge commits already on `main` +are unaffected, since a ruleset governs new pushes rather than existing history. +This said "three" until 2026-09-06, which was true on 2026-08-15; nine more have +landed since, and none of them changes the reasoning. **`required_approving_review_count: 0`.** GitHub does not count self-approval, so `1` deadlocks every merge in a single-maintainer repository. This is the diff --git a/CHANGELOG.md b/CHANGELOG.md index 16f1c1a..5d6635e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,38 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this project a ## [Unreleased] +### Fixed, the ruleset profile's supporting evidence had gone stale and one reading of it was backwards + +- **`.github/rulesets/README.md` counted a repository that no longer exists.** It argued + for `required_signatures` from "all fourteen commits on `main`" and against a merge + commit from "the three merge commits already on `main`". Measured 2026-09-06 at + `d4f533f`: 66 commits and 12 merge commits. Both conclusions survive the + re-measurement; neither piece of evidence did. The numbers are now stated against the + commit they were taken at, so they stay true as `main` advances instead of needing a + hand edit per merge. +- **A local `git log --format='%G?'` reads this repository's signatures backwards, and an + audit acted on it.** It reports `N`, "no signature", for the thirteen SSH-signed + commits, because git 2.55 cannot classify an SSH signature without a configured + `gpg.ssh.allowedSignersFile`; supply any such file and the same commit reports `U`. The + other fifty-three are PGP-signed by GitHub's web-flow key and report `E`, key not in + the local keyring. Zero commits on `main` carry no signature and all 66 report + `verification.verified: true` from the API, which is also the verification GitHub's own + `required_signatures` rule uses. The document now names the instrument, the trap and + the one-call measurement, because "thirteen unsigned commits" is a failed read + published as a measurement, this portfolio's most common defect, produced here by the + check that was looking for it. +- **`tests/test_ruleset_evidence.py`** holds both halves. The commit and merge counts are + re-measured from the SHA the prose pins, so a number edited without moving the pin, or + a pin advanced without re-measuring, fails; a checkout too shallow to see that commit + fails rather than passing vacuously. Every required status check context in + `main.json` is derived from the workflow files, so a renamed job fails here instead of + silently emptying the profile the day it is applied. Each gate was run against the + fault it exists to catch, with the sabotage asserted present in the file before the + result was read. +- Not changed: the ruleset is still **not applied**, re-confirmed 2026-09-06 + (`rulesets` is `[]`, `main` reports `"protected": false`). Applying it is the owner's + action and stays open as #15. + ### Added, a refresh can now say what it moved - **`python -m perimeter.diff OLD NEW`, and `make diff`.** A refresh of the pinned diff --git a/tests/test_ruleset_evidence.py b/tests/test_ruleset_evidence.py new file mode 100644 index 0000000..e6b5c16 --- /dev/null +++ b/tests/test_ruleset_evidence.py @@ -0,0 +1,249 @@ +"""The evidence in `.github/rulesets/README.md` must still be true of this repository. + +`tests/test_ruleset.py` holds the ruleset document itself: the owner's bypass is present +and is the only entry. This module holds the *prose around it*, which is what a person +actually follows when they decide whether to apply the profile, and which drifted badly +between 2026-08-15 and 2026-09-06: + +* it said `main` carried "fourteen commits" when it carried sixty-six; +* it said "three merge commits" when there were twelve; +* it recommended `required_signatures` on the strength of a fourteen-commit reading that + nothing had re-run. + +The conclusions all survived re-measurement. The evidence for them did not, and evidence +that is not re-read is indistinguishable from evidence that is wrong. + +Two things are gated here, chosen because neither can go stale on its own and neither +needs the network: + +1. **The commit counts, pinned to a SHA.** Git history is immutable, so a count taken at + a named commit is true forever. The document states the SHA it measured at; this reads + that SHA back out of the prose and re-measures. New commits on `main` do not falsify + it and do not jam the queue — a hand-maintained "commits on main" counter that every + merge invalidates is a gate that stops the repository, and this is deliberately not + that. What does fail it is somebody advancing the SHA without re-measuring, or editing + a number without moving the SHA. + +2. **The required status check contexts.** The document itself warns that "a required + context that matches nothing is a gate that has silently gone away". Every context in + `main.json` is derived here from the workflow files, so renaming a job breaks this test + instead of silently emptying the profile on the day it is applied. + +Fail-closed throughout, per ADR-0004: a missing document, an unparseable one, a SHA that +is not present in the checkout, or a prose sentence this module cannot find is a failure, +never a skip and never a default. A shallow clone that cannot see the pinned commit fails +here rather than passing vacuously; `.github/workflows/ci.yml` gives the `verify` job +`fetch-depth: 0` for exactly this reason, and losing that must break something. +""" + +from __future__ import annotations + +import json +import re +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +RULESET = ROOT / ".github" / "rulesets" / "main.json" +RULESET_DOC = ROOT / ".github" / "rulesets" / "README.md" +CI_WORKFLOW = ROOT / ".github" / "workflows" / "ci.yml" +CODEQL_WORKFLOW = ROOT / ".github" / "workflows" / "codeql.yml" + +MEASURED_AT = re.compile(r"Re-measured (\d{4}-\d{2}-\d{2}) at\n`([0-9a-f]{7,40})`") +COMMIT_COUNT = re.compile(r"the tip of `main`: \*\*(\d+) commits\*\*") +MERGE_COUNT = re.compile(r"the \*\*(\d+)\*\* merge commits already on `main`") + + +def read_doc() -> str: + """The ruleset prose, or a failure. Never an empty string a regex would then miss in.""" + if not RULESET_DOC.is_file(): + pytest.fail(f"{RULESET_DOC} is missing; its claims are what this module checks") + text = RULESET_DOC.read_text(encoding="utf-8") + if not text.strip(): + pytest.fail(f"{RULESET_DOC} is empty") + return text + + +def one_match(pattern: re.Pattern[str], text: str, what: str) -> re.Match[str]: + """Exactly one match, or a failure naming what could not be found. + + Zero matches means the sentence was reworded and this gate stopped watching it, which + must be loud. Two means the claim is stated twice and the copies can disagree. + """ + found = pattern.findall(text) + if not found: + pytest.fail( + f"{RULESET_DOC} no longer states {what} in the form this gate reads " + f"({pattern.pattern!r}). Re-word the gate with the prose, or the number " + "stops being checked." + ) + if len(found) > 1: + pytest.fail(f"{RULESET_DOC} states {what} {len(found)} times: {found}") + match = pattern.search(text) + assert match is not None + return match + + +def git(*args: str) -> str: + result = subprocess.run( # noqa: S603 -- fixed argv, no shell, no untrusted input + ["git", "-C", str(ROOT), *args], # noqa: S607 -- git is the thing being read + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + pytest.fail( + f"`git {' '.join(args)}` failed in {ROOT}: {result.stderr.strip()}. " + "If the pinned commit is missing, the checkout is shallow: " + "`.github/workflows/ci.yml` must keep `fetch-depth: 0` on the verify job. " + "A measurement that could not run is not a measurement that agreed." + ) + return result.stdout.strip() + + +def pinned_sha() -> str: + """The commit the document says it measured at, proven present in this checkout.""" + sha = one_match(MEASURED_AT, read_doc(), "the commit it measured at").group(2) + kind = git("cat-file", "-t", sha) + if kind != "commit": + pytest.fail(f"{sha} is a {kind}, not a commit") + return sha + + +def test_the_documents_commit_count_is_what_that_commit_carries() -> None: + """Sixty-six, and it stays sixty-six because the SHA does not move on its own.""" + claimed = int(one_match(COMMIT_COUNT, read_doc(), "the commit count").group(1)) + actual = int(git("rev-list", "--count", pinned_sha())) + assert claimed == actual, ( + f"{RULESET_DOC} claims {claimed} commits on `main` at the pinned commit; that " + f"commit carries {actual}. Re-measure, or move the pin and re-measure." + ) + + +def test_the_documents_merge_commit_count_is_what_that_commit_carries() -> None: + """The count `required_linear_history` is argued against.""" + claimed = int(one_match(MERGE_COUNT, read_doc(), "the merge commit count").group(1)) + actual = int(git("rev-list", "--count", "--merges", pinned_sha())) + assert claimed == actual, ( + f"{RULESET_DOC} claims {claimed} merge commits on `main` at the pinned commit; " + f"that commit carries {actual}." + ) + + +def test_the_measurement_date_is_not_older_than_the_pinned_commit() -> None: + """A date earlier than the commit it claims to have read is a copied-forward date.""" + doc = read_doc() + claimed_date = one_match(MEASURED_AT, doc, "the measurement date").group(1) + commit_date = git("show", "-s", "--format=%cs", pinned_sha()) + assert claimed_date >= commit_date, ( + f"{RULESET_DOC} says it measured on {claimed_date} at a commit dated " + f"{commit_date}, which cannot have happened." + ) + + +def workflow_job_ids(path: Path) -> set[str]: + """Top-level job ids in a workflow, read as text rather than with a YAML parser. + + The repository's runtime has no YAML dependency and this gate is not worth adding one + for: job ids are two-space-indented keys under `jobs:`, and the shape is stable. + """ + if not path.is_file(): + pytest.fail( + f"{path} is missing; the contexts this profile requires come from it" + ) + text = path.read_text(encoding="utf-8") + body = text.split("\njobs:\n", 1) + if len(body) != 2: + pytest.fail(f"{path} has no top-level `jobs:` block") + return set(re.findall(r"^ ([A-Za-z0-9_-]+):$", body[1], re.M)) + + +def workflow_job_names(path: Path) -> set[str]: + """`name:` values on jobs, which are the contexts GitHub reports when present.""" + if not path.is_file(): + pytest.fail(f"{path} is missing") + text = path.read_text(encoding="utf-8") + return set(re.findall(r"^ name: (.+)$", text, re.M)) + + +def required_contexts() -> list[str]: + if not RULESET.is_file(): + pytest.fail(f"{RULESET} is missing") + ruleset = json.loads(RULESET.read_text(encoding="utf-8")) + for rule in ruleset.get("rules", []): + if rule.get("type") == "required_status_checks": + checks = rule["parameters"]["required_status_checks"] + return [check["context"] for check in checks] + pytest.fail(f"{RULESET} declares no required_status_checks rule") + + +def reportable_contexts() -> set[str]: + """Every context the two pull-request workflows can report.""" + return ( + workflow_job_ids(CI_WORKFLOW) + | workflow_job_names(CI_WORKFLOW) + | workflow_job_names(CODEQL_WORKFLOW) + ) + + +def test_every_required_context_is_a_job_that_exists() -> None: + """A required context matching no job is a deadlock on apply, and a silent hole before it.""" + available = reportable_contexts() + missing = [c for c in required_contexts() if c not in available] + assert not missing, ( + f"{RULESET} requires status checks no workflow reports: {missing}. Available: " + f"{sorted(available)}. Applying this would deadlock every pull request; leaving " + "it applied after a rename would quietly stop requiring the renamed job." + ) + + +def test_the_profile_requires_at_least_the_five_contexts_it_documents() -> None: + """A positive control: the check above would also pass on an empty context list.""" + contexts = required_contexts() + assert len(contexts) >= 5, ( + "the documented profile requires five contexts; this file requires " + f"{len(contexts)}: {contexts}. A shorter list passes the matching test " + "vacuously, which is why this one exists." + ) + + +@pytest.mark.parametrize( + "context", + [ + "verify", + "secret-scan", + "sast", + "zizmor", + "codeql (actions · python · javascript)", + ], +) +def test_each_documented_context_is_still_required(context: str) -> None: + """The five named in the README's table, held one by one so a drop names itself.""" + assert context in required_contexts(), ( + f"{context!r} is documented in {RULESET_DOC} as a required check and is not in " + f"{RULESET}." + ) + + +def test_the_context_check_rejects_a_context_that_matches_nothing() -> None: + """Negative control on the matcher itself, not on the committed file.""" + available = reportable_contexts() + assert "a-job-that-does-not-exist" not in available + + +def test_the_document_does_not_read_local_signature_status_as_truth() -> None: + """The trap that produced the wrong audit must stay written down. + + A local `git log --format='%G?'` reports `N` for this repository's SSH-signed commits + when no allowed-signers file is configured. Reading that as "unsigned" is how an + audit reported thirteen unsigned commits that do not exist. The document explains it; + if that explanation is deleted the next reader repeats the mistake. + """ + doc = read_doc() + for fragment in ("%G?", "allowedSignersFile", "verification.verified"): + assert fragment in doc, ( + f"{RULESET_DOC} no longer mentions {fragment!r}; the local-verification trap " + "it records is how this repository's signature evidence was misread once." + )