Skip to content

fix: enforce ZIP aggregate size budget#1022

Merged
mldangelo-oai merged 2 commits intomainfrom
mdangelo/codex/enforce-zip-aggregate-budget
Apr 16, 2026
Merged

fix: enforce ZIP aggregate size budget#1022
mldangelo-oai merged 2 commits intomainfrom
mdangelo/codex/enforce-zip-aggregate-budget

Conversation

@mldangelo-oai
Copy link
Copy Markdown
Contributor

Summary

  • Add a ZIP aggregate uncompressed-size budget before member extraction so many individually allowed entries cannot bypass per-entry limits.
  • Record archive-wide uncompressed size metadata and fail closed with zip_analysis_incomplete when the budget is exceeded.
  • Keep an actual-byte guard during extraction to catch malformed ZIP metadata that understates member sizes.
  • Add positive and near-match regression tests for split-entry archive budget handling.

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 -q
  • uv 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=1
  • uv 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/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

Workflow run and artifacts

Performance Benchmarks

Compared 19 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 19 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 182.02ms -> 182.02ms (-0.0%).

Benchmark Target Size Files Baseline Current Change Status
tests/benchmarks/test_scan_benchmarks.py::test_detect_file_format_safe_pickle safe_model.pkl 49.4 KiB 1 32.2us 29.1us -9.4% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_safe_payloads[safe_small] safe_small 68 B 1 57.2us 54.0us -5.7% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_multi_stream_padded_payload multi_stream_padded 4.1 KiB 1 138.5us 133.0us -3.9% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_opcode_budget_tail_payload opcode_budget_tail 14 B 1 77.9us 75.2us -3.4% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_safe_payloads[long_benign_string] long_benign_string 1.0 MiB 1 1.12ms 1.09ms -2.8% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_dangerous_global_payloads[malicious_reduce] malicious_reduce 52 B 1 78.8us 76.7us -2.6% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payloads[nested_raw] nested_raw 78 B 1 101.7us 99.3us -2.3% stable
tests/benchmarks/test_scan_benchmarks.py::test_skip_filter_plain_text_files - 4.6 KiB 256 13.38ms 13.10ms -2.1% stable
tests/benchmarks/test_scan_benchmarks.py::test_validate_file_type_pytorch_zip state_dict.pt 1.5 MiB 1 50.8us 50.0us -1.6% stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_pytorch_zip state_dict.pt 1.5 MiB 1 29.03ms 29.45ms +1.4% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_dangerous_global_payloads[stack_global] stack_global 21 B 1 68.1us 67.4us -1.0% stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_directory duplicate-corpus 840.0 KiB 81 45.96ms 45.59ms -0.8% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_stream chunked_stream 278.2 KiB 1 6.64ms 6.68ms +0.6% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payloads[nested_hex] nested_hex 130 B 1 108.8us 109.2us +0.4% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payloads[nested_base64] nested_base64 98 B 1 103.6us 104.0us +0.4% stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_mixed_directory mixed-corpus 1.7 MiB 54 70.65ms 70.90ms +0.4% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_safe_payloads[safe_large] safe_large 278.2 KiB 1 3.44ms 3.45ms +0.3% stable
tests/benchmarks/test_scan_benchmarks.py::test_scan_safe_pickle safe_model.pkl 49.4 KiB 1 10.41ms 10.39ms -0.2% stable
tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_hidden_suspicious_string_budget hidden_suspicious_string 8.0 KiB 1 576.6us 576.7us +0.0% stable

@mldangelo-oai mldangelo-oai changed the title [codex] fix: enforce ZIP aggregate size budget fix: enforce ZIP aggregate size budget Apr 15, 2026
@mldangelo-oai mldangelo-oai marked this pull request as ready for review April 15, 2026 23:31
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +79 to +87
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@mldangelo-oai mldangelo-oai force-pushed the mdangelo/codex/enforce-zip-aggregate-budget branch from 1d53190 to 335d3b9 Compare April 16, 2026 06:37
@mldangelo-oai mldangelo-oai merged commit 94d576f into main Apr 16, 2026
1 check passed
@mldangelo-oai mldangelo-oai deleted the mdangelo/codex/enforce-zip-aggregate-budget branch April 16, 2026 06:37
@github-actions github-actions bot mentioned this pull request Apr 16, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +99 to +100
else:
positive_limits.append(self.DEFAULT_MAX_TOTAL_UNCOMPRESSED_SIZE)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +542 to +548
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant