chore: back-merge v2.0.1 into develop - #765
Conversation
Also fix the chart's api and ray image tags, which pinned a non-existent '1.1.13' tag (published tags carry the 'v' prefix) since 81c039e. - pyproject.toml + uv.lock: 2.0.0 -> 2.0.1 - compose: openrag and openrag-admin-ui pins -> v2.0.1 - chart: version 0.5.1 -> 0.5.2, appVersion -> 2.0.1 - chart values: ray, openrag, adminUi image tags -> v2.0.1
release: v2.0.1
… check The GA image build was guarded on github.event.base_ref == 'refs/heads/main'. That field is empty for a tag pushed to a branch-protected main: protected main only receives PR-merge commits (server-side), never client branch pushes, so GitHub records no branch association and base_ref comes through empty. The guard therefore skipped every job silently — a green run that built nothing. It worked for v2.0.0 only because main was still unprotected then. Replace it with a verify-tag gate that the three build jobs depend on: - fires for release tags (v*) but not RC tags (build_rc.yml owns -rc.) - verifies the tagged commit is reachable from origin/main via merge-base, failing LOUD (red) instead of skipping silently if a tag is off-main First release affected: v2.0.1.
fix(ci): make build.yml GA build fire on release tags under protected main
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release version is updated to 2.0.1 across package, Helm, and Docker Compose metadata. GA tag builds now verify ancestry from ChangesRelease Alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant verify-tag
participant origin-main
participant image-build-jobs
GitHubActions->>verify-tag: Start for GA release tag
verify-tag->>origin-main: Fetch main
verify-tag->>verify-tag: Check tagged commit is ancestor of main
verify-tag->>image-build-jobs: Allow image builds after successful verification
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
coderabbitai
Bot
added
the
chore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/build.yml:
- Around line 20-22: Update the verify-tag job’s if condition to allow only refs
matching the exact GA tag format v<major>.<minor>.<patch>, excluding prerelease
suffixes and non-numeric versions; ensure downstream image build and publish
jobs remain gated by this validation.
- Around line 26-35: Update the “Fail if the tagged commit is not on main” step
to pass the tag name through its env configuration and reference the environment
variable in the shell commands, including both status messages, instead of
interpolating github.ref_name directly in run. Preserve the existing ancestor
check and failure behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 27923b82-b15f-4989-8b74-b6535d5b2447
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/build.ymlinfra/charts/openrag-stack/Chart.yamlinfra/charts/openrag-stack/values.yamlinfra/compose/docker-compose.yamlpyproject.toml
hedhoud
left a comment
hedhoud
left a comment
There was a problem hiding this comment.
The release/version back-merge itself looks consistent, but I would not merge the GA publishing guard in this shape.
The workflow still accepts broad v* tags, so a typo or non-GA prerelease tag that is reachable from main can publish release images and latest. The tag name is also interpolated directly inside the shell script for a privileged publishing workflow; Git accepts metacharacters in ref names, so this should be treated as untrusted data.
Please tighten the GA tag validation to exact release tags and pass the tag name as data in the shell step. After that, the rest of the PR looks fine to me.
hedhoud
left a comment
hedhoud
left a comment
There was a problem hiding this comment.
Adding precise anchors for the requested changes.
…lation Review findings on the verify-tag gate added in #764 (raised by @hedhoud and CodeRabbit/zizmor), all three confirmed against the merged workflow: 1. Tag format was too loose. The guard only rejected '-rc.', so v2.0.1-rc1, v2.0.1-beta, vfoo etc. passed and would publish GA images and move :latest. The rc1 case is the sharp one: build_rc.yml triggers on 'v*-rc.*' which requires the dot, so a one-character typo matched neither workflow's intent. Now validated against ^v[0-9]+\.[0-9]+\.[0-9]+$ and failed loud. 2. Template injection. ${{ github.ref_name }} expanded into the run body before the shell ran, and git permits ; $ ` " | & in ref names — arbitrary code execution in a job that holds packages:write and Docker Hub credentials. Tag name and SHA now passed via env: and referenced as shell variables. 3. Checkout persisted credentials, inconsistent with build_rc.yml which already sets persist-credentials: false on all three checkouts (95fd86f). The repo is public, so the origin/main fetch still works without them. Behavior: vX.Y.Z on main builds; vX.Y.Z-rc.N skips to build_rc.yml; malformed or prerelease tags and off-main tags now fail loudly instead of publishing.
|
Thanks @hedhoud — both findings confirmed valid, and I verified them against the merged workflow rather than just the diff. To be precise on the first one: the On the second, agreed it should be treated as untrusted data: zizmor also flagged a third item in the same step — the checkout did not set Fix is in #767, targeting Sequence: merge #767 into |
fix(ci): harden GA publish guard — exact tag format, no shell interpolation
|
@hedhoud the guard fix has landed — #767 is merged into Both of your findings are addressed here:
Diff into Could you clear the |
hedhoud
left a comment
hedhoud
left a comment
There was a problem hiding this comment.
Approved. The back-merge now carries the hardened GA publish guard from #767: exact GA tag validation is in place, tag/SHA values are passed through env rather than interpolated into the shell body, and the publish jobs remain gated behind verify-tag.
A few CI jobs were still pending when I checked; branch protection should keep the merge blocked until those complete.
|
Thanks @hedhoud. Resolving your The comment still anchors to live code because line 22 (the job-level |
2 participants
Standard git-flow back-merge after the v2.0.1 release. Brings two things onto
develop:pyproject/uv.lock→ 2.0.1, chart 0.5.2, compose + chart image tags → v2.0.1) — so the next cycle does not start from 2.0.0.verify-taggate that replaced the unusablebase_refcheck. Without this, the next release from develop re-breaks identically (silent skip of all image builds under protected main).No new features; pure release reconciliation.
develophas not moved sincerelease/2.0.1was cut, so this is conflict-free.Summary by CodeRabbit
New Release
Reliability