fix(gateway): docs/optimal RBAC denials return Envelope with remediate

The docs and knowledge-base HTTP routes enforced authorization inline and
raised raw HTTPException(403) with no recovery hint, so agents received
remediate=null and an un-actionable error, and the optimal route held the
RBAC decision itself (a layer-separation violation).

Move the knowledge-base authorization decision into a gateway module
(services/gateway/kb_authz) that returns an Envelope.not_authorized with a
non-null remediate naming the roles allowed to perform the action. The docs
RBAC already lives in DocsService; render its UnauthorizedError through the
same gateway helper. Both route groups now return the Envelope wire-dict at
top level (HTTP 403) instead of a bare detail string, keeping the routes
thin (HTTP translation only). Permitted callers are unaffected.
This commit is contained in:
Renn F
2026-06-03 19:41:10 +02:00
parent 16c425799c
commit 45836b4d6e
5 changed files with 256 additions and 83 deletions
+32 -22
View File
@@ -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,
+62 -56
View File
@@ -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):
+90
View File
@@ -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,
)
+33
View File
@@ -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."""
+39 -5
View File
@@ -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