--lan: the desktop stays keyless; phone URLs drop loopback
Dogfooding caught both: the auto-opened desktop tab (and every old polling tab) 403'd for lack of the key, and the "from your phone" list offered 127.0.0.1. Loopback CLIENT connections now skip the key — a network peer cannot arrive with a loopback client address — and fall through to the same Host/Origin guard as the localhost default, so rebinding pages (foreign Host) and cross-origin POSTs (foreign Origin) from a local browser stay blocked. Loopback addresses are filtered out of the printed phone URLs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g
This commit is contained in:
co-authored by
Claude Fable 5
parent
429ceb1a21
commit
eb2e841d27
@@ -318,7 +318,13 @@ def create_app(
|
|||||||
host = (request.url.hostname or "").lower()
|
host = (request.url.hostname or "").lower()
|
||||||
origin = request.headers.get("origin")
|
origin = request.headers.get("origin")
|
||||||
origin_host = urlsplit(origin).hostname if origin else None
|
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
|
# --lan has no login, so EVERY request — reads included: shelf
|
||||||
# photos and pipeline state are private — needs the per-run
|
# photos and pipeline state are private — needs the per-run
|
||||||
# key from the printed URL; a cookie carries it afterwards.
|
# 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.
|
# type it once; the cookie remembers it from then on.
|
||||||
token = secrets.token_urlsafe(16) if lan else None
|
token = secrets.token_urlsafe(16) if lan else None
|
||||||
if lan:
|
if lan:
|
||||||
ips = sorted(h for h in lan_hosts() if h.replace(".", "").isdigit())
|
hosts = lan_hosts()
|
||||||
names = sorted(h for h in lan_hosts() if not h.replace(".", "").isdigit())
|
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)
|
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"bggpipe web UI: {url}?k={token}")
|
||||||
typer.echo(f" from your phone, open: {addresses}")
|
typer.echo(f" from your phone, open: {addresses}")
|
||||||
|
|||||||
@@ -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):
|
def test_localhost_mutations_pass_with_origin_and_port(tmp_path):
|
||||||
# the path every real browser takes: Origin present + port in Host —
|
# the path every real browser takes: Origin present + port in Host —
|
||||||
# regressing the header parsing must fail loudly here
|
# 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 = {}
|
captured = {}
|
||||||
monkeypatch.setattr("uvicorn.run", lambda app, **kw: captured.update(kw, app=app))
|
monkeypatch.setattr("uvicorn.run", lambda app, **kw: captured.update(kw, app=app))
|
||||||
cfg = make_cfg(tmp_path)
|
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)
|
wr.run_web_review(cfg, port=9999, lan=True)
|
||||||
out = capsys.readouterr().out
|
out = capsys.readouterr().out
|
||||||
assert captured["host"] == "0.0.0.0"
|
assert captured["host"] == "0.0.0.0"
|
||||||
assert "?k=" in out # every printed URL carries the access key
|
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
|
assert "Use only on a network you trust" in out
|
||||||
wr.run_web_review(cfg, port=9999, lan=False)
|
wr.run_web_review(cfg, port=9999, lan=False)
|
||||||
assert captured["host"] == "127.0.0.1"
|
assert captured["host"] == "127.0.0.1"
|
||||||
|
|||||||
Reference in New Issue
Block a user