Repository navigation
fix(security): name the files that hold a publish-workflow matcher and account for every subject - #4563
Conversation
…he two-family matcher A manifest matcher that accepts the canonical publisher beside the legacy one is one subject holding one group. The pin guard refused it (two identities in one subject) and the signing-revision report stopped finding its consumer, so no canonical approval could pass CI. The pin guard now reads that form with its own strict parse: the exact prefix, the legacy family first, the canonical family second, each once. The legacy ref keeps the existing allow-list; the canonical ref must be one concrete commit. Every other arrangement is refused, as is a subject naming only the canonical family. The report selects the two-family subject and keeps the legacy family as its anchor. No matcher changes: the canonical approvals record is still empty. Part of #4502 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ations Two ways the pin guard accepted a matcher wider than it reported, found by the independent review of this change and present before it: - An alternation over refs with no group around it was split on the bar and each half judged alone, although the second half is anchored to nothing. - The subject pattern selects a line and matches anywhere on it, and only the text after the single @ was judged, so a value whose identity was not the shared workflow could be selected by a comment and pass on its ref. A single-family subject must now be exactly the anchored legacy identity, a ref and the closing anchor, and several refs must be grouped. Part of #4502 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d account for every subject The pin guard's floor was a count, so a matcher rewritten in a spelling its patterns do not select went unjudged while the others made the number up. The floor is now the list of files that hold a matcher, and every other subject must be proved to name a different repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai full review |
✅ Action performedFull review finished. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (13)
📝 WalkthroughWalkthroughThe guard now enforces per-file matcher counts and validates selected and unselected subject keys. It supports the specified legacy-then-canonical publisher subject form, with distinct ref rules for each family. The signing-revision report discovers combined subjects when the legacy publisher path is present. New tests cover matcher inventory, subject parsing, ref validation, and report discovery. The CI path filter now includes the matcher inventory file. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change makes the publish-workflow signing check stricter. The check now reads its matchers from a named list of files and fails on any matcher it cannot account for. No problems were found that should block merging. If a future YAML file uses a Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue Full details: Docstring CoverageExplanation Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. (1 skipped: 1 unsupported.)
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 |
…d-files-4558 # Conflicts: # scripts/guard-shared-publish-workflow-pin.sh
There was a problem hiding this comment.
🤖 Generated by the Agentic Engineer
Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)
Reviewed commit: fadae75445d0ad15dc46c9a004ca88815ac94176
- CodeRabbit: rate limited. Its last review in this account at 08:58Z states that none of the two included reviews per hour remain; the next is available at about 09:58Z and is owed to two other pull requests whose content changed.
- Codex: unavailable, usage limit since 2026-10-06T04:40:17Z.
- Cursor Bugbot: unavailable, usage limit since 2026-10-06T04:42:02Z.
This head is the previous head 8643433250a8bbe6e311fb4f5139cf6a25549c6a plus one merge of main. CodeRabbit reviewed that previous head in full and reported no findings, and nothing this pull request contributes has changed since:
- The branch was stacked on the final head of #4556. #4556 was squash-merged at 09:35Z, so merging
mainconflicted in the pin check only, and the conflict was resolved to this branch's side, which already contained everything #4556 merged. - Compared line by line, the change this pull request makes against its old base and the change it makes against
mainnow are identical. - Against
mainit touches exactly the four intended files: the pin check, its list of files that hold a matcher, its test and one CI filter line. - The pin check's suite passes on this head (101 assertions) and the check passes on the real tree (7 subjects).
Verdict: no P0/P1 findings
Evaluation at
|
1 participant
Why
The check that keeps signing rules for the shared publish workflows pinned to a fixed revision could report success without having examined one of them. A rule written in an unusual but equivalent way was skipped, and only a minimum count stood in the way, which the other rules satisfied.
What
The check now names every place a rule is expected and fails when one is missing, extra or somewhere unexpected. Every other signing rule in the repository must be shown to belong to a different project, so a rule the check cannot place is refused instead of skipped.
Fixes #4558