Skip to content
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.

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

Update pre-commit #249

Merged
merged 2 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 3 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ name: Lint

on: [push, pull_request, workflow_dispatch]

permissions: {}

Comment on lines +5 to +6
Copy link
Member

Choose a reason for hiding this comment

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

Is this added just to explicitly state that no extra permissions are granted (I would assume that's already the default)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this added just to explicitly state that no extra permissions are granted

Yes.

(I would assume that's already the default)?

No, see below.


Furthermore, users often don't realize that the default GITHUB_TOKEN permissions can be very broad, meaning that workflows that don't configure any permissions at all can still provide excessive credentials to their individual jobs.

...

In practice, this means that workflows should almost always set permissions: {} at the workflow level to disable all permissions by default, and then set specific job-level permissions as needed.

https://woodruffw.github.io/zizmor/audits/#excessive-permissions

The default access (permissive) is read/write for most of the scopes:

https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation, this is good to know!

So the default permissions are too permissive, the contents: read that was explicitly set before was still not needed, and the permissions: {} we have now disables all permissions.

env:
FORCE_COLOR: 1
PIP_DISABLE_PIP_VERSION_CHECK: 1

permissions:
contents: read

jobs:
lint:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
- uses: actions/setup-python@v5
with:
python-version: "3.x"
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ name: Test

on: [push, pull_request, workflow_dispatch]

permissions:
contents: read
permissions: {}

env:
FORCE_COLOR: 1
Expand All @@ -19,6 +18,8 @@ jobs:

steps:
- uses: actions/checkout@v4
with:
persist-credentials: false

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
Expand Down
17 changes: 11 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,42 @@ repos:
- id: trailing-whitespace

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.7.1
rev: v0.9.6
hooks:
- id: ruff-format

- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.29.4
rev: 0.31.1
hooks:
- id: check-github-workflows

- repo: https://github.com/rhysd/actionlint
rev: v1.7.3
rev: v1.7.7
hooks:
- id: actionlint

- repo: https://github.com/woodruffw/zizmor-pre-commit
rev: v1.3.1
hooks:
- id: zizmor

- repo: https://github.com/tox-dev/pyproject-fmt
rev: v2.5.0
hooks:
- id: pyproject-fmt

- repo: https://github.com/abravalheri/validate-pyproject
rev: v0.22
rev: v0.23
hooks:
- id: validate-pyproject

- repo: https://github.com/tox-dev/tox-ini-fmt
rev: 1.4.1
rev: 1.5.0
hooks:
- id: tox-ini-fmt

- repo: https://github.com/rbubley/mirrors-prettier
rev: v3.3.3
rev: v3.5.1
hooks:
- id: prettier
files: templates/switchers.js
Expand Down
5 changes: 2 additions & 3 deletions build_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def __init__(self, name, *, status, branch_or_tag=None):
if status not in self.STATUSES:
raise ValueError(
"Version status expected to be one of: "
f"{', '.join(self.STATUSES|set(self.SYNONYMS.keys()))}, got {status!r}."
f"{', '.join(self.STATUSES | set(self.SYNONYMS.keys()))}, got {status!r}."
)
self.name = name
self.branch_or_tag = branch_or_tag
Expand Down Expand Up @@ -732,8 +732,7 @@ def build(self):
shell=True,
)
subprocess.check_output(
"sed -i s/\N{REPLACEMENT CHARACTER}/?/g "
f"{self.checkout}/Doc/**/*.rst",
f"sed -i s/\N{REPLACEMENT CHARACTER}/?/g {self.checkout}/Doc/**/*.rst",
shell=True,
)

Expand Down
4 changes: 2 additions & 2 deletions check_times.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def calc_time(lines: list[str]) -> None:
fmt_duration = format_seconds(state_data["last_build_duration"])
reason = state_data["triggered_by"]
print(
f"{start:%Y-%m-%d %H:%M UTC} | {version: <7} | {language: <8} | {fmt_duration :<14} | {reason}"
f"{start:%Y-%m-%d %H:%M UTC} | {version: <7} | {language: <8} | {fmt_duration:<14} | {reason}"
)

if line.endswith("Build start."):
Expand All @@ -64,7 +64,7 @@ def calc_time(lines: list[str]) -> None:
timestamp = f"{line[:16]} UTC"
_, fmt_duration = line.removesuffix(").").split("(")
print(
f"{timestamp: <20} | --FULL- | -BUILD-- | {fmt_duration :<14} | -----------"
f"{timestamp: <20} | --FULL- | -BUILD-- | {fmt_duration:<14} | -----------"
)

if in_progress:
Expand Down