diff --git a/services/booth/README.md b/services/booth/README.md index 693f0ca..0abb538 100644 --- a/services/booth/README.md +++ b/services/booth/README.md @@ -70,6 +70,16 @@ figures across mixed kinds and will fail if you don't. tells the truth about whether anything here is blurred. - **Reveal is per-viewer and never persisted.** Click ๐Ÿ‘ reveal; a reload re-hides. With JS off it stays blurred, which is the safe direction to fail. + + โš  This button shipped INERT on 2026-09-20 and stayed that way for a day. Its + handler sat after the content block's closing tag, and a child template's + out-of-block content is silently DISCARDED by Jinja โ€” the button rendered, + the handler never reached the browser, and two commits plus this README said + it worked. The suite passed throughout because nothing asserted against the + served page. `test_reveal_handler_actually_reaches_the_served_page` now greps + the HTTP RESPONSE, and `test_no_orphaned_markup_after_the_content_block` + guards the structure. Both were confirmed to FAIL when the defect is + reintroduced, which is the only way to know a guard guards anything. - **Covers inherit it.** If a booth's cover image is blurred, the index card's thumb is blurred too โ€” otherwise the front page undoes the censoring. - **Inline docs are blurred too**, not just images and video. That branch puts @@ -77,8 +87,25 @@ figures across mixed kinds and will fail if you don't. ## Keeping a booth (round trip, both directions) -`โ˜…` on an ephemeral card promotes it to the kept lane; `release` in the kept -lane sends it back. Equivalent CLI: `booth keep ` / `booth unkeep `. +Three places, all doing the same thing: + +- **Index, ephemeral card** โ€” `โ˜…` promotes to the kept lane. +- **Index, kept card** โ€” `release` demotes, and `ร—` now WIPES DIRECTLY. The old + rule was release-then-find-it-in-the-other-lane; that protected nothing and + cost a hunt, because the board you just released is loose in a feed that + turns over. Protection lives in the confirmation now, which names the booth + and says KEPT. +- **Inside a booth** โ€” `โ˜† keep` / `โ˜… kept โ€” release`, beside *Wipe now*, so you + do not have to go back to the index. These post a `next` field to stay on the + page; `next` is a form field and therefore attacker-controlled, so only + same-site absolute paths are honoured (`//host`, schemes and backslashes are + refused). + +Equivalent CLI: `booth keep ` / `booth unkeep `. + +โš  Release BUMPS the directory mtime, so a released board's age resets and it +survives another full TTL. Unkeep-and-wait is a 24h delay, not a delete โ€” which +is exactly why the direct `ร—` was worth adding. โš  Until 2026-09-19 the UI only went one way โ€” the kept lane could release, but an ephemeral booth could only be kept from a shell. The `/keep` route and the diff --git a/services/booth/booth/app.py b/services/booth/booth/app.py index 624bccb..4f59e32 100644 --- a/services/booth/booth/app.py +++ b/services/booth/booth/app.py @@ -704,6 +704,9 @@ def create_app( **base_ctx, "name": name, "name_url": quote(name, safe=""), + # The page could not previously tell keep from release, so it + # offered neither and you had to go back to the index. + "kept": is_kept(booth), # links.md is rendered AS the board below, so it must not also # appear as a markdown doc tile โ€” that would show the same # content twice, once interactive and once not. @@ -992,16 +995,25 @@ def create_app( toggle_pin(resolve_booth(name), entry) return RedirectResponse(url=f"/b/{quote(name, safe='')}/", status_code=303) + def _safe_next(nxt: str) -> str: + """Where to land after keep/unkeep. Defaults to the index; a booth page + can ask to stay put. Only same-site absolute paths are honoured โ€” `//` + and any scheme are refused, because a redirect target taken from a form + field is an open redirect if you do not check it.""" + if nxt.startswith("/") and not nxt.startswith("//") and "\\" not in nxt: + return nxt + return "/" + @app.post("/b/{name}/keep") - def booth_keep(name: str): + def booth_keep(name: str, next: str = Form("/")): (resolve_booth(name) / KEEP_MARKER).touch() - return RedirectResponse(url="/", status_code=303) + return RedirectResponse(url=_safe_next(next), status_code=303) @app.post("/b/{name}/unkeep") - def booth_unkeep(name: str): + def booth_unkeep(name: str, next: str = Form("/")): # missing_ok: releasing an already-released board is a no-op, not a 500. (resolve_booth(name) / KEEP_MARKER).unlink(missing_ok=True) - return RedirectResponse(url="/", status_code=303) + return RedirectResponse(url=_safe_next(next), status_code=303) @app.post("/b/{name}/blur") def booth_blur(name: str, f: str = Form(...), on: str = Form("1")): diff --git a/services/booth/booth/templates/base.html b/services/booth/booth/templates/base.html index 5b77584..ffc6bef 100644 --- a/services/booth/booth/templates/base.html +++ b/services/booth/booth/templates/base.html @@ -253,6 +253,20 @@ /* โ˜… keep, mirroring .wipe on the other shoulder of the card. Same hover-to-reveal language as .release in the kept lane. */ .keepit{position:absolute;top:.5rem;left:.5rem;margin:0;opacity:0;transition:opacity .12s} + /* ร— on a KEPT card. Same shoulder as the ephemeral ร—, deliberately tinted so + it does not read as the same weight of action. */ + .wipe-kept{position:absolute;top:.5rem;right:.5rem;margin:0;opacity:0;transition:opacity .12s} + .card-kept:hover .wipe-kept,.wipe-kept:focus-within{opacity:1} + .wipe-kept button{font:inherit;line-height:1;cursor:pointer;border:1px solid var(--line); + border-radius:.3rem;padding:.02rem .3rem;background:var(--bg);color:var(--muted)} + .wipe-kept button:hover{background:var(--aus-red,#ff6b6b);color:var(--fg-on-accent,#fff); + border-color:var(--aus-red,#ff6b6b)} + /* keep / release from inside a booth, beside "Wipe now" */ + .keep-lg{display:inline-block;margin:0 .4rem 0 0} + .keep-lg button{font:inherit;font-size:.8rem;line-height:1;padding:.32rem .6rem; + cursor:pointer;border:1px solid var(--line);border-radius:.3rem; + background:var(--bg);color:var(--fg)} + .keep-lg button:hover{background:var(--aus-blue);color:var(--fg-on-accent)} .card:hover .keepit,.keepit:focus-within{opacity:1} .keepit button{font:inherit;font-size:.9rem;line-height:1;padding:.1rem .34rem; cursor:pointer;border:1px solid var(--line);border-radius:.3rem; diff --git a/services/booth/booth/templates/booth.html b/services/booth/booth/templates/booth.html index 9830a0f..4958aea 100644 --- a/services/booth/booth/templates/booth.html +++ b/services/booth/booth/templates/booth.html @@ -23,6 +23,19 @@ {# A durable multi-writer board gets no one-click wipe โ€” same rule as the kept lane on the index. Remove rows with the per-row ร—, or release the board from the index and wipe it from there. #} + {# Promote or release without going back to the index. `next` keeps you on + this page instead of bouncing you to /. #} + {% if kept %} +
+ + +
+ {% else %} +
+ + +
+ {% endif %} {% if not board %}
@@ -285,17 +298,22 @@ refresh(); })(); - -{% endblock %} - +{% endblock %} diff --git a/services/booth/booth/templates/index.html b/services/booth/booth/templates/index.html index efabb84..3e58a2b 100644 --- a/services/booth/booth/templates/index.html +++ b/services/booth/booth/templates/index.html @@ -40,17 +40,26 @@ {{ b.name }}
{{ b.count }} item{{ '' if b.count == 1 else 's' }} ยท kept ยท โฌ‡ zip
- {# Still no ร— here โ€” a one-click wipe next to the durable stuff is a - footgun. But "deliberate" must not mean "impossible from the UI", - which is what it meant before: the only routes out were ssh or a - hand-written API call. Release drops the sentinel and the board moves - to the ephemeral lane, where the ร— already lives. Two deliberate - acts, both reachable, and the first one is reversible. + {# There IS a ร— here now (operator, 2026-09-21). The old rule was + release-then-find-it-in-the-other-lane, on the theory that two + deliberate acts protect durable boards. In practice it protects + nothing and costs a hunt: the board you just released is loose in a + feed that turns over, and you have to go find it to finish the job + you had already decided on. - The confirm says "wipe it from there" rather than "let it expire" on - purpose: releasing BUMPS the directory mtime, so the board's age - resets and it survives another full TTL. Unkeep-and-wait is a 24h - delay, not a delete. #} + The protection now lives in the CONFIRMATION, not in the number of + lanes you must traverse โ€” this one names the booth and says the word + KEPT, where the ephemeral ร— just asks. A deliberate act, one click, + reachable. + + Release still exists and is still the reversible option. Note it + BUMPS the directory mtime, so the board's age resets and it survives + another full TTL โ€” unkeep-and-wait is a 24h delay, not a delete, + which is exactly why a direct ร— was worth adding. #} + + +
diff --git a/services/booth/tests/test_booth.py b/services/booth/tests/test_booth.py index ecc89e4..47ab88c 100644 --- a/services/booth/tests/test_booth.py +++ b/services/booth/tests/test_booth.py @@ -1487,3 +1487,95 @@ def test_blur_toggle_posts_the_opposite_state(client): body = c.get("/b/bo/").text assert 'name="on" value="0"' in body, "a blurred item must offer un-blur" assert "โ—‰ blurred" in body + + +# --------------------------------------------------------------------------- +# Operator-reported, 2026-09-21: "the reveal button doesn't do anything". +# It rendered and was inert โ€” the handler sat after {%- endblock -%} in a child +# template, which Jinja DISCARDS. Two commits and a README claimed +# click-to-reveal worked. The suite passed the whole time because nothing +# asserted against the SERVED page. +# --------------------------------------------------------------------------- + + +def test_reveal_handler_actually_reaches_the_served_page(client): + """The regression that matters. Assert the handler is IN THE RESPONSE, not + that the template file contains the text โ€” the template contained it fine + and the browser never saw it.""" + c, root = client + d = root / "bo" + d.mkdir() + _png(d / "x.png") + set_blurred(d, "x.png", True) + + body = c.get("/b/bo/").text + assert 'class="reveal"' in body, "the button must render" + assert "classList.toggle('revealed')" in body, ( + "the handler must reach the page โ€” a button with no handler is a dead " + "control, which is exactly what shipped" + ) + + +def test_no_orphaned_markup_after_the_content_block(client): + """Structural guard for the same defect class: anything a child template + puts outside a block is silently dropped, so the only safe number of + closing content-block tags is one, at the very end.""" + tpl = (pathlib.Path(__file__).parent.parent + / "booth" / "templates" / "booth.html").read_text() + after = tpl[tpl.rindex("{% endblock %}") + len("{% endblock %}"):] + assert after.strip() == "", ( + f"content after the final endblock is discarded by Jinja: {after[:120]!r}" + ) + + +def test_booth_page_offers_keep_when_ephemeral_and_release_when_kept(client): + """Operator: 'adding a keep button inside a booth'. Both states, because a + control that always says the same thing cannot be driving off real state.""" + c, root = client + d = root / "bo" + d.mkdir() + _png(d / "x.png") + + body = c.get("/b/bo/").text + assert "โ˜† keep" in body and "release" not in body.split("boothhead")[1][:900] + + c.post("/b/bo/keep", data={"next": "/b/bo/"}, follow_redirects=False) + body = c.get("/b/bo/").text + assert "โ˜… kept โ€” release" in body + + +def test_keep_from_inside_a_booth_stays_on_the_booth_page(client): + """Without `next` the route redirects to /, which throws you out of the + booth you were reading.""" + c, root = client + (root / "bo").mkdir() + r = c.post("/b/bo/keep", data={"next": "/b/bo/"}, follow_redirects=False) + assert r.headers["location"] == "/b/bo/" + r = c.post("/b/bo/keep", follow_redirects=False) # no next + assert r.headers["location"] == "/" + + +def test_next_refuses_an_open_redirect(client): + """`next` comes from a form field, so it is attacker-controlled input.""" + c, root = client + (root / "bo").mkdir() + for evil in ("//evil.example/x", "https://evil.example/x", "\\\\evil"): + r = c.post("/b/bo/keep", data={"next": evil}, follow_redirects=False) + assert r.headers["location"] == "/", f"{evil!r} must not be honoured" + + +def test_kept_lane_offers_a_direct_wipe_beside_release(client): + """Operator: 'allow an x to delete next to release so i don't have to + release and then find it to delete it.'""" + c, root = client + d = root / "bo" + d.mkdir() + _png(d / "x.png") + (d / KEEP_MARKER).touch() + + body = c.get("/").text + assert 'class="wipe wipe-kept"' in body, "kept card must offer a direct ร—" + assert 'action="/b/bo/unkeep"' in body, "release must still be there too" + # ...and it actually wipes. + c.post("/b/bo/delete", follow_redirects=False) + assert not d.exists()