chore: pin GitHub Actions to commit SHAs#97
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThis PR pins GitHub Actions references to specific commit SHAs across workflow files, and updates one Firefox packaging step to pass the XPI path through an environment variable before zipping. ChangesAction SHA pinning across workflows
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/security-scan-artifacts.yml (1)
98-104: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
github/codeql-action/upload-sarif@v4was left unpinned while every other action in this file was pinned to a SHA.The PR objective states the test plan includes "regex verification that only SHA references remain," but
upload-sarifat lines 100, 120, and 201 still uses the floating@v4tag. This breaks the consistency of the supply-chain hardening this PR is meant to deliver across the workflow.Also applies to: 118-124, 199-205
🤖 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/security-scan-artifacts.yml around lines 98 - 104, The upload-sarif steps are still using the floating github/codeql-action/upload-sarif@v4 tag, which breaks the SHA-only pinning requirement in this workflow. Update every upload-sarif usage in security-scan-artifacts.yml to reference the same pinned commit SHA pattern used by the other actions, keeping the existing step names and with-blocks unchanged..github/workflows/security-scan-source.yml (1)
136-141: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
github/codeql-action/upload-sarif@v4remains unpinned, breaking the stated "only SHA references remain" goal.Same gap as in
security-scan-artifacts.yml: every otheruses:in this file (checkout, setup-python, pip-audit, setup-node, upload-artifact) was pinned to a commit SHA, but this step at line 138 still floats on@v4.🤖 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/security-scan-source.yml around lines 136 - 141, The upload-sarif step in the security scan workflow is still using a floating tag instead of a commit SHA, which violates the pinning policy. Update the github/codeql-action/upload-sarif@v4 reference in the upload semgrep SARIF step to the appropriate pinned SHA, matching the style already used by the other actions in this workflow.
🧹 Nitpick comments (7)
.github/workflows/release-android-apk.yml (2)
172-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffOptional:
softprops/action-gh-releasecould be replaced withgh releasescript step.zizmor's
superfluous-actionshint notes this functionality is already available via theghCLI bundled on GitHub-hosted runners, avoiding a third-party action dependency. Not required for this PR's pinning scope.🤖 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/release-android-apk.yml at line 172, The release workflow currently uses the third-party softprops/action-gh-release step for publishing the APK; consider replacing that action usage with an equivalent gh release CLI step in the release-android-apk workflow. Update the release job to use the built-in gh command on GitHub-hosted runners while preserving the existing release behavior and inputs from the current release step.Source: Linters/SAST tools
68-68: 🔒 Security & Privacy | 🔵 TrivialCaching steps flagged for cache-poisoning exposure (pre-existing, unrelated to this PR).
zizmor flags both the
actions/setup-nodecache and theactions/cachekeystore-caching step as enabling caching by default (cache-poisoning), which is relevant since this job has write permissions (contents: write,actions: write). Worth a follow-up to scope cache keys tightly (e.g., per-branch/per-ref) or restrict cache restore to trusted refs.Also applies to: 80-80
🤖 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/release-android-apk.yml at line 68, The Android release workflow has cache-poisoning exposure because trusted write-capable jobs are using broad caching via actions/setup-node and the keystore cache step. Update the release-android-apk job to either disable default caching in setup-node or scope cache keys/restore conditions tightly (for example by branch or trusted ref), and review the keystore cache so it only restores for trusted refs. Use the actions/setup-node step and the keystore-caching step in this workflow to locate the changes.Source: Linters/SAST tools
.github/workflows/publish-crates-io.yml (1)
50-50: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCheckout step persists GitHub credentials in local git config.
actions/checkoutdefaults topersist-credentials: true, leaving theGITHUB_TOKENin the git config for the rest of the job. Combined withid-token: writepermission in this job, an unrelated compromised step later in the job could exfiltrate the token. Static analysis (zizmor) flags this asartipacked.🔒 Proposed fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: falseThis same pattern recurs in
publish-docker-image.yml,publish-firefox-extension.yml,publish-npm-libraries.yml,publish-python-libraries.yml, andrelease-android-apk.yml.🤖 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/publish-crates-io.yml at line 50, The checkout step in the publish/release workflows is leaving GitHub credentials in the local git config because actions/checkout is using its default credential persistence. Update the checkout invocation to disable persisted credentials with persist-credentials set to false in the affected workflow files, including publish-crates-io and the other matching publish/release workflows, so later steps cannot reuse the token.Source: Linters/SAST tools
.github/workflows/test-and-build.yml (1)
122-125: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider setting
persist-credentials: falseon checkout.Static analysis (zizmor) flags that this
actions/checkoutstep doesn't setpersist-credentials: false, which leaves theGITHUB_TOKENpersisted in the local git config for the remainder of the job. Given this PR's entire purpose is supply-chain hardening, and this job later runs untrusted dependency installs (npm ci,cargo, etc.) and uploads artifacts, persisting credentials increases the blast radius if any subsequent step is compromised.🔒 Suggested fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 + persist-credentials: false🤖 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/test-and-build.yml around lines 122 - 125, The checkout step in the workflow is persisting the default GitHub credentials, which broadens risk for later untrusted build steps. Update the existing actions/checkout usage in the test-and-build workflow to set persist-credentials to false so the GITHUB_TOKEN is not left in local git config; keep the change within the checkout step identified by actions/checkout.Source: Linters/SAST tools
.github/workflows/security-scan-source.yml (1)
84-90: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCheckout step persists credentials by default.
Same
artipackedfinding flagged by zizmor here. This job runs Semgrep/Bandit/pip-audit against repo contents and doesn't need the token to persist for the whole job.🔒 Suggested fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 + persist-credentials: false🤖 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/security-scan-source.yml around lines 84 - 90, The checkout step is persisting Git credentials unnecessarily in the security-scan workflow. Update the existing actions/checkout step in security-scan-source.yml to disable credential persistence so the job can still fetch the repo for Semgrep/Bandit/pip-audit without leaving the token available for the rest of the workflow.Source: Linters/SAST tools
.github/workflows/security-scan-artifacts.yml (1)
64-79: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCheckout step still persists credentials by default.
zizmor flags this checkout step for the
artipackedissue: the defaultpersist-credentials: trueleaves theGITHUB_TOKENon disk for the remainder of the job, including untrusted steps (e.g. scanning third-party artifacts). Since this job only needs to read the repo for context, setpersist-credentials: false.🔒 Suggested fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 + persist-credentials: falseAs per the workflows' "minimal permissions principle" guideline, credential exposure should be limited to what each job actually requires.
🤖 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/security-scan-artifacts.yml around lines 64 - 79, The checkout step in this workflow is leaving credentials persisted on disk by default, which is unnecessary for this read-only context. Update the actions/checkout step in the security-scan-artifacts job to explicitly disable credential persistence by setting persist-credentials to false, keeping the rest of the artifact-check and download flow unchanged.Sources: Coding guidelines, Linters/SAST tools
.github/workflows/security-scan-dast.yml (1)
56-58: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCheckout step persists credentials by default.
Same
artipackedfinding as the other scan workflows:persist-credentials: falseshould be set since this job doesn't need to push or use the token beyond initial checkout.🔒 Suggested fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 + persist-credentials: false🤖 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/security-scan-dast.yml around lines 56 - 58, The checkout step is leaving credentials available by default in this scan workflow, which is unnecessary for a read-only job. Update the actions/checkout step in the security scan workflow to set persist-credentials to false alongside the existing fetch-depth setting so the token is not retained after checkout.Source: Linters/SAST tools
🤖 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/lighthouse-scan.yml:
- Line 56: The checkout step currently leaves the GitHub token persisted in git
config, which is unnecessary for the subsequent Lighthouse action. Update the
actions/checkout invocation in the lighthouse scan workflow to disable
credential persistence by setting persist-credentials to false, keeping the
existing checkout reference and locating it via the checkout step in the
workflow.
In @.github/workflows/lint.yml:
- Line 39: The checkout step in the lint workflow is persisting credentials
unnecessarily; update the actions/checkout usage in the workflow to disable
credential persistence. Since this job only needs the repo contents and the Gist
update uses a separate token, keep the checkout token out of git config by
setting the appropriate checkout option on the existing checkout step in
lint.yml.
In @.github/workflows/publish-firefox-extension.yml:
- Around line 47-48: The packaging step in the Firefox publish workflow is
vulnerable because `inputs.xpi_path` is expanded directly inside the shell
command in `package firefox extension`. Harden `run` by moving the input into an
environment variable for that step and referencing the variable in the `zip`
invocation instead of interpolating the template expression inline. Keep the fix
localized to the workflow job and the `package firefox extension` step so the
path value is treated as data, not shell syntax.
---
Outside diff comments:
In @.github/workflows/security-scan-artifacts.yml:
- Around line 98-104: The upload-sarif steps are still using the floating
github/codeql-action/upload-sarif@v4 tag, which breaks the SHA-only pinning
requirement in this workflow. Update every upload-sarif usage in
security-scan-artifacts.yml to reference the same pinned commit SHA pattern used
by the other actions, keeping the existing step names and with-blocks unchanged.
In @.github/workflows/security-scan-source.yml:
- Around line 136-141: The upload-sarif step in the security scan workflow is
still using a floating tag instead of a commit SHA, which violates the pinning
policy. Update the github/codeql-action/upload-sarif@v4 reference in the upload
semgrep SARIF step to the appropriate pinned SHA, matching the style already
used by the other actions in this workflow.
---
Nitpick comments:
In @.github/workflows/publish-crates-io.yml:
- Line 50: The checkout step in the publish/release workflows is leaving GitHub
credentials in the local git config because actions/checkout is using its
default credential persistence. Update the checkout invocation to disable
persisted credentials with persist-credentials set to false in the affected
workflow files, including publish-crates-io and the other matching
publish/release workflows, so later steps cannot reuse the token.
In @.github/workflows/release-android-apk.yml:
- Line 172: The release workflow currently uses the third-party
softprops/action-gh-release step for publishing the APK; consider replacing that
action usage with an equivalent gh release CLI step in the release-android-apk
workflow. Update the release job to use the built-in gh command on GitHub-hosted
runners while preserving the existing release behavior and inputs from the
current release step.
- Line 68: The Android release workflow has cache-poisoning exposure because
trusted write-capable jobs are using broad caching via actions/setup-node and
the keystore cache step. Update the release-android-apk job to either disable
default caching in setup-node or scope cache keys/restore conditions tightly
(for example by branch or trusted ref), and review the keystore cache so it only
restores for trusted refs. Use the actions/setup-node step and the
keystore-caching step in this workflow to locate the changes.
In @.github/workflows/security-scan-artifacts.yml:
- Around line 64-79: The checkout step in this workflow is leaving credentials
persisted on disk by default, which is unnecessary for this read-only context.
Update the actions/checkout step in the security-scan-artifacts job to
explicitly disable credential persistence by setting persist-credentials to
false, keeping the rest of the artifact-check and download flow unchanged.
In @.github/workflows/security-scan-dast.yml:
- Around line 56-58: The checkout step is leaving credentials available by
default in this scan workflow, which is unnecessary for a read-only job. Update
the actions/checkout step in the security scan workflow to set
persist-credentials to false alongside the existing fetch-depth setting so the
token is not retained after checkout.
In @.github/workflows/security-scan-source.yml:
- Around line 84-90: The checkout step is persisting Git credentials
unnecessarily in the security-scan workflow. Update the existing
actions/checkout step in security-scan-source.yml to disable credential
persistence so the job can still fetch the repo for Semgrep/Bandit/pip-audit
without leaving the token available for the rest of the workflow.
In @.github/workflows/test-and-build.yml:
- Around line 122-125: The checkout step in the workflow is persisting the
default GitHub credentials, which broadens risk for later untrusted build steps.
Update the existing actions/checkout usage in the test-and-build workflow to set
persist-credentials to false so the GITHUB_TOKEN is not left in local git
config; keep the change within the checkout step identified by actions/checkout.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88c3c82a-2f39-40d1-87fa-22ed13574e14
📒 Files selected for processing (16)
.github/workflows/lighthouse-scan.yml.github/workflows/lint.yml.github/workflows/npm-audit-autofix.yml.github/workflows/post-publish-verification.yml.github/workflows/publish-crates-io.yml.github/workflows/publish-docker-image.yml.github/workflows/publish-firefox-extension.yml.github/workflows/publish-npm-libraries.yml.github/workflows/publish-python-libraries.yml.github/workflows/release-android-apk.yml.github/workflows/release-github.yml.github/workflows/security-scan-artifacts.yml.github/workflows/security-scan-dast.yml.github/workflows/security-scan-source.yml.github/workflows/set-git-tag.yml.github/workflows/test-and-build.yml
… review - Pin github/codeql-action/upload-sarif@v4 (missed by the original regex scan since it has a multi-segment path) to its commit SHA in security-scan-source.yml, security-scan-artifacts.yml, and post-publish-verification.yml. - Set persist-credentials: false on actions/checkout in jobs that don't perform a raw git push, reducing the blast radius if a later step in the job is compromised. Left untouched in npm-audit-autofix.yml, release-android-apk.yml, and set-git-tag.yml, which rely on the persisted token for git commit/tag/push. - Fix a pre-existing shell injection in publish-firefox-extension.yml by passing inputs.xpi_path through an env var instead of interpolating it directly into the zip command.
1 participant
Summary
@v7,@v0,@v0.4.1, etc.) in.github/workflows/*.ymlwith full 40-character commit SHAs, eliminating the supply-chain risk of a tag being re-pointed to different code after review.actions/cache@v6→ latestv6.1.0,softwareforgood/check-artifact-v4-existence@v0.4.1→ latestv0.4.3).# vX.Y.Zcomment for readability and future maintenance (Dependabot can still track these).gh api repos/<owner>/<repo>/git/ref/tags/<tag>(dereferencing annotated tag objects to their underlying commit where needed) and verified to be exactly 40 hex characters before being written.rust-lang/crates-io-auth-action@v1has nov1tag —v1is a moving branch. Resolved it to the tip of that branch's current commit (matchingv1.0.5, its latest tagged release) and pinned to that SHA.Test plan
actionlint .github/workflows/*.ymlpasses with no errors@vX/@vX.Y.Zreferences via regex scan (only 40-char SHA refs remain).github/workflows/were touchedSummary by CodeRabbit