Security: Pin CI dep versions and tidy code up#34770
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR pins GitHub Actions to specific commit SHAs across twelve CI workflow files, hardens ChangesWorkflow pinning and outputs changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/nx.yml:
- Line 67: The workflow step currently executes "npx nx-cloud@latest
start-ci-run ..." which pulls a mutable `@latest` package; replace this with a
pinned version or the locally installed CLI to avoid supply-chain drift—e.g.,
change the invocation that references "npx nx-cloud@latest" to a fixed version
token like "npx nx-cloud@<version>" or call the repo's installed binary (e.g.,
"npx nx-cloud" relying on lockfile) so the step that runs start-ci-run
--distribute-on="${{ steps.dist.outputs.config }}"
--stop-agents-after="$ALL_TASKS" uses a deterministic nx-cloud release.
In @.github/workflows/trigger-circle-ci-workflow.yml:
- Around line 79-88: The outputs reference steps.get-parameters.outputs.workflow
but there is no step with id "get-parameters"; update the workflow so the step
that computes the "workflow" value is given id: get-parameters (or change the
outputs reference to the actual step id), and ensure that the step writes the
output named "workflow" (using echo "workflow=..." >> $GITHUB_OUTPUT) so that
outputs.workflow resolves correctly; look for the step that sets workflow values
and add or correct the id to "get-parameters" and confirm outputs.workflow
points to steps.get-parameters.outputs.workflow.
🪄 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: 5d36af4a-6b8b-4f59-90af-e7fc544d2b7d
📒 Files selected for processing (15)
.github/workflows/agent-scan.yml.github/workflows/copilot-setup-steps.yml.github/workflows/cron-weekly.yml.github/workflows/danger-js.yml.github/workflows/fork-checks.yml.github/workflows/generate-sandboxes.yml.github/workflows/handle-release-branches.yml.github/workflows/markdown-link-check-config.json.github/workflows/nx.yml.github/workflows/prepare-non-patch-release.yml.github/workflows/prepare-patch-release.yml.github/workflows/publish.yml.github/workflows/stale.yml.github/workflows/triage.yml.github/workflows/trigger-circle-ci-workflow.yml
💤 Files with no reviewable changes (2)
- .github/workflows/markdown-link-check-config.json
- .github/workflows/cron-weekly.yml
| @@ -66,7 +66,7 @@ jobs: | |||
| fi | |||
| - run: npx nx-cloud@latest start-ci-run --distribute-on="${{ steps.dist.outputs.config }}" --stop-agents-after="$ALL_TASKS" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mutable runtime dependency usage in workflows
rg -n --type=yaml '@latest|npx\s+nx-cloud@' .github/workflows
# Discover whether nx-cloud is already pinned in repo package manifests
fd -HI '^package\.json$' -E node_modules -x rg -n '"nx-cloud"|@nrwl/nx-cloud' {}Repository: storybookjs/storybook
Length of output: 223
Pin nx-cloud to a specific version instead of using @latest.
Line 67 uses nx-cloud@latest, which fetches a mutable package at runtime and reintroduces supply-chain drift. Pin to a specific version or use a locally installed CLI backed by a lockfile.
Current code
- run: npx nx-cloud@latest start-ci-run --distribute-on="${{ steps.dist.outputs.config }}" --stop-agents-after="$ALL_TASKS"
🤖 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/nx.yml at line 67, The workflow step currently executes
"npx nx-cloud@latest start-ci-run ..." which pulls a mutable `@latest` package;
replace this with a pinned version or the locally installed CLI to avoid
supply-chain drift—e.g., change the invocation that references "npx
nx-cloud@latest" to a fixed version token like "npx nx-cloud@<version>" or call
the repo's installed binary (e.g., "npx nx-cloud" relying on lockfile) so the
step that runs start-ci-run --distribute-on="${{ steps.dist.outputs.config }}"
--stop-agents-after="$ALL_TASKS" uses a deterministic nx-cloud release.
There was a problem hiding this comment.
Pull request overview
This PR hardens Storybook’s GitHub Actions workflows against supply-chain and cache-poisoning risks by pinning action dependencies to immutable SHAs, tightening pull_request_target guidance/permissions, and reducing reliance on third-party actions.
Changes:
- Pinned multiple GitHub Action dependencies to commit SHAs across CI/release workflows.
- Migrated some workflow state passing from
$GITHUB_ENVto step/job outputs ($GITHUB_OUTPUT) and added explicitpermissions: {}forpull_request_targetworkflows. - Removed the weekly markdown link check workflow (and its config) and replaced a JSON-property action with local
jq.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/trigger-circle-ci-workflow.yml | Adds pull_request_target security warning + pins HTTP request action; migrates env → outputs. |
| .github/workflows/triage.yml | Pins balazsorban44/nissuer to a SHA. |
| .github/workflows/stale.yml | Pins actions/stale to a SHA. |
| .github/workflows/publish.yml | Pins multiple third-party actions used in release publishing. |
| .github/workflows/prepare-patch-release.yml | Pins checkout + discord notification action. |
| .github/workflows/prepare-non-patch-release.yml | Pins checkout + discord notification action. |
| .github/workflows/nx.yml | Pins checkout/setup-node/github-script/nx-set-shas actions to SHAs. |
| .github/workflows/markdown-link-check-config.json | Removes markdown link checker configuration. |
| .github/workflows/handle-release-branches.yml | Pins checkout + replaces JSON-property action usage with jq; migrates env → outputs. |
| .github/workflows/generate-sandboxes.yml | Pins checkout/setup-node + pins discord notification action. |
| .github/workflows/fork-checks.yml | Pins checkout across fork validation/test jobs. |
| .github/workflows/danger-js.yml | Pins checkout for Danger JS workflow. |
| .github/workflows/cron-weekly.yml | Removes weekly markdown link check workflow. |
| .github/workflows/copilot-setup-steps.yml | Updates pinned checkout SHA. |
| .github/workflows/agent-scan.yml | Adds pull_request_target security warning, sets empty default permissions, pins actions, and tweaks checkout step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/agent-scan.yml (1)
63-69: ⚡ Quick winMake the checkout explicitly read-only.
This job clears all default scopes, but
actions/checkoutstill recommendscontents: readunless you provide alternate auth, and it keeps auth material available to later steps unlesspersist-credentials: falseis set. Since this workflow only needs a trusted, read-only checkout of the base SHA, tightening both here reduces token exposure without changing the job’s behavior. (github.com)Suggested diff
agentscan: if: | github.repository_owner == 'storybookjs' && github.event.pull_request.head.repo.full_name != github.repository && !contains( fromJSON('["dependabot[bot]", "github-actions[bot]","storybook-bot"]'), github.event.pull_request.user.login ) runs-on: ubuntu-latest permissions: + contents: read pull-requests: write steps: - name: Checkout code from `next`/`main` branch (trusted code, not PR author code) uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + persist-credentials: false ref: ${{ github.event.pull_request.base.sha }}🤖 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/agent-scan.yml around lines 63 - 69, The checkout step is leaving auth material available; update the permissions and checkout config to make the base-branch checkout explicitly read-only by adding "contents: read" to the job's permissions (in addition to the existing "pull-requests: write") and setting "persist-credentials: false" in the actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd step's with block (keep the existing ref: ${{ github.event.pull_request.base.sha }}).
🤖 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.
Nitpick comments:
In @.github/workflows/agent-scan.yml:
- Around line 63-69: The checkout step is leaving auth material available;
update the permissions and checkout config to make the base-branch checkout
explicitly read-only by adding "contents: read" to the job's permissions (in
addition to the existing "pull-requests: write") and setting
"persist-credentials: false" in the
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd step's with block
(keep the existing ref: ${{ github.event.pull_request.base.sha }}).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8de85d29-4834-40fa-ab6d-bf4f1c0bb6b5
📒 Files selected for processing (1)
.github/workflows/agent-scan.yml
14c4bb0 to
732f066
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/handle-release-branches.yml (1)
68-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet explicit token permissions for
next-release-branch-check.This job only computes/prints values, but currently inherits default
GITHUB_TOKENpermissions. Add an explicit empty permission set to reduce blast radius.Suggested fix
next-release-branch-check: if: ${{ always() && github.repository_owner == 'storybookjs' }} needs: [branch-checks, get-next-release-branch] runs-on: ubuntu-latest + permissions: {} steps:🤖 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/handle-release-branches.yml around lines 68 - 87, Add an explicit empty permission set to the next-release-branch-check job to avoid inheriting GITHUB_TOKEN privileges: inside the job definition for next-release-branch-check (the job that includes steps with id is-next-release-branch and relevant-base-branch), add a permissions: none (or permissions: {}) stanza at the same indentation as runs-on so the job runs with no token permissions.
♻️ Duplicate comments (1)
.github/workflows/trigger-circle-ci-workflow.yml (1)
79-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
outputs.workflowstill points at a nonexistent step.Line 88 reads
steps.get-parameters.outputs.workflow, but none of the steps above hasid: get-parameters. That leavesneeds.get-parameters.outputs.workflowempty, so Line 95 can skip the CircleCI trigger entirely.Either give the writer step an
id: get-parameters, or coalesce outputs from uniquely named steps instead of referencing a missing id.#!/bin/bash set -euo pipefail FILE="$(fd -p '.github/workflows/trigger-circle-ci-workflow.yml' | head -n1)" sed -n '75,95p' "$FILE" printf '\nMatches for the referenced output and any matching step id:\n' rg -n 'steps\.get-parameters\.outputs\.workflow|id:\s*get-parameters' "$FILE"🤖 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/trigger-circle-ci-workflow.yml around lines 79 - 88, The outputs block references steps.get-parameters.outputs.workflow but no step has id: get-parameters; fix by either adding id: get-parameters to the step that decides the workflow (the conditional run step(s) that echo "workflow=...") so the output can be read as steps.get-parameters.outputs.workflow, or change outputs.workflow to point to an existing step id that actually sets the workflow output; update the step that emits the workflow value (or give the writer step an explicit id: get-parameters) so the outputs.workflow mapping resolves.
🤖 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/agent-scan.yml:
- Around line 85-91: The restore-keys entry is actor-unsafe because
"restore-keys: agentscan-cache-" can match caches from other github.actor
values; update the workflow so restore-keys either is removed or is actor-scoped
to match the primary key. Concretely, change the restore-keys value to the same
actor-specific prefix (agentscan-cache-${{ github.actor }}) or delete the
restore-keys line so the actions/cache step only restores the exact key defined
by key: agentscan-cache-${{ github.actor }}; adjust the block using the symbols
key and restore-keys to make the cache actor-isolated.
- Around line 67-70: Update the checkout step that uses
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd (the "Checkout code
from `next`/`main` branch (trusted code, not PR author code)" step) to set
persist-credentials: false so the runner does not leave authentication tokens
available to later steps; also tighten the cache step(s) that use restore-keys
like "agentscan-cache-" to avoid cross-actor restores by making the restore key
more specific (include the actor or other unique identifier) so caches created
by a different `${{ github.actor }}` cannot be restored inadvertently.
In @.github/workflows/fork-checks.yml:
- Around line 16-18: The checkout steps using the actions/checkout action (the
lines with "uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" /
actions/checkout) should add persist-credentials: false to avoid storing the
token in git config; update every checkout step in the workflow (all occurrences
of actions/checkout) to include the with: persist-credentials: false key
alongside the existing fetch-depth: 2 so credential persistence is disabled for
these read-only fork-check jobs.
In @.github/workflows/handle-release-branches.yml:
- Around line 82-85: The run step is vulnerable because it injects branch names
directly into a single-quoted shell string; change it to pass the branch outputs
as separate, quoted shell arguments so they cannot break out and execute code:
use the existing outputs (steps.is-next-release-branch.outputs.result,
needs.branch-checks.outputs.branch,
steps.relevant-base-branch.outputs.relevant-base-branch) but call echo with
quoted expansions or assign them to shell variables first, e.g. echo "WARNING:
Do not push directly to the branch:" "${{ needs.branch-checks.outputs.branch }}"
"This branch is created..." "${{
steps.relevant-base-branch.outputs.relevant-base-branch }}" and then exit 1 —
this ensures any special characters in the branch names are treated as literal
text, not shell syntax.
---
Outside diff comments:
In @.github/workflows/handle-release-branches.yml:
- Around line 68-87: Add an explicit empty permission set to the
next-release-branch-check job to avoid inheriting GITHUB_TOKEN privileges:
inside the job definition for next-release-branch-check (the job that includes
steps with id is-next-release-branch and relevant-base-branch), add a
permissions: none (or permissions: {}) stanza at the same indentation as runs-on
so the job runs with no token permissions.
---
Duplicate comments:
In @.github/workflows/trigger-circle-ci-workflow.yml:
- Around line 79-88: The outputs block references
steps.get-parameters.outputs.workflow but no step has id: get-parameters; fix by
either adding id: get-parameters to the step that decides the workflow (the
conditional run step(s) that echo "workflow=...") so the output can be read as
steps.get-parameters.outputs.workflow, or change outputs.workflow to point to an
existing step id that actually sets the workflow output; update the step that
emits the workflow value (or give the writer step an explicit id:
get-parameters) so the outputs.workflow mapping resolves.
🪄 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: abad636f-bf91-4399-8881-1cc603f40a89
📒 Files selected for processing (14)
.github/workflows/agent-scan.yml.github/workflows/copilot-setup-steps.yml.github/workflows/cron-weekly.yml.github/workflows/fork-checks.yml.github/workflows/generate-sandboxes.yml.github/workflows/handle-release-branches.yml.github/workflows/markdown-link-check-config.json.github/workflows/nx.yml.github/workflows/prepare-non-patch-release.yml.github/workflows/prepare-patch-release.yml.github/workflows/publish.yml.github/workflows/stale.yml.github/workflows/triage.yml.github/workflows/trigger-circle-ci-workflow.yml
💤 Files with no reviewable changes (2)
- .github/workflows/cron-weekly.yml
- .github/workflows/markdown-link-check-config.json
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/publish.yml
4673699 to
ac1e497
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/e530eddf-6816-430d-8aff-28fa7acd8411 Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Steve Dodier-Lazaro <Sidnioulz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/fa027030-e33c-4090-a74c-b4a3162952af Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
ac1e497 to
2d59dc9
Compare
What I did
I reviewed all our CI workflows for potential cache poisoning issues, and made the following updates at the same time:
pull_request_targetworkflows with security rules to followGITHUB_ENVwith step outputs for more accurate context validation (your IDE now tells you when a previous job output might be undefined)jqcall to reduce reliance on depsChecklist for Contributors
Testing
This will need to be manually audited and all subsequent runs of the jobs monitored.
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Chores
Removals