Skip to content

fix: bound weight distribution tensor extraction - #1581

Merged
mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-weight-distribution-load-bounds
Jun 9, 2026
Merged

mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-weight-distribution-load-bounds

Conversation

@mldangelo-oai

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

Copy link
Copy Markdown
Contributor

Summary

  • enforce independent per-tensor and cumulative extraction budgets before materializing PyTorch, HDF5, TensorFlow, and ONNX weights
  • synchronize with the explicit torch.load opt-in policy and retain bounded primitive ZIP fallback analysis
  • accept exactly one credible PyTorch serialization root and fail closed when multiple roots could hide the real pickle behind a decoy
  • reject ambiguous members and bound pickle reads, graph growth, memo use, and storage expansion
  • deduplicate shared tensor/storage aliases while preserving valid benign matrices
  • fail closed to secure defaults for invalid byte-limit configuration while preserving explicit 0 as unlimited
  • preserve benign internal HDF5 links and keep incomplete diagnostics accurate

Review fixes

  • skip rank-1 TensorFlow checkpoint variables before applying extraction budgets or loading them
  • ignore HDF5 external links only when neither the local link path nor target path is weight-like
  • retain fail-closed handling for external weight tensors and weight groups
  • preserve the legacy max_array_size=0 unlimited behavior by defaulting the new aggregate budget to unlimited, while honoring an explicitly configured aggregate cap
  • replace the storage introspection suppression block with a single-assignment fallback
  • reject multiple credible PyTorch pickle roots instead of selecting a shallow decoy
  • synchronize with current main

Validation

  • complete associated weight-distribution module: 87 passed, 1 skipped (TensorFlow unavailable)
  • focused new false-positive, fail-closed, and compatibility regressions: 12 passed
  • scoped Ruff check/format, mypy, and git diff --check: clean
  • full suite intentionally left to CI per review workflow

Published state

  • current main: ff9e1aef042e847ed20a5dbd14c96a8550e3b467
  • head: c10a77c969fd70d1e9a060cb82f49315c96056dd
  • exact verified tree: 0fe94eb8f16dff5c86036db9777545cbae152e4f

Fresh CI is running on the published head.

@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.333s -> 1.323s (-0.7%).

Workload Benchmark Target Size Files Baseline Current Change Status
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 519.2us 530.8us +2.2% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 583.2us 596.0us +2.2% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 510.0us 519.4us +1.8% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 100.17ms 98.73ms -1.4% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 108.09ms 109.57ms +1.4% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 464.39ms 458.47ms -1.3% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 492.0us 498.0us +1.2% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 367.82ms 363.46ms -1.2% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 117.50ms 117.91ms +0.3% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 66.01ms 66.16ms +0.2% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 525.4us 526.6us +0.2% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 106.41ms 106.46ms +0.1% stable

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-weight-distribution-load-bounds branch from d62d93c to 8ec9ac6 Compare June 8, 2026 19:39
@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 8, 2026 19:40

mldangelo-oai commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor Author

@codex Please apply the validated local integration from commit 5ca0c9c8 onto the current PR head 2dfd9d0ff4862206399b9c8c65310c1b26e13cba and merge current main. The local Git credential cannot publish the commit.

Required corrections:

  1. Select one PyTorch ZIP serialization root for preflight and fallback. Prefer credible root markers, then the shallowest data.pkl; count only that root's exact data/<storage> members. Fail closed on equally plausible roots or duplicate selected data.pkl members. Ignore nested decoy extras/data.pkl and extras/data/* members.
  2. Account CPU PyTorch aliases by unique untyped_storage() identity, but count every non-CPU tensor's host materialization independently. Keep the logical per-tensor size check and reject a CPU view before NumPy conversion when its retained backing storage exceeds max_array_size (pytorch_tensor_storage_size_limit).
  3. Skip definitely rank < 2 weights before materialization in primary PyTorch, primitive ZIP fallback, HDF5, TensorFlow checkpoints, TensorFlow Const, and ONNX paths where shape metadata is available.
  4. Preserve current-main's literal enable_unsafe_torch_load is True requirement and bounded restricted-pickle graph/memo checks. Do not restore version-gated automatic torch.load.
  5. Catch malformed pickletools.genops() streams during ZIP preflight, record pytorch_pickle_parse_failed, and return inconclusive before torch.load; otherwise explicit unsafe opt-in can turn malformed metadata into a clean zero-layer result.

Focused regressions include nested ZIP decoys/ambiguity, CPU and non-CPU shared storage, oversized backing storage before NumPy, malformed pickle before load, rank-1 budget preservation, bounded pickle reads/graphs, and per-format size preflights.

Validated on the current-main integration:

  • PROMPTFOO_DISABLE_TELEMETRY=1 ./.venv/bin/pytest -q tests/scanners/test_weight_distribution_scanner.py --maxfail=1 -> 83 passed, 1 skipped
  • touched-file Ruff check and format check: clean
  • touched-file mypy: clean
  • git diff --check: clean

There are currently no review threads. Do not merge the published head until these corrections are present.

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

Testing

  • ✅ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -q tests/scanners/test_weight_distribution_scanner.py --maxfail=1
  • ✅ uv run ruff check modelaudit/scanners/weight_distribution_scanner.py tests/scanners/test_weight_distribution_scanner.py
  • ✅ uv run ruff format --check modelaudit/scanners/weight_distribution_scanner.py tests/scanners/test_weight_distribution_scanner.py
  • ✅ uv run mypy modelaudit/scanners/weight_distribution_scanner.py tests/scanners/test_weight_distribution_scanner.py
  • ✅ 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/
  • ✅ git diff --check
  • ⚠️ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -n auto -m "not slow and not integration" --maxfail=1 was started but manually stopped after an extended run in this environment before completion.

View task →

Copy link
Copy Markdown
Contributor Author

Validated corrective repair is committed locally as 4ee46a27 on mdangelo/codex/pr-1581-final, but HTTPS push is blocked by the repository credential (Invalid username or token).

The repair preserves current-main's literal enable_unsafe_torch_load is True gate and restricted-pickle graph/memo checks, and adds:

  • one credible PyTorch ZIP serialization root shared by preflight and fallback; root markers beat storage-only roots, shallowest wins, ties/duplicate selected data.pkl fail closed, and nested decoy extras/data.pkl / extras/data/* are ignored;
  • bounded data.pkl reads using remaining budget, malformed/read-failure preflight that blocks torch.load, and direct numeric data/<storage> accounting only;
  • per-tensor logical checks plus unique CPU untyped_storage() accounting, backing-storage checks before NumPy, and per-materialization accounting for non-CPU tensors;
  • rank-1 early exits before materialization across PyTorch primary/fallback, HDF5, TensorFlow checkpoint/Const, and ONNX;
  • adversarial false-positive/false-negative regressions for all of the above.

Validation: 88 passed, 1 skipped in tests/scanners/test_weight_distribution_scanner.py (TensorFlow integration skip), full Ruff lint/format clean, and full mypy clean across 464 files. No full pytest suite was run per campaign instructions.

@codex address that feedback by applying the equivalent of local commit 4ee46a27 to mdangelo/codex/fix-weight-distribution-load-bounds. Do not merge the currently published head until this repair is present.

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

Testing

  • ✅ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -q tests/scanners/test_weight_distribution_scanner.py --maxfail=1
  • ✅ 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/
  • ✅ git diff --check

View task →

Copy link
Copy Markdown
Contributor Author

Critical audit complete and repair published at 72175e428fd52838fdd39acf566e4aee83b41d99.

Validated fixes and edge cases:

  • merged current main and preserved the newer literal-True unsafe-load opt-in
  • rejected ambiguous/duplicate data.pkl roots while allowing a uniquely marked valid PyTorch root
  • bounded ZIP pickle bytes, object growth, opcode/memo growth, selected storage bytes, per-tensor bytes, and cumulative retained bytes
  • prevented rank-1 metadata from consuming retained tensor budget
  • deduplicated primitive and torch backing-storage aliases to avoid repeated allocation/accounting
  • changed invalid, negative, boolean, and non-finite tensor limits from fail-open unlimited behavior to the secure 100 MiB default
  • retained benign near-match coverage for unrelated ZIP metadata, shared sequences, internal HDF5 links, nonnumeric metadata, and marked-root selection

Focused validation: 83 passed, 1 TensorFlow-dependent skip; scoped Ruff, formatting, mypy, and git diff --check are clean. No unresolved review threads were present.

Comment thread modelaudit/scanners/weight_distribution_scanner.py Fixed
Comment thread modelaudit/scanners/weight_distribution_scanner.py Fixed
Comment thread modelaudit/scanners/weight_distribution_scanner.py Fixed

@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: 72175e428f

ℹ️ 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/weight_distribution_scanner.py Outdated
@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-weight-distribution-load-bounds branch from 72175e4 to bcd6e9f Compare June 9, 2026 05:35

Copy link
Copy Markdown
Contributor Author

Published the review follow-up as bcd6e9ffc46dc41c3a32b81d96ce1a263f5b6aa3 on current main (exact tree 6e80307ee44e07b4dea0054eb8d2e98087935919). The P1 decoy-root finding is fixed by requiring exactly one credible PyTorch serialization root; multiple credible roots now produce an explicit inconclusive result, with a regression covering root-level decoy plus nested real weights. The three code-quality findings are also addressed. Validation: full associated scanner module 84 passed, 1 expected TensorFlow skip; focused current-main root-selection slice 3 passed; scoped Ruff, format, mypy, and diff checks are clean.

@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: bcd6e9ffc4

ℹ️ 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/weight_distribution_scanner.py
Comment thread modelaudit/scanners/weight_distribution_scanner.py
Comment thread modelaudit/scanners/weight_distribution_scanner.py
@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-weight-distribution-load-bounds branch from bcd6e9f to c10a77c Compare June 9, 2026 06:48

Copy link
Copy Markdown
Contributor Author

Published all three fresh review fixes on current main at c10a77c969fd70d1e9a060cb82f49315c96056dd (exact tested tree 0fe94eb8f16dff5c86036db9777545cbae152e4f). Rank-1 TensorFlow variables are skipped before budgeting/loading; unrelated HDF5 external links are ignored only when both local and target paths are non-weight-like, preserving fail-closed external weight handling; and legacy max_array_size=0 now defaults the aggregate budget to unlimited while an explicit aggregate cap still wins. Complete associated module: 87 passed, 1 skipped; focused new positive/negative cases: 12 passed; scoped Ruff, mypy, format, and diff checks are clean. Fresh CI is running while the queue continues.

@mldangelo-oai
mldangelo-oai merged commit dc0051c into main Jun 9, 2026
27 of 30 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-weight-distribution-load-bounds branch June 9, 2026 07:33
@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