fix(r3): fold heid's bug hunt — no link offers a pair that 404s, NUL booth names, a FIFO marker, encoded view-state names

Navigation was built from the review ring while the compare GET also demands
containment, so an outside symlink (which stays in the ring) was offered by
the strip, the steps, the review's Compare control and the flag landing, and
404ed on arrival. Every one is now built from the compare ring (the review
ring filtered by the same conjunction, _in_booth).

Two pre-existing gaps compare inherits, fixed at the source: resolve_booth
caught only OSError, so a NUL in the booth segment was a 500; record_view
opened its marker blocking, so a planted FIFO hung every look. Plus: the page
treats %73ide=a as side=a, and the subgrid engine floor is stated. Two
findings refuted (a chorded click mid-drag never fires pointerup, measured;
booth_items never yields an unquotable rel). r3.toml: 57 rows.
This commit is contained in:
vh
2026-09-24 14:39:06 -07:00
parent 23f1bdb41f
commit f8d136a521
7 changed files with 176 additions and 40 deletions
+44 -19
View File
@@ -466,8 +466,12 @@ def record_view(booth: Path) -> None:
# the utime is not decoration: the marker must read as NOW or the whole
# mechanism is a file nobody's clock looks at.
try:
# O_NONBLOCK: a FIFO planted at the marker would otherwise block this
# open forever with no reader — it cannot raise, so the swallow below
# never sees it, and the look costs the page after all (r3 heid bug
# hunt, hulda). Non-blocking, a reader-less FIFO fails with ENXIO.
fd = os.open(booth / VIEW_MARKER,
os.O_WRONLY | os.O_CREAT | os.O_NOFOLLOW, 0o644)
os.O_WRONLY | os.O_CREAT | os.O_NOFOLLOW | os.O_NONBLOCK, 0o644)
try:
os.utime(fd)
finally:
@@ -1124,7 +1128,9 @@ def create_app(
candidate = data_dir / name
try:
resolved = candidate.resolve()
except OSError:
except (OSError, ValueError):
# ValueError: an embedded NUL (`/b/g%00/…`) is not an OSError, and
# hostile input is a 404, never a 500 (r3 heid bug hunt, hulda)
raise HTTPException(status_code=404, detail="no such booth")
# resolved.parent must be the data dir itself — blocks symlink escape + nesting.
if resolved.parent != data_dir or not resolved.is_dir():
@@ -1547,7 +1553,8 @@ def create_app(
a, b = form.get("a"), form.get("b")
if isinstance(a, str) and a and isinstance(b, str) and b:
try:
ring = review_chain(booth_items(resolve_booth(name)))
booth = resolve_booth(name)
ring = _compare_ring(booth, booth_items(booth))
except HTTPException:
ring = []
if a in ring and b in ring:
@@ -1964,6 +1971,7 @@ def create_app(
members = [r for r in ring if by_rel[r].group == item.group]
group = {"key": item.group, "k": members.index(f) + 1, "n": len(members)}
open_now = open_marks(marks)
cring = _compare_ring(booth, items)
return templates.TemplateResponse(
request, "view.html", {
**common,
@@ -1985,10 +1993,12 @@ def create_app(
"is_last": pos == len(ring) - 1,
"tray": [x for x in film if x["flagged"]],
"back_url": f"/b/{quote(name, safe='')}/#item-{item.url}",
# R3 C2: this item against the NEXT in the ring (itself in a
# ring of one), keyed by rel like every compare URL
# R3 C2: this item against the NEXT in the compare ring
# (itself in a ring of one), keyed by rel like every compare
# URL. This item passed the containment check above, so it
# is in the compare ring.
"compare_url": (f"/b/{quote(name, safe='')}/compare?a={quote(f, safe='/')}"
f"&b={quote(ring[(pos + 1) % len(ring)], safe='/')}"),
f"&b={quote(cring[(cring.index(f) + 1) % len(cring)], safe='/')}"),
})
# .md renders, .txt/.log show as text — viewable in-booth, no download
@@ -2007,6 +2017,28 @@ def create_app(
url=f"/b/{quote(name, safe='')}/{quote(f, safe='/')}", status_code=307
)
def _in_booth(booth: Path, rel) -> bool:
"""The view route's rule for a path: it resolves, stays inside the
booth, and is a file. NEVER RAISES — it is asked once per ring item."""
if not isinstance(rel, str) or not rel:
return False
try:
target = (booth / rel).resolve()
# the separator matters: a sibling booth `g-extra` shares `g`'s prefix
return str(target).startswith(str(booth) + os.sep) and target.is_file()
except (OSError, ValueError):
# ValueError: an embedded NUL — hostile input, never a 500
return False
def _compare_ring(booth: Path, items) -> list[str]:
"""THE COMPARE RING (R3 C1): the review ring — item order, media only —
filtered to what a compare can open. `booth_items` follows symlinks, so
a link pointing outside the booth is in the review ring and compare
404s it; every compare link, the review's Compare control and the flag
landing are built from THIS list, so none offers a pair that 404s (heid
bug hunt, 3 of 4)."""
return [r for r in review_chain(items) if _in_booth(booth, r)]
def _compare_side(booth: Path, ring: list[str], rel) -> str:
"""One side of a compare, or a 404 (R3 C1). A CONJUNCTION: the view
route's resolve / containment / is_file check, AND membership of the
@@ -2014,14 +2046,7 @@ def create_app(
symlinks, so a link pointing outside the booth is IN the ring and only
containment refuses it. The view's check alone is not enough — it
renders a doc, and compare takes media only."""
if not isinstance(rel, str) or not rel:
raise HTTPException(status_code=404, detail="no such item")
try:
target = (booth / rel).resolve()
except (OSError, ValueError):
# ValueError: an embedded NUL — hostile input, a 404, never a 500
raise HTTPException(status_code=404, detail="no such item")
if not str(target).startswith(str(booth) + os.sep) or not target.is_file():
if not _in_booth(booth, rel):
raise HTTPException(status_code=404, detail="no such item")
if rel not in ring:
raise HTTPException(status_code=404, detail="no such item")
@@ -2043,14 +2068,14 @@ def create_app(
load, keeps them. They are `str`, not an int or a Literal, so an
unknown value reads as the default and never as a 422.
The filmstrip is the review ring in RING ORDER (the item order filtered
to media — the review's `film`).
The filmstrip is the compare ring in RING ORDER (the review's `film`:
the item order filtered to media, less what compare cannot open).
"""
booth = resolve_booth(name)
items = booth_items(booth)
ring = review_chain(items)
a = _compare_side(booth, ring, a)
b = _compare_side(booth, ring, b)
a = _compare_side(booth, review_chain(items), a)
b = _compare_side(booth, review_chain(items), b)
ring = _compare_ring(booth, items)
# A look records both — below the 404s, so only a real pair counts.
record_view(booth)
record_seen(booth, a, items)