fix(security): keep the shared publish-workflow matchers pinned to a fixed revision - #3019
Merged
Merged
Conversation
…fixed revision Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅
|
…bad shapes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
No review requested at this head yet, deliberately. CodeRabbit refused twice in the last twenty minutes Recording it so the next run resumes at the right lane instead of re-deriving this. Self-review already applied, and it found a real gap rather than confirming the diff: the guard |
Contributor
Author
CodeRabbit is rate-limited at this head ( @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e9a831008
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… every path Three gaps Codex found at 4e9a831, all reproduced against the real repository before fixing. The tag arm matched by shell GLOB PREFIX, so `(refs/tags/v.+)?refs/heads/.+$` — an optional tag group followed by a branch ref — was accepted and the guard reported all eight subjects pinned. Reproduced: exit 0 on a subject permitting refs/heads. Each alternative is now matched whole against an explicit grammar, and a ref that is not a fully grouped alternation is rejected outright rather than having a leading paren stripped and the remainder read as part of one alternative. The count floor only proved the eight KNOWN subjects were still found; a ninth consumer written in valid multiline YAML or under a new key was invisible to the subject pattern while the existing eight satisfied the floor. Discovery now keys on the shared workflow identity alone, with no key name or line structure, and any reference the strict pattern did not validate fails closed naming the file. The guard ran only in ci.yaml, whose triggers are pull_request and merge_group. A matcher widened by a direct push to main therefore reached production through the manual CD route with the check never having run. It is now also a step in cd.yaml's pre-deploy contract job and in validate-main.yaml, matching the reason already stated in both files: the routes to production must share one gate, or the weakest becomes the real policy. Ablations, each firing its own named branch: optional-tag-group, bare branch ref in the alternation, a tag form smuggling a second ref kind through `/`, a bare `.+`, a bare `@main`, and a new consumer in an unmatched form. Control: the clean repository still reports all 8 pinned, exit 0.
…ys single-step scripts/validate-dr-signing requires validate-publication-contract to run exactly one command step: any other step in that job could export BASH_ENV or put a fake `go` on $PATH through $GITHUB_ENV/$GITHUB_PATH, leaving the validator step looking correct while it was subverted. Adding the pin guard as a second step there broke that invariant — which is the invariant working, not an obstacle to route around. The guard now runs in its own job in both workflows, and cd.yaml's deploy-prod gains it as a `needs` prerequisite, so the manual deploy is still gated. Neither validator job grows a tampering surface, and the guard covers the direct-push and manual-deploy paths as intended.
…entry Adding the pin guard to deploy-prod's `needs` wrapped that line past the formatter's width, and validate-dr-signing's ablations mutate it as a literal — three of them reported "ablation changed nothing", i.e. they had silently stopped biting. That is the meta-guard doing exactly its job, and re-aiming a security validator's ablations to accommodate a formatting change is the wrong direction. validate-publication-contract now depends on the pin guard instead, so the deploy is still blocked when a matcher is widened (the guard's failure skips the contract job, which skips deploy-prod) while deploy-prod's `needs` line stays byte-identical and every ablation keeps biting. The guard also stays in its own job, so the contract job still runs exactly one command step.
Contributor
Author
All three Codex findings from the previous round are fixed and their threads resolved; CI is green at @coderabbitai review |
Contributor
|
✅ Action performedReview finished.
|
Contributor
📝 WalkthroughWalkthroughThe pull request adds a Bash guard for shared publish workflow pinning. The guard validates Cosign subjects against full revisions or constrained tags. CI, main-branch validation, and production deployment workflows run the guard. Production publication-contract validation now depends on the shared pin validation job. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
Author
User evaluation at Positive control first. Against the deployed configuration it reports Then the five failure modes, each mutation verified to have actually landed (a
The last two are the ones worth having. A guard whose grep silently stops matching reports a clean Also linked the delivery issue: this PR previously said only |
devantler
marked this pull request as ready for review
August 8, 2026 17:09
This was referenced Aug 9, 2026
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.
Why
#2816 tightened the cosign matchers that decide which signer we trust, from "any ref" to a fixed
revision. That was done by hand, in eight separate places, and nothing keeps it that way. Widening
any one of them back does not look like breakage — verification still runs, it just accepts more —
so schema checks, kubeconform and the deploy all stay green while the control quietly weakens.
This makes the property enforced instead of conventional, which is the first acceptance criterion
on #2818 and the cheapest of the options weighed there.
What
A CI guard asserting that every cosign subject naming a shared
devantler-tech/actionspublishworkflow pins a fixed revision — rejecting both a branch ref and a bare "any ref" wildcard.
Two things worth the maintainer's attention:
running from
mainand the merge queue, so its subject legitimately contains a branch ref. A guardthat banned branch refs outright would fail on the correct deployed config — and a control that
fires at the known-good state is one that gets switched off the first time it blocks a release.
Scoped by subject instead, so it says nothing about first-party workflows.
guard fails closed if it ever finds fewer, because a refactor that moved them out of its reach
would otherwise report a clean repository while checking nothing.
This keeps #2816's property from regressing; it does not restrict trust to approved revisions —
a superseded SHA still matches. That needs a generated allow-list and remains open on #2818.
Fixes #3021
Part of #2818