Skip to content

security: pin action to sha#391

Merged
bearpong merged 8 commits into
mainfrom
ci/pin-actions-to-sha
May 26, 2026
Merged

security: pin action to sha#391
bearpong merged 8 commits into
mainfrom
ci/pin-actions-to-sha

Conversation

@qtipbera
Copy link
Copy Markdown
Contributor

@qtipbera qtipbera commented May 22, 2026

  • Identify the single requested review comment to address
  • Inspect current workflow file state and related prior commits
  • Run existing repo validation commands before making changes
  • Apply only the requested workflow suggestion
  • Run targeted validation after the change
  • Commit and report progress with the minimal patch
  • Run final parallel validation and summarize results

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Validate GitHub Actions workflow by pinning third-party actions to immutable commit SHAs, reducing supply-chain risk from upstream tag movement.

Changes:

  • Pinned actions/checkout, actions/setup-node, pnpm/action-setup, and actions/cache to specific SHAs.
  • Pinned dorny/paths-filter to a specific SHA.
Comments suppressed due to low confidence (1)

.github/workflows/validate.yml:165

  • The base checkout step here implicitly uses the default branch HEAD. In a pull_request_target workflow that later uses secrets and writes PR comments, it’s safer/more deterministic to checkout the PR base commit explicitly via ref: ${{ github.event.pull_request.base.sha }}.
    - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
    - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
      with:
        repository: ${{ github.event.pull_request.head.repo.full_name }}
        ref: ${{ github.event.pull_request.head.sha }}
        path: ./head

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
Comment thread .github/workflows/validate.yml Outdated
@qtipbera qtipbera force-pushed the ci/pin-actions-to-sha branch from 85f54a8 to 9335752 Compare May 22, 2026 15:48
Copy link
Copy Markdown
Collaborator

@bearpong bearpong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CI is broken as no one is running. Please make sure this is not breaking it

@qtipbera
Copy link
Copy Markdown
Contributor Author

@bearpong

CI is failing because pnpm/action-setup v2.4.1 declares using: node16 in its action.yml. GitHub runners have been forcing Node.js 16 actions to run under Node.js 20 since early 2024, and v2 is not compatible — the maintainers flagged it explicitly in the repo README:

The v2 version of this action has stopped working with newer Node.js versions. Please, upgrade to the latest version to fix any issues.

https://github.com/pnpm/action-setup

pinact faithfully pinned the SHA of the v2 branch (eae0cfeb28... = v2.4.1), but the underlying action was already dead. This same bug exists on main today with the floating @v2 ref — it just manifests differently depending on runner state.

Fix

Upgrade pnpm/action-setup from v2.4.1 to v6.0.8 (all 7 occurrences):

-      uses: pnpm/action-setup@eae0cfeb286e66ffb5155f1a79b90583a127a68b # v2.4.1
+      uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093 # v6.0.8

v6.0.8 uses node24. The version: 9.15.9 input works identically — no other changes needed.

I verified the SHA:

$ git ls-remote --tags https://github.com/pnpm/action-setup.git | grep v6.0.8
d15e628...  refs/tags/v6.0.8
0e279bb...  refs/tags/v6.0.8^{}   ← commit SHA (pin target)

Heads up: Node.js 24 deadline June 2

The other pinned actions in this PR (actions/checkout@v4.3.1, actions/setup-node@v4.4.0, actions/cache@v4.3.0) all use Node.js 20. GitHub is forcing Node.js 24 on all runners starting June 2, 2026 — 8 days from now. After that date, these will start throwing deprecation warnings and eventually break.

v5 releases exist for all of them. Recommend a follow-up commit (or separate PR) bumping:

Current Target SHA
actions/checkout v4.3.1 v5.0.1 93cb6efe1820...
actions/setup-node v4.4.0 v5.0.0 a0853c245446...
actions/cache v4.3.0 v5.0.5 27d5ce7f107f...
dorny/paths-filter v3.0.3 v4.0.1 fbd0ab8f3e69...

I'll submit a fix to this PR now.

This comment was marked as resolved.

…ub.event.pull_request.base.sha to the first checkout step in format, data-consistency, detect-changes, upload-assets, images, coingecko, and pyth. With pull_request_target, a bare actions/checkout gets default branch HEAD, which can drift from the PR's recorded base commit. schema already had this; now all 8 jobs are consistent.

This comment was marked as resolved.

…node24 Add persist-credentials: false to every checkout step — both base and head — across all 8 jobs (15 checkout steps total). Prevents GITHUB_TOKEN from being written into .git/config on pull_request_target. upload-assets head checkout already had it. Upgrade remaining actions to node24 before June 2 deadline: actions/checkout v4.3.1 → v5.0.1 actions/setup-node v4.4.0 → v5.0.0 actions/cache v4.3.0 → v5.0.5 dorny/paths-filter v3.0.3 → v4.0.1

This comment was marked as resolved.

@qtipbera
Copy link
Copy Markdown
Contributor Author

Superseded by #392.

#392 splits validate.yml into two workflows (validate + post-validate) to isolate secrets from fork code. It includes all the changes from this PR — SHA pinning, node24 upgrades, head.refhead.sha, persist-credentials: false, ref: base.sha on all base checkouts — plus the security split, cache key hardening, and unused secret removal.

If we merge this first, #392 will conflict on validate.yml. Since #392 is the superset, merge #392 and close this one.

@bearpong
Copy link
Copy Markdown
Collaborator

@qtipbera please let's keep changes atomical and debuggable, I prefer not to merge all those changes at once.
I'd prefer first to upgrade packages, then we split the CI

@qtipbera
Copy link
Copy Markdown
Contributor Author

@bearpong Makes sense. Merge order:

  1. This PR (security: pin action to sha #391) — SHA pinning, node24 upgrades, persist-credentials: false, ref: base.sha on base checkouts. Pure infrastructure, no behavioral changes.
  2. security: Ci/fix validate workflow #392 — security split (validate + post-validate), cache key hardening, unused secret removal. I'll rebase it onto main after this lands.

This PR is ready for re-review when you are.

@bearpong
Copy link
Copy Markdown
Collaborator

ty @qtipbera, CI still seems broken though

image

@qtipbera
Copy link
Copy Markdown
Contributor Author

CI is broken because we disabled the Validate workflow for security reasons. PR #393 unblocks, then we can run CI on this one (#391).

@qtipbera
Copy link
Copy Markdown
Contributor Author

qtipbera commented May 26, 2026

@bearpong looks like CI has now passed ser

@bearpong bearpong merged commit 527ccff into main May 26, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants