fix: enforce ZIP aggregate size budget#1022
Conversation
Performance BenchmarksCompared
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d53190e96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if configured_file_limit is not None and configured_file_limit != 0: | ||
| return self._normalize_positive_int_config(configured_file_limit, default_limit) | ||
|
|
||
| if configured_entry_limit is not None: | ||
| if configured_entry_limit == 0: | ||
| return self.UNLIMITED_ARCHIVE_SIZE | ||
| return self._normalize_positive_int_config(configured_entry_limit, default_limit) | ||
|
|
||
| return default_limit |
There was a problem hiding this comment.
Preserve unlimited max_file_size semantics for ZIP entries
max_file_size=0 is documented/used as “unlimited” (see core.py), but _get_max_entry_size() now treats 0 as “ignore and use default”, returning 10GB unless max_entry_size is explicitly set. This silently introduces a hard per-entry cap for default scans and can make valid large ZIP members fail with ZIP Entry Scan errors/inconclusive outcomes.
Useful? React with 👍 / 👎.
1d53190 to
335d3b9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 335d3b9a09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else: | ||
| positive_limits.append(self.DEFAULT_MAX_TOTAL_UNCOMPRESSED_SIZE) |
There was a problem hiding this comment.
Honor explicit ZIP aggregate limits above the default cap
_get_max_total_uncompressed_size() always inserts the 10GB default when max_zip_total_uncompressed_size is unset, then returns min(...) across all limits. This makes explicit higher public limits (for example max_total_size/max_file_size >10GB) ineffective, so valid large archives are still forced into zip_analysis_incomplete unexpectedly.
Useful? React with 👍 / 👎.
| if extracted_uncompressed_size + total_size > max_total_uncompressed_size: | ||
| raise ValueError( | ||
| "ZIP archive exceeds maximum total uncompressed size of " | ||
| f"{max_total_uncompressed_size} bytes", | ||
| ) | ||
| tmp.write(chunk) | ||
| extracted_uncompressed_size += total_size |
There was a problem hiding this comment.
Stop scanning after aggregate-byte guard is tripped
If the runtime aggregate check fails, the exception is handled per-entry and the loop continues. Since extracted_uncompressed_size is only updated after successful extraction, bytes already read for the failing member are never accounted for. A malformed archive with understated sizes can repeatedly hit this path and drive decompression work beyond the configured aggregate budget.
Useful? React with 👍 / 👎.
Summary
zip_analysis_incompletewhen the budget is exceeded.Validation
uv run ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/uv run ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest tests/scanners/test_zip_scanner.py -quv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -n auto -m "not slow and not integration" --maxfail=1uv run ruff format --check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/uv run ruff check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/