Skip to content

fix: bound keras zip config traversal - #1555

Merged
mldangelo-oai merged 7 commits into
mainfrom
mdangelo/codex/fix-keras-config-traversal-budget
Jun 9, 2026
Merged

mldangelo-oai merged 7 commits into
mainfrom
mdangelo/codex/fix-keras-config-traversal-budget

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • bound Keras config.json depth, admitted/pending work, direct-field work, string count, and total string characters
  • build an iterative bounded projection for legacy model, layer, inbound-node, and compile metric/loss consumers
  • decouple admitted structural traversal from generic string exhaustion so queued security-relevant layers remain visible
  • preserve only fixed security fields after generic overflow through a separately bounded aggregate literal reserve
  • keep the independently bounded unsafe-deserialization and get_file CVE detectors on the original config
  • add constant-work direct checks for queued unsafe-deserialization nodes and direct/nested get_file callable shapes
  • bound archive-origin slicing before whitespace normalization, regex matching, or evidence redaction
  • preserve admitted scalar security fields at depth and item boundaries while omitting unadmitted values cleanly
  • cap effective configurable depth at 256 so recursive legacy helpers cannot reach Python recursion limits
  • reserve queued children before enqueueing, preventing nested-wide pending-work amplification
  • avoid duplicate and recursive kwargs URL extraction across nested get_file calls
  • stop detector loops immediately after budget exhaustion and fail closed with explicit inconclusive metadata
  • retain current-main truthy get_file extraction semantics and their regression matrix
  • remove the unused StringLookup inconclusive-reason constant reported by code quality

Adversarial review

The review reproduced and fixed:

  • wide and nested container CPU/memory amplification
  • unbounded root layer, inbound-node, and compile metric/loss scans after preflight exhaustion
  • benign nested kwargs strings being double-counted
  • descendant kwargs subtrees being rescanned once per ancestor
  • depth/item/string boundary handling erasing admitted Lambda, custom-layer, unsafe-deserialization, and get_file findings
  • nested callable dictionaries dropping get_file evidence
  • security-string fallback exceeding the configured aggregate character budget
  • archive origins bypassing the string budget during strip/regex handling
  • unknown post-overflow projection keys escaping the direct-work item bound
  • synthetic None placeholders producing false structural evidence
  • recursive projection cleanup and configured-depth recursion failures

The final read-only audit's two boundedness findings were fixed and covered by regressions.

Validation

  • complete associated Keras ZIP module: 506 passed, 73 skipped
  • adversarial traversal-budget class: 45 passed
  • affected get_file and unsafe-deserialization CVE classes: 64 passed
  • scoped Ruff lint and format checks
  • scoped mypy
  • git diff --check
  • exact remote tree differs from current main in three files

Optional dependency skips require unavailable HDF5/ONNX packages. Per review policy, the full repository suite was left to CI.

Published head: 8ecc44aab1c57c701280bb61224a0b98c0eec6cd

@github-actions

github-actions Bot commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Workflow run and artifacts

Performance Benchmarks

Compared 12 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 12 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 1.370s -> 1.348s (-1.6%).

Workload Benchmark Target Size Files Baseline Current Change Status
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 128.63ms 118.97ms -7.5% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 109.83ms 105.41ms -4.0% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 111.00ms 108.32ms -2.4% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 599.7us 588.3us -1.9% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 100.96ms 99.80ms -1.1% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 530.0us 524.9us -1.0% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 526.6us 531.5us +0.9% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 374.33ms 372.09ms -0.6% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 474.04ms 472.30ms -0.4% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 68.09ms 68.32ms +0.3% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 499.6us 501.0us +0.3% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 520.2us 520.2us +0.0% stable

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 8, 2026 21:14

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

Copy link
Copy Markdown

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: 2447f8e23b

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated
Comment thread modelaudit/scanners/keras_zip_scanner.py Outdated

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

Copy link
Copy Markdown

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: 2447f8e23b

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated

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

Copy link
Copy Markdown

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: 8d6fa15c82

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated

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

Copy link
Copy Markdown

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: 8d6fa15c82

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated
Comment thread modelaudit/scanners/keras_zip_scanner.py Fixed

Copy link
Copy Markdown
Contributor Author

Critical review found and fixed an additional bounded-traversal bypass beyond the existing review threads: _validate_config_traversal_budget() reported overflow, but _scan_model_config() and compile/evidence helpers still traversed the original unbounded config. Local commit 42b9a2c9 routes all downstream structured analysis through an order-preserving bounded projection, preserves admitted malicious prefixes, removes the unresolved unused constant, and adds focused regressions. Current-main integrated head: c1a426bb.

Validation:

  • 556 passed in tests/scanners/test_keras_zip_scanner.py
  • repository Ruff check clean
  • repository Ruff format check clean
  • repository mypy clean (464 files)
  • git diff --check clean
  • Published Python CI failure is unrelated: TestWeightDistributionScanner::test_pytorch_model_scan, fixed on current main by test: opt in PyTorch weight scan fixture #1593

Shell GitHub credentials reject pushes in this environment. @codex apply/push local commits 42b9a2c9 and c1a426bb to mdangelo/codex/fix-keras-config-traversal-budget, then resolve the remaining code-quality thread after the remote head updates.

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

Copy link
Copy Markdown

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: fe046f2191

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated
Comment thread modelaudit/scanners/keras_zip_scanner.py Outdated

Copy link
Copy Markdown
Contributor Author

Completed the final critical review and published the exact validated tree at c0b536989aec902b878ad3c4dd4f147d7c8c1596.

In addition to removing the unused constant from the live code-quality thread, this closes the remaining traversal gaps found during adversarial QA: downstream layer/inbound/compile consumers now receive an iterative bounded projection; independently bounded CVE detectors retain original-config boundary visibility; nested kwargs are not double-counted or recursively rescanned; admitted scalar security fields survive depth/item exhaustion; omitted fields do not create synthetic None evidence; and configurable depth is capped at the recursion-safe 256 ceiling.

Final associated validation: 492 passed, 73 optional-dependency skips; 31 focused adversarial budget tests; Ruff, formatting, mypy, and diff checks clean. All review threads are resolved, and the remote diff is exactly the intended three files.

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

Copy link
Copy Markdown

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: c0b536989a

ℹ️ 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 thread modelaudit/scanners/keras_zip_scanner.py Outdated
Comment thread modelaudit/scanners/keras_zip_scanner.py Outdated
Comment thread modelaudit/scanners/keras_zip_scanner.py Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • Removed the unused Keras StringLookup external-vocabulary inconclusive-reason constant; the surrounding config traversal reason constants now flow directly into the Keras version patterns. modelaudit/scanners/keras_zip_scanner.pyL245-L250
  • Routed parsed .keras config.json handling through a bounded projection before all structured CVE checks and model-config scanning, so downstream analysis no longer traverses the original unbounded config after traversal-budget overflow. modelaudit/scanners/keras_zip_scanner.pyL795-L819
  • Added _project_bounded_config() to build an order-preserving, budget-admitted config projection while preserving already-admitted strings, dicts, lists, and scalar nodes for later analysis. modelaudit/scanners/keras_zip_scanner.pyL1180-L1264
  • Added regression coverage proving downstream scans use the bounded projection: an admitted get_file prefix remains detected while overflow-only layers are not analyzed. tests/scanners/test_keras_zip_scanner.pyL7810-L7837
  • Committed the changes on the current branch: 49009ca fix: project bounded keras configs before analysis.

Testing

  • ✅ uv run pytest tests/scanners/test_keras_zip_scanner.py::TestKerasZipConfigTraversalBudget -q
  • ✅ uv run pytest tests/scanners/test_keras_zip_scanner.py -q
  • ✅ 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/
  • ✅ uv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run ruff check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run ruff format --check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ git diff --check
  • ⚠️ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -n auto -m "not slow and not integration" --maxfail=1 — ran for 17:10 and reached 10295 passed, 589 skipped, then stopped on two unrelated CLI symlink-report failures in tests/test_cli.py.

View task →

Copy link
Copy Markdown
Contributor Author

Published final validated head 793040ae.

Late review findings addressed:

  • admitted nested get_file gadgets remain detectable after one or multiple literal overflows
  • queued unsafe-deserialization nodes remain detectable after overflow
  • queued Lambda/custom-layer findings survive generic string exhaustion through a separately bounded security reserve
  • nested callable dictionaries preserve exact get_file references without matching helper near-matches
  • archive origins are capped before strip/regex/redaction
  • post-overflow security strings and unknown projection keys remain aggregate-budget bounded

Validation on the exact published tree:

  • Keras ZIP scanner module: 506 passed, 73 skipped
  • traversal-budget regressions: 45 passed
  • affected CVE classes: 64 passed
  • scoped Ruff, format, mypy, and git diff --check: clean

@mldangelo-oai
mldangelo-oai merged commit 6e6ba57 into main Jun 9, 2026
11 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-keras-config-traversal-budget branch June 9, 2026 03:54

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

Copy link
Copy Markdown

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: 8ecc44aab1

ℹ️ 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 +1387 to +1388
):
break

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve security fields at item boundary

When a dict is the last item admitted by max_items, this break can fire while queuing its first scalar child, before the item-limit fallback that preserves security keys runs. That projects an already-admitted Lambda/custom layer as {}, so the scan is only inconclusive and misses the layer finding.

Useful? React with 👍 / 👎.

Comment on lines +418 to +419
_CONFIG_PROJECTION_KEYS_AFTER_STRING_LIMIT: ClassVar[frozenset[str]] = frozenset(
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve nested callable keys after overflow

After a prior string overflow, keys not in this allowlist are dropped; that includes activation and initializer fields scanned by _check_nested_serialized_module_references. A layer with config.activation -> posix.system becomes only inconclusive, missing CVE-2025-1550. guidance

Useful? React with 👍 / 👎.

@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
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