-
-
Notifications
You must be signed in to change notification settings - Fork 8
chore(deps): bump actions/github-script from 8.0.0 to 9.0.0 #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -67,10 +67,10 @@ | |||||
| accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} | ||||||
| command: pages deploy docs/build --project-name=ralph-starter-docs --branch=${{ github.head_ref }} | ||||||
|
|
||||||
| # Comment preview URL on the PR | ||||||
|
Check failure on line 70 in .github/workflows/deploy-docs.yml
|
||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 deploy-docs.yml pins actions/github-script to the mutable tag @v9.0.0 instead of the full commit SHA used in every other workflow updated by this PR. A mutable tag can be force-pushed by the upstream maintainer to point to arbitrary code, creating a supply chain attack vector. Fix by replacing @v9.0.0 with @3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0. Extended reasoning...What the bug is: In Why it matters: Version tags like Why existing code does not prevent it: GitHub Actions has no built-in enforcement of SHA pinning. The Step-by-step proof:
How to fix: Replace line 70 in # Before (mutable — vulnerable)
uses: actions/github-script@v9.0.0
# After (immutable — safe)
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0This matches the pattern already used in all other updated workflows in this PR. |
||||||
| - name: Comment Preview URL | ||||||
| if: github.event_name == 'pull_request' | ||||||
| uses: actions/github-script@v8 | ||||||
| uses: actions/github-script@v9.0.0 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/deploy-docs.yml
Line: 73
Comment:
**Mutable tag instead of SHA pin**
`deploy-docs.yml` uses `@v9.0.0` (a mutable tag) while all five other updated files consistently use a SHA-pinned reference (`@3a2844b7e9c422d3c10d287c895573f7108da1b3`). A maintainer or attacker who force-pushes the `v9.0.0` tag could cause this step to execute arbitrary code; SHA pinning eliminates that risk.
```suggestion
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| with: | ||||||
| script: | | ||||||
| const branch = context.payload.pull_request.head.ref | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟣 A pre-existing script injection vulnerability exists in bundle-analysis.yml: the bundleBreakdown variable is constructed using a backtick template literal with a direct GitHub Actions expression (const bundleBreakdown = backtick-dollar-brace steps.compare.outputs.bundle_breakdown end-brace-backtick), so GitHub Actions interpolates the raw value into JS source before parsing — a malicious PR could craft dist/ filenames with backticks (SyntaxError) or dollar-brace sequences (arbitrary JS execution). The fix is to pass the value via an env: block and read it as process.env.BUNDLE_BREAKDOWN.
Extended reasoning...
What the bug is and how it manifests
In .github/workflows/bundle-analysis.yml (around line 130), the bundleBreakdown variable is assigned via a backtick template literal with a raw GitHub Actions expression:
const bundleBreakdown =
${{ steps.compare.outputs.bundle_breakdown }};GitHub Actions evaluates ${{ ... }} expressions BEFORE the script engine parses the JavaScript. The raw string value is substituted directly into the JS source code — whatever bundle_breakdown contains becomes literal characters in the JavaScript source.
The specific code path that triggers it
The value of bundle_breakdown is set in the Compare bundle sizes step using du -sh dist/*. The dist/ directory is built directly from the PR's own source code via pnpm build. On Linux, filenames can contain any character except null and forward-slash, including backticks and dollar-brace sequences.
Why existing code does not prevent it
There is no sanitization of filenames before they are captured by du -sh dist/*, no escaping when writing to GITHUB_OUTPUT, and no sanitization when reading the output into the JavaScript context. The template literal interpolation happens at the GitHub Actions layer, before the script engine ever sees the code.
What the impact would be
The workflow runs on pull_request events and has pull-requests:write permission. A malicious contributor could craft a PR that generates files in dist/ with specially crafted names. A filename containing a backtick terminates the template literal and causes a SyntaxError (denial of service). A filename containing a dollar-brace expression causes that expression to be evaluated as JavaScript, potentially exfiltrating secrets or posting arbitrary content to the PR.
How to fix it
Pass the value via an env: block and access it as an environment variable. Add BUNDLE_BREAKDOWN under env: pointing to the steps output, then read process.env.BUNDLE_BREAKDOWN in the script instead of using a template literal. This keeps the value out of the JS source code entirely.
Step-by-step proof
This bug is pre-existing and was not introduced by the v8-to-v9 upgrade in this PR, but the PR touches this exact step, making it an appropriate opportunity to flag and fix.