Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors many GitHub Actions: disables persisted checkout credentials, narrows token permissions, removes in-workflow commit/push steps in multiple workflows, upgrades several action versions, and consolidates documentation build/deploy into a reusable docs-deploy workflow plus a generated-docs-sync workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant Repo as Repository
participant Sync as Generated-Docs-Sync
participant Tox as Tox Envs
participant Deploy as Docs-Deploy
participant CF as Cloudflare Pages
Repo->>Sync: push to main triggers generated-docs-sync
Sync->>Tox: run tox envs (readme, cli-docs, schema-docs, llms-txt)
Tox->>Sync: produce generated artifacts
Sync->>Repo: commit & push generated docs (uses GH_TOKEN auth)
Sync->>Deploy: call reusable docs-deploy (checkout_ref = final_sha, deploy_branch=dev)
Deploy->>Repo: checkout final_sha
Deploy->>Deploy: build site (zensical build --clean)
Deploy->>CF: optional deploy to Cloudflare Pages (when deploy_branch set)
CF-->>Deploy: deployment result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3095 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 87 87
Lines 18306 18306
Branches 2091 2091
=========================================
Hits 18306 18306
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/changelog.yaml (1)
18-71:⚠️ Potential issue | 🟠 MajorAvoid inline release-tag interpolation while the PAT is available.
Line 70 injects
github.event.release.tag_namedirectly into shell. A crafted tag containing shell syntax can execute in the PAT-bearing step. Pass the tag throughenv, quote it, fail fast, and skip the credentialed push when there is no commit.🛡️ Proposed hardening
- name: Get release info and update CHANGELOG env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_TAG: ${{ github.event.release.tag_name }} run: | set -euo pipefail - TAG="${{ github.event.release.tag_name }}" + TAG="$RELEASE_TAG" DATE=$(gh release view "$TAG" --json publishedAt --jq '.publishedAt | split("T")[0]') BODY=$(gh release view "$TAG" --json body --jq '.body // ""') @@ - name: Commit and push env: PUSH_TOKEN: ${{ secrets.PAT }} + RELEASE_TAG: ${{ github.event.release.tag_name }} run: | + set -euo pipefail + : "${PUSH_TOKEN:?PUSH_TOKEN is required}" + git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add CHANGELOG.md - git diff --cached --quiet || git commit -m "docs: update CHANGELOG.md for ${{ github.event.release.tag_name }}" + if git diff --cached --quiet; then + exit 0 + fi + git commit -m "docs: update CHANGELOG.md for ${RELEASE_TAG}" git push "https://x-access-token:${PUSH_TOKEN}@github.com/${{ github.repository }}.git" HEAD:main🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/changelog.yaml around lines 18 - 71, The workflow currently interpolates github.event.release.tag_name directly into shell commands, which risks shell injection when the PAT is available; move the tag into the step env (set TAG: ${{ github.event.release.tag_name }} on the "Get release info and update CHANGELOG" and "Commit and push" steps), always reference it as "$TAG" (quoting expansions like in the commit message and any URLs) to avoid word-splitting, and harden the push step by making the push conditional: after git add and git diff --cached --quiet, if there are no staged changes exit successfully (skip the credentialed git push) rather than interpolating the tag into the push command; keep set -euo pipefail to fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/autofix-apply.yaml:
- Around line 28-31: The workflow extracts the autofix.patch into the repo
workspace before actions/checkout@v4 which by default runs git clean and deletes
that patch; update the job so the patch is saved to the runner temp directory
(use $RUNNER_TEMP) or set the checkout step (actions/checkout@v4) to clean:
false so the file survives; locate the step that uploads/downloads the artifact
and change its destination to $RUNNER_TEMP or modify the checkout step
configuration to include clean: false (references: GH_TOKEN, RUN_ID,
WORKFLOW_NAME, and the actions/checkout@v4 step where the patch is applied).
- Around line 56-69: The artifact metadata (data dict and keys
data["head_repo"], data["head_ref"], data["head_sha"], data["commit_message"])
must be treated as untrusted: verify that data["workflow"] matches
os.environ["WORKFLOW_NAME"] and additionally validate
head_repo/head_ref/head_sha against the authoritative workflow_run/PR details
(fetch the workflow_run or PR API and compare) before using them to drive
checkout/push; do not inject raw commit_message or refs into shell source—export
validated values into the environment and write only safe, validated values
(e.g., sanitized head_repo, head_ref, head_sha, PR_NUMBER) to GITHUB_OUTPUT, and
deliver multi-line commit_message via a guarded mechanism (e.g., set as an env
var or store in a tempfile) rather than writing unescaped content directly into
the shell-interpolated output.
In @.github/workflows/cli-docs.yaml:
- Around line 105-123: The script currently interpolates
github.event.pull_request.head.ref and
github.event.pull_request.head.repo.full_name directly into the run: body (used
to set TARGET_REPO and TARGET_REF), which allows shell injection; move these
pull-request-derived values into the job env (e.g., PR_HEAD_REF and
PR_HEAD_REPO) alongside PUSH_TOKEN, then in the run block assign
TARGET_REPO="$PR_HEAD_REPO" and TARGET_REF="$PR_HEAD_REF" (use double quotes
around the variable expansions everywhere, including the git push URL and ref)
so the untrusted values are treated as data rather than executed code and the
git push still uses the existing PUSH_TOKEN.
In @.github/workflows/lint.yaml:
- Around line 78-96: Move all interpolated GitHub context values into the
workflow env block and reference them as quoted shell variables to prevent
branch-name injection: define env entries for TARGET_REPO and TARGET_REF (using
github.repository, github.ref_name,
github.event.pull_request.head.repo.full_name and
github.event.pull_request.head.ref as appropriate) alongside PUSH_TOKEN, then
update the shell script to use the quoted variables (e.g., "$TARGET_REPO" and
"$TARGET_REF") when constructing the git push URL and ref; ensure no direct
${...} GitHub context is expanded inside the run script and always quote the
variables to avoid shell metacharacter injection.
In @.github/workflows/llms-txt.yaml:
- Around line 95-113: Move all GitHub context interpolations out of the inline
run script and into the job env so secrets are not exposed in the script scope;
specifically, add env entries for the values currently interpolated in the run
block (github.event_name, github.ref_name,
github.event.pull_request.head.repo.full_name,
github.event.pull_request.head.ref) and then update the run logic that sets
TARGET_REPO and TARGET_REF (the variables used before the git push command) to
reference those env variables instead of using "${{ ... }}" inline; keep using
PUSH_TOKEN from env for the push URL but ensure the git push uses the env-based
TARGET_REPO and TARGET_REF variables.
In @.github/workflows/readme.yaml:
- Around line 91-109: The inline shell uses GitHub contexts directly to set
TARGET_REPO and TARGET_REF which allows shell injection; move the context values
into workflow env variables (e.g., add env entries for PUSH_TOKEN,
PR_TARGET_REPO and PR_TARGET_REF or similar) and then reference those env vars
inside the run script to assign TARGET_REPO and TARGET_REF (replace uses of
github.event.pull_request.head.repo.full_name and
github.event.pull_request.head.ref with the new env vars). Ensure all context
expansions used in the run block are only read from the environment (not
interpolated into the script), and use the existing PUSH_TOKEN env var for the
git push URL.
In @.github/workflows/schema-docs.yaml:
- Around line 95-113: The workflow currently expands github context values
directly inside the shell run block (using github.event.pull_request.head.ref /
.head.repo.full_name) which can lead to shell injection; instead, add
environment variables (e.g., TARGET_REPO and TARGET_REF) at the step level or
export them at the start of the run by assigning the github context to env vars
(using the workflow env mapping) and then reference those shell-safe variables
(TARGET_REPO, TARGET_REF) in the script; update the step so PUSH_TOKEN,
TARGET_REPO and TARGET_REF are provided via env and replace any direct
github.context expansions in the run script with the corresponding shell
variables (TARGET_REPO, TARGET_REF) when building the git push URL.
---
Outside diff comments:
In @.github/workflows/changelog.yaml:
- Around line 18-71: The workflow currently interpolates
github.event.release.tag_name directly into shell commands, which risks shell
injection when the PAT is available; move the tag into the step env (set TAG:
${{ github.event.release.tag_name }} on the "Get release info and update
CHANGELOG" and "Commit and push" steps), always reference it as "$TAG" (quoting
expansions like in the commit message and any URLs) to avoid word-splitting, and
harden the push step by making the push conditional: after git add and git diff
--cached --quiet, if there are no staged changes exit successfully (skip the
credentialed git push) rather than interpolating the tag into the push command;
keep set -euo pipefail to fail fast.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3d1eb6f-dc23-4d92-ae52-a675ebaea26c
📒 Files selected for processing (16)
.github/workflows/autofix-apply.yaml.github/workflows/changelog.yaml.github/workflows/cli-docs.yaml.github/workflows/codeql.yaml.github/workflows/codespell.yaml.github/workflows/codspeed.yaml.github/workflows/config-types.yaml.github/workflows/docs.yaml.github/workflows/lint.yaml.github/workflows/llms-txt.yaml.github/workflows/publish.yaml.github/workflows/readme.yaml.github/workflows/release-draft.yaml.github/workflows/release-notify.yaml.github/workflows/schema-docs.yaml.github/workflows/test.yaml
Contributor
|
📚 Docs Preview: https://pr-3095.datamodel-code-generator.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/readme.yaml (1)
29-33:⚠️ Potential issue | 🟡 MinorPR workflow no longer fails on generated-doc drift.
With the
--notestflag,tox run -e readmeskips executing the commands defined intox.ini(which defaults to--check). The script is then run directly on line 33 without--check, so it only regenerates the files and discards the output. A PR that forgets to regenerate the docs will pass CI and only be caught (and silently fixed) later bygenerated-docs-sync.yamlonmain.Remove
--notestso the tox command actually runs the script with--checkand fails the PR on drift:♻️ Suggested change
- - name: Setup environment - run: tox run -vv --notest --skip-missing-interpreters false -e readme - env: - UV_PYTHON_PREFERENCE: "only-managed" - - name: Update README - run: .tox/readme/bin/python scripts/update_command_help_on_markdown.py + - name: Check README is up to date + run: tox run -vv --skip-missing-interpreters false -e readme + env: + UV_PYTHON_PREFERENCE: "only-managed"Also,
fetch-depth: 0(line 20) is no longer needed now that the push logic is gone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/readme.yaml around lines 29 - 33, Remove the --notest flag from the tox invocation so the readme job actually runs the readme environment with its default check (change the line invoking "tox run -vv --notest --skip-missing-interpreters false -e readme" to drop "--notest") so that the tox-managed "--check" will fail the job on generated-doc drift instead of silently regenerating via the direct ".tox/readme/bin/python scripts/update_command_help_on_markdown.py" invocation; also remove the now-unnecessary "fetch-depth: 0" setting from the workflow since push logic has been removed..github/workflows/cli-docs.yaml (1)
34-42:⚠️ Potential issue | 🟡 MinorAdd
--checkflags to both scripts to fail PR on stale CLI documentation.
build_cli_docs.pyandbuild_prompt_data.pyboth support a--checkmode that validates outputs are up-to-date and returns error code 1 if they differ from what's already committed. The workflow currently executes them in build mode (lines 39 and 41), so a PR that changes CLI options without regeneratingdocs/cli-reference/**orsrc/datamodel_code_generator/prompt_data.pywill pass CI and be silently fixed later bygenerated-docs-sync.yaml.Invoke these scripts with
--checkto catch drift in the PR:
- Line 39:
.tox/cli-docs/bin/python scripts/build_cli_docs.py --check- Line 41:
.tox/cli-docs/bin/python scripts/build_prompt_data.py --checkAlso remove
fetch-depth: 0at line 25, as the workflow has no git push operations that require full history.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cli-docs.yaml around lines 34 - 42, Update the CI step that runs the CLI doc scripts so both script invocations include the --check flag: add --check to build_cli_docs.py invocation (scripts/build_cli_docs.py) and to build_prompt_data.py invocation (scripts/build_prompt_data.py) so the workflow fails when generated docs or prompt data are stale; also remove the fetch-depth: 0 setting from the workflow checkout configuration since no full git history is required for these steps.
🧹 Nitpick comments (1)
.github/workflows/docs-deploy.yaml (1)
48-54: Consider defensively routingdeploy_branchthroughenvto prevent future injection risk in this reusable workflow.Although current callers in
docs.yamlare safe—build-previewpasses a PR number (numeric),build-productionpasses hardcodedmain, andbuild-promits it entirely—this reusable workflow acceptsdeploy_branchas an arbitrary input. Routing it through an environment variable protects against a future caller inadvertently passing an untrusted value (e.g., derived fromgithub.head_ref), which would otherwise be interpolated into the shell command.🛡️ Suggested pattern
- name: Deploy if: inputs.deploy_branch != '' + env: + DEPLOY_BRANCH: ${{ inputs.deploy_branch }} uses: cloudflare/wrangler-action@v3 with: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} - command: pages deploy site --project-name=datamodel-code-generator --branch=${{ inputs.deploy_branch }} + command: pages deploy site --project-name=datamodel-code-generator --branch="$DEPLOY_BRANCH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-deploy.yaml around lines 48 - 54, The Deploy step currently interpolates inputs.deploy_branch directly into the shell command; instead set an environment variable (e.g., DEPLOY_BRANCH) from inputs.deploy_branch on the Deploy step and update the command to reference that env var (e.g., $DEPLOY_BRANCH) so the workflow routes the untrusted input through env; modify the Deploy job that uses cloudflare/wrangler-action@v3 to add env: DEPLOY_BRANCH: ${{ inputs.deploy_branch }} and change the command to use the environment variable rather than embedding ${{ inputs.deploy_branch }} directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/generated-docs-sync.yaml:
- Around line 70-81: Confirm whether it's intentional that commits pushed with
GH_TOKEN (the env value used in the "Commit generated docs" step) should skip
triggering other workflows; if other workflows must run on these automated
commits, switch from using github.token to a repo PAT stored as a secret (e.g.,
DOCS_SYNC_PAT) and use that secret instead of GH_TOKEN for the push
authentication; also move the push URL and token into the step's env block for
consistency with the PR, and add a fast-forward safety pull (git fetch && git
pull --rebase) before git push to avoid failing on concurrent external pushes.
---
Outside diff comments:
In @.github/workflows/cli-docs.yaml:
- Around line 34-42: Update the CI step that runs the CLI doc scripts so both
script invocations include the --check flag: add --check to build_cli_docs.py
invocation (scripts/build_cli_docs.py) and to build_prompt_data.py invocation
(scripts/build_prompt_data.py) so the workflow fails when generated docs or
prompt data are stale; also remove the fetch-depth: 0 setting from the workflow
checkout configuration since no full git history is required for these steps.
In @.github/workflows/readme.yaml:
- Around line 29-33: Remove the --notest flag from the tox invocation so the
readme job actually runs the readme environment with its default check (change
the line invoking "tox run -vv --notest --skip-missing-interpreters false -e
readme" to drop "--notest") so that the tox-managed "--check" will fail the job
on generated-doc drift instead of silently regenerating via the direct
".tox/readme/bin/python scripts/update_command_help_on_markdown.py" invocation;
also remove the now-unnecessary "fetch-depth: 0" setting from the workflow since
push logic has been removed.
---
Nitpick comments:
In @.github/workflows/docs-deploy.yaml:
- Around line 48-54: The Deploy step currently interpolates inputs.deploy_branch
directly into the shell command; instead set an environment variable (e.g.,
DEPLOY_BRANCH) from inputs.deploy_branch on the Deploy step and update the
command to reference that env var (e.g., $DEPLOY_BRANCH) so the workflow routes
the untrusted input through env; modify the Deploy job that uses
cloudflare/wrangler-action@v3 to add env: DEPLOY_BRANCH: ${{
inputs.deploy_branch }} and change the command to use the environment variable
rather than embedding ${{ inputs.deploy_branch }} directly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a51d9bd5-f5ff-401f-96fa-9afa19fe8be4
📒 Files selected for processing (10)
.github/workflows/changelog.yaml.github/workflows/cli-docs.yaml.github/workflows/docs-deploy.yaml.github/workflows/docs.yaml.github/workflows/generated-docs-sync.yaml.github/workflows/lint.yaml.github/workflows/llms-txt.yaml.github/workflows/readme.yaml.github/workflows/schema-docs.yaml.pre-commit-config.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/changelog.yaml
- .github/workflows/lint.yaml
- .github/workflows/docs.yaml
- .github/workflows/llms-txt.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores