From 5bb25245a56d1eca5c48f4c98d591fc196017836 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 6 Jun 2026 14:33:24 -0400 Subject: [PATCH] =?UTF-8?q?fix(archive):=20magic-byte=20archive=20detectio?= =?UTF-8?q?n=20so=20mis-named=20archives=20extract=20=E2=80=94=20#713=20pa?= =?UTF-8?q?rt=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patreon attachment downloads land with sanitized URL-blob filenames (01_https___www.patreon.com_media-u_v3_) whose Path.suffix is junk, never .zip — so the extension-only is_archive() filed them as opaque PostAttachments and never extracted them ("No images attached to this post"). Add archive_extractor.detect_archive_format() — extension first, then magic-byte sniff (zipfile.is_zipfile + RAR/7z signatures). is_archive(), extract_archive(), and safe_probe._inspect_archive() (the bomb-guard) all route through it, so a mis-named/extension-less archive is now detected, bomb-guarded, integrity-tested, AND extracted regardless of filename. Stops new ones; part 2 re-extracts the already-imported backlog. Tests: mis-named zip detected + extracted; non-archive dotted name not misdetected; _inspect_archive on a mis-named zip; signature updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/app/services/archive_extractor.py | 46 ++++++++++++++++++++--- backend/app/utils/safe_probe.py | 22 ++++++----- tests/test_archive_extractor.py | 18 +++++++++ tests/test_safe_probe.py | 12 +++++- 4 files changed, 83 insertions(+), 15 deletions(-) diff --git a/backend/app/services/archive_extractor.py b/backend/app/services/archive_extractor.py index 45babef..d6b6566 100644 --- a/backend/app/services/archive_extractor.py +++ b/backend/app/services/archive_extractor.py @@ -16,9 +16,45 @@ log = logging.getLogger(__name__) ARCHIVE_EXTS = {".zip", ".cbz", ".rar", ".7z"} +# Magic-byte signatures, so an archive with a mangled / extension-less filename +# is still recognised. Patreon attachment download URLs sanitize to names like +# `01_https___www.patreon.com_media-u_v3_131083093`, whose `Path.suffix` is junk +# (`.com_media-u_v3_131083093`), never `.zip` — an extension-only gate filed +# those as opaque PostAttachments and NEVER extracted them (operator-flagged +# 2026-06-06). Detection is by extension first (cheap), then header sniff. +_RAR_MAGIC = b"Rar!\x1a\x07" +_7Z_MAGIC = b"7z\xbc\xaf\x27\x1c" + + +def detect_archive_format(path: Path) -> str | None: + """Return "zip" | "rar" | "7z" for an archive, else None. + + Trusts a known extension first, then falls back to magic-byte sniffing so a + mis-named or extension-less archive is still handled. (zip covers .cbz too.) + """ + ext = Path(path).suffix.lower() + if ext in (".zip", ".cbz"): + return "zip" + if ext == ".rar": + return "rar" + if ext == ".7z": + return "7z" + try: + if zipfile.is_zipfile(path): + return "zip" + with open(path, "rb") as fh: + head = fh.read(8) + except OSError: + return None + if head.startswith(_RAR_MAGIC): + return "rar" + if head.startswith(_7Z_MAGIC): + return "7z" + return None + def is_archive(path: Path) -> bool: - return Path(path).suffix.lower() in ARCHIVE_EXTS + return detect_archive_format(path) is not None @contextmanager @@ -32,16 +68,16 @@ def extract_archive(path: Path): members: list[tuple[str, Path]] = [] try: try: - ext = Path(path).suffix.lower() - if ext in (".zip", ".cbz"): + fmt = detect_archive_format(path) + if fmt == "zip": with zipfile.ZipFile(path) as zf: zf.extractall(base) - elif ext == ".rar": + elif fmt == "rar": import rarfile with rarfile.RarFile(path) as rf: rf.extractall(base) - elif ext == ".7z": + elif fmt == "7z": import py7zr with py7zr.SevenZipFile(path, "r") as zf: diff --git a/backend/app/utils/safe_probe.py b/backend/app/utils/safe_probe.py index d8e5360..beb0bc1 100644 --- a/backend/app/utils/safe_probe.py +++ b/backend/app/utils/safe_probe.py @@ -149,9 +149,8 @@ def _run_probe(path_str: str) -> tuple[str, str | None]: call the same code path. """ path = Path(path_str) - ext = path.suffix.lower() try: - total, test_bad = _inspect_archive(path, ext) + total, test_bad = _inspect_archive(path) except Exception as exc: # noqa: BLE001 — clean rejection return ("error", f"{type(exc).__name__}: {exc}") if total is not None and total > MAX_ARCHIVE_UNCOMPRESSED_BYTES: @@ -162,24 +161,29 @@ def _run_probe(path_str: str) -> tuple[str, str | None]: return ("ok", None) -def _inspect_archive(path: Path, ext: str): +def _inspect_archive(path: Path): """Return (total_uncompressed_bytes | None, first_bad_member | None) - for the archive. Format-specific; raises on a structurally-broken - container (caught by the child as a clean rejection).""" - if ext in (".zip", ".cbz"): + for the archive. Format detected by extension OR magic bytes (so a + mis-named archive is still bomb-guarded + integrity-tested, matching the + extractor's gate); raises on a structurally-broken container (caught by the + child as a clean rejection).""" + from ..services.archive_extractor import detect_archive_format + + fmt = detect_archive_format(path) + if fmt == "zip": import zipfile with zipfile.ZipFile(path) as zf: total = sum(zi.file_size for zi in zf.infolist()) return total, zf.testzip() - if ext == ".rar": + if fmt == "rar": import rarfile with rarfile.RarFile(path) as rf: total = sum(getattr(ri, "file_size", 0) for ri in rf.infolist()) rf.testrar() return total, None - if ext == ".7z": + if fmt == "7z": import py7zr with py7zr.SevenZipFile(path, "r") as zf: @@ -187,5 +191,5 @@ def _inspect_archive(path: Path, ext: str): total = getattr(info, "uncompressed", None) ok = zf.test() # True / None when all members pass return total, (None if ok in (True, None) else "7z test reported corruption") - # Unknown extension — nothing to test; treat as clean. + # Not a recognised archive — nothing to test; treat as clean. return None, None diff --git a/tests/test_archive_extractor.py b/tests/test_archive_extractor.py index 24e3a95..420655c 100644 --- a/tests/test_archive_extractor.py +++ b/tests/test_archive_extractor.py @@ -39,6 +39,24 @@ def test_cbz_alias(tmp_path): assert [n for n, _ in members] == ["p1.jpg"] +def test_misnamed_zip_detected_and_extracted(tmp_path): + """A real zip whose filename has no usable extension (Patreon attachment + URL-blob name) is detected by magic bytes and its members extracted — + previously it was filed as an opaque attachment and never opened.""" + z = tmp_path / "01_https___www.patreon.com_media-u_v3_131083093" + _zip(z, {"a.jpg": b"img", "b.png": b"img2"}) + assert is_archive(z) + with extract_archive(z) as members: + assert sorted(n for n, _ in members) == ["a.jpg", "b.png"] + + +def test_non_archive_with_dotted_name_is_not_archive(tmp_path): + """A non-archive file with a dotted/extension-less name isn't misdetected.""" + f = tmp_path / "01_https___www.patreon.com_media-u_v3_999" + f.write_bytes(b"\x89PNG\r\n\x1a\n not really but not a zip") + assert not is_archive(f) + + def test_corrupt_archive_yields_nothing(tmp_path): bad = tmp_path / "broken.zip" bad.write_bytes(b"not a zip at all") diff --git a/tests/test_safe_probe.py b/tests/test_safe_probe.py index 98a67b3..85e5c85 100644 --- a/tests/test_safe_probe.py +++ b/tests/test_safe_probe.py @@ -40,11 +40,21 @@ def test_probe_archive_corrupt_zip_clean_rejection(tmp_path): def test_inspect_archive_reports_size_and_clean_integrity(tmp_path): z = tmp_path / "sized.zip" _zip(z, {"a.txt": b"x" * 100, "b.txt": b"y" * 50}) - total, bad = safe_probe._inspect_archive(z, ".zip") + total, bad = safe_probe._inspect_archive(z) assert total == 150 assert bad is None +def test_inspect_archive_detects_misnamed_zip(tmp_path): + """A zip with a mangled / extension-less name (Patreon URL-blob attachment) + is still bomb-guarded + integrity-tested via magic-byte detection.""" + z = tmp_path / "01_https___www.patreon.com_media-u_v3_131083093" + _zip(z, {"a.txt": b"x" * 100}) + total, bad = safe_probe._inspect_archive(z) + assert total == 100 + assert bad is None + + def test_run_probe_bomb_guard(tmp_path, monkeypatch): """In-process call to _run_probe so the monkeypatched cap takes effect. The public probe_archive runs in a subprocess which re-imports the