Pin Third-Party Actions to Immutable Commit SHAs and Eliminate Workflow Expression Injection - #5
Conversation
…ow Expression Injection
### Description
This pull request addresses high- and medium-severity CI/CD supply chain and script execution vulnerabilities identified in `shared-workflows` during the workspace-wide security audit (**FM-07, FM-08**).
Because workflows in this repository are designed for reusable consumption (`workflow_call`) across all organization repositories, referencing mutable action tags presented an organization-wide supply-chain risk. Additionally, inline expression evaluation inside shell run blocks opened potential script-injection vectors. This PR pins external actions to verified commit SHAs and enforces environment variable indirection for workflow parameters.
### Key Changes & Remediations
#### 1. Immutable Action Pinning (FM-07 - `ci.yml`, `dependency-cooldown.yml`, `dependency-cooldown-audit.yml`)
* **Digest Pinning:** Replaced mutable `@v7` action tags with full 40-character commit digests verified via the GitHub REST API:
* `actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1` (`# v7`)
* `actions/setup-node@820762786026740c76f36085b0efc47a31fe5020` (`# v7`)
* **Supply Chain Boundary:** Prevents upstream tag hijacking or unreviewed mutations from injecting untrusted steps into downstream CI pipelines across the organization.
#### 2. Safe Expression Handling (FM-08 - `dependency-cooldown.yml`)
* **Environment Indirection:** In the `Reject unsupported events` step, eliminated inline `${{ github.event_name }}` interpolation from the bash script body.
* **Shell Variable Expansion:** Passed the context value through `env: EVENT_NAME: ${{ github.event_name }}` and referenced it as `$EVENT_NAME` within the command line.
### How to Review
1. Inspect `.github/workflows/` files (`ci.yml`, `dependency-cooldown.yml`, `dependency-cooldown-audit.yml`) to confirm that all `actions/checkout` and `actions/setup-node` invocations reference full commit digests.
2. Verify in `dependency-cooldown.yml` that no `${{ ... }}` expressions remain inside the `run:` execution block.
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
APPROVE — The workflow hardening is correct and preserves the existing behavior.
No blocking findings.
I verified that all six external actions/checkout and actions/setup-node references are pinned to full commit SHAs, that those SHAs exactly match the official v7 tag targets, and that both upstream commits are verified. The unsupported-event value is now passed through env and expanded by the shell as data, removing the inline Actions expression from the run block.
Validation:
- Reviewed the complete diff from base
10f146a1064e04caa6b50d86c29523592ecedf87to head776dd0bdd8611bb5842ef3f91650703d1942642f. actionlintv1.7.12 passed all three workflow files.npm ci,npm run typecheck,npm run build, andnpm testpassed (107 tests); the committed bundle remained clean.- The exact-head GitHub Actions check passed.
Non-blocking note: the three edited YAML files were converted from LF to CRLF, which makes git diff --check report every line as trailing whitespace and obscures the otherwise small patch. Restoring LF would keep future diffs and blame cleaner, but this does not alter the validated workflow behavior.
|
please recheck |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
APPROVE — The workflow hardening is correct and preserves the existing behavior.
No blocking findings.
I verified that all six external actions/checkout and actions/setup-node invocations are pinned to full commit SHAs, that those SHAs exactly match the official v7 refs, and that both upstream commits have valid GitHub verification. The unsupported event name is now passed through env and expanded by the shell as data, eliminating inline Actions expression interpolation from the run block.
Validation:
- Reviewed the complete diff from base
10f146a1064e04caa6b50d86c29523592ecedf87to head28b5cd80e2ffb23fd2a25bc16e9af65af760e16d; the three follow-up commits have the same tree as the initial patch. actionlintv1.7.12 passed all three workflow files.npm ci,npm run typecheck,npm run build, andnpm testpassed (107 tests); the committed bundle remained clean.- The exact-head GitHub Actions CI check passed.
Non-blocking: the three edited workflow files still use CRLF, so git diff --check reports each added line as trailing whitespace and the patch remains noisier than its semantic changes. Restoring LF would improve future diffs and blame, but this does not affect the validated workflow behavior.
Description
This pull request addresses high- and medium-severity CI/CD supply chain and script execution vulnerabilities identified in
shared-workflowsduring the workspace-wide security audit (FM-07, FM-08).Because workflows in this repository are designed for reusable consumption (
workflow_call) across all organization repositories, referencing mutable action tags presented an organization-wide supply-chain risk. Additionally, inline expression evaluation inside shell run blocks opened potential script-injection vectors. This PR pins external actions to verified commit SHAs and enforces environment variable indirection for workflow parameters.Key Changes & Remediations
1. Immutable Action Pinning (FM-07 -
ci.yml,dependency-cooldown.yml,dependency-cooldown-audit.yml)@v7action tags with full 40-character commit digests verified via the GitHub REST API:actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1(# v7)actions/setup-node@820762786026740c76f36085b0efc47a31fe5020(# v7)2. Safe Expression Handling (FM-08 -
dependency-cooldown.yml)Reject unsupported eventsstep, eliminated inline${{ github.event_name }}interpolation from the bash script body.env: EVENT_NAME: ${{ github.event_name }}and referenced it as$EVENT_NAMEwithin the command line.How to Review
.github/workflows/files (ci.yml,dependency-cooldown.yml,dependency-cooldown-audit.yml) to confirm that allactions/checkoutandactions/setup-nodeinvocations reference full commit digests.dependency-cooldown.ymlthat no${{ ... }}expressions remain inside therun:execution block.