Harden GH Actions supply chain: add zizmor static analysis + 7-day dependency cool-down - #3712
Conversation
…ependency updates Recent supply-chain attacks have exploited both malicious/misconfigured GitHub Actions workflows and newly-published malicious package versions slipping into CI before the ecosystem catches them. This adds two independent mitigations: - Run zizmor (static analysis for GitHub Actions workflows) both in CI, on any push/PR that touches .github/workflows/**, and as a local pre-commit hook, so workflow misconfigurations (e.g. injectable expressions, overly broad permissions, unpinned actions) are caught before merge. - Set uv's exclude-newer to a rolling "7d" window in pyproject.toml, so `uv lock` won't resolve a dependency version until it has been published for at least 7 days. This gives the community time to flag newly-introduced malicious or broken releases before this repo picks them up. Confirmed via `uv lock` that this is stored as a self-updating exclude-newer-span (P7D) in uv.lock rather than a fixed cutoff date. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDdYm3kAV35FottKdRFXo2
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
There was a problem hiding this comment.
🟡 Not ready to approve
The new exclude-newer = "7d" setting depends on relatively new uv behavior, so the repo should pin/document a minimum uv version to prevent inconsistent lock/sync behavior across environments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR strengthens the repository’s supply-chain posture by adding automated/static checks for GitHub Actions workflows and by introducing a dependency “cool-down” window for Python dependency resolution via uv.
Changes:
- Add a new GitHub Actions workflow that runs
zizmorstatic analysis on workflow changes under.github/workflows/**. - Add
zizmoras a local pre-commit hook to surface workflow security issues earlier. - Configure uv to avoid resolving dependency versions published within the last 7 days, updating
uv.lockaccordingly.
File summaries
| File | Description |
|---|---|
.github/workflows/actions-static-analysis.yml |
Adds zizmor-based static analysis for workflow changes with pinned action SHAs and restricted permissions. |
.pre-commit-config.yaml |
Adds a zizmor pre-commit hook to run workflow static analysis locally. |
pyproject.toml |
Adds exclude-newer = "7d" to delay dependency resolution to versions older than 7 days. |
uv.lock |
Regenerated lockfile reflecting the new uv options (exclude-newer-span = "P7D"), plus minor resolver churn. |
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The exclude-newer = "7d" setting added in a prior commit guards against newly-published third-party malicious/vulnerable packages by delaying resolution to versions at least 7 days old. That protection isn't needed for packages MIT ODL owns and iterates on rapidly (ol-concourse, django-aqueduct, open-edx-plugins, ol-django) — the 7-day delay only slows down consuming our own releases without adding any security benefit. Add [tool.uv.exclude-newer-package] with "0d" overrides for each in-house package so uv always resolves them to the latest available version while the global 7-day cool-down still applies to everything else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDdYm3kAV35FottKdRFXo2
|
Follow-up: added `[tool.uv.exclude-newer-package]` to exempt MIT ODL's own in-house packages (ol-concourse, django-aqueduct, the open-edx-plugins, and ol-django packages) from the new 7-day `exclude-newer` cool-down. The 7-day delay is meant to guard against newly-published third-party malicious/vulnerable packages — it shouldn't slow down consuming our own releases, which we control and iterate on rapidly. Each exempted package gets `"0d"` (always resolve to latest), while the global `exclude-newer = "7d"` still applies to everything else. Ran `uv lock` to regenerate the lockfile — it succeeded, and the only diff is the new `[options.exclude-newer-package]` metadata block recording the per-package overrides; no dependency versions changed. |
The zizmor CI check was failing on 3 high-severity unpinned-uses findings (astral-sh/setup-uv@v7, openapi-generators/openapitools-generator-action@v1 x2). Ran `zizmor --fix=all` to pin those refs to SHAs with version comments, plus incidental artipacked fixes (persist-credentials: false) and a ref-version-mismatch correction the same pass caught. Also: - Add required-version = ">=0.9.17" to [tool.uv] in pyproject.toml. That's the uv release that introduced relative-duration exclude-newer support (used by this table's exclude-newer = "7d"); older uv now fails loudly with a version-mismatch error instead of silently mishandling the setting. `uv lock` re-run confirms this is a no-op for the lockfile. - Add args to the pre-commit zizmor hook to match the CI workflow's own --min-severity=high --min-confidence=medium gate, so pre-commit.ci and CI agree on what blocks a PR instead of pre-commit.ci enforcing a stricter, unfiltered threshold. --no-progress is kept since args: overrides all default args. Remaining lower-severity zizmor findings (10 medium excessive-permissions, below the CI gate) and an unrelated uv.lock jinxed/ansicon marker regression are tracked in #3715 rather than bundled here.
ChristopherChudzicki
left a comment
There was a problem hiding this comment.
Still approving, but maybe remove the pull_request trigger?
Both push and pull_request fired on every PR commit for the same path-scoped check, running zizmor twice per push. push alone still covers PR branch commits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Dropped the |
…h to main Bot reviewers (Copilot, Sentry) correctly flagged that a push-only trigger misses fork-based PRs and can't act as a required merge-gate status check. Scoping push to the default branch avoids the original double-run problem (push firing on every commit to a same-repo PR branch, redundant with pull_request) while restoring PR-gate coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…endencies The allowlist exempting in-house MIT ODL packages from the 7-day uv dependency cool-down was copy-pasted org-wide, unpruned. Reviewers on two separate PRs independently flagged the same thing: most of the ~44 entries (mostly Open edX plugins) aren't dependencies of this repo at all. Trimmed to the intersection with this repo's own dependency closure (uv.lock's locked package set, or pyproject.toml's declared deps where no lockfile exists). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…age allowlist The previous commit edited pyproject.toml's allowlist without regenerating uv.lock, so 'uv sync --locked' correctly rejected the mismatch in CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ings - ci.yml: add workflow-level `permissions: contents: read`. All six jobs (python-tests, javascript-tests, build-nextjs-container, build-storybook, openapi-generated-client-check-v0/v1) only checkout, build, or test — none push, publish, or comment — so a single read-only default resolves both the workflow-level and per-job excessive-permissions findings. - openapi-diff.yml: add job-level `permissions: contents: read, pull-requests: write` to the openapi-diff job. It posts/updates a PR comment via peter-evans/find-comment and peter-evans/create-or-update-comment using secrets.GITHUB_TOKEN, which requires pull-requests: write. - publish-pages.yml: add workflow-level `permissions: contents: read`. The build job only checks out and builds Storybook, so it picks up the read-only default; the deploy job already declares its own narrower permissions (pages: write, id-token: write) which continue to override. - actions-static-analysis.yml: lower zizmor gate from min-severity=high to min-severity=medium (min-confidence unchanged at medium). - .pre-commit-config.yaml: lower the local zizmor hook's --min-severity from high to medium to match CI. Verified with `zizmor --min-severity=medium --min-confidence=medium .github/workflows/`: no findings to report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What are the relevant tickets?
N/A
Description (What does it do?)
Hardens GitHub Actions supply-chain security and reduces exposure to newly-introduced malicious/vulnerable dependency releases via three changes:
Add a zizmor GitHub Actions static-analysis workflow (
.github/workflows/actions-static-analysis.yml). It runs on push/PR whenever files under.github/workflows/**change, checking out the repo withpersist-credentials: falseand running zizmorcore/zizmor-action (pinned to a specific commit SHA, tagged v0.6.2) withmin-severity: high/min-confidence: medium. This is the same pattern already in use in the sibling repo mitodl/mitxonline. zizmor detects common GitHub Actions security issues such as unpinned third-party actions, overly broadpermissions:, and injectable template expressions inrun:steps.Add zizmor as a local pre-commit hook (
.pre-commit-config.yaml), usingzizmorcore/zizmor-pre-commitatv1.29.0, placed alongside the other lint-style hooks (next toshellcheck). This surfaces the same class of workflow issues at commit time instead of only in CI.Delay dependency version resolution by 7 days (
pyproject.toml). Addedexclude-newer = "7d"to the existing[tool.uv]table. uv 0.11+ treats a relative duration string like"7d"as a rolling window rather than a fixed date — it's stored inuv.lockas a self-updatingexclude-newer-span = "P7D"(confirmed by inspecting the regenerated lockfile), not a one-time cutoff. This meansuv lockwill never resolve a package version that was published less than 7 days ago, giving the community time to catch and yank newly-published malicious or broken releases before this repo picks them up.Screenshots (if appropriate):
N/A — no UI changes.
How can this be tested?
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/actions-static-analysis.yml'))"parses without error, as does.pre-commit-config.yaml.pre-commit run zizmor --files .github/workflows/actions-static-analysis.yml— passed cleanly (network-enabled sandbox, so the hook environment was actually fetched and executed, not just statically reviewed).uv lockin the repo root after addingexclude-newer = "7d". It re-resolved (Resolving despite existing lockfile due to addition of global exclude newer ...) and succeeded —Resolved 385 packages. The resultinguv.locknow contains:exclude-newer/exclude-newer-spanlines was a platform-marker change on theansiconpackage (sys_platform == 'win32'marker dropped), which is unrelated resolver churn, not a version bump..github/workflows/**changes as scoped by thepaths:filter.Additional Context
The zizmor workflow and pre-commit hook pin their action/hook versions by SHA/tag (
actions/checkout@3d3c42e...= v7.0.1,zizmorcore/zizmor-action@3dc1ecc...= v0.6.2,zizmorcore/zizmor-pre-commit= v1.29.0) rather than a floating major-version tag, consistent with supply-chain hardening best practice for GitHub Actions.