Skip to content

Commit b4a5a24

Browse files
committed
fix(dashboard): fail closed on remote state reads
1 parent dcea4b4 commit b4a5a24

2 files changed

Lines changed: 248 additions & 47 deletions

File tree

dashboard/server.py

Lines changed: 39 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -192,53 +192,46 @@ def _rate_key(base: str, request: Optional[Request]) -> str:
192192
logger = logging.getLogger(__name__)
193193

194194

195-
# Reads that expose operational or credential-adjacent state. These stay open
196-
# to a LOCAL caller (zero-config use is the point) but must not be readable by
197-
# an anonymous remote caller when the dashboard is bound to 0.0.0.0.
195+
# Public API metadata needed before a caller can authenticate or before the UI
196+
# can choose a provider. Everything else under an API namespace is
197+
# state-bearing unless it is explicitly admitted here.
198198
#
199-
# Measured before this list existed, from a routable remote address with auth
200-
# off: /api/logs, /api/secrets/status, /api/github/status, /api/tasks,
201-
# /api/council/transcripts and /api/proofs all returned 200.
199+
# This is deliberately an ALLOWLIST, not the old list of sensitive prefixes.
200+
# That list silently started every new read family public. Measured from a
201+
# routable remote address with auth off, routes added after the list --
202+
# /api/context, /api/notifications, /api/agents, /api/usage,
203+
# /api/prd-observations and every /api/v2/* read -- all reached their handlers
204+
# and returned project, tenant or audit state.
202205
#
203-
# /health and /metrics are deliberately ABSENT: a container health probe and a
204-
# Prometheus scrape must keep working with no configuration, and neither
205-
# carries workspace content.
206-
_SENSITIVE_READ_PREFIXES = (
207-
"/api/logs",
208-
"/api/secrets",
209-
"/api/github",
210-
"/api/tasks",
211-
"/api/projects",
212-
"/api/council",
213-
"/api/proofs",
214-
"/api/phases",
215-
"/api/memory",
216-
"/api/learnings",
217-
"/api/learning",
218-
"/api/escalations",
219-
"/api/spec",
220-
"/api/checkpoints",
221-
"/api/enterprise",
222-
"/api/collab",
223-
"/api/cost",
224-
"/api/budget",
225-
"/api/findings",
226-
"/api/operator",
227-
"/api/fleet",
228-
"/api/registry",
229-
"/api/wiki",
230-
"/api/activity",
231-
"/api/session",
232-
"/api/failures",
233-
"/api/prompt",
234-
"/api/quality",
235-
"/api/migration",
236-
"/api/managed",
237-
"/api/app-runner",
238-
"/api/playwright",
239-
"/api/checklist",
240-
"/api/control",
241-
)
206+
# /health, /metrics, /.well-known/*, docs and static/UI routes are outside the
207+
# API namespaces and remain public. The three entries below contain only
208+
# capability/auth bootstrap metadata and are already pinned as public by the
209+
# dashboard auth inventory tests.
210+
_PUBLIC_API_GET_PATHS = frozenset({
211+
"/api/auth/info",
212+
"/api/enterprise/status",
213+
"/api/providers/models",
214+
})
215+
216+
217+
def _is_state_bearing_get(path: str) -> bool:
218+
"""Classify dashboard reads at the namespace boundary.
219+
220+
A future GET under /api or the mounted Purple Lab's /lab/api namespace is
221+
private by default. Exact public metadata is admitted above; probes,
222+
discovery documents and UI/static files live outside these namespaces.
223+
Normalize one trailing slash so FastAPI's redirect spelling cannot turn a
224+
public metadata request into a remote-only failure.
225+
"""
226+
normalized = path.rstrip("/") or "/"
227+
if normalized in _PUBLIC_API_GET_PATHS:
228+
return False
229+
return (
230+
normalized == "/api"
231+
or normalized.startswith("/api/")
232+
or normalized == "/lab/api"
233+
or normalized.startswith("/lab/api/")
234+
)
242235

243236

244237
def _trusted_proxies() -> frozenset:
@@ -1274,8 +1267,7 @@ async def dashboard_control_boundary(request: Request, call_next):
12741267
content={"detail": "cross-origin mutation refused"},
12751268
)
12761269
if not gated:
1277-
path = request.url.path
1278-
gated = any(path.startswith(pfx) for pfx in _SENSITIVE_READ_PREFIXES)
1270+
gated = _is_state_bearing_get(request.url.path)
12791271
if gated:
12801272
try:
12811273
require_local_or_authenticated(request)
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
"""Remote auth-off reads fail closed at one dashboard namespace boundary.
2+
3+
The old boundary enumerated sensitive prefixes. That shape leaked every read
4+
family added later: context, notifications, agents, usage, PRD observations,
5+
v2 tenants and v2 audit all reached their handlers from a routable address
6+
when auth was off. The tests below enumerate the live route tables and drive
7+
the app as raw ASGI without lifespan startup, so a handler cannot hide a
8+
missing boundary behind database, filesystem or startup failures.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import asyncio
14+
import os
15+
import pathlib
16+
import re
17+
import sys
18+
import unittest
19+
20+
21+
sys.dont_write_bytecode = True
22+
_ROOT = pathlib.Path(__file__).resolve().parents[2]
23+
if str(_ROOT) not in sys.path:
24+
sys.path.insert(0, str(_ROOT))
25+
26+
27+
_SENTINEL = "PACKET606-STATE-MUST-NOT-LEAK"
28+
_PUBLIC_API_GETS = {
29+
"/api/auth/info",
30+
"/api/enterprise/status",
31+
"/api/providers/models",
32+
}
33+
_PUBLIC_PROBES_AND_UI = (
34+
"/health",
35+
"/metrics",
36+
"/.well-known/agent.json",
37+
"/openapi.json",
38+
"/docs",
39+
"/favicon.svg",
40+
"/",
41+
"/cost",
42+
"/trust",
43+
)
44+
45+
46+
def _materialize(path: str) -> str:
47+
"""Turn a FastAPI route template into a harmless routable test path."""
48+
return re.sub(r"\{[^}]+\}", _SENTINEL, path)
49+
50+
51+
async def _raw_get(app, path: str, host: str = "203.0.113.7"):
52+
"""Issue one no-lifespan ASGI GET and return (status, response body)."""
53+
messages = []
54+
delivered = False
55+
56+
async def receive():
57+
nonlocal delivered
58+
if not delivered:
59+
delivered = True
60+
return {"type": "http.request", "body": b"", "more_body": False}
61+
return {"type": "http.disconnect"}
62+
63+
async def send(message):
64+
messages.append(message)
65+
66+
scope = {
67+
"type": "http",
68+
"asgi": {"version": "3.0", "spec_version": "2.3"},
69+
"http_version": "1.1",
70+
"method": "GET",
71+
"scheme": "http",
72+
"path": path,
73+
"raw_path": path.encode("utf-8"),
74+
"query_string": b"",
75+
"headers": [(b"host", b"dashboard.example")],
76+
"client": (host, 5555),
77+
"server": ("dashboard.example", 80),
78+
"root_path": "",
79+
}
80+
await app(scope, receive, send)
81+
status = next(
82+
message["status"]
83+
for message in messages
84+
if message["type"] == "http.response.start"
85+
)
86+
body = b"".join(
87+
message.get("body", b"")
88+
for message in messages
89+
if message["type"] == "http.response.body"
90+
)
91+
return status, body
92+
93+
94+
def _dashboard_get_inventory(server):
95+
"""Return every registered state-bearing GET, including the mounted lab."""
96+
paths = set()
97+
for route in server.app.routes:
98+
methods = getattr(route, "methods", None) or set()
99+
path = getattr(route, "path", None)
100+
if path and "GET" in methods and server._is_state_bearing_get(path):
101+
paths.add(path)
102+
103+
# Mounts have no methods at the parent table. Enumerate the mounted
104+
# Purple Lab app too, while testing the full path seen by the parent
105+
# middleware. Other mounts are static assets and carry no API routes.
106+
child = getattr(route, "app", None)
107+
# /lab is wrapped by _MountAuthGuard; unwrap only that transparent
108+
# boundary adapter to reach the mounted FastAPI route table.
109+
child = getattr(child, "_app", child)
110+
child_routes = getattr(child, "routes", None)
111+
if path == "/lab" and child_routes:
112+
for child_route in child_routes:
113+
child_methods = getattr(child_route, "methods", None) or set()
114+
child_path = getattr(child_route, "path", None)
115+
if child_path and "GET" in child_methods:
116+
full_path = path + child_path
117+
if server._is_state_bearing_get(full_path):
118+
paths.add(full_path)
119+
return sorted(paths)
120+
121+
122+
class RemoteAuthOffReadBoundary(unittest.TestCase):
123+
@classmethod
124+
def setUpClass(cls):
125+
os.environ.pop("LOKI_ENTERPRISE_AUTH", None)
126+
os.environ.pop("LOKI_OIDC_ISSUER", None)
127+
os.environ.pop("LOKI_OIDC_CLIENT_ID", None)
128+
from dashboard import auth, server # noqa: PLC0415
129+
130+
cls.auth, cls.server = auth, server
131+
cls._enterprise = auth.ENTERPRISE_AUTH_ENABLED
132+
cls._oidc = auth.OIDC_ENABLED
133+
auth.ENTERPRISE_AUTH_ENABLED = False
134+
auth.OIDC_ENABLED = False
135+
136+
@classmethod
137+
def tearDownClass(cls):
138+
cls.auth.ENTERPRISE_AUTH_ENABLED = cls._enterprise
139+
cls.auth.OIDC_ENABLED = cls._oidc
140+
141+
def test_public_api_allowlist_is_exact_and_complete(self):
142+
self.assertEqual(self.server._PUBLIC_API_GET_PATHS, _PUBLIC_API_GETS)
143+
144+
def test_named_old_red_families_are_in_the_dynamic_inventory(self):
145+
inventory = set(_dashboard_get_inventory(self.server))
146+
for path in (
147+
"/api/context",
148+
"/api/notifications",
149+
"/api/agents",
150+
"/api/usage",
151+
"/api/prd-observations",
152+
"/api/v2/tenants",
153+
"/api/v2/audit",
154+
):
155+
self.assertIn(path, inventory)
156+
157+
def test_every_state_bearing_get_is_403_before_a_handler_can_leak(self):
158+
inventory = _dashboard_get_inventory(self.server)
159+
self.assertGreaterEqual(
160+
len(inventory), 150,
161+
"route inventory unexpectedly shrank; mounted or dashboard reads "
162+
"may have escaped enumeration",
163+
)
164+
165+
async def exercise_all():
166+
failures = []
167+
for template in inventory:
168+
path = _materialize(template)
169+
status, body = await _raw_get(self.server.app, path)
170+
if status != 403 or _SENTINEL.encode() in body:
171+
failures.append((template, status, body[:160]))
172+
return failures
173+
174+
failures = asyncio.run(exercise_all())
175+
self.assertFalse(
176+
failures,
177+
"remote auth-off state-bearing GETs escaped the boundary: %r"
178+
% failures,
179+
)
180+
181+
def test_probes_public_metadata_and_ui_are_not_boundary_blocked(self):
182+
async def exercise_all():
183+
results = {}
184+
for path in _PUBLIC_PROBES_AND_UI + tuple(sorted(_PUBLIC_API_GETS)):
185+
results[path] = await _raw_get(self.server.app, path)
186+
return results
187+
188+
results = asyncio.run(exercise_all())
189+
for path, (status, body) in results.items():
190+
self.assertNotEqual(
191+
status, 403,
192+
"%s was blocked by the remote read boundary: %r"
193+
% (path, body[:160]),
194+
)
195+
self.assertEqual(results["/health"][0], 200)
196+
self.assertEqual(results["/metrics"][0], 200)
197+
for path in _PUBLIC_API_GETS:
198+
self.assertEqual(results[path][0], 200, path)
199+
200+
def test_loopback_state_reads_keep_zero_config_behavior(self):
201+
for path in ("/api/context", "/api/usage", "/api/v2/tenants"):
202+
status, _body = asyncio.run(
203+
_raw_get(self.server.app, path, host="127.0.0.1")
204+
)
205+
self.assertNotEqual(status, 403, path)
206+
207+
208+
if __name__ == "__main__":
209+
unittest.main()

0 commit comments

Comments
 (0)