From 368655f9b21dc061e9fa9c785f4a89c3e7be6a6d Mon Sep 17 00:00:00 2001 From: Tet-9 Date: Mon, 5 Oct 2026 09:33:09 +0100 Subject: [PATCH] Validate manifest input strictly and report unreadable manifest files cleanly --- excore/eval/holdout.py | 4 ++-- excore/manifest.py | 16 +++++++++++----- excore/quantizers/__init__.py | 4 ++-- excore/sources.py | 6 +++--- tests/test_manifest.py | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/excore/eval/holdout.py b/excore/eval/holdout.py index b1e35e3..e67b580 100644 --- a/excore/eval/holdout.py +++ b/excore/eval/holdout.py @@ -64,8 +64,8 @@ def epoch_now(today: _dt.date | None = None) -> int: def list_shards(root: str | Path) -> list[Path]: root = Path(root) - shards = [p for p in root.iterdir() if p.is_dir() and SHARD_RE.match(p.name) and (p / "reference.npz").exists()] - return sorted(shards, key=lambda p: int(SHARD_RE.match(p.name).group(1))) + shards = [p for p in root.iterdir() if p.is_dir() and SHARD_RE.fullmatch(p.name) and (p / "reference.npz").exists()] + return sorted(shards, key=lambda p: int(SHARD_RE.fullmatch(p.name).group(1))) def select_shard(root: str | Path | None = None, epoch: int | None = None) -> Path: diff --git a/excore/manifest.py b/excore/manifest.py index 2006626..5bc6d49 100644 --- a/excore/manifest.py +++ b/excore/manifest.py @@ -187,7 +187,7 @@ def parse_manifest(text: str, spec: ModelSpec, *, expected_track: str = TRACK_DE if raw.get("track") != expected_track: problems.append(f"track must be {expected_track!r}") name = raw.get("name") - if not isinstance(name, str) or not NAME_RE.match(name): + if not isinstance(name, str) or not NAME_RE.fullmatch(name): problems.append("name must match [a-z0-9][a-z0-9._-]{0,63}") name = "invalid" description = raw.get("description", "") @@ -223,7 +223,7 @@ def parse_manifest(text: str, spec: ModelSpec, *, expected_track: str = TRACK_DE for k in sorted(set(r) - _RULE_KEYS, key=str): problems.append(f"rule {i}: unknown key {k!r}") match = r.get("match") - if not isinstance(match, str) or not PATTERN_RE.match(match): + if not isinstance(match, str) or not PATTERN_RE.fullmatch(match): problems.append(f"rule {i}: match must be a glob over unit names (e.g. 'L*.mlp')") continue layers = None @@ -261,9 +261,15 @@ def parse_manifest(text: str, spec: ModelSpec, *, expected_track: str = TRACK_DE def load_manifest(path: str | Path, spec: ModelSpec, *, expected_track: str = TRACK_DEFAULT) -> Manifest: p = Path(path) - if p.stat().st_size > MAX_BYTES: - raise ManifestError([f"{p.name}: manifest exceeds {MAX_BYTES} bytes"]) - return parse_manifest(p.read_text(encoding="utf-8"), spec, expected_track=expected_track) + try: + if p.stat().st_size > MAX_BYTES: + raise ManifestError([f"{p.name}: manifest exceeds {MAX_BYTES} bytes"]) + text = p.read_text(encoding="utf-8") + except OSError as exc: + raise ManifestError([f"cannot read manifest {str(path)!r}: {exc.strerror or exc}"]) from None + except UnicodeDecodeError: + raise ManifestError([f"{p.name}: manifest is not valid UTF-8 text"]) from None + return parse_manifest(text, spec, expected_track=expected_track) # -- expansion ---------------------------------------------------------------- diff --git a/excore/quantizers/__init__.py b/excore/quantizers/__init__.py index e09d3e5..0010e86 100644 --- a/excore/quantizers/__init__.py +++ b/excore/quantizers/__init__.py @@ -127,7 +127,7 @@ def _ggml(fmt: Format): def register(q: Quantizer) -> Quantizer: - if not NAME_RE.match(q.name): + if not NAME_RE.fullmatch(q.name): raise QuantizerError(f"bad quantizer name {q.name!r}; use [a-z][a-z0-9_]{{0,31}}") if q.name in _REGISTRY and type(_REGISTRY[q.name]) is not type(q): raise QuantizerError(f"quantizer {q.name!r} is already registered by another class") @@ -153,7 +153,7 @@ def ensure_loaded(names: Iterable[str]) -> None: for name in names: if name in _REGISTRY: continue - if not NAME_RE.match(name): + if not NAME_RE.fullmatch(name): raise QuantizerError(f"bad quantizer name {name!r}") module = _MODULE_FOR.get(name, name) try: diff --git a/excore/sources.py b/excore/sources.py index 06a0e5d..ae0973f 100644 --- a/excore/sources.py +++ b/excore/sources.py @@ -50,9 +50,9 @@ def unpinned_reasons(lock: dict) -> list[str]: if not files: out.append("base model has no pinned files") for f in files: - if not HEX64.match(str(f.get("sha256") or "")): + if not HEX64.fullmatch(str(f.get("sha256") or "")): out.append(f"{f.get('name', '?')}: sha256 is not pinned") - if not COMMIT.match(str(lock.get("sources", {}).get("llama_cpp", {}).get("commit") or "")): + if not COMMIT.fullmatch(str(lock.get("sources", {}).get("llama_cpp", {}).get("commit") or "")): out.append("llama.cpp commit is not pinned") return out @@ -124,7 +124,7 @@ def pin(lock_path: str | Path, root: str | Path, *, repo: str | None, revision: base = lock["sources"]["base"] base.update(repo=repo, revision=revision, files=entries) if llama_cpp_commit: - if not COMMIT.match(llama_cpp_commit): + if not COMMIT.fullmatch(llama_cpp_commit): raise SourcesError("llama.cpp commit must be a full 40-character hash") lock["sources"]["llama_cpp"]["commit"] = llama_cpp_commit if is_pinned(lock): diff --git a/tests/test_manifest.py b/tests/test_manifest.py index a916675..3e85055 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -123,3 +123,35 @@ def test_default_only_checked_where_it_still_applies(spec): assert m.expand(spec)["lm_head"].format == "Q6_K" with pytest.raises(ManifestError, match="not a legal format"): parse("default: Q2_K\n", spec).expand(spec) + + +def test_name_and_match_must_match_in_full(spec): + # '$' matches before a trailing newline, so these used to be accepted + base = "schema: excore/manifest@1\ntrack: CPU-35B\ndefault: Q4_K\n" + with pytest.raises(ManifestError, match="name must match"): + parse_manifest(base + 'name: "abc\\n"\n', spec) + with pytest.raises(ManifestError, match="match must be a glob"): + parse_manifest(base + 'name: abc\nrules:\n - {match: "lm_head\\n", format: Q8_0}\n', spec) + + +def test_load_manifest_reports_unreadable_files_cleanly(spec, tmp_path): + with pytest.raises(ManifestError, match="cannot read manifest"): + load_manifest(tmp_path / "missing.yaml", spec) + with pytest.raises(ManifestError, match="cannot read manifest"): + load_manifest(tmp_path, spec) # a directory + binary = tmp_path / "binary.yaml" + binary.write_bytes(b"\xff\xfe\x00 not utf-8") + with pytest.raises(ManifestError, match="not valid UTF-8"): + load_manifest(binary, spec) + + +def test_pin_checks_use_the_whole_string(): + from excore.sources import COMMIT, HEX64 + assert HEX64.fullmatch("a" * 64) and not HEX64.fullmatch("a" * 64 + "\n") + assert COMMIT.fullmatch("b" * 40) and not COMMIT.fullmatch("b" * 40 + "\n") + + +def test_check_reports_a_missing_manifest_without_a_traceback(tmp_path, capsys): + from excore.cli import main + assert main(["check", str(tmp_path / "nope.yaml")]) == 2 + assert "cannot read manifest" in capsys.readouterr().err