Skip to content

raise instead of skipping model directories that cannot be listed - #652

Open
Support-itecz wants to merge 1 commit into
sigstore:mainfrom
Support-itecz:unlistable-dir-raise
Open

raise instead of skipping model directories that cannot be listed#652
Support-itecz wants to merge 1 commit into
sigstore:mainfrom
Support-itecz:unlistable-dir-raise

Conversation

@Support-itecz

Copy link
Copy Markdown
Contributor

Summary

check_file_or_directory accepts any path that stats as a directory, but glob("**/*") drops a subtree it cannot list without surfacing the error.

  • CPython swallows the failure inside the per-directory generator (PermissionError on 3.10-3.12, OSError in glob._Globber on 3.13+)
  • the manifest records only files, so an unreadable directory contributes zero entries and nothing raises
  • mkdir model/evil; cp backdoor.py model/evil/; chmod 0111 model/evil keeps backdoor.py out of the manifest and verify still reports success; mode 0111 still grants search, so whatever loads the model opens the planted file normally
  • both the file and shard serializers call this helper, so one check covers both walk loops

The docstring already said this call raises when there are permission errors. Signing or verifying a tree you cannot fully read now fails instead of quietly covering less than it appears to.

Checklist
  • All commits are signed-off, using DCO
  • All new code has docstrings and type annotations
  • All new code is covered by tests. Aim for at least 90% coverage. CI is configured to highlight lines not covered by tests.
  • Public facing changes are paired with documentation changes
  • Release note has been added to CHANGELOG.md if needed

@Support-itecz
Support-itecz requested review from a team as code owners July 29, 2026 18:06
check_file_or_directory accepts any path that stats as a directory, but
glob("**/*") drops a subtree it cannot list without surfacing the error.
CPython swallows the failure inside the per-directory generator
(PermissionError on 3.10-3.12, OSError in glob._Globber on 3.13+), and the
manifest records only files, so an unreadable directory contributes zero
entries and nothing raises.

That leaves a model tree that verifies clean while part of it was never
hashed: mkdir model/evil; cp backdoor.py model/evil/; chmod 0111 model/evil
keeps backdoor.py out of the manifest and verify still reports success. Mode
0111 still grants search, so whatever loads the model opens the planted file
normally.

The helper now scandirs any directory it is handed and raises when the
listing fails, which its own docstring already claimed it did for permission
errors. Both the file and shard serializers call it, so one check covers both
walk loops.

Signed-off-by: Itecz Solution <support@itecz.au>
@Support-itecz
Support-itecz force-pushed the unlistable-dir-raise branch from 29c4ccd to 91b5a85 Compare July 29, 2026 18:07
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