diff --git a/CHANGELOG.md b/CHANGELOG.md index 395cec5..3d1dd95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,9 +59,11 @@ releases may contain breaking changes. folder-wrapped layouts, junk entries like `__MACOSX` ignored), `Lexicon.save_zip()` writes one (carrying media, `WritingSystems/`, and other package files through verbatim); `validate`, `stats`, `check-media`, and - `export` accept a `.zip` path on the CLI. Extraction rejects path-traversal - members and is capped (entry count and a 10 GiB uncompressed total) against - zip bombs. + `export` accept a `.zip` path on the CLI, and the streaming commands + (`stats`, `export`) extract only the `.lift` rather than the whole package. + Extraction is capped at 100,000 members and 10 GiB (the whole package for a + full extraction, the `.lift` alone for a streaming one), and refuses members + whose paths escape the extraction directory. - Validation: `validate_file()` / `iter_problems()` / `Lexicon.iter_problems()` returning a `Problem` stream, each carrying the file, entry, and line it concerns. RELAX NG layer with two documented diff --git a/docs/en/guides/cli.md b/docs/en/guides/cli.md index ae33f0b..7953a37 100644 --- a/docs/en/guides/cli.md +++ b/docs/en/guides/cli.md @@ -21,7 +21,7 @@ sil-lift export PATH [-o OUT] [--langs L] [--tsv] `sort` rewrites only the `.lift` file; companion `.lift-ranges` files are left untouched (sort those separately with the `RangesFile` API). -`validate`, `stats`, `check-media`, and `export` also accept a zipped LIFT package (a `.zip` in either layout — files at the archive root, or nested under one top-level folder); it is extracted to a temporary directory and discarded when the command finishes. +`validate`, `stats`, `check-media`, and `export` also accept a zipped LIFT package (a `.zip` in either layout — files at the archive root, or nested under one top-level folder); it is extracted to a temporary directory and discarded when the command finishes. The streaming commands `stats` and `export` extract only the `.lift` itself, so they stay cheap on media-heavy packages; `validate` and `check-media` need the whole folder and extract all of it. Examples: diff --git a/docs/en/guides/lift-export-interop.md b/docs/en/guides/lift-export-interop.md index 1edf3eb..1777e35 100644 --- a/docs/en/guides/lift-export-interop.md +++ b/docs/en/guides/lift-export-interop.md @@ -8,8 +8,12 @@ Writing LIFT is much easier than parsing it: an exporter only emits the subset o LIFT is usually moved around as a single `.zip` — FieldWorks and The Combine both import and export that way — so `sil-lift` reads and writes zipped packages directly, in either layout the ecosystem uses: the files at the archive root, or nested under one top-level folder. -- **Read:** `sil_lift.load("package.zip")` extracts to a temp directory, locates the single `.lift`, and loads it (companions and media resolve as usual). The `validate`, `stats`, `check-media`, and `export` CLI commands accept a `.zip` path too, so the gate below runs against a package as-is. Extraction is hardened against hostile archives — path-traversal members are refused, and the entry count and total uncompressed size (10 GiB) are capped against zip bombs. -- **Write:** `Lexicon.save_zip("out.zip", wrap_folder="MyDict")` packages the `.lift`, its `.lift-ranges`, and every other file in the source folder (media, `WritingSystems/`, `consent/`, ...) into a zip. `wrap_folder` defaults to a top-level folder named after the zip (the FieldWorks/Combine import convention); pass `False` for a flat archive. +- **Read:** `sil_lift.load("package.zip")` extracts to a temp directory, locates the single `.lift`, and loads it (companions and media resolve as usual). + - The `validate`, `stats`, `check-media`, and `export` CLI commands accept a `.zip` path too, so the gate below runs against a package as-is. + - `stats` and `export` stream, and extract only the `.lift` rather than the whole package — so they stay cheap on a media-heavy one, and the extraction limit applies to the `.lift` alone rather than to everything beside it. + - Extraction is capped at 10 GiB and 100,000 members; a package over either limit is refused with a `LiftParseError`, as is one whose member paths escape the extraction directory. +- **Write:** `Lexicon.save_zip("out.zip", wrap_folder="MyDict")` packages the `.lift`, its `.lift-ranges`, and every other file in the source folder (media, `WritingSystems/`, `consent/`, ...) into a zip. + - `wrap_folder` defaults to a top-level folder named after the zip (the FieldWorks/Combine import convention); pass `False` for a flat archive. The `.lift` and `.lift-ranges` keep their byte-fidelity inside the package; the zip container itself is not byte-reproducible. diff --git a/src/sil_lift/_zip.py b/src/sil_lift/_zip.py index 7ed90c4..c986fab 100644 --- a/src/sil_lift/_zip.py +++ b/src/sil_lift/_zip.py @@ -4,9 +4,14 @@ (both produced and accepted by FieldWorks and The Combine): the files at the archive root (``foo.lift`` beside ``WritingSystems/``, ``audio/``, ...), or nested one level under a single folder (``Foo/foo.lift`` ...). This module -extracts to a temporary directory, locates the single ``.lift`` (its parent is -the package root), and hands off to the ordinary path-based reader/writer — so -media resolution, companion discovery, and byte-fidelity all work unchanged. +locates the single ``.lift`` member (its parent is the package root), extracts +to a temporary directory, and hands off to the ordinary path-based +reader/writer — so media resolution, companion discovery, and byte-fidelity all +work unchanged. + +Streaming reads (:func:`lift_source`) extract that one member and nothing else: +they resolve neither companions nor media, so writing the rest of an +audio-heavy package to disk would cost gigabytes that nothing goes on to read. The archive *container* is not byte-reproducible (zip carries timestamps, compression, and ordering); the guarantee is at the file level — the ``.lift`` @@ -20,14 +25,14 @@ import tempfile import zipfile from contextlib import contextmanager -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import TYPE_CHECKING from ._errors import LiftParseError from ._model import Lexicon if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterable, Iterator __all__ = ["load_zip", "save_zip"] @@ -40,19 +45,60 @@ _EXTRACT_CHUNK = 1 << 20 # 1 MiB -def _size_limit_message(zip_path: Path) -> str: +def _size_limit_message(zip_path: Path, member: str | None = None) -> str: + """The size-cap refusal, for the package as a whole or for one member.""" limit_gib = _MAX_UNCOMPRESSED_BYTES / 1024**3 - return f"{zip_path.name}: uncompressed size exceeds the {limit_gib:.0f} GiB limit" + if member is None: + return f"{zip_path.name}: uncompressed size exceeds the {limit_gib:.0f} GiB limit" + return f"{zip_path.name}: {member!r} alone exceeds the {limit_gib:.0f} GiB limit" + + +def _select_lift_member(names: Iterable[str]) -> str: + """The single ``.lift`` member of an archive listing; its parent is the root. + + Handles both the flat and folder-wrapped layouts, and ignores junk such as + ``__MACOSX`` and dotfile entries that some zip tools add. One path stored + multiple times counts once — some writers add a second listing entry rather + than replacing the first, and extraction overwrites, so what lands on disk + is a single file. + + The suffix match is case-insensitive, so a ``.LIFT`` member resolves the + same way on every platform rather than only where the filesystem happens to + case-fold. How such a name then finds its companion is not this layer's + concern: it is the same question a case-variant ``.lift`` in a plain folder + raises, and ``Lexicon._resolve_ranges`` is where it is answered. + """ + lifts = list( + dict.fromkeys( # de-duplicate, preserving listing order + name + for name in names + if name.lower().endswith(".lift") + and not any( + part == "__MACOSX" or part.startswith(".") for part in PurePosixPath(name).parts + ) + ) + ) + if not lifts: + raise LiftParseError("no .lift file found in the archive") + if len(lifts) > 1: + found = ", ".join(sorted(PurePosixPath(name).name for name in lifts)) + raise LiftParseError(f"multiple .lift files found in the archive: {found}") + return lifts[0] -def _safe_extract(zip_path: Path, dest: Path) -> None: - """Extract ``zip_path`` into ``dest``, defending against malicious archives. +def _safe_extract(zip_path: Path, dest: Path, *, only_lift: bool = False) -> str: + """Extract ``zip_path`` into ``dest``; returns the ``.lift`` member's name. Path-traversal members (``..`` or absolute, resolved against ``dest``) are - rejected, the entry count is capped, and the total uncompressed size is - capped at ``_MAX_UNCOMPRESSED_BYTES`` — checked against the declared sizes - up front, then again while streaming each member to disk, since a crafted - archive's declared size can lie. + rejected and the entry count is capped, both over the whole listing however + much of it gets written. Bytes written are capped as they stream, since a + crafted archive's declared size can lie. + + ``only_lift`` narrows the write to the ``.lift`` itself, which is all a + streaming read needs. The whole listing's declared sizes then have nothing + to guard, so the up-front check covers that one member instead: a zip bomb + that declares itself is refused with nothing written, and one that lies + about its size is still caught as it streams. """ dest_root = dest.resolve() try: @@ -65,7 +111,15 @@ def _safe_extract(zip_path: Path, dest: Path) -> None: raise LiftParseError( f"{zip_path.name}: unsafe path in archive: {info.filename!r}" ) - if sum(info.file_size for info in infos) > _MAX_UNCOMPRESSED_BYTES: + member = _select_lift_member(info.filename for info in infos) + if only_lift: + # getinfo() takes the last listing entry for a path stored + # multiple times, the one that's left after full extraction. + lift_info = archive.getinfo(member) + if lift_info.file_size > _MAX_UNCOMPRESSED_BYTES: + raise LiftParseError(_size_limit_message(zip_path, member)) + infos = [lift_info] + elif sum(info.file_size for info in infos) > _MAX_UNCOMPRESSED_BYTES: raise LiftParseError(_size_limit_message(zip_path)) written = 0 for info in infos: @@ -78,47 +132,29 @@ def _safe_extract(zip_path: Path, dest: Path) -> None: while chunk := source.read(_EXTRACT_CHUNK): written += len(chunk) if written > _MAX_UNCOMPRESSED_BYTES: - raise LiftParseError(_size_limit_message(zip_path)) + raise LiftParseError( + _size_limit_message(zip_path, info.filename if only_lift else None) + ) sink.write(chunk) except zipfile.BadZipFile as exc: raise LiftParseError(f"{zip_path.name}: not a valid zip archive: {exc}") from exc - - -def _find_lift_root(tree: Path) -> Path: - """The single ``.lift`` file in an extracted tree; its parent is the root. - - Handles both the flat and folder-wrapped layouts, and ignores junk such as - ``__MACOSX`` and dotfile entries that some zip tools add. - """ - lifts = [ - p - for p in tree.rglob("*.lift") - if p.is_file() - and not any( - part == "__MACOSX" or part.startswith(".") for part in p.relative_to(tree).parts - ) - ] - if not lifts: - raise LiftParseError("no .lift file found in the archive") - if len(lifts) > 1: - names = ", ".join(sorted(p.name for p in lifts)) - raise LiftParseError(f"multiple .lift files found in the archive: {names}") - return lifts[0] + return member @contextmanager def lift_source(path: Path) -> Iterator[Path]: """Yield a ``.lift`` path for ``path``, extracting a ``.zip`` to a temp dir. - A non-zip path is yielded unchanged. A zip is extracted for the duration of - the ``with`` block, then removed — for streaming callers that only read. + A non-zip path is yielded unchanged. From a zip only the ``.lift`` member + is extracted, for the duration of the ``with`` block — all a streaming + caller reads, since it resolves neither companions nor media. """ if path.suffix.lower() != ".zip": yield path return with tempfile.TemporaryDirectory(prefix="sil-lift-") as tmp: - _safe_extract(path, Path(tmp)) - yield _find_lift_root(Path(tmp)) + member = _safe_extract(path, Path(tmp), only_lift=True) + yield Path(tmp) / member def load_zip(path: Path, *, resolve_ranges: bool = True) -> Lexicon: @@ -130,9 +166,8 @@ def load_zip(path: Path, *, resolve_ranges: bool = True) -> Lexicon: """ tmp = tempfile.TemporaryDirectory(prefix="sil-lift-") try: - _safe_extract(path, Path(tmp.name)) - lift_path = _find_lift_root(Path(tmp.name)) - lexicon = Lexicon.load(lift_path, resolve_ranges=resolve_ranges) + member = _safe_extract(path, Path(tmp.name)) + lexicon = Lexicon.load(Path(tmp.name) / member, resolve_ranges=resolve_ranges) except BaseException: tmp.cleanup() raise diff --git a/tests/test_zip.py b/tests/test_zip.py index 0bbb053..2a35ccf 100644 --- a/tests/test_zip.py +++ b/tests/test_zip.py @@ -1,11 +1,12 @@ import json import zipfile -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest import sil_lift from sil_lift._cli import main +from sil_lift._zip import lift_source CORPUS_DIR = Path(__file__).parent / "corpus" PAIR_DIR = CORPUS_DIR / "ranges" # test20080407.lift + companion, fully clean @@ -68,6 +69,19 @@ def test_zip_with_no_lift_errors(tmp_path: Path) -> None: sil_lift.load(path) +def test_zip_tolerates_one_lift_stored_twice(tmp_path: Path) -> None: + # Some writers add a second listing entry rather than replacing the first, + # so the same path appears twice; extraction overwrites, leaving one file. + path = tmp_path / "dup.zip" + with zipfile.ZipFile(path, "w") as archive: + for arcname, src in PAIR.items(): + archive.write(src, arcname) + archive.write(PAIR_DIR / "test20080407.lift", "test20080407.lift") + assert len(sil_lift.load(path).entries) == 1 + with lift_source(path) as lift_path: + assert lift_path.is_file() + + def test_zip_with_multiple_lift_errors(tmp_path: Path) -> None: path = tmp_path / "two.zip" with zipfile.ZipFile(path, "w") as archive: @@ -131,6 +145,101 @@ def test_cli_export_accepts_zip(tmp_path: Path) -> None: assert any("abat" in row for row in rows[1:]) # the entry id in full-entry.lift +_PKG = PurePosixPath("Pkg") # the package's wrapper folder inside the archive +_PKG_LIFT = _PKG / "test20080407.lift" + + +def _package_with_media(dst: Path, *, media: bytes = b"\0" * 8192) -> Path: + with zipfile.ZipFile(dst, "w") as archive: + for arcname, src in PAIR.items(): + archive.write(src, (_PKG / arcname).as_posix()) + archive.writestr((_PKG / "audio" / "big.wav").as_posix(), media) + return dst + + +def test_lift_source_extracts_only_the_lift(tmp_path: Path) -> None: + package = _package_with_media(tmp_path / "pkg.zip") + with lift_source(package) as lift_path: + root = lift_path.parents[len(_PKG_LIFT.parts) - 1] # up out of the member path + assert root.name.startswith("sil-lift-") # the temp dir, not somewhere above it + written = sorted(p.relative_to(root).as_posix() for p in root.rglob("*") if p.is_file()) + assert written == [_PKG_LIFT.as_posix()] # no media, no companion + + +def test_lift_source_skips_the_whole_package_size_cap( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # The cap guards bytes written to disk, and a streaming read writes one + # member — so a package whose media is far larger than the limit still + # streams, while the full extraction behind load() refuses it. + package = _package_with_media(tmp_path / "pkg.zip") + monkeypatch.setattr("sil_lift._zip._MAX_UNCOMPRESSED_BYTES", 4000) + with lift_source(package) as lift_path: + assert lift_path.is_file() + with pytest.raises(sil_lift.LiftParseError, match="exceeds"): + sil_lift.load(package) + + +def test_lift_source_caps_an_oversized_lift_member( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + package = _package_with_media(tmp_path / "pkg.zip") + monkeypatch.setattr("sil_lift._zip._MAX_UNCOMPRESSED_BYTES", 100) # under the .lift's size + with pytest.raises(sil_lift.LiftParseError, match="exceeds"), lift_source(package): + pass + + +def test_lift_source_refuses_a_declared_oversized_lift_unextracted( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + package = tmp_path / "declared.zip" + with zipfile.ZipFile(package, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr("Pkg/big.lift", b"" * 10) + archive.getinfo("Pkg/big.lift").file_size = 10**9 + monkeypatch.setattr("sil_lift._zip._MAX_UNCOMPRESSED_BYTES", 4000) + with pytest.raises(sil_lift.LiftParseError, match=r"big\.lift' alone"), lift_source(package): + pass + + +def test_lift_source_counts_a_lift_stored_twice_once( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + package = tmp_path / "dup.zip" + with zipfile.ZipFile(package, "w") as archive: + for _ in range(2): + archive.write(PAIR_DIR / "test20080407.lift", "Pkg/test20080407.lift") + size = (PAIR_DIR / "test20080407.lift").stat().st_size + monkeypatch.setattr("sil_lift._zip._MAX_UNCOMPRESSED_BYTES", 2 * size - 1) + with lift_source(package) as lift_path: # one copy written, not two + assert lift_path.stat().st_size == size + + +def test_lift_source_rejects_path_traversal(tmp_path: Path) -> None: + package = tmp_path / "evil.zip" + with zipfile.ZipFile(package, "w") as archive: + archive.write(PAIR_DIR / "test20080407.lift", "Pkg/test20080407.lift") + archive.writestr("../evil.txt", b"x") # never extracted, still refused + with pytest.raises(sil_lift.LiftParseError, match="unsafe path"), lift_source(package): + pass + + +def test_lift_source_yields_the_path_extraction_wrote(tmp_path: Path) -> None: + # The yielded path is built from the member name, not found on disk, so + # normalizing a separator on one side alone breaks it. POSIX only: zipfile + # rewrites os.sep to "/", so a backslash never reaches the code on Windows. + package = tmp_path / "backslash.zip" + source = PAIR_DIR / "test20080407.lift" + with zipfile.ZipFile(package, "w") as archive: + archive.write(source, "Pkg\\test20080407.lift") + with lift_source(package) as lift_path: + assert lift_path.read_bytes() == source.read_bytes() + + +def test_lift_source_passes_a_plain_lift_through() -> None: + with lift_source(PAIR_DIR / "test20080407.lift") as lift_path: + assert lift_path == PAIR_DIR / "test20080407.lift" + + def test_save_zip_roundtrip_wrapped(tmp_path: Path) -> None: lex = sil_lift.load(_make_zip(tmp_path / "src.zip", PAIR, wrap="Src")) out = tmp_path / "out.zip"