fix(zip): fail closed on MAR handler parse errors

[mldangelo-oai]

Summary

• add regression coverage for manifest-less .mar handler static analysis parse-error path
• ensure invalid Python handler syntax emits a failed TorchServe Handler Static Analysis warning check with entry context
• keep scope limited to ZIP scanner tests only

Validation

• UV_CACHE_DIR=/tmp/uv-cache uv run pytest tests/scanners/test_zip_scanner.py -k malformed_python_handler --maxfail=1
• UV_CACHE_DIR=/tmp/uv-cache uv run ruff check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
• UV_CACHE_DIR=/tmp/uv-cache uv run ruff format --check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
• UV_CACHE_DIR=/tmp/uv-cache uv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
• UV_CACHE_DIR=/tmp/uv-cache uv run pytest -n auto -m "not slow and not integration" --maxfail=1

Summary by CodeRabbit

• Tests
  • Added a unit test verifying malformed Python handlers in TorchServe archives produce a single warning with parse-error details and expected metadata.
• Bug Fixes
  • Ensured failed handler/static analysis within an archive correctly marks the overall scan as not complete and influences the final reported scan status.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d325bc43-9f4b-4e4a-b763-4facd5d50c95

📥 Commits

Reviewing files that changed from the base of the PR and between 92cab7f and 5651ffd.

📒 Files selected for processing (2)

• modelaudit/scanners/zip_scanner.py
• tests/scanners/test_zip_scanner.py

---

Walkthrough

Updates ZIP scanner behavior to propagate a failed Python static analysis from nested .mar entries into the overall ZIP scan completion status. Adds a unit test that creates a manifest-less TorchServe .mar with syntactically invalid handler.py and asserts the emitted check details and overall scan outcome.

Changes

Cohort / File(s)	Summary
Tests tests/scanners/test_zip_scanner.py	Added test_scan_manifestless_mar_reports_malformed_python_handler: builds a manifest-less .mar with invalid handler.py, runs the ZIP scanner, and asserts a single TorchServe Handler Static Analysis check fails with CheckStatus.FAILED, IssueSeverity.WARNING, analysis_kind == "syntax", parse_error text contains an expected token message, entry == "handler.py", location "{mar_path}:handler.py", and overall scan success == False with warnings and no errors.
Scanner logic modelaudit/scanners/zip_scanner.py	When merging nested .mar Python analysis results, inspect mar_python_result.success and set scan_complete = False if it is false so a failed handler analysis prevents the ZIP scan from finishing as successful; include analysis_kind: "syntax" and parse_error in the check details for parse failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble bytes beneath the moonlit log,

a malformed handler tucked inside a bog.
Syntax stumbles, the parser sings its plea—
A warning blooms, the scanner caught the spree.
I hop, I thump: that bug won't run free.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: improving handling of MAR handler parse errors by failing closed (failing safely) in the ZIP scanner.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/zip-mar-parse-error-coverage

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/scanners/test_zip_scanner.py`:
- Around line 343-354: The test currently verifies the warning details for a
malformed TorchServe handler but doesn't assert the overall scan failed; update
the assertions in test_zip_scanner.py around the block using result =
self.scanner.scan(str(mar_path)) to include an explicit failure contract by
asserting result.success is False immediately after the scan call (before
checking individual checks), ensuring the fail-closed behavior for parse errors
is enforced.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: f9358464-bf9e-49dd-858f-3951c0ed57b5

📥 Commits

Reviewing files that changed from the base of the PR and between 70e30e8 and c3a35b6.

📒 Files selected for processing (1)

• tests/scanners/test_zip_scanner.py

[github-actions]

Workflow run and artifacts

Performance Benchmarks

Compared 6 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 6 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 683.64ms -> 684.97ms (+0.2%).

Benchmark	Target	Size	Files	Baseline	Current	Change	Status
tests/benchmarks/test_scan_benchmarks.py::test_scan_mixed_directory	mixed-corpus	1.7 MiB	54	142.25ms	135.79ms	-4.5%	stable
tests/benchmarks/test_scan_benchmarks.py::test_validate_file_type_pytorch_zip	state_dict.pt	1.5 MiB	1	47.9us	50.0us	+4.3%	stable
tests/benchmarks/test_scan_benchmarks.py::test_detect_file_format_safe_pickle	safe_model.pkl	49.4 KiB	1	162.8us	167.4us	+2.8%	stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_directory	duplicate-corpus	840.0 KiB	81	479.21ms	487.41ms	+1.7%	stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_safe_pickle	safe_model.pkl	49.4 KiB	1	28.10ms	27.83ms	-1.0%	stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_pytorch_zip	state_dict.pt	1.5 MiB	1	33.87ms	33.72ms	-0.5%	stable

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/scanners/test_zip_scanner.py`:
- Around line 343-345: Add an explicit assertion that the scan failed closed by
checking result.has_errors is True alongside the existing success check: after
calling self.scanner.scan(str(mar_path)) and the existing assert result.success
is False, add assert result.has_errors is True to make the operational failure
signal explicit (referencing the scanner.scan call and the result object).

🪄 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: ASSERTIVE

Plan: Pro

Run ID: 017042c8-e3b8-44c6-923a-84e7dd6c45e0

📥 Commits

Reviewing files that changed from the base of the PR and between c3a35b6 and 92cab7f.

📒 Files selected for processing (2)

• modelaudit/scanners/zip_scanner.py
• tests/scanners/test_zip_scanner.py