Harden GitHub Action#3542
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe composite action updates its install and argument handling, and a new smoke workflow exercises generation twice against a prepared schema fixture and validates the output plus sentinel handling. ChangesComposite action hardening and smoke test
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant TempFixture
participant CompositeAction
participant GeneratedModel
Workflow->>TempFixture: copy schema and touch sentinel filename
Workflow->>CompositeAction: run generate step
CompositeAction->>GeneratedModel: write generated/model.py
Workflow->>CompositeAction: run generate step again
CompositeAction->>GeneratedModel: re-write generated/model.py
Workflow->>GeneratedModel: grep expected class and field
Workflow->>TempFixture: assert sentinel is absent
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
📚 Docs Preview: https://pr-3542.datamodel-code-generator.pages.dev |
Merging this PR will degrade performance by 11%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_perf_all_options_enabled |
4.6 s | 5.2 s | -12.34% |
| ❌ | WallTime | test_perf_complex_refs |
1.7 s | 1.9 s | -11.55% |
| ❌ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.2 s | 2.5 s | -11.22% |
| ❌ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.7 s | 1.9 s | -10.89% |
| ❌ | WallTime | test_perf_openapi_large |
2.6 s | 2.9 s | -10.72% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2 |
3 s | 3.4 s | -10.71% |
| ❌ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.7 s | 1.9 s | -10.7% |
| ❌ | WallTime | test_perf_deep_nested |
4.5 s | 5 s | -10.59% |
| ❌ | WallTime | test_perf_graphql_style_pydantic_v2 |
689.4 ms | 768.2 ms | -10.25% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing harden-github-action (4ae5b77) with main (958c5b6)
Footnotes
-
98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
CodSpeed is not actionable for this PR: the diff only touches |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3542 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 161 161
Lines 34363 34363
Branches 4011 4011
=========================================
Hits 34363 34363
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 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: 1
🧹 Nitpick comments (2)
.github/workflows/action-smoke.yaml (1)
37-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused fixture file.
injection.json(line 42) is created but never referenced by any subsequent step — only theinjection$(touch action-injection-ran).jsonfixture is used asinput. Consider removing the unused copy.♻️ Proposed cleanup
mkdir -p "$RUNNER_TEMP/action-smoke/schemas" "$RUNNER_TEMP/action-smoke/generated" cp tests/data/jsonschema/simple_string.json "$RUNNER_TEMP/action-smoke/schemas/injection\$(touch action-injection-ran).json" - cp tests/data/jsonschema/simple_string.json "$RUNNER_TEMP/action-smoke/schemas/injection.json"🤖 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/action-smoke.yaml around lines 37 - 42, The smoke fixture setup in the Prepare smoke fixtures step creates an extra copy that is never used. Remove the redundant cp for the unused injection.json fixture and keep only the fixture actually referenced later by the action-smoke workflow, using the existing Prepare smoke fixtures block as the location to update.action.yml (1)
65-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winValidate
extrasagainst the documented allowlist.Line 67 embeds raw
inputs.extrasinto the pip requirement. Reject anything outsidegraphql,http,validation,ruff, andallbefore buildingPACKAGE.Proposed hardening
EXTRAS="" if [ -n "$INPUT_EXTRAS" ]; then - EXTRAS="[$INPUT_EXTRAS]" + IFS=',' read -r -a REQUESTED_EXTRAS <<< "$INPUT_EXTRAS" + NORMALIZED_EXTRAS=() + for EXTRA in "${REQUESTED_EXTRAS[@]}"; do + EXTRA="${EXTRA//[[:space:]]/}" + case "$EXTRA" in + graphql|http|validation|ruff|all) + NORMALIZED_EXTRAS+=("$EXTRA") + ;; + *) + echo "::error::Unsupported extras value: $EXTRA" + exit 1 + ;; + esac + done + EXTRAS="[$(IFS=,; echo "${NORMALIZED_EXTRAS[*]}")]" fi🤖 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 `@action.yml` around lines 65 - 68, The `EXTRAS` handling in the action script is using raw `INPUT_EXTRAS` directly, so `PACKAGE` can be built with unsupported or unsafe extras. Add validation in the same shell block that sets `EXTRAS` to reject any value outside the documented allowlist (`graphql`, `http`, `validation`, `ruff`, `all`) before constructing `PACKAGE`, and keep the change localized around the `INPUT_EXTRAS`/`EXTRAS` logic so the allowed values are enforced consistently.
🤖 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 `@action.yml`:
- Around line 110-117: The extra-args parsing in the action’s shell script
currently runs `shlex.split` inside process substitution, which can hide parser
failures and let `datamodel-codegen` continue with incomplete arguments.
Refactor the `INPUT_EXTRA_ARGS` handling so the `python3` parser is executed as
a normal command with its exit status checked explicitly before appending to
`ARGS`, and keep the logic localized around the existing `EXTRA_ARG`/`ARGS`
block.
---
Nitpick comments:
In @.github/workflows/action-smoke.yaml:
- Around line 37-42: The smoke fixture setup in the Prepare smoke fixtures step
creates an extra copy that is never used. Remove the redundant cp for the unused
injection.json fixture and keep only the fixture actually referenced later by
the action-smoke workflow, using the existing Prepare smoke fixtures block as
the location to update.
In `@action.yml`:
- Around line 65-68: The `EXTRAS` handling in the action script is using raw
`INPUT_EXTRAS` directly, so `PACKAGE` can be built with unsupported or unsafe
extras. Add validation in the same shell block that sets `EXTRAS` to reject any
value outside the documented allowlist (`graphql`, `http`, `validation`, `ruff`,
`all`) before constructing `PACKAGE`, and keep the change localized around the
`INPUT_EXTRAS`/`EXTRAS` logic so the allowed values are enforced consistently.
🪄 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: 0ac8f210-b007-43cb-a92c-5680c7225bf6
📒 Files selected for processing (2)
.github/workflows/action-smoke.yamlaction.yml
koxudaxi
force-pushed
the
harden-github-action
branch
from
e7057f5 to
4ae5b77
Compare
Breaking Change AnalysisResult: Breaking changes detected Reasoning: The PR only modifies action.yml and adds a smoke-test workflow — no Python library, CLI code, generated-output, or template changes. However, action.yml changes alter the behavior of the published GitHub Action for existing users: (1) extra-args is now parsed with shlex.split instead of being injected raw into the shell, so users relying on shell expansion (globs, Content for Release NotesDefault Behavior Changes
This analysis was performed by Claude Code Action |
|
🎉 Released in 0.67.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes