diff --git a/CLAUDE.md b/CLAUDE.md index e0da8da..5aea5a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,10 +93,16 @@ JSON and blurs the neighbour, the bug being fixed (heid bug-hunt, 3 of 3). So 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. +**Reads lenient, writes strict; and a writer is judged by its reader.** The +renderer's `read_blurred` turns anything it cannot read into an empty set, +because a damaged file must cost the blur and never the page. The writer +builds on `_load`, the same parse, which REFUSES instead: a regular file it +cannot read (a permission, over the cap, not JSON) is never overwritten with a +set that forgot what it held. That is the `.marks.json` wipe again, and the +blur writer shipped without the guard for a night. After writing, +`set_blurred` re-reads and raises `BlurUnwritable` unless the reader returns +exactly the set asked for. The route answers either refusal with 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 diff --git a/booth/blur.py b/booth/blur.py index 460d861..c79e0a6 100644 --- a/booth/blur.py +++ b/booth/blur.py @@ -92,41 +92,67 @@ def _read_capped(path: Path) -> bytes | None: os.close(fd) +def _load(booth: Path) -> set[str]: + """The blur set, STRICTLY: raises BlurUnwritable for a REGULAR file at + either name that cannot be read as its format (a permission, over the size + cap, not JSON), where `read_blurred` would say "nothing blurred". The + writer builds on this; the renderer on the lenient one. One parse, two + postures, so they cannot disagree about what a file means, only about what + to do when it cannot be read. + + Only a regular file can hold a set anyone wrote. A link, a directory or a + FIFO at either name holds nothing to lose, so it reads as empty here too, + and whether the write can then land is `set_blurred`'s postcondition to + judge (a link is replaced; a directory is refused). + + ANYTHING at `.blurred.json` 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: no one could have meant them, and dropping them loses + nothing. + """ + current, legacy = booth / BLUR_FILE, booth / LEGACY_BLUR_FILE + for path in (current, legacy): + try: + st = os.lstat(path) + except FileNotFoundError: + continue + except OSError as exc: + raise BlurUnwritable(f"cannot stat {path.name} in {booth.name!r} ({exc})") from exc + if not stat.S_ISREG(st.st_mode): + return set() + raw = _read_capped(path) + if raw is None: + raise BlurUnwritable(f"{path.name} in {booth.name!r} is not a readable file of sane size") + text = raw.decode("utf-8", "surrogateescape") + if path is legacy: + return {ln.strip() for ln in text.splitlines() if ln.strip()} + try: + data = json.loads(text) + except (ValueError, RecursionError) as exc: + # RecursionError: a deeply nested array blows the parser's stack, + # and it is neither a ValueError nor an OSError (the `.seen` hole). + raise BlurUnwritable(f"{BLUR_FILE} in {booth.name!r} is not JSON") from exc + if not isinstance(data, list): + raise BlurUnwritable(f"{BLUR_FILE} in {booth.name!r} is not a JSON array") + return {r for r in data if isinstance(r, str) and r and _encodable(r)} + return set() + + 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 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. - - 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. + blocking, and refused unless it is a regular file of sane size. A damaged + file costs the blur, never the page. The WRITER does not get this leniency; + see `_load`. """ try: - os.lstat(booth / BLUR_FILE) - except FileNotFoundError: - raw = _read_capped(booth / LEGACY_BLUR_FILE) - if raw is None: - return set() - text = raw.decode("utf-8", "surrogateescape") - return {ln.strip() for ln in text.splitlines() if ln.strip()} - except OSError: + return _load(booth) + except BlurUnwritable: return set() - raw = _read_capped(booth / BLUR_FILE) - if raw is None: - return set() - try: - 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). - 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: @@ -150,6 +176,11 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]: 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. + WRITES ARE STRICT. The set it builds on comes from `_load`, which refuses + (BlurUnwritable, nothing changed) where the renderer's reader would say + "nothing blurred": a file it cannot read is never overwritten with a set + that forgot what it held. + 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, @@ -159,7 +190,10 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]: `booth blur`) can lose one toggle, as the line format could. """ check_rel(rel) - current = read_blurred(booth) + # STRICT, never `read_blurred`: an empty set from a file that could not be + # read would be written back over it, and whatever it held would be gone + # (the `.marks.json` wipe of 2026-09-21; groa: a cross-uid EACCES). + current = _load(booth) if on: current.add(rel) else: @@ -175,6 +209,9 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]: try: fd, tmp = tempfile.mkstemp(prefix=".blurred.", suffix=".tmp", dir=booth) try: + # mkstemp makes 0600; the line-format writer left 0644, and a + # reader under another uid must still see the set (groa). + os.fchmod(fd, 0o644) with os.fdopen(fd, "wb") as fh: fh.write(body) os.replace(tmp, path) diff --git a/persistent-memory.md b/persistent-memory.md index 21ff036..e7061e3 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -48,7 +48,11 @@ _As of 2026-09-23:_ `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 + unreadable-reads-as-revealed (the `.seen` posture). ⚠ **groa's late retry + exposed that the WRITER was building on the lenient reader**: an unreadable, + oversized or malformed `.blurred.json` read as empty and was then + overwritten, which is the `.marks.json` wipe. Fixed 2026-09-24: writes are + strict (`_load`), and the file is 0644 again, not mkstemp's 0600. `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 diff --git a/tests/mutations/blur_storage.toml b/tests/mutations/blur_storage.toml index 4275a86..887482d 100644 --- a/tests/mutations/blur_storage.toml +++ b/tests/mutations/blur_storage.toml @@ -33,29 +33,29 @@ 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 = ''' - text = raw.decode("utf-8", "surrogateescape") - return {ln.strip()''' + if path is legacy: + return {ln.strip()''' new = ''' - text = raw.decode("utf-8", "surrogateescape") - try: - json.loads(text) - except ValueError: - return set() - return {ln.strip()''' + if path is legacy: + 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" +test = "tests/test_blur.py::test_the_raw_read_never_blocks_on_a_fifo" old = ''' fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' new = ''' @@ -64,7 +64,7 @@ new = ''' [[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" +test = "tests/test_blur.py::test_the_raw_read_never_follows_a_link" old = ''' fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' new = ''' @@ -134,17 +134,17 @@ label = "the legacy file is sniffed for JSON again (a `[\"a.png\"]` line blurs t 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()''' + if path is legacy: + 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()''' + if path is legacy: + 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" @@ -169,9 +169,9 @@ 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)}''' + 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}''' + 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" @@ -210,3 +210,51 @@ except ImportError as exc: new = ''' except ZeroDivisionError as exc: src = os.environ["BOOTH_SRC"]''' + +# ---- reads lenient, writes strict (groa's retry; the .marks.json lesson) ------- + +[[mutation]] +label = "the writer builds on the lenient reader (an unreadable set is overwritten)" +file = "booth/blur.py" +test = "tests/test_blur.py::test_an_unreadable_blur_file_is_never_overwritten" +old = ''' + current = _load(booth)''' +new = ''' + current = read_blurred(booth)''' + +[[mutation]] +label = "an unreadable or oversized regular file reads as empty for the writer" +file = "booth/blur.py" +test = "tests/test_blur.py::test_an_oversized_blur_file_is_never_overwritten" +old = ''' + raise BlurUnwritable(f"{path.name} in {booth.name!r} is not a readable file of sane size")''' +new = ''' + return set()''' + +[[mutation]] +label = "a malformed set reads as empty for the writer" +file = "booth/blur.py" +test = "tests/test_blur.py::test_a_malformed_blur_file_is_never_overwritten" +old = ''' + raise BlurUnwritable(f"{BLUR_FILE} in {booth.name!r} is not JSON") from exc''' +new = ''' + return set()''' + +[[mutation]] +label = "the set is written 0600 (mkstemp's default)" +file = "booth/blur.py" +test = "tests/test_blur.py::test_the_blur_file_is_world_readable_as_it_always_was" +old = ''' + os.fchmod(fd, 0o644)''' +new = ''' + pass''' + +[[mutation]] +label = "a link or a FIFO at the name blocks the writer instead of reading as no set" +file = "booth/blur.py" +test = "tests/test_blur.py::test_a_write_replaces_a_planted_symlink_rather_than_writing_through_it" +old = ''' + if not stat.S_ISREG(st.st_mode): + return set()''' +new = ''' + pass''' diff --git a/tests/test_blur.py b/tests/test_blur.py index e742b92..e9d36d1 100644 --- a/tests/test_blur.py +++ b/tests/test_blur.py @@ -266,7 +266,8 @@ def test_a_planted_directory_at_the_blur_file_is_a_refusal_not_a_crash(tmp_path) 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.""" + with the request, so there is nothing to refuse. A directory holds no set, + so strict writes (below) have nothing to protect here.""" (tmp_path / BLUR_FILE).mkdir() assert set_blurred(tmp_path, "a.png", False) == set() @@ -375,3 +376,70 @@ def test_the_cli_fails_closed_without_its_package(tmp_path): 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() + + +# ---- reads lenient, writes strict (groa's retry, and marks' lesson) ------------ +# +# The reader turns anything it cannot read into an EMPTY set, which is right for +# rendering: a damaged file costs the blur, never the page. A writer that builds +# on that empty set then replaces the file, and whatever it could not read is +# gone. That is the `.marks.json` wipe of 2026-09-21 +# (persistent-memory.d/2026-09-21-marks-write-wiped-judgment.md), and a +# cross-uid reader that got EACCES would do it here (groa). + + +def test_an_unreadable_blur_file_is_never_overwritten(tmp_path): + """Defeating change: set_blurred building on the lenient reader.""" + set_blurred(tmp_path, "a.png", True) + before = (tmp_path / BLUR_FILE).read_bytes() + os.chmod(tmp_path / BLUR_FILE, 0) + try: + with pytest.raises(BlurUnwritable): + set_blurred(tmp_path, "b.png", True) + finally: + os.chmod(tmp_path / BLUR_FILE, 0o644) + assert (tmp_path / BLUR_FILE).read_bytes() == before + + +def test_a_malformed_blur_file_is_never_overwritten(tmp_path): + (tmp_path / BLUR_FILE).write_text("not json at all") + with pytest.raises(BlurUnwritable): + set_blurred(tmp_path, "a.png", True) + assert (tmp_path / BLUR_FILE).read_text() == "not json at all" + + +def test_an_oversized_blur_file_is_never_overwritten(tmp_path, monkeypatch): + import booth.blur as blur + set_blurred(tmp_path, "a.png", True) + before = (tmp_path / BLUR_FILE).read_bytes() + monkeypatch.setattr(blur, "BLUR_MAX_BYTES", 4) + with pytest.raises(BlurUnwritable): + set_blurred(tmp_path, "b.png", False) + assert (tmp_path / BLUR_FILE).read_bytes() == before + + +def test_the_blur_file_is_world_readable_as_it_always_was(tmp_path): + """groa: mkstemp creates 0600, where the line-format writer left 0644, so a + reader under another uid saw nothing. Defeating change: no chmod.""" + set_blurred(tmp_path, "a.png", True) + assert (tmp_path / BLUR_FILE).stat().st_mode & 0o777 == 0o644 + + +# The open flags are the SECOND layer: `_load` lstat-checks for a regular file +# first, so a FIFO or a link never reaches `os.open` through the public API, and +# a mutation run found the flags VACUOUS there. They still close the race (a +# file swapped for a FIFO or a link between the lstat and the open), so they +# are held to account directly, where nothing stands in front of them. + + +def test_the_raw_read_never_blocks_on_a_fifo(tmp_path): + from booth.blur import _read_capped + os.mkfifo(tmp_path / "f") + assert _within(5, lambda: _read_capped(tmp_path / "f")) is None + + +def test_the_raw_read_never_follows_a_link(tmp_path): + from booth.blur import _read_capped + (tmp_path / "real.json").write_text('["a.png"]') + (tmp_path / "link").symlink_to(tmp_path / "real.json") + assert _read_capped(tmp_path / "link") is None