Skip to content

fix(routing): prefer validated safetensors framing - #1612

Merged
mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-safetensors-protocol-routing
Jun 10, 2026
Merged

mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-safetensors-protocol-routing

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Contributor

Summary

  • recognize validated SafeTensors framing before protocol-less pickle probing
  • preserve malicious protocol-less pickle routing
  • add a regression for a SafeTensors header length that decodes as STACK_GLOBAL; STOP

Campaign evidence

The Hugging Face campaign found valid SafeTensors shards routed as pickle or pickle_routing_inconclusive because the little-endian header length also formed a protocol-less pickle prefix. The strong SafeTensors frame validation was only reached after the weaker pickle probe.

This is separate from the zlib collision fixed in #1610.

Validation

  • regression fails on origin/main: detect_file_format(...) == "pickle"
  • pytest -q tests/utils/file/test_filetype.py tests/test_core.py -k 'protocolless or pickle_shaped_safetensors' (37 passed)
  • Ruff format and lint pass on changed Python files
  • mypy passes on changed Python files

@github-actions

github-actions Bot commented Jun 10, 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.342s -> 1.339s (-0.2%).

Workload Benchmark Target Size Files Baseline Current Change Status
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 566.1us 546.1us -3.5% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 512.2us 525.7us +2.6% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 88.05ms 86.73ms -1.5% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 519.5us 523.4us +0.8% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 457.0us 460.2us +0.7% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 133.89ms 134.66ms +0.6% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 441.36ms 438.82ms -0.6% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 109.91ms 110.47ms +0.5% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 65.74ms 65.57ms -0.3% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 574.9us 576.4us +0.3% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 386.74ms 386.15ms -0.2% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 113.80ms 113.83ms +0.0% stable

@mldangelo-oai

Copy link
Copy Markdown
Contributor Author

Coordinator sidecar review on exact head found two blocking issues:

  1. P1 false negative: executable SafeTensors/pickle polyglots scan clean.
  • returns before the protocol-less pickle security probe runs.
  • Reproduced with an 8,364-byte dual-valid artifact: full scan routes to , returns exit code , and reports zero issues, while direct reports critical findings and a mocked path invokes .
  • The new regression in only covers stack-invalid , not a dual-valid malicious positive.
  1. P2 regression: the same early return bypasses the existing TensorFlow ambiguity handling.
  • It returns before the arbitration later in the same function.
  • The existing regression at fails with .

Recommended fix shape: resolve SafeTensors/pickle overlap centrally and prefer pickle, dual scanning, or an explicit fail-closed / inconclusive result when a structurally valid dangerous pickle also exists; do not return before the existing TensorFlow guard runs.

@mldangelo-oai

Copy link
Copy Markdown
Contributor Author

Coordinator sidecar review on exact head 76bdfc0 found two blocking issues.

  1. P1 false negative: executable SafeTensors/pickle polyglots scan clean.
  • modelaudit/utils/file/detection.py returns safetensors before the protocol-less pickle security probe runs.
  • Reproduced with an 8,364-byte dual-valid artifact: full scan routes to safetensors, returns exit code 0, and reports zero issues, while direct PickleScanner reports critical S201 posix.system findings and a mocked pickle.loads path invokes system("true").
  • The new regression in tests/utils/file/test_filetype.py only covers stack-invalid STACK_GLOBAL; STOP, not a dual-valid malicious positive.
  1. P2 regression: the same early return bypasses the existing TensorFlow ambiguity handling.
  • It returns before the arbitration later in the same function.
  • The existing regression at tests/test_core.py:6779 fails with expected tensorflow_protobuf_routing_inconclusive, actual safetensors.

Recommended fix shape: resolve SafeTensors/pickle overlap centrally and prefer pickle, dual scanning, or an explicit fail-closed or inconclusive result when a structurally valid dangerous pickle also exists; do not return before the existing TensorFlow guard runs.

Route structurally valid protocol collisions to the scanners that can safely inspect them while keeping benign near-matches on the safetensors path. Preserve overlap findings across nested archives and scanner selection, with bounded pickle and compression probes.
…fetensors-protocol-routing

# Conflicts:
#	modelaudit/utils/file/detection.py
#	tests/utils/file/test_filetype.py
Keep reusable-context and unloaded-frozen-module tests independent of modules and meta-path finders loaded during full-suite collection.
@mldangelo-oai

Copy link
Copy Markdown
Contributor Author

Current head: 746f54001309c6d5b3f50cbbfb1f34caad90b109. Routing overlap fixes, mainline merge, and full validation are complete. @codex review

Comment thread modelaudit/scanners/archive_dispatch.py Fixed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@mldangelo-oai

Copy link
Copy Markdown
Contributor Author

Current head: e3203a7160dbb0df1547d49f82ac268fe9f8f326. Addressed the live code-quality thread and revalidated the SafeTensors routing slice. @codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@mldangelo-oai
mldangelo-oai merged commit 2f782ba into main Jun 10, 2026
32 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-safetensors-protocol-routing branch June 10, 2026 23:16
@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