fix: prevent script injection in release workflow github-script steps#950
fix: prevent script injection in release workflow github-script steps#950
Conversation
Pass github context values via env vars instead of interpolating them directly into github-script script blocks, preventing potential RCE via untrusted input in github.event.inputs.title. Signed-off-by: Tomer Figenblat <tomer@figenblat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the release workflow’s github-script steps to eliminate direct expression interpolation and instead pass potentially untrusted values through environment variables, reading them via process.env to avoid script injection and RCE risk. Sequence diagram for updated release workflow github-script stepssequenceDiagram
actor Developer
participant GitHub
participant ReleaseWorkflow
participant CreateReleaseNameStep
participant CreateReleaseStep
participant GitHubAPI
Developer->>GitHub: Trigger release workflow dispatch
GitHub->>ReleaseWorkflow: Start release job with inputs
ReleaseWorkflow->>CreateReleaseNameStep: Run github-script action
CreateReleaseNameStep->>CreateReleaseNameStep: Read NEXT_VERSION from env
CreateReleaseNameStep->>CreateReleaseNameStep: Read INPUT_TITLE from env via process.env
CreateReleaseNameStep->>CreateReleaseNameStep: Build release name string
CreateReleaseNameStep->>ReleaseWorkflow: Output release_name value
ReleaseWorkflow->>CreateReleaseStep: Run github-script action
CreateReleaseStep->>CreateReleaseStep: Read NEXT_VERSION from env via process.env
CreateReleaseStep->>CreateReleaseStep: Read RELEASE_NAME from env via process.env
CreateReleaseStep->>GitHubAPI: POST create release with tag_name and name
GitHubAPI-->>CreateReleaseStep: Release created, return html_url
CreateReleaseStep-->>ReleaseWorkflow: Set html_url output
ReleaseWorkflow-->>GitHub: Job completes
GitHub-->>Developer: Report workflow result
Flow diagram for CreateReleaseName github-script logicflowchart TD
A[Start CreateReleaseNameStep] --> B[Read NEXT_VERSION from env]
B --> C[Set retval to NEXT_VERSION]
C --> D{Is INPUT_TITLE set in env?}
D -- Yes --> E[Concatenate ' - ' and INPUT_TITLE to retval]
D -- No --> F[Keep retval unchanged]
E --> G[core.setOutput value = retval]
F --> G[core.setOutput value = retval]
G --> H[End CreateReleaseNameStep]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request refactors the GitHub Actions release workflow to access version and title data through environment variables instead of direct workflow context interpolation. It updates the release steps to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Test Results66 tests 66 ✅ 1s ⏱️ Results for commit 8c7c8cc. ♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #950 +/- ##
=======================================
Coverage 96.02% 96.02%
=======================================
Files 1 1
Lines 327 327
=======================================
Hits 314 314
Misses 13 13 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml:
- Around line 76-89: The workflow currently interpolates step outputs like ${{
steps.bumper.outputs.next }} and ${{ steps.bumper.outputs.dev }} directly inside
run: blocks (e.g., the "Set new project version" step and the "Commit, tag, and
push" step); change each such step to expose those outputs via an env: mapping
(e.g., NEXT_VERSION: ${{ steps.bumper.outputs.next }} and DEV_VERSION: ${{
steps.bumper.outputs.dev }}) and then reference the env var (e.g., $NEXT_VERSION
or $DEV_VERSION) inside the shell commands (echo, sed, git commit message,
tagging, etc.) so all occurrences of direct interpolation are replaced with env
var usage for defense-in-depth.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml: - Around line 76-89: The workflow currently interpolates step outputs like ${{ steps.bumper.outputs.next }} and ${{ steps.bumper.outputs.dev }} directly inside run: blocks (e.g., the "Set new project version" step and the "Commit, tag, and push" step); change each such step to expose those outputs via an env: mapping (e.g., NEXT_VERSION: ${{ steps.bumper.outputs.next }} and DEV_VERSION: ${{ steps.bumper.outputs.dev }}) and then reference the env var (e.g., $NEXT_VERSION or $DEV_VERSION) inside the shell commands (echo, sed, git commit message, tagging, etc.) so all occurrences of direct interpolation are replaced with env var usage for defense-in-depth..github/workflows/release.yml (1)
76-89: Consider applying the same env-var pattern torun:steps that interpolate step outputs (defense-in-depth).Lines 80–81, 89, 104, 108, 144, and 151 still interpolate
${{ steps.bumper.outputs.next }}and${{ steps.bumper.outputs.dev }}directly into shellrun:blocks. While these originate from a trusted, pinned action (tomerfi/version-bumper-action@2.0.6), applying the sameenv:/$VARIABLEpattern would provide defense-in-depth against any future supply-chain compromise of that action.Example for the "Set new project version" step
- name: Set new project version + env: + NEXT_VERSION: ${{ steps.bumper.outputs.next }} run: | - echo "${{ steps.bumper.outputs.next }}" > VERSION - sed -i 's/ version: .*/ version: "${{ steps.bumper.outputs.next }}"/g' mkdocs.yml + echo "$NEXT_VERSION" > VERSION + sed -i "s/ version: .*/ version: \"$NEXT_VERSION\"/g" mkdocs.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 76 - 89, The workflow currently interpolates step outputs like ${{ steps.bumper.outputs.next }} and ${{ steps.bumper.outputs.dev }} directly inside run: blocks (e.g., the "Set new project version" step and the "Commit, tag, and push" step); change each such step to expose those outputs via an env: mapping (e.g., NEXT_VERSION: ${{ steps.bumper.outputs.next }} and DEV_VERSION: ${{ steps.bumper.outputs.dev }}) and then reference the env var (e.g., $NEXT_VERSION or $DEV_VERSION) inside the shell commands (echo, sed, git commit message, tagging, etc.) so all occurrences of direct interpolation are replaced with env var usage for defense-in-depth.
Replace direct interpolation of step outputs in shell run blocks with env var references for defense-in-depth against supply-chain compromise of upstream actions. Signed-off-by: Tomer Figenblat <tomer@figenblat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
@coderabbitai resolve Addressed the defense-in-depth nitpick in 8c7c8cc — all |
|
I'll resolve my review comments now. 🐰✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Actions performedComments resolved. Auto-approval is disabled; enable |
Summary
release.ymlgithub-scriptsteps, as recommended by Sourcery AI.github.event.inputs.titleand step outputs throughenv:vars instead of interpolating them directly intoscript:blocks.process.envinside the script, treating them as data rather than executable code.Validation
actionlintpasses with no new findings.