memory: snapshot for shutdown — nothing in flight; the r3 seam pass and the upload-name lessons split to detail files

2026-09-25 10:51:51 -07:00
parent 1a59242709
commit 34ac1683ee
3 changed files with 106 additions and 15 deletions
@@ -0,0 +1,40 @@
+# The r3 seam pass: what only it could see
+
+_2026-09-24. Contract `docs/contracts/r3_compare.contract.md` (design-dev);
+althing thread `01M3952NCDRRJX5XDFSPMSP5HJ`._
+
+design-dev's heid contract panel read r3 cold. Our seam pass read it against
+the real `app.py`, `items.py`, `base.html` and `view.html`, plus small probes
+on a scratch booth. It found 12 mismatches, all folded before a line of code.
+The build then went through heid code-review and bug-hunt, and our own gate,
+with nothing structural left. design-dev: "S4 and S8 would each have cost a
+round."
+
+## The ones worth remembering, because they recur
+
+- **`data-region` ids must be unique on a page.** The in-place client's
+  `swap()` in `base.html` keeps only the FIRST fresh node for each id, then
+  replaces EVERY live node that has that id with a copy of it. Two per-side
+  regions sharing an id would make B's flag button a copy of A's after any
+  save, so pressing B flags A, with nothing to show it happened. Also: do not
+  key a region by rel (a pair with `a == b` duplicates it) and do not prefix
+  it `item-` (swap reads a missing `item-*` as a stale tile, not a reload).
+  This is a mechanical rule that belongs in CLAUDE.md; it is not there yet.
+- **Moving code breaks mutation anchors.** 21 of r2c's 24 `view.html` rows
+  were anchored inside the inline script that r3 moved to `_stage_js.html`,
+  and `mutation_check.py` fails a row whose anchor is gone. A contract that
+  moves code must list the re-pointed rows in its "Assertions that change"
+  table, and gate on the table still falsifying, not only on the suite staying
+  green.
+- **"404 the way view does" was two rules, and neither was view's.** A
+  missing `f` is a 422 from FastAPI (required `str`), not a 404; declare
+  `= ""` and 404 by hand. `booth_items` follows symlinks, so a link pointing
+  outside the booth is IN `review_chain`, while view 404s it through
+  resolve plus containment. Compare needed the conjunction.
+- **Route order.** `/b/{name}/{filepath:path}` is a catch-all; a new
+  `/b/{name}/<word>` route must register before it. A booth file literally named
+  `<word>` becomes unreachable (accepted, as for view/marks/asks/embed.json).
+- **A resolve done twice is a race.** After the merge, the review route checked
+  `f` and then resolved the whole ring again, so `cring.index(f)` could raise
+  (a 500) when a file vanished in between. design-dev's `d54bb04` judges each
+  rel once per request.
@@ -0,0 +1,55 @@
+# Upload names: two crashes found, then two holes in the fix
+
+_2026-09-24. Commits `92c774e`, `225ba32`; heid bug-hunt thread `01M3AXG27KQDMA6P3RTAAYMEHP`._
+
+## What was wrong
+
+design-dev's r3 bug hunt (hulda) found that `/upload` returned a 500 for a
+multipart filename carrying a NUL: `safe_upload_name` stripped path, dots and
+length but not NUL, and `(dest / name).open("wb")` raised ValueError. The
+upload's `except Exception` tore the booth down and re-raised.
+
+Fixing it turned up a second 500 on the same line: the cap was
+`base[:200]`, 200 CHARACTERS. NAME_MAX is 255 BYTES, so 200 two-byte
+characters (`é`, or any CJK name) raised ENAMETOOLONG.
+
+## The first fix, and the two holes a single arm found in it
+
+`92c774e` stripped NUL first, then applied the dot rule, then capped at 200
+UTF-8 bytes via `encode("utf-8", "surrogatepass")[:200].decode("utf-8",
+"ignore")`, taking the cut out of the stem so the extension survived.
+
+Heid's own review of that diff found nothing. hulda, reading the whole snapshot
+against the declared invariants, found:
+
+1. **The surrogate was dropped LAST.** The final `decode("ignore")` removed a
+   lone surrogate after `lstrip(".")` had already run, so `"\ud800.forever"`
+   came out as `.forever`, which is the keep marker, and `"\ud800.."` as `..`.
+   The NUL had been moved first for exactly this reason; the same discipline
+   was not applied to the other droppable class. It was not reachable over HTTP:
+   Starlette decodes a multipart filename strictly (utf-8, else latin-1), so it
+   never yields a lone surrogate. The fix made the helper right by construction
+   anyway.
+2. **A cut could manufacture a kind.** A suffix too long to keep (>16 bytes)
+   was cut like text, and the cut could land on a shorter suffix that means
+   something: `"a"*196 + ".png" + "x"*17` became `….png`, an image.
+3. The 16-byte extension threshold was unguarded: every test suffix was 4
+   bytes, so `<= 4` survived. A `.jpeg` case now pins it.
+
+`225ba32` does one pass first (NUL and everything unencodable), then basename
+and the dot rule, then the byte cap. A cut whose `classify`/`doc_kind` differs
+from the original's has its dots replaced with `_`. Falsifiers:
+`tests/mutations/upload_names.toml`, 7/7.
+
+## The lessons
+
+- **A sanitiser drops everything droppable FIRST, then applies the structural
+  rules.** Anything dropped after a rule can defeat that rule.
+- **A NUL test through httpx `files=` proves nothing.** httpx
+  percent-escapes the NUL, so the server sees a literal `%00`. Post a raw
+  multipart body. The first integration test passed pre-fix for this reason.
+- **Truncating by length can change a file's meaning.** In the Booth the kind
+  comes from the extension, so a cut has to preserve the kind, not only the
+  byte count.
+- **The single-arm hunt earned its cost.** Heid's own read traced only the
+  hunks; hulda read the declared invariants against the whole bundle.