Fix cached_files silently returning stale file on read-only filesystem (EROFS) - #47852
Merged
Conversation
…m (EROFS) When the HF Hub cache lives on a read-only filesystem (EROFS, errno 30), `hf_hub_download` contacts the Hub, resolves a newer commit hash, and then fails with an `OSError(EROFS)` when trying to write the new snapshot pointer or blob to disk. That error is not caught inside `hf_hub_download` itself and bubbles up to the `except` block in `cached_files`. The existing guards there only re-raise for `RepositoryNotFoundError`, `RevisionNotFoundError`, `PermissionError` (EACCES, errno 13), and `ValueError`. Python maps EACCES to its own `PermissionError` subclass, but EROFS (errno 30) is a plain `OSError` that does **not** satisfy `isinstance(e, PermissionError)`. Without this fix it falls through to the stale-cache recovery block, which silently returns whatever old file was previously cached — even though the Hub has a newer revision. Fix: add an explicit `elif isinstance(e, OSError) and e.errno == errno.EROFS` guard that re-raises the error before the recovery block, giving callers (e.g. the CI `_with_tmpdir_cache_fallback` wrapper) a chance to catch it and retry the download against a writable temporary cache directory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
CI recapDashboard: View test results in Grafana |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the HF Hub cache is on a read-only filesystem (in some rare CI environments, a pre-populated cache may be mounted read-only, potentially for security reasons),
hf_hub_downloadstill contacts the Hub to resolve the latest commit hash. If a newer revision exists, it then tries to write the new snapshot pointer / blob to disk and fails withOSError: [Errno 30] Read-only file system (EROFS).On the
cached_filesside, EROFS is not handled: Python only promotes EACCES (errno 13) to its ownPermissionErrorsubclass, so the existingisinstance(e, PermissionError)guard catches EACCES but misses EROFS (errno 30), which is a plainOSError. Without this fix, the EROFS error falls through to the stale-cache recovery block, which silently returns the old cached file — even when the Hub has a newer revision.Fix
Add an explicit
elif isinstance(e, OSError) and e.errno == errno.EROFSguard that re-raises the error before the stale-cache recovery block, giving callers a chance to catch it and retry with a writable cache path (our patch inconftest.py).Why EROFS reaches
cached_filesbut EACCES doesn'tThe difference comes down to where the kernel rejects the write:
chmod 555/chown rootPermissionError(subclass ofOSError)mount -o ro/ read-only volumeOSErrorchmod/chown, the filesystem is still writable — the kernel reaches the DAC check, finds the user lacks permission on that path, and returns EACCES. Python promotes this toPermissionError, so the existing guard incached_filescatches it.MS_RDONLY) before any permission check. This returns a plainOSError(errno.EROFS)that Python does not promote toPermissionError, so it slips past the existing guard.Impact
This changes behaviour only when
cached_filesis called with a cache directory on a filesystem mounted read-only viamount -o ro(or equivalent kernel-level read-only mount). This is a rare setup — primarily used in our own CI for security/isolation purposes (pre-populated read-only cache volumes). The behaviour change for regular users (writable cache, network errors, permission-bit errors) is zero.Testing
Verified by: