diff --git a/README.md b/README.md index b37e3ff..56a2df0 100644 --- a/README.md +++ b/README.md @@ -103,9 +103,11 @@ Publish a video, audio takes or images and get a link; whoever has it watches, r heed review publish cut1.mp4 --title "Cut 1" --note-file notes.md # prints the link heed review publish cut2.mp4 --to # a new cut, same link and thread heed review comments --json # read the feedback back +heed review publish cut2.mp4 --to --password-file pw.txt --expires 7d # gate it (pip install 'heed[gate]') +heed review revoke # end the link for everyone ``` -The link is a capability (128 random bits in the URL fragment); the thread endpoint is rate-limited, capped and honeypotted. Serve it with `make_review_app(rootdir, is_moderator=email_moderator([...]))`, or `heed review serve`. On an origin shared with other apps, a login is ambient authority (any script there acts as the owner), so require a typed-in secret instead: `is_moderator=token_moderator(secret, also=email_moderator([...]))` (`heed review serve --moderator-token-env VAR`); the review page's *Moderate* button asks for it and keeps it in memory only. A dedicated origin (e.g. `review.`) is the stronger fix and is a deployment decision. The reusable part is `heed.threads` (a hardened comment thread per key, `make_thread_router`); `heed.review` is one consumer of it. Agents: the shipped skill `heed/data/skills/heed-review`. Rationale: [`misc/docs/adr/0001-review-pages-and-comment-threads-live-in-heed.md`](misc/docs/adr/0001-review-pages-and-comment-threads-live-in-heed.md). +A link sent to a client can carry a password, an expiry and a revoke (`heed.review_access`, on [caplinks](https://github.com/i2mint/caplinks); the `gate` extra): the page answers 401 until unlocked with the password (a cookie; wrong tries limited per client and per page), 410 once expired, 404 once revoked, and pages without any of it stay open as before. The link is a capability (128 random bits in the URL fragment); the thread endpoint is rate-limited, capped and honeypotted. Serve it with `make_review_app(rootdir, is_moderator=email_moderator([...]))`, or `heed review serve`. On an origin shared with other apps, a login is ambient authority (any script there acts as the owner), so require a typed-in secret instead: `is_moderator=token_moderator(secret, also=email_moderator([...]))` (`heed review serve --moderator-token-env VAR`); the review page's *Moderate* button asks for it and keeps it in memory only. A dedicated origin (e.g. `review.`) is the stronger fix and is a deployment decision. The reusable part is `heed.threads` (a hardened comment thread per key, `make_thread_router`); `heed.review` is one consumer of it. Agents: the shipped skill `heed/data/skills/heed-review`. Rationale: [`misc/docs/adr/0001-review-pages-and-comment-threads-live-in-heed.md`](misc/docs/adr/0001-review-pages-and-comment-threads-live-in-heed.md). ## Documents diff --git a/heed/data/skills/heed-review/SKILL.md b/heed/data/skills/heed-review/SKILL.md index 635c5a1..029fbf3 100644 --- a/heed/data/skills/heed-review/SKILL.md +++ b/heed/data/skills/heed-review/SKILL.md @@ -19,6 +19,7 @@ It prints the link (`https://apps.thorwhalen.com/review/#` on tw). The ` - Video, audio and images only (anything else is refused). Several files go on one page, one player each, in the order given. There is no folder argument; use a glob. - Every player has a **Download** link (`.../media/?download=1`, served as an attachment), so whoever holds the link can save the file; publish nothing you would not hand them as a file. - **A new version goes on the same page**: `heed review publish cut2.mp4 --to `. The link and the thread stay; a file with the same name replaces the old one. +- **A link for a client**: `--password-file pw.txt` and/or `--expires 7d` on `publish` (extra `heed[gate]`); `heed review revoke ` ends it for everyone. Never pass a password as a command-line argument. - The note is shown as plain text above the media (a `.md` reads fine as text). ## Read the comments back diff --git a/heed/review.py b/heed/review.py index e143c28..7f6de4b 100644 --- a/heed/review.py +++ b/heed/review.py @@ -19,6 +19,8 @@ pages/.json the page: title, note, media list, created_at media// the media files, served with byte ranges (seekable) comments//.json the thread (heed.threads) + access/.json optional password, expiry, revoke (heed.review_access); + no file means open: back this folder up too Python:: @@ -43,6 +45,7 @@ import re import secrets import shutil +import sys from collections.abc import Callable, Iterable, MutableMapping from datetime import datetime, timezone from importlib.resources import files as _package_files @@ -52,6 +55,7 @@ from fastapi.responses import FileResponse, HTMLResponse from pydantic import BaseModel, Field +from heed.review_access import ReviewGate, revoke_access, set_access from heed.storage import make_report_store from heed.threads import ( ID_PATTERN, @@ -239,9 +243,15 @@ def publish_review( note: str | None = None, rootdir: str | os.PathLike | None = None, to: str | None = None, + password: str | None = None, + expires=None, ) -> ReviewPage: """Publish ``media`` as a new review page (or add it to page ``to``). + ``password`` and ``expires`` (``"7d"``, ``"2026-11-01"``, ``"never"``) gate the + page (:mod:`heed.review_access`, needs ``heed[gate]``); on an existing page they + change only what is given. Neither: the page is open to whoever has the link. + The files are copied into the data root. Adding to an existing page keeps its link and its thread, so a new cut can go where the old one's comments are; a file with the name of one already on the page replaces it. @@ -266,6 +276,9 @@ def publish_review( page.title = title if note is not None: page.note = note + if password is not None or expires is not None: + # first: a bad password or expiry must not leave an open page behind + set_access(stores.root, page.id, password=password, expires=expires) folder = stores.root / "media" / page.id folder.mkdir(parents=True, exist_ok=True) for src in sources: @@ -326,6 +339,7 @@ def make_review_app( rate_limit_per_minute: int = 10, max_comments_per_thread: int = 500, serve_shell: bool = True, + gate: ReviewGate | None = None, ) -> FastAPI: """The review API (and, with ``serve_shell``, the page at ``/``). @@ -335,8 +349,15 @@ def make_review_app( ``GET /ui/review.{js,css}``. There is deliberately no HTTP listing of pages: it would hand every capability to any script running as the owner (the CLI's ``ls`` lists them). + + A page given a password, an expiry or a revoke (:func:`publish_review`, + ``heed review revoke``) is gated by ``gate`` (a :class:`ReviewGate`, built over + ``rootdir`` with ``trusted_proxies`` by default): 401 until ``POST + /pages/{id}/unlock`` ``{"password": ...}`` sets a cookie, 410 once expired, 404 + once revoked. Pages with no such record are open, as ever. """ stores = ReviewStores(rootdir) + gate = gate or ReviewGate(stores.root, trusted_proxies=trusted_proxies) app = FastAPI(title="heed review", docs_url=None, redoc_url=None, openapi_url=None) @app.middleware("http") @@ -352,14 +373,27 @@ def _page_or_404(page_id: str) -> ReviewPage: raise HTTPException(404, "No such page") return page + def _open_page_or_404(page_id: str, request: Request) -> ReviewPage: + page = _page_or_404(page_id) + gate.check(request, page_id) + return page + @app.get("/pages/{page_id}") - def get_page(page_id: str, response: Response): + def get_page(page_id: str, request: Request, response: Response): response.headers.update(_JSON_HEADERS) - return _page_or_404(page_id).model_dump(mode="json") + return _open_page_or_404(page_id, request).model_dump(mode="json") + + @app.post("/pages/{page_id}/unlock") + async def unlock_page(page_id: str, request: Request): + _page_or_404(page_id) + response = await gate.unlock(request, page_id) + response.headers.update(_JSON_HEADERS) + return response @app.get("/pages/{page_id}/media/{name}") - def get_media(page_id: str, name: str, download: bool = False): + def get_media(page_id: str, name: str, request: Request, download: bool = False): # The link already lets its holder fetch the file: a download adds nothing. + _open_page_or_404(page_id, request) path = stores.media_path(page_id, name) if path is None: raise HTTPException(404, "No such media") @@ -381,6 +415,7 @@ def get_media(page_id: str, name: str, download: bool = False): rate_limiter=rate_limiter, rate_limit_per_minute=rate_limit_per_minute, max_comments_per_thread=max_comments_per_thread, + thread_gate=lambda request, page_id: gate.check(request, page_id), ) ) @@ -460,6 +495,20 @@ def main(argv: list[str] | None = None) -> int: pub.add_argument("--note-file", help="read the note from this file (e.g. a .md)") pub.add_argument("--to", help="add the media to this existing page (same link)") pub.add_argument("--base-url", help=f"the page's URL (default ${ENV_BASE_URL})") + pub.add_argument( + "--password-file", + help="gate the page with the password in this file ('-' reads stdin); " + "not an argument, which other users' `ps` would show", + ) + pub.add_argument( + "--expires", + help="the link stops working then: 7d, 2026-11-01, an ISO time, never", + ) + + rev = sub.add_parser( + "revoke", help="end a page's link for everyone (it answers 404)" + ) + rev.add_argument("page") com = sub.add_parser("comments", help="print a page's comments") com.add_argument("page") @@ -490,10 +539,33 @@ def main(argv: list[str] | None = None) -> int: return 0 if a.cmd == "publish": note = Path(a.note_file).read_text() if a.note_file else a.note - page = publish_review(a.media, title=a.title, note=note, rootdir=a.dir, to=a.to) + password = None + if a.password_file: + raw = ( + sys.stdin.read() + if a.password_file == "-" + else Path(a.password_file).read_text() + ) + password = raw.rstrip("\r\n") + page = publish_review( + a.media, + title=a.title, + note=note, + rootdir=a.dir, + to=a.to, + password=password, + expires=a.expires, + ) print(review_url(page.id, a.base_url)) return 0 stores = ReviewStores(a.dir) + if a.cmd == "revoke": + if stores.page(a.page) is None: + print(f"No review page {a.page!r} in {stores.root}") + return 1 + revoke_access(stores.root, a.page) + print("revoked") + return 0 if a.cmd == "comments": return _print_comments(stores, a.page, as_json=a.json) if a.cmd == "ls": diff --git a/heed/review_access.py b/heed/review_access.py new file mode 100644 index 0000000..4bae7e0 --- /dev/null +++ b/heed/review_access.py @@ -0,0 +1,279 @@ +"""Access control for review pages: an optional password, expiry and revoke. + +A review page is a capability (its 128-bit id), and by default stays one forever. This +module adds what a link sent to a client needs, on top of `caplinks +`_ (the optional ``heed[gate]`` extra): a password, +an expiry, and a revoke, kept as one record per page under ``/access/.json``. + +A page **with no record is open**, exactly as before; a page with one is gated. So +nothing changes for existing pages, and ``caplinks`` is only imported when a record +exists or one is being written (a gated page whose ``caplinks`` is missing answers 503, +never open). + +Python:: + + >>> import tempfile + >>> root = tempfile.mkdtemp() + >>> page_id = "A" * 22 + >>> rec = set_access(root, page_id, password="open sesame", expires="7d") + >>> rec["protected"] + True + >>> revoke_access(root, page_id)["revoked"] is not None + True + +HTTP: :class:`ReviewGate` is what :func:`heed.review.make_review_app` consults. A gated +page answers 401 until ``POST /pages/{id}/unlock`` with the password sets a cookie +(HMAC keyed by the stored hash, so a new password or a revoke ends every open session); +an expired page answers 410, a revoked one 404. Wrong passwords are limited per client +per page, and a page that takes too many wrong ones from everyone is locked for a while. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import tempfile +import time +from collections.abc import Iterator, MutableMapping +from pathlib import Path + +from fastapi import HTTPException, Request, Response +from starlette.concurrency import run_in_threadpool + +from heed.security import RateLimiter, client_ip, make_rate_limiter, rate_limit_key +from heed.threads import read_capped, valid_id + +ACCESS_DIR = "access" +COOKIE_PREFIX = "heed_unlock_" +#: hex chars of the page id's hash in the cookie name (the id itself is the capability +#: and must not travel to other apps on the origin) +COOKIE_ID_HEX = 16 +#: password tries one client gets on one page per window +DFLT_MAX_ATTEMPTS = 10 +#: wrong passwords one page takes from everyone per window before it locks +DFLT_MAX_PAGE_FAILURES = 30 +DFLT_ATTEMPT_WINDOW_S = 300.0 +#: how long an unlock cookie lasts (never past the page's expiry) +DFLT_UNLOCK_MAX_AGE_S = 7 * 24 * 3600 +MAX_UNLOCK_BYTES = 2_048 +_MISSING_EXTRA = ( + "gated review pages need caplinks: pip install 'heed[gate]' (or caplinks itself)" +) + + +class _JsonRecords(MutableMapping): + """``{id: dict}`` kept as ``/.json`` files (what caplinks stores).""" + + def __init__(self, folder: Path): + self.folder = folder + + def _path(self, key: str) -> Path: + if not valid_id(key): + raise KeyError(key) + return self.folder / f"{key}.json" + + def __getitem__(self, key): + try: + return json.loads(self._path(key).read_text()) + except FileNotFoundError: + raise KeyError(key) from None + + def __setitem__(self, key, value): + path = self._path(key) + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=path.parent, suffix=".tmp") # 0600: a hash + with os.fdopen(fd, "w") as f: + json.dump(value, f) + os.replace(tmp, path) + + def __delitem__(self, key): + try: + self._path(key).unlink() + except FileNotFoundError: + raise KeyError(key) from None + + def __iter__(self) -> Iterator[str]: + if self.folder.is_dir(): + yield from (p.stem for p in self.folder.glob("*.json")) + + def __len__(self): + return sum(1 for _ in self) + + def __contains__(self, key): + try: + return self._path(key).is_file() + except KeyError: + return False + + +def _links(rootdir): + try: + from caplinks import CapabilityLinks + except ImportError as e: + raise ImportError(_MISSING_EXTRA) from e + return CapabilityLinks(_JsonRecords(Path(rootdir).expanduser() / ACCESS_DIR)) + + +def set_access(rootdir, page_id: str, *, password: str | None = None, expires=None): + """Gate page ``page_id`` (creating its access record, or updating it). + + ``password`` (at least 8 characters) and ``expires`` (``"7d"``, ``"2026-11-01"``, + an ISO timestamp, ``"never"``) are changed only when given, so + ``set_access(root, id, expires="30d")`` keeps the password. Returns the record + without the hash. A revoked page stays revoked (``KeyError``). + """ + links = _links(rootdir) + if links.record(page_id) is None: + return links.create( + {}, + password=password, + expires="never" if expires is None else expires, + token=page_id, + ) + out = links.record(page_id) + if password is not None: + out = links.set_password(page_id, password) + if expires is not None: + out = links.set_expires(page_id, expires) + return out + + +def revoke_access(rootdir, page_id: str): + """End page ``page_id`` for every visitor: it answers 404 from now on. + + A page with no access record is first gated (so it can be revoked), then revoked. + """ + links = _links(rootdir) + if links.record(page_id) is None: + if page_id in links.store: # a damaged record: replace it, closed + del links.store[page_id] + links.create({}, expires="never", token=page_id) + return links.revoke(page_id) + + +def _cookie_name(page_id: str) -> str: + """The unlock cookie's name: a hash of the id, so the id never leaves the page. + + >>> _cookie_name("A" * 22) == _cookie_name("A" * 22) != _cookie_name("B" * 22) + True + """ + return COOKIE_PREFIX + hashlib.sha256(page_id.encode()).hexdigest()[:COOKIE_ID_HEX] + + +class ReviewGate: + """The HTTP side of the gate: refuse, unlock, and limit password tries. + + ``rate_limiter`` counts every try of one client on one page; + ``page_failure_limiter`` counts the wrong passwords a page takes from everyone and, + past its limit, locks the page's unlock for ``lock_s`` seconds. Both are anything + with ``allow(key) -> bool`` (heed's Redis limiter fits). Behind a reverse proxy set + ``trusted_proxies``. + """ + + def __init__( + self, + rootdir, + *, + trusted_proxies=(), + rate_limiter: RateLimiter | None = None, + page_failure_limiter: RateLimiter | None = None, + unlock_max_age_s: int = DFLT_UNLOCK_MAX_AGE_S, + lock_s: float = DFLT_ATTEMPT_WINDOW_S, + clock=time.monotonic, + ): + self.records = _JsonRecords(Path(rootdir).expanduser() / ACCESS_DIR) + self.trusted_proxies = trusted_proxies + self.rate_limiter = rate_limiter or make_rate_limiter( + max_per_window=DFLT_MAX_ATTEMPTS, + window_seconds=DFLT_ATTEMPT_WINDOW_S, + namespace="unlock-client", + ) + self.page_failure_limiter = page_failure_limiter or make_rate_limiter( + max_per_window=DFLT_MAX_PAGE_FAILURES, + window_seconds=DFLT_ATTEMPT_WINDOW_S, + namespace="unlock-page", + ) + self.unlock_max_age_s = unlock_max_age_s + self.lock_s = lock_s + self._clock = clock + self._locked: dict[str, float] = {} # page id -> when its lock ends + + def _open_record(self, page_id: str): + """``(links, record)`` of a gated page that is neither revoked nor expired; + ``None`` for an ungated one. Raises 404 (revoked), 410 (expired), 503 (no + caplinks to judge a gated page: closed, never open).""" + if page_id not in self.records: + return None + try: + links = _links(self.records.folder.parent) + except ImportError: + raise HTTPException( + 503, "This page is protected and cannot be opened here" + ) from None + state, rec = links.status(page_id) + if state == "missing": + raise HTTPException(404, "No such page") + if state == "expired": + raise HTTPException(410, "This link has expired") + return links, rec + + def check(self, request: Request, page_id: str) -> None: + """Raise ``HTTPException`` (404 revoked, 410 expired, 401 locked) unless the + page is open to ``request``; an ungated page always is.""" + found = self._open_record(page_id) + if found is None: + return + links, rec = found + cookie = request.cookies.get(_cookie_name(page_id)) + if not links.unlocked(rec, cookie, max_age_s=self.unlock_max_age_s): + raise HTTPException(401, "Password required") + + async def unlock(self, request: Request, page_id: str) -> Response: + """Check the posted password; set the unlock cookie when it is right.""" + found = self._open_record(page_id) + if found is None: + return Response(status_code=204) + links, rec = found + if not rec.get("password"): + return Response(status_code=204) + try: + raw = await read_capped(request, MAX_UNLOCK_BYTES, what="Password") + data = json.loads(raw or b"{}") + password = data["password"] + if not isinstance(password, str): + raise TypeError + except (ValueError, KeyError, TypeError, RecursionError): + raise HTTPException(400, "Expected a JSON object with a password") from None + if self._locked.get(page_id, 0) > self._clock() or not self.rate_limiter.allow( + f"{page_id}:{self._client(request)}" + ): + raise HTTPException( + 429, "Too many attempts. Wait a few minutes and try again." + ) + # scrypt is slow on purpose: keep it off the event loop + if not await run_in_threadpool(links.check_password, rec, password): + if not self.page_failure_limiter.allow(page_id): + self._locked[page_id] = self._clock() + self.lock_s + raise HTTPException(401, "That password is not right.") + proto = request.headers.get("x-forwarded-proto", request.url.scheme) + response = Response(status_code=204, headers={"Cache-Control": "no-store"}) + response.set_cookie( + _cookie_name(page_id), + links.unlock_value(rec), + max_age=links.unlock_max_age(rec, max_age_s=self.unlock_max_age_s), + path=request.scope.get("root_path") or "/", + httponly=True, + samesite="strict", + secure=proto.split(",")[0].strip() == "https", + ) + return response + + def _client(self, request: Request) -> str: + peer = request.client.host if request.client else None + ip = client_ip( + peer, + request.headers.get("x-forwarded-for"), + trusted_proxies=self.trusted_proxies, + ) + return rate_limit_key(ip) diff --git a/heed/static/review/review.js b/heed/static/review/review.js index b453f57..d5a35e9 100644 --- a/heed/static/review/review.js +++ b/heed/static/review/review.js @@ -13,6 +13,8 @@ try { return decodeURIComponent(location.hash.replace(/^#/, "")); } catch (e) { return ""; } })(); const REFRESH_MS = 30000; + let refreshTimer = null; + let listening = false; // Per-viewer conveniences only (the commenter's name); the thread lives server-side. const prefs = { @@ -62,7 +64,11 @@ }); let data = null; try { data = await res.json(); } catch (e) { /* empty body */ } - if (!res.ok) throw new Error((data && data.detail) || res.statusText || "Request failed"); + if (!res.ok) { + const err = new Error((data && data.detail) || res.statusText || "Request failed"); + err.status = res.status; + throw err; + } return data; } @@ -196,10 +202,37 @@ el("div", { class: "actions" }, submit, status), ); + // A page with a password answers 401 until the visitor unlocks it (a cookie is set). + function askPassword(message) { + const input = el("input", { type: "password", name: "password", required: "", autocomplete: "current-password", autofocus: "" }); + const status = el("p", { class: "status error", role: "status", text: message || "" }); + root.replaceChildren( + el("h1", { text: "Review" }), + el("p", { class: "muted", text: "This page is protected. Enter the password you were given." }), + el("form", { + onsubmit: async (ev) => { + ev.preventDefault(); + try { + await call("POST", "/pages/" + encodeURIComponent(pageId) + "/unlock", { password: input.value }); + start(); + } catch (e) { + status.textContent = e.message; + input.select(); + } + }, + }, el("label", {}, "Password", input), el("div", { class: "actions" }, el("button", { class: "primary", type: "submit", text: "Open" })), status), + ); + } + async function start() { let page; try { page = await call("GET", "/pages/" + encodeURIComponent(pageId)); } - catch (e) { fail("This review page does not exist, or the link is wrong."); return; } + catch (e) { + if (e.status === 401) askPassword(); + else if (e.status === 410) fail("This link has expired. Ask whoever sent it for a new one."); + else fail("This review page does not exist, or the link is wrong."); + return; + } document.title = page.title; root.replaceChildren( el("h1", { text: page.title }), @@ -211,8 +244,12 @@ form, ); try { await loadThread(); } catch (e) { list.replaceChildren(el("li", { class: "muted", text: e.message })); } - setInterval(() => { if (!document.hidden && !bodyInput.matches(":focus")) loadThread().catch(() => {}); }, REFRESH_MS); - document.addEventListener("visibilitychange", () => { if (!document.hidden) loadThread().catch(() => {}); }); + if (refreshTimer) clearInterval(refreshTimer); + refreshTimer = setInterval(() => { if (!document.hidden && !bodyInput.matches(":focus")) loadThread().catch(() => {}); }, REFRESH_MS); + if (!listening) { + listening = true; + document.addEventListener("visibilitychange", () => { if (!document.hidden) loadThread().catch(() => {}); }); + } } window.addEventListener("hashchange", () => location.reload()); diff --git a/heed/threads.py b/heed/threads.py index 45079a9..bf07000 100644 --- a/heed/threads.py +++ b/heed/threads.py @@ -305,6 +305,7 @@ def make_thread_router( trusted_proxies=(), honeypot_field: str = "website", on_comment: Callable[[Comment], None] | None = None, + thread_gate: Callable[[Request, str], None] | None = None, ) -> APIRouter: """The HTTP surface of comment threads (``threads``: thread id -> its store). @@ -313,7 +314,9 @@ def make_thread_router( ``DELETE /threads/{thread}/{id}`` deletes when ``is_moderator(request)`` and the request carries the ``X-Heed-Moderate`` header. A thread for which ``thread_exists`` is false answers 404 to everything, which is how a host makes - threads capabilities. A filled ``honeypot_field`` is accepted silently and + threads capabilities; ``thread_gate(request, thread)`` is called on every request + to an existing thread and raises ``HTTPException`` to refuse it (a password, say). + A filled ``honeypot_field`` is accepted silently and stored nowhere. Behind a reverse proxy, set ``trusted_proxies`` or every visitor shares the proxy's rate-limit bucket. @@ -338,9 +341,11 @@ def make_thread_router( warned = [] router = APIRouter() - def _require_thread(thread: str) -> MutableMapping: + def _require_thread(thread: str, request: Request) -> MutableMapping: if not (valid_id(thread) and thread_exists(thread)): raise HTTPException(404, "No such thread") + if thread_gate is not None: + thread_gate(request, thread) return threads(thread) def _client_key(request: Request) -> str: @@ -361,7 +366,7 @@ def _client_key(request: Request) -> str: @router.get("/threads/{thread}") def get_thread(thread: str, request: Request, response: Response): - store = _require_thread(thread) + store = _require_thread(thread, request) response.headers.update(THREAD_RESPONSE_HEADERS) return { "thread": thread, @@ -371,11 +376,11 @@ def get_thread(thread: str, request: Request, response: Response): @router.post("/threads/{thread}", status_code=201) async def post_comment(thread: str, request: Request, response: Response): - store = _require_thread(thread) + store = _require_thread(thread, request) response.headers.update(THREAD_RESPONSE_HEADERS) if not limiter.allow(_client_key(request)): raise HTTPException(429, "Too many comments; try again in a minute") - raw = await _read_capped(request, MAX_COMMENT_BYTES) + raw = await read_capped(request, MAX_COMMENT_BYTES) try: data = json.loads(raw or b"{}") if not isinstance(data, dict): @@ -405,7 +410,7 @@ async def post_comment(thread: str, request: Request, response: Response): @router.delete("/threads/{thread}/{comment_id}") def remove_comment(thread: str, comment_id: str, request: Request): - store = _require_thread(thread) + store = _require_thread(thread, request) if request.headers.get(MODERATE_HEADER) != "1" or not is_moderator(request): raise HTTPException(403, "Only a moderator can delete comments") if not delete_comment(store, comment_id): @@ -415,15 +420,15 @@ def remove_comment(thread: str, comment_id: str, request: Request): return router -async def _read_capped(request: Request, cap: int) -> bytes: +async def read_capped(request: Request, cap: int, *, what: str = "Comment") -> bytes: """The request body, refusing (413) anything over ``cap`` bytes while streaming.""" declared = request.headers.get("content-length") if declared and declared.isdigit() and int(declared) > cap: - raise HTTPException(413, "Comment too large") + raise HTTPException(413, f"{what} too large") chunks, size = [], 0 async for chunk in request.stream(): size += len(chunk) if size > cap: - raise HTTPException(413, "Comment too large") + raise HTTPException(413, f"{what} too large") chunks.append(chunk) return b"".join(chunks) diff --git a/pyproject.toml b/pyproject.toml index 16d725f..05e77fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,6 +46,8 @@ heed = "heed.__main__:main" github = ["ghapi>=1.0"] # Shared rate limiting across workers/hosts (rate_limit_backend="redis"). redis = ["redis>=4.2"] +# Passwords, expiry and revoke on review pages (heed.review_access). +gate = ["caplinks>=0.0.3"] # Kept as an alias for existing installs: enlace is now a core dependency (for # enlace.ratelimit), so this extra adds nothing. enlace = ["enlace>=0.1.62"] @@ -59,7 +61,7 @@ mcp = ["py2mcp>=0.1.0"] # only falls back to `httpx` with a deprecation warning, while starlette <1 # knows `httpx` alone. Both are declared so the suite collects on either # resolution; they are distinct top-level modules and coexist fine. -dev = ["pytest", "httpx", "httpx2", "pytest-asyncio", "Pillow>=10.0"] +dev = ["pytest", "httpx", "httpx2", "pytest-asyncio", "Pillow>=10.0", "caplinks>=0.0.3"] [tool.pytest.ini_options] # wads CI runs `pytest --doctest-modules` with NO path argument, so collection is diff --git a/tests/test_review.py b/tests/test_review.py index 234fb86..dab56c7 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -301,3 +301,176 @@ def test_a_weak_moderator_token_is_refused(): for weak in ("", "short", None, " padded-token-value-1 ", "tökén-with-non-ascii-1"): with _pytest.raises(ValueError, match="token"): token_moderator(weak) + + +# ------------------------------------------------------------ gated pages (heed#88) + +PW = "open sesame" + + +def _gated(root, clip, **kw): + page = publish_review([clip], title="Cut", rootdir=root, **kw) + return page, _client(root) + + +def test_an_ungated_page_is_open_and_makes_no_access_record(root, clip): + page, client = _gated(root, clip) + assert client.get(f"/pages/{page.id}").status_code == 200 + assert not (root / "access").exists() + + +def test_a_password_gates_page_media_and_thread_until_unlocked(root, clip): + page, client = _gated(root, clip, password=PW) + paths = [ + f"/pages/{page.id}", + f"/pages/{page.id}/media/{page.media[0].name}", + f"/threads/{page.id}", + ] + assert [client.get(p).status_code for p in paths] == [401, 401, 401] + assert ( + client.post( + f"/threads/{page.id}", json={"author": "a", "body": "b"} + ).status_code + == 401 + ) + assert ( + client.post(f"/pages/{page.id}/unlock", json={"password": "wrong"}).status_code + == 401 + ) + assert client.get(paths[0]).status_code == 401 + ok = client.post(f"/pages/{page.id}/unlock", json={"password": PW}) + assert ok.status_code == 204 + assert "httponly" in ok.headers["set-cookie"].lower() + assert [client.get(p).status_code for p in paths] == [200, 200, 200] + + +def test_an_unlock_does_not_open_another_page(root, clip): + a, client = _gated(root, clip, password=PW) + b = publish_review([clip], title="B", rootdir=root, password="another one") + client.post(f"/pages/{a.id}/unlock", json={"password": PW}) + assert client.get(f"/pages/{a.id}").status_code == 200 + assert client.get(f"/pages/{b.id}").status_code == 401 + + +def test_changing_the_password_ends_open_sessions(root, clip): + page, client = _gated(root, clip, password=PW) + client.post(f"/pages/{page.id}/unlock", json={"password": PW}) + publish_review([], rootdir=root, to=page.id, password="a new password") + assert client.get(f"/pages/{page.id}").status_code == 401 + ok = client.post(f"/pages/{page.id}/unlock", json={"password": "a new password"}) + assert ok.status_code == 204 + + +def test_expiry_answers_410_and_revoke_404(root, clip): + from datetime import datetime, timedelta, timezone + + page, client = _gated(root, clip, expires="1d") + assert ( + client.get(f"/pages/{page.id}").status_code == 200 + ) # expiry alone: no password + rec_path = root / "access" / f"{page.id}.json" + import json + + rec = json.loads(rec_path.read_text()) + past = datetime.now(timezone.utc) - timedelta(hours=1) + rec["expires"] = past.isoformat() + rec_path.write_text(json.dumps(rec)) + assert client.get(f"/pages/{page.id}").status_code == 410 + assert client.get(f"/threads/{page.id}").status_code == 410 + publish_review([], rootdir=root, to=page.id, expires="3d") # extend: open again + assert client.get(f"/pages/{page.id}").status_code == 200 + assert main(["--dir", str(root), "revoke", page.id]) == 0 + assert client.get(f"/pages/{page.id}").status_code == 404 + assert client.get(f"/pages/{page.id}/media/{page.media[0].name}").status_code == 404 + + +def test_revoking_an_ungated_page_closes_it(root, clip): + page, client = _gated(root, clip) + assert main(["--dir", str(root), "revoke", page.id]) == 0 + assert client.get(f"/pages/{page.id}").status_code == 404 + + +def test_wrong_passwords_are_limited_per_client(root, clip): + from heed.review_access import ReviewGate + from heed.security import InMemoryRateLimiter + + page = publish_review([clip], title="Cut", rootdir=root, password=PW) + gate = ReviewGate(root, rate_limiter=InMemoryRateLimiter(max_per_window=3)) + client = _client(root, gate=gate) + url = f"/pages/{page.id}/unlock" + codes = [client.post(url, json={"password": "nope"}).status_code for _ in range(5)] + assert codes == [401, 401, 401, 429, 429] + assert client.post(url, json={"password": PW}).status_code == 429 # even the right one + + +def test_a_page_taking_too_many_wrong_passwords_locks_for_everyone(root, clip): + from heed.review_access import ReviewGate + from heed.security import InMemoryRateLimiter + + now = [0.0] + page = publish_review([clip], title="Cut", rootdir=root, password=PW) + gate = ReviewGate( + root, + rate_limiter=InMemoryRateLimiter(max_per_window=100), + page_failure_limiter=InMemoryRateLimiter(max_per_window=2), + lock_s=60, + clock=lambda: now[0], + ) + client = _client(root, gate=gate) + url = f"/pages/{page.id}/unlock" + wrong = {"password": "nope"} + assert [client.post(url, json=wrong).status_code for _ in range(3)] == [401] * 3 + assert client.post(url, json={"password": PW}).status_code == 429 # locked + now[0] = 61.0 + assert client.post(url, json={"password": PW}).status_code == 204 # lock over + + +def test_a_bad_password_at_publish_leaves_no_open_page(root, clip): + with pytest.raises(ValueError): + publish_review([clip], title="Cut", rootdir=root, password="short") + assert ReviewStores(root).all_pages() == [] + page = publish_review([clip], title="Cut", rootdir=root) + with pytest.raises(ValueError): + publish_review([clip], rootdir=root, to=page.id, password="short") + + +def test_the_unlock_cookie_does_not_carry_the_page_id(root, clip): + page, client = _gated(root, clip, password=PW) + ok = client.post(f"/pages/{page.id}/unlock", json={"password": PW}) + assert page.id not in ok.headers["set-cookie"] + + +def test_revoking_a_page_with_a_damaged_record_still_closes_it(root, clip): + page, client = _gated(root, clip, password=PW) + (root / "access" / f"{page.id}.json").write_text("{not json") + assert client.get(f"/pages/{page.id}").status_code == 404 # closed, not open + assert main(["--dir", str(root), "revoke", page.id]) == 0 + assert client.get(f"/pages/{page.id}").status_code == 404 + + +def test_a_gated_page_without_caplinks_fails_closed(root, clip, monkeypatch): + page, client = _gated(root, clip, password=PW) + monkeypatch.setitem(__import__("sys").modules, "caplinks", None) + assert client.get(f"/pages/{page.id}").status_code == 503 + + +def test_unlock_rejects_a_malformed_or_oversized_body(root, clip): + page, client = _gated(root, clip, password=PW) + url = f"/pages/{page.id}/unlock" + assert client.post(url, content=b"nope").status_code == 400 + assert client.post(url, json={"password": 5}).status_code == 400 + assert client.post(url, json={"password": "x" * 5000}).status_code == 413 + + +def test_publish_cli_reads_the_password_from_a_file(root, clip, tmp_path, capsys): + pw = tmp_path / "pw" + pw.write_text(PW + "\n") + argv = ["--dir", str(root), "publish", str(clip), "--title", "t"] + assert main([*argv, "--password-file", str(pw), "--expires", "7d"]) == 0 + page_id = capsys.readouterr().out.strip().lstrip("#") + client = _client(root) + assert client.get(f"/pages/{page_id}").status_code == 401 + assert ( + client.post(f"/pages/{page_id}/unlock", json={"password": PW}).status_code + == 204 + )