diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index 56274fd..df0aac0 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -318,7 +318,13 @@ def create_app( 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: + client_ip = request.client.host if request.client else "" + loopback = client_ip.startswith("127.") or client_ip == "::1" + # Same-machine browsing stays keyless under --lan: a network peer + # cannot arrive with a loopback CLIENT address, and these requests + # fall through to the localhost guard below, which still blocks + # rebinding and cross-origin pages by their foreign Host/Origin. + if lan_token is not None and not (loopback and host in ALLOWED_HOSTS): # --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. @@ -1085,8 +1091,13 @@ def run_web_review( # type it once; the cookie remembers it from then on. token = secrets.token_urlsafe(16) if lan else None if lan: - 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()) + hosts = lan_hosts() + ips = sorted( + h + for h in hosts + if h.replace(".", "").isdigit() and not h.startswith("127.") + ) + names = sorted(h for h in 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}") diff --git a/tests/test_webreview.py b/tests/test_webreview.py index 5d644e8..9dd4443 100644 --- a/tests/test_webreview.py +++ b/tests/test_webreview.py @@ -942,6 +942,42 @@ def test_lan_token_gates_every_request(tmp_path): ) +def test_lan_loopback_stays_keyless_but_guarded(tmp_path): + cfg = make_cfg(tmp_path) + app = create_app(cfg, client=unauthorized_client(tmp_path), lan_token="sekret") + local = TestClient( + app, base_url="http://127.0.0.1:8377", client=("127.0.0.1", 50000) + ) + # the desktop browser (old tabs, the auto-opened one) needs no key + assert local.get("/api/pipeline").status_code == 200 + assert ( + local.post( + "/api/edit-title", + json={ + "title_raw": "Citadels", + "source_photos": "shelf.jpg", + "confirm": True, + }, + headers={"origin": "http://127.0.0.1:8377"}, + ).status_code + == 200 + ) + # but loopback keylessness never extends to foreign Hosts (rebinding) + # or foreign Origins (classic CSRF from a local browser) + rebound = TestClient( + app, base_url="http://evil.example:8377", client=("127.0.0.1", 50000) + ) + assert rebound.get("/api/state").status_code == 403 + assert ( + local.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 @@ -971,10 +1007,17 @@ def test_run_web_review_lan_branch_binds_and_warns(tmp_path, monkeypatch, capsys captured = {} monkeypatch.setattr("uvicorn.run", lambda app, **kw: captured.update(kw, app=app)) cfg = make_cfg(tmp_path) + monkeypatch.setattr( + wr, "lan_hosts", lambda: {"127.0.0.1", "192.168.1.5", "erics-mac.local"} + ) 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 "http://192.168.1.5:9999/?k=" in out + # no phone can reach loopback: it never appears in the phone list + assert "phone" not in out.split("http://127.0.0.1:9999")[-1].split("\n")[0] + assert out.count("http://127.0.0.1:9999") == 1 # the desktop line only 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"