diff --git a/heed/review_access.py b/heed/review_access.py index 4bae7e0..d8b189d 100644 --- a/heed/review_access.py +++ b/heed/review_access.py @@ -30,19 +30,18 @@ from __future__ import annotations +import functools 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 +from heed.threads import read_capped ACCESS_DIR = "access" COOKIE_PREFIX = "heed_unlock_" @@ -62,57 +61,16 @@ ) -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 _records_dir(rootdir) -> Path: + return Path(rootdir).expanduser() / ACCESS_DIR def _links(rootdir): try: - from caplinks import CapabilityLinks + from caplinks import CapabilityLinks, FileTexts, JsonRecords except ImportError as e: raise ImportError(_MISSING_EXTRA) from e - return CapabilityLinks(_JsonRecords(Path(rootdir).expanduser() / ACCESS_DIR)) + return CapabilityLinks(JsonRecords(FileTexts(_records_dir(rootdir)))) def set_access(rootdir, page_id: str, *, password: str | None = None, expires=None): @@ -146,7 +104,10 @@ def revoke_access(rootdir, page_id: str): """ links = _links(rootdir) if links.record(page_id) is None: - if page_id in links.store: # a damaged record: replace it, closed + path = _records_dir(rootdir) / f"{page_id}.json" + if path.is_symlink(): # caplinks refuses to follow it: replace it, closed + path.unlink() + elif page_id in links.store: # a damaged record del links.store[page_id] links.create({}, expires="never", token=page_id) return links.revoke(page_id) @@ -182,7 +143,7 @@ def __init__( lock_s: float = DFLT_ATTEMPT_WINDOW_S, clock=time.monotonic, ): - self.records = _JsonRecords(Path(rootdir).expanduser() / ACCESS_DIR) + self.rootdir = rootdir self.trusted_proxies = trusted_proxies self.rate_limiter = rate_limiter or make_rate_limiter( max_per_window=DFLT_MAX_ATTEMPTS, @@ -197,16 +158,27 @@ def __init__( 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 + + @functools.cached_property + def _guard(self): + """The lock-out (caplinks' ``AttemptGuard``), built on the first unlock.""" + from caplinks import AttemptGuard + + return AttemptGuard( + client_limiter=self.rate_limiter, + link_limiter=self.page_failure_limiter, + lock_s=self.lock_s, + clock=self._clock, + ) 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 + if not os.path.lexists(_records_dir(self.rootdir) / f"{page_id}.json"): + return None # no record: open (a bad or symlinked one gates: 404) try: - links = _links(self.records.folder.parent) + links = _links(self.rootdir) except ImportError: raise HTTPException( 503, "This page is protected and cannot be opened here" @@ -245,16 +217,13 @@ async def unlock(self, request: Request, page_id: str) -> Response: 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)}" - ): + if not self._guard.allow(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 + self._guard.failed(page_id) 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"}) diff --git a/pyproject.toml b/pyproject.toml index 05e77fb..b4b64af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,7 @@ 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"] +gate = ["caplinks>=0.0.4"] # 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"] @@ -61,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", "caplinks>=0.0.3"] +dev = ["pytest", "httpx", "httpx2", "pytest-asyncio", "Pillow>=10.0", "caplinks>=0.0.4"] [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 dab56c7..43116ef 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -379,14 +379,14 @@ def test_expiry_answers_410_and_revoke_404(root, clip): 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 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 main(["--dir", str(root), "revoke", "--", page.id]) == 0 assert client.get(f"/pages/{page.id}").status_code == 404 @@ -444,7 +444,7 @@ 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 main(["--dir", str(root), "revoke", "--", page.id]) == 0 assert client.get(f"/pages/{page.id}").status_code == 404 @@ -474,3 +474,17 @@ def test_publish_cli_reads_the_password_from_a_file(root, clip, tmp_path, capsys client.post(f"/pages/{page_id}/unlock", json={"password": PW}).status_code == 204 ) + + +def test_a_dangling_or_symlinked_record_closes_the_page_and_revoke_replaces_it( + root, clip, tmp_path +): + page, client = _gated(root, clip, password=PW) + record = root / "access" / f"{page.id}.json" + record.unlink() + record.symlink_to(tmp_path / "nowhere.json") # dangling + assert client.get(f"/pages/{page.id}").status_code == 404 + assert main(["--dir", str(root), "revoke", "--", page.id]) == 0 + assert ( + not record.is_symlink() and client.get(f"/pages/{page.id}").status_code == 404 + )