Skip to content

v0.30.0

Choose a tag to compare

@github-actions github-actions released this 31 Jul 21:40
b53f931

Fixed

  • A braked run could push straight to the default branch (#542). The brake watched gh pr merge,
    the REST merge endpoints and Board-Merge.ps1 — and missed the simplest route of all:
    git push origin HEAD:main puts work on main with one command and matched nothing. Present in
    every version of the brake, including shipped 0.29.0, and found only when an external review was
    pointed at whether the door was really shut rather than at the change in front of it.

    Not quite an oversight, which is the interesting part: the delete pattern carried a comment
    calling HEAD:main "an ordinary push refspec", written while guarding against over-matching.
    A test pinned that reading in place. Both were correct about the delete pattern and wrong about
    the vocabulary — this guard defines merge as "putting work on the default branch", which is
    exactly what that refspec does. The obsolete assertion was corrected, not worked around.

    Now refused: refspecs onto main/master (HEAD:main, branch:main, +HEAD:main,
    HEAD:refs/heads/main) and pushing the default branch by name.

    How the branch name must end took three tries, each one a real defect — and the CI reviewer
    caught both mistakes
    , in the first two reviews it managed to publish after #543 raised its turn
    cap. \b refused HEAD:main-cleanup and HEAD:master.bak: legitimate branches, blocked, on the
    run's most common command. Replacing it with (\s|$) fixed that and reopened a different
    hole
    — that only accepts a space or end-of-segment, and the segment splitter does not treat a
    lone & or a redirection as a separator, so git push origin HEAD:main& (background the push,
    same effect) matched nothing, a case the original \b had caught. It now ends at a lookahead
    over the shell's own terminator set, which covers both directions.

    A third review round found the anchor itself was wrong — and had always been. Every git
    rule in this file began \bgit\s+push\b, which demands that push follow git
    immediately. One ordinary global flag shook all of them off at once: git -C . push origin HEAD:main, git -c k=v push …, git --no-pager push …. That was never about the new patterns —
    the pre-existing branch-delete rules had the same hole, and git -C . push origin --delete f
    went through untouched. All four git rules now share one prefix that tolerates flag-shaped tokens
    between the program and the subcommand, so fixing it once fixed the older rules too. Only
    flag-shaped tokens are allowed in that gap, so git commit -m "push to main" is still not read
    as a push, and the repetition is bounded rather than * — this runs on every tool call, and an
    unbounded nested quantifier over adversarial input is a stall waiting to happen (measured: 1 ms
    against a command carrying 200 flag-like tokens).

    Round four found two more in that fix — and measuring the repair found a third nobody had
    seen.
    The flag allowance was capped at five, which was itself a bypass: six -c flags and the
    rule stopped matching, and chaining -c is ordinary scripting. And [:/] treated / as a
    refspec separator, which it is not — release/master, team/main and HEAD:team/main were all
    refused for merely ending in the default branch's name.

    Removing the cap came with an argument, made by the reviewer and accepted by me, that the
    repetition could not blow up because its character classes are disjoint. The argument was
    wrong and only the stopwatch said so:
    --flag repeated 1000 times ran the matcher past three
    minutes, because -{1,2} and [^\s;|&]+ can both consume the second dash — two readings per
    token, 2^1000 for a thousand of them. This code runs on every tool call, so that is not a slow
    path, it is a wedged session, and a wedged guard is a removed guard. The form now allows exactly
    one reading per token: measured at 3 ms with no match and 334 ms in the absurd worst case, with
    three timing tests to catch a return — the suite was green with the hang present.

    Recorded in the source, because this file keeps teaching it: "the classes are disjoint so it
    cannot blow up" is an argument, not a measurement. Time it.

    Round five found no bypass at all — the first round on this change that did not. Its one
    finding was about honesty rather than permission: git push origin :main deletes the remote
    default branch, but the merge rule matched it first (its source side allowed zero characters),
    so the refusal announced "merge is marked irreversible" for a command that removes main. Not a
    bypass — the contract filter runs per pattern, so a delete-braking contract already refused it —
    but a control is worth exactly as much as the account it gives of itself, which is this tool's
    founding defect. The refspec source now requires at least one character, so an empty source falls
    through to the delete rule where it belongs.

    Recorded as a decision rather than left to chance: a contract that brakes merges but not
    deletes no longer stops that command. It is a delete, and the guard follows the contract instead
    of inventing policy — the rule this file opens with. There is a test saying so.

    Round six, also no bypass, and the last one taken: the prefix before the refspec excluded
    only ;, so the matcher could step past a background & and read a later something:main in
    the same segment as the push target — git push origin fine & echo notes:main was refused for
    text belonging to a different command. A false positive, never a bypass (a looser prefix can only
    add matches), fixed because over-blocking on the run's most common command is the argument this
    whole entry rests on. The prefix now stops at the same operators the branch-name lookahead uses.
    The review's second example, > log:main.txt, did not reproduce — checked rather than assumed.

    Stopped here deliberately. Rounds one to four found real defects — a bypass, a false
    positive, an anchor broken for months, and a matcher that could hang the session. Rounds five and
    six found no bypass at all: a mislabelled verb and this over-block. The curve flattened, so the
    loop was ended on judgement rather than run until it produced nothing.

    Known and accepted: git push main is read as a push to the default branch even when main
    is the name of a remote. A false positive, not a bypass, on a rare spelling — and this pattern
    deliberately errs toward refusing.

    The original over-block, for the record: the tests missed it because the case they checked,
    maintenance, continues with t (a word character) and so passed for the wrong reason
    proving nothing about - or .. A green test that passes by accident is the same failure this
    whole entry is about. The reviewer also noted
    that HEAD:refs/heads/main was claimed in the description and the code comment but asserted
    nowhere; it has a test now. Deliberately still allowed, since
    this pattern sits on the run's most common command and over-blocking is how a control gets
    switched off: git push -u origin <branch>, a bare git push, --force-with-lease on its own
    branch, and any branch whose name merely contains main (issue-9-domain-model,
    feature/maintenance, HEAD:my-domain). Stated limit: the pure core cannot ask the repo what
    its default branch is, so a project whose default is neither main nor master is not covered.

  • The reviewer's turn cap sat one turn above what a real review needs (#543). Measured across
    recent runs: reviews that actually published consumed 4 / 13 / 17 / 18 / 19 turns against a
    cap of 20, and three consecutive runs on a large PR died at 21. A run that hits the cap leaves
    no review, so the verification added in #510 fails the check — correctly — and the review gate
    ends up blocking legitimate PRs on an infrastructure limit. That is how a control gets switched
    off: not by argument, but by being wrong often enough. Raised to 40, a little over twice the
    observed maximum; time was never the constraint (those runs finish in 2-4 minutes against a
    20-minute job timeout).

  • -EndToEnd shipped inert in 0.29.0, and after five review findings it is staying inert — on
    purpose
    (#536, #541). Field-testing the autonomy boundary found the ordered end-to-end close
    could refuse but never allow. Three separate blockers were fixed; then three rounds of external
    review found five live false-permission paths in the mechanism that would have opened it, two of
    them complete bypasses. The boundary held throughout — nothing merged that should not — and
    every failure failed safe.
    The order is now recorded, explained, and acted on nowhere.

    What was genuinely broken and is now fixed:

    1. The tests condition could never be satisfied. Invoke-BrakeMergeCheck dot-sources
      Board-ReviewGate.ps1, whose param() block runs in the caller's scope and declares
      [int]$PR = 0. $PR silently became 0 immediately before gh pr checks $PR, which exits 1
      with empty output, so the CI condition was always false. Proven against a real PR whose four
      checks all passed: the gate refused for "missing test evidence"; the same inputs without the
      clobber were permitted. Stated precisely: the bug is gone because the code that had it was
      withdrawn with the allowance, not because a line was patched around it. What remains is the
      trap, recorded where the gate will be rebuilt and guarded by a test that proves the review
      gate really does clobber a caller's $PR — so #541 cannot walk into it again.
    2. The tests requirement ignored the contract, being hardcoded $true; and reading it cast
      the string "false" to $true, the trap already closed for the marker's endToEnd.
    3. The brief never mentioned the order, so a session carrying one was briefed exactly like a
      session without one. It now says the order was given, that it cannot yet be acted on, and that
      a refused merge is the control working — not a failure to route around. Without that, a run
      reads its own refusal as an error and hunts for another way.

    Why the ordered close stays shut. Opening it — even only for the gate's own script, which
    re-checks four conditions and refuses on its own — made two latent holes reachable, and neither
    is a string-matching bug (#541):

    Bypass Why the gate could not defend itself
    cd C:\ ; pwsh <genuine-gate> -PR 42 The hook judges per segment and allows it; the gate then resolves its marker from its runtime cwd, finds none outside the worktree, and skips all four conditions. The two halves of the control disagreed about which directory the run was in, and the run picked
    [abios-review] … sha=<head> The review condition is a PR comment the run is able to post itself — the self-certification the design had already removed for the test condition

    Three earlier findings were fixed before these: the gated path matched a bare filename (any
    file so named passed), then a canonical path pattern — which was itself wrong, since the
    installed plugin lives at <version>/scripts/, so it would have refused the genuine gate in
    every project except this repo — then a substring test that a mention-as-argument satisfied,
    then a token match that a <gate>-bypass.ps1 prefix satisfied while breaking any install under
    C:\Program Files. Also fixed on the way: a failed gh pr checks read being laundered into
    "this project has no CI", and dod.tests: false waiving red CI rather than just the
    requirement to have a suite.

    The pattern is the finding: every round's tests passed, and the next round still found a live
    false-permission path.
    That is the argument for keeping the door shut until #541 gives the gate
    a trustworthy armed context and evidence the subject cannot mint.

Added

  • The brake hook is under test at last — the piece Claude Code actually executes, and the only
    one that can produce a refusal, had no tests. 17 now drive the real script over real stdin and
    pin both fail directions: silence outside an armed run, denial for anything unclear inside one
    (corrupt marker, emptied list, the string "true" posing as an order), plus the cd-out shape
    that motivated closing the ordered path.
  • Structural regression guards (AST) asserting the merge check never reads a name a dot-source
    destroys, and that the tests requirement is passed rather than assumed.
  • An adversarial sweep of 27 evasion classes — command substitution, backticks, env-var prefixes,
    redirections, line continuations, quote-splitting, bash -c wrapping, per-segment vouching —
    its catalogue taken from liberzon/claude-hooks (MIT, registered in knowledge/), which solves
    the same sub-command decomposition problem. Ideas, not code.