fix(supply-chain): route promote-core lock update through an auto-merge PR#238
Conversation
…ge PR promote-core's update-lock job pushed the approved-lock entry straight to main. On a protected main (PR + merge-queue + required catalog-schema check + verified-signatures rules) that bot push is rejected with GH013 on four counts, so the lock entry never lands even though publish/sign/attest succeed. Commit the lock entry to a per-run branch instead, open a PR, and enable squash auto-merge. The merge queue lands it web-flow-signed after the schema check runs on the PR, preserving RT-10 (no [skip ci]; the validator still gates the change) while satisfying every main-branch rule. Adds pull-requests: write, an image_id slug guard (it reaches a git ref name), a run-attempt-unique branch to survive re-runs, and a best-effort auto-merge enable that leaves the PR open for manual merge rather than failing the promotion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR modifies the update-lock job in supply-chain-promote-core.yml, replacing direct pushes to main with a branch-based PR flow. It adds pull-requests write permission, introduces unique feature branch naming, adds input validation for LOCK_FILE, IMAGE_ID, and GHCR_REF, and enables squash auto-merge on the created PR. ChangesUpdate-lock Workflow Refactor
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant LockBranch
participant UpdaterScript
participant GitHubPR
Workflow->>LockBranch: create/switch to unique branch
Workflow->>UpdaterScript: run scripts/update_approved_lock.py
UpdaterScript-->>Workflow: updated LOCK_FILE (or no changes)
Workflow->>LockBranch: commit and push changes
Workflow->>GitHubPR: gh pr create (or locate existing PR)
Workflow->>GitHubPR: enable --auto --squash merge
GitHubPR-->>Workflow: auto-merge enabled or warning left open
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 ReviewBUILD FAILING: do not merge until CI is green. Critical (must fix before merge)
Important (should fix)
Suggested / InformationalForce-push is unnecessary given the branch name is already run-unique; SonarQube: 0 PR-specific issues (never analyzed, consistent with Critical #1); 0 outstanding hotspots on main. Reviewers: Copilot review was not requested for this PR (ruleset did not trigger it); CodeRabbit's check shows green but its actual comment reveals it was rate-limited and never ran ("you've reached your PR review limit"). Neither bot contributed findings to this review. 🤖 Generated with Claude Code |
Addresses pr-review findings on #238: - Fix PR_BODY markdown rendering: build with printf instead of a multi-line double-quoted string, which was baking the YAML step's own indentation into the PR description as literal whitespace. - Preserve gh pr create's stderr instead of discarding it, so an auth/rate-limit/validation failure is distinguishable from the expected "PR already exists" fallback path. - Add a regex guard on GHCR_REF (ghcr_name/ghcr_tag are caller-controlled and previously unvalidated) before it reaches PR_BODY or any git/gh command; bound IMAGE_ID's length in its existing guard. - Drop the unnecessary --force on the per-run branch push so an unexpected collision fails loudly instead of silently overwriting. - Add RAD #ASSUME/#VERIFY markers documenting the GITHUB_TOKEN approval-gate risk (a bot-opened PR's required checks may need manual approval to run), the merge-queue assumption on caller repos, and the lock-updater script's idempotency assumption. - Reference #239 (tracked follow-up to sign the bot commit itself). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PR Fix SummaryAddressed 9 of the pr-review's 10 auto-fixable findings in one commit (d8cc725): Critical
Important
Suggested
Not changed: branch-naming convention (not a real violation) and missing issue reference (no issue existed to link). Verification: yamllint, actionlint (matched to this repo's CI CI re-run triggered by this push; watching for the previously-phantom required checks (Security Gate Validation, Dependency & Standards Validation, Check REUSE Compliance) to fire on the new head SHA. |
There was a problem hiding this comment.
Pull request overview
This PR updates the reusable supply-chain-promote-core workflow so the approved-lock update is routed through a per-run pull request with squash auto-merge, instead of attempting a direct push to main, which fails on protected branches.
Changes:
- Switch
update-lockfrom push-to-main to: create a per-run branch, open a PR, and enable squash auto-merge. - Add
pull-requests: writepermission to allow PR creation and auto-merge enablement. - Add input-shape validation for
image_idandGHCR_REF, and introduce a uniqueLOCK_BRANCHname keyed onrun_idandrun_attempt.
| @@ -432,6 +451,22 @@ jobs: | |||
| echo "::error::lock_file has unexpected path shape: ${LOCK_FILE}" | |||
| exit 1 | |||
| fi | |||
The shape-only regex accepted values like "catalog/../.github/workflows/ci.yml" because ".." matches [a-zA-Z0-9._/-]. Add an explicit ".." substring rejection after the shape check. Addresses Copilot review finding on PR #238.
|
Fixed in a00c4e1: LOCK_FILE's shape regex allowed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/supply-chain-promote-core.yml:
- Around line 453-476: The validation in the workflow job is only checking
line-oriented patterns, so newline-containing inputs can still pass and IMAGE_ID
also allows ".." segments. Update the checks around LOCK_FILE, IMAGE_ID, and
GHCR_REF to validate the entire string contents and reject embedded newlines or
traversal-like sequences, then add a git check-ref-format --branch validation
for LOCK_BRANCH before it is used with git switch -c. Keep the existing guard
logic in the same validation block so the final branch ref and related inputs
are sanitized before any git or gh command usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c474e56c-cd3f-4634-ad27-9de247489772
📒 Files selected for processing (1)
.github/workflows/supply-chain-promote-core.yml
grep's ^/$ anchors are per-line, not per-string: LOCK_FILE, IMAGE_ID, and GHCR_REF values containing an embedded newline could pass their grep-based shape check via a matching first line while the full value differed. Switch the three shape checks to bash's [[ =~ ]], which anchors to the whole string. Also reject consecutive dots in IMAGE_ID explicitly (git already rejects them in the derived branch ref, but this gives a clear error instead of a raw git failure) and add a git check-ref-format guard on LOCK_BRANCH before git switch -c. Addresses CodeRabbit review finding on PR #238.
|
Fixed in 010d2a5: CodeRabbit's finding was correct. grep's ^/$ anchors are per-line, not per-string, so LOCK_FILE/IMAGE_ID/GHCR_REF values with an embedded newline could pass the shape check via a matching first line. Swapped all three to bash's [[ =~ ]] (whole-string anchors). Also added explicit '..' rejection for IMAGE_ID and a git check-ref-format guard on LOCK_BRANCH before git switch -c, both confirmed by direct testing. |
|
Problem
promote-core's
update-lockjob pushes the approved-lock entry directly tomain. On a protected main this is rejected with
GH013on four counts atonce (observed in container-images run 28477911147):
So even when publish + cosign sign + CycloneDX attest + SLSA provenance all
succeed, the lock entry never lands. RT-10's push-to-main design is
incompatible with a protected main branch.
Fix
Commit the lock entry to a per-run branch, open a PR, and enable
squash auto-merge. The merge queue lands it web-flow-signed (satisfying the
signature rule) after the
Validate catalog schemacheck runs on the PR. Thispreserves RT-10 (no
[skip ci]; the validator still gates the change) whilesatisfying every main-branch rule, with no bypass actor.
Also in this change
pull-requests: writeadded to the job (needed to open + auto-merge the PR).image_idslug guard: it now reaches a git ref name, so it is constrained to^[a-zA-Z0-9][a-zA-Z0-9._-]*$before any git use.run_id+run_attemptso a re-run never collides witha prior attempt's branch/PR.
open for manual merge rather than failing the promotion.
queue serializes and a real approved-lock conflict surfaces on the PR).
Rollout
Callers pin promote-core by SHA, so after merge the container-images
supply-chain-mirror.ymlcaller must be re-pinned to this commit's SHA beforethe bake exercises the new path.
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes