Skip to content

Fix external data validation for Hugging Face Hub shared blob store - #34099

Open
Tianlei Wu (tianleiwu) wants to merge 1 commit into
mainfrom
tlwu/fix-hf-shared-blobs-33950
Open

Tianlei Wu (tianleiwu) wants to merge 1 commit into
mainfrom
tlwu/fix-hf-shared-blobs-33950

Conversation

@tianleiwu

Copy link
Copy Markdown
Contributor

Description

Since huggingface_hub 1.32, the Hugging Face Hub cache stores Xet files once per cache in a blob store shared across repos (<cache>/blobs/<hash prefix>/<hash>), and the repo's blobs/<etag> becomes a relative symlink to it. A model and its external data in one snapshot can then resolve into different folders (for example blobs/28/… and blobs/d4/…, or one in the repo's blobs/ and one in the shared store). The symlink fallback from #27374 only accepts external data next to the resolved model, so models with external data loaded from a default hf_hub_download path fail with External data path escapes model directory. This PR teaches both external data validators about the shared store without trusting anything outside the cache's blob folders.

Summary of Changes

Validation (onnxruntime/core/framework/tensorprotoutils.cc)

New Pass 3 in ValidateExternalDataPath, reached only when the model is a symlink and Passes 1–2 fail. If the canonical model directory is in a snapshot of a cache repo (<cache>/<type>s--<repo id>/snapshots/<revision>/…, also from a subfolder such as onnx/), external data is accepted when:

  1. its path does not leave the model directory before symlinks are resolved (no .. escape), and
  2. both the model and the external data resolve to files under the blob stores of that cache: the repo's blobs/ folder or the cache-wide blobs/ folder.

ValidateOpenedExternalDataPath applies the same blob-store check to the canonical path of the opened file, so the opened-file validation from #33063 also covers the new case. The cost is one extra canonicalization, and only after the existing passes fail.

File Change
onnxruntime/core/framework/tensorprotoutils.cc Add IsInHuggingFaceCacheBlobStores and use it in both validators.
onnxruntime/test/framework/tensorutils_test.cc HuggingFaceCachePathValidationTest: 11 tests on a cache built like huggingface_hub does, with two-hop relative symlinks (6 accepted layouts, 5 rejected).
onnxruntime/test/python/onnxruntime_test_python_symlink_data.py End-to-end InferenceSession test for the shared store and for a mixed repo-local/shared layout.

Security notes

  • Paths are compared in canonical form. A canonical path has no symlinks, so a file whose canonical path is under <cache>/blobs is stored in that real folder. If a blobs/ folder is itself a symlink or junction, nothing matches (huggingface_hub also never uses a symlinked blobs/ folder as the shared store).
  • The model itself must resolve into the blob stores. This rejects symlinked models that point outside the cache, and symlink/.. tricks in the model path.
  • The no-.. requirement keeps a malicious model from reaching other cached repos' files. In a real cache, the entries of a snapshot folder are symlinks that huggingface_hub creates to files of the same repo.
  • Someone who can write into the cache root gains nothing new. With the existing Pass 2 (for example ORT 1.31.1), they can already make a model read any blob by putting the model file in the same shard folder (verified). huggingface_hub also gives the store and shard folders the same permissions as the cache root.
  • Unchanged: empty, absolute, and in-memory-tag locations are still rejected before any fallback, and Passes 1–2 behave as before.

Comparison with #33974

#33974 targets the same issue. It also checks the .huggingface-shared-blobs marker (version 1) and blob names (hex, 40/64 characters, 2-character shard), and requires a models-- repo folder. These checks are not security boundaries, since anyone who can create the layout can also create the marker and names. They do tie ORT to huggingface_hub internals, so a future layout change would break loading again. I built both and loaded the same synthetic caches:

Scenario #33974 This PR
Shared store, model and data in different shards loads loads
Mixed repo-local/shared blobs (both directions) loads loads
datasets--… repo fails loads
Model folder reached through a symlinked directory fails loads
Shared store marker version other than 1 fails loads
Location ../x; data symlink outside the cache; symlinked blobs/ rejected rejected

Motivation and Context

Fixes #33950. Downstream projects currently work around it by pinning huggingface_hub<=1.31, setting HF_HUB_DISABLE_SHARED_BLOBS=1, or copying files out of the cache.

Testing

  • Real cache: reproduced the issue with onnxruntime==1.31.1 and huggingface_hub==2.4.0 (onnx-community/sam2.1-hiera-tiny-ONNX, fresh HF_HOME). With this PR, the same cache loads. The two mixed layouts created by downloading one of the files with HF_HUB_DISABLE_SHARED_BLOBS=1 also fail with 1.31.1 and load with this PR.
  • onnxruntime_test_all --gtest_filter='HuggingFaceCachePathValidationTest.*:PathValidationTest.*:GetExtDataFromTensorProtoTest.*': all pass. Without the tensorprotoutils.cc change, the 6 tests for accepted layouts fail. Removing any one of the conditions (no .. escape, model in a blob store, blobs/ folder bound, repo folder name, symlinked blobs/, opened-file check) makes the targeted test fail.
  • python -m pytest onnxruntime/test/python/onnxruntime_test_python_symlink_data.py: 4 passed (plus 2 subtests).
  • Full onnxruntime_test_all on Linux x64 (CPU, Release): 2897 passed. lintrunner: no issues.
  • Not run locally: Windows and WASM. The new tests skip when symlinks cannot be created, and the load test is excluded on WASM like the other external-file tests in that file.

Checklist

  • Tests added/updated
  • No breaking changes
  • Documentation updated (not applicable)

huggingface_hub >= 1.32 stores Xet files once per cache in
<cache>/blobs/<hash prefix>/<hash> and makes the repo's blobs/<etag> a
symlink to it. A model and its external data in a snapshot can then
resolve into different folders, so the symlink fallback, which only
accepts data next to the resolved model, fails with "External data path
escapes model directory".

Also accept external data of a symlinked model in a snapshot of a Hugging
Face Hub cache repo when the data path stays under the model directory
before symlinks are resolved and both the model and the data resolve into
the blob stores of that cache (the repo's blobs/ folder or the shared
blobs/ folder). Apply the same check to the opened file.

Fixes #33950

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 11, 2026 06:22

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.

🔵 Needs a closer look

The fallback expands external-data access into cache-wide blob stores, requiring human security review of cache-layout trust assumptions and isolation between repositories.

No open issues 🎉

What changed in this PR

This PR addresses #33950 by extending ONNX Runtime’s external-data validation to support Hugging Face Hub’s shared blob store, where a model and its weights can resolve into different directories.

Changes:

  • Adds cache-aware fallback checks to both path validation and opened-file validation, preserving the existing validation passes.
  • Adds C++ tests for shared and mixed blob layouts, nested snapshots, and rejected paths.
  • Adds Python end-to-end inference coverage for shared-store and mixed repo-local/shared layouts.

No actionable findings were identified.

File Description
onnxruntime/​test/​python/​onnxruntime_test_python_symlink_data.py Adds synthetic cache helpers and end-to-end inference tests.
onnxruntime/​test/​framework/​tensorutils_test.cc Adds cache-layout acceptance, rejection, and external-data loading tests.
onnxruntime/​core/​framework/​tensorprotoutils.cc Recognizes cache blob stores in pre-open and opened-file validation.

Review effort: Balanced

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants