mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[F022][F023][F024][F025][F026] api: scrub secrets from 422 log, gate a2a/dashboard/orchestrator routes, SSE session-per-query
- middleware: redact known credential fields (git_token/api_key/token/...) from the 422 request-validation log line; response body unchanged - a2a: require_any_authenticated_agent on /message/send + /message/stream; subscribe_to_task opens a short-lived session per poll instead of holding one asyncpg connection for the full SSE lifetime (pool exhaustion) + auth - dashboard: gate auditor flag/report mutating routes to Auditor or CEO - orchestrator: router-level CEO gate on all control routes (spawn/stop/...) TDD; ruff/mypy clean; 449 unit/api tests green; no type:ignore/noqa.
This commit is contained in:
@@ -363,6 +363,47 @@ def _uuid_field_remediation(errors: Sequence[Any]) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
# Credential-bearing request fields. A 422 on a secret-bearing request
|
||||
# would otherwise dump the plaintext GitHub PAT / provider API key / bearer
|
||||
# token into structlog output before the route ever encrypts it. The per-field
|
||||
# ``errors`` carry only field names and types (never values), so they stay
|
||||
# logged unchanged. Match by exact key name so a renamed secret field is
|
||||
# caught by the next audit pass rather than silently leaking.
|
||||
_SECRET_FIELD_NAMES: frozenset[str] = frozenset(
|
||||
{
|
||||
"git_token",
|
||||
"api_key",
|
||||
"auth_token",
|
||||
"token",
|
||||
"password",
|
||||
"secret",
|
||||
"client_secret",
|
||||
"access_token",
|
||||
"refresh_token",
|
||||
}
|
||||
)
|
||||
|
||||
_REDACTED = "***REDACTED***"
|
||||
|
||||
|
||||
def _scrub_secrets(value: Any) -> Any:
|
||||
"""Return a deep copy of ``value`` with known secret fields redacted.
|
||||
|
||||
Recurses into nested dicts and lists so a secret inside ``nested: {...}``
|
||||
or a list element is also scrubbed. Non-secret fields are preserved so ops
|
||||
can still see which field broke. The original ``rve.body`` is not mutated
|
||||
(the 422 response body echoes the client's own submission unscrubbed).
|
||||
"""
|
||||
if isinstance(value, dict):
|
||||
return {
|
||||
k: (_REDACTED if k in _SECRET_FIELD_NAMES else _scrub_secrets(v))
|
||||
for k, v in value.items()
|
||||
}
|
||||
if isinstance(value, list):
|
||||
return [_scrub_secrets(v) for v in value]
|
||||
return value
|
||||
|
||||
|
||||
async def request_validation_handler(request: Request, exc: Exception) -> JSONResponse:
|
||||
"""Log the rejected body before returning the standard 422 response.
|
||||
|
||||
@@ -373,15 +414,21 @@ async def request_validation_handler(request: Request, exc: Exception) -> JSONRe
|
||||
|
||||
When the failure is a truncated ``task_id`` (the recurring agent mistake),
|
||||
add a ``remediate`` hint so the agent knows to retry with the full UUID.
|
||||
|
||||
F022: the log line scrubs known credential-bearing fields
|
||||
(``git_token`` / ``api_key`` / ``auth_token`` / …) from the body before
|
||||
logging. The 422 *response* body is unchanged — the client sent those
|
||||
values, only the server's own log is redacted.
|
||||
"""
|
||||
rve = cast("RequestValidationError", exc)
|
||||
body = rve.body if isinstance(rve.body, str | bytes | dict | list) else None
|
||||
errors = rve.errors()
|
||||
body_for_log = _scrub_secrets(body) if isinstance(body, dict | list) else body
|
||||
logger.warning(
|
||||
"Request validation failed",
|
||||
path=request.url.path,
|
||||
method=request.method,
|
||||
body=body,
|
||||
body=body_for_log,
|
||||
errors=errors,
|
||||
)
|
||||
content: dict[str, Any] = {"detail": errors, "body": body}
|
||||
|
||||
+31
-11
@@ -24,6 +24,7 @@ from fastapi.responses import JSONResponse
|
||||
from sse_starlette import EventSourceResponse
|
||||
|
||||
from roboco.api.deps import CurrentAgentSlug, DbSession
|
||||
from roboco.api.routes.v1._role_dep import require_any_authenticated_agent
|
||||
from roboco.api.schemas.a2a_chat import (
|
||||
ConversationCloseRequest,
|
||||
ConversationCreateRequest,
|
||||
@@ -39,6 +40,7 @@ from roboco.api.schemas.a2a_chat import (
|
||||
PairListResponse,
|
||||
PairResponse,
|
||||
)
|
||||
from roboco.db.base import get_session_factory
|
||||
from roboco.enforcement import A2AAccessDeniedError
|
||||
from roboco.models.a2a import (
|
||||
A2AConversationStatus,
|
||||
@@ -107,7 +109,10 @@ async def get_agent_card(
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@router.post("/message/send")
|
||||
@router.post(
|
||||
"/message/send",
|
||||
dependencies=[require_any_authenticated_agent],
|
||||
)
|
||||
async def send_message(
|
||||
request: SendMessageRequest,
|
||||
db: DbSession,
|
||||
@@ -187,7 +192,10 @@ async def send_message(
|
||||
return {"status": "success", "a2a_request": result}
|
||||
|
||||
|
||||
@router.post("/message/stream")
|
||||
@router.post(
|
||||
"/message/stream",
|
||||
dependencies=[require_any_authenticated_agent],
|
||||
)
|
||||
async def send_message_stream(
|
||||
request: Request,
|
||||
body: SendMessageRequest,
|
||||
@@ -275,22 +283,31 @@ async def send_message_stream(
|
||||
)
|
||||
|
||||
|
||||
@router.get("/tasks/{task_id}/subscribe")
|
||||
@router.get(
|
||||
"/tasks/{task_id}/subscribe",
|
||||
dependencies=[require_any_authenticated_agent],
|
||||
)
|
||||
async def subscribe_to_task(
|
||||
request: Request,
|
||||
task_id: str,
|
||||
db: DbSession,
|
||||
) -> EventSourceResponse:
|
||||
"""
|
||||
Subscribe to task updates via SSE.
|
||||
|
||||
Opens a persistent connection that streams task state changes
|
||||
until the task reaches a terminal state or client disconnects.
|
||||
"""
|
||||
service = A2AService(db)
|
||||
|
||||
# Validate task exists
|
||||
a2a_task = await service.get_task(task_id)
|
||||
F024: each poll opens a SHORT-LIVED session via ``get_session_factory``
|
||||
and closes it before the next ``asyncio.sleep`` — never holding one
|
||||
asyncpg connection across the full SSE lifetime (up to 1 hour / 720
|
||||
polls), which previously exhausted the pool one connection per connected
|
||||
client. The route takes no ``db: DbSession`` for the same reason.
|
||||
"""
|
||||
session_factory = get_session_factory()
|
||||
|
||||
# Validate task exists with a short-lived session (released immediately).
|
||||
async with session_factory() as session:
|
||||
a2a_task = await A2AService(session).get_task(task_id)
|
||||
if a2a_task is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
@@ -298,7 +315,7 @@ async def subscribe_to_task(
|
||||
)
|
||||
|
||||
async def generate_updates() -> AsyncGenerator[dict[str, Any]]:
|
||||
"""Stream task updates."""
|
||||
"""Stream task updates — one short-lived session per poll."""
|
||||
poll_count = 0
|
||||
max_polls = 720 # 1 hour at 5s interval
|
||||
last_state = None
|
||||
@@ -307,8 +324,11 @@ async def subscribe_to_task(
|
||||
if await request.is_disconnected():
|
||||
break
|
||||
|
||||
# Refresh task state from DB
|
||||
task = await service.get_task(task_id)
|
||||
# F024: refresh task state from a per-poll session that is
|
||||
# released before the sleep below — never held across the poll
|
||||
# interval, so the asyncpg pool is free between queries.
|
||||
async with session_factory() as session:
|
||||
task = await A2AService(session).get_task(task_id)
|
||||
if task is None:
|
||||
break
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ from uuid import UUID
|
||||
|
||||
from fastapi import APIRouter, HTTPException, Query, status
|
||||
|
||||
from roboco.api.deps import DbSession
|
||||
from roboco.api.deps import CurrentAgentContext, DbSession
|
||||
from roboco.api.schemas.dashboard import (
|
||||
AuditorDashboard,
|
||||
AuditorFlag,
|
||||
@@ -23,6 +23,7 @@ from roboco.api.schemas.dashboard import (
|
||||
TeamHealth,
|
||||
UsageSummary,
|
||||
)
|
||||
from roboco.models import AgentRole
|
||||
from roboco.models.base import Team
|
||||
from roboco.models.dashboard import CreateFlagParams
|
||||
from roboco.services.dashboard import get_dashboard_service
|
||||
@@ -32,6 +33,22 @@ from roboco.services.usage import get_usage_service
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
# The auditor flag/report mutating routes are gated to the Auditor and the
|
||||
# CEO. The Auditor is the silent-observer role whose flags/reports feed the
|
||||
# CEO; the CEO overrides. Mirrors ``_require_curator`` in playbooks.py and
|
||||
# ``_require_ceo`` in release.py. Read-only auditor views (``GET
|
||||
# /auditor/flags``, ``GET /auditor/reports``, ``GET /auditor``) stay open —
|
||||
# the dashboard is observable by any authenticated operator.
|
||||
_AUDITOR_OR_CEO_ROLES = frozenset({AgentRole.AUDITOR, AgentRole.CEO})
|
||||
|
||||
|
||||
def _require_auditor_or_ceo(agent: CurrentAgentContext) -> None:
|
||||
if agent.role not in _AUDITOR_OR_CEO_ROLES:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="Only the Auditor or CEO may mutate auditor flags or reports",
|
||||
)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# AUDITOR DASHBOARD
|
||||
@@ -154,8 +171,10 @@ async def get_auditor_flags(
|
||||
async def create_auditor_flag(
|
||||
data: CreateFlagRequest,
|
||||
db: DbSession,
|
||||
agent: CurrentAgentContext,
|
||||
) -> AuditorFlag:
|
||||
"""Create a new auditor flag."""
|
||||
_require_auditor_or_ceo(agent)
|
||||
service = get_dashboard_service(db)
|
||||
params = CreateFlagParams(
|
||||
severity=data.severity.value,
|
||||
@@ -184,9 +203,11 @@ async def create_auditor_flag(
|
||||
async def resolve_auditor_flag(
|
||||
flag_id: UUID,
|
||||
db: DbSession,
|
||||
agent: CurrentAgentContext,
|
||||
notes: str | None = None,
|
||||
) -> dict[str, str]:
|
||||
"""Resolve an auditor flag."""
|
||||
_require_auditor_or_ceo(agent)
|
||||
service = get_dashboard_service(db)
|
||||
if not service.resolve_flag(flag_id, notes):
|
||||
raise HTTPException(
|
||||
@@ -226,8 +247,10 @@ async def get_auditor_reports(
|
||||
async def create_auditor_report(
|
||||
data: CreateReportRequest,
|
||||
db: DbSession,
|
||||
agent: CurrentAgentContext,
|
||||
) -> AuditorReport:
|
||||
"""Create a new auditor report."""
|
||||
_require_auditor_or_ceo(agent)
|
||||
service = get_dashboard_service(db)
|
||||
report = service.create_report(
|
||||
report_type=data.report_type,
|
||||
@@ -250,8 +273,10 @@ async def create_auditor_report(
|
||||
async def send_auditor_report(
|
||||
report_id: UUID,
|
||||
db: DbSession,
|
||||
agent: CurrentAgentContext,
|
||||
) -> dict[str, str]:
|
||||
"""Mark a report as sent to CEO."""
|
||||
_require_auditor_or_ceo(agent)
|
||||
service = get_dashboard_service(db)
|
||||
if not service.send_report(report_id):
|
||||
raise HTTPException(
|
||||
|
||||
@@ -5,10 +5,11 @@ API endpoints for managing the Agent Orchestrator.
|
||||
"""
|
||||
|
||||
from datetime import datetime
|
||||
from typing import Annotated
|
||||
|
||||
from fastapi import APIRouter, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, Header, HTTPException, status
|
||||
|
||||
from roboco.api.deps import get_orchestrator, set_orchestrator
|
||||
from roboco.api.deps import _check_agent_auth_token, get_orchestrator, set_orchestrator
|
||||
from roboco.api.schemas.orchestrator import (
|
||||
AgentStatusResponse,
|
||||
OrchestratorStatusResponse,
|
||||
@@ -16,8 +17,42 @@ from roboco.api.schemas.orchestrator import (
|
||||
SpawnAgentRequest,
|
||||
WaitingAgentResponse,
|
||||
)
|
||||
from roboco.foundation.identity import Role
|
||||
|
||||
router = APIRouter()
|
||||
# Orchestrator control routes (spawn / stop / resolve-wait / mark-waiting,
|
||||
# plus the read-only status views) are operator/CEO control surfaces — any
|
||||
# client that could reach the API could previously spawn, stop, or
|
||||
# manipulate any agent's runtime state. The guard mirrors the panel-token
|
||||
# approach used by the WebSocket streams (DB-free): it binds the presented
|
||||
# ``X-Agent-ID`` to a verified HMAC token and asserts the role is CEO. In
|
||||
# dev (header-trust) mode a missing token is a no-op (the panel/operator
|
||||
# flow keeps working), but a presented-but-forged token is still rejected —
|
||||
# the same contract as the v1 flow role guards and the do router. CEO is the
|
||||
# sole operator role; agents (developers/QA/PMs) drive the orchestrator via
|
||||
# MCP verbs, not these HTTP routes, so a developer token is correctly 403'd
|
||||
# here.
|
||||
_CEO_ROLE = Role.CEO.value
|
||||
|
||||
|
||||
def _require_ceo(
|
||||
x_agent_id: Annotated[str, Header(alias="X-Agent-ID")],
|
||||
x_agent_role: Annotated[str, Header(alias="X-Agent-Role")],
|
||||
x_agent_team: Annotated[str | None, Header(alias="X-Agent-Team")] = None,
|
||||
x_agent_token: Annotated[str | None, Header(alias="X-Agent-Token")] = None,
|
||||
) -> None:
|
||||
# Bind the role header to a verified token BEFORE trusting it (same
|
||||
# defense-in-depth contract as the v1 flow role guards in _role_dep.py).
|
||||
_check_agent_auth_token(x_agent_id, x_agent_role, x_agent_team, x_agent_token)
|
||||
# ``Role`` is a StrEnum so the lowercase header string compares equal to
|
||||
# its matching member.
|
||||
if x_agent_role.lower() != _CEO_ROLE:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="Only the CEO/operator may control the orchestrator",
|
||||
)
|
||||
|
||||
|
||||
router = APIRouter(dependencies=[Depends(_require_ceo)])
|
||||
|
||||
# Re-export set_orchestrator for bootstrap code
|
||||
__all__ = ["router", "set_orchestrator"]
|
||||
|
||||
Reference in New Issue
Block a user