Repository navigation
ci(deploy): gate production deploy to release publication, not every main push - #1398
Conversation
Production was shipping on every push to main: docker-publish built images on each main commit and Deploy to Homelab chained off it via workflow_run, so every merge auto-deployed (and raced — see #1397). Gate deploys to deliberate release publication instead: - swap the workflow_run(main) trigger for release: types: [published]; workflow_dispatch (manual + rollback) is kept unchanged - add one resolve_sha step as the single source of truth for the shipped commit. On a release it derefs the (annotated) tag to its commit SHA — github.sha is unreliable on release events. Rollback and workflow_dispatch paths are preserved - every downstream step (image wait, webhook, Sentry create/finalize, version validation, homelab wait) now reads that one resolved SHA, so the Sentry release, deployed :<sha> image, and validated status can never disagree docker-publish still builds :<sha>/:latest on main push, so the image for a release commit already exists when the release is cut. Building artifacts is decoupled from shipping them.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe production deploy workflow trigger is changed from ChangesDeploy trigger and SHA resolution refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Failed to generate code suggestions for PR |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy.yml (1)
133-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
docker_waitcan falsely fail valid release deploys because run lookup is capped at 10.With release-triggered shipping, the tagged commit can be older than the latest 10 docker-publish runs. The current query then misses the real run and Line 178 can abort a valid deploy.
Suggested lookup fix (filter by commit instead of scanning last 10)
- run_info=$(gh run list \ - --repo "${{ github.repository }}" \ - --workflow=docker-publish.yml \ - --json headSha,status,conclusion,databaseId \ - --limit 10 \ - | jq -r --arg sha "$commit_sha" \ - '.[] | select(.headSha == $sha) | "\(.databaseId) \(.status) \(.conclusion)"' \ - | head -1) + run_info=$(gh run list \ + --repo "${{ github.repository }}" \ + --workflow=docker-publish.yml \ + --commit "$commit_sha" \ + --json status,conclusion,databaseId \ + --limit 1 \ + --jq '.[] | "\(.databaseId) \(.status) \(.conclusion)"' \ + | head -1)Also applies to: 148-181
🤖 Prompt for 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. In @.github/workflows/deploy.yml around lines 133 - 141, The docker_wait function uses a gh run list query with --limit 10 that only scans the last 10 Docker publish runs, which can miss older tagged commits in release deployments and cause false failures. Modify the gh run list command to either remove the arbitrary limit or add a filter to the query itself to specifically target the commit SHA instead of relying on scanning only recent runs. This issue appears in the docker_wait logic around the gh run list invocation and potentially in subsequent similar queries elsewhere in the workflow file, so apply the same fix approach to all locations where gh run list queries are made without proper commit-based filtering.
🤖 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/deploy.yml:
- Around line 61-105: The `deploy_sha` variable resolved in the resolve_sha step
is derived from user-controlled input (inputs.rollback_sha on workflow_dispatch)
and is currently exported without strict validation, then inlined directly into
multiple downstream shell scripts at lines 121, 237, 443, and 685. This creates
a template-injection vulnerability. Add strict validation of deploy_sha in the
resolve_sha step to ensure it matches the expected git commit SHA format (using
a regex pattern to validate it contains only hexadecimal characters of the
correct length), and then update all downstream usages at those four locations
to safely reference the value via environment variable assignment with proper
shell quoting rather than direct template expansion into command strings.
---
Outside diff comments:
In @.github/workflows/deploy.yml:
- Around line 133-141: The docker_wait function uses a gh run list query with
--limit 10 that only scans the last 10 Docker publish runs, which can miss older
tagged commits in release deployments and cause false failures. Modify the gh
run list command to either remove the arbitrary limit or add a filter to the
query itself to specifically target the commit SHA instead of relying on
scanning only recent runs. This issue appears in the docker_wait logic around
the gh run list invocation and potentially in subsequent similar queries
elsewhere in the workflow file, so apply the same fix approach to all locations
where gh run list queries are made without proper commit-based filtering.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5943b77-e450-46a6-860a-9ba35ed9fa77
📒 Files selected for processing (1)
.github/workflows/deploy.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test — bot
- GitHub Check: Checks
- GitHub Check: Test — shared
- GitHub Check: Test — backend
- GitHub Check: Test — frontend
- GitHub Check: cubic · AI code reviewer
- GitHub Check: quality / SAST (CodeQL) (javascript-typescript)
- GitHub Check: quality / Lint (lint)
- GitHub Check: Build — backend
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/deploy.yml
[info] 121-121: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 237-237: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 443-443: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 685-685: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (1)
.github/workflows/deploy.yml (1)
16-17: LGTM!Also applies to: 35-37
Address CodeRabbit Major: rollback_sha is user-controlled on
workflow_dispatch and flowed into resolve_sha.outputs.sha, which was
then inlined as ${{ }} into run: scripts — a template-injection path.
- resolve_sha now rejects anything but a 7-40 char lowercase hex SHA
before exporting (covers short rollback SHA through full commit SHA)
- the four steps that still inlined the output (docker_wait, Sentry
create/finalize, homelab wait) now read it from env, matching the
webhook/validate steps and the workflow's pre-existing injection-safe
pattern
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Fixed in ffe13e1 (@cubic-dev-ai fyi): |
@LucasSantana-Dev Looks good — thanks for the flag. The main things I was going to flag from the first diff are addressed:
No new concerns from the updated diff — this looks ready to ship. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
Address cubic P2: docker_wait found the build via `gh run list --limit 10`, which was always safe under workflow_run coupling (deploy fired right after the build). With release-triggered deploys a release can be cut for an older tag whose docker-publish run has fallen out of the 10-run window, so the capped lookup would miss it and falsely abort. Query the build by exact head_sha via the Actions API instead — immune to how many builds ran since the tagged commit. Same id/status/ conclusion output, same not-yet-queued retry and fallback paths.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@cubic-dev-ai P2 fixed in 0b4902a: |
@LucasSantana-Dev That looks right to me. Switching from The rest of the logic lines up too:
One minor thing I noticed — the previous No concerns from the updated diff. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Modifies CI/CD deployment trigger from every main push to release publications; adds SHA resolution logic and validation.
Re-trigger cubic
|
This branch was successfully deployed
1 participant
Production currently ships on every push to
main—docker-publishbuilds images on each main commit and Deploy to Homelab chains off it viaworkflow_run, so every merge auto-deploys to prod (and they race — see #1397). This makes shipping accidental rather than deliberate.Change
Gate production deploys to GitHub Release publication:
workflow_run(Build & Push Docker Images / main) →release: types: [published].workflow_dispatch(manual deploy + rollback) is unchanged.resolve_shastep as the single source of truth for the shipped commit. On a release it dereferences the annotated tag to its commit SHA (github.shais unreliable on release events). Rollback andworkflow_dispatchpaths are preserved.:<sha>image, and the validated commit status can never disagree (they were independently re-derived in 6 places before).What does NOT change
docker-publish.ymlstill builds:<sha>+:lateston every main push, so the image for a release commit already exists when the release is cut. Building artifacts is decoupled from shipping them.Productionenvironment protection (wait timer) still applies — now on release deploys.workflow_dispatchrollback_shais untouched.Shipping flow after this
merge to main→ images built (no deploy) → cut releasevX(push annotated tag →Releaseworkflow publishes it) → Deploy to Homelab fires for the tagged commit.Known limitation
If a release tag points at a commit that never built a
:<sha>image (e.g. a docs-only HEAD thatdocker-publishpath-filtered out), the deploy falls to the existing:latestpath. In practice release tags sit on code/package.jsoncommits that always build. Flagging for review rather than over-engineering.Verification
actionlint: 0 new findings vsmainbaseline (7 pre-existingSC2016info notes on intentional single-quotedjq/node -e, identical on both).workflow_run/ straygithub.shareferences remain outside the intendedworkflow_dispatchpath.@cubic-dev-ai please review.
Summary by cubic
Gate production deploys to GitHub Release publication instead of every push to
main. Adds SHA validation and env-based propagation to prevent injection, keeping Sentry, image tags, and commit status in sync, and improves Docker build lookup for older tags.workflow_run→release: [published];workflow_dispatch(manual/rollback) stays.resolve_shato pick the shipped commit once; on release, dereferences the annotated tag to its commit SHA.rollback_sha.head_shavia the Actions API (not a recent window), so releases for older tags don’t fail lookup.docker-publish.ymlstill builds:<sha>and:latestonmain; building artifacts is decoupled from shipping them.Written for commit 0b4902a. Summary will update on new commits.
Summary by CodeRabbit