fix(blur): writes are strict, so a set the writer cannot read is never overwritten

groa's late retry on the blur bug-hunt, adjudicated against the landed code.
Its four bugs were already fixed, but a robustness note (mkstemp's 0600 locks
out a reader under another uid, which then "sees nothing and replaces it")
pointed at a real gap. set_blurred built on read_blurred, the renderer's
lenient reader, which turns an unreadable, oversized or malformed
`.blurred.json` into an empty set. The writer then replaced the file, and
whatever it held was gone. This is the `.marks.json` wipe of 2026-09-21 in a
new module, and it shipped for a night.

- `_load` is the one parse with two postures. read_blurred maps its refusal to
  "nothing blurred" (a damaged file costs the blur, never the page).
  set_blurred lets it raise BlurUnwritable, which the route answers with 409
  and the CLI with exit 3, and changes nothing.
- It refuses only for a REGULAR file it cannot read. A link, a directory or a
  FIFO at either name holds no set anyone wrote, so it reads as empty, and the
  postcondition judges whether the write can land: a link is replaced, a
  directory refused.
- The file is 0644 again, as the line-format writer left it (fchmod after
  mkstemp).

The open flags in `_read_capped` became a second layer behind the new lstat
check, and the mutation run caught their rows VACUOUS through the public API.
They are now held to account by direct tests, because they still close the
lstat-to-open race. blur_storage.toml: 25/25. No second panel was run: this
folds one reviewer note plus the repo's own recorded lesson, with a test and
a proved row for each behaviour.
This commit is contained in:
vh
2026-09-24 00:56:25 -07:00
parent 7d4a26f486
commit 8a78a9bd1d
5 changed files with 220 additions and 57 deletions
+10 -4
View File
@@ -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 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. 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 **Reads lenient, writes strict; and a writer is judged by its reader.** The
raises `BlurUnwritable` unless the reader returns exactly the set asked for. renderer's `read_blurred` turns anything it cannot read into an empty set,
One postcondition covers a planted directory, a permission and a race without because a damaged file must cost the blur and never the page. The writer
a branch per way the disk can be wrong; the route answers it 409, never 500. 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 **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 predicate for its keys.** The blur set is written by the service and by `booth
+64 -27
View File
@@ -92,41 +92,67 @@ def _read_capped(path: Path) -> bytes | None:
os.close(fd) 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]: def read_blurred(booth: Path) -> set[str]:
"""Blurred rels for a booth. Missing, unreadable or malformed -> empty set. """Blurred rels for a booth. Missing, unreadable or malformed -> empty set.
NEVER RAISES and NEVER BLOCKS. `booth_items` calls this for every booth the 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 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 may be planted: each is opened without following a link and without
blocking, and refused unless it is a regular file of sane size. 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;
ANYTHING at `.blurred.json` (a link or a directory included) means the see `_load`.
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: try:
os.lstat(booth / BLUR_FILE) return _load(booth)
except FileNotFoundError: except BlurUnwritable:
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 set() 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: 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 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. `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 SUCCESS IS DEFINED BY THE READER. After writing, `read_blurred` must return
exactly the set asked for; anything else raises BlurUnwritable. That one exactly the set asked for; anything else raises BlurUnwritable. That one
check covers a planted directory at either name, a permission, and a race, 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. `booth blur`) can lose one toggle, as the line format could.
""" """
check_rel(rel) 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: if on:
current.add(rel) current.add(rel)
else: else:
@@ -175,6 +209,9 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]:
try: try:
fd, tmp = tempfile.mkstemp(prefix=".blurred.", suffix=".tmp", dir=booth) fd, tmp = tempfile.mkstemp(prefix=".blurred.", suffix=".tmp", dir=booth)
try: 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: with os.fdopen(fd, "wb") as fh:
fh.write(body) fh.write(body)
os.replace(tmp, path) os.replace(tmp, path)
+5 -1
View File
@@ -48,7 +48,11 @@ _As of 2026-09-23:_
`a..b.png` and refuses an empty path, and a missing package fails closed. `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 Declined: the `Item` positional-constructor break (booth_items is the only
constructor, INV-1), the fdopen fd leak, the short read, and 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:** 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 "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 0/1), and the CLI's `.blurbooth` `touch` still follows a symlink where the
+72 -24
View File
@@ -33,29 +33,29 @@ label = "a JSON-only reader: every live line-format file un-blurs on deploy"
file = "booth/blur.py" file = "booth/blur.py"
test = "tests/test_blur.py::test_the_legacy_line_format_still_reads" test = "tests/test_blur.py::test_the_legacy_line_format_still_reads"
old = ''' old = '''
return {ln.strip() for ln in text.splitlines() if ln.strip()}''' return {ln.strip() for ln in text.splitlines() if ln.strip()}'''
new = ''' new = '''
return set()''' return set()'''
[[mutation]] [[mutation]]
label = "a legacy file that is not JSON reads as nothing instead of falling back" label = "a legacy file that is not JSON reads as nothing instead of falling back"
file = "booth/blur.py" file = "booth/blur.py"
test = "tests/test_blur.py::test_a_legacy_rel_that_starts_with_a_bracket_still_reads" test = "tests/test_blur.py::test_a_legacy_rel_that_starts_with_a_bracket_still_reads"
old = ''' old = '''
text = raw.decode("utf-8", "surrogateescape") if path is legacy:
return {ln.strip()''' return {ln.strip()'''
new = ''' new = '''
text = raw.decode("utf-8", "surrogateescape") if path is legacy:
try: try:
json.loads(text) json.loads(text)
except ValueError: except ValueError:
return set() return set()
return {ln.strip()''' return {ln.strip()'''
[[mutation]] [[mutation]]
label = "a FIFO blocks the read (no O_NONBLOCK)" label = "a FIFO blocks the read (no O_NONBLOCK)"
file = "booth/blur.py" 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 = ''' old = '''
fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)'''
new = ''' new = '''
@@ -64,7 +64,7 @@ new = '''
[[mutation]] [[mutation]]
label = "the read follows a planted symlink (no O_NOFOLLOW)" label = "the read follows a planted symlink (no O_NOFOLLOW)"
file = "booth/blur.py" 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 = ''' old = '''
fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)''' fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)'''
new = ''' new = '''
@@ -134,17 +134,17 @@ label = "the legacy file is sniffed for JSON again (a `[\"a.png\"]` line blurs t
file = "booth/blur.py" file = "booth/blur.py"
test = "tests/test_blur.py::test_a_legacy_line_that_is_valid_json_still_reads_as_a_line" test = "tests/test_blur.py::test_a_legacy_line_that_is_valid_json_still_reads_as_a_line"
old = ''' old = '''
text = raw.decode("utf-8", "surrogateescape") if path is legacy:
return {ln.strip()''' return {ln.strip()'''
new = ''' new = '''
text = raw.decode("utf-8", "surrogateescape") if path is legacy:
try: try:
d = json.loads(text) d = json.loads(text)
if isinstance(d, list): if isinstance(d, list):
return {r for r in d if isinstance(r, str)} return {r for r in d if isinstance(r, str)}
except ValueError: except ValueError:
pass pass
return {ln.strip()''' return {ln.strip()'''
[[mutation]] [[mutation]]
label = "no postcondition: a planted directory's OSError is swallowed as success" 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" file = "booth/blur.py"
test = "tests/test_blur.py::test_a_lone_surrogate_in_the_file_is_skipped_and_writes_still_work" test = "tests/test_blur.py::test_a_lone_surrogate_in_the_file_is_skipped_and_writes_still_work"
old = ''' 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 = ''' 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]] [[mutation]]
label = "the writer writes a set the reader would refuse and read as nothing" label = "the writer writes a set the reader would refuse and read as nothing"
@@ -210,3 +210,51 @@ except ImportError as exc:
new = ''' new = '''
except ZeroDivisionError as exc: except ZeroDivisionError as exc:
src = os.environ["BOOTH_SRC"]''' 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'''
+69 -1
View File
@@ -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): 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 """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() (tmp_path / BLUR_FILE).mkdir()
assert set_blurred(tmp_path, "a.png", False) == set() 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): def test_a_fifo_at_the_legacy_name_does_not_block_the_read(tmp_path):
os.mkfifo(tmp_path / LEGACY_BLUR_FILE) os.mkfifo(tmp_path / LEGACY_BLUR_FILE)
assert _within(5, lambda: read_blurred(tmp_path)) == set() 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