diff --git a/backend/src/control_center/api/routes_llm.py b/backend/src/control_center/api/routes_llm.py index e376093..c56d40d 100644 --- a/backend/src/control_center/api/routes_llm.py +++ b/backend/src/control_center/api/routes_llm.py @@ -2,14 +2,31 @@ import asyncio import os from pathlib import Path +from typing import Optional import httpx -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Header from fastapi.responses import JSONResponse -from control_center.core.auth import require_permission +from control_center.core.jwt_verify import TokenInvalid, verify_token router = APIRouter() + +def _has_permission(authorization: Optional[str], permission: str) -> bool: + """Non-raising counterpart to core.auth.require_permission -- same + pattern as routes_dashboard.py's own _has_permission (duplicated + rather than imported, since that one is private to its own module). + Used only to decide which fields of a response are safe to include, + never to reject the request outright.""" + if not authorization or not authorization.lower().startswith("bearer "): + return False + token = authorization.split(" ", 1)[1].strip() + try: + payload = verify_token(token) + except TokenInvalid: + return False + return permission in (payload.get("permissions") or []) + OLLAMA_URL = os.environ.get("OLLAMA_BASE_URL", "http://ollama:11434") @router.get("/llms") @@ -148,14 +165,24 @@ def _index_size_bytes(index_root: Path) -> int: @router.get("/knowledge-base") async def get_knowledge_base( - _admin: dict = Depends(require_permission("platform.manage_infra")), + authorization: Optional[str] = Header(default=None), ) -> JSONResponse: - # Gated per-route (not via llm_router's include, which is ungated so - # GET /llms above can stay public). Returns absolute internal - # filesystem paths (pubmed_root/index_root) -- same platform.manage_infra - # bar as /summary/docker/config/storage. Commit 8705cbf first added - # this gate at the router level; the 2026-09-02 investigation moved it - # here so it no longer also covers /llms. + # PUBLIC_FIELDS-style split (same pattern as routes_dashboard.py's + # /dashboard/summary "knowledge" section): this used to be gated + # per-route behind platform.manage_infra for its *entire* response, + # because pubmed_root/index_root are absolute internal filesystem + # paths -- same bar as /summary/docker/config/storage. But that + # blanket gate also hid the aggregate fields (abstract/domain counts, + # index size, rag_status) that carry no such sensitivity, which is + # what broke generate_report.py's unauthenticated fetch (it only ever + # read the aggregate fields -- see scripts/sections/knowledge_base.py + # -- and has no way to authenticate itself). Aggregate stats are now + # always returned; only pubmed_root/index_root stay behind + # platform.manage_infra, checked below via _has_permission rather + # than a hard Depends() so the rest of the response survives an + # absent/insufficient token instead of 401ing outright. + has_infra_permission = _has_permission(authorization, "platform.manage_infra") + workspace = Path(os.environ.get("WORKSPACE_ROOT", "/workspace")) pubmed_root = None @@ -227,6 +254,6 @@ async def check_rag() -> str: "size_gb": round(index_size_bytes / 1e9, 2), "domain_list": sorted(indexed_domains)[:20], }, - "pubmed_root": str(pubmed_root) if pubmed_root else None, - "index_root": str(index_root) if index_root else None, + "pubmed_root": str(pubmed_root) if (pubmed_root and has_infra_permission) else None, + "index_root": str(index_root) if (index_root and has_infra_permission) else None, }) diff --git a/backend/tests/test_main.py b/backend/tests/test_main.py index b48f6fc..cde262f 100644 --- a/backend/tests/test_main.py +++ b/backend/tests/test_main.py @@ -620,8 +620,17 @@ class TestPlatformManageInfraAuth(unittest.TestCase): this list, for the same reason -- see report_data()'s own docstring in main.py and TestReportData above. Unlike the 09-02 cases, this one is a conscious, accepted tradeoff (gitStatus[] becoming public), not a - correction of an over-gate. Everything still in _cases() below stays - gated -- that is the regression guard this decision must not weaken.""" + correction of an over-gate. + + NOTE (2026-09-12 decision): /knowledge-base has also been removed from + this list. Unlike /report/status and /llms, it isn't fully public -- + only its aggregate fields are (abstract/domain counts, index size, + rag_status); pubmed_root/index_root stay gated behind + platform.manage_infra via routes_llm.py's own _has_permission check, + same PUBLIC_FIELDS-style split routes_dashboard.py's /dashboard/summary + already uses. See TestKnowledgeBasePublicFields below. Everything + still in _cases() below stays gated -- that is the regression guard + this decision must not weaken.""" def _cases(self): return ( @@ -632,7 +641,6 @@ def _cases(self): ("GET", "/"), ("GET", "/report"), ("GET", "/coverage/status"), - ("GET", "/knowledge-base"), ("GET", "/storage"), ("GET", "/cron/jobs"), ("GET", "/cron/jobs/mysql-backup/log"), @@ -834,8 +842,9 @@ class TestLlmsPublicAccess(unittest.TestCase): Commit 8705cbf's blanket llm_router gate collapsed it into the admin tier; it backs ControlApp's anonymous LLMs page (91755fb, docs/public-control-center.md). GET /knowledge-base on the same - router stays gated -- see TestKnowledgeBaseStillGated below and - TestPlatformManageInfraAuth.""" + router is a narrower case -- its aggregate fields are public too + (see TestKnowledgeBasePublicFields below) but pubmed_root/index_root + stay gated, unlike /llms which has nothing gated left in it.""" def test_200_when_no_token(self): resp = client.get("/llms") @@ -852,18 +861,60 @@ def test_200_with_token_too(self): self.assertEqual(client.get("/llms", headers=_admin_headers()).status_code, 200) +class TestKnowledgeBasePublicFields(unittest.TestCase): + """2026-09-12 decision: GET /knowledge-base's aggregate fields + (abstract/domain counts, index size, rag_status) are public -- + generate_report.py's knowledge_base_section_html calls this route + unauthenticated and only ever reads those fields (never + pubmed_root/index_root), so the route's former blanket + platform.manage_infra gate meant this section of the ecosystem + report always failed, not just under load -- see + routes_llm.py's get_knowledge_base docstring for the full reasoning. + pubmed_root/index_root are absolute internal filesystem paths and + stay gated, same PUBLIC_FIELDS-style split as + routes_dashboard.py's /dashboard/summary.""" + + def test_200_when_no_token_with_aggregate_fields(self): + resp = client.get("/knowledge-base") + self.assertEqual(resp.status_code, 200) + body = resp.json() + self.assertIn("rag_status", body) + self.assertIn("total", body["abstracts"]) + self.assertIn("domains_with_abstracts", body["abstracts"]) + self.assertIn("domains_indexed", body["faiss_index"]) + self.assertIn("size_gb", body["faiss_index"]) + self.assertIn("domain_list", body["faiss_index"]) + + def test_paths_null_when_no_token(self): + body = client.get("/knowledge-base").json() + self.assertIsNone(body["pubmed_root"]) + self.assertIsNone(body["index_root"]) + + def test_paths_null_with_insufficient_permission(self): + body = client.get("/knowledge-base", headers=_cron_only_headers()).json() + self.assertIsNone(body["pubmed_root"]) + self.assertIsNone(body["index_root"]) + + def test_200_with_token_too(self): + self.assertEqual(client.get("/knowledge-base", headers=_admin_headers()).status_code, 200) + + class TestOverGateRegressionGuard(unittest.TestCase): """The 2026-09-02 revert must not spill past /report/status + /llms, and the 2026-09-03 decision must not spill past /report/data on top of those. Every route below stays platform.manage_infra-gated (401 w/o token) exactly as commit 8705cbf left it. Overlaps TestPlatformManageInfraAuth on purpose -- this one is the named, - human-readable list from the change's own scope statement.""" + human-readable list from the change's own scope statement. + + /knowledge-base is deliberately not in this list any more (2026-09-12 + decision, see TestKnowledgeBasePublicFields) -- it now returns 200 + without a token, just with pubmed_root/index_root nulled out, so it + no longer belongs in a "still 401s" regression guard.""" STILL_GATED = ( "/", "/coverage/status", - "/knowledge-base", "/storage", "/cron/jobs", "/cron/jobs/mysql-backup/log", diff --git a/backend/tests/test_routes_llm.py b/backend/tests/test_routes_llm.py index 13bcda1..d234121 100644 --- a/backend/tests/test_routes_llm.py +++ b/backend/tests/test_routes_llm.py @@ -22,14 +22,17 @@ from control_center.core.jwt_verify import JWT_SECRET from control_center.main import app -# Route exposure (2026-09-02 public/admin-split investigation): llm_router -# has NO blanket gate -- GET /llms is deliberately public, GET -# /knowledge-base carries its own per-route platform.manage_infra Depends. -# These tests exercise the routes' own logic, not authorization (see -# test_main.py's TestLlmsPublicAccess / TestPlatformManageInfraAuth for -# the access checks), so the client carries a fixed always-sufficient -# token by default -- harmless for /llms, required for /knowledge-base, -# same convention as test_routes_docker.py. +# Route exposure: llm_router has NO blanket gate -- GET /llms is +# deliberately public. GET /knowledge-base (2026-09-12 decision) always +# returns its aggregate fields, but only includes pubmed_root/index_root +# when the caller's token carries platform.manage_infra (checked via +# _has_permission, not a hard Depends -- see test_main.py's +# TestKnowledgeBasePublicFields for the access-split checks). These +# tests exercise the routes' own logic, not authorization, so the +# client carries a fixed always-sufficient token by default -- harmless +# for /llms, and means pubmed_root/index_root are populated below +# wherever the underlying dirs exist, same convention as +# test_routes_docker.py. _INFRA_TOKEN = jwt.encode({"sub": "1", "permissions": ["platform.manage_infra"]}, JWT_SECRET, algorithm="HS256") client = TestClient(app, headers={"Authorization": f"Bearer {_INFRA_TOKEN}"})