fix(blur): fold the heid bug-hunt: two file names, a reader-judged writer, one predicate
The heid bug-hunt panel on 4cfbce5 (hulda, regin, kimi; groa timed out) found
four real defects in the round-trip fix, and three of its arms converged on the
worst: it re-created the bug it existed to fix.
- Two names, never a sniffed file (3/3). JSON went into the OLD `.blurred`, and
the reader guessed the format from the bytes, so a legacy file whose one line
is an item named `["a.png"]` read as {"a.png"} and blurred the neighbour. The
set now lives in `.blurred.json`, JSON only. The legacy `.blurred` is read as
lines only, and only while `.blurred.json` is absent; the first write retires
it, after the new file is in place.
- A planted directory is a 409, not a 500 (2/3 plus a third angle, executed by
the seat). The reader was hardened against it and the writer was not:
os.replace and unlink raised IsADirectoryError through the route. Now the
writer is judged by its reader: set_blurred re-reads after writing and raises
BlurUnwritable unless the set on disk is the set asked for. That one check
covers a directory at either name, a permission and a race.
- A lone surrogate is dropped on read (hulda, executed). `"\ud800"` is a valid
JSON string that no filename can produce, and the UTF-8 encode raised on it
at every later write.
- The writer respects the reader's size cap (2/3). Nothing capped the write,
and the reader reads an oversized file as EMPTY, which reveals everything.
- One predicate, check_rel, for the route and the CLI (2/3). The CLI's `*..*`
substring guard refused `a..b.png`, which the route accepts. It also refuses
an empty path now (regin, kimi), and every item is checked before any is
written.
- `booth blur` fails closed, with a message and exit 3, when its package is
missing (kimi), as `link` already does.
Declined, with reasons: the Item positional-constructor break (booth_items is
the only constructor, INV-1), the fdopen fd leak and the short read (not
constructible on a local filesystem, and the `.seen` shape), and
unreadable-reads-as-revealed (blur is cosmetic; the `.seen` posture).
blur_storage.toml: 20/20 proved. One row came back VACUOUS on its first run,
because `set() or X` is X, and was rewritten before counting.
This commit is contained in:
@@ -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"]'''
|
||||
|
||||
+157
-7
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user