fix(hub): prevent path traversal in sharded checkpoint loading

[goingforstudying-ctrl]

Fixes #46097

This PR addresses a path traversal vulnerability (CWE-22) in get_checkpoint_shard_files where a malicious model.safetensors.index.json could reference arbitrary files outside the model directory.

Changes

• Validation before path construction: Reject shard filenames containing .. or starting with path separators (/ or \)
• Path resolution verification: After os.path.join, use os.path.realpath to resolve symlinks and verify the resolved path stays within the model directory
• Comprehensive tests: Added 4 test cases covering:
  • Path traversal with ..
  • Absolute paths
  • Symlink escapes
  • Valid local shards (ensure normal operation still works)

Security Impact

This prevents arbitrary file reads when loading models from untrusted local directories. An attacker who can provide a malicious checkpoint index could previously access:

• System files (/etc/passwd, etc.)
• Other users' cached models
• API tokens stored in ~/.cache/huggingface/token

Testing

All new tests pass:

pytest tests/utils/test_hub_path_traversal.py -v

4 passed in 0.28s

[goingforstudying-ctrl]

Hi @vasqu, this PR fixes a path traversal vulnerability (CWE-22) in sharded checkpoint loading. The issue is that model.safetensors.index.json can contain malicious .. sequences, allowing arbitrary file reads. I have added validation to reject any weight file path containing .. or absolute paths, along with comprehensive tests. Would appreciate your review when you have a moment. Thanks!

[vasqu]

cc @Cyrilvallez

But tbh it cannot lead to any exploit afaik #46097 (comment) so three shouldnt be any need to do anything

[goingforstudying-ctrl]

@vasqu Thanks for the feedback and for linking the discussion. I understand the argument that malicious paths will simply fail at load time rather than succeed.

However, I still believe this fix has value for a few reasons:

1. Defense in depth — Failing late (during tensor deserialization) with an opaque file-not-found or format error is worse than failing early with a clear security message. Early validation gives users immediate, actionable feedback.

2. CWE-22 compliance — Path traversal is classified as a vulnerability regardless of whether the resulting path happens to be exploitable in a specific codebase. Security audits and automated scanners (SAST/DAST) flag unvalidated path construction as a finding.

3. Future-proofing — The checkpoint loading code may evolve. If a future change introduces symlink following, dynamic path resolution, or alternative storage backends, the traversal vector could become exploitable. Validating paths now prevents that class of bug from ever emerging.

4. Minimal cost — The check is a simple string validation with negligible runtime overhead, and the tests ensure it behaves correctly.

That said, if the maintainers' consensus is that this is unnecessary noise, I'm happy to close the PR. Just wanted to make the case for the fix.

[goingforstudying-ctrl]

@vasqu @Cyrilvallez Thanks for the feedback. I understand the position that the current behavior is safe in practice since malicious paths will fail at load time.

However, I'd still argue for merging this as a defense-in-depth measure:

1. Early failure is better than late failure — A clear ValueError with a descriptive message is more actionable for users than an opaque file-not-found or deserialization error that occurs deeper in the stack.

2. SAST/DAST compliance — Automated security scanners flag unvalidated path construction as a CWE-22 finding regardless of exploitability. Merging this prevents false positives in security audits.

3. Future-proofing — If the codebase ever introduces symlink following, dynamic path resolution, or alternative storage backends, this check prevents the traversal vector from becoming exploitable.

4. Minimal cost — The validation is a simple string check with negligible runtime overhead, and the tests ensure correctness.

That said, I'm happy to close this if the maintainers' consensus is that it's unnecessary. Just wanted to make the case.