Skip to content

fix(ci): harden GitHub Actions workflows#400

Open
emptyhammond wants to merge 6 commits into
mainfrom
fix/workflow-security-hardening
Open

fix(ci): harden GitHub Actions workflows#400
emptyhammond wants to merge 6 commits into
mainfrom
fix/workflow-security-hardening

Conversation

@emptyhammond
Copy link
Copy Markdown
Contributor

@emptyhammond emptyhammond commented May 20, 2026

  • Pin all GitHub Actions to SHA across all 10 workflows
  • Scope GitHub App tokens to minimum required permissions
  • Add explicit least-privilege permissions to e2e-web-cli-parallel.yml
  • Remove dependency cache from release/publish workflows

…ub-app risks

The workflow_run trigger runs from the default branch with full secret
access while checking out PR-controlled code. Although gated to
dependabot[bot] PRs only, the GitHub App token was unrestricted --
meaning a compromised or bypassed gate could grant Claude broad write
access to any resource the App can reach.

Changes:
- Pin all 5 actions to SHA to prevent supply-chain attacks via tag
  hijacking (a compromised action tag can execute arbitrary code with
  the workflow secret access)
- Scope the GitHub App token to only contents:write and
  pull-requests:write (the minimum Claude needs to push fixes and
  comment on PRs), limiting blast radius if the dependabot author
  gate is bypassed
… github-app risks

pull_request_target runs with the default branch workflow definition but
has full secret access, and this workflow checks out PR-controlled code
(github.event.pull_request.head.ref). The GitHub App token was
unrestricted, so a bypassed dependabot author gate could grant write
access to any resource the App can reach.

Changes:
- Pin all 4 actions to SHA to prevent supply-chain attacks via tag
  hijacking (a compromised action tag can execute arbitrary code with
  the workflow secret access)
- Scope the GitHub App token to only contents:write (the minimum
  needed to push the regenerated lockfile), limiting blast radius if
  the dependabot author gate is bypassed
….yml

Both workflows created GitHub App tokens with the App full permission
set. If the App has permissions beyond what these workflows need (e.g.
admin access, repository creation), a compromised action in the chain
could exploit the over-privileged token.

Changes:
- Pin all actions to SHA in both workflows to prevent supply-chain
  attacks via tag hijacking
- Scope claude-review.yml App token to contents:read,
  pull-requests:write, issues:write (minimum for reviewing PRs and
  posting comments)
- Scope pr-overview.yml App token to contents:read,
  pull-requests:write (minimum for reading diffs and posting overview
  comments)
- Remove unused issues:write from pr-overview.yml workflow-level
  permissions (the workflow only comments on PRs, never on issues)
…arallel.yml

This workflow had no explicit permissions block, inheriting the
repository default (often write-all). None of the 5 jobs (setup,
auth-tests, session-tests, ui-tests, rate-limit-test) need write access
to the repository, PR, or any other scope -- they only read source code,
build, and run Playwright tests. Granting write permissions increases
the blast radius if any of the 25 unpinned third-party actions in the
chain is compromised via tag hijacking.

Changes:
- Pin all 25 action uses to SHA to prevent supply-chain attacks via tag
  hijacking
- Add explicit permissions: contents: read at workflow level so no job
  receives unnecessary write access to the repository, packages, PRs,
  issues, deployments, or other GitHub resources
Both release workflows (release.yml, release-web-cli.yml) used
cache: pnpm in actions/setup-node, restoring cached dependencies from
a shared cache namespace. PR-triggered workflows (test.yml,
e2e-web-cli-parallel.yml) write to this same cache using keys an
attacker can predict. A malicious PR could poison the pnpm store cache,
and when a release tag is pushed, the publish job would restore the
poisoned cache -- potentially injecting malicious code into the
published npm package.

Changes:
- Remove cache: pnpm from setup-node in both release workflows so
  publish jobs always install fresh dependencies from the registry,
  eliminating the cross-workflow cache poisoning vector
- Pin all actions to SHA to prevent supply-chain attacks via tag
  hijacking (especially critical in publish workflows that hold
  NPM_TOKEN)
Actions referenced by mutable version tags (e.g. @v6) are vulnerable
to supply-chain attacks: if a tag is hijacked or force-pushed, the
compromised action executes with the workflow secrets and permissions.
This is especially dangerous for workflows that access E2E API keys
and access tokens (e2e-tests.yml, test.yml).

Pin all remaining action uses across these three workflows to
immutable commit SHAs so that the exact code running in CI is
cryptographically locked to a reviewed version.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

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

Project Deployment Actions Updated (UTC)
cli-web-cli Ready Ready Preview, Comment May 20, 2026 8:51pm

Request Review

@emptyhammond emptyhammond requested a review from AndyTWF May 20, 2026 20:57
@emptyhammond emptyhammond marked this pull request as ready for review May 20, 2026 20:57
@claude-code-ably-assistant
Copy link
Copy Markdown

Walkthrough

This PR hardens all 10 GitHub Actions workflows against supply-chain and privilege-escalation attacks. Every third-party action reference is pinned to an immutable commit SHA (replacing mutable version tags), GitHub App tokens are scoped to the minimum permissions each workflow actually needs, and the pnpm dependency cache is removed from release/publish workflows to eliminate the cross-workflow cache-poisoning vector.

Changes

Area Files Summary
CI – Action pinning audit.yml, e2e-tests.yml, test.yml Pin actions/checkout, pnpm/action-setup, actions/setup-node, and actions/cache to SHA
CI – Action pinning (large) e2e-web-cli-parallel.yml Pin all 25 action uses across 5 jobs + add permissions: contents: read at workflow level
CI – App token scoping claude-review.yml Scope App token to contents:read, pull-requests:write, issues:write; pin 3 actions
CI – App token scoping pr-overview.yml Scope App token to contents:read, pull-requests:write; remove unused issues:write; pin 3 actions
CI – App token scoping dependabot-claude-fix.yml Scope App token to contents:write, pull-requests:write; pin 5 actions
CI – App token scoping dependabot-lockfile.yml Scope App token to contents:write only; pin 4 actions
CI – Cache poisoning fix release.yml, release-web-cli.yml Remove cache: pnpm from setup-node so publish jobs always install fresh from the registry

Review Notes

  • Cache removal from release workflows: release.yml and release-web-cli.yml no longer restore the pnpm store cache before publishing. First-run publish jobs will be slightly slower (fresh install every time), but this eliminates the cross-workflow cache-poisoning path where a malicious PR could inject code into a published npm package.
  • SHA comments: All pinned SHAs include an inline # vN comment for human readability — reviewers should spot-check a few SHAs against the upstream action releases to confirm they match the tagged versions.
  • e2e-web-cli-parallel.yml permissions: The new permissions: contents: read is set at the workflow level and applies to all 5 jobs. None of the jobs write to the repo, so this is correct, but any future job added to this workflow that needs write access will need an explicit job-level override.
  • pr-overview.yml issues:write removal: The workflow only comments on PRs, not issues, so dropping this permission is correct and safe.
  • No functional changes: All changes are to CI configuration only — no source code, tests, or runtime behavior is affected.

@claude-code-ably-assistant
Copy link
Copy Markdown

Review Summary\n\nThis PR correctly pins all GitHub Actions to SHA commits and scopes GitHub App token permissions across 10 workflows. SHA pins are consistent across all files and most permission changes are correct.\n\nOne functional issue to flag:\n\n### pr-overview.yml - missing issues:write on app token will break comment posting\n\nThe PR removes issues: write from both the top-level GITHUB_TOKEN permissions and the app token. However, the workflow allows Claude to run gh pr comment (explicitly in the allowedTools list), which posts via the GitHub Issues API endpoint - that endpoint requires issues: write.\n\nWithout issues:write, the gh pr comment call will return a 403 and the PR overview comment will silently fail.\n\nThe inconsistency is visible by comparing the two workflows: claude-review.yml correctly keeps issues:write on the app token because it also uses gh pr comment. The pr-overview.yml change apparently intended to narrow the top-level GITHUB_TOKEN (reasonable), but accidentally also stripped it from the app token that actually posts the comment.\n\nFix: Add issues:write back to the app token in pr-overview.yml.\n\n---\n\n### Everything else looks good\n\n- SHA pins are consistent for all 10 workflows with no mismatches\n- claude-review.yml app token retains issues:write correctly\n- dependabot-lockfile.yml app token scoped to contents:write only (correct - just pushes to branches, no PR comments)\n- dependabot-claude-fix.yml app token scoped to contents:write, pull-requests:write (correct)\n- e2e-web-cli-parallel.yml gaining a top-level permissions: contents: read is a good tightening\n- Removing cache: pnpm from release.yml and release-web-cli.yml is intentional and defensible - release workflows should install fresh from the lockfile, not from potentially stale cached state

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant