Skip to content

Add zizmor to pre-commit and fix findings #1812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hugovk wants to merge 3 commits into
base: main
Choose a base branch
from
+29 −9
Open

Add zizmor to pre-commit and fix findings #1812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92476cf
Add zizmor to pre-commit and fix findings
hugovk May 26, 2026
a2b3a3c
Add handy env vars
hugovk May 26, 2026
ea5ef9d
Replace pre-commit with prek to remove Node.js 20 deprecation warning
hugovk May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ updates:
actions:
patterns:
- "*"
cooldown:
# https://blog.yossarian.net/2025/11/21/We-should-all-be-using-dependency-cooldowns
# Cooldowns protect against supply chain attacks by avoiding the
# highest-risk window immediately after new releases.
Comment on lines +15 to +16
Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the false sense of security this gives us, this comment makes this seem fail-safe.

Copy link
Copy Markdown
Member Author

@hugovk hugovk May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a copy-paste from python/cpython#141866 which references https://blog.yossarian.net/2025/11/21/We-should-all-be-using-dependency-cooldowns.

Nothing is fail-safe in security, there are many things we can do to protect against these attacks, and this is a good one. They layer on top of each other.

Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I find the comment additionally misleading. I suggest either:

  • Removing it.
  • Adding a link to the Dependabot issue highlighting the flaw in their cooldown system.

Copy link
Copy Markdown
Member Author

@hugovk hugovk May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's ask @gpshead who wrote the comment and @woodruffw who wrote the blog post.

Copy link
Copy Markdown
Member

@gpshead gpshead May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency cooldowns are not a false sense of security. most supply chain attacks are found and fixed and yanked from the upstream package distribution ecosystems within a few days. I don't think we should block this PR on wanting the comment better, but we should apply any edit making it better to all places we've added this.

dependabot defaults to not having a cooldown - https://docs.github.com/en/code-security/reference/supply-chain-security/dependabot-options-reference#cooldown- - this is simply how we enable the good thing. (it'd be great if they changed their default in light of the modern world, but that's not our problem to solve)

Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d like to clarify that I’m not against cooldowns (in fact, I support the idea). However, Dependabots implementation is poor, and any threat actor can easily circumvent it (dependabot/dependabot-core#13078).

default-days: 14

- package-ecosystem: pip
directory: "/"
Expand All @@ -21,3 +26,5 @@ updates:
pip:
patterns:
- "*"
cooldown:
default-days: 14
8 changes: 5 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ jobs:
timeout-minutes: 10

steps:
- uses: actions/checkout@v6
- uses: actions/setup-python@v6
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
with:
python-version: "3"
- name: Install uv
uses: hynek/setup-cached-uv@v2
uses: hynek/setup-cached-uv@4300ec2180bc77d705e626a34e381b81a4772c51 # v2.5.0
- name: Build docs
run: make html
- name: Link check
Expand Down
12 changes: 9 additions & 3 deletions .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ on: [push, pull_request, workflow_dispatch]

permissions: {}

env:
FORCE_COLOR: 1
RUFF_OUTPUT_FORMAT: github

jobs:
lint:
runs-on: ubuntu-latest
timeout-minutes: 10

steps:
- uses: actions/checkout@v6
- uses: actions/setup-python@v6
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
with:
python-version: "3.x"
- uses: pre-commit/action@v3.0.1
- uses: j178/prek-action@cbc2f23eb5539cf20d82d1aabd0d0ecbcc56f4e3 # v2.0.2
11 changes: 8 additions & 3 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.7
rev: v0.15.14
hooks:
- id: ruff
name: Run Ruff (lint)
Expand All @@ -9,7 +9,7 @@ repos:
name: Run Ruff (format)

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v6.0.0
hooks:
- id: check-case-conflict
- id: check-merge-conflict
Expand All @@ -18,8 +18,13 @@ repos:
- id: end-of-file-fixer
- id: trailing-whitespace

- repo: https://github.com/zizmorcore/zizmor-pre-commit
rev: v1.25.2
hooks:
- id: zizmor

- repo: https://github.com/sphinx-contrib/sphinx-lint
rev: v0.9.1
rev: v1.0.2
hooks:
- id: sphinx-lint
args: [--enable=default-role]
Expand Down
Loading