🔒(ci) harden every workflow install and pin every action to a hash - #1607
🔒(ci) harden every workflow install and pin every action to a hash#1607davd-gzl wants to merge 6 commits into
Conversation
A package could run arbitrary code on the runner while installing, and the versions were resolved rather than taken from the pins. The two pip calls now take wheels only, and uv run no longer resolves an environment of its own.
gitlint-core keeps the linter at 0.19.1, which the wheels-only pin was resolving down to 0.18.0 through an sdist-only sh. --ignore-scripts stops three npm ci, one yarn install and one npm install -g running the scripts of what they fetch, and yarn is pinned to 1.22.22. curl holds the dockerize download and its redirects to https.
A tag and a branch both move, so actions/checkout@v6 and numerique-gouv/action-trivy-cache@main ran whatever the owner had last pushed. Each of the 65 uses now names a 40-character commit, with the version it resolved to in a trailing comment. dependabot.yml keeps the hash and that comment moving together.
The repository is already called meet, so the file name said nothing about what the workflow holds. The print-statement check now excludes the whole workflows directory rather than one file by name: its own grep carries the literal print(, so the rename would otherwise match it on the deleted lines and fail the job.
--no-sync already stops uv run resolving an environment of its own, and that is what the version findings were about. Building is a separate guarantee that nothing on the line carried, so --no-build now says it outright. It is inert beside --no-sync, and the three lint jobs are unchanged.
|
PR Summary by QodoHarden GitHub Actions installs and pin all workflow actions to commit SHAs
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. SDK install skips SWC setup
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
lebaudantoine
left a comment
lebaudantoine
left a comment
There was a problem hiding this comment.
I would probably squash your last commit.
Thank you for your contribution
| - package-ecosystem: "github-actions" | ||
| directory: "/" | ||
| schedule: | ||
| interval: "weekly" |
There was a problem hiding this comment.
I'll push for frequent updates when security issues arise, and monthly upgrades on a regular basis. Regarding the repo configuration, we already allowlist specific actions that are permitted to run. Updating every action hash every week would be a pain, without actually making the team more secure or more productive.
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v6 | ||
| uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 |
There was a problem hiding this comment.
We already allowlist specific action hashes in the repository settings, so @main actions cannot run regardless of what the owner pushes. However, making this explicit and versioned directly in the code remains a solid best practice.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking opportunity to narrow the print-check exclusion so other workflow files remain covered. The action pins and installation restrictions have no established current failure path, while the broadened workflow exclusion only weakens future print-statement enforcement. Files Needing Attention: .github/workflows/ci.yml
|
| Filename | Overview |
|---|---|
| .github/workflows/ci.yml | Hardens dependency installation and action references; the print-check exclusion is broader than required and creates a non-blocking enforcement gap. |
| .github/workflows/crowdin-download.yml | Pins checkout and Crowdin actions to immutable commits without changing their configuration. |
| .github/workflows/docker-hub.yml | Pins Docker, Trivy, and deployment actions to the commits corresponding to their previous references. |
| .github/workflows/release-helm-chart.yaml | Pins checkout, Helm setup, and chart publication actions without changing release inputs. |
Reviews (1): Last reviewed commit: "🔥(ci) leave the action pins to Renovate" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if: always() | ||
| run: | | ||
| ! git diff origin/${{ github.event.pull_request.base.ref }}..HEAD -- . ':(exclude)**/meet.yml' | grep "print(" | ||
| ! git diff origin/${{ github.event.pull_request.base.ref }}..HEAD -- . ':(exclude).github/workflows/**' | grep "print(" |
There was a problem hiding this comment.
The directory-wide pathspec removes every workflow from print-statement enforcement even though only ci.yml contains the self-referential pattern. Restricting the exclusion to this file preserves coverage for changes to the other workflows.
| ! git diff origin/${{ github.event.pull_request.base.ref }}..HEAD -- . ':(exclude).github/workflows/**' | grep "print(" | |
| ! git diff origin/${{ github.event.pull_request.base.ref }}..HEAD -- . ':(exclude).github/workflows/ci.yml' | grep "print(" |
|
Your point about the weekly noise stands either way. What tells you a dependency needs update right now? I can shape this pull request around that if possible |
2 participants
Problem
Every install step in these workflows fetches packages and runs code from them before the job's own commands start, with
GITHUB_TOKENin the environment.Every
uses:names a tag or a branch, and either one its owner can repoint at different code without the reference changing.SonarCloud counts 60 security findings across the four files.
Design
Installs
pip installrefuses source distributions, so nothing runs asetup.pyon the runner.uv runstops re-resolving what theuv sync --lockedabove it already pinned.npm ciandyarn installskip dependency lifecycle scripts. The dockerize download stays on https across redirects.Pins
All 65
uses:name a 40-character commit, each one the commit its old ref already pointed at, with the version in a trailing comment. Nothing about what runs changes.Renovate is configured to write and update this form, though its GitHub Actions branch has been sitting on the dashboard rather than opening. Either way the refs are as current as they were: a stale tag is not fresher than a stale hash, it just changes underneath you.
The rename
meet.ymlsays nothing the repository name does not, so it becomesci.yml.What this leaves
Five of the 60 findings, all MAJOR. The gate reads the worst finding rather than the count, so it stays red.
Three are
uv sync --locked, which has nowhere to put--no-buildwhilebrevo-pythonships as source only. Two arepip install .[dev], which wants a lock file forsrc/summarythat is a maintainer's call rather than this branch's.Glossary
wheel: a package that is a zip and gets unpacked.
source distribution: a package that is a tarball whose
setup.pythe installer runs, on the runner, with the job's environment around it.lifecycle script:
preinstall,install,postinstallorprepare, which npm and yarn run from every package in the resolved tree, not only the onespackage.jsonnames.pin: a
uses:naming a commit rather than a tag or a branch, so the owner cannot change what runs.