diff --git a/roboco/api/routes/docs.py b/roboco/api/routes/docs.py index 0d5e1971..dde7455c 100644 --- a/roboco/api/routes/docs.py +++ b/roboco/api/routes/docs.py @@ -7,7 +7,8 @@ Agents use these to write/read docs without path confusion. from uuid import UUID -from fastapi import APIRouter, HTTPException, Query, status +from fastapi import APIRouter, HTTPException, Query, Response, status +from fastapi.responses import JSONResponse from roboco.agents_config import get_agent_team from roboco.api.deps import CurrentAgentContext, DbSession @@ -20,9 +21,26 @@ from roboco.api.schemas.docs import ( ) from roboco.services.base import NotFoundError, UnauthorizedError, ValidationError from roboco.services.docs import WriteDocInput, get_docs_service +from roboco.services.gateway.kb_authz import docs_denial_envelope router = APIRouter() + +def _unauthorized_response(err: UnauthorizedError) -> JSONResponse: + """Render a docs-service denial as the gateway Envelope (HTTP 403). + + The RBAC decision is made in ``DocsService`` (it raises + ``UnauthorizedError``); this only renders that denial at the HTTP + boundary. The body is the Envelope wire-dict at top level so the agent + receives a non-null ``remediate`` instead of a bare ``detail`` string. + """ + envelope = docs_denial_envelope(err.action, err.reason) + return JSONResponse( + status_code=status.HTTP_403_FORBIDDEN, + content=envelope.as_dict(), + ) + + # Module-level Query defaults _list_task_id_query: UUID | None = Query(None, description="Filter by task ID") _read_path_query: str = Query( @@ -40,7 +58,7 @@ async def write_doc( data: WriteDocRequest, db: DbSession, agent: CurrentAgentContext, -) -> WriteDocResponse: +) -> WriteDocResponse | JSONResponse: """ Write a documentation file. @@ -87,10 +105,7 @@ async def write_doc( detail=e.message, ) from e except UnauthorizedError as e: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=e.message, - ) from e + return _unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -108,7 +123,7 @@ async def read_doc( db: DbSession, agent: CurrentAgentContext, path: str = _read_path_query, -) -> ReadDocResponse: +) -> ReadDocResponse | JSONResponse: """ Read a documentation file by path. @@ -133,10 +148,7 @@ async def read_doc( detail=e.message, ) from e except UnauthorizedError as e: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=e.message, - ) from e + return _unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -154,7 +166,7 @@ async def list_docs( db: DbSession, agent: CurrentAgentContext, task_id: UUID | None = _list_task_id_query, -) -> ListDocsResponse: +) -> ListDocsResponse | JSONResponse: """ List documentation files. @@ -190,10 +202,7 @@ async def list_docs( count=len(docs), ) except UnauthorizedError as e: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=e.message, - ) from e + return _unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -206,14 +215,17 @@ async def list_docs( # ============================================================================= -@router.delete("/delete", status_code=status.HTTP_204_NO_CONTENT) +@router.delete("/delete") async def delete_doc( db: DbSession, agent: CurrentAgentContext, path: str = _read_path_query, -) -> None: +) -> Response: """ Delete a documentation file. + + Returns 204 on success; a denied delete returns the gateway Envelope + (HTTP 403) with a non-null remediate, like the other docs endpoints. """ service = get_docs_service(db) @@ -223,16 +235,14 @@ async def delete_doc( path=path, ) await db.commit() + return Response(status_code=status.HTTP_204_NO_CONTENT) except ValidationError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=e.message, ) from e except UnauthorizedError as e: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=e.message, - ) from e + return _unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, diff --git a/roboco/api/routes/optimal.py b/roboco/api/routes/optimal.py index 38cc5c87..adb13fac 100644 --- a/roboco/api/routes/optimal.py +++ b/roboco/api/routes/optimal.py @@ -11,6 +11,7 @@ from uuid import uuid4 import structlog from fastapi import APIRouter, HTTPException, status +from fastapi.responses import JSONResponse from roboco.api.deps import ( CurrentAgentContext, @@ -69,6 +70,7 @@ from roboco.models.optimal import ( IndexErrorParams, ) from roboco.models.permissions import KBAction +from roboco.services.gateway.kb_authz import authorize_kb_action from roboco.services.optimal import ( IndexType, QueryContext, @@ -85,6 +87,28 @@ logger = structlog.get_logger() router = APIRouter() +def _kb_denial_response( + permissions: PermissionServiceDep, + agent: CurrentAgentContext, + action: str, +) -> JSONResponse | None: + """Gateway Envelope (HTTP 403) when the KB action is denied, else None. + + The authorization decision itself lives in the gateway + (``authorize_kb_action``); this only renders a denial verdict at the HTTP + boundary. The body is the Envelope wire-dict at top level — not nested + under ``detail`` — so the agent receives a non-null ``remediate`` it can + act on, matching the gateway Envelope contract. + """ + denial = authorize_kb_action(permissions, agent, action) + if denial is None: + return None + return JSONResponse( + status_code=status.HTTP_403_FORBIDDEN, + content=denial.as_dict(), + ) + + # ============================================================================= # INDEXING ENDPOINTS # ============================================================================= @@ -99,7 +123,7 @@ async def index_code( request: IndexCodeRequest, agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> IndexResponse: +) -> IndexResponse | JSONResponse: """ Index code files/directories. @@ -108,11 +132,9 @@ async def index_code( - Directories - Glob patterns (e.g., "src/**/*.py") """ - if not permissions.can_perform_kb_action(agent, KBAction.INDEX_CODE): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to index code", - ) + denied = _kb_denial_response(permissions, agent, KBAction.INDEX_CODE) + if denied is not None: + return denied service = await get_optimal_service() count = await service.index_code( @@ -135,7 +157,7 @@ async def index_documentation( request: IndexDocsRequest, agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> IndexResponse: +) -> IndexResponse | JSONResponse: """ Index documentation files. @@ -144,11 +166,9 @@ async def index_documentation( - URLs (single page or crawl with /**) - Glob patterns """ - if not permissions.can_perform_kb_action(agent, KBAction.INDEX_DOCS): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to index documentation", - ) + denied = _kb_denial_response(permissions, agent, KBAction.INDEX_DOCS) + if denied is not None: + return denied service = await get_optimal_service() count = await service.index_documentation( @@ -418,13 +438,11 @@ async def get_context( async def get_stats( agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> IndexStatsResponse: +) -> IndexStatsResponse | JSONResponse: """Get statistics about all indexes.""" - if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to view index statistics", - ) + denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + if denied is not None: + return denied service = await get_optimal_service() stats = await service.get_all_index_stats() @@ -435,11 +453,11 @@ async def get_stats( ) -@router.get("/stats/staleness") +@router.get("/stats/staleness", response_model=None) async def check_staleness( agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> dict[str, Any]: +) -> dict[str, Any] | JSONResponse: """ Check if indexes are stale (source files modified after last indexing). @@ -448,11 +466,9 @@ async def check_staleness( Declared BEFORE `/stats/{index_type}` so FastAPI matches the literal `staleness` segment instead of treating it as an `index_type` param. """ - if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to view index staleness", - ) + denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + if denied is not None: + return denied service = await get_optimal_service() return await service.check_index_staleness() @@ -463,13 +479,11 @@ async def get_single_index_stats( index_type: str, agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> SingleIndexStatsResponse: +) -> SingleIndexStatsResponse | JSONResponse: """Get statistics for a specific index type.""" - if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to view index statistics", - ) + denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + if denied is not None: + return denied # Validate index type try: @@ -511,17 +525,15 @@ async def clear_index( index_type: str, agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> ClearIndexResponse: +) -> ClearIndexResponse | JSONResponse: """ Clear a specific index. Warning: This permanently deletes all documents in the index. """ - if not permissions.can_perform_kb_action(agent, KBAction.CLEAR_INDEX): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to clear indexes", - ) + denied = _kb_denial_response(permissions, agent, KBAction.CLEAR_INDEX) + if denied is not None: + return denied try: idx_type = IndexType(index_type) @@ -543,13 +555,11 @@ async def list_documents( agent: CurrentAgentContext, permissions: PermissionServiceDep, pagination: PaginationDep, -) -> DocumentListResponse: +) -> DocumentListResponse | JSONResponse: """List documents in a specific index (paginated).""" - if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to view index documents", - ) + denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + if denied is not None: + return denied try: idx_type = IndexType(index_type) except ValueError as e: @@ -589,17 +599,15 @@ async def refresh_index( request: RefreshRequest, agent: CurrentAgentContext, permissions: PermissionServiceDep, -) -> RefreshIndexResponse: +) -> RefreshIndexResponse | JSONResponse: """ Refresh an index with updated sources. Re-indexes the specified sources to pick up changes. """ - if not permissions.can_perform_kb_action(agent, KBAction.REFRESH_INDEX): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to refresh indexes", - ) + denied = _kb_denial_response(permissions, agent, KBAction.REFRESH_INDEX) + if denied is not None: + return denied try: idx_type = IndexType(request.index_type) @@ -627,13 +635,13 @@ async def refresh_index( ) -@router.post("/kb/reindex") +@router.post("/kb/reindex", response_model=None) async def reindex_all( agent: CurrentAgentContext, permissions: PermissionServiceDep, force: bool = False, timeout_seconds: int = 300, # 5 minute default -) -> dict[str, Any]: +) -> dict[str, Any] | JSONResponse: """ Trigger re-indexing of code and documentation. @@ -650,11 +658,9 @@ async def reindex_all( """ import asyncio - if not permissions.can_perform_kb_action(agent, KBAction.INDEX_CODE): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not authorized to trigger reindexing", - ) + denied = _kb_denial_response(permissions, agent, KBAction.INDEX_CODE) + if denied is not None: + return denied try: async with asyncio.timeout(timeout_seconds): diff --git a/roboco/services/gateway/kb_authz.py b/roboco/services/gateway/kb_authz.py new file mode 100644 index 00000000..bf486b92 --- /dev/null +++ b/roboco/services/gateway/kb_authz.py @@ -0,0 +1,90 @@ +"""Authorization decisions for docs/optimal, expressed as gateway Envelopes. + +The HTTP routes for documentation (`api.routes.docs`) and the knowledge +base (`api.routes.optimal`) used to embed RBAC checks inline and raise raw +``HTTPException(403)`` with no recovery hint. That mixed an authorization +decision into the HTTP layer and broke the gateway Envelope contract: +agents received ``remediate=null`` and had nothing actionable to do. + +This module owns those decisions. It turns a denial into an +``Envelope.not_authorized(...)`` carrying a non-null ``remediate`` that +names the roles allowed to perform the action, so the agent knows how to +recover (escalate to a role that holds the permission). The routes stay +thin: they ask here for a verdict and translate it to the wire. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from roboco.models.permissions import KB_PERMISSIONS +from roboco.services.gateway.envelope import Envelope + +if TYPE_CHECKING: + from roboco.models.permissions import AgentContext + from roboco.services.permissions import PermissionService + + +def _roles_allowed_for(action: str) -> list[str]: + """Roles whose KB permission set includes ``action`` (sorted, stable).""" + return sorted( + role.value for role, actions in KB_PERMISSIONS.items() if action in actions + ) + + +def _remediate_for_action(action: str) -> str: + """Recovery hint naming who can perform ``action``.""" + allowed = _roles_allowed_for(action) + if allowed: + return ( + f"role not permitted for '{action}' — ask one of these roles to " + f"run it: {', '.join(allowed)}" + ) + return f"'{action}' is not granted to any role; escalate to the CEO" + + +def authorize_kb_action( + permissions: PermissionService, + agent: AgentContext, + action: str, +) -> Envelope | None: + """Verdict on a knowledge-base action. + + Returns ``None`` when allowed. On denial returns an + ``Envelope.not_authorized`` whose ``remediate`` tells the agent which + roles may perform the action. + """ + if permissions.can_perform_kb_action(agent, action): + return None + return Envelope.not_authorized( + message=f"role '{agent.role.value}' not authorized to {action}", + remediate=_remediate_for_action(action), + ) + + +_DOCS_WRITE_ACTIONS = frozenset({"write_doc", "delete_doc"}) + + +def docs_denial_envelope(action: str, reason: str | None) -> Envelope: + """Wrap a docs-service authorization denial as a gateway Envelope. + + The docs RBAC decision already lives in ``DocsService`` (it raises + ``UnauthorizedError`` with an ``action`` and human ``reason``). This + keeps the remediate-hint ownership in the gateway: the route hands the + denial here and gets back the Envelope-shaped body with a non-null + ``remediate``. + """ + if action in _DOCS_WRITE_ACTIONS: + remediate = ( + f"role not permitted to {action} — only documenters and cell PMs " + "may write or delete docs; ask a documenter to perform it" + ) + else: + remediate = ( + f"role not permitted to {action} — ask a documenter or cell PM, " + "or use roboco_kb_search to find the document instead" + ) + return Envelope.not_authorized( + message=reason or f"not authorized: {action}", + remediate=remediate, + ) diff --git a/tests/integration/test_docs_routes.py b/tests/integration/test_docs_routes.py index 64ba507c..dff94bdc 100644 --- a/tests/integration/test_docs_routes.py +++ b/tests/integration/test_docs_routes.py @@ -110,6 +110,39 @@ async def test_write_doc_unauthorized(docs_client: AsyncClient) -> None: assert response.status_code == HTTPStatus.FORBIDDEN +@pytest.mark.asyncio +async def test_write_doc_unauthorized_envelope_remediate( + docs_client: AsyncClient, +) -> None: + """A denied write returns an Envelope-shaped body with a real remediate. + + The docs RBAC decision lives in the service (UnauthorizedError); the + route must surface it as the gateway Envelope contract — error + + non-null remediate — not a bare {"detail": ...} HTTPException body. + """ + with patch("roboco.api.routes.docs.get_docs_service") as mock_get: + mock_service = AsyncMock() + mock_service.write_doc = AsyncMock(side_effect=UnauthorizedError("write_doc")) + mock_get.return_value = mock_service + response = await docs_client.post( + "/api/docs/write", + json={ + "task_id": str(uuid4()), + "filename": "test.md", + "doc_type": "api", + "title": "Test", + "content": "Some content", + }, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.FORBIDDEN + body = response.json() + assert body["error"] == "not_authorized" + assert body["message"] + assert body["remediate"] is not None + assert body["remediate"].strip() + + @pytest.mark.asyncio async def test_write_doc_not_found(docs_client: AsyncClient) -> None: """Service raises NotFoundError → 404.""" diff --git a/tests/integration/test_optimal_routes.py b/tests/integration/test_optimal_routes.py index 41c3dc8e..337d81b7 100644 --- a/tests/integration/test_optimal_routes.py +++ b/tests/integration/test_optimal_routes.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from http import HTTPStatus from types import SimpleNamespace from typing import TYPE_CHECKING @@ -10,7 +11,8 @@ from uuid import uuid4 import pytest import pytest_asyncio -from fastapi import FastAPI, HTTPException +from fastapi import FastAPI +from fastapi.responses import JSONResponse from httpx import ASGITransport, AsyncClient from roboco.api.deps import get_agent_context from roboco.api.routes.optimal import check_staleness @@ -84,6 +86,29 @@ async def test_index_code_forbidden(dev_optimal_client: AsyncClient) -> None: assert response.status_code == HTTPStatus.FORBIDDEN +@pytest.mark.asyncio +async def test_index_code_forbidden_envelope_remediate( + dev_optimal_client: AsyncClient, +) -> None: + """A denied KB call returns an Envelope-shaped body with a real remediate. + + Agents go through the gateway Envelope contract: a denial must carry + error="not_authorized" plus a non-null remediate hint so the agent can + recover, not a bare {"detail": ...} HTTPException body. + """ + response = await dev_optimal_client.post( + "/api/optimal/kb/index/code", + json={"sources": ["a.py"]}, + headers=_DEV_HDR, + ) + assert response.status_code == HTTPStatus.FORBIDDEN + body = response.json() + assert body["error"] == "not_authorized" + assert body["message"] + assert body["remediate"] is not None + assert body["remediate"].strip() + + @pytest.mark.asyncio async def test_index_code_success(optimal_client: AsyncClient) -> None: with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get: @@ -369,15 +394,24 @@ async def test_check_staleness_helper_authorized() -> None: @pytest.mark.asyncio async def test_check_staleness_helper_unauthorized() -> None: - """Lines 479-482: kb VIEW_STATS denied → HTTPException 403.""" + """KB VIEW_STATS denied → gateway Envelope JSONResponse (HTTP 403). + + The authorization decision now lives in the gateway; the route returns + the Envelope wire-dict (with a non-null remediate) at top level instead + of raising a bare HTTPException. + """ agent = AgentContext(agent_id=uuid4(), role=AgentRole.DEVELOPER, team=Team.BACKEND) fake_perm = MagicMock() fake_perm.can_perform_kb_action.return_value = False - with pytest.raises(HTTPException) as exc: - await check_staleness(agent=agent, permissions=fake_perm) - assert exc.value.status_code == HTTPStatus.FORBIDDEN + result = await check_staleness(agent=agent, permissions=fake_perm) + assert isinstance(result, JSONResponse) + assert result.status_code == HTTPStatus.FORBIDDEN + body = json.loads(bytes(result.body)) + assert body["error"] == "not_authorized" + assert body["remediate"] is not None + assert body["remediate"].strip() @pytest.mark.asyncio