Skip to content

fix: pin 19 unpinned action(s),extract 2 unsafe expression(s) to env vars#7435

Open
dagecko wants to merge 6 commits intoOpenBB-finance:developfrom
dagecko:runner-guard/fix-ci-security
Open

fix: pin 19 unpinned action(s),extract 2 unsafe expression(s) to env vars#7435
dagecko wants to merge 6 commits intoOpenBB-finance:developfrom
dagecko:runner-guard/fix-ci-security

Conversation

@dagecko
Copy link
Copy Markdown

@dagecko dagecko commented Mar 26, 2026

This is a re-submission of #7434, which was closed due to a branch issue on my end. Same fixes, apologies for the noise.

Security: Harden GitHub Actions workflows

Hey, I found some CI/CD security issues in this repo's GitHub Actions workflows. These are the same vulnerability classes that were exploited in the tj-actions/changed-files supply chain attack. I've been reviewing repos that are affected and submitting fixes where I can.

This PR applies mechanical fixes and flags anything else that needs a manual look. Happy to answer any questions.

Fixes applied

Rule Severity File Description
RGS-007 high .github/workflows/build-desktop-osx64.yml Pinned 2 third-party action(s) to commit SHA
RGS-007 high .github/workflows/build-desktop-osxARM.yml Pinned 2 third-party action(s) to commit SHA
RGS-007 high .github/workflows/build-desktop-win64.yml Pinned 5 third-party action(s) to commit SHA
RGS-007 high .github/workflows/draft-release.yml Pinned 1 third-party action(s) to commit SHA
RGS-007 high .github/workflows/general-linting.yml Pinned 1 third-party action(s) to commit SHA
RGS-002 high .github/workflows/general-linting.yml Extracted 1 unsafe expression(s) to env vars
RGS-007 high .github/workflows/gh-branch-name-check.yml Pinned 1 third-party action(s) to commit SHA
RGS-007 high .github/workflows/gh-pr-labels.yml Pinned 2 third-party action(s) to commit SHA
RGS-007 high .github/workflows/release-desktop.yml Pinned 1 third-party action(s) to commit SHA
RGS-002 high .github/workflows/release-desktop.yml Extracted 1 unsafe expression(s) to env vars
RGS-007 high .github/workflows/test-unit-desktop-osx64.yml Pinned 1 third-party action(s) to commit SHA
RGS-007 high .github/workflows/test-unit-desktop-osxARM.yml Pinned 1 third-party action(s) to commit SHA
RGS-007 high .github/workflows/test-unit-desktop-win64.yml Pinned 1 third-party action(s) to commit SHA
RGS-007 high .github/workflows/test-unit-desktop-winARM.yml Pinned 1 third-party action(s) to commit SHA

Additional findings (manual review recommended)

| Rule | Severity | File | Description |
| RGS-003 | high | .github/workflows/release-desktop.yml | Filename Injection via Git Diff or File Listing |
| RGS-003 | high | .github/workflows/release-desktop.yml | Filename Injection via Git Diff or File Listing |
| RGS-007 | medium | .github/workflows/general-linting.yml | Unpinned Third-Party Action Using Mutable Tag |

Why this matters

GitHub Actions workflows that use untrusted input in run: blocks or reference unpinned third-party actions are vulnerable to code injection and supply chain attacks. These are the same vulnerability classes exploited in the tj-actions/changed-files incident which compromised CI secrets across thousands of repositories.

How to verify

Review the diff, each change is mechanical and preserves workflow behavior:

  • Expression extraction: Moves ${{ }} expressions from run: blocks into env: mappings, preventing shell injection
  • SHA pinning: Pins third-party actions to immutable commit SHAs (original version tag preserved as comment)

If this PR is not welcome, just close it and I won't send another.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 26, 2026

CLA assistant check
All committers have signed the CLA.

@dagecko
Copy link
Copy Markdown
Author

dagecko commented Mar 27, 2026

CLA signed.

  • Chris

@deeleeramone
Copy link
Copy Markdown
Contributor

Hi, @dagecko and thanks for the PR. IIUC, some of these commit SHA will not work in this context. For example, the Rust Toolchain commit is not found on branch. Can you please check these again?

Screenshot 2026-03-27 at 4 43 01 PM Screenshot 2026-03-27 at 4 42 10 PM

@dagecko
Copy link
Copy Markdown
Author

dagecko commented Mar 28, 2026

Hey @deeleeramone, thanks for flagging that. I looked into it and the SHAs are all valid full 40-character commits. The "not found on branch" message you're seeing is a known GitHub UI quirk with dtolnay/rust-toolchain specifically. That action uses orphan commits for each toolchain version, so they're valid but not reachable from the master branch history. The dtolnay/rust-toolchain docs (https://github.com/dtolnay/rust-toolchain) explicitly support pinning via full-length commit SHA.

That said, if any of these are causing CI failures or you'd prefer a different approach for specific actions, let me know and I'm happy to adjust. Hope that helps.

  • Chris

@deeleeramone
Copy link
Copy Markdown
Contributor

Hey @dagecko, this is what GitHub has to say about it, we should probably follow along.

  1. dtolnay/rust-toolchain@631a55b... (comment: # stable)
    Used in build-desktop-osx64.yml and build-desktop-win64.yml.
  • This SHA is on the stable branch, not master. The commit page says "This commit does not belong to any branch on this repository" — the stable branch has since been force-pushed past it (current HEAD is 29eef33).
  • The repo's README explicitly warns: "it is required that you pick a SHA that is within the history of the master branch. Any commit that is not within the history of master will eventually get garbage-collected and your workflows will fail."
  • Fix: Pin to the master branch SHA (e97e2d8..., which is already used elsewhere as # v1) and pass toolchain: stable as an input, or use the @stable tag unpinned.
  1. SSLcom/esigner-codesign@b7f8ff3... (comment: # develop)
    Used 3 times in build-desktop-win64.yml.
  • This SHA is on the develop branch, which is a pre-release/integration branch. The actual v1.3.2 release tag points to cf5f6c1 (the merge commit on main), which is already correctly used elsewhere in the same file.
  • Fix: Use cf5f6c1... (# v1.3.2) consistently for all references in that file.

@dagecko
Copy link
Copy Markdown
Author

dagecko commented Apr 2, 2026

@deeleeramone apologies for the incorrect SHAs and thank you for taking the time to dig into both of these. You were right on both counts. Fixed and pushed:

  1. dtolnay/rust-toolchain: updated to the master branch SHA (3c5f7ea). The stable branch SHAs get garbage collected per the repo README. I missed that warning and should have caught it.

  2. sslcom/esigner-codesign: updated all 3 references to use cf5f6c1 (v1.3.2 on main) consistently. The 4th reference was already correct. No reason to pin to the develop branch when the release tag is right there.

Appreciate the thorough review.

  • Chris (dagecko)

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.

3 participants