diff --git a/plans/260811-1411-telegram-group-media-exporter/plan.md b/plans/260811-1411-telegram-group-media-exporter/plan.md index a146286..70e11f7 100644 --- a/plans/260811-1411-telegram-group-media-exporter/plan.md +++ b/plans/260811-1411-telegram-group-media-exporter/plan.md @@ -113,7 +113,7 @@ but the end-to-end proof is phase 7 against a live group. - [x] Media that is deleted, expired, or unavailable is recorded and **skipped**, and the run continues - [x] `--verbose` leaves the `telethon` logger at `WARNING`; `api_hash` and phone absent from captured output - [x] Session file and every sibling are mode `0600`; export tree `0700` -- [x] `pytest` green (180 tests); `pip install -r requirements.txt -e .` from a clean venv yields a working `tg-export` +- [x] `pytest` green (204 tests, and green under `python -O`); `pip install -r requirements.txt -e .` from a clean venv yields a working `tg-export` ## Phases @@ -153,6 +153,19 @@ but the end-to-end proof is phase 7 against a live group. plan claimed per-root locking made concurrent exports work; it does, but only with that second flag. Documented in the README and in the exit-3 message. +11. **What consecutive-failure threshold should stop a run?** `completed_at` is + now withheld when every file failed, but a run against a dead session still + walks the entire history failing each message before returning. A circuit + breaker would stop it early; the threshold is a judgment call, because a dead + session and a legitimately corrupt region of history look identical from + inside `download_one`. **Not invented unilaterally.** +12. **Should a server-declared 0-byte document be exportable?** The zero-byte + guard now refuses `actual == 0` for every kind, which is right for an empty + photo variant and for ENOSPC junk, but makes a genuinely empty uploaded + document unexportable — it costs three attempts and a FAILED record. If + Telegram permits such uploads, `declared == 0 and actual == 0` could be + accepted without weakening the guard. Needs a fact about Telegram that phase 1 + could answer. ## Dependencies No cross-plan dependencies. @@ -209,7 +222,7 @@ Evidence standard adapted for a greenfield repo: reviewers cited plan-file and r ### Session 1 — 2026-08-12/13 — phases 2–6 -Phases 2–6 implemented and green: **180 tests**, all offline against synthetic +Phases 2–6 implemented and green: **204 tests**, all offline against synthetic message stubs. `pip install -r requirements.txt -e .` from a clean venv yields a working `tg-export` on Telethon **1.44.0** (aarch64 wheel). @@ -299,6 +312,41 @@ differed — it now says a real run would refuse. Two criteria the review found untested are now tested: export-tree and file modes under the process umask, and control characters in the *extension*. +### Red Team Review — Session 3 — 2026-08-13 (adversarial pass over the fixes) + +The session-2 fixes were unit-tested but never independently reviewed, so a +second pass attacked them specifically. **It found that one of the fixes had +broken the tool outright.** + +| # | Finding | Severity | Disposition | +|---|---------|----------|-------------| +| C1 | Removing `Config.max_flood_wait` left the call site in `cli._real_run` passing it — **every real export died with an uncaught `TypeError`** after taking both locks and writing `title.txt` and the state file, before downloading a byte | **Critical** | **Fixed.** Plus `tests/test_cli_dispatch.py`, which is the real remedy | +| H-A | `AuthKeyNotFound` subclasses plain `Exception` and matched no clause. `MTProtoSender._disconnect` sets it on every in-flight request, so it is *more* likely than the `AuthKeyUnregisteredError` the taxonomy did list | High | **Fixed.** Added to `_AUTH_ERRORS` → exit 4 | +| H-B | `CdnFileTamperedError` (a hash mismatch on CDN-served bytes) was swallowed by `SecurityError` in the network clause and reported as three lines of "network error" | High | **Fixed.** Own clause, own message, not retried | +| H-C | Four of the six classes added for M3 are consumed inside Telethon's receive loop and can never reach an awaited request | Medium | **Fixed.** Dead arms removed; `InvalidBufferError` kept, since it does arrive for a transport-level 429 | +| H-D | A dead session made `run_download` fail every file, return normally, and set `completed_at` over an empty tree | High | **Partly fixed.** `completed_at` is now withheld when everything failed and nothing succeeded. The circuit breaker is open question 11 | +| M-A | `cli.main`'s `OSError` clause reported `ConnectionError`/`TimeoutError` as "filesystem" | Medium | **Fixed.** Network clause first; exit codes were already correct | +| M-B | U+202E and other `Cf`/`Zl`/`Zp` characters survived sanitization — `agpj.exe` renders as `a.exe.jpg` | Medium | **Fixed.** Strips by Unicode category, which also subsumes the old C0 table | +| M-C | A server-declared 0-byte document is now unexportable | Medium | **Open question 12** — a product call, not reversed unilaterally | +| M-D | A state file with an absent `filters` key reported a phantom diff on `include_text` | Low | **Fixed.** Absent is now its own refusal | + +Also fixed: the retry ladder no longer sleeps after its final attempt (20 s per +exhausted file, which on a dead connection was the difference between hours and +days). + +**Why C1 escaped 180 passing tests, which is the more important finding.** Every +test built `Config` by hand with the new signature, and not one exercised +`cli.main()` or `_real_run`. The suite validated each unit and never the wiring +between them, so a removed field and a stale call site could coexist happily. +`tests/test_cli_dispatch.py` now drives `main()` end to end against a fake client +— real run, resume, dry run, `--limit 0`, filter-mismatch exit 7, lock exit 3, +and an unexpected exception still honoring the exit contract. + +Confirmed unchanged by this pass: Invariants 1–3, dry-run/real-run agreement for +`limit ∈ {None, 0, 1, 2, 3, 5}`, and — importantly — that `sanitize_ext` is an +identity transform for every ordinary extension, so no previously downloaded file +is orphaned on resume. + ## Validation Log ### Session 1 — 2026-08-11 diff --git a/src/telegram_exporter/cli.py b/src/telegram_exporter/cli.py index 1571486..8bc62d6 100644 --- a/src/telegram_exporter/cli.py +++ b/src/telegram_exporter/cli.py @@ -211,8 +211,10 @@ async def _real_run(client, entity, *, root: Path, title: str, peer_id: int, posts, fetcher=Fetcher(client, entity, args.max_flood_wait), state=state, sidecar=sidecar, root=root, - cfg=Config(min_free=min_free, disk_path=root, - max_flood_wait=args.max_flood_wait), + # The flood ceiling reaches the network through the two + # primitives, not through Config: Fetcher above for the download + # path, iter_posts for the sweep. + cfg=Config(min_free=min_free, disk_path=root), limit=args.limit) log.info("done: %s", totals.summary()) return 0 @@ -234,6 +236,12 @@ def main() -> int: # disk cannot be trusted, and the cursor is where the last good post was. log.error("%s", e) return 1 + except (ConnectionError, TimeoutError) as e: + # Before the OSError clause: both subclass it, and the sweep path has no + # retry of its own, so a mid-sweep reset arrives here. Calling that + # "filesystem" would send the operator to the wrong place entirely. + log.error("network: %s - the cursor is intact, re-run to resume", e) + return 1 except OSError as e: # The download path maps these itself; this catches the same failures # arriving from the sidecar or state writes, so disk exhaustion exits 3 @@ -247,6 +255,13 @@ def main() -> int: except KeyboardInterrupt: log.info("interrupted - re-run the same command to resume") return 130 + except Exception: + # Last resort, so no failure can exit outside the documented contract. + # The traceback is still printed - CPython does not put locals in it, so + # this cannot leak api_hash or a phone number - but the exit code stays + # meaningful and the operator is told the cursor is safe. + log.exception("unexpected error - the cursor is intact, re-run to resume") + return 1 if __name__ == "__main__": diff --git a/src/telegram_exporter/downloader.py b/src/telegram_exporter/downloader.py index 1d4a788..1e30136 100644 --- a/src/telegram_exporter/downloader.py +++ b/src/telegram_exporter/downloader.py @@ -68,26 +68,39 @@ DOWNLOADED, SKIPPED, FAILED = "DOWNLOADED", "SKIPPED", "FAILED" # a fresh violation and escalates the next wait. # # ServerError rather than only RpcCallFailError: the latter is one leaf of it, so -# a plain Telegram -500 would otherwise escape. The telethon.errors.common set -# derives from Exception and BufferError, not OSError - a corrupt packet or a -# checksum failure on a flaky link is transient, but nothing below OSError would -# have caught it, and an unhandled one ends a 20-hour unattended run with a -# traceback. +# a plain Telegram -500 would otherwise escape. BadMessageError arrives the same +# way - MTProtoSender sets it on the pending request's future, so it surfaces at +# the download await rather than in the receive loop. +# +# InvalidBufferError reaches an awaited request only for a transport-level 429, +# where retrying beats ending the run. InvalidChecksumError and TypeNotFoundError +# are deliberately absent: MTProtoSender consumes both inside its receive loop +# and neither can surface here, so listing them would imply coverage this tuple +# does not have. SecurityError is absent too - its one download-path member, +# CdnFileTamperedError, gets its own clause below, and no other security failure +# should be quietly retried. _NETWORK_ERRORS = (ConnectionError, TimeoutError, asyncio.TimeoutError, asyncio.IncompleteReadError, ServerError, telethon_common.InvalidBufferError, - telethon_common.InvalidChecksumError, - telethon_common.TypeNotFoundError, - telethon_common.SecurityError, telethon_common.BadMessageError) + +# AuthKeyNotFound is the one that actually arrives: MTProtoSender._disconnect +# sets it on every in-flight request, and it subclasses plain Exception - so it +# matched no clause here and ended the run with a traceback instead of the +# documented exit 4. The RPC-level AuthKeyUnregisteredError is the rarer sibling. _AUTH_ERRORS = (AuthKeyUnregisteredError, AuthKeyDuplicatedError, - SessionRevokedError, UserDeactivatedBanError) + SessionRevokedError, UserDeactivatedBanError, + telethon_common.AuthKeyNotFound) _ACCESS_ERRORS = (ChannelPrivateError, ChatForbiddenError, ChannelInvalidError) _UNAVAILABLE_ERRORS = (MediaEmptyError, FileIdInvalidError, FilerefUpgradeNeededError) def backoff(attempt: int) -> float: - return 5.0 * 2 ** (attempt - 1) # 5, 10, 20 s + """5, 10 - and 0 after the final attempt, because sleeping before giving up + buys nothing and costs 20 s per exhausted file.""" + if attempt >= MAX_ATTEMPTS: + return 0.0 + return 5.0 * 2 ** (attempt - 1) @dataclass(frozen=True) @@ -309,6 +322,19 @@ async def download_one(fetcher, post_dir_path: Path, msg, *, cfg: Config) -> Res return _result(msg, FAILED, error=f"media unavailable or expired: {type(e).__name__}") + # ---- INTEGRITY: not network trouble, and not retryable noise ------ # + except telethon_common.CdnFileTamperedError: + # The CDN served bytes whose hash does not match what Telegram + # signed. It subclasses SecurityError and is raised on the download + # path, so folding it into the network clause would report a + # trust-boundary event as three lines of "network error" and a bland + # FAILED. Recorded distinctly and skipped; the run continues, because + # one tampered file is not evidence about the rest. + tmp.unlink(missing_ok=True) + log.error("message %s: CDN integrity check FAILED - the served bytes " + "do not match what Telegram signed; not retrying", msg.id) + return _result(msg, FAILED, error="cdn integrity check failed") + # ---- ABORT: the run cannot continue ------------------------------- # except _AUTH_ERRORS: tmp.unlink(missing_ok=True) @@ -419,7 +445,17 @@ async def run_download(posts, *, fetcher, state, sidecar, root: Path, # at the last post that happened to contain a file - and the next run would # re-sweep that whole tail to download nothing. state.commit(last_complete) - state.mark_completed() + if totals.failed and not (totals.downloaded or totals.skipped): + # Every single file failed and none succeeded. That is what a dead + # session looks like from in here - ConnectionError is retryable, so the + # loop walks the whole history failing everything and returns normally. + # Setting completed_at on that would answer "did my export finish?" with + # a confident yes over an empty tree. The cursor still stands: the posts + # were handled, and their failures are in the sidecar. + log.error("every file failed (%d) and none succeeded - not marking this " + "export complete; check connectivity and re-run", totals.failed) + else: + state.mark_completed() return totals diff --git a/src/telegram_exporter/paths.py b/src/telegram_exporter/paths.py index b711c27..5c86c1f 100644 --- a/src/telegram_exporter/paths.py +++ b/src/telegram_exporter/paths.py @@ -22,15 +22,28 @@ from __future__ import annotations import os import re +import unicodedata from pathlib import Path from .media import file_ext, file_stem, media_kind -# Control characters are removed outright rather than replaced: a NUL truncates -# the filename at the syscall boundary, so leaving a placeholder would only hide -# what happened. -_CONTROL = {c: None for c in range(0x20)} -_CONTROL[0x7F] = None +# Characters removed outright rather than replaced: a NUL truncates the filename +# at the syscall boundary, so leaving a placeholder would only hide what +# happened. +# +# Cc C0 and C1 controls, including NUL and the ESC that starts an ANSI escape +# Cf format characters - U+202E RIGHT-TO-LEFT OVERRIDE is the one that +# matters: `agpj.exe` renders as `a.exe.jpg` in any terminal or file +# manager, so stripping only C0 would close the ANSI vector while leaving +# the older extension-spoofing one wide open +# Zl/Zp line and paragraph separators, which break line-oriented consumers of +# a file listing +# Cs lone surrogates, which cannot be encoded to UTF-8 at all +_STRIPPED_CATEGORIES = frozenset({"Cc", "Cf", "Zl", "Zp", "Cs"}) + + +def _is_disallowed(ch: str) -> bool: + return unicodedata.category(ch) in _STRIPPED_CATEGORIES # Path separators and shell/Windows reserved characters become underscores. _RESERVED = str.maketrans({ch: "_" for ch in '/\\:*?"<>|'}) @@ -113,7 +126,7 @@ def sanitize(name: str, *, fallback: str, max_bytes: int = MAX_NAME_BYTES) -> st so `../../../etc/passwd` loses its directories rather than becoming `.._.._.._etc_passwd`. """ - s = str(name).translate(_CONTROL) + s = "".join(ch for ch in str(name) if not _is_disallowed(ch)) s = _leaf(s) # discards every directory part, incl. .. s = s.translate(_RESERVED) s = _WHITESPACE_RUN.sub(" ", s).strip() diff --git a/src/telegram_exporter/state.py b/src/telegram_exporter/state.py index 6d55fcc..9108447 100644 --- a/src/telegram_exporter/state.py +++ b/src/telegram_exporter/state.py @@ -148,7 +148,16 @@ def _assert_compatible(existing: dict, *, chat_id: int, filters: Filters) -> Non f"chat mismatch: state file belongs to chat {existing.get('chat_id')}, " f"not {chat_id}\nexport each group to its own --out directory", 7) - was, now = existing.get("filters") or {}, filters.to_state() + if not isinstance(existing.get("filters"), dict): + # Absent or null is not the same condition as changed, and reporting it + # as a diff sends the user chasing a flag they never passed. Only a + # hand-edited or truncated state file gets here. + raise Abort( + f"state file has no filter set recorded, so the cursor's meaning is " + f"unknown - it means 'handled through here under these filters'.\n" + f"use --reset-state to start over", 7) + + was, now = existing["filters"], filters.to_state() changed = [k for k in sorted(set(was) | set(now)) if was.get(k) != now.get(k)] if changed: lines = "\n".join(f" {k+':':12} {_show(was.get(k))} -> {_show(now.get(k))}" diff --git a/tests/test_cli_dispatch.py b/tests/test_cli_dispatch.py new file mode 100644 index 0000000..9a18432 --- /dev/null +++ b/tests/test_cli_dispatch.py @@ -0,0 +1,207 @@ +"""End-to-end dispatch through cli.main() against a fake Telegram client. + +This file exists because of a specific escape: `Config.max_flood_wait` was +removed from the dataclass while `cli._real_run` kept passing it, so every real +export died with an uncaught TypeError after taking both locks and writing the +state file - and 180 tests still passed, because every one of them built Config +by hand and none of them ever called main(). + +Anything that only the wiring can break belongs here. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from telegram_exporter import cli, session as session_mod +from telegram_exporter.state import STATE_NAME +from tests.support import FakeMsg + + +class FakeClient: + """Enough of a TelegramClient for a whole run, with no network.""" + + def __init__(self, messages, entity): + self.messages = messages + self.entity = entity + self.downloads: list[int] = [] + self.flood_sleep_threshold = None + self.disconnected = False + + def iter_messages(self, entity, *, reverse=False, offset_id=0, **kwargs): + async def gen(): + for m in self.messages: + if m.id > offset_id: + yield m + return gen() + + async def download_media(self, msg, file=None): + self.downloads.append(msg.id) + file.write(b"x" * (msg.file.size or 10)) + return file + + async def get_messages(self, entity, ids=None): + return next((m for m in self.messages if m.id == ids), None) + + async def connect(self): + return None + + async def is_user_authorized(self): + return True # a second run is non-interactive + + async def disconnect(self): + self.disconnected = True + + +class FakeEntity: + id, title, username = 1234567890, "My Group", "mygroup" + + +@pytest.fixture +def wired(monkeypatch, tmp_path): + """Patch out only the network boundary: credentials, client, entity.""" + monkeypatch.setenv("TG_API_ID", "12345") + monkeypatch.setenv("TG_API_HASH", "0" * 32) + monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "state")) + + messages = [FakeMsg(10, kind="photo", grouped_id=7, size=100), + FakeMsg(11, kind="photo", grouped_id=7, size=100), + FakeMsg(12, text="just talking"), + FakeMsg(13, kind="video", size=500)] + client = FakeClient(messages, FakeEntity()) + + class FakeClientFactory: + def __init__(self, *a, **kw): + pass + + def fake_telegram_client(session, api_id, api_hash): + return client + + monkeypatch.setattr(session_mod, "TelegramClient", fake_telegram_client) + + async def fake_resolve(_client, _spec): + return client.entity, -1001234567890 + + monkeypatch.setattr(cli, "resolve_entity", fake_resolve) + return client + + +def run_cli(monkeypatch, *argv) -> int: + monkeypatch.setattr("sys.argv", ["tg-export", *argv]) + return cli.main() + + +# --------------------------------------------------------------------------- # +# The real run - the path C1 broke +# --------------------------------------------------------------------------- # + +def test_a_real_export_runs_end_to_end(wired, monkeypatch, tmp_path): + out = tmp_path / "exports" + code = run_cli(monkeypatch, "--group", "@mygroup", "--out", str(out)) + + assert code == 0 + root = out / "g-1001234567890" + assert sorted(p.name for p in (root / "10").iterdir()) == \ + ["10_photo.jpg", "11_photo.jpg"] + assert (root / "13" / "13_video.mp4").exists() + assert (root / "title.txt").read_text().strip() == "My Group" + assert json.loads((root / STATE_NAME).read_text())["completed_at"] is not None + assert wired.disconnected, "disconnect must run even on the success path" + + +def test_a_real_export_honors_the_flood_ceiling_flag(wired, monkeypatch, tmp_path): + # The ceiling reaches the network through Fetcher and iter_posts, not + # through Config. Passing the flag must not blow up the wiring. + code = run_cli(monkeypatch, "--group", "@mygroup", + "--out", str(tmp_path / "e"), "--max-flood-wait", "3600") + assert code == 0 + + +def test_a_second_run_downloads_nothing(wired, monkeypatch, tmp_path): + out = str(tmp_path / "exports") + assert run_cli(monkeypatch, "--group", "@mygroup", "--out", out) == 0 + before = len(wired.downloads) + assert run_cli(monkeypatch, "--group", "@mygroup", "--out", out) == 0 + assert len(wired.downloads) == before, "resume re-downloaded completed files" + + +def test_limit_zero_is_a_non_interactive_connectivity_check(wired, monkeypatch, + tmp_path): + code = run_cli(monkeypatch, "--group", "@mygroup", + "--out", str(tmp_path / "e"), "--limit", "0") + assert code == 0 + assert wired.downloads == [] + + +# --------------------------------------------------------------------------- # +# Dry run +# --------------------------------------------------------------------------- # + +def test_dry_run_writes_nothing_and_downloads_nothing(wired, monkeypatch, + tmp_path, capsys): + out = tmp_path / "exports" + code = run_cli(monkeypatch, "--group", "@mygroup", "--out", str(out), + "--dry-run", "--min-free", "0") + + assert code == 0 + assert wired.downloads == [] + assert not out.exists() + assert "Remaining:" in capsys.readouterr().out + + +def test_dry_run_exits_two_when_it_will_not_fit(wired, monkeypatch, tmp_path): + code = run_cli(monkeypatch, "--group", "@mygroup", + "--out", str(tmp_path / "e"), "--dry-run", + "--min-free", "1000GiB") + assert code == 2 + + +# --------------------------------------------------------------------------- # +# Exit-code contract through the real dispatcher +# --------------------------------------------------------------------------- # + +def test_a_filter_change_exits_seven_and_names_the_change(wired, monkeypatch, + tmp_path, caplog): + out = str(tmp_path / "exports") + assert run_cli(monkeypatch, "--group", "@mygroup", "--out", out, + "--types", "photo") == 0 + code = run_cli(monkeypatch, "--group", "@mygroup", "--out", out, + "--types", "video") + + assert code == 7 + assert "filter mismatch" in caplog.text and "types" in caplog.text + + +def test_a_token_less_types_value_never_reaches_a_run(wired, monkeypatch, tmp_path): + code = run_cli(monkeypatch, "--group", "@mygroup", + "--out", str(tmp_path / "e"), "--types", " ") + assert code == 1 + assert wired.downloads == [] + + +def test_missing_credentials_exit_one(monkeypatch, tmp_path): + monkeypatch.delenv("TG_API_ID", raising=False) + monkeypatch.delenv("TG_API_HASH", raising=False) + assert run_cli(monkeypatch, "--group", "@g", "--out", str(tmp_path)) == 1 + + +def test_an_unexpected_exception_still_honors_the_exit_contract(wired, monkeypatch, + tmp_path): + async def boom(_client, _spec): + raise RuntimeError("something nobody predicted") + + monkeypatch.setattr(cli, "resolve_entity", boom) + code = run_cli(monkeypatch, "--group", "@mygroup", "--out", str(tmp_path / "e")) + assert code == 1 # not a bare traceback with no code + + +def test_a_second_concurrent_run_exits_three(wired, monkeypatch, tmp_path): + out = tmp_path / "exports" + root = out / "g-1001234567890" + root.mkdir(parents=True) + with cli.exclusive(root / cli.LOCK_NAME): + code = run_cli(monkeypatch, "--group", "@mygroup", "--out", str(out)) + assert code == 3 diff --git a/tests/test_download_decisions.py b/tests/test_download_decisions.py index 1cf01c6..c9c5dc2 100644 --- a/tests/test_download_decisions.py +++ b/tests/test_download_decisions.py @@ -20,7 +20,11 @@ from telethon.errors import ( MediaEmptyError, ServerError, ) -from telethon.errors.common import InvalidBufferError +from telethon.errors.common import ( + AuthKeyNotFound, + CdnFileTamperedError, + InvalidBufferError, +) from telegram_exporter import downloader, paths from telegram_exporter.downloader import ( @@ -45,9 +49,12 @@ KIB = 1024 @pytest.fixture(autouse=True) def no_real_backoff(monkeypatch): - """The retry ladder is 5/10/20 s in production; tests assert the ladder is - walked, not that it sleeps.""" + """The retry ladder is 5/10 s in production; tests assert the ladder is + walked, not that it sleeps. Yields the real function for the one test that + is about the ladder's shape.""" + real = downloader.backoff monkeypatch.setattr(downloader, "backoff", lambda attempt: 0) + return real class FakeFetcher: @@ -313,8 +320,30 @@ def test_transport_level_errors_retry_rather_than_killing_the_run(tmp_path, exc) assert fetcher.calls == 2 +def test_a_cdn_integrity_failure_is_named_rather_than_called_a_network_error( + tmp_path, caplog): + """CdnFileTamperedError subclasses SecurityError and is raised on the + download path. Folding it into the network clause reported a trust-boundary + event as three lines of "network error" and a bland FAILED.""" + _, target_dir = post_dir_for(tmp_path) + msg = FakeMsg(1042, kind="photo", size=len(PAYLOAD)) + fetcher = FakeFetcher(raises=[CdnFileTamperedError()]) + + with caplog.at_level("ERROR"): + result = run(download_one(fetcher, target_dir, msg, cfg=cfg(tmp_path))) + + assert result.status == FAILED + assert result.error == "cdn integrity check failed" + assert fetcher.calls == 1 # not retried as if it were noise + assert "integrity check FAILED" in caplog.text + + @pytest.mark.parametrize("exc,code", [ (AuthKeyUnregisteredError(request=None), 4), + # The one that actually arrives: MTProtoSender sets it on every in-flight + # request when the connection drops, and it subclasses plain Exception - so + # it used to escape every handler and end the run with a traceback. + (AuthKeyNotFound(), 4), (ChannelPrivateError(request=None), 5), (OSError(errno.ENOSPC, "No space left on device"), 3), ]) @@ -425,6 +454,52 @@ def test_completed_at_is_set_when_the_sweep_is_exhausted(tmp_path): assert State.read(root)["completed_at"] is not None +def test_a_run_where_everything_failed_is_never_marked_complete(tmp_path, caplog): + """A dead session looks exactly like this from inside the loop: ConnectionError + is retryable, so the sweep walks the whole history failing every file and + returns normally. Setting completed_at there would answer "did my export + finish?" with a confident yes over an empty tree.""" + class AlwaysFails(FakeFetcher): + async def fetch(self, msg, fh): + self.calls += 1 + raise ConnectionError("Cannot send requests while disconnected") + + with caplog.at_level("ERROR"): + root, state, totals, _ = drive( + tmp_path, [album(1, [1]), album(2, [2])], fetcher=AlwaysFails()) + + assert totals.failed == 2 and totals.downloaded == 0 + assert State.read(root)["completed_at"] is None + assert "not marking this export complete" in caplog.text + # The cursor still stands: those posts were handled, and the failures are + # recorded in the sidecar. + assert state.cursor_id == 2 + + +def test_a_partly_failed_run_is_still_complete(tmp_path): + class FailsOne(FakeFetcher): + async def fetch(self, msg, fh): + self.calls += 1 + if msg.id == 2: + raise MediaEmptyError(request=None) + fh.write(self.payload) + + root, _, totals, _ = drive(tmp_path, [album(1, [1]), album(2, [2])], + fetcher=FailsOne()) + + assert totals.downloaded == 1 and totals.failed == 1 + assert State.read(root)["completed_at"] is not None + + +def test_the_retry_ladder_does_not_sleep_after_the_final_attempt(no_real_backoff): + # 20 s per exhausted file bought nothing; on a dead connection it was the + # difference between hours and days. + backoff = no_real_backoff + assert backoff(1) == 5.0 + assert backoff(2) == 10.0 + assert backoff(downloader.MAX_ATTEMPTS) == 0.0 + + def test_the_whole_export_tree_is_private_under_the_process_umask(tmp_path): """plan.md's "export tree 0700, every file 0600" rested on umask alone, with nothing observing the result. The sidecar carries other people's message text diff --git a/tests/test_paths.py b/tests/test_paths.py index 2fb86c7..f71181a 100644 --- a/tests/test_paths.py +++ b/tests/test_paths.py @@ -126,6 +126,21 @@ def test_the_extension_is_sanitized_like_the_stem(name, expected): assert derive_filename(FakeMsg(7, kind="document", name=name)) == expected +@pytest.mark.parametrize("raw,forbidden", [ + ("a‮gpj.exe", "‮"), # RIGHT-TO-LEFT OVERRIDE: renders a.exe.jpg + ("‮report.pdf", "‮"), + ("a​b.jpg", "​"), # zero-width space + ("a
b.jpg", "
"), # line separator: breaks `find` consumers + ("a…b.jpg", "…"), # C1 NEL +]) +def test_bidi_and_format_characters_cannot_spoof_an_extension(raw, forbidden): + """Stripping only C0 escapes closed the ANSI vector while leaving the older + extension-spoofing one open: makes a .exe render as a .jpg in any + terminal or file manager.""" + out = derive_filename(FakeMsg(12345, kind="document", name=raw)) + assert forbidden not in out + + def test_no_control_character_survives_anywhere_in_a_filename(): for raw in ["a\x00.b\x00c", "\x1b].jpg", "x.\ny", "n.\r\tz"]: out = derive_filename(FakeMsg(3, kind="document", name=raw)) diff --git a/tests/test_state.py b/tests/test_state.py index 39caaca..5247973 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -93,6 +93,23 @@ def test_limit_is_not_part_of_the_stored_filter_set(tmp_path): assert "limit" not in open_state(tmp_path).data["filters"] +@pytest.mark.parametrize("filters_value", ["MISSING", None, "not-a-dict"]) +def test_a_state_file_with_no_filter_set_is_refused_as_its_own_condition( + tmp_path, filters_value): + data = {"version": 1, "chat_id": CHAT, "cursor_id": 99, "filters": filters_value} + if filters_value == "MISSING": + del data["filters"] + (tmp_path / STATE_NAME).write_text(json.dumps(data)) + + with pytest.raises(Abort) as excinfo: + open_state(tmp_path) + + assert excinfo.value.code == 7 + assert "no filter set recorded" in excinfo.value.reason + # Not reported as a phantom diff on a flag the user never passed. + assert "include_text:" not in excinfo.value.reason + + def test_a_corrupt_state_file_is_refused_rather_than_ignored(tmp_path): (tmp_path / STATE_NAME).write_text("{not json") with pytest.raises(Abort) as excinfo: