Skip to content

Commit b04ab94

Browse files
committed
fix(auth): add type guards for cross-site browser request headers and fix test mock headers for loopback auth checks
1 parent 5398131 commit b04ab94

6 files changed

Lines changed: 24 additions & 12 deletions

File tree

memanto/app/routes/auth_deps.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def _is_loopback_host(host: str | None) -> bool:
107107

108108
def _is_loopback_origin(origin: str | None) -> bool:
109109
"""Return True when a browser Origin points at the local Memanto host."""
110-
if not origin:
110+
if not origin or not isinstance(origin, str):
111111
return False
112112
try:
113113
parsed = urlsplit(origin)
@@ -120,7 +120,7 @@ def _is_loopback_origin(origin: str | None) -> bool:
120120

121121
def _is_loopback_host_header(host: str | None) -> bool:
122122
"""Return True when an HTTP Host header names a loopback interface."""
123-
if not host:
123+
if not host or not isinstance(host, str):
124124
return False
125125
try:
126126
hostname = urlsplit(f"//{host}").hostname
@@ -132,10 +132,14 @@ def _is_loopback_host_header(host: str | None) -> bool:
132132
def _is_cross_site_browser_request(request: Request) -> bool:
133133
"""Detect browser requests that must not inherit loopback trust."""
134134
origin = request.headers.get("origin")
135-
if origin is not None:
135+
if origin is not None and isinstance(origin, str):
136136
return not _is_loopback_origin(origin)
137137

138-
fetch_site = request.headers.get("sec-fetch-site", "").strip().lower()
138+
fetch_site = request.headers.get("sec-fetch-site", "")
139+
if isinstance(fetch_site, str):
140+
fetch_site = fetch_site.strip().lower()
141+
else:
142+
fetch_site = ""
139143
return fetch_site in {"cross-site", "same-site"}
140144

141145

memanto/app/ui/routes/ui_router.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from datetime import datetime, timedelta, timezone
1515
from pathlib import Path
1616
from typing import Any
17-
from memanto.app.routes.auth_deps import _is_cross_site_browser_request
1817

1918
from fastapi import (
2019
APIRouter,
@@ -29,7 +28,11 @@
2928

3029
from memanto.app.clients.backend import Backend
3130
from memanto.app.config import settings
32-
from memanto.app.routes.auth_deps import clear_session_cookie, set_session_cookie
31+
from memanto.app.routes.auth_deps import (
32+
_is_cross_site_browser_request,
33+
clear_session_cookie,
34+
set_session_cookie,
35+
)
3336
from memanto.app.utils.temporal_helpers import utc_date_str
3437
from memanto.app.utils.validation import validate_safe_id
3538
from memanto.cli.client.direct_client import DirectClient
@@ -95,8 +98,6 @@ async def _require_local(request: Request) -> None:
9598
),
9699
)
97100

98-
from memanto.app.routes.auth_deps import _is_cross_site_browser_request
99-
100101
if _is_cross_site_browser_request(request):
101102
raise HTTPException(
102103
status_code=403,

tests/test_api.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ async def test_cross_site_loopback_cannot_create_agent(self, client):
184184
response = await client.post(
185185
"/api/v2/agents",
186186
headers={
187+
"Host": "localhost:8000",
187188
"Origin": "https://evil.example",
188189
"Sec-Fetch-Site": "cross-site",
189190
},
@@ -205,7 +206,10 @@ async def test_cross_site_loopback_cannot_activate_agent(
205206

206207
response = await client.post(
207208
"/api/v2/agents/cross-site-activate/activate",
208-
headers={"Sec-Fetch-Site": "cross-site"},
209+
headers={
210+
"Host": "localhost:8000",
211+
"Sec-Fetch-Site": "cross-site",
212+
},
209213
)
210214

211215
assert response.status_code == 401

tests/test_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def isolated_dirs():
8585
async def http():
8686
"""Fresh async HTTP client pointing at the real app (no mocks)."""
8787
async with AsyncClient(
88-
transport=ASGITransport(app=app), base_url="http://test"
88+
transport=ASGITransport(app=app), base_url="http://localhost:8000"
8989
) as client:
9090
yield client
9191

tests/test_remaining_ui_auth.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,5 @@ def test_require_local_allows_loopback_for_conflict_scans(self):
139139

140140
mock_request = MagicMock()
141141
mock_request.client.host = "127.0.0.1"
142+
mock_request.headers = {}
142143
asyncio.run(_require_local(mock_request)) # must not raise

tests/test_ui_auth.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,14 @@ def test_testclient_host_rejected(self):
149149

150150
def test_loopback_origin_accepted(self):
151151
"""Same-origin UI requests from localhost must be allowed."""
152-
from memanto.app.ui.routes.ui_router import _is_loopback_origin
152+
from memanto.app.routes.auth_deps import _is_loopback_origin
153153

154154
assert _is_loopback_origin("http://localhost:8000") is True
155155
assert _is_loopback_origin("http://127.0.0.1:8000") is True
156156
assert _is_loopback_origin("http://[::1]:8000") is True
157157

158158
def test_remote_origin_rejected(self):
159-
from memanto.app.ui.routes.ui_router import _is_loopback_origin
159+
from memanto.app.routes.auth_deps import _is_loopback_origin
160160

161161
assert _is_loopback_origin("https://evil.example") is False
162162

@@ -166,6 +166,7 @@ def test_require_local_allows_loopback(self):
166166

167167
mock_request = MagicMock()
168168
mock_request.client.host = "127.0.0.1"
169+
mock_request.headers = {}
169170
asyncio.run(_require_local(mock_request)) # must not raise
170171

171172
def test_require_local_allows_ipv4_mapped_loopback(self):
@@ -174,4 +175,5 @@ def test_require_local_allows_ipv4_mapped_loopback(self):
174175

175176
mock_request = MagicMock()
176177
mock_request.client.host = "::ffff:127.0.0.1"
178+
mock_request.headers = {}
177179
asyncio.run(_require_local(mock_request)) # must not raise

0 commit comments

Comments
 (0)