fix(ui): compare Host and Origin as authorities, not as raw strings
The Host cross-check added for M-10 compared the Host header to the Origin's
netloc with ==. That does not survive a reverse proxy: a browser omits the
default port from Origin, while Traefik forwards Host (and X-Forwarded-Host)
with it, so `memory.example.com` and `memory.example.com:443` are one authority
spelled two ways. The result was a 403 {"error":"bad origin"} on every
state-changing POST in the deployed UI — approve, reject, edit, delete and every
health action — which left the UI effectively read-only.
Both sides are now normalised to (host, port) with the scheme's default port
filled in, lowercased, IPv6 literals handled. An allowed Origin paired with a
foreign Host is still refused, which is the case the check exists for: the tests
cover evil.example, evil.example:443 and a suffix-match attempt alongside the
proxy shapes that must pass.
Reverting the comparison fails the proxy test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0139vHnNpS2sLNELjrAHX5Cu
This commit is contained in:
1 parent
272346d8ad
commit
647f8ceb9b
2 files changed
+63
-2
No files matched your search
+27
-2
@@ -637,8 +637,7 @@ def _origin_allowed(origin: str, settings: Any, request: Request) -> bool:
|
||||
parsed_origin = urlparse(origin)
|
||||
if not parsed_origin.scheme or not parsed_origin.netloc:
|
||||
return False
|
||||
host = _request_host(request)
|
||||
if host != parsed_origin.netloc:
|
||||
if not _same_authority(_request_host(request), parsed_origin):
|
||||
return False
|
||||
allowed = set(settings.ui_allowed_origins)
|
||||
if settings.ui_base_url:
|
||||
@@ -653,6 +652,32 @@ def _request_host(request: Request) -> str:
|
||||
return forwarded or request.headers.get("host", "")
|
||||
|
||||
|
||||
def _split_authority(value: str, scheme: str) -> tuple[str, str]:
|
||||
"""(host, port) with the scheme's default port filled in, lowercased."""
|
||||
v = value.strip().lower()
|
||||
if v.startswith("["): # IPv6 literal: [::1]:8081
|
||||
host, _, rest = v.partition("]")
|
||||
host, port = host + "]", rest[1:] if rest.startswith(":") else ""
|
||||
else:
|
||||
host, _, port = v.partition(":")
|
||||
return host, port or ("443" if scheme == "https" else "80")
|
||||
|
||||
|
||||
def _same_authority(host_header: str, origin: Any) -> bool:
|
||||
"""Does the Host header name the same authority as this Origin?
|
||||
|
||||
A plain string comparison does not survive a reverse proxy: browsers omit the
|
||||
default port from Origin while Traefik forwards Host with it, so
|
||||
`memory.example.com` and `memory.example.com:443` are the same authority
|
||||
spelled two ways — comparing them raw refused every state-changing POST in
|
||||
the deployed UI. Normalising the default port and case still refuses the case
|
||||
this check exists for, an allowed Origin paired with a foreign Host (M-10).
|
||||
"""
|
||||
if not host_header:
|
||||
return False
|
||||
return _split_authority(host_header, origin.scheme) == _split_authority(origin.netloc, origin.scheme)
|
||||
|
||||
|
||||
def _diff(old: str, new: str, label: str) -> str:
|
||||
return "\n".join(
|
||||
difflib.unified_diff(
|
||||
|
||||
@@ -416,3 +416,39 @@ async def test_auth_disabled_is_loudly_logged_and_lets_requests_through(settings
|
||||
assert (await c.get("/ui/inbox")).status_code == 200
|
||||
finally:
|
||||
await svc.stop()
|
||||
|
||||
|
||||
async def test_post_survives_a_proxy_that_forwards_the_default_port(app_and_svc):
|
||||
"""Browsers omit the default port from Origin; Traefik forwards Host with it.
|
||||
Comparing the two raw refused every state-changing POST in the deployed UI."""
|
||||
app, svc = app_and_svc
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
w = await svc.write_note(title="T", body="v1", type="fact")
|
||||
|
||||
for host in (
|
||||
"memory.cloud.traunseenet.com:443", # proxy keeps the default port
|
||||
"MEMORY.Cloud.Traunseenet.com", # case differs
|
||||
"memory.cloud.traunseenet.com", # the plain form still works
|
||||
):
|
||||
async with _client(app, cookie) as c:
|
||||
r = await c.post(
|
||||
f"/ui/n/{w['id']}/state",
|
||||
data={"state": "archive"},
|
||||
headers={"origin": "https://memory.cloud.traunseenet.com", "host": host},
|
||||
)
|
||||
assert r.status_code != 403, f"host {host!r} was refused"
|
||||
|
||||
|
||||
async def test_a_foreign_host_is_still_refused_with_an_allowed_origin(app_and_svc):
|
||||
"""The normalisation must not weaken what the check exists for (M-10)."""
|
||||
app, svc = app_and_svc
|
||||
cookie = SessionCodec(svc.settings.session_secret, 12).issue(Principal("u1", "M", ""))
|
||||
w = await svc.write_note(title="T2", body="v1", type="fact")
|
||||
for host in ("evil.example", "evil.example:443", "memory.cloud.traunseenet.com.evil.example"):
|
||||
async with _client(app, cookie) as c:
|
||||
r = await c.post(
|
||||
f"/ui/n/{w['id']}/state",
|
||||
data={"state": "archive"},
|
||||
headers={"origin": "https://memory.cloud.traunseenet.com", "host": host},
|
||||
)
|
||||
assert r.status_code == 403, f"host {host!r} was accepted"
|
||||
Reference in new issue
Block a user