Update github workflows & precommit to use pinned hashes instead of tags#170
Conversation
WalkthroughThis PR pins GitHub Actions and pre-commit hook revisions to specific commit SHAs, and updates the Sphinx docs configuration to add project-root import path setup before importing ChangesCI and pre-commit pinning
Docs build configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/check.yml (1)
22-22: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider disabling credential persistence on checkout.
zizmor flags this step for not setting
persist-credentials: false. Since none of the later steps in this job need to push/commit, the lingering GitHub token in git config is unnecessary attack surface if any subsequent step is compromised.🔒 Suggested hardening
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + 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/check.yml at line 22, The checkout step is leaving GitHub credentials persisted in git config, which is unnecessary for this job. Update the actions/checkout usage in the workflow to disable credential persistence by setting persist-credentials to false, and keep the change scoped to the checkout step since later steps do not need to push or commit.Source: Linters/SAST tools
.github/workflows/python-publish.yml (1)
26-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider disabling credential persistence on checkout.
Same zizmor
artipackedfinding as the other workflows: nopersist-credentials: falseon the checkout step. Worth hardening here too, given this job also handles a publishing secret.🔒 Suggested hardening
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + 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/python-publish.yml around lines 26 - 28, The checkout step in the Python publish workflow is missing the credential-hardening setting, which leaves git credentials persisted unnecessarily. Update the existing actions/checkout step in this workflow to explicitly disable credential persistence by setting persist-credentials to false, keeping the rest of the publishing job intact.Source: Linters/SAST tools
.github/workflows/unit_tests.yml (1)
37-41: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider disabling credential persistence on checkout.
Same
artipackedfinding as the other workflows.🔒 Suggested hardening
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + 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/unit_tests.yml around lines 37 - 41, The checkout step in this workflow should disable persisted credentials to match the other hardened workflows and address the same artipacked finding. Update the existing actions/checkout usage in the unit_tests workflow to explicitly turn off credential persistence, keeping the rest of the job setup unchanged; the relevant symbol to locate is the checkout step before setup-uv.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.
Nitpick comments:
In @.github/workflows/check.yml:
- Line 22: The checkout step is leaving GitHub credentials persisted in git
config, which is unnecessary for this job. Update the actions/checkout usage in
the workflow to disable credential persistence by setting persist-credentials to
false, and keep the change scoped to the checkout step since later steps do not
need to push or commit.
In @.github/workflows/python-publish.yml:
- Around line 26-28: The checkout step in the Python publish workflow is missing
the credential-hardening setting, which leaves git credentials persisted
unnecessarily. Update the existing actions/checkout step in this workflow to
explicitly disable credential persistence by setting persist-credentials to
false, keeping the rest of the publishing job intact.
In @.github/workflows/unit_tests.yml:
- Around line 37-41: The checkout step in this workflow should disable persisted
credentials to match the other hardened workflows and address the same
artipacked finding. Update the existing actions/checkout usage in the unit_tests
workflow to explicitly turn off credential persistence, keeping the rest of the
job setup unchanged; the relevant symbol to locate is the checkout step before
setup-uv.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0875486d-2b2e-4f4d-a605-db6609f75753
📒 Files selected for processing (4)
.github/workflows/check.yml.github/workflows/python-publish.yml.github/workflows/unit_tests.yml.pre-commit-config.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/python-publish.yml (2)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing version comment on pinned SHA.
Unlike the other pinned actions in this file (checkout, setup-python), the
pypa/gh-action-pypi-publishpin has no trailing# vX.Y.Zcomment, making it harder for humans/Dependabot to track which release this corresponds to.diff
- uses: pypa/gh-action-pypi-publish@cef221092ed1bacb1cc03d23a2d87d1d172e277b + uses: pypa/gh-action-pypi-publish@cef221092ed1bacb1cc03d23a2d87d1d172e277b # v1.13.0🤖 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/python-publish.yml at line 39, The pinned pypa/gh-action-pypi-publish SHA is missing a trailing version comment, unlike the other action pins in this workflow. Update the uses entry for pypa/gh-action-pypi-publish to include its matching release tag comment (for example, a vX.Y.Z note) so the pin is easier to identify and maintain alongside the existing checkout and setup-python references.
39-42: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftConsider migrating to PyPI trusted publishing.
Static analysis (zizmor) flags this step; using a long-lived
PYPI_API_TOKENsecret instead of OIDC-based trusted publishing is a weaker security posture. This predates this PR's pinning changes and can be addressed separately.🤖 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/python-publish.yml around lines 39 - 42, The PyPI publish step still uses the long-lived PYPI_API_TOKEN secret via pypa/gh-action-pypi-publish, which should be migrated to trusted publishing. Update the publish job in python-publish.yml to use OIDC-based PyPI trusted publishing instead of passing secrets.PYPI_API_TOKEN, keeping the existing pinned action reference unless the publish configuration requires a compatible version change.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 `@docs/conf.py`:
- Line 19: The standalone import in docs/conf.py is being flagged by Ruff E402
because it comes after sys.path manipulation; update the import of undate to
either include a local E402 exemption or add docs/conf.py to the per-file
ignores so the configuration passes lint. Use the undate import as the anchor
point when making the change.
- Around line 14-19: The Sphinx import setup in conf.py is using
Path().resolve(), which depends on the current working directory and can point
at docs/ instead of the project root. Update the path bootstrap near
sys.path.insert and import undate to anchor it off the conf.py location (using
the file’s parent chain) so the repository root is added consistently regardless
of where Sphinx is run from.
---
Nitpick comments:
In @.github/workflows/python-publish.yml:
- Line 39: The pinned pypa/gh-action-pypi-publish SHA is missing a trailing
version comment, unlike the other action pins in this workflow. Update the uses
entry for pypa/gh-action-pypi-publish to include its matching release tag
comment (for example, a vX.Y.Z note) so the pin is easier to identify and
maintain alongside the existing checkout and setup-python references.
- Around line 39-42: The PyPI publish step still uses the long-lived
PYPI_API_TOKEN secret via pypa/gh-action-pypi-publish, which should be migrated
to trusted publishing. Update the publish job in python-publish.yml to use
OIDC-based PyPI trusted publishing instead of passing secrets.PYPI_API_TOKEN,
keeping the existing pinned action reference unless the publish configuration
requires a compatible version change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: add7bcdf-0fc4-434a-9dae-6eaaab8f55fb
📒 Files selected for processing (5)
.github/workflows/check.yml.github/workflows/python-publish.yml.github/workflows/unit_tests.yml.pre-commit-config.yamldocs/conf.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/unit_tests.yml
- .github/workflows/check.yml
- .pre-commit-config.yaml
| import sys | ||
| from pathlib import Path | ||
|
|
||
| sys.path.insert(0, str(Path().resolve())) | ||
|
|
||
| import undate |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how docs are built (invocation directory) and whether undate is installed as a dependency for docs build
fd -e cfg -e yml -e yaml -e toml -e mk Makefile . 2>/dev/null
rg -n "sphinx-build|make html|docs" .readthedocs.yaml .readthedocs.yml Makefile docs/Makefile pyproject.toml 2>/dev/nullRepository: dh-tech/undate-python
Length of output: 760
Anchor the Sphinx source path to docs/conf.py
Path().resolve() depends on the current working directory, so this can add docs/ instead of the project root when Sphinx runs from that directory. That makes import undate fail unless the package is already installed.
🛠️ Suggested fix
-sys.path.insert(0, str(Path().resolve()))
+sys.path.insert(0, str(Path(__file__).parent.parent.resolve()))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import sys | |
| from pathlib import Path | |
| sys.path.insert(0, str(Path().resolve())) | |
| import undate | |
| import sys | |
| from pathlib import Path | |
| sys.path.insert(0, str(Path(__file__).parent.parent.resolve())) | |
| import undate |
🤖 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 `@docs/conf.py` around lines 14 - 19, The Sphinx import setup in conf.py is
using Path().resolve(), which depends on the current working directory and can
point at docs/ instead of the project root. Update the path bootstrap near
sys.path.insert and import undate to anchor it off the conf.py location (using
the file’s parent chain) so the repository root is added consistently regardless
of where Sphinx is run from.
|
|
||
| sys.path.insert(0, str(Path().resolve())) | ||
|
|
||
| import undate |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ruff config for docs/conf.py exclusions or E402 handling
rg -n "E402|docs/conf.py|exclude" pyproject.toml .ruff.toml ruff.toml 2>/dev/nullRepository: dh-tech/undate-python
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files | sed -n '1,200p'
printf '\n== lint config search ==\n'
rg -n --hidden --glob '!**/.git/**' '(^|[^A-Za-z0-9_])E402([^A-Za-z0-9_]|$)|docs/conf\.py|exclude|extend-exclude|per-file-ignores|ignore\s*=' pyproject.toml .ruff.toml ruff.toml setup.cfg tox.ini noxfile.py .pre-commit-config.yaml .github/workflows 2>/dev/null || true
printf '\n== docs/conf.py ==\n'
if [ -f docs/conf.py ]; then
cat -n docs/conf.py | sed -n '1,120p'
fi
printf '\n== ruff/flake8 related files ==\n'
fd -a 'pyproject.toml|.ruff.toml|ruff.toml|setup.cfg|tox.ini|noxfile.py|pre-commit-config.yaml' . 2>/dev/null || trueRepository: dh-tech/undate-python
Length of output: 7471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/conf.py path status ==\n'
if [ -f docs/conf.py ]; then
wc -l docs/conf.py
sed -n '1,80p' docs/conf.py | cat -n
else
echo "docs/conf.py not found"
fi
printf '\n== search for lint config and CI lint invocations ==\n'
rg -n --hidden --glob '!**/.git/**' \
'ruff|flake8|pyproject\.toml|\.ruff\.toml|ruff\.toml|E402|per-file-ignores|extend-exclude|exclude|docs/conf\.py' \
.github pyproject.toml .ruff.toml ruff.toml setup.cfg tox.ini noxfile.py pre-commit-config.yaml 2>/dev/null || trueRepository: dh-tech/undate-python
Length of output: 4224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/conf.py ==\n'
if [ -f docs/conf.py ]; then
wc -l docs/conf.py
sed -n '1,120p' docs/conf.py | cat -n
else
echo "docs/conf.py not found"
fi
printf '\n== config and CI search ==\n'
rg -n --hidden --glob '!**/.git/**' \
'ruff|flake8|E402|per-file-ignores|extend-exclude|exclude|docs/conf\.py' \
.github pyproject.toml .ruff.toml ruff.toml setup.cfg tox.ini noxfile.py .pre-commit-config.yaml 2>/dev/null || trueRepository: dh-tech/undate-python
Length of output: 4675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== candidate config files ==\n'
git ls-files | rg '(^|/)(pyproject\.toml|\.ruff\.toml|ruff\.toml|setup\.cfg|tox\.ini|noxfile\.py|\.pre-commit-config\.yaml|\.github/workflows/.*\.(yml|yaml))$' || true
printf '\n== docs/conf.py excerpt ==\n'
if [ -f docs/conf.py ]; then
sed -n '1,80p' docs/conf.py | cat -n
fiRepository: dh-tech/undate-python
Length of output: 3330
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== pyproject.toml ruff section ==\n'
sed -n '100,170p' pyproject.toml | cat -n
printf '\n== workflow lint steps ==\n'
sed -n '1,220p' .github/workflows/check.yml | cat -n
printf '\n== pre-commit ruff entries ==\n'
sed -n '1,120p' .pre-commit-config.yaml | cat -nRepository: dh-tech/undate-python
Length of output: 5062
Add an E402 exemption for this import. ruff check runs on docs/conf.py with no file-specific ignore, so import undate after sys.path mutation will be flagged unless this line gets # noqa: E402 or the file is added to per-file ignores.
🤖 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 `@docs/conf.py` at line 19, The standalone import in docs/conf.py is being
flagged by Ruff E402 because it comes after sys.path manipulation; update the
import of undate to either include a local E402 exemption or add docs/conf.py to
the per-file ignores so the configuration passes lint. Use the undate import as
the anchor point when making the change.
Summary by CodeRabbit