fix: close two CSRF origin-check security gaps
- Extend origin check to all /api/ requests (not just state-changing methods), closing the GET/HEAD/OPTIONS bypass that allowed cross-origin reads - origin_allowed() now returns False when both Origin and Referer headers are absent, preventing script-initiated requests from bypassing the check - Update AGENTS.md and README.md to document the changes
This commit is contained in:
@@ -53,6 +53,7 @@ The upstream request includes `"logprobs": true`. `parse_llama_stream_chunk()` e
|
|||||||
- Guest session by default (`POST /api/auth/guest`), admin unlock via 4-digit PIN (`POST /api/auth/login`)
|
- Guest session by default (`POST /api/auth/guest`), admin unlock via 4-digit PIN (`POST /api/auth/login`)
|
||||||
- Admin required for PUT/DELETE/PATCH + all POST except allowlist (`/api/chat`, `/api/search`, `/api/auth/*`)
|
- Admin required for PUT/DELETE/PATCH + all POST except allowlist (`/api/chat`, `/api/search`, `/api/auth/*`)
|
||||||
- IP allowlist, rate limiting, origin checking, payload size limits — all enforced in `app.py` middleware
|
- IP allowlist, rate limiting, origin checking, payload size limits — all enforced in `app.py` middleware
|
||||||
|
- Origin check applies to **all** `/api/` requests (not just state-changing methods); `origin_allowed()` returns `False` when both `Origin` and `Referer` headers are absent, closing CSRF read gap
|
||||||
- `JARVISCHAT_ADMIN_PIN` env var required on first boot (or `JARVISCHAT_ALLOW_DEFAULT_PIN=true`)
|
- `JARVISCHAT_ADMIN_PIN` env var required on first boot (or `JARVISCHAT_ALLOW_DEFAULT_PIN=true`)
|
||||||
|
|
||||||
### Database
|
### Database
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ Developer wiki: [docs/wiki/Home.md](docs/wiki/Home.md)
|
|||||||
- **Secure SSE protocol** — raw search results are no longer leaked in the SSE event stream
|
- **Secure SSE protocol** — raw search results are no longer leaked in the SSE event stream
|
||||||
- **FTS5 query safety** — operator keywords (`AND`, `OR`, `NOT`, `NEAR`) are double-quoted to prevent parse errors
|
- **FTS5 query safety** — operator keywords (`AND`, `OR`, `NOT`, `NEAR`) are double-quoted to prevent parse errors
|
||||||
- **All 8 test files fixed** — rewired imports after the modular refactor; all 26 tests pass
|
- **All 8 test files fixed** — rewired imports after the modular refactor; all 26 tests pass
|
||||||
|
- **Origin check extended to all API methods** — GET/HEAD/OPTIONS requests no longer bypass origin checking (was limited to POST/PUT/DELETE/PATCH)
|
||||||
|
- **Missing headers now rejected** — `origin_allowed()` returns `False` when both `Origin` and `Referer` are absent, closing the CSRF read gap for script-initiated requests
|
||||||
|
|
||||||
## Features
|
## Features
|
||||||
|
|
||||||
|
|||||||
2
app.py
2
app.py
@@ -102,7 +102,7 @@ async def session_auth_middleware(request: Request, call_next):
|
|||||||
"/api/auth/heartbeat", "/api/auth/guest",
|
"/api/auth/heartbeat", "/api/auth/guest",
|
||||||
}
|
}
|
||||||
|
|
||||||
if path.startswith("/api/") and is_state_changing(request.method):
|
if path.startswith("/api/"):
|
||||||
if not origin_allowed(request):
|
if not origin_allowed(request):
|
||||||
audit_event("origin_check", "denied", ip=ip, role="none",
|
audit_event("origin_check", "denied", ip=ip, role="none",
|
||||||
details=f"{request.method} {path}", warning=True)
|
details=f"{request.method} {path}", warning=True)
|
||||||
|
|||||||
@@ -156,7 +156,7 @@ def origin_allowed(request: Request) -> bool:
|
|||||||
parsed = urlparse(referer)
|
parsed = urlparse(referer)
|
||||||
ref_origin = f"{parsed.scheme}://{parsed.netloc}".rstrip("/")
|
ref_origin = f"{parsed.scheme}://{parsed.netloc}".rstrip("/")
|
||||||
return ref_origin == expected_origin or ref_origin in TRUSTED_ORIGINS
|
return ref_origin == expected_origin or ref_origin in TRUSTED_ORIGINS
|
||||||
return True
|
return False
|
||||||
|
|
||||||
|
|
||||||
def is_state_changing(method: str) -> bool:
|
def is_state_changing(method: str) -> bool:
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ def test_guest_read_only_admin_write_blocked(tmp_path: Path):
|
|||||||
guest = client.post("/api/auth/guest", headers={"Origin": "http://testserver"})
|
guest = client.post("/api/auth/guest", headers={"Origin": "http://testserver"})
|
||||||
assert guest.status_code == 200
|
assert guest.status_code == 200
|
||||||
sid = guest.json()["session_id"]
|
sid = guest.json()["session_id"]
|
||||||
headers = {"X-Session-ID": sid}
|
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
||||||
|
|
||||||
read_resp = client.get("/api/memories", headers=headers)
|
read_resp = client.get("/api/memories", headers=headers)
|
||||||
assert read_resp.status_code == 200
|
assert read_resp.status_code == 200
|
||||||
@@ -76,5 +76,5 @@ def test_logout_revokes_session(tmp_path: Path):
|
|||||||
logout = client.post("/api/auth/logout", headers=headers)
|
logout = client.post("/api/auth/logout", headers=headers)
|
||||||
assert logout.status_code == 200
|
assert logout.status_code == 200
|
||||||
|
|
||||||
after = client.get("/api/memories", headers={"X-Session-ID": sid})
|
after = client.get("/api/memories", headers={"X-Session-ID": sid, "Origin": "http://testserver"})
|
||||||
assert after.status_code == 401
|
assert after.status_code == 401
|
||||||
|
|||||||
@@ -172,7 +172,7 @@ def test_memory_command_paths_remember_and_forget(tmp_path: Path, monkeypatch):
|
|||||||
remember_events = parse_sse_payloads(remember_resp.text)
|
remember_events = parse_sse_payloads(remember_resp.text)
|
||||||
assert any("Remembered" in p.get("token", "") for p in remember_events)
|
assert any("Remembered" in p.get("token", "") for p in remember_events)
|
||||||
|
|
||||||
memories_after_add = client.get("/api/memories", headers={"X-Session-ID": sid})
|
memories_after_add = client.get("/api/memories", headers={"X-Session-ID": sid, "Origin": "http://testserver"})
|
||||||
assert memories_after_add.status_code == 200
|
assert memories_after_add.status_code == 200
|
||||||
assert memories_after_add.json().get("count", 0) >= 1
|
assert memories_after_add.json().get("count", 0) >= 1
|
||||||
|
|
||||||
@@ -188,6 +188,6 @@ def test_memory_command_paths_remember_and_forget(tmp_path: Path, monkeypatch):
|
|||||||
forget_events = parse_sse_payloads(forget_resp.text)
|
forget_events = parse_sse_payloads(forget_resp.text)
|
||||||
assert any("Forgot" in p.get("token", "") for p in forget_events)
|
assert any("Forgot" in p.get("token", "") for p in forget_events)
|
||||||
|
|
||||||
memories_after_forget = client.get("/api/memories", headers={"X-Session-ID": sid})
|
memories_after_forget = client.get("/api/memories", headers={"X-Session-ID": sid, "Origin": "http://testserver"})
|
||||||
assert memories_after_forget.status_code == 200
|
assert memories_after_forget.status_code == 200
|
||||||
assert memories_after_forget.json().get("count", 0) == 0
|
assert memories_after_forget.json().get("count", 0) == 0
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ def test_unhandled_api_exception_returns_friendly_error_with_incident_key(
|
|||||||
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()[
|
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()[
|
||||||
"session_id"
|
"session_id"
|
||||||
]
|
]
|
||||||
headers = {"X-Session-ID": sid}
|
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
||||||
|
|
||||||
def boom(_topic=None):
|
def boom(_topic=None):
|
||||||
raise RuntimeError("super secret db internals")
|
raise RuntimeError("super secret db internals")
|
||||||
|
|||||||
@@ -40,5 +40,5 @@ def test_middleware_blocks_disallowed_ip(tmp_path: Path, monkeypatch):
|
|||||||
def test_middleware_allows_local_ip(tmp_path: Path, monkeypatch):
|
def test_middleware_allows_local_ip(tmp_path: Path, monkeypatch):
|
||||||
monkeypatch.setattr(app, "get_client_ip", lambda _req: "192.168.50.109")
|
monkeypatch.setattr(app, "get_client_ip", lambda _req: "192.168.50.109")
|
||||||
with make_client(tmp_path) as client:
|
with make_client(tmp_path) as client:
|
||||||
resp = client.post("/api/auth/guest")
|
resp = client.post("/api/auth/guest", headers={"Origin": "http://testserver"})
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
|
|||||||
@@ -28,8 +28,8 @@ def test_stats_rate_limit_hits_429(tmp_path: Path):
|
|||||||
app.RATE_WINDOW_SECONDS = 60
|
app.RATE_WINDOW_SECONDS = 60
|
||||||
try:
|
try:
|
||||||
with make_client(tmp_path) as client:
|
with make_client(tmp_path) as client:
|
||||||
sid = client.post("/api/auth/guest").json()["session_id"]
|
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()["session_id"]
|
||||||
headers = {"X-Session-ID": sid}
|
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
||||||
|
|
||||||
r1 = client.get("/api/stats", headers=headers)
|
r1 = client.get("/api/stats", headers=headers)
|
||||||
r2 = client.get("/api/stats", headers=headers)
|
r2 = client.get("/api/stats", headers=headers)
|
||||||
@@ -56,7 +56,7 @@ def test_large_login_payload_rejected_413(tmp_path: Path):
|
|||||||
|
|
||||||
def test_chat_message_length_rejected_413(tmp_path: Path):
|
def test_chat_message_length_rejected_413(tmp_path: Path):
|
||||||
with make_client(tmp_path) as client:
|
with make_client(tmp_path) as client:
|
||||||
sid = client.post("/api/auth/guest").json()["session_id"]
|
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()["session_id"]
|
||||||
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
||||||
message = "x" * (config.MAX_CHAT_MESSAGE_CHARS + 1)
|
message = "x" * (config.MAX_CHAT_MESSAGE_CHARS + 1)
|
||||||
resp = client.post(
|
resp = client.post(
|
||||||
@@ -69,7 +69,7 @@ def test_chat_message_length_rejected_413(tmp_path: Path):
|
|||||||
|
|
||||||
def test_search_query_length_rejected_413(tmp_path: Path):
|
def test_search_query_length_rejected_413(tmp_path: Path):
|
||||||
with make_client(tmp_path) as client:
|
with make_client(tmp_path) as client:
|
||||||
sid = client.post("/api/auth/guest").json()["session_id"]
|
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()["session_id"]
|
||||||
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
headers = {"X-Session-ID": sid, "Origin": "http://testserver"}
|
||||||
query = "q" * (config.MAX_SEARCH_QUERY_CHARS + 1)
|
query = "q" * (config.MAX_SEARCH_QUERY_CHARS + 1)
|
||||||
resp = client.post(
|
resp = client.post(
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ def test_guest_can_list_skills(tmp_path: Path):
|
|||||||
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()[
|
sid = client.post("/api/auth/guest", headers={"Origin": "http://testserver"}).json()[
|
||||||
"session_id"
|
"session_id"
|
||||||
]
|
]
|
||||||
resp = client.get("/api/skills", headers={"X-Session-ID": sid})
|
resp = client.get("/api/skills", headers={"X-Session-ID": sid, "Origin": "http://testserver"})
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
payload = resp.json()
|
payload = resp.json()
|
||||||
assert payload["count"] >= 1
|
assert payload["count"] >= 1
|
||||||
@@ -50,7 +50,7 @@ def test_admin_can_toggle_skill_enabled_state(tmp_path: Path):
|
|||||||
assert disable.status_code == 200
|
assert disable.status_code == 200
|
||||||
assert disable.json()["skill"]["enabled"] is False
|
assert disable.json()["skill"]["enabled"] is False
|
||||||
|
|
||||||
active = client.get("/api/skills/active", headers={"X-Session-ID": sid})
|
active = client.get("/api/skills/active", headers={"X-Session-ID": sid, "Origin": "http://testserver"})
|
||||||
assert active.status_code == 200
|
assert active.status_code == 200
|
||||||
assert all(skill["key"] != "search.web" for skill in active.json()["skills"])
|
assert all(skill["key"] != "search.web" for skill in active.json()["skills"])
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user