Skip to content

Governance: a pure matcher for the PRs an instance governs, and a token only it can make (#704 S1) [PROTECTED: owner merges] - #833

Merged
jayden1711 merged 3 commits into
mainfrom
issue-704
Oct 20, 2026
Merged

Governance: a pure matcher for the PRs an instance governs, and a token only it can make (#704 S1) [PROTECTED: owner merges]#833
jayden1711 merged 3 commits into
mainfrom
issue-704

Conversation

@jayden1711

@jayden1711 jayden1711 commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Intent: This is slice S1 of governance mode (#704). It answers one question: "does this instance govern this PR the engine didn't open?" The answer is one pure function that fails closed. Its "yes" is a token that only that function can make. Nothing calls it yet, so no install's behaviour changes. Later slices build on it: mergeDecision's source input (S2), the governance policy key (S3), and intake plus mergeOurs in merge-flow (S4). The full slice plan is at the end.

Risk class: protected. It adds src/governance.ts, and src/** is engine (.worklane/review.yaml), so the owner merges.

Part of #704 (slice S1 of 9; it does not close the issue).

Round 2 (c074181e, after round 1's CHANGES NEEDED):

  • the token is bound to its repo;
  • every compared fact is validated as ASCII of its kind before an ASCII-only case fold;
  • the author's REST shape is pinned, with authorType;
  • the base-repo refusal has a test of its own;
  • the tarball delta is re-measured with the UI built.

Details are in each section below.

What it does

  • governedPr(source, scope, pr) governs a PR only when the owner's source names it. A PR it governs gets back a GovernedPr token. Any other PR gets why, a reason that starts "not governed:". The source can name it three ways:
    • authors: the author's login is on the list. The login is REST's user.login, verbatim, and must match whole, in any ASCII case.
      • An App's login keeps its [bot] suffix (renovate[bot]).
      • Some Bots have no suffix (Copilot).
      • User claude and Bot claude[bot] are two different accounts.
    • label: the label is on the PR now, and its latest application in the timeline was by an owner or writer.
    • all: every same-repo PR into the default branch.
  • What it never governs. Each of these is a refusal in the code, written as the issue's constraints ask:
    • governance off (source null);
    • no source set (the defaults);
    • a fork, or a head repo GitHub no longer has (headRepo: null);
    • an author it can't resolve (empty, ghost, or not a valid GitHub login);
    • an author whose REST user.type isn't User or Bot, or a [bot] login GitHub calls a User;
    • a base repo other than the instance's;
    • a base branch other than the default branch (compared exactly);
    • a head that isn't an exact SHA-1 or SHA-256;
    • a PR number that isn't a positive integer;
    • round 2: any repo name (scope.repo, pr.baseRepo, pr.headRepo) or branch name (scope.defaultBranch, pr.base) that is empty or isn't plain ASCII of its kind. Branch names follow git's check-ref-format rules, including a .lock component anywhere (a.lock/b, added after round 2's review). So two blanks never match.
  • ASCII-only case fold. Names are compared with ASCII case folded and nothing else. Unicode folding would map the Kelvin sign K onto k.
  • GovernedPr is a frozen {repo, number, head, via} with non-writable fields, and three helpers:
    • isGoverned(x) says whether x is a real token.
    • governs(token, repo, n, head) is true only for a real token minted for repo, for PR n, at exactly head.
    • Nothing else can say "governed".

By-design protection

  • An opaque token, with a check at run time. A real token comes only from governedPr(). Three things make this hold at run time, not just in the types:

    • GovernedPr's constructor throws unless it gets a symbol (MINT) that the module never exports.
    • isGoverned and governs accept only objects in a module-private WeakSet, which only the constructor adds to.
    • Each field is defined non-writable and non-configurable, and the token is frozen before it's added. Object.freeze and Object.defineProperty are captured when the module loads.

    So a real token can't be pointed at another repo, PR or head. That includes code that swaps Object.freeze out later (measured below).

  • What the later slices get. S2's mergeDecision and S4's mergeOurs will take a GovernedPr instead of a boolean. A caller can't claim "governed" with a literal, a cast, an any or a deserialized object: it throws or reads as not governed (measured below).

  • No lint is added. The protection is the run-time brand itself, the same pattern as Required checks count only from the App or workflow pinned for their name [PROTECTED: owner merges] #788's PinnedChecks.

What this can never do

  • Merge, hold or change any PR. Nothing imports src/governance.ts (inferred: it's a new file; measured: grep -rn "governance.js" src/ finds no importer).
  • Turn governance mode on. There's no policy key yet (S3). With source null, it governs nothing (measured: test "off (no source) governs nothing").
  • Govern a fork, an unresolved or mistyped author, another base repo or branch, a blank or non-ASCII name, or a label a viewer or unmapped account applied last (measured: the tests and the property test below).
  • Let a token for one repo govern another (measured: test "a token minted for one repo never governs another"). Round 1's claim that the token "ties it to one PR" was false across repos, because forks and mirrors share heads (measured by the reviewer). It's corrected here.
  • Vouch for its inputs. A token proves only that the facts it was given passed. Reading those facts from GitHub is S4's job: the PR from REST, the head repo, every page of the label timeline, and roles. S4 must read them itself, never from a webhook payload or a PR body (inferred; written in the module's header and the type's doc comments).

Compromised component

  • Agent user, or a worker in a worktree. It can propose a change to src/governance.ts, for example adding a source or dropping the fork check. That's an engine path, so the PR waits for the owner (inferred from .worklane/review.yaml's engine category). At run time it has no code path to the token: MINT isn't exported, and a forged object fails isGoverned (measured).
  • Coordinator process. Nothing in it calls this yet. Once S4 wires it, a coordinator that passes false facts mints a true token for them. The token can't detect lying inputs. One case it can't detect is a reader that drops a Bot's [bot] suffix. GraphQL does this: claude[bot] arrives as claude with type Bot. That matches a listed User claude (measured in the adversarial pass). Refusing a Bot with no suffix would also refuse Copilot. So the defence is S4's contract: REST user.login, verbatim, plus a reader test that a bot's suffix arrives (inferred; written into the slice plan).
  • Dashboard, hub viewer, other instances. No route reaches this module (inferred: no importer). Another instance's token names its own repo and fails governs() for this one (measured).
  • Someone who can open PRs on the repo, or an outside agent. Governance isn't on and nothing is wired, so none of their PRs is governed. Once it's wired, the cases they control are refused: a fork, a label they applied as a viewer, a look-alike login (measured in the adversarial pass).
  • Someone with push access. Same as today: changing this file is an owner-merged engine change (inferred).
  • Code running inside the engine's process. It could monkeypatch WeakSet.prototype.has before this module loads, or swap the module. The brand isn't a sandbox against code already in-process (inferred). Replacing Object.freeze or Object.defineProperty after load no longer leaves a token writable (measured).

How it was verified

  • Fail-without, round 1 (measured). The module was new, so the only fail-without was that the tests fail on the tree without it. I wrote test/governance.test.ts first and built it on unchanged origin/main 6e0d7d81:

    • npm run build: error TS2307: Cannot find module '../src/governance.js'
    • running the test: Error [ERR_MODULE_NOT_FOUND], # fail 1.

  • Fail-without, round 2 (measured). I updated the test file first and ran it against the round-1 module (ef3b100d, unchanged):

    • npm run build reported 24 type errors;
    • # tests 17, # pass 11, # fail 6.

    The six failures:

    • not ok 6 repo names validated as ASCII;
    • not ok 8 the author's REST shape;
    • not ok 11 empty or invalid branch names;
    • not ok 14 the token pinned to its repo;
    • not ok 15 a token minted for one repo never governs another;
    • not ok 17 the property test.

  • The isolated base-repo test catches its mutant (measured). I ran it against a scratch copy of the built module with the base-repo comparison removed. No repo file was edited.

    • Round 1's case (both repos acme/other): the mutant still refuses it, through the fork check, so it did not catch the mutant.
    • Round 2's case (base acme/other, head acme/app): the mutant governs it and the real module refuses it, so it catches the mutant.

  • After (measured): # tests 17, # pass 17, # fail 0.

  • Forgery tests (measured, in test/governance.test.ts). None of these is governed, and governs is false for each:

    • a structural copy;
    • the same fields by hand;
    • Object.create(GovernedPr.prototype) plus the fields;
    • Object.setPrototypeOf(fields, GovernedPr.prototype);
    • a JSON round trip;
    • structuredClone;
    • a Proxy of a real token;
    • Object.create(real).

    Building one also fails: the constructor with a wrong symbol, Reflect.construct and a subclass all throw. Writing any of repo, number, head or via throws TypeError, and each field's descriptor has writable: false.

  • Property test (measured). It runs 1,080 combinations: 6 sources × 5 authors × 3 head repos × 2 bases × 6 label states. In every case:

    • exactly one of token and why is set;
    • a governed result means: source on, same repo, default branch, a resolved author, and a via the source names;
    • governs(token, scope.repo, n, head) holds, and governs(token, 'acme/other', n, head) doesn't.

  • Neighbouring tests (measured). I ran the tests that scan all of src/ (package-files, review, feature-map, brand, isolate, push-check, change-files-renames, claude-credential-guard, owner-terminal-removed, slot-dir-writes, slot-path) plus governance: # tests 248, # pass 248, # fail 0. The full suite runs in CI.

  • Other checks (measured).

    • npm run typecheck: 0 errors.
    • npm run check:denylist: denylist: 1311 files clean.
    • node scripts/check-dco.mjs origin/main HEAD: DCO: all commits signed off.
    • The push gate: 1 commit(s) and 1 tip(s) clean.

Adversarial pass

I ran these with a scratch script against the built dist/src/governance.js at c074181e. No repo file was edited. The reviewer's round-1 probes (a second module instance, vm realms, workers) all failed closed, and the code paths they exercise didn't change (inferred).

Attack Result (measured)
Cyrillic look-alike author (аgent-one), under all or listed not governed: author unknown
Listed author with a trailing space; a--b not governed: author unknown
Kelvin-sign repo acme/Kit on both sides, scope acme/kit not governed: its base repo is not a valid name
Empty default branch and empty base not governed: the instance's repo or default branch is not a valid name
authorType missing not governed: author unknown or the wrong kind of account
claude[bot] with type User not governed (test)
GraphQL-shaped bot (claude, type Bot) against a listed User claude governed via authors. This is the one shape the matcher can't refuse without also refusing Copilot. It's closed by S4's REST contract and reader test (named gap)
governs with another repo; with acme/app\n false
Object.freeze and Object.defineProperty replaced after load, then a token minted and number set to 99 the assignment throws TypeError; governs(t, …, 99) false, governs(t, …, 9) true
Object.defineProperty(real, 'head', …) throws TypeError: Cannot redefine property: head
Reflect.set(real, 'number', 8) false, unchanged
Prototype pollution: GovernedPr.prototype.number = 8 the token's own fields win
governs with an upper-cased head, or the number as a string false
A Proxy over the class with a construct trap throws "made only by governedPr()"
Config label ' governed ' / blanks matches governed / no source set
Fork acme/app-fork; base Main against main not governed
Label applied by a role outside owner/writer (admin) not governed
A writer labels it, then a viewer removes and re-adds it not governed (test)

What I fixed because of it. In round 2, everything round 1's review found:

  • repo binding;
  • the Kelvin-sign fold and validation;
  • blank branch names;
  • the author's shape;
  • the isolated base-repo test.

On my own initiative I also defined the fields non-writable, with Object.freeze and Object.defineProperty captured at load. The reviewer had made that optional.

What it leaves open (named gaps).

  • The GraphQL-shaped bot, above. S4 must use REST user.login verbatim (inferred; in the slice plan).
  • Timeline completeness. S4's timeline reader must read every page, oldest first, and fail closed on a partial read. A truncated read that keeps a writer's early labeled but misses a viewer's later one would over-govern (inferred). The matcher can't detect truncation.
  • The labeller's role is read at decision time, not labelling time. A viewer promoted later makes an old application count. S4 documents that (inferred).
  • Two login checks disagree. src/dashboard-auth.ts validLogin accepts a--b and a- but not [bot]; LOGIN here is the reverse. Neither accepts Enterprise Managed User logins (name_shortcode). Both fail closed; EMU support is a question for the owner, and one shared helper is worth its own issue (inferred).
  • Legacy logins with a double or trailing hyphen are refused (guess: rare).

Package size

Measured with npm pack --dry-run --json --ignore-scripts through the npm wrapper. Main came from a separate worktree at 6e0d7d81 (this branch's base). Both trees ran npm run build and npm run build:web, so dist/web is included.

  • Tarball: 1,903,282 → 1,904,675 (+1,393).
  • Unpacked: 5,938,018 → 5,942,886 (+4,868).
  • Files: 512 → 513.
  • Per file: dist/src/governance.js +4,868 (new). That's the only file that changed; the test isn't packed.
  • Round 1's body said +1,195 for the tarball, measured without dist/web. The reviewer measured round 1's head with the UI at +953; the correct round-1 figure is +953.
  • Round 2 grows it by validation and repo binding.
  • PACKAGE_BUDGET (1,950,000 / 6,060,000) is unchanged and still has headroom (measured).

CHANGELOG

None: users see nothing, because nothing is wired (inferred). S3, where the policy key appears, adds the bullet.

New dependencies

None.

Slice plan (all of #704)

@jayden1711
governance: a pure matcher for the PRs an instance governs, and a tok…
ef3b100 
…en only it can make

Governance mode (#704) lets the engine judge PRs it didn't open. Before any of that is wired, the question "is this PR
governed?" needs one answer that fails closed and can't be faked. This adds it as a new module that nothing calls yet,
so no install changes behaviour.

governedPr(source, scope, pr) governs a PR only when the owner's source names it: its author is listed (whole login,
any case, bots included), a label is on it whose latest application was by an owner or writer, or "all". It never
governs with the mode off, with no source, a fork or a head repo GitHub no longer has, an unresolved author or ghost,
another repo or base branch, or a head that isn't an exact SHA.

The answer is a GovernedPr: frozen at birth, made only by the matcher (its constructor needs a symbol the module
never exports) and recognised only through a module-private WeakSet. A structural copy, Object.create, a JSON round
trip, structuredClone or a Proxy is never governed, and governs(token, n, head) ties it to one PR at one head.

Test (fails without): test/governance.test.ts. On the tree without src/governance.ts the build reports TS2307 and
the test run fails with ERR_MODULE_NOT_FOUND; the module is new, so that is the only fail-without there is.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 mentioned this pull request Oct 19, 2026
Merged
@jayden1711

jayden1711 commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 1/2)

Reviewed head ef3b100d against base 6e0d7d81 (= origin/main at review time). Labels: measured (command run, output quoted), inferred (reasoned from code, not run), guess.

The design is right. An opaque token checked at run time is a by-design protection, and every forgery I tried fails closed, across realms too. The required changes are four small fail-closed gaps in the matcher and its contract. Three of them over-govern on malformed or differently shaped facts, which is the dangerous direction.

Acceptance (S1's done-when, from the slice plan and the issue's "Source rules")

Criterion Result Evidence
A fork is never governed met measured: test "a PR from a fork…" passes; headRepo: null refused
A label applied by a viewer or a non-mapped account doesn't govern met measured: test lines 86-89 (viewer, some-bot[bot] with role null, removed then re-added by a viewer). Inferred: S4's natural role source, roleOf, returns null for any [bot] or App login, because validLogin rejects [ (src/dashboard-auth.ts:80,148)
An author not on the list isn't governed met measured: test lines 38-40 (other login, renovate vs renovate[bot], prefix)
Every refusal tested partly inferred from code: the base-repo refusal (governance.ts:115) has no test that fails without it (R3)
Forgery: copy, Object.create, JSON, mutation met measured, see "Token" below
typecheck, denylist met measured: npm run typecheck clean; denylist: 1311 files clean

Fail-without (method (b): a fresh worktree at the base, test written in with Write)

  • measured: npx tsc -p tsconfig.json on 6e0d7d81 plus the PR's test/governance.test.ts (byte-identical to the head copy: git diff --no-index exit 0) gives test/governance.test.ts(5,115): error TS2307: Cannot find module '../src/governance.js'.
  • measured: node --import ./dist/test/isolate.js --test dist/test/governance.test.js gives Error [ERR_MODULE_NOT_FOUND] … dist/src/governance.js, # tests 1, # pass 0.
  • measured: at the head, the same run gives # tests 12, # pass 12, # fail 0.
  • The PR body labels this fail-without "weak" (the tests fail only because the module is new). That label is honest (inferred: nothing existed before to compare against).

1. Matcher against GitHub's real data shapes

Probed with a scratch script against the built dist/src/governance.js. No repo file was edited.

Measured GitHub shapes (read-only gh api on public repos):

  • REST pulls/{n}.user: dependabot[bot] / Bot (chore(deps): bump the codeql-actions group with 3 updates cli/cli#14751) and Copilot / Bot (id 198982749, no [bot] suffix).
  • GraphQL author.login for the same dependabot PR is dependabot, with __typename: Bot. The suffix is dropped.
  • gh pr list --json author gives app/dependabot.
  • User claude (id 81847, type User) and Bot claude[bot] (id 209825114) both exist.
Case Result
[bot] suffix, any case governed when listed whole; renovate ≠ renovate[bot] (measured)
Copilot (a Bot with no suffix) under all governed (measured). The source comment "bots count (name[bot])" is incomplete (R4)
GraphQL-shaped author renovate vs listed person renovate governed (measured). With a GraphQL or gh reader, the bot claude[bot] reads as claude, which is a real, different User (measured above). The matcher can't tell them apart because PrFacts.author doesn't pin the REST shape (R4). Today's backlog reader uses REST user.login (src/backlog/github.ts:98), so S4 would probably be safe (inferred)
ghost, Ghost, '' refused (measured)
Case folding on logins ASCII only, in effect (measured): the Kelvin sign K lowercases to k, but LOGIN has no u flag, so Kit is "author is unknown"
Case folding on repos over-governs (measured): headRepo: 'acme/Kit' and baseRepo: 'acme/Kit' are both governed for scope acme/kit. scope.repo is validated, but pr.headRepo and pr.baseRepo aren't (R2). Not reachable from GitHub's own data, since repo and owner names are ASCII (inferred)
scope.defaultBranch: '' with pr.base: '' governed via all (measured) (R2)
Removed, then re-added by a viewer not governed (measured, test line 89)
Label applied by a bot or App not governed when the role is null (measured, test line 87)
Labels in any case match (measured); toLowerCase folds non-ASCII labels too (Kelvin Kit = kit, measured), but only an owner or writer's application counts, so this isn't an escalation (inferred)
EMU login mona-cat_octo (GitHub's documented IDP-USERNAME_SHORT-CODE form, measured from docs.github.com) refused: "author is unknown" (measured). Fails closed, but misses every Enterprise Managed User. See the owner question
a--b, a- refused (measured); fails closed, as the body says

The token isn't bound to a repo (measured): a token minted for scope acme/other, PR 7 at HEAD, gives governs(token, 7, HEAD) === true. A PR is identified by repo and number, and the same head SHA exists in a repo, its forks and its mirrors. Today one Coordinator runs per process (inferred from src/service.ts:398). Even so, the body's claim "ties it to one PR at one head" is not accurate (R1).

2. Forging the token (all measured)

  • Second module instance (import(url + '?second')): the two are distinct, and each instance's token fails the other's isGoverned, governs and instanceof.
  • vm context:
    • the same real object passed through is still governed (correct);
    • a spread copy, structuredClone and a prototype-swapped copy made in the other realm are all refused;
    • a strict write from the other realm throws TypeError: Cannot assign to read only property 'number'.
  • worker_threads:
    • a real token posted to a worker and back is refused here and in the worker;
    • a token the worker mints with its own module instance is refused here, by both isGoverned and governs.
  • The WeakSet fails closed across realms (measured).
  • Known in-process limit (measured): with Object.freeze monkeypatched to a no-op before minting, a real token can be re-pointed (governs(t, 99, HEAD) gives true). The body already scopes in-process code out ("not a sandbox"). This is optional hardening, not required: capture Object.freeze at module load, or define the fields non-writable.

3. Slice plan and overlap (measured: gh pr diff <n> --name-only for each of #746 #769 #770 #775 #788 #789 #794 #803 #816 #818 #819 #820 #825 #826 #828 #829 #830 #831 #834 #835 #836)

4. Durability rule

  • By design: an opaque GovernedPr with a module-private mint symbol and a module-private WeakSet, checked at run time. The body names it under "By-design protection". No lint is added (measured: the diff has 2 files, neither a lint).
  • No model-compensating scaffolding (measured: the diff is a pure function and a test).

5. Package size (measured, npm pack --dry-run --json --ignore-scripts, with npm run build and npm run build:web in both worktrees)

  • Tarball: 1,903,282 → 1,904,235 (+953).
  • Unpacked: 5,938,018 → 5,941,358 (+3,340).
  • Entries: 512 → 513.
  • Body's numbers: the unpacked delta matches the body. The body's tarball delta (+1,195) was taken without dist/web, so its compression differs.
  • Budget: under PACKAGE_BUDGET (1,950,000 / 6,060,000, test/package-files.test.ts:114). No raise needed.

6. Protection, commits, CI

  • Protected: yes (measured). src/** is engine in main's .worklane/review.yaml:7. The title carries [PROTECTED: owner merges], the body has **Risk class:** protected, the needs-owner label is on, and both ## What this can never do and ## Compromised component are present.
  • "Can never do" claims: true. Nothing imports the module (inferred: it's a new file; the body's grep is cited).
  • Commit: 1 commit, Signed-off-by present, no Co-Authored-By or other attribution (measured: git log -1 --format=%B). The PR body has no attribution line (measured: grep count 0).
  • Body claims: labelled. "Part of Governance mode: the engine runs only its evaluator, merge policy and locks in front of PRs other agents or people open, with no agents of its own; external reviews and CI are policy inputs, never the decision #704" is correct, since this slice doesn't close the issue.
  • Name, dependencies, scope: no product name in src/governance.ts (measured: grep count 0). No new dependencies (CI new-deps pass). No scope creep: 2 new files, as planned.
  • CI on ef3b100: all green (measured: gh pr checks 833). That covers all 8 test shards on Node 22 and 24, dco, denylist, push-gate, scope, new-deps, agentshield and scan. The real-systemd jobs were skipped, as is normal for a non-setup change.

Required changes

  1. Bind the token to its repo.
    • Carry repo (the scope.repo it was minted for) in GovernedPr, and make governs(token, repo, number, head) check it, case-insensitively over ASCII.
    • Add a test that a token minted for acme/other doesn't govern the same number and head in acme/app.
    • Principle 1, and principle 5: the claim "ties it to one PR" is currently false across repos, measured above.
  2. Validate every fact the matcher compares, not only scope.repo.
    • Refuse a pr.baseRepo or pr.headRepo that fails REPO, or fold ASCII only. Either closes the measured Kelvin-sign match.
    • Refuse an empty or invalid scope.defaultBranch and pr.base. That closes the measured '' === '' → governed case.
    • A test for each. Principle 1: the matcher's job is to fail closed on malformed facts. It already does this for the head, the number and the scope repo.
  3. Test the base-repo refusal on its own.
    • Add a case with baseRepo: 'acme/other' and headRepo: 'acme/app'.
    • Today the only case (test line 73) sets both repos to acme/other, so the fork refusal would still pass it if governance.ts:115 were deleted (inferred from the check order).
  4. Pin the author's data shape in the contract that S4 builds on.
    • PrFacts.author is the REST pulls user.login, verbatim: bots keep [bot]. It's never GraphQL's author.login, which drops the suffix (measured: dependabot), and never gh's app/<slug>.
    • Cite the measured collision: User claude vs Bot claude[bot].
    • Fix the comment "bots count (name[bot])" to note that Copilot is a Bot with no suffix (measured).
    • Preferred, by design: add authorType: 'User' | 'Bot' from REST user.type, and refuse a [bot] login whose type isn't Bot, or any type other than User or Bot.
    • Add the S4 obligation to the slice plan: S4's reader test asserts that a bot author reaches the matcher with its suffix.

Named gaps (not blocking)

  • S4's timeline reader must read every page and fail closed on a partial read. A truncated read that holds a writer's early labeled event but misses a viewer's later re-application would over-govern, and the matcher can't detect truncation (inferred).
  • The label rule uses the labeller's role at decision time, not at labelling time. A viewer promoted later makes their old application count. That's S4's choice to document (inferred).
  • Two login regexes now disagree. src/dashboard-auth.ts:80 validLogin accepts a--b and a- but not [bot]. governance.ts:91 is the reverse. Neither accepts EMU _shortcode logins. Consider one shared helper (inferred).

Question for the owner

@jayden1711
governance: the token names its repo, and every fact is validated bef…
c074181 
…ore it is compared

Review round 1 on #833 found the matcher over-governing in three ways, all measured: a token minted for another
repo passed governs() for the same number and head (forks and mirrors share heads); repo names were case-folded with
Unicode rules unvalidated, so the Kelvin sign made acme/Kit match acme/kit; and an empty default branch matched an
empty base. It also found the author's data shape unpinned: GraphQL drops a Bot's [bot] suffix, and User claude and
Bot claude[bot] are different accounts.

Now GovernedPr carries the repo it was minted for and governs(token, repo, number, head) checks it. Every repo,
branch and login is validated as plain ASCII of its kind before an ASCII-only case fold, and an empty or invalid one
is refused on either side. PrFacts pins REST's user.login verbatim plus user.type: a type other than User or Bot, or
a [bot] login GitHub calls a User, is refused. The token's fields are defined non-writable with Object.freeze and
Object.defineProperty captured at load. A test isolates the base-repo refusal (base acme/other, head acme/app).

Test (fails without): test/governance.test.ts. Against the round-1 module the updated file fails 6 of 17 (repo names,
author shape, branch names, the token's repo binding twice, the property test).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Round 2: c074181e

Each required change, with its evidence.

  1. The token names its repo.
    • GovernedPr {repo, number, head, via}, and governs(token, repo, number, head) checks the repo after validating it, with an ASCII-only fold.
    • Tests: "the token: frozen, pinned to its repo, PR and head" and "a token minted for one repo never governs another".
    • The body's "ties it to one PR" claim is corrected (measured: both tests pass, and both failed on ef3b100d).
  2. Every compared fact is validated before it is compared.
    • scope.repo, pr.baseRepo and pr.headRepo must be ASCII owner/name.
    • scope.defaultBranch and pr.base must be non-empty, ASCII and valid under check-ref-format. Branches are compared exactly; names fold ASCII case only.
    • Tests: "repo names: validated as ASCII…" (Kelvin sign on each side) and "an empty or invalid branch name, on either side, is refused" (measured: both failed on ef3b100d; both pass now).
  3. The base-repo refusal is tested on its own. The case is baseRepo: acme/other, headRepo: acme/app. Measured against a scratch mutant of the built module with the comparison removed:
    • round 1's case doesn't catch the mutant;
    • the new case does.
  4. The author's REST shape is pinned.
    • PrFacts.author is documented as REST user.login verbatim, never GraphQL or gh's app/<slug>.
    • authorType: 'User' | 'Bot' is required. Any other type is refused, and so is a [bot] login typed User.
    • The comment now names Copilot and the claude / claude[bot] pair.
    • Test: "the author is REST's user.login verbatim, with its user.type" (measured: failed on ef3b100d; passes now).
    • The slice plan records S4's obligations: the REST shape, a reader test that a bot's suffix arrives, every timeline page, and failing closed on a partial read.
  5. Body and plan.

Also done, the optional item. The fields are defined non-writable, and Object.freeze and Object.defineProperty are captured at load. Measured: with both replaced after load, a newly minted token still throws on assignment and doesn't govern the re-pointed number.

Fail-without (measured). The updated test/governance.test.ts against the round-1 module gives # tests 17, # pass 11, # fail 6. At c074181e it gives 17 of 17. The neighbouring tests pass 248 of 248. Typecheck is clean and the denylist reports 1311 files clean.

A remaining gap (measured). A GraphQL-shaped bot (claude, type Bot) matches a listed User claude. Refusing it would also refuse Copilot. It's closed by S4's REST contract and reader test, and named in the body.

@jayden1711

jayden1711 commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: PASS

Round 2, head c074181e, base 6e0d7d81. Labels: measured (command run, output quoted), inferred (reasoned from code, not run), guess.

All four of round 1's required changes are met, and the protection is by design. Every attack below failed closed except one: the GraphQL-shaped bot. The builder names that one as a gap, and it can't be closed in S1 from login and type alone (measured, see §2). One claim needs a small correction (§3), which doesn't block the merge.

Round-1 required changes

# Change Result Evidence
R1 Token bound to its repo met measured: a token for acme/kit → governs(t, 'acme/other', 7, HEAD) gives false; a token for acme/other → governs(t, 'acme/kit', …) gives false; ACME/Kit gives true
R2 Validate repos and branches met measured: Kelvin acme/Kit as head repo → "comes from a fork"; as base repo → "base repo is not a valid name"; empty default branch and empty base → "repo or default branch is not a valid name"
R3 Isolated base-repo test met measured: test 10 passes; inferred from code: with governance.ts:140 removed, base acme/other / head acme/app passes the fork check and would be governed. The builder's mutant run agrees; I didn't repeat it (rule 8)
R4 Author shape pinned met measured: authorType must be User or Bot, and claude[bot] typed User is refused; doc comments name REST user.login, Copilot, and claude vs claude[bot]

1. Attacks (scratch script against the built dist/src/governance.js; no repo file edited)

Round-1 attacks, rerun (all measured):

  • Second module instance (?second): each instance's token fails the other's governs.
  • vm realm:
    • the same object passed through is still governed;
    • spread, structuredClone and prototype-swapped copies are refused;
    • a strict write throws TypeError, and number stays 7.
  • worker_threads:
    • a token minted in the worker is real there, but refused here;
    • a real token echoed back is refused both here and in the worker.

The new validation (all measured):

  • Unicode in owner names: a Cyrillic аcme/kit is refused as head repo, base repo and scope.repo.

  • Repo shapes: each of these is refused on both sides: acme/kit/, /acme/kit, acme//kit, a trailing or leading space, a trailing \n, and a bare acme.

  • Branches with ..: ma..in and .. are refused.

  • Other branch shapes, all refused: .main, main., main.lock, feat/.x, @, a@{b, a space or tab, máin, -main, a//b, ~ ^ : ? * [ \.

  • refs/heads/main:

    • against main (either way round): refused, because branches are compared exactly;
    • as both the default branch and the base: governed. git accepts it as a branch name (git check-ref-format --branch refs/heads/main exits 0), so this is correct.

  • authorType outside User/Bot: each of these is refused: Organization, Mannequin, bot, user, BOT, '', undefined, null, and 'Bot '.

  • Author shapes:

    • Copilot typed Bot, listed: governed (correct);
    • Copilot's GraphQL shape copilot-swe-agent: not governed, which fails closed;
    • app/renovate, ghost, and '' typed Bot: all refused.

  • Labels: a Kelvin-sign Kit no longer matches a configured kit, because the fold is ASCII only.

  • Object.freeze patched after load (defineProperty too): the write throws TypeError, the token stays frozen, and governs(t, …, 99) gives false.

  • Object.freeze and defineProperty patched before load, in a fresh instance:

    • that instance mints writable tokens, which can be re-pointed and pass its own governs;
    • the main instance refuses them.

    This is code that runs before the module loads, which the body scopes out ("Code running inside the engine's process").

2. Can S1 refuse the GraphQL-shaped bot without refusing Copilot? No, not from login and type alone (measured)

  • The measured shapes:
  • Why login and type can't decide: REST's Copilot / Bot has exactly the same shape as a GraphQL-stripped claude / Bot. Any rule like "a Bot needs [bot]" refuses Copilot.
  • Closing it by design in S1 or S3 without a code allowlist: proposal only; this changes the shape of the owner's config, so it's the owner's call:
    • (a) Typed entries. A plain list entry matches only a User-typed author, a name[bot] entry matches only a Bot, and a Bot with no suffix needs an entry the owner explicitly types as a Bot (e.g. { login: 'Copilot', type: 'Bot' }). A GraphQL-stripped claude / Bot then matches nothing the owner wrote for the User claude (inferred). This is the owner's own list, not a built-in allowlist, so it isn't a principle-9 exception.
    • (b) Account ids. S3 resolves each listed login to its numeric id when governance is turned on, using REST users/{login}: measured, claude is 81847, claude[bot] is 209825114, Copilot is 198982749. PrFacts then carries user.id. This also closes the case where a renamed login is re-registered (inferred). Whether GraphQL's databaseId equals REST's id is a guess.
    • (c) A built-in list of known suffix-less Bots. That would be a principle-9 loosening, so it's only noted here, not proposed.
  • Today: the existing reader is REST user.login (src/backlog/github.ts:98), and S4's contract and reader test are recorded in the slice plan. The gap is named correctly in the body (measured: the "Compromised component" section and the slice plan's S4 obligations).

3. One claim to correct (not blocking)

  • The claim: the BRANCH comment and the body both say branch names "follow git's check-ref-format rules".
  • Where it's false: a.lock/b is accepted (measured: as both the default branch and the base, it's governed), but git check-ref-format --branch a.lock/b gives fatal: … not a valid branch name (measured). git rejects any component ending in .lock, not only the whole name.
  • Impact: none reachable. Both sides would have to be the same name, and GitHub can't hold such a branch (inferred).
  • Fix: principle 5 still wants the claim corrected. Add (?!.*\.lock(?:\/|$)), or say "close to check-ref-format". Either can land in this PR before the owner merges, or in S3.

4. Fail-without (method (b): fresh worktrees, with the test written in using the Write tool; byte-identical to the head copy, measured by git diff --no-index --exit-code → 0)

  • Base 6e0d7d81 (measured):
    • tsc → test/governance.test.ts(5,115): error TS2307: Cannot find module '../src/governance.js';
    • the run → Error [ERR_MODULE_NOT_FOUND], # tests 1, # fail 1.
  • Round-1 module ef3b100d (measured): # tests 17, # pass 11, # fail 6.
    • The six that fail: not ok 6 repo names, 8 author shape, 11 branch names, 14 token pinned to its repo, 15 cross-repo token, 17 property.
    • Test 11 fails on its first case, both "": a real behavioural failure, not a compile error.
    • This matches the builder's report.
  • Head c074181e (measured): # tests 17, # pass 17, # fail 0.

5. Durability rule

  • By design: the protection is an opaque GovernedPr, checked at run time:
    • a module-private mint symbol;
    • a module-private WeakSet;
    • non-writable fields;
    • freeze and defineProperty captured at load.
  • It's named under "By-design protection". No lint is added, and there's no model-compensating scaffolding (measured: the diff touches src/governance.ts and test/governance.test.ts only).

6. Package size (measured, npm pack --dry-run --json --ignore-scripts, with build and build:web in both trees)

  • Tarball: 1,903,282 → 1,904,675 (+1,393).
  • Unpacked: 5,938,018 → 5,942,886 (+4,868).
  • Entries: 512 → 513.
  • Against the body: matches the body exactly. The body also corrects round 1's figure to +953.
  • Budget: under PACKAGE_BUDGET (1,950,000 / 6,060,000).

7. Slice plan, overlap, CI, commits

Questions for the owner

  1. Author entries: by login and type, or by id?
    • The GraphQL gap can't be closed from login and type alone (§2, measured).
    • Should S3's authors take typed entries (a) or ids resolved when governance is turned on (b)? Either makes the GraphQL shape impossible by design, instead of resting on S4's contract.
    • A built-in list of suffix-less Bots (c) would be a loosening, so it isn't proposed.
  2. Enterprise Managed User logins (from round 1, still open). name_shortcode logins are refused here and by validLogin. Both fail closed. Should they be supported engine-wide, as a separate issue?

@jayden1711
governance: a branch name with a .lock component anywhere is refused,…
737d5cd 
… as git refuses it

The branch check claimed git's check-ref-format rules but refused `.lock` only at the end, so `a.lock/b` passed;
git rejects any component ending in `.lock` (measured by the #833 round-2 review). The lookahead now refuses it in
every component. This only refuses more, so it can't make a PR governed that wasn't.

Test (fails without): test/governance.test.ts, the branch-name test with `a.lock/b` and `x/a.lock/b`; against the
previous module it fails (16 of 17 pass).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711
jayden1711 merged commit 963eca3 into main Oct 20, 2026
26 of 27 checks passed
@jayden1711
jayden1711 deleted the issue-704 branch October 20, 2026 01:02
@jayden1711 jayden1711 mentioned this pull request Oct 20, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-owner

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jayden1711