Skip to content

Validate manifest input strictly and report unreadable manifest files cleanly - #6

Merged
ikeige-coder merged 1 commit into
ikeige-coder:mainfrom
Tet-9:fix/manifest-input-validation
Oct 5, 2026
Merged

ikeige-coder merged 1 commit into
ikeige-coder:mainfrom
Tet-9:fix/manifest-input-validation

Conversation

@Tet-9

@Tet-9 Tet-9 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Two fixes in the manifest path, which treats files as untrusted input.

  1. Name, glob, sha256, commit and shard checks used re.match with a trailing $, and $ also matches before a trailing newline, so values like "abc\n" passed. They now use fullmatch (manifest.py, sources.py, quantizers, holdout.py).
  2. load_manifest let FileNotFoundError, IsADirectoryError and UnicodeDecodeError escape, so excore check on a typo'd path or a binary file printed a raw traceback. They now surface as a ManifestError, which the CLI already reports cleanly with exit code 2.

Tests cover each case.

@Tet-9
Tet-9 requested a review from ikeige-coder as a code owner October 5, 2026 08:33

@ikeige-coder ikeige-coder left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved. re.match with a $ anchor accepts a trailing newline, so a manifest name like "evil\n" passed validation. Manifest names and patterns come from untrusted miner PRs, so fullmatch is the right fix. Clean errors for unreadable or non-UTF-8 manifests are also a good catch, and the tests cover both. Merging.

@ikeige-coder
ikeige-coder merged commit 8483c97 into ikeige-coder:main Oct 5, 2026
2 checks passed
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.

2 participants