fix(ui): stop our own Referrer-Policy from breaking every write
This is why approving a note returned {"error":"bad origin"}, and it was never
the Host comparison: the UI shipped `Referrer-Policy: no-referrer`.
Per the Fetch standard, that policy makes a browser send `Origin: null` on an
HTML form submission — and suppress the Referer the CSRF guard falls back to.
Every write in this UI is a form POST and the guard fails closed, so both of its
inputs were destroyed by our own security header and every approve, reject,
edit, delete and health action was refused. On master too: this has never worked
in a real browser, on any build.
The tests did not catch it because they set Origin by hand, which is exactly
what a browser under that policy will not do.
`same-origin` sends the real Origin on same-origin requests and nulls it only
cross-origin — precisely the case the guard exists to refuse — while still
sending no referrer information off-site. The header test now asserts the
policy is not any of the values that null the Origin on a form submission, so
the reason is pinned rather than the string, and a new test posts the shape a
browser actually sends: a real Origin and no Referer.
`Origin: null` is now named in the refusal log, since it means something
upstream stripped the origin rather than the caller being on the wrong host.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0139vHnNpS2sLNELjrAHX5Cu
This commit is contained in:
1 parent
85783c360b
commit
f6de7845ad
2 files changed
+66
-2
No files matched your search
+20
-2
@@ -154,7 +154,7 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
"fields": {
|
||||
"path": path,
|
||||
"method": request.method,
|
||||
"reason": "no Origin or Referer" if candidate is None else "origin not allowed",
|
||||
"reason": _refusal_reason(candidate),
|
||||
"origin": candidate,
|
||||
"host": request.headers.get("host"),
|
||||
"x_forwarded_host": request.headers.get("x-forwarded-host"),
|
||||
@@ -178,7 +178,14 @@ def build_ui_app(svc: MemoryService) -> FastAPI: # noqa: C901 - one place, many
|
||||
response = await call_next(request)
|
||||
response.headers["Content-Security-Policy"] = CSP
|
||||
response.headers["X-Content-Type-Options"] = "nosniff"
|
||||
response.headers["Referrer-Policy"] = "no-referrer"
|
||||
# NOT "no-referrer", however tempting: per the Fetch standard a referrer
|
||||
# policy of `no-referrer` makes the browser send `Origin: null` on HTML
|
||||
# form submissions — and suppress the Referer that the CSRF guard falls
|
||||
# back to. Every write in this UI is a form POST, so that header refused
|
||||
# every one of them with "bad origin". `same-origin` nulls the Origin
|
||||
# only on cross-origin requests, which is precisely the case the guard
|
||||
# is meant to refuse, and still sends no referrer information off-site.
|
||||
response.headers["Referrer-Policy"] = "same-origin"
|
||||
response.headers["X-Frame-Options"] = "DENY"
|
||||
return response
|
||||
|
||||
@@ -665,6 +672,17 @@ def _origin_allowed(origin: str, settings: Any, request: Request) -> bool:
|
||||
return origin.rstrip("/") in {a.rstrip("/") for a in allowed}
|
||||
|
||||
|
||||
def _refusal_reason(candidate: str | None) -> str:
|
||||
if candidate is None:
|
||||
return "no Origin or Referer"
|
||||
if candidate.strip().lower() == "null":
|
||||
# Worth calling out by name: an opaque origin means something upstream
|
||||
# stripped it — a referrer policy of no-referrer, a sandboxed iframe, or
|
||||
# a privacy extension — not that the caller is on the wrong host.
|
||||
return "opaque origin (Origin: null) — check Referrer-Policy"
|
||||
return "origin not allowed"
|
||||
|
||||
|
||||
def _allowed_origins(settings: Any) -> set[str]:
|
||||
allowed = set(settings.ui_allowed_origins)
|
||||
if settings.ui_base_url:
|
||||
|
||||
@@ -295,6 +295,17 @@ async def test_security_headers_are_set(app_and_svc):
|
||||
r = await c.get("/ui/inbox")
|
||||
assert r.headers["content-security-policy"].startswith("default-src 'self'")
|
||||
assert r.headers["x-frame-options"] == "DENY"
|
||||
# Per the Fetch standard these policies make a browser send `Origin: null`
|
||||
# on an HTML form submission. Every write in this UI is a form POST and the
|
||||
# CSRF guard fails closed, so shipping one of them silently refuses every
|
||||
# write in a real browser while every test that sets Origin by hand passes.
|
||||
assert r.headers["referrer-policy"] not in {
|
||||
"no-referrer",
|
||||
"no-referrer-when-downgrade",
|
||||
"strict-origin",
|
||||
"strict-origin-when-cross-origin",
|
||||
}
|
||||
assert r.headers["referrer-policy"] == "same-origin"
|
||||
|
||||
|
||||
async def test_post_from_a_foreign_origin_is_refused(app_and_svc):
|
||||
@@ -493,3 +504,38 @@ async def test_a_write_with_no_origin_header_says_so_in_the_log(app_and_svc, cap
|
||||
assert r.status_code == 403
|
||||
record = next(r for r in caplog.records if "CSRF" in r.message)
|
||||
assert record.fields["reason"] == "no Origin or Referer"
|
||||
|
||||
|
||||
async def test_an_opaque_origin_is_refused_and_named_in_the_log(app_and_svc, caplog):
|
||||
"""`Origin: null` is what a browser sends for a form POST under
|
||||
Referrer-Policy: no-referrer. Refusing it is right, but the log has to say
|
||||
*why* — it means something stripped the origin, not that the caller is on
|
||||
the wrong host."""
|
||||
import logging as _logging
|
||||
|
||||
app, svc = app_and_svc
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
with caplog.at_level(_logging.WARNING, logger="mdmem.ui"):
|
||||
async with _client(app, cookie) as c:
|
||||
r = await c.post(
|
||||
"/ui/n/whatever/state", data={"state": "archive"}, headers={"origin": "null"}
|
||||
)
|
||||
assert r.status_code == 403
|
||||
record = next(r for r in caplog.records if "CSRF" in r.message)
|
||||
assert "opaque origin" in record.fields["reason"]
|
||||
|
||||
|
||||
async def test_a_real_browser_form_post_is_accepted(app_and_svc):
|
||||
"""The end-to-end shape a browser actually sends for a same-origin form POST
|
||||
under the policy this app ships: a real Origin, no Referer."""
|
||||
app, svc = app_and_svc
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
w = await svc.write_note(title="Form post", body="v1", type="fact")
|
||||
async with _client(app, cookie) as c:
|
||||
r = await c.post(
|
||||
f"/ui/n/{w['id']}/approve",
|
||||
data={},
|
||||
headers={"origin": "https://memory.cloud.traunseenet.com"},
|
||||
)
|
||||
assert r.status_code == 303
|
||||
assert svc.get_note(w["id"]).state == "active"
|
||||
Reference in new issue
Block a user