ci(helm): roll prereleases on main pushes + manual trigger#3461
ci(helm): roll prereleases on main pushes + manual trigger#3461
Conversation
Renames helm-pr-prerelease.yml to helm-prerelease.yml. The workflow now also runs on main pushes (paths: hosting/k8s/helm/**) so the chart doesn't go stale between releases, and supports workflow_dispatch with an optional appVersion override for manual builds. Behavior: - pull_request: <base>-pr<N>.<sha>, posts/updates a PR comment as before - push to main: <base>-main.<sha>, summary written to run page - workflow_dispatch: <base>-<ref-slug>.<sha>, optional appVersion override
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📜 Recent 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). (32)
WalkthroughThe Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
🧭 Helm Chart Prerelease PublishedVersion: Install: helm upgrade --install trigger \
oci://ghcr.io/triggerdotdev/charts/trigger \
--version "4.4.4-pr3461.bcf28dc"
|
- override appVersion via yq + env (no sed escape issues for user input) - fall back REF_SLUG to 'manual' when sanitization strips it empty - ignore helm prerelease workflow files in pr_checks.yml so unrelated CI edits don't trigger the full typecheck/unit/e2e suite
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/helm-prerelease.yml (1)
69-77:⚠️ Potential issue | 🟠 MajorRestrict PR write token scope to PR-only execution path.
With Line 69-Line 72 allowing
pushandworkflow_dispatch, keepingpull-requests: writeat Line 77 gives non-PR runs unnecessary write scope. Split PR commenting into a separate PR-only job (or otherwise isolate permissions) so non-PR runs keep minimal token access.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/helm-prerelease.yml around lines 69 - 77, The workflow currently grants pull-requests: write unconditionally while the job's if condition allows push and workflow_dispatch, so restrict write scope by moving the PR-specific permission into a separate job or conditional job that only runs for pull_request events; locate the job with the if block and the permissions stanza and create a PR-only job (or duplicate the existing job) that uses if: github.event_name == 'pull_request' and sets permissions: pull-requests: write, while the original job (for push and workflow_dispatch) keeps only minimal permissions (e.g., contents: read, packages: write) so non-PR runs never receive pull-requests: write.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/helm-prerelease.yml:
- Around line 69-77: The workflow currently grants pull-requests: write
unconditionally while the job's if condition allows push and workflow_dispatch,
so restrict write scope by moving the PR-specific permission into a separate job
or conditional job that only runs for pull_request events; locate the job with
the if block and the permissions stanza and create a PR-only job (or duplicate
the existing job) that uses if: github.event_name == 'pull_request' and sets
permissions: pull-requests: write, while the original job (for push and
workflow_dispatch) keeps only minimal permissions (e.g., contents: read,
packages: write) so non-PR runs never receive pull-requests: write.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 573a0b15-0e0f-454c-8e41-b061d8f80126
📒 Files selected for processing (2)
.github/workflows/helm-prerelease.yml.github/workflows/pr_checks.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/pr_checks.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). (32)
- GitHub Check: prerelease
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (python)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: .github/workflows/release-helm.yml:42-46
Timestamp: 2025-06-25T13:24:23.836Z
Learning: In .github/workflows/release-helm.yml, the user nicktrn confirmed that using 'entrypoint' with 'docker://' steps works fine, contrary to previous analysis suggesting it's unsupported.
🔇 Additional comments (3)
.github/workflows/helm-prerelease.yml (3)
106-120: Event-specific prerelease versioning logic looks solid.The PR/push/manual branching is clear, and the empty-slug fallback prevents malformed manual versions.
128-134:appVersionoverride hardening is a good improvement.Using
yqwithstrenv(APP_VERSION)avoids the prior shell-escaping pitfalls for manual inputs.
147-160: Good UX split between run summary and PR comments.Writing
$GITHUB_STEP_SUMMARYfor all runs and gating PR comments topull_requestkeeps non-PR runs informative without noisy PR API calls.Also applies to: 163-173
|
ready |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| PRERELEASE_VERSION="${BASE_VERSION}-main.${SHORT_SHA}" | ||
| else | ||
| SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) | ||
| REF_SLUG=$(echo "${{ github.ref_name }}" | tr '/' '-' | tr -cd 'a-zA-Z0-9-') |
There was a problem hiding this comment.
🟡 Script injection via direct interpolation of github.ref_name in shell command
On line 115, ${{ github.ref_name }} is directly interpolated into a run: shell script. GitHub Actions expands ${{ }} expressions before the shell executes, so if the ref name contains shell metacharacters (e.g., backticks, $(), etc.), they will be interpreted by the shell. For example, a branch named test`malicious-command` would execute malicious-command. While this requires write access to the repo (to both create the branch and trigger workflow_dispatch), it's a well-documented GitHub Actions script injection anti-pattern that could allow exfiltration of secrets available in the workflow (e.g., GITHUB_TOKEN with packages: write).
Recommended fix: use an environment variable
Pass github.ref_name via an environment variable so the shell receives it as data rather than code:
env:
REF_NAME: ${{ github.ref_name }}
run: |
REF_SLUG=$(echo "$REF_NAME" | tr '/' '-' | tr -cd 'a-zA-Z0-9-')Prompt for agents
In .github/workflows/helm-prerelease.yml, line 115, the expression ${{ github.ref_name }} is directly interpolated into a shell run: block, which is a script injection vulnerability. GitHub Actions expands the expression before shell execution, so shell metacharacters in branch names (backticks, $(), etc.) would be executed.
The fix is to pass github.ref_name through an environment variable instead of direct interpolation. Change the else block (lines 113-119) to pass REF_NAME as an env var on the step, and reference $REF_NAME in the shell script. Something like:
env:
REF_NAME: ${{ github.ref_name }}
run: |
SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7)
REF_SLUG=$(echo "$REF_NAME" | tr '/' '-' | tr -cd 'a-zA-Z0-9-')
...
Note: github.sha is safe since it's always a hex string, but github.ref_name is user-influenced and needs the env var treatment.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Today the helm prerelease workflow only fires on PRs that touch
hosting/k8s/helm/**. Two consequences we ran into:changeset-release/mainPR's prerelease comment goes stale once the release branch gets force-pushed without a helm-touching commit (the bot'sChart.yamlbump alone doesn't seem to refire the trigger reliably).appVersion(e.g.v4.4.5) whose Docker images don't exist until after merge + tag. So that prerelease chart can't actually be installed end-to-end.Renames the workflow to
helm-prerelease.ymland adds two new triggers:push: mainwithpaths: hosting/k8s/helm/**-> rolling prereleases versioned<base>-main.<sha>.appVersionstays at whateverChart.yamlhas (i.e. last released), so installs pull real images. Tests that chart structure is deployable, even if the app code is one release behind.workflow_dispatchwith optionalapp_versioninput -> manually trigger a prerelease and optionally overrideappVersion(e.g. pin tomainor a specific tag). Useful for testing chart + app-version combinations on demand.PR behavior unchanged: same
<base>-pr<N>.<sha>versioning, same posted/updated comment.Why not also bypass paths for
changeset-release/main? The release PR's chart references not-yet-builtv4.4.5images, so those prereleases aren't actually installable. The rolling main prerelease covers the testable case better.Why not SHA-pin
appVersionto a built image likemain-<sha>? Bigger change - the docker publish workflows currently only push:main(no SHA-suffixed tag). Worth doing later if we want first-class "install one chart, get exactly that commit's app code" testing, but out of scope here.Diff is mostly a rename. Substantive changes:
pushandworkflow_dispatchtriggersprereleasejobif:extended for the new event typesgithub.event_name == 'pull_request'github.reffor non-PR runs