Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBumps several tooling/context versions, disables credential persistence on multiple GitHub checkout steps and pins one tag action, updates pre-commit hooks (adds JSON-schema check and revisions), adds a new Copier field with test fixtures, and adds a Ruff ignore rule and minor template/docs tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / Repo
participant CI as GitHub Actions
participant Checkout as actions/checkout
participant TagAction as github-tag-action
rect #f0f9ff
Note over CI,Checkout: Checkout steps now run with\npersist-credentials: false
end
Dev->>CI: Push / Merge
CI->>Checkout: actions/checkout (persist-credentials: false)
Checkout-->>CI: Repository checked out (no persisted creds)
CI->>CI: run pre-commit / linters / build
alt Tag-on-merge job
CI->>Checkout: actions/checkout (persist-credentials: false)
CI->>TagAction: github-tag-action @ a22cf086...
TagAction-->>CI: create/push tag
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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.
Pull Request Overview
This PR bumps FastAPI and Uvicorn to newer versions and implements security improvements to GitHub workflows based on zizmor recommendations.
- Updates FastAPI from 0.116.1 to 0.117.1 and Uvicorn from 0.35.0 to 0.36.0
- Adds security configurations to GitHub Actions workflows including
persist-credentials: falseand permissions restrictions - Introduces a new copyright holder field for templates and fixes documentation typos
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| extensions/context.py | Updates FastAPI, Uvicorn, and other package versions |
| template/.github/workflows/*.yaml.jinja-base | Adds security configurations to workflow templates |
| copier.yaml | Adds new copyright holder field configuration |
| tests/copier_data/*.yaml | Updates test data with copyright holder examples |
| .pre-commit-config.yaml | Updates pre-commit hooks and adds GitHub workflow validation |
| .github/workflows/*.yaml | Applies security improvements to repository workflows |
| template/README.md.jinja-base | Fixes incomplete sentence in documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| - name: Update Devcontainer Hash | ||
| if: ${{ github.actor == 'dependabot[bot]' && github.event_name == 'push' }} | ||
| if: ${{ github.event.pull_request.user.login == 'dependabot[bot]' && github.event_name == 'push' }} |
There was a problem hiding this comment.
The condition github.event.pull_request.user.login == 'dependabot[bot]' will fail because github.event.pull_request is not available in push events. This should use github.actor == 'dependabot[bot]' as in the original code, or check the event type first.
| if: ${{ github.event.pull_request.user.login == 'dependabot[bot]' && github.event_name == 'push' }} | |
| if: ${{ github.actor == 'dependabot[bot]' && github.event_name == 'push' }} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
template/.github/zizmor.yml (1)
1-5: Avoid brittle line-number-specific ignores.Prefer ignoring the file path (or a more targeted pattern) to prevent breakage when lines shift.
Apply:
- - get-values.yaml:28 + - get-values.yamlConfirm this scope is acceptable for your use case.
template/README.md.jinja-base (1)
10-11: Clarify the script source link.“this repo” can be ambiguous when rendered downstream; consider linking directly to the script in the base template.
.pre-commit-config.yaml (1)
182-186: Added GitHub workflows schema check is a solid addition.Optionally add a second hook to validate template/.github/workflows as well.
+ - repo: https://github.com/python-jsonschema/check-jsonschema + rev: 83987cd6ad8943c7f029b500b14aaf82c00a01fa # frozen: 0.34.0 + hooks: + - id: check-jsonschema + name: check-template-github-workflows + args: ["--builtin-schema", "github-workflows"] + files: ^template/.github/workflows/.*\.ya?ml$copier.yaml (1)
12-12: Microcopy: clarify help textConsider lowercase “copyright” and being explicit about the notice to reduce ambiguity.
Apply:
- help: What's the human-readable organization or username that should be set for the Copyright? + help: What's the human-readable organization or username to use in the copyright notice?.github/workflows/tag-on-merge.yaml (1)
16-21: Consider pinningactions/checkoutto a commit SHA.You pinned the tag action to a SHA; applying the same to
actions/checkout@v5.0.0would align supply‑chain hardening across steps..github/workflows/ci.yaml (2)
27-30: Pinactions/checkoutto a commit SHA for supply‑chain hardening.Same recommendation as elsewhere; pinning reduces risk from mutable tags.
85-87: Repeat: pin this checkout to a commit SHA as well.Keeps consistency and hardens the workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.devcontainer/devcontainer.json(1 hunks).devcontainer/install-ci-tooling.py(1 hunks).github/workflows/ci.yaml(3 hunks).github/workflows/tag-on-merge.yaml(1 hunks).pre-commit-config.yaml(5 hunks)copier.yaml(1 hunks)extensions/context.py(2 hunks)ruff.toml(1 hunks)template/.github/reusable_workflows/build-docker-image.yaml.jinja-base(1 hunks)template/.github/workflows/ci.yaml.jinja-base(1 hunks)template/.github/workflows/get-values.yaml.jinja-base(1 hunks)template/.github/workflows/pre-commit.yaml.jinja-base(1 hunks)template/.github/zizmor.yml(1 hunks)template/README.md.jinja-base(1 hunks)tests/copier_data/data1.yaml(1 hunks)tests/copier_data/data2.yaml(1 hunks)
🔇 Additional comments (17)
.devcontainer/devcontainer.json (1)
62-62: Devcontainer context hash bump looks good.Confirm the pre-commit hook computed this value (compute-devcontainer-context-hash) so CI prebuild triggers are accurate.
If needed, run locally: pre-commit run -a compute-devcontainer-context-hash
ruff.toml (1)
48-49: OK to ignore D105.Matches your docstring policy; aligns with existing D10x ignores.
.pre-commit-config.yaml (4)
105-129: Prettier mirror switch and expanded exclude look good.rbubley/mirrors-prettier at v3.6.2 is fine; excluding template/.* avoids unintended formatting churn.
214-218: Zizmor hook intentionally disabled.Matches PR note; ensure the tracking issue stays open until re-enabled.
Add the issue link in a comment here for traceability, if you like.
229-229: Hadolint bump to v2.14.0 approved.
237-237: Ruff pre-commit bump to v0.13.1 approved..devcontainer/install-ci-tooling.py (1)
8-8: UV version bump verified — matches in both files.
Both .devcontainer/install-ci-tooling.py and extensions/context.py set UV version to "0.8.19".tests/copier_data/data1.yaml (1)
9-9: New copier field added — defined but not wired throughcopier.yaml defines repo_org_name_for_copyright (default: "{{ repo_org_name }}"); tests set it in tests/copier_data/data1.yaml and tests/copier_data/data2.yaml (data1: "The Greatest Org", data2: "Initech Corporation"). No usages found in templates/ or README — either wire this field into the templates/README where intended or confirm it's intentionally unused, and make test fixtures consistent.
extensions/context.py (1)
14-15: Version bumps look aligned — external compatibility OK; run a local dependency scan
- FastAPI 0.117.1 + Uvicorn 0.36.0: compatible, no known breaking changes.
- fastapi-offline 1.7.4 requires fastapi>=0.99.0, so compatible with FastAPI 0.117.x.
- I could not verify repo pins (ripgrep returned "No files were searched"). Re-run a dependency scan against requirements.txt / pyproject.toml / setup.cfg and confirm the runtime Python version meets package minimums.
tests/copier_data/data2.yaml (1)
10-10: Test fixture updated correctlyValue looks good; plain scalar with spaces is valid YAML. Matches the new copier variable.
Please confirm data1.yaml was updated similarly and that template rendering tests cover both values.
copier.yaml (1)
10-14: Good addition; default derives from repo_org_name — no legacy usages foundOccurrences found: copier.yaml; tests/copier_data/data1.yaml, tests/copier_data/data2.yaml. Ripgrep reported no instances of "{{ repo_org_name }}" near "Copyright".
.github/workflows/tag-on-merge.yaml (1)
20-23: Good hardening: disabled checkout creds + pinned action by SHA. Please confirm push behavior.Nice upgrade in supply‑chain posture. Since checkout creds are disabled, ensure
mathieudutour/github-tag-actionpushes tags/commits using the providedGITHUB_TOKENinternally and does not rely on checkout’s persisted credentials.template/.github/workflows/ci.yaml.jinja-base (1)
54-55: LGTM: checkout without credential persistence.Consistent with least‑privilege principles; no issues spotted.
template/.github/reusable_workflows/build-docker-image.yaml.jinja-base (1)
69-71: LGTM: disabling persisted credentials on checkout.Matches the security posture elsewhere; no impact on ECR auth since OIDC is used later.
.github/workflows/ci.yaml (1)
160-160: Nice: explicit minimal permissions.
permissions: {}on this job is appropriate since it only gates earlier results.template/.github/workflows/pre-commit.yaml.jinja-base (1)
36-45: LGTM: disabled checkout credential persistence across both paths.Consistent hardening; pre‑commit doesn’t push, so this is safe.
template/.github/workflows/get-values.yaml.jinja-base (1)
32-33: Confirmed — action configures credentials for the push.
template/.github/actions/update-devcontainer-hash/action.yml.jinja-base calls actions/checkout with persist-credentials: true, configures git user.name/email, and runs git push; no additional auth changes required despite the workflow's persist-credentials: false.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.pre-commit-config.yaml (3)
105-131: Prettier mirror bump looks good; consider trimming target files to avoid redundant include/exclude.
- Good move to rbubley/mirrors-prettier and v3.6.2; exclude of template/.* is sensible.
- Optional: since YAML/YML/MD/JSONC are excluded, drop them from the files pattern to reduce hook overhead.
Apply this minimal tweak:
- files: (.*.json)|(.*.ts)|(.*.jsx)|(.*.tsx)|(.*.yaml)|(.*.yml)|(.*.md)|(.*.html)|(.*.css)|(.*.scss)|(.*.less)|(.*.vue)|(.*.graphql)|(.*.gql) + files: (.*.json)|(.*.ts)|(.*.jsx)|(.*.tsx)|(.*.html)|(.*.css)|(.*.scss)|(.*.less)|(.*.vue)|(.*.graphql)|(.*.gql)
182-186: Nice addition of workflow schema checks; optionally make it repository-wide per run.
If you want the workflow validation to run even when no workflow files are staged (useful after schema/tooling changes), add always_run and disable filename passing.- repo: https://github.com/python-jsonschema/check-jsonschema rev: 83987cd6ad8943c7f029b500b14aaf82c00a01fa # frozen: 0.34.0 hooks: - id: check-github-workflows + pass_filenames: false + always_run: true
214-218: zizmor is intentionally commented; can help wire it up behind a flag.
Recommend adding a minimal config and gating via stages or a dedicated repo-local toggle when you’re ready.I can prepare a starter .zizmor.yaml and suggested hook config aligned with your workflows. Want me to open a draft?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/devcontainer.json(1 hunks).pre-commit-config.yaml(5 hunks)copier.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- copier.yaml
- .devcontainer/devcontainer.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.13.1, --data-file tests/copier_data/data1.yaml)
🔇 Additional comments (2)
.pre-commit-config.yaml (2)
229-229: Fix hadolint version — v2.14.0 is not a released tagNo release v2.14.0 exists; latest stable is v2.12.0 (v2.12.1-beta pre-release). Update .pre-commit-config.yaml (line 229) to pin a valid hadolint release (e.g., v2.12.0).
rev: 4e697ba704fd23b2409b947a319c19c3ee54d24f # frozen: v2.14.0Likely an incorrect or invalid review comment.
237-263: Ruff pre-commit bump approved — verify config location and rules..pre-commit-config.yaml pins astral-sh/ruff-pre-commit rev a113f03edeabb71305f025e6e14bd2cd68660e29 (v0.13.1). No ruff.toml or ruff-test.toml found in the repo; confirm where ruff is configured (ruff.toml / ruff-test.toml or pyproject.toml) and verify configs don't use deprecated rules after the upgrade.
Why is this change necessary?
Newer versions
How does this change address the issue?
Bumps them
What side effects does this change have?
None
How is this change tested?
Downstream repos
Other
Made some modifications to github workflows based on zizmor recommendations. But disabled zizmor for now and created an issue to fully implement it later.
Created a question around copyright holder name in the template.
Fixed typo in Readme
Summary by CodeRabbit
New Features
Chores
Documentation
Tests