diff --git a/README.md b/README.md index 923ca9e..1fddaf6 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ Non-secret knobs (`photos_dir`, `data_dir`, the vision model, the rate limit) li uv run bggpipe web # opens http://127.0.0.1:8377/ — the whole app in the browser ``` -The app is localhost-only by default. `--lan` also serves it to your local network — handy for proofreading from the couch or snapping shelf photos on your phone and uploading them straight into the Photos page — but mind the trade: there is no login, so anyone on the network can operate the pipeline. Use it on networks you trust (or use a device VPN like Tailscale against the localhost default instead). +The app is localhost-only by default. `--lan` also serves it to your local network — handy for proofreading from the couch or snapping shelf photos on your phone and uploading them straight into the Photos page. It prints a link carrying a per-run access key (`?k=...`): open that exact link on the phone once and a cookie remembers it. The key is the only lock — there is no login behind it — so still prefer networks you trust (or use a device VPN like Tailscale against the localhost default instead). Six pages in one local app: **Pipeline** (run stages, watch live output), **Photos** (drag-and-drop upload, gallery, reshoot tickets), **Review** (keyboard-first match and edition decisions), **Titles** (every read off your shelves, alphabetized — and where you proofread them: fix misreads, add cues, split multi-copy lines, remove non-games), **Queue** (exactly what upload will do, plus its full log), and **Library** (your enriched collection, browsable once real BGG data lands). The real upload sits behind a confirmation and behind the stub-data lock. Prefer the terminal? Every stage is also a command, and the two interfaces share all state: diff --git a/src/bggpipe/cli.py b/src/bggpipe/cli.py index 8a91b69..7c51ba3 100644 --- a/src/bggpipe/cli.py +++ b/src/bggpipe/cli.py @@ -100,8 +100,8 @@ def web( bool, typer.Option( "--lan", - help="Also serve to your local network (phone, tablet). The app " - "has no login — only use on a network you trust.", + help="Also serve to your local network behind a per-run access " + "key (trusted networks only)", ), ] = False, config: ConfigOpt = None, diff --git a/src/bggpipe/static/app.css b/src/bggpipe/static/app.css index f250ded..4196d6e 100644 --- a/src/bggpipe/static/app.css +++ b/src/bggpipe/static/app.css @@ -479,7 +479,7 @@ button.danger { color: var(--stop-ink); border-color: var(--stop); background: # .wordmark small { display: inline; margin-left: .5rem; font-size: .68rem; } nav[aria-label="Primary"] { flex-direction: row; padding: 0 .4rem .1rem; - overflow-x: auto; -webkit-overflow-scrolling: touch; + overflow-x: auto; } nav[aria-label="Primary"] a { flex: 0 0 auto; @@ -499,6 +499,9 @@ button.danger { color: var(--stop-ink); border-color: var(--stop); background: # h1 { font-size: 1.3rem; margin: .9rem 0 .7rem; } .keyhelp { display: none; } /* no keyboard on a phone */ button, .linkbtn { padding: .45rem .9rem; } /* finger-sized */ + /* component buttons set their own padding at higher specificity — + * they need the touch bump spelled out */ + .rowactions button, .ticket button, .card.merge button { padding: .4rem .8rem; } .card, .ticket { flex-direction: column; } .shots { flex-basis: auto; } diff --git a/src/bggpipe/static/app.js b/src/bggpipe/static/app.js index cb06099..d6fe286 100644 --- a/src/bggpipe/static/app.js +++ b/src/bggpipe/static/app.js @@ -82,6 +82,16 @@ async function refreshBadges() { refreshBadges().catch(() => {}); setInterval(() => refreshBadges().catch(() => {}), 5000); +/* One-line match summary for a titles/photo table row. Empty when the + * row has no BGG data yet, so the mobile stacker can hide the cell. */ +function metaLine(c) { + return [ + c.bgg_name ? esc(c.bgg_name) + (c.bgg_id ? " · " + esc(c.bgg_id) : "") : "", + c.version_name ? esc(c.version_name) : "", + c.type === "rpgitem" ? `RPG · local only` : "", + ].filter(Boolean).join(" · "); +} + /* Status chip for a catalog entry — shared by the catalog and photo pages. */ function statusChip(c) { if (c.status === "awaiting_resolve") return `awaiting BGG`; diff --git a/src/bggpipe/templates/pages/help.html b/src/bggpipe/templates/pages/help.html index d6dcdb1..4ff183f 100644 --- a/src/bggpipe/templates/pages/help.html +++ b/src/bggpipe/templates/pages/help.html @@ -61,6 +61,6 @@

Your data, on disk

Everything lives in flat files under data/ — inspectable, hand-editable, and git-friendly. The pipeline artifacts: titles.json (what was read), matches.csv (what it matched), to_add.csv/to_update.csv (what upload will do), upload_log.csv (what it did), games.json (the library). Your curation: title_edits.json, title_splits.json, title_removals.json, unidentified_dismissed.json.

-

Credentials never live in files — only environment variables, set up by bggpipe init. The app runs on localhost only.

+

Credentials never live in files — only environment variables, set up by bggpipe init. The app serves localhost only, unless started with --lan — that opens it to your network behind a per-run access key printed at startup (no login beyond the key; trusted networks only).

More depth: the README covers setup and photo technique; docs/bgg-upload-flow.md documents the upload automation.

diff --git a/src/bggpipe/templates/pages/photo.html b/src/bggpipe/templates/pages/photo.html index dc41e76..1ebd3be 100644 --- a/src/bggpipe/templates/pages/photo.html +++ b/src/bggpipe/templates/pages/photo.html @@ -64,10 +64,7 @@ function render(state, photos) { ${esc(c.title_raw)} ${statusChip(c)} - ${[ - c.bgg_name ? esc(c.bgg_name) + (c.bgg_id ? " · " + esc(c.bgg_id) : "") : "", - c.version_name ? esc(c.version_name) : "", - ].filter(Boolean).join(" · ")} + ${metaLine(c)} `).join("") + `` : `

${info.extracted ? "No titles were read from this photo." @@ -97,8 +94,8 @@ async function refresh() { ]); const payload = JSON.stringify([state.catalog, state.unidentified, photos]); if (payload === LAST) return; - LAST = payload; render(state, photos); + LAST = payload; // after render: a throw must not freeze the page as "current" } document.addEventListener("keydown", e => { diff --git a/src/bggpipe/templates/pages/queue.html b/src/bggpipe/templates/pages/queue.html index 79d1e32..b5c109a 100644 --- a/src/bggpipe/templates/pages/queue.html +++ b/src/bggpipe/templates/pages/queue.html @@ -19,7 +19,7 @@ function render(q) { ? table(["game", "version", "seen in"], q.to_add.map(r => ` ${esc(r.bgg_name)} · ${esc(r.bgg_id)} ${r.version_name ? esc(r.version_name) : `no version`} - ${esc(r.source_photos.split(";").join(", "))}`)) + ${esc((r.source_photos ?? "").split(";").join(", "))}`)) : `

Nothing queued — run diff from the pipeline first.

`; html += `

Version updates — ${q.to_update.length} existing entr${q.to_update.length === 1 ? "y" : "ies"} gaining a version

`; @@ -48,8 +48,8 @@ async function refresh() { const q = await fetchJSON("/api/queue"); const payload = JSON.stringify(q); if (payload === LAST) return; - LAST = payload; render(q); + LAST = payload; // after render: a throw must not freeze the page as "current" } refresh().catch(err => errorBanner(err.message || err)); diff --git a/src/bggpipe/templates/pages/titles.html b/src/bggpipe/templates/pages/titles.html index f4c910e..7234af1 100644 --- a/src/bggpipe/templates/pages/titles.html +++ b/src/bggpipe/templates/pages/titles.html @@ -59,11 +59,7 @@ function render() { ${c.shaky ? `shaky read` : ""} ${statusChip(c)} - ${[ - c.bgg_name ? esc(c.bgg_name) + (c.bgg_id ? " · " + esc(c.bgg_id) : "") : "", - c.version_name ? esc(c.version_name) : "", - c.type === "rpgitem" ? `RPG · local only` : "", - ].filter(Boolean).join(" · ")} + ${metaLine(c)} ${c.photos.map(p => `${esc(p)}` ).join(", ")} @@ -92,9 +88,9 @@ async function refresh() { if (EDITING) return; // never repaint under an open editor const payload = JSON.stringify(state.catalog); if (payload === LAST) return; - LAST = payload; CATALOG = state.catalog; render(); + LAST = payload; // after render: a throw must not freeze the page as "current" } document.getElementById("catbody").addEventListener("click", async e => { diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index 1dbfe97..99669af 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -17,6 +17,7 @@ from __future__ import annotations import io import json import os +import secrets import socket import threading import time @@ -279,6 +280,7 @@ def create_app( stages: dict[str, Callable[..., object]] | None = None, jobs: JobRunner | None = None, allowed_hosts: set[str] | None = None, + lan_token: str | None = None, ) -> FastAPI: app = FastAPI(title="bggpipe") stages = stages or _default_stages(cfg) @@ -305,22 +307,64 @@ def create_app( app_warnings: list[str] = list(startup_notes) ALLOWED_HOSTS = {"127.0.0.1", "localhost", "testserver"} | (allowed_hosts or set()) + LAN_COOKIE = "bggpipe_key" @app.middleware("http") async def origin_guard(request, call_next): # A hostile webpage can fire preflight-free cross-origin POSTs at a - # localhost server (bodyless run triggers, multipart photo posts). - # Mutations must come from us: same-host, and no foreign Origin. - if request.method not in ("GET", "HEAD", "OPTIONS"): - host = (request.headers.get("host") or "").split(":")[0] - origin = request.headers.get("origin") - origin_host = urlsplit(origin).hostname if origin else None - if host not in ALLOWED_HOSTS or ( - origin_host is not None and origin_host not in ALLOWED_HOSTS + # localhost server (bodyless run triggers, multipart photo posts) — + # and via DNS rebinding it can read GETs too. url.hostname parses + # the Host header properly (ports, IPv6 brackets, lowercase). + host = (request.url.hostname or "").lower() + origin = request.headers.get("origin") + origin_host = urlsplit(origin).hostname if origin else None + if lan_token is not None: + # --lan has no login, so EVERY request — reads included: shelf + # photos and pipeline state are private — needs the per-run + # key from the printed URL; a cookie carries it afterwards. + supplied = ( + request.query_params.get("k") or request.cookies.get(LAN_COOKIE) or "" + ) + if not secrets.compare_digest(supplied, lan_token): + typer.echo( + f"refused {request.method} {request.url.path} " + f"(host {host!r}): missing or wrong access key", + err=True, + ) + return JSONResponse( + { + "detail": "missing or wrong access key — open the " + "exact URL printed where the server started " + "(it ends in ?k=...)" + }, + status_code=403, + ) + if request.method not in ("GET", "HEAD", "OPTIONS") and ( + origin_host is not None and origin_host != host ): return JSONResponse( {"detail": "cross-origin request refused"}, status_code=403 ) + response = await call_next(request) + if request.query_params.get("k"): + # the key came in the typed URL: hand it to the browser so + # navigation and fetches keep working without it + response.set_cookie( + LAN_COOKIE, lan_token, httponly=True, samesite="lax" + ) + return response + if request.method not in ("GET", "HEAD", "OPTIONS") and ( + host not in ALLOWED_HOSTS + or (origin_host is not None and origin_host not in ALLOWED_HOSTS) + ): + typer.echo( + f"refused {request.method} {request.url.path}: host {host!r}" + f" / origin {origin_host!r} not on this server's allowlist", + err=True, + ) + return JSONResponse( + {"detail": "cross-origin request refused"}, status_code=403 + ) return await call_next(request) def freshen() -> None: @@ -1019,8 +1063,8 @@ def _dev_app() -> FastAPI: from bggpipe.config import load_config path = os.environ.get("BGGPIPE_CONFIG") or None - extra = lan_hosts() if os.environ.get("BGGPIPE_LAN") else None - return create_app(load_config(Path(path) if path else None), allowed_hosts=extra) + token = os.environ.get("BGGPIPE_LAN_TOKEN") or None + return create_app(load_config(Path(path) if path else None), lan_token=token) def run_web_review( @@ -1036,17 +1080,24 @@ def run_web_review( import uvicorn url = f"http://127.0.0.1:{port}{landing}" - extra_hosts = lan_hosts() if lan else None + token = secrets.token_urlsafe(6) if lan else None if lan: - addresses = ", ".join(f"http://{h}:{port}/" for h in sorted(extra_hosts or [])) + ips = sorted(h for h in lan_hosts() if h.replace(".", "").isdigit()) + names = sorted(h for h in lan_hosts() if not h.replace(".", "").isdigit()) + addresses = ", ".join(f"http://{h}:{port}/?k={token}" for h in ips + names) + typer.echo(f"bggpipe web UI: {url}?k={token}") + typer.echo(f" from your phone, open: {addresses}") typer.echo( - f"bggpipe web UI: {url} — ALSO reachable from your network: {addresses}" - ) - typer.echo( - " --lan: the app has NO login. Anyone on this network can run " + " --lan: no login beyond that key — anyone who has it can run " "stages, change your data, and (once unlocked) drive uploads " "to your BGG account. Use only on a network you trust." ) + if not ips: + typer.echo( + " couldn't determine this machine's network address — find " + "it in your network settings and open " + f"http://:{port}/?k={token}" + ) else: typer.echo( f"bggpipe web UI: {url} (localhost only; dashboard at /, review " @@ -1061,8 +1112,8 @@ def run_web_review( # write — i.e. on every review decision. if config_path: os.environ["BGGPIPE_CONFIG"] = str(config_path) - if lan: - os.environ["BGGPIPE_LAN"] = "1" + if token: + os.environ["BGGPIPE_LAN_TOKEN"] = token typer.echo(" --dev: restarting on source changes") uvicorn.run( "bggpipe.webreview:_dev_app", @@ -1075,7 +1126,7 @@ def run_web_review( ) else: uvicorn.run( - create_app(cfg, allowed_hosts=extra_hosts), + create_app(cfg, lan_token=token), host="0.0.0.0" if lan else "127.0.0.1", # noqa: S104 — opted in port=port, log_level="warning", diff --git a/tests/test_webreview.py b/tests/test_webreview.py index 8235b41..5d644e8 100644 --- a/tests/test_webreview.py +++ b/tests/test_webreview.py @@ -894,9 +894,87 @@ def test_lan_allowed_hosts_admit_network_but_not_strangers(tmp_path): ) -def test_lan_hosts_reports_this_machine(tmp_path): +def test_lan_hosts_are_lowercase_nonempty_and_v4_only(monkeypatch): + import socket as socket_mod + from bggpipe.webreview import lan_hosts + monkeypatch.setattr(socket_mod, "gethostname", lambda: "Erics-Mac.Example.COM") + monkeypatch.setattr( + socket_mod, + "getaddrinfo", + lambda *a, **k: (_ for _ in ()).throw(OSError("no dns")), + ) hosts = lan_hosts() - assert hosts # at least the hostname - assert all(h == h.lower() or "." in h for h in hosts) + assert "erics-mac.example.com" in hosts + assert "erics-mac.local" in hosts + assert "" not in hosts # an empty entry would admit Host-less mutations + assert all(":" not in h for h in hosts) # no v6 forms, no ports + + +def test_lan_token_gates_every_request(tmp_path): + cfg = make_cfg(tmp_path) + app = create_app(cfg, client=unauthorized_client(tmp_path), lan_token="sekret") + phone = TestClient(app, base_url="http://192.168.1.99:8377") + # reads are gated too: shelf photos and state are private + assert phone.get("/api/state").status_code == 403 + assert phone.get("/api/state?k=wrong").status_code == 403 + first = phone.get("/titles?k=sekret") + assert first.status_code == 200 + assert "bggpipe_key" in first.cookies # the URL key becomes a cookie + # cookie carries the session: mutations work from ANY host the phone + # used (no allowlist dependence — DHCP/multi-interface safe), with a + # same-origin Origin header and a port, like a real phone browser + res = phone.post( + "/api/edit-title", + json={"title_raw": "Citadels", "source_photos": "shelf.jpg", "confirm": True}, + headers={"origin": "http://192.168.1.99:8377"}, + ) + assert res.status_code == 200 + # a foreign Origin is still refused even with the key + assert ( + phone.post( + "/api/dismiss", + json={"photo": "shelf.jpg"}, + headers={"origin": "http://evil.example"}, + ).status_code + == 403 + ) + + +def test_localhost_mutations_pass_with_origin_and_port(tmp_path): + # the path every real browser takes: Origin present + port in Host — + # regressing the header parsing must fail loudly here + cfg = make_cfg(tmp_path) + app = create_app(cfg, client=unauthorized_client(tmp_path)) + web = TestClient(app, base_url="http://localhost:8377") + res = web.post( + "/api/edit-title", + json={"title_raw": "Citadels", "source_photos": "shelf.jpg", "confirm": True}, + headers={"origin": "http://localhost:8377"}, + ) + assert res.status_code == 200 + assert ( + TestClient(app, base_url="http://attacker.example:8377") + .post( + "/api/edit-title", + json={"title_raw": "Citadels", "source_photos": "shelf.jpg"}, + ) + .status_code + == 403 + ) + + +def test_run_web_review_lan_branch_binds_and_warns(tmp_path, monkeypatch, capsys): + import bggpipe.webreview as wr + + captured = {} + monkeypatch.setattr("uvicorn.run", lambda app, **kw: captured.update(kw, app=app)) + cfg = make_cfg(tmp_path) + wr.run_web_review(cfg, port=9999, lan=True) + out = capsys.readouterr().out + assert captured["host"] == "0.0.0.0" + assert "?k=" in out # every printed URL carries the access key + assert "Use only on a network you trust" in out + wr.run_web_review(cfg, port=9999, lan=False) + assert captured["host"] == "127.0.0.1"