fix(release): narrow signing identity and reconcile release posture - #195
Conversation
PLAN-48 widened the accepted certificate identity to admit refs/pull/<N>/merge on the premise that a merge-triggered release signs under the merge ref. This project's own signing certificates refute it: 0.1.0 (workflow_dispatch) and 0.1.1 (pull_request) both carry job_workflow_ref = .../release.yml@refs/heads/main. The merge path has never produced a merge-ref identity. Narrow all four carrier sites in three files to the single anchored form and rewrite the justification to lead with the decoded certificates, demoting GitHub's events-that-trigger-workflows reference to corroboration. The cosign verify step's flag set and the branches: [main] base filter are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
…request closed The signing comment claimed the merge path carries GITHUB_REF = refs/pull/<N>/merge. It does not: a pull request closed by being merged runs against the base branch it was merged into. State instead that only two events reach the signing job -- workflow_dispatch and a merged-close .github/project.yml PR -- and that both yield refs/heads/main, naming the merged == true gate as the reason no other close gets there. Comments only; no executable line changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
release.yml:19 forward-references ADR-0035 for the decision that a release is cut by a guarded version-bump merge or a deliberate dispatch; the record did not exist. Author it, filling the 0035 gap out of sequence because that forward reference pins the number. ADR-0035 records the two triggers and their asymmetry, why the centrally-owned version-changed guard is the discrimination the 2026-07-12 trigger lacked, why it is consumed and never re-implemented locally, that the paths: filter is a prefilter and never the decision, and that ADR-0034's trigger-removal-merges-on-its-own rule and residual operator risks carry forward unchanged. ADR-0034 is flipped to Superseded with a pointer to ADR-0035; its body is left verbatim as the record of what was decided at the time. doc/README.adoc's index NOTE range widens to ADR-0020 through ADR-0035. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
…two-path trigger The document asserted a dispatch-only posture throughout -- title, intro, the two-step cutting procedure, the artifact lead-in, the diagram alt text, the recovery procedure's dispatch-bound phrasing, the tag-provenance note, and the whole 'Dispatch is the only trigger' subsection. All of it is false against release.yml as it now stands. Replace it with the two-path account: the guarded version-bump merge is the publishing act, workflow_dispatch is the unconditional fallback, the centrally-owned version-changed guard is the decision, and the paths: filter is a prefilter that must never be read as the release condition. '=== A trigger removal merges on its own' survives byte-unchanged -- only its framing sentence moved -- and the relocation-stubs recovery paragraph is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
… narrative rewrite make false The runbook claimed ADR-0035 was still owed and its reference dangling, that release-process.adoc was STALE on the trigger and not authoritative there, and it cited two section names that do not exist in that file while calling the rewrite tracked follow-up work. D3 and D4 make all of it false. Re-point the residual-risk citation at :459 from the now-superseded ADR-0034 to ADR-0035. Also corrects one claim outside the enumerated carrier list, in the same file: 'The guard has NEVER been observed' asserted that no merge-triggered release had ever run. 0.1.1's signing certificate refutes it -- github_workflow_trigger=pull_request at the #186 merge commit -- so the guard has been observed firing. What remains unobserved is the guard refusing, which is what the section now says. Step 3(i) and the frontmatter description are byte-unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
…ease-flow diagram The doc/development/README.adoc Contents row stated the retired posture four times over -- the dispatch-only link text, 'then deliberately dispatch', 'what one dispatch produces', and 'dispatch is the only trigger'. Rewritten to match the reconciled narrative. release-publish-flow.svg is rendered into the published documentation by release-process.adoc and carried the posture in four places, including a visible in-diagram note reading 'Manual dispatch is the only trigger -- no event-driven trigger exists on the release workflow', which is flatly false in operator-facing output. The authoring comment, the desc accessibility text, the visible note and the entry stage label plus its sub-label now all name both paths. viewBox, palette, style classes, theme-handling declaration and stage geometry are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
The stubs branch's job finished: the relocation is complete and permanent, immortalised in the published 1.0.1 POM on Maven Central. What the branch still cost was a standing never-merge clause in the release runbook -- a permanent trap guarding a branch whose work is done. Removes both carriers in the same change: the pre-cut guard block in Step 2 (with its fetch and ancestry check) and the Critical-rules restatement. Also clears one orphaned reference to the removed guard in the Step 1 enumeration of pre-cut assertions. The forward-looking recovery knowledge is deliberately retained -- 'if a version must be abandoned, cut a patch version and publish relocation stubs' is a procedure for a future accident, not a statement about this branch -- as is the 2026-07-12 incident record. Provenance is preserved by the tag relocation-stubs-published, pushed to origin at 67c8339 and confirmed on the remote before any deletion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
Both were introduced by this plan's own edits and caught by the pre-submission self-review gate: - Step 5 Path A pointed at "The guard has NEVER been observed", a heading this plan renamed. The dangling pointer also asserted the opposite of the section it targeted: that no cut had been read off a run. Repoint it and state the real position - exactly one merge-triggered cut (0.1.1) has been read, so a firing guard confirms that precedent while a refusing guard would still be a first observation. - The post-cut merge-hold note read "(ADR-0035 records it as one)", whose pronoun binds by nearest antecedent to "a closed safeguard" - inverting a safety-critical posture. ADR-0035 says the opposite twice. Name the referent explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
…laim The See-also rewrite deleted the clause that had disambiguated its own pronoun. The bullet's subject is doc/development/release-process.adoc, so "This file is also the surviving narrative account of the 2026-07-12 incident" read as a claim about that document - which mentions the date once, in passing. The account actually lives in this runbook (line 80 claims exactly that role) and in ADR-0035. Name the referent so the two competing claims collapse to one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
… the release runbook See-also The sentence documented an absence in a different file than the bullet's stated subject, carried no rule, guard or actionable pointer, and was carried over verbatim rather than authored by this plan. The de.cuioss.sheriff.api history it gestured at is already recorded twice in live form: the incident section earlier in this runbook, and ADR-0034/0035. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release model now supports guarded version-bump merges and main-branch manual dispatch. ADR-0035 supersedes ADR-0034. Cosign verification accepts only the main-branch workflow identity, and release documentation reflects the updated process. ChangesRelease policy
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍(Review updated until commit 4b31f4e)
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/release/SKILL.md (1)
343-343: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the stale reference to the deleted stubs check.
This change deletes the relocation-stubs procedure, but Line [450] still says “the same discipline as the stubs check in Step 2.” That reference points to a removed mechanism. Replace it with a reference to the current exact tag check.
As per path instructions, a removed mechanism must not leave a procedural reference to a nonexistent guard.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b9264dd0-afd8-404a-876b-0633b30ec2f8
⛔ Files ignored due to path filters (1)
doc/resources/diagrams/release-publish-flow.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
.claude/skills/release/SKILL.md.github/workflows/release.ymldoc/README.adocdoc/adr/0034-Publication_is_triggered_only_by_a_deliberate_standalone_invocation_never_by_a_repository_event_or_an_automated_lifecycle_step.adocdoc/adr/0035-A_release_is_cut_by_a_guarded_version-bump_merge_or_a_deliberate_dispatch_and_the_version-changed_decision_is_centrally_owned.adocdoc/development/README.adocdoc/development/release-process.adocdoc/user/container-image.adoc
Triage dispositionsIn reply to comment_id:
|
…guard predicates Two CodeRabbit findings on PR #195, both confirmed against the live tree. The narrowed Cosign identity accepts exactly one ref, refs/heads/main, and justifies that by claiming both events able to reach the lane yield it. The merge path enforces the claim through `branches: [main]`; the dispatch path only assumed it, because `workflow_dispatch:` takes no branch filter. The `release` job's `if:` now carries a dispatch-scoped `github.ref == 'refs/heads/main'` operand, so a non-main dispatch is refused before the Maven publication rather than after it. The `pull_request` operand is unchanged, and the ref test is deliberately not extended to the merge path, which is already base-filtered. release-process.adoc described only one of the central guard's two refusal predicates. `.claude/skills/release/SKILL.md` states both, and the runbook is right: the guard refuses when the declared version did not move OR when a tag for it already exists. The document now names both, with an anchor the other sections reference. The load-bearing case is the image-lane-failure procedure, which is by definition the tag-exists scenario yet attributed a no-release outcome to the version predicate alone -- the misdiagnosis that sends an operator to the unconditional dispatch and republishes a published coordinate. Co-Authored-By: Claude <noreply@anthropic.com>
… release if: CodeRabbit finding e3a37b, plus one staleness this PR's own D1 introduced. The tag-absence rationale opened by pointing at "the same discipline as the stubs check in Step 2". D7 retired the relocation-stubs procedure in this PR and Step 2 is now an open-PR count check, so the pointer led nowhere. The clause is dropped rather than repointed: the paragraph IS the exact tag check's rationale, so a pointer to that check would be circular, and the remaining text already explains the discipline completely. Sweeping the rest of the file surfaced a second orphan of the same kind. Step 3(i) check 2 quotes the release job's `if:` verbatim as a four-way stop-gate, and D1 just changed that expression. An operator re-asserting the trigger at the base-branch SHA would have found a mismatch and treated a correct workflow as a stop. The check now quotes both operands and says what the new one buys. No other surviving reference to the retired stubs procedure remains; the abandoned-version remedy at the ported "image lane fails" section is the live mirror of release-process.adoc, not an orphan. Co-Authored-By: Claude <noreply@anthropic.com>
…alidated TASK-009 added a real gate on the dispatch path (the release job's if: confines a dispatch to refs/heads/main), but six sites across two files still described that path as unconditional. Same root defect, one sweep. .claude/skills/release/SKILL.md - the READ-THIS-FIRST path table row an operator reads first - Path B's opening characterisation - the Critical rule about dispatching over an undiagnosed refusal doc/development/release-process.adoc - the intro sentence - the "workflow_dispatch is the fallback" paragraph - the normative Trigger rules section, which omitted the new operand entirely The drift ran toward under-claiming safety, so nothing risked an unsafe publication; the cost was an operator expecting a dispatch from a release branch to publish and getting a silently skipped job with no documented explanation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyPQuo2bwuyPL6DWPdabZQ
|
/review |
Summary
The repository's prose still asserted a dispatch-only release posture that PR #171 retired, and
the Cosign signing identity had been widened to accept
refs/pull/<N>/mergeon a premise thisproject's own signing certificates refute. This PR narrows the identity back to a single exact ref
and reconciles every statement of the release posture with the two-path trigger that is actually
live.
Seven peer deliverables:
...release.yml@refs/heads/mainin every carrierand rewrite the justification around decoded first-party certificate evidence.
.github/workflows/release.yml's account ofGITHUB_REFunderpull_request: closed.doc/development/release-process.adocacross its full posture surface..claude/skills/release/SKILL.md.release/relocation-stubsbranch together with both runbook guards thatreference it.
Changes
.github/workflows/release.yml— signing identity narrowed to the single anchored ref in both theSign the published digestcomment block and the verify step'sIDENTITY_REGEXP; theGITHUB_REFcommentary corrected to state that the signing job is reachable only on merged-close and on
dispatch, and that both yield
refs/heads/main. Thecosign verifystep's other flags and thebranches: [main]base filter are untouched.doc/user/container-image.adoc— the operator-facing verify command, the surrounding prose and thewhy the identity is a pattern subsection rewritten for the single-ref form.
doc/adr/0035-A_release_is_cut_by_a_guarded_version-bump_merge_or_a_deliberate_dispatch_and_the_version-changed_decision_is_centrally_owned.adoc— new ADR recording the live two-path trigger and the central version-changed ownership. The number
0035is pinned byrelease.yml, so it fills a gap out of sequence; that is intended.doc/adr/0034-Publication_is_triggered_only_by_a_deliberate_standalone_invocation_never_by_a_repository_event_or_an_automated_lifecycle_step.adoc— status flipped to Superseded with a pointer to ADR-0035. The body is left verbatim.
doc/development/release-process.adoc— the dispatch-only narrative replaced by the guardedmerge-plus-dispatch account, across the title, the intro, Cutting a release and the trigger-rules
section.
.claude/skills/release/SKILL.md— residual drift cleared (the dangling-ADR-0035 statements, thestale See also entry), and D7's two
release/relocation-stubsguards removed together: theHARD BOUNDARYblock with its fetch and ancestry check, and the Critical-rules restatement with itsqualified-refspec fetch.
doc/development/README.adoc,doc/README.adoc— remaining statements of the retired posturecorrected.
doc/resources/diagrams/release-publish-flow.svg— the published flow diagram updated to show bothtrigger paths.
Post-merge follow-up (required)
This PR leaves exactly one action outstanding. It is deferred by explicit operator ruling, not
forgotten. Whoever merges this PR owns it.
Precondition — merge this PR first. Until these runbook edits are on
main,mainstill carriesa live ancestry check against
refs/heads/release/relocation-stubs. Deleting the branch while thatcheck is live leaves
git merge-base --is-ancestorunable to resolve the ref, so the check lands onthe runbook's own arm:
That fail-closed state is precisely what this plan exists to remove, so running the delete early would
reintroduce it. Merge first, then delete.
The safety is already in place. The tag
relocation-stubs-publishedis pushed and confirmed at67c8339— the same commitrelease/relocation-stubspoints at — so the published relocation stubskeep a durable provenance pointer whether or not the branch is deleted. Maven Central artifacts are
never withdrawn, so nothing further is owed once the delete is done.
Completion check. The follow-up is complete when
returns empty. A run that shows neither the branch nor the tag is the irreversible failure state and
must be escalated immediately, never retried.
Evidence note on D1
D1's narrowing is backed by first-party evidence, not by vendor documentation. Both signed image
digests were pulled anonymously from GHCR and their signing certificates decoded:
job_workflow_refin the signing certificateworkflow_dispatch.../release.yml@refs/heads/mainpull_request(merged close).../release.yml@refs/heads/mainBoth triggers — including the merge-triggered one that motivated the widening — sign at
refs/heads/main. There is no counterexample, so narrowing the Cosign identity to that single refcannot break verification of any published artifact. GitHub's
events-that-trigger-workflowsreference is retained in the prose only as corroboration.
Test Plan
verify -Ppre-commit) — the footprint touches.github/workflows/release.yml, so the documentation-only skip does not apply.verifygreen.zero occurrences of the widened form and the narrowed form in the expected count. Stated
limitation: this gate is exhaustive over the named list only — no sanctioned tool in this project
can run a repository-wide content search across
.github/**and.claude/**, so a carrier in anunnamed file under those trees would not have been caught.
Related Issues
None — this plan originates from an orchestrator epic, not from a GitHub issue.
Intent
The problem. Two independent defects, both about the release path. First, the repository's prose still asserts a dispatch-only release posture that PR #171 retired:
release.ymlhas carried a guardedpull_request: closedtrigger alongsideworkflow_dispatchsince then, and the version-changed decision is centrally owned. Second, PLAN-48 widened the Cosign signing identity to also acceptrefs/pull/<N>/merge, on the premise that a merge-triggered run signs under the merge ref. This project's own signing certificates refute that premise: the 0.1.1 release was cut by apull_request-triggered run and signed atrefs/heads/main.The approach. Narrow the identity back to a single anchored ref (
...release.yml@refs/heads/main) in every carrier, keeping--certificate-identity-regexpso the operator-facing command shape is unchanged, and rewrite the justification to lead with the decoded certificate rather than with vendor documentation. Then reconcile the prose with the trigger that is actually live: correctrelease.yml's account ofGITHUB_REF, author ADR-0035 (the number pinned byrelease.yml:19) superseding ADR-0034, rewrite the release-process narrative, clear the residual runbook drift, and retire therelease/relocation-stubsbranch guard now that the branch's provenance is pinned by a tag.Explicit non-goals.
.github/project.ymlis not[Intent truncated — 1391 of 2226 characters shown; full outline in the plan workspace]
Summary by CodeRabbit
New Features
Security
Documentation