diff --git a/CLAUDE.md b/CLAUDE.md index e0b8dd1..86142b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,9 +66,9 @@ that list in the same commit. No database. `ls ~/booth-data` tells you everything the service knows. Per-booth operator state is a **dotfile inside the booth**: `.forever` (keep), -`.viewed` (last deliberate look — U4's "viewing is activity"), `.blurred` (the -per-item blur set, a JSON ARRAY — see below), `.seen` (R2: rels looked at full -size, a JSON ARRAY), `.blurbooth` (the whole booth fogged — a MARKER like `.forever`, not +`.viewed` (last deliberate look — U4's "viewing is activity"), `.blurred.json` +(the per-item blur set, a JSON ARRAY — see below; the legacy `.blurred` is +read-only), `.seen` (R2: rels looked at full size, a JSON ARRAY), `.blurbooth` (the whole booth fogged — a MARKER like `.forever`, not JSON, because a boolean has no rels to round-trip), `.marks.json` + `.marks.lock` (judgment), `.pins` (link-board pin ids), `.uploaded` (upload-booth marker). `booth_items()` skips `name.startswith(".")`, so a new dotfile costs nothing in item counts, galleries or zips. That skip is why the @@ -79,19 +79,30 @@ inventing a sidecar-per-item. O_NONBLOCK` with an `S_ISREG` check and a size cap.** A rel may carry a leading space or a newline, and line-stripped storage does not round-trip it: `.blurred` was one stripped rel per line, and blurring `" a.png"` blurred `a.png` instead. -`.seen` was written as JSON for exactly that reason (design-dev, R2), and -`.blurred` now matches it (`booth/blur.py`). The open flags mean a planted -symlink is refused and a FIFO cannot hang the read, which is the outage in -`persistent-memory.d/2026-09-22-size-cap-opened-a-hang.md`. **Any new dotfile -inherits that shape.** `.blurred`'s reader still accepts the old line format, -so a booth written before the change keeps its blur until its next write -upgrades the file. Do not remove that fallback while a line-format file can -still exist. +`.seen` was written as JSON for exactly that reason (design-dev, R2), and the +blur set now matches it in `.blurred.json` (`booth/blur.py`). The open flags +mean a planted symlink is refused and a FIFO cannot hang the read, which is the +outage in `persistent-memory.d/2026-09-22-size-cap-opened-a-hang.md`. **Any new +dotfile inherits that shape.** -**A dotfile with two writers has ONE implementation of the writer.** `.blurred` -is written by the service and by `booth blur`, and both call -`booth.blur.set_blurred`; the CLI used to keep a grep/printf writer of its own, -and two writers of one format is how the formats drift apart. +⚠ **A format change gets a NEW NAME, never a sniffed file.** The first cut of +the blur fix wrote JSON into `.blurred` and guessed the format from the bytes; +a legacy file whose one line is an item literally named `["a.png"]` parses as +JSON and blurs the neighbour, the bug being fixed (heid bug-hunt, 3 of 3). So +`.blurred.json` is JSON only, the legacy `.blurred` is lines only and read only +while `.blurred.json` is absent, and the first write retires it. Do not remove +that legacy read while a line-format file can still exist. + +**A writer is judged by its reader.** `set_blurred` re-reads after writing and +raises `BlurUnwritable` unless the reader returns exactly the set asked for. +One postcondition covers a planted directory, a permission and a race without +a branch per way the disk can be wrong; the route answers it 409, never 500. + +**A dotfile with two writers has ONE implementation of the writer, and one +predicate for its keys.** The blur set is written by the service and by `booth +blur`; both call `booth.blur.set_blurred`, and both ask `check_rel` what an +item path is. The CLI used to keep a grep/printf writer and a `*..*` guard of +its own, which refused `a..b.png` where the route accepted it. ### 3. One resolver for item facts diff --git a/booth/app.py b/booth/app.py index 1c8d75c..1ee506f 100644 --- a/booth/app.py +++ b/booth/app.py @@ -104,7 +104,7 @@ from booth.items import ( # noqa: E402,F401 ) # The per-item blur writer lives with its reader in booth/blur.py, stdlib-only so # `scripts/booth blur` shares both. Re-exported: tests import it from here. -from booth.blur import set_blurred # noqa: E402,F401 +from booth.blur import BlurUnwritable, check_rel, set_blurred # noqa: E402,F401 # Sentinel dotfile that exempts a booth from the TTL sweep. A dotfile because # the existing listing code already skips dotfiles, so it costs nothing in item @@ -2216,14 +2216,20 @@ def create_app( """Toggle one item's blur. Reversible and cosmetic, so no confirmation. See BLUR_FILE: this hides an item from a glance, it does not protect it.""" booth = resolve_booth(name) - # Guard the path the same way the file route must: a blur entry is only - # ever a booth-relative path, never an escape. NEVER STRIPPED: " a.png" - # and "a.png" are two items, and a stripped `f` blurred the neighbour - # (heid bug-hunt on r2b merge 1). A leading "/" is never part of a rel. + # NEVER STRIPPED: " a.png" and "a.png" are two items, and a stripped + # `f` blurred the neighbour (heid bug-hunt on r2b merge 1). A leading + # "/" is never part of a rel. What else a blur entry may be is + # `check_rel`'s one predicate, shared with `booth blur`, so the route + # and the CLI cannot disagree about which items are addressable. rel = f.lstrip("/") - if ".." in Path(rel).parts: - raise HTTPException(status_code=400, detail="bad item path") - set_blurred(booth, rel, on not in ("0", "false", "")) + try: + set_blurred(booth, rel, on not in ("0", "false", "")) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) + except BlurUnwritable as exc: + # The state on disk is wrong (a planted directory, a permission), + # not the request: a refusal, never a 500. + raise HTTPException(status_code=409, detail=str(exc)) return RedirectResponse(url=f"/b/{quote(name, safe='')}/", status_code=303) @app.post("/b/{name}/delete") diff --git a/booth/blur.py b/booth/blur.py index 3037aaa..460d861 100644 --- a/booth/blur.py +++ b/booth/blur.py @@ -1,19 +1,26 @@ -"""Per-item blur storage — `.blurred`, one JSON array of booth-relative paths. +"""Per-item blur storage — `.blurred.json`, one JSON array of booth-relative paths. ⚠ STDLIB ONLY (CLAUDE.md invariant 1). `scripts/booth blur` imports this under -the system python3 with no venv, so the service and the CLI share ONE reader and -ONE writer. The CLI used to keep its own grep/printf line writer, and two -writers of one file is how the formats would drift apart. +the system python3 with no venv, so the service and the CLI share ONE reader, +ONE writer and ONE predicate for what an item path is. The CLI used to keep its +own grep/printf line writer, and two writers of one file is how formats drift. ⚠ COSMETIC ONLY. A blurred item is still served, still in the zip, still on disk. The Booth has no auth: if a thing must not be SEEN, it must not be in a booth. -The `.seen` shape, for `.seen`'s reason: a rel may carry a leading space or a -newline, and the old one-stripped-rel-per-line format could not round-trip it. -Blurring " a.png" stored "a.png" and blurred the neighbour instead. The line -format is still READ, so a booth written before this change keeps its blur -until its next write upgrades the file. +WHY A NEW FILE NAME, NOT A NEW FORMAT IN THE OLD FILE. `.blurred` was one +stripped rel per line, which could not round-trip a rel with a leading space or +a newline (blurring " a.png" blurred "a.png"). A JSON array fixes that, the +`.seen` shape. Writing it into the OLD name would force the reader to sniff +which format it is looking at, and sniffing cannot be made safe: a legacy file +whose one line is an item literally named `["a.png"]` parses as a JSON array and +would blur the neighbour, the very bug this module exists to fix (heid bug-hunt, +3 of 3 arms). So the two formats live at two names and neither is ever guessed: + + .blurred.json current. JSON only, never read as lines. + .blurred legacy, READ ONLY, and only while `.blurred.json` is absent. + Lines only, never read as JSON. The first write replaces it. """ from __future__ import annotations @@ -24,95 +31,165 @@ import stat import tempfile from pathlib import Path -BLUR_FILE = ".blurred" +BLUR_FILE = ".blurred.json" +LEGACY_BLUR_FILE = ".blurred" -# A blur set bigger than this is not one this service or the CLI wrote: a JSON -# array of every rel in a 270-item booth is a few KB. Same bound as `.seen`. +# A blur set bigger than this is not one this module wrote: a JSON array of +# every rel in a 270-item booth is a few KB. Same bound as `.seen`, and the +# WRITER enforces it too, so the writer can never produce a file the reader +# would refuse and read as nothing. BLUR_MAX_BYTES = 1 << 20 +class BlurUnwritable(Exception): + """The blur set on disk could not be made to hold what was asked: something + that is not ours is in the way (a directory at the name, a permission), or + the set would outgrow what the reader accepts. A refusal about the STATE ON + DISK, not about the request, so the route answers 409, never 500.""" + + +def check_rel(rel: str) -> str: + """The one predicate for what a blur entry may be, shared by the route and + the CLI so the two cannot disagree about which items are addressable. + + A booth-relative path: not empty, not absolute, no `..` COMPONENT (so + `a..b.png` is a fine name, and `a/../b` is not), and encodable back to the + bytes of a filename. Stored EXACTLY as given otherwise — never stripped. + Raises ValueError; returns `rel` unchanged.""" + if not rel or rel.startswith("/") or ".." in rel.split("/"): + raise ValueError(f"not a booth-relative item path: {rel!r}") + if not _encodable(rel): + raise ValueError(f"not a filename this box can hold: {rel!r}") + return rel + + +def _encodable(rel: str) -> bool: + """A real filename decodes under surrogateescape to U+DC80..U+DCFF at worst, + which encodes back. A lone U+D800 cannot come from any filename, only from a + planted JSON escape, and would make every later write raise.""" + try: + rel.encode("utf-8", "surrogateescape") + except UnicodeEncodeError: + return False + return True + + +def _read_capped(path: Path) -> bytes | None: + """A regular file's bytes, or None. Never follows a link, never blocks on a + FIFO, never reads past the cap, never raises.""" + try: + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + except OSError: + return None + try: + st = os.fstat(fd) + if not stat.S_ISREG(st.st_mode) or st.st_size > BLUR_MAX_BYTES: + return None + return os.read(fd, BLUR_MAX_BYTES + 1) + except OSError: + return None + finally: + os.close(fd) + + def read_blurred(booth: Path) -> set[str]: """Blurred rels for a booth. Missing, unreadable or malformed -> empty set. NEVER RAISES and NEVER BLOCKS. `booth_items` calls this for every booth the - Desk renders, and any fleet session can write into a booth, so the file may - be planted: it is opened without following a link and without blocking (a - FIFO with no writer), and refused unless it is a regular file of sane size. + Desk renders, and any fleet session can write into a booth, so either file + may be planted: each is opened without following a link and without + blocking, and refused unless it is a regular file of sane size. - Reads BOTH formats. A JSON array of strings is the current one; anything - that does not parse as a JSON array is the legacy one-rel-per-line format, - read exactly as before (stripped, blank lines dropped). Legacy rels starting - with "[" still read, because they fail the JSON parse and fall through. - Non-string members of an array are skipped, not fatal. + ANYTHING at `.blurred.json` (a link or a directory included) means the + current format is in charge, and the legacy file is not consulted, so a + stale `.blurred` left beside a newer set can never speak. Members that are + not strings, are empty, or could not be a filename are skipped. """ try: - fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) - except OSError: - return set() - try: - st = os.fstat(fd) - if not stat.S_ISREG(st.st_mode) or st.st_size > BLUR_MAX_BYTES: + os.lstat(booth / BLUR_FILE) + except FileNotFoundError: + raw = _read_capped(booth / LEGACY_BLUR_FILE) + if raw is None: return set() - raw = os.read(fd, BLUR_MAX_BYTES + 1) + text = raw.decode("utf-8", "surrogateescape") + return {ln.strip() for ln in text.splitlines() if ln.strip()} except OSError: return set() - finally: - os.close(fd) - try: - text = raw.decode("utf-8", "surrogateescape") - except UnicodeDecodeError: # pragma: no cover - surrogateescape cannot fail + raw = _read_capped(booth / BLUR_FILE) + if raw is None: return set() try: - data = json.loads(text) + data = json.loads(raw.decode("utf-8", "surrogateescape")) except (ValueError, RecursionError): # RecursionError: a deeply nested array blows the parser's stack, and # it is neither a ValueError nor an OSError (the `.seen` hole). - data = None - if isinstance(data, list): - return {r for r in data if isinstance(r, str)} - return {ln.strip() for ln in text.splitlines() if ln.strip()} + return set() + if not isinstance(data, list): + return set() + return {r for r in data if isinstance(r, str) and r and _encodable(r)} + + +def _discard(path: Path) -> None: + try: + path.unlink() + except OSError: + pass # judged by the postcondition in set_blurred, not here def set_blurred(booth: Path, rel: str, on: bool) -> set[str]: """Add or remove one rel from the blur set, and return the new set. - `rel` is stored EXACTLY as given; callers must not strip it. Written as a - JSON array in sorted order (CLAUDE.md invariant 6), so the same set is the - same bytes. An empty set removes the file: an empty marker is a lie by - omission. + `rel` must pass `check_rel` (ValueError otherwise) and is stored EXACTLY as + given. Written as a JSON array in sorted order (CLAUDE.md invariant 6), so + the same set is the same bytes; an empty set removes the file, because an + empty marker is a lie by omission. The first write also retires a legacy + `.blurred`, AFTER the new file is in place, so a crash between the two + leaves the new file in charge. Atomic replace (CLAUDE.md invariant 5) through a temp file created with - O_EXCL, so a crash mid-write cannot leave a shorter, and therefore more - revealing, set; a planted `.blurred.*.tmp` symlink cannot redirect the - write, and `os.replace` swaps a planted `.blurred` symlink out rather than - writing through it. + O_EXCL: a crash mid-write cannot leave a shorter, more revealing set, and + `os.replace` swaps a planted symlink out rather than writing through it. + + SUCCESS IS DEFINED BY THE READER. After writing, `read_blurred` must return + exactly the set asked for; anything else raises BlurUnwritable. That one + check covers a planted directory at either name, a permission, and a race, + without a branch per way the disk can be wrong. NOT locked. Two writers racing (the operator's click and a session's - `booth blur`) can lose one toggle; both are rare, deliberate and visible on - the next render, so this matches what the line format did. + `booth blur`) can lose one toggle, as the line format could. """ + check_rel(rel) current = read_blurred(booth) if on: current.add(rel) else: current.discard(rel) path = booth / BLUR_FILE - if not current: + legacy = booth / LEGACY_BLUR_FILE + if current: + body = json.dumps(sorted(current), ensure_ascii=False).encode("utf-8", "surrogateescape") + if len(body) > BLUR_MAX_BYTES: + raise BlurUnwritable( + f"{len(current)} blurred items would exceed the {BLUR_MAX_BYTES}-byte " + f"bound the reader accepts; nothing was changed") try: - path.unlink() - except FileNotFoundError: - pass - return current - body = json.dumps(sorted(current), ensure_ascii=False).encode("utf-8", "surrogateescape") - fd, tmp = tempfile.mkstemp(prefix=".blurred.", suffix=".tmp", dir=booth) - try: - with os.fdopen(fd, "wb") as fh: - fh.write(body) - os.replace(tmp, path) - except BaseException: - try: - os.unlink(tmp) + fd, tmp = tempfile.mkstemp(prefix=".blurred.", suffix=".tmp", dir=booth) + try: + with os.fdopen(fd, "wb") as fh: + fh.write(body) + os.replace(tmp, path) + except BaseException: + _discard(Path(tmp)) + raise except OSError: - pass - raise + pass # judged by the postcondition below + else: + _discard(legacy) + else: + _discard(path) + _discard(legacy) + if read_blurred(booth) != current: + raise BlurUnwritable( + f"the blur set in {booth.name!r} could not be written; is something other " + f"than a file at {BLUR_FILE} or {LEGACY_BLUR_FILE}?") return current diff --git a/persistent-memory.md b/persistent-memory.md index e33813e..773ebc0 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -31,15 +31,24 @@ _As of 2026-09-23:_ 1:1 with native image drag killed; the mode is remembered per viewer. Pan offset across items is parked to r3 (compare). Then **r3, compare mode**: ours is only the `booth_items` support he asks for. -- ✅ **`.blurred` ROUND-TRIPS ANY REL** (operator: "fix the blur"). A JSON array - via stdlib-only `booth/blur.py`, the one writer for both the service and - `booth blur`. The legacy line format is still read, and a write upgrades it. - The bug design-dev's bug-hunt found (a stripped rel blurring its neighbour) - had no live victims: 6 `.blurred` files, 42 rels, 0 with edge whitespace. - `Item.blurred_self` came along, so blur state has one reader (invariant 3). - **Still open and ours, not done:** the "off"-means-ON idiom drift between - /blur, /blurbooth and /flag (forms only send 0/1), and the CLI's - `.blurbooth` `touch` still follows a symlink where the service no longer does. +- ✅ **THE BLUR SET ROUND-TRIPS ANY REL** (operator: "fix the blur"). It lives + in `.blurred.json`, a JSON array written through stdlib-only `booth/blur.py`, + which is the one writer and one `check_rel` predicate for both the service + and `booth blur`. The legacy `.blurred` is read as lines, only while no + `.blurred.json` exists, and the first write retires it. The original bug (a + stripped rel blurred its neighbour) had no live victims: 6 legacy files, 42 + rels, none with edge whitespace, none parseable as JSON. The heid bug-hunt + (3 arms, groa timed out) folded: a planted directory now gets a 409 instead + of a 500, the legacy file is never sniffed for JSON, a lone-surrogate member + is dropped, the writer respects the reader's size cap, the CLI takes + `a..b.png` and refuses an empty path, and a missing package fails closed. + Declined: the `Item` positional-constructor break (booth_items is the only + constructor, INV-1), the fdopen fd leak, the short read, and + unreadable-reads-as-revealed (the `.seen` posture). `Item.blurred_self` came + along, so blur state has one reader (invariant 3). **Still ours, not done:** + "off" means ON for /blur and /blurbooth but OFF for /flag (forms only send + 0/1), and the CLI's `.blurbooth` `touch` still follows a symlink where the + service no longer does. - ⚠ **THE BROWSER SUITE WAS FLAKY UNDER LOAD, AND THE CAUSE IS STILL UNCONFIRMED.** design-dev's suspect: Google Fonts stalling "networkidle". He reproduced the exact error with a stalled font request (sufficiency only). diff --git a/scripts/booth b/scripts/booth index 051a42f..399cfc7 100755 --- a/scripts/booth +++ b/scripts/booth @@ -325,32 +325,49 @@ case "$cmd" in exit 0 fi - # Every item is checked BEFORE anything is written, so a refused path - # leaves the blur set exactly as it was. + # Items are made booth-relative here; WHETHER each one is an item path is + # booth.blur.check_rel's call, the same predicate the web route uses, so + # `booth blur g a..b.png` and the operator's click agree. (A `*..*` + # substring test here refused `a..b.png`, which the route accepted.) items=() for item in "$@"; do - item="${item#"$DATA/$b/"}"; item="${item#/}" - case "$item" in - *..*) echo "refusing path with '..': $item" >&2; exit 2 ;; - esac + item="${item#"$DATA/$b/"}" [ -e "$DATA/$b/$item" ] || echo "warning: no such item in $b: $item" >&2 items+=("$item") done - # ONE WRITER. `.blurred` is a JSON array now (a rel may carry a leading - # space or a newline, and the old line format could not round-trip it), and - # the service writes it too — so the CLI goes through the same stdlib-only - # booth.blur the service does, never a grep/printf of its own. Items travel - # as argv, which carries any byte but NUL; an env var or a line would not. - # An emptied set removes the file (booth.blur), so `ls -a` still says - # whether anything here is blurred at all. + # ONE WRITER. `.blurred.json` is a JSON array (a rel may carry a leading + # space or a newline, which the old `.blurred` line format could not + # round-trip), and the service writes it too, so the CLI goes through the + # same stdlib-only booth.blur, never a grep/printf of its own. Items travel + # as argv, which carries any byte but NUL. EVERY item is checked before ANY + # is written, so a refused path leaves the blur set exactly as it was. + # Exit 2: an item path refused. Exit 3: nothing written, and why (the + # package is missing, or something that is not a file is in the way). BOOTH_SRC="$(booth_src)" BOOTH_DIR="$DATA/$b" python3 -c ' import os, sys from pathlib import Path sys.path.insert(0, os.environ["BOOTH_SRC"]) -from booth.blur import set_blurred # stdlib only — no venv needed +try: + from booth.blur import BlurUnwritable, check_rel, set_blurred # stdlib only +except ImportError as exc: + src = os.environ["BOOTH_SRC"] + sys.stderr.write(f"booth blur: cannot load booth.blur from {src} ({exc}).\n" + " Run the booth script from its checkout, beside its booth/ package. Nothing was changed.\n") + sys.exit(3) on = sys.argv[1] == "blur" -for rel in sys.argv[2:]: - set_blurred(Path(os.environ["BOOTH_DIR"]), rel, on) +rels = [r.lstrip("/") for r in sys.argv[2:]] +for rel in rels: + try: + check_rel(rel) + except ValueError as exc: + sys.stderr.write(f"booth blur: refusing {rel!r}: {exc}. Nothing was changed.\n") + sys.exit(2) +for rel in rels: + try: + set_blurred(Path(os.environ["BOOTH_DIR"]), rel, on) + except BlurUnwritable as exc: + sys.stderr.write(f"booth blur: {exc}\n") + sys.exit(3) ' "$cmd" "${items[@]}" if [ "$cmd" = blur ]; then echo "blurred (cosmetic — still served): $URL/b/$b/" diff --git a/tests/mutations/blur_storage.toml b/tests/mutations/blur_storage.toml index d53a074..4275a86 100644 --- a/tests/mutations/blur_storage.toml +++ b/tests/mutations/blur_storage.toml @@ -33,54 +33,60 @@ label = "a JSON-only reader: every live line-format file un-blurs on deploy" file = "booth/blur.py" test = "tests/test_blur.py::test_the_legacy_line_format_still_reads" old = ''' - return {ln.strip() for ln in text.splitlines() if ln.strip()}''' + return {ln.strip() for ln in text.splitlines() if ln.strip()}''' new = ''' - return set()''' + return set()''' [[mutation]] label = "a legacy file that is not JSON reads as nothing instead of falling back" file = "booth/blur.py" test = "tests/test_blur.py::test_a_legacy_rel_that_starts_with_a_bracket_still_reads" old = ''' - data = None''' + text = raw.decode("utf-8", "surrogateescape") + return {ln.strip()''' new = ''' - return set()''' + text = raw.decode("utf-8", "surrogateescape") + try: + json.loads(text) + except ValueError: + return set() + return {ln.strip()''' [[mutation]] label = "a FIFO blocks the read (no O_NONBLOCK)" file = "booth/blur.py" test = "tests/test_blur.py::test_a_fifo_blur_file_does_not_block_the_read" old = ''' - fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' new = ''' - fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW)''' + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW)''' [[mutation]] label = "the read follows a planted symlink (no O_NOFOLLOW)" file = "booth/blur.py" test = "tests/test_blur.py::test_a_symlinked_blur_file_is_not_followed_on_read" old = ''' - fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' new = ''' - fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NONBLOCK)''' + fd = os.open(path, os.O_RDONLY | os.O_NONBLOCK)''' [[mutation]] label = "the write goes through a planted symlink instead of replacing it" file = "booth/blur.py" test = "tests/test_blur.py::test_a_write_replaces_a_planted_symlink_rather_than_writing_through_it" old = ''' - os.replace(tmp, path)''' + os.replace(tmp, path)''' new = ''' - path.write_bytes(Path(tmp).read_bytes()); os.unlink(tmp)''' + path.write_bytes(Path(tmp).read_bytes()); os.unlink(tmp)''' [[mutation]] label = "the stored order is not the stated one (invariant 6)" file = "booth/blur.py" test = "tests/test_blur.py::test_the_file_is_a_json_array_in_sorted_order" old = ''' - body = json.dumps(sorted(current), ensure_ascii=False)''' + body = json.dumps(sorted(current), ensure_ascii=False)''' new = ''' - body = json.dumps(sorted(current, reverse=True), ensure_ascii=False)''' + body = json.dumps(sorted(current, reverse=True), ensure_ascii=False)''' [[mutation]] label = "the CLI ignores the verb: `unblur` blurs" @@ -92,13 +98,13 @@ new = ''' on = True''' [[mutation]] -label = "the CLI writes past a refused '..' path" -file = "scripts/booth" +label = "the CLI writes past a refused '..' path (the shared predicate loses its component check)" +file = "booth/blur.py" test = "tests/test_blur.py::test_the_cli_still_refuses_a_dotdot_path" old = ''' - *..*) echo "refusing path with '..': $item" >&2; exit 2 ;;''' + if not rel or rel.startswith("/") or ".." in rel.split("/"):''' new = ''' - *..*) echo "refusing path with '..': $item" >&2 ;;''' + if not rel or rel.startswith("/"):''' [[mutation]] label = "the item's own blur is the composed one (booth fog leaks into it)" @@ -120,3 +126,87 @@ new = ''' out = [] read_blurred(child) for it in booth_items(child):''' + +# ---- the heid bug-hunt on this change (hulda, regin, kimi), folded ------------- + +[[mutation]] +label = "the legacy file is sniffed for JSON again (a `[\"a.png\"]` line blurs the neighbour)" +file = "booth/blur.py" +test = "tests/test_blur.py::test_a_legacy_line_that_is_valid_json_still_reads_as_a_line" +old = ''' + text = raw.decode("utf-8", "surrogateescape") + return {ln.strip()''' +new = ''' + text = raw.decode("utf-8", "surrogateescape") + try: + d = json.loads(text) + if isinstance(d, list): + return {r for r in d if isinstance(r, str)} + except ValueError: + pass + return {ln.strip()''' + +[[mutation]] +label = "no postcondition: a planted directory's OSError is swallowed as success" +file = "booth/blur.py" +test = "tests/test_blur.py::test_a_planted_directory_at_the_blur_file_is_a_refusal_not_a_crash" +old = ''' + if read_blurred(booth) != current:''' +new = ''' + if False:''' + +[[mutation]] +label = "the route turns a disk-state refusal into a 500" +file = "booth/app.py" +test = "tests/test_blur.py::test_the_route_answers_a_planted_directory_with_409" +old = ''' + raise HTTPException(status_code=409, detail=str(exc))''' +new = ''' + raise''' + +[[mutation]] +label = "a lone surrogate from a planted file reaches the writer" +file = "booth/blur.py" +test = "tests/test_blur.py::test_a_lone_surrogate_in_the_file_is_skipped_and_writes_still_work" +old = ''' + return {r for r in data if isinstance(r, str) and r and _encodable(r)}''' +new = ''' + return {r for r in data if isinstance(r, str) and r}''' + +[[mutation]] +label = "the writer writes a set the reader would refuse and read as nothing" +file = "booth/blur.py" +test = "tests/test_blur.py::test_the_writer_never_writes_a_set_the_reader_would_refuse" +old = ''' + if len(body) > BLUR_MAX_BYTES:''' +new = ''' + if False:''' + +[[mutation]] +label = "an empty item path is accepted and stored" +file = "booth/blur.py" +test = "tests/test_blur.py::test_the_cli_refuses_an_empty_item_path_before_writing" +old = ''' + if not rel or rel.startswith("/") or ".." in rel.split("/"):''' +new = ''' + if rel.startswith("/") or ".." in rel.split("/"):''' + +[[mutation]] +label = "a double dot INSIDE a name is refused (the old `*..*` substring rule)" +file = "booth/blur.py" +test = "tests/test_blur.py::test_the_cli_accepts_a_double_dot_inside_a_name" +old = ''' + if not rel or rel.startswith("/") or ".." in rel.split("/"):''' +new = ''' + if not rel or rel.startswith("/") or ".." in rel:''' + +[[mutation]] +label = "the CLI dies with a traceback when its package is missing" +file = "scripts/booth" +test = "tests/test_blur.py::test_the_cli_fails_closed_without_its_package" +old = ''' +except ImportError as exc: + src = os.environ["BOOTH_SRC"]''' +new = ''' +except ZeroDivisionError as exc: + src = os.environ["BOOTH_SRC"]''' diff --git a/tests/test_blur.py b/tests/test_blur.py index 13198cd..e742b92 100644 --- a/tests/test_blur.py +++ b/tests/test_blur.py @@ -2,10 +2,11 @@ `.blurred` was one stripped rel per line, so a rel with a leading space could not survive a write: blurring " a.png" stored "a.png", and toggled the -neighbour instead (heid bug-hunt on r2b merge 1, reported to booth-dev). It is -now a JSON array, the `.seen` shape, read without following a link or blocking -on a FIFO. The old line format is still READ, so nothing live changes until the -next write upgrades it. +neighbour instead (heid bug-hunt on r2b merge 1, reported to booth-dev). The set +now lives in `.blurred.json`, a JSON array (the `.seen` shape), read without +following a link or blocking on a FIFO. The legacy `.blurred` is still READ, as +lines, while no `.blurred.json` exists; the first write retires it. Two names, +so neither format is ever sniffed (heid bug-hunt on this change, 3 of 3 arms). Two writers share the file: the service (the operator's per-item control) and `scripts/booth blur` (a session at post time). Both go through `booth.blur`, @@ -21,11 +22,14 @@ import subprocess import sys import threading +import pytest + from fastapi.testclient import TestClient sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) from booth.app import BLUR_FILE, create_app, read_blurred, set_blurred # noqa: E402 +from booth.blur import BLUR_MAX_BYTES, LEGACY_BLUR_FILE, BlurUnwritable # noqa: E402 from booth.items import booth_items # noqa: E402 PNG = b"\x89PNG\r\n\x1a\n" @@ -97,21 +101,23 @@ def test_emptying_the_set_removes_the_file(tmp_path): def test_the_legacy_line_format_still_reads(tmp_path): """Six live booths hold line-format files. Defeating change: a JSON-only reader, which would un-blur every one of them on deploy.""" - (tmp_path / BLUR_FILE).write_text("a.png\nsub/b.png\n\n") + (tmp_path / LEGACY_BLUR_FILE).write_text("a.png\nsub/b.png\n\n") assert read_blurred(tmp_path) == {"a.png", "sub/b.png"} def test_a_legacy_rel_that_starts_with_a_bracket_still_reads(tmp_path): """A line-format file whose first rel happens to begin with "[" is not JSON, and must fall back to lines rather than read as nothing.""" - (tmp_path / BLUR_FILE).write_text("[draft] a.png\nb.png\n") + (tmp_path / LEGACY_BLUR_FILE).write_text("[draft] a.png\nb.png\n") assert read_blurred(tmp_path) == {"[draft] a.png", "b.png"} def test_a_write_upgrades_a_legacy_file_and_keeps_its_rels(tmp_path): - (tmp_path / BLUR_FILE).write_text("a.png\n") + """And retires the legacy file, so it can never speak again.""" + (tmp_path / LEGACY_BLUR_FILE).write_text("a.png\n") set_blurred(tmp_path, "b.png", True) assert json.loads((tmp_path / BLUR_FILE).read_text("utf-8")) == ["a.png", "b.png"] + assert not (tmp_path / LEGACY_BLUR_FILE).exists() # ---- a planted file: never blocks, never follows ---------------------------- @@ -225,3 +231,147 @@ def test_app_py_never_reads_the_blur_file_itself(): if isinstance(n, ast.Call) and getattr(n.func, "id", getattr(n.func, "attr", None)) == "read_blurred" ] assert calls == [] + + +# ---- the heid bug-hunt on this change (3 arms), folded ------------------------- + + +@pytest.mark.parametrize("line", ['["a.png"]', "[]", "[1,2]"]) +def test_a_legacy_line_that_is_valid_json_still_reads_as_a_line(tmp_path, line): + """3 of 3 arms. Sniffing one file for two formats misread a legacy file + whose ONE line is an item literally named like a JSON array: `["a.png"]` + read as {"a.png"}, un-blurring the item and blurring its neighbour — the bug + this change exists to fix, recreated by its migration. Defeating change: + trying JSON on the legacy file.""" + (tmp_path / LEGACY_BLUR_FILE).write_text(line + "\n") + assert read_blurred(tmp_path) == {line} + + +def test_a_stale_legacy_file_is_silent_once_the_current_one_exists(tmp_path): + (tmp_path / LEGACY_BLUR_FILE).write_text("old.png\n") + (tmp_path / BLUR_FILE).write_text('["new.png"]') + assert read_blurred(tmp_path) == {"new.png"} + + +def test_a_planted_directory_at_the_blur_file_is_a_refusal_not_a_crash(tmp_path): + """2 of 3 arms plus a third from another angle: the reader was hardened + against a planted directory, the writer was not, and `os.replace` onto a + directory raised IsADirectoryError through the route. Defeating change: + letting the OSError out of set_blurred.""" + (tmp_path / BLUR_FILE).mkdir() + with pytest.raises(BlurUnwritable): + set_blurred(tmp_path, "a.png", True) + assert (tmp_path / BLUR_FILE).is_dir(), "a planted directory is not ours to remove" + + +def test_unblurring_under_a_planted_directory_is_not_an_error(tmp_path): + """Nothing reads as blurred and nothing was asked to be: the reader agrees + with the request, so there is nothing to refuse.""" + (tmp_path / BLUR_FILE).mkdir() + assert set_blurred(tmp_path, "a.png", False) == set() + + +def test_a_planted_directory_at_the_legacy_name_does_not_block_a_write(tmp_path): + (tmp_path / LEGACY_BLUR_FILE).mkdir() + set_blurred(tmp_path, "a.png", True) + assert read_blurred(tmp_path) == {"a.png"} + + +def test_the_route_answers_a_planted_directory_with_409(tmp_path): + b = _booth(tmp_path, "g", {"a.png": PNG}) + (b / BLUR_FILE).mkdir() + c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False)) + r = c.post("/b/g/blur", data={"f": "a.png", "on": "1"}, follow_redirects=False) + assert r.status_code == 409 + + +def test_a_lone_surrogate_in_the_file_is_skipped_and_writes_still_work(tmp_path): + """hulda, execution-verified: `"\\ud800"` is a valid JSON string no filename + can produce, and the writer's UTF-8 encode raised on it, so one planted + escape froze the booth's blur. Defeating change: keeping every str member.""" + (tmp_path / BLUR_FILE).write_text('["\\ud800", "a.png"]') + assert read_blurred(tmp_path) == {"a.png"} + assert set_blurred(tmp_path, "b.png", True) == {"a.png", "b.png"} + + +@pytest.mark.parametrize("rel", ["", "/abs.png", "a/../b.png", "..", "\ud800.png"]) +def test_a_rel_that_is_not_an_item_path_is_refused(tmp_path, rel): + with pytest.raises(ValueError): + set_blurred(tmp_path, rel, True) + assert not (tmp_path / BLUR_FILE).exists() + + +def test_a_double_dot_inside_a_name_is_an_item_path(tmp_path): + """A `..` COMPONENT is an escape; `a..b.png` is a filename.""" + assert set_blurred(tmp_path, "a..b.png", True) == {"a..b.png"} + + +def test_the_route_refuses_an_empty_rel(tmp_path): + """kimi: `f="/"` stripped to "" and was stored as a member no item can have.""" + _booth(tmp_path, "g", {"a.png": PNG}) + c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False)) + r = c.post("/b/g/blur", data={"f": "/", "on": "1"}, follow_redirects=False) + assert r.status_code == 400 + assert not (tmp_path / "g" / BLUR_FILE).exists() + + +def test_the_writer_never_writes_a_set_the_reader_would_refuse(tmp_path, monkeypatch): + """2 of 3 arms: nothing capped the writer, the reader refuses a file over + the cap and reads it as EMPTY, so the write that crossed it revealed every + item. Defeating change: no size check before the write.""" + import booth.blur as blur + set_blurred(tmp_path, "a.png", True) + monkeypatch.setattr(blur, "BLUR_MAX_BYTES", len(b'["a.png"]') + 3) + with pytest.raises(BlurUnwritable): + set_blurred(tmp_path, "bbbbbbbb.png", True) + monkeypatch.setattr(blur, "BLUR_MAX_BYTES", BLUR_MAX_BYTES) + assert read_blurred(tmp_path) == {"a.png"}, "a refused write changed the set" + + +def test_the_cli_accepts_a_double_dot_inside_a_name(tmp_path): + """2 of 3 arms: the CLI's `*..*` substring guard refused `a..b.png`, which + the route accepts. One predicate now serves both.""" + b = _booth(tmp_path, "g", {"a..b.png": PNG}) + r = _cli(tmp_path, "blur", "g", "a..b.png") + assert r.returncode == 0, r.stderr + assert read_blurred(b) == {"a..b.png"} + + +def test_the_cli_refuses_an_empty_item_path_before_writing(tmp_path): + """regin: `booth blur g /` stored an empty member. Refused, and a valid + item named alongside it is not written either.""" + b = _booth(tmp_path, "g", {"a.png": PNG}) + r = _cli(tmp_path, "blur", "g", "a.png", "/") + assert r.returncode == 2 + assert read_blurred(b) == set() + + +def test_the_cli_refuses_a_planted_directory_with_a_message(tmp_path): + b = _booth(tmp_path, "g", {"a.png": PNG}) + (b / BLUR_FILE).mkdir() + r = _cli(tmp_path, "blur", "g", "a.png") + assert r.returncode == 3 + assert "Traceback" not in r.stderr and BLUR_FILE in r.stderr + + +def test_the_cli_fails_closed_without_its_package(tmp_path): + """kimi: the `link` verb says why and exits 3 when booth/ is missing; the + `blur` verb died with a bare traceback. Same deployment shape as + test_cli's link test: the script alone, no package beside it.""" + b = _booth(tmp_path, "g", {"a.png": PNG}) + lone = tmp_path / "lone" / "scripts" + lone.mkdir(parents=True) + (lone / "booth").write_text(SCRIPT.read_text()) + (lone / "booth").chmod(0o755) + env = {k: v for k, v in os.environ.items() if k != "PYTHONPATH"} + env.update(BOOTH_DATA_DIR=str(tmp_path), BOOTH_URL="http://booth.invalid") + r = subprocess.run([str(lone / "booth"), "blur", "g", "a.png"], capture_output=True, + text=True, env=env, cwd="/tmp", timeout=30) + assert r.returncode == 3 + assert "Traceback" not in r.stderr + assert read_blurred(b) == set() + + +def test_a_fifo_at_the_legacy_name_does_not_block_the_read(tmp_path): + os.mkfifo(tmp_path / LEGACY_BLUR_FILE) + assert _within(5, lambda: read_blurred(tmp_path)) == set()