Skip to content

[ML] Fail closed on incomplete TorchScript pre-load state-hook scan - #3149

Open
edsavage wants to merge 2 commits into
elastic:mainfrom
edsavage:fix/preload-scan-fail-closed
Open

[ML] Fail closed on incomplete TorchScript pre-load state-hook scan#3149
edsavage wants to merge 2 commits into
elastic:mainfrom
edsavage:fix/preload-scan-fail-closed

Conversation

@edsavage

@edsavage edsavage commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Close a bypass of the pre-load __setstate__/__getstate__ scan: PyTorch/miniz can truncate long zip entry names so getRecord fails; the previous fail-open skip allowed torch::jit::load to still run hooks.
  • Refuse archives with oversized record names or any unreadable record, and fatal with a distinct incomplete-scan message before load.
  • Add a long-path evasion fixture and unit coverage for the fail-closed path.

Follow-up to #3078. Related to elastic/security#12621.

Test plan

  • ml_test_pytorch_inference --run_test=CModelGraphValidatorTest/testPreLoadScan*
  • CI pytorch_inference unit tests on PR builds
  • Confirm benign/prepacked models still pass pre-load scan (existing tests)

Reject unreadable or oversized zip record names so path-length truncation
cannot skip the __setstate__/__getstate__ scan before torch::jit::load.

Co-authored-by: Cursor <cursoragent@cursor.com>
@elasticsearchmachine

Copy link
Copy Markdown

Pinging @elastic/ml-core (Team:ML)

@elasticsearchmachine

Copy link
Copy Markdown

Hi @edsavage, I've created a changelog YAML for you.

@edsavage edsavage added v9.4.6 v9.5.2 v8.19.21 auto-backport Automatically merge backport PRs when CI passes labels Aug 13, 2026
@edsavage
edsavage requested a lite review from Copilot August 13, 2026 00:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the TorchScript pre-load custom state-hook (__setstate__/__getstate__) archive scan in bin/pytorch_inference by failing closed when the scan cannot be completed safely, closing a path-length truncation evasion where torch::jit::load could still resolve and execute hooks.

Changes:

  • Make scanArchiveForCustomStateHooks fail closed (return an explicit sentinel) when encountering unreadable records or suspiciously long record names that can be truncated by PyTorch/miniz.
  • Update pre-load validation to fatal with a dedicated “incomplete scan” message before invoking torch::jit::load.
  • Add a long-path evasion fixture generator and unit test coverage for the new fail-closed behavior.

Reviewed changes

Copilot reviewed 2 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
docs/changelog/3149.yaml Adds changelog entry documenting the fail-closed scan behavior.
dev-tools/generate_malicious_models.py Adds generator for a long-path evasion malicious TorchScript fixture ZIP.
bin/pytorch_inference/unittest/CModelGraphValidatorTest.cc Adds unit tests for long-path evasion/incomplete scan handling and related assertions.
bin/pytorch_inference/Main.cc Treats incomplete scan sentinel as fatal before model load.
bin/pytorch_inference/CModelGraphValidator.h Documents fail-closed semantics and introduces sentinel + safe name-length constant.
bin/pytorch_inference/CModelGraphValidator.cc Implements fail-closed behavior for unreadable records and oversized record names.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Labels

auto-backport Automatically merge backport PRs when CI passes >bug :ml v8.19.21 v9.4.6 v9.5.2 v9.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants