A captain-approved supersession can pass CI, on a signature - #62
Merged
Merged
Conversation
The required check `Base assertions re-verified` re-runs the base's assertions on a GitHub runner. It cannot see data/supersessions/<project>.md, which is captain-private and gitignored by design, so a branch the captain has approved superseding reports the same findings there and stays red forever. Nothing the branch pushes fixes it, because the branch is not what is wrong: it is the approval CI cannot read. The only ways out were a mislabelled CI waiver or a GitHub-UI merge - the exact bypass that check exists to prevent. This follows the CI waiver's own pattern rather than inventing a second one: a master key that never leaves the captain's machine, a key derived per repository for its Actions secrets, and a signed line in the PR body. - bin/fm-supersession-lib.sh stays the one parser and gains the one matcher, so the merge gate, the viewer, and CI answer "is this covered?" identically. Its canonical entry line is the wire between the record and a reader that cannot see it, and it carries no date and no reason. - bin/fm-supersession-attest-lib.sh owns the attestation's payload, entry token and published line. Its HMAC domain is separate from the waiver's, and so is the key it publishes: the waiver key skips a whole suite, this one only excuses findings the captain named, so a theft of one cannot escalate into the other. - bin/fm-supersession-attest.sh signs from the record itself, which is the authority: a worker holds neither the key nor a path anything invites it to write that record on. - bin/fm-supersession-verify.sh reads the PR body LIVE, because an approval always arrives as a body edit and a re-run replays the original payload. - bin/fm-reverify-base.sh excuses covered findings, blocks uncovered ones unchanged, names every override, and renders a fourth outcome, superseded, rather than reporting an authorized override as a pass. The workflow wiring and its tests are the next commits.
The verifying job checks out the BASE, like ci.yml's waiver job: the re-verification job runs the branch's own scripts by construction, so a secret in its environment would be a secret every PR could read. It hands over only its non-secret verdict. The required job takes a `needs:` on it, so it also takes `if: always()` - a required check that is skipped never reports, and branch protection then waits on it forever. A failed verifying job therefore yields no approvals and every finding blocks, which is the state before this change. tests/fm-supersession-attest.test.sh covers both directions: a covered finding with a valid signature passes, and no signature, a wrong signature, a wrong commit, a wrong task, a widened token, the master key, another repository's key, that repository's CI-waiver key, and an uncovered finding each still fail. tests/fm-reverify-base.test.sh covers the excusal itself, including the class narrowing and the unreadable-approval refusal, and asserts the workflow shape the secret separation depends on.
docs/configuration.md owns the secret's provisioning, as it does for the CI waiver; the script headers keep the payload, verdict and signing mechanics. Every other mention is a cross-reference: AGENTS.md gains one line telling firstmate what to run when an approved entry clears the merge gate and leaves the PR's own check red, and docs/architecture.md and bin/fm-pr-merge.sh's header each gain a pointer rather than a second copy of the contract.
An attestation is a snapshot of the approvals as they stood when it was signed, so a revocation does not invalidate a line already published for that same commit. Nothing merges on that - the merge gate reads the live record and refuses there - which is what makes the snapshot acceptable, and saying so is what stops a later reader mistaking the check's green for the record's word.
Renaming the three-outcome assertion would delete an assertion the base has, which is precisely the finding this whole change exists to let a captain approve - and approving it would have been the wrong answer here, since the older assertion is still true. It stays exactly as the base wrote it, and the superseded outcome gets an assertion of its own.
always() also runs a job when the whole run was CANCELLED, so a superseded run would execute the re-verification, see its dependency as cancelled, and leave a spurious red required check that only clears on the next push. .github/workflows/no-mistakes-required.yml already carries that reasoning for the same shape; this follows it rather than restating it.
The job that reads the PR body is the one that already succeeded, so `gh run rerun --failed` would re-run only the verdict job and reuse an attestation verdict taken before the line existed. The printed next step now says all jobs, and why.
CI caught this on this PR itself: a second job has to hand its verdict over through `needs:`, a required job that `needs:` another is skipped when that one fails, and the repair for that is a job-level condition - which the base's own test forbids, because a job-level condition is also how a required check stops reporting. Changing that assertion would have needed a captain supersession, and a supersession cannot be honoured until this change is on main, so the PR adding the escape hatch would have needed the escape hatch. Keeping the verification in the required job removes the dependency rather than patching it. The secret stays out of the branch's reach on two properties the tests now assert together: the verifier is the BASE's copy, checked out separately, and it runs before any step that executes the branch's own scripts, while a step's env reaches only that step.
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
1 participant
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 problem
A captain-approved test-assertion supersession cannot pass CI today.
bin/fm-pr-merge.shreadsdata/supersessions/<project>.md, excuses the covered findings and proceeds. The required checkBase assertions re-verifiedre-runs the same base assertions on a GitHub runner, cannot see that record - it is captain-private and gitignored by design - reports the same findings and goes red.Nothing the branch pushes fixes that, because the branch is not what is wrong: it is the approval that CI cannot read. The only ways out were a mislabelled CI waiver or a GitHub-UI merge, which is the exact bypass that check exists to prevent.
Live instance: #61, whose 5 approved entries cover all 66 findings. The merge gate passes; the check is red.
What this adds
A captain-signed attestation that carries the approvals into CI, following the CI waiver's existing pattern rather than inventing a second one.
bin/fm-supersession-attest.sh-publisha repository's key,signthe approvals for one commit,attesta task (reads the PR's current head, signs, appends the line to the body, prints the command that re-runs the check).bin/fm-supersession-verify.sh- decides in CI which approvals carry a valid signature for the current head.bin/fm-supersession-attest-lib.sh- the signed payload, the entry token, the published line.bin/fm-supersession-lib.sh- unchanged grammar, now also exposing the one matcher and a canonical entry line, so the merge gate, the viewer and CI answer "is this covered?" identically. There is no second parser.bin/fm-reverify-base.sh- excuses covered findings, blocks uncovered ones exactly as before, names every override, and renders a fourth outcome,superseded.The properties that matter
The signature is the authority, not the file. Minting a line needs the master key at
config/ci-waiver-secretand a fully-formed entry in the captain's private record. A worker holds neither: no brief, scaffold or status protocol points at that record, which is the standingdata/projects.mdalready has at the attestation exemption, and the key is never handed to a worker in any form.bin/fm-ci-waiver-lib.sh's residual same-user limit applies unchanged.No dispatch flag gates it, deliberately. A CI waiver is decided at dispatch, so a token minted there is the right authority for it. A supersession can only be decided after the gate reports which assertion the branch supersedes, so there is nothing to have flagged in advance. The record is the decision.
The record is not published. The line carries each entry's matching half - its identifier or glob and the class it excuses - and never the captain's stated reason or the approval date.
Never a blanket green. A finding no entry covers still blocks, in its own class. An entry approved for one class does not excuse another. An approval this fleet's matcher cannot read is
could-not-verify, never "no approvals".A separate published key. The Actions secret is
FM_SUPERSESSION_SECRET, derived from the same master under its own domain, not the repository'sFM_CI_WAIVER_SECRET. The waiver key skips a PR's whole test suite; this one only excuses findings the captain named, so a theft of this one cannot escalate into that one. Cost: onepublishper repository, once.The secret is never readable by the branch's own scripts. The re-verification job runs them by construction, so two properties keep the secret out of reach, and the tests assert both together: the verifier is the base's own copy, checked out separately exactly as
ci.yml's waiver job does, and it runs before any step that executes the branch's scripts, while a step'senv:reaches only that step.It is a step of that job rather than a second job, and this PR is why. A second job hands its verdict over through
needs:; a required job thatneeds:another is skipped when that one fails; a skipped required check never reports, so branch protection waits forever; and the repair for that is a job-level condition - which the base's own test forbids, for the same reason. Changing that assertion would have needed a captain supersession, and a supersession cannot be honoured until this change is on main. The PR adding the escape hatch would have needed the escape hatch. Keeping the verification in the required job removes the dependency instead.The body is read live. An approval always arrives as an edit to an open PR, editing a body triggers no workflow, and re-running the check replays the payload the PR had before the edit. Only a live read sees it.
Different words for different things. A waiver says there is no test evidence; a supersession says there is and the captain overrode a specific assertion. The check reports
reverify: waivedorreverify: superseded, and the separate HMAC domain means neither line can ever verify as the other.What it does not answer
An attestation is a snapshot of the approvals as they stood when it was signed, so revoking an entry afterwards does not invalidate a line already published for that same commit. Nothing merges on that:
bin/fm-pr-merge.shreads the live record at merge time and refuses there. The check reports; the record decides.Tests
tests/fm-supersession-attest.test.sh(22 cases) covers both directions: a covered finding with a valid signature passes, and no signature, a forged signature, another commit, another task id, a token edited after signing, the master key, another repository's key, that repository's CI-waiver key, and a repository the task does not push to each fail. It also covers the signer's refusals (no record, no fully-formed entry, no key), the live body read, CRLF bodies, and exit 0 on every verdict.tests/fm-reverify-base.test.sh(39 cases, 8 new) covers the excusal itself - class narrowing, the unreadable-approval refusal, the findings-do-not-account-for-the-counts refusal, inertness with no attestation - and the workflow shape the secret separation depends on.Full behaviour suite and
bin/fm-lint.shboth clean.Verified against the live case
PR 61's 66 findings, read from its own failed run's log, matched against the captain's real record through the same extraction the signer uses and the same matcher CI applies: 66 covered, 0 uncovered. Identifiers outside the approvals - including in the same two files, for the class the globs do not cover - still block.
Making that PR itself green needs, in order: this PR landing; PR 61 merging main forward, since a
pull_requestrun uses the PR's own workflow file and the base must carry the verifier;bin/fm-supersession-attest.sh publish kirangathani/firstmateonce;bin/fm-supersession-attest.sh attest <task-id>; and a re-run of the Base re-verification workflow.