Repository navigation
fix(ci): verify the published pages instead of pushing them to main - #39
Merged
Merged
Conversation
Both leaderboard.yml and benchmark.yml order their gate step last on purpose — the page publishes first, then the gate fails the run so a broken defense cannot be missed. That ordering was unguarded: when the push step failed, the job aborted before the gate ever ran. The scheduled leaderboard run on 2026-09-07 is the case in point. It regenerated the page, was rejected pushing to protected main, and exited without ever executing "every governed defense must hold" — so a genuine defense regression would have been indistinguishable from a git error. Guard both gates with `!cancelled() && steps.regen.outcome == 'success'` so publishing and judging are independent: the gate runs whenever there are numbers to judge, and is skipped only when regeneration produced none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pyproject pins signetry-core @ v0.8.0, but the committed page still labelled the live row 0.7.0 — the version the page last successfully published under. Every run since has regenerated the correct label and then failed to push it, so the page has been stale rather than wrong-by- generation. This is byte-for-byte what the leaderboard workflow produces: regenerated with `signetry-eval leaderboard --with-detection --version 0.8.0`, the same command and engine-detection branch CI takes, yielding the identical two-line diff (one in .md, one in .json) that the failed runs attempted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`leaderboard.yml` and `benchmark.yml` regenerated their page and pushed the
result straight to `main`. `main` requires a pull request, so every run that
actually had something to publish was rejected by branch protection:
remote: error: GH006: Protected branch update failed for refs/heads/main.
remote: error: - Changes must be made through a pull request.
The weekly refresh (run 34128111507, 2026-09-07) and the v0.3.0 release run
(33526247640, 2026-09-01) both died there, so the published numbers went stale
rather than current — LEADERBOARD.json still labelled the live row 0.7.0 after
pyproject was pinned to signetry-core v0.8.0. benchmark.yml carried the same
bug and only looked green: its push step exited 0 on "no change to publish".
Both workflows now verify instead of publish. Each regenerates its page, diffs
it against what is committed, and fails if the committed page is stale.
`permissions:` drops from `contents: write` to `contents: read`, so neither can
write to the repository at all, and both also run on `pull_request` so a stale
page blocks the merge instead of being found a week later by the schedule.
Refreshing is an ordinary pull request — a governance product whose own numbers
arrive by a bot bypassing branch protection argues against its own thesis.
Four further defects the failure was hiding:
- The gates were masked. Both workflows ordered their gate (`signetry-eval run`
for the governed defenses, corpus parity for detection) after the push, and a
rejected push aborts the job before it. A broken defense or a detection
regression would have read as a git error. 55a7af2 added the `!cancelled()`
guard; tests/test_published_numbers.py now keeps it there.
- CI and the page documented different commands. The workflow inlined its own
copy of the generation commands while the page told readers to run
`signetry-eval leaderboard --markdown`, which omits --with-detection and
--version and so produces a different page. Under a regenerate-and-diff check
that drift is a stale-page failure nobody can explain from the log. The new
Makefile is the single recipe, for CI, for contributors, and in the
instructions printed on the page itself.
- pyproject.toml was not a trigger path, though the pinned signetry-core is the
live row's version label — precisely the staleness that went unnoticed.
- `git diff` says nothing about an untracked file, so a page deleted from the
repo and regenerated would have passed the check while not being published at
all. check-* asserts git tracks the page first.
CONTRIBUTING.md documents the `make published-numbers` refresh path, and
`make verify-published-numbers` reproduces the whole check locally.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signetry Reviewer — 🟣 Escalate to a designated reviewerEscalate to a designated reviewer — this PR touches security-sensitive surface (.github/workflows/benchmark.yml, .github/workflows/leaderboard.yml). No blocking issue was found automatically, but a human owner should sign off. Deterministic gates (the authority)
Findings (2, 0 blocking)
Sensitive surfaceThis PR changes security-sensitive paths that warrant a designated reviewer:
MergeA designated reviewer / code owner should sign off before merge (sensitive surface).
|
Two bugs in the verify design, both found by running it on its own pull request (#39), where `leaderboard` and `benchmark` failed. The checkout ref was a falsy ternary: ref: ${{ github.event_name == 'pull_request' && '' || (... || 'main') }} Actions' `&&` and `||` yield operand values, not booleans, so a falsy true branch collapses straight through: `true && ''` is `''`, and `'' || 'main'` is `'main'`. Every event checked out the default branch, so the PR verified `main` instead of itself — the check said nothing about the change under review. Now the *release* case is the true branch (its ref is the tag, not where the page is committed) and everything else verifies `github.ref`, which on a pull request is `refs/pull/N/merge`: the merge result, which is what actually lands. Both branches are non-empty strings. The second bug is why that one was survivable rather than obvious, and it is the worse of the two. `main` carries no Makefile, so the regeneration step ran `make leaderboard` against a tree without the recipe — and make exited 0: make: Nothing to be done for 'leaderboard'. `leaderboard/` is a real directory in this repository, so with no Makefile to say otherwise make treats the target as already up to date. The step reported success, uploaded the untouched committed page as its artifact, and only the diff step failed — with "the page is stale", which was not the problem. That is the same shape as the push step that exited 0 on "no change to publish": a step that succeeds without doing its job. Both workflows now confirm the Makefile is present before trusting `make`, and say "wrong ref?" when it is not. tests/test_published_numbers.py grows three guards, each mutation-checked against the bug it describes: no ternary with an empty true branch, the ref expression must verify `github.ref` and special-case only `release`, and the regeneration step must assert the recipe exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The leaderboard could not publish the leaderboard
leaderboard.ymlandbenchmark.ymlregenerated their page and pushed the result straight tomain.mainrequires a pull request, so every run that actually had something to publish was rejected by branch protection:So the published numbers went stale rather than current:
docs/LEADERBOARD.jsonstill labelled the live row0.7.0afterpyproject.tomlhad been pinned tosignetry-core v0.8.0.benchmark.ymlcarried the identical bug and looked green the whole time — its push step exited 0 onno change to publish, so the failure only appeared once the table actually changed.Four more defects the first one was hiding
signetry-eval runfor the governed defenses, corpus parity for detection — after the push. A rejected push aborts the job before the gate, so a broken defense or a detection regression would have surfaced as a git error, or not at all.signetry-eval leaderboard --markdown— which omits--with-detectionand--version, and therefore produces a different page. Under a regenerate-and-diff check, that drift is a stale-page failure nobody could explain from the log.pyproject.tomlwas not a trigger path, though the pinnedsignetry-coreis the live row's version label. That is precisely the staleness that went unnoticed above.git diffsays nothing about an untracked file, so a page deleted from the repo and then regenerated would have sailed through a diff-based check as "current" while not being published at all.The fix: verify, don't publish
Both workflows now regenerate their page, diff it against what is committed, and fail if the committed page is stale.
permissions:drops fromcontents: writetocontents: read, so neither workflow can write to this repository at all.Refreshing is an ordinary pull request. A governance product whose own numbers arrive by a bot bypassing branch protection is arguing against its own thesis.
contents: writecontents: readmain(rejected)make leaderboard, commit, open a PR"Makefile, shared by CI and contributorsif: !cancelled() && steps.regen.outcome == 'success'pull_request, +pyproject.toml/Makefilepathspublished-numbersgroup${{ github.workflow }}-${{ github.ref }}One recipe, in a
MakefileCI runs these targets rather than its own copy of the commands, so the page you regenerate locally is byte-for-byte the page CI expects. The generated headers and the in-page instructions now name
make leaderboard/make benchmark— the command that actually writes the file.check-*asserts git tracks the page before diffing it, closing the untracked-file hole.The design is now enforced by tests, not by memory
New
tests/test_published_numbers.py(11 tests) asserts that neither workflow requests write access, neither runsgit push/git commit/git config user, both regenerate viamake <target>and verify viamake check-<target>, both gates keep the!cancelled()guard, and each committed page's header names its recipe and its verifying workflow.Mutation-checked: re-granting
contents: write, re-adding agit push, and deleting the gate guard each fail these tests. They run insidetest (3.11/3.12/3.13), which is already a required check — so the regression cannot come back quietly.Verification
Clean clone of the head commit, then exactly what CI runs:
make check-leaderboardmake check-benchmarksignetry-eval runsignetry-eval corpus --min-recall 1.0 --max-fp 0pytest -qruff check signetry_eval/ tests/actionlinton both workflowsThe only content change to the generated pages is the header line and the regeneration instructions — the numbers themselves are unchanged, which is the point.
Notes for review
test (3.11),test (3.12),test (3.13),cla),enforce_admins: true, strict. No ruleset was created and no bypass actor was added.pull_requestruns take a read-only token and use no secrets.CHANGELOG.mdgains an[Unreleased]entry rather than rewriting the released 0.3.0 section.CONTRIBUTING.mddocuments the refresh path in The published pages are generated, and in both "adding a case" checklists.🤖 Generated with Claude Code