diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 3def7b0..d58e9a0 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -2314,3 +2314,18 @@ evidence yet." focused Unraid contracts **19 passed** and the full suite **208 passed, 1 warning**. - Exact next action: harden MCP per-client authorization and evidence completeness, then replace inferred n8n status with explicit heartbeat/execution telemetry. + +## MCP trust boundary and trace completeness (2026-08-10) + +- The MCP API now validates the ITWorx Hub delegated client-id shape in addition to the + shared service secret, supports an explicit tenant assertion and rejects cross-tenant + calls. Readiness and project-bound client identities remain compatible with the Hub's + documented connector contract; arbitrary/spoofed labels no longer enter the audit log. +- Every successful tool response returns `X-Correlation-Id`, `X-Tenant-Id` and + `Cache-Control: no-store`. Knowledge calls additionally expose available versus + returned source counts and persist tenant, locale and source coverage in their audit + metadata. `contracts/mcp-tools.json` is versioned to 1.2.0 with this trust contract. +- Evidence: ruff and mypy passed; focused MCP tests **14 passed** and full Unraid suite + **210 passed, 1 warning**. +- Exact next action: add authenticated n8n workflow heartbeats and execution results, + surface stale/healthy workflow state, then update generated contracts and E2E coverage. diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 5f76470..2fbf7a9 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -1,6 +1,9 @@ from __future__ import annotations +import hmac +import re from collections.abc import Generator +from dataclasses import dataclass from fastapi import Depends, Header, HTTPException, Request, status from sqlalchemy.orm import Session @@ -60,12 +63,30 @@ def require_operations_manager( return user +@dataclass(frozen=True) +class McpClientContext: + client_id: str + tenant: str + + +_MCP_CLIENT_ID = re.compile( + r"^itworx-mcp-hub:(?:readiness|mobilityops:[A-Za-z0-9][A-Za-z0-9._:-]{0,127})$" +) + + def require_mcp_service_token( x_service_token: str = Header(..., alias="X-Service-Token"), - x_client_id: str = Header(default="unknown-mcp-client", alias="X-Client-Id"), -) -> str: - if x_service_token != settings.mcp_hub_service_token: + x_client_id: str = Header(..., alias="X-Client-Id", min_length=1, max_length=180), + x_tenant_id: str | None = Header(default=None, alias="X-Tenant-Id", max_length=120), +) -> McpClientContext: + if not hmac.compare_digest(x_service_token, settings.mcp_hub_service_token): raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid service token" ) - return x_client_id + if not _MCP_CLIENT_ID.fullmatch(x_client_id): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, detail="Untrusted MCP client identity" + ) + if x_tenant_id is not None and x_tenant_id != settings.ragcore_tenant: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Tenant mismatch") + return McpClientContext(client_id=x_client_id, tenant=settings.ragcore_tenant) diff --git a/backend/app/api/routers/mcp_integrations.py b/backend/app/api/routers/mcp_integrations.py index c4128e5..8713c1a 100644 --- a/backend/app/api/routers/mcp_integrations.py +++ b/backend/app/api/routers/mcp_integrations.py @@ -3,11 +3,11 @@ from __future__ import annotations import uuid from datetime import date -from fastapi import APIRouter, Depends, Header, Query +from fastapi import APIRouter, Depends, Header, Query, Response from sqlalchemy import select from sqlalchemy.orm import Session -from app.api.deps import get_db, require_mcp_service_token +from app.api.deps import McpClientContext, get_db, require_mcp_service_token from app.core.config import get_settings from app.core.errors import AppError from app.models.booking import Booking @@ -42,7 +42,8 @@ def get_correlation_id( def _audit_service_request( - db: Session, *, client_id: str, tool: str, status_label: str, correlation_id: str + db: Session, *, client_id: str, tool: str, status_label: str, correlation_id: str, + metadata: dict[str, object] | None = None, ) -> None: record_audit_event( db, @@ -51,43 +52,53 @@ def _audit_service_request( action="mcp_tool_request", entity_type="mcp_tool", correlation_id=uuid.UUID(correlation_id), - metadata={"tool": tool, "status": status_label}, + metadata={"tool": tool, "status": status_label, **(metadata or {})}, ) db.commit() +def _set_trace_headers(response: Response, correlation_id: str, tenant: str) -> None: + response.headers["X-Correlation-Id"] = correlation_id + response.headers["X-Tenant-Id"] = tenant + response.headers["Cache-Control"] = "no-store" + + @router.get("/operations-summary", response_model=OperationsSummaryOut) def operations_summary( + response: Response, db: Session = Depends(get_db), - client_id: str = Depends(require_mcp_service_token), + client: McpClientContext = Depends(require_mcp_service_token), correlation_id: str = Depends(get_correlation_id), ) -> OperationsSummaryOut: metrics = compute_metrics(db) + _set_trace_headers(response, correlation_id, client.tenant) _audit_service_request( db, - client_id=client_id, + client_id=client.client_id, tool="fleet_ops_get_operations_summary", status_label="ok", correlation_id=correlation_id, ) - return OperationsSummaryOut(tenant=settings.ragcore_tenant, metrics=metrics) + return OperationsSummaryOut(tenant=client.tenant, metrics=metrics) @router.get("/attention-vehicles", response_model=list[AttentionVehicleOut]) def attention_vehicles( + response: Response, minimum_severity: str = Query(default="medium", pattern="^(low|medium|high)$"), date_filter: date | None = Query(default=None, alias="date"), limit: int = Query(default=20, ge=1, le=50), db: Session = Depends(get_db), - client_id: str = Depends(require_mcp_service_token), + client: McpClientContext = Depends(require_mcp_service_token), correlation_id: str = Depends(get_correlation_id), ) -> list[AttentionVehicleOut]: results = list_attention_vehicles( db, minimum_severity=minimum_severity, on_or_before=date_filter, limit=limit ) + _set_trace_headers(response, correlation_id, client.tenant) _audit_service_request( db, - client_id=client_id, + client_id=client.client_id, tool="fleet_ops_list_attention_vehicles", status_label="ok", correlation_id=correlation_id, @@ -98,15 +109,17 @@ def attention_vehicles( @router.get("/vehicles/{vehicle_ref}", response_model=McpVehicleDetailOut) def vehicle_details( vehicle_ref: str, + response: Response, db: Session = Depends(get_db), - client_id: str = Depends(require_mcp_service_token), + client: McpClientContext = Depends(require_mcp_service_token), correlation_id: str = Depends(get_correlation_id), ) -> McpVehicleDetailOut: + _set_trace_headers(response, correlation_id, client.tenant) vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref)) if vehicle is None: _audit_service_request( db, - client_id=client_id, + client_id=client.client_id, tool="fleet_ops_get_vehicle_details", status_label="not_found", correlation_id=correlation_id, @@ -128,7 +141,7 @@ def vehicle_details( _audit_service_request( db, - client_id=client_id, + client_id=client.client_id, tool="fleet_ops_get_vehicle_details", status_label="ok", correlation_id=correlation_id, @@ -150,18 +163,29 @@ def vehicle_details( @router.post("/search-knowledge", response_model=GroundedAnswer) def search_knowledge( body: McpKnowledgeSearchRequest, + response: Response, db: Session = Depends(get_db), - client_id: str = Depends(require_mcp_service_token), + client: McpClientContext = Depends(require_mcp_service_token), correlation_id: str = Depends(get_correlation_id), ) -> GroundedAnswer: provider = get_knowledge_provider() answer = provider.ask(body.question, correlation_id, language=body.locale) + source_count_available = len(answer.sources) answer.sources = answer.sources[: body.max_sources] + _set_trace_headers(response, correlation_id, client.tenant) + response.headers["X-Sources-Available"] = str(source_count_available) + response.headers["X-Sources-Returned"] = str(len(answer.sources)) _audit_service_request( db, - client_id=client_id, + client_id=client.client_id, tool="fleet_ops_search_knowledge", status_label=answer.evidence_state, correlation_id=correlation_id, + metadata={ + "tenant": client.tenant, + "locale": body.locale, + "sources_available": source_count_available, + "sources_returned": len(answer.sources), + }, ) return answer diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index bb5fe02..954b428 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -75,7 +75,7 @@ def test_demo_reset_preserves_integration_telemetry(ops_client, client): "/api/v1/integrations/mcp/operations-summary", headers={ "X-Service-Token": settings.mcp_hub_service_token, - "X-Client-Id": "reset-probe", + "X-Client-Id": "itworx-mcp-hub:mobilityops:reset-probe", }, ) assert probe.status_code == 200 @@ -83,4 +83,4 @@ def test_demo_reset_preserves_integration_telemetry(ops_client, client): assert client.post("/api/v1/demo/login", json={"role": "operations_manager"}).status_code == 200 events = client.get("/api/v1/audit", params={"action": "mcp_tool_request"}).json() - assert any(event["actor_label"] == "reset-probe" for event in events) + assert any(event["actor_label"].endswith(":reset-probe") for event in events) diff --git a/backend/tests/test_mcp_integrations.py b/backend/tests/test_mcp_integrations.py index 70d1876..4b19a8a 100644 --- a/backend/tests/test_mcp_integrations.py +++ b/backend/tests/test_mcp_integrations.py @@ -5,7 +5,7 @@ def _headers(token: str | None = None, client_id: str = "test-mcp-client"): settings = get_settings() return { "X-Service-Token": token if token is not None else settings.mcp_hub_service_token, - "X-Client-Id": client_id, + "X-Client-Id": f"itworx-mcp-hub:mobilityops:{client_id}", } @@ -16,12 +16,31 @@ def test_operations_summary_requires_service_token(client): assert response.status_code == 401 +def test_operations_summary_rejects_spoofed_client_identity(client): + settings = get_settings() + response = client.get( + "/api/v1/integrations/mcp/operations-summary", + headers={"X-Service-Token": settings.mcp_hub_service_token, "X-Client-Id": "spoofed"}, + ) + assert response.status_code == 403 + + +def test_operations_summary_rejects_cross_tenant_request(client): + response = client.get( + "/api/v1/integrations/mcp/operations-summary", + headers={**_headers(), "X-Tenant-Id": "another-tenant"}, + ) + assert response.status_code == 403 + + def test_operations_summary_returns_metrics(client): response = client.get("/api/v1/integrations/mcp/operations-summary", headers=_headers()) assert response.status_code == 200 body = response.json() assert "metrics" in body assert body["metrics"]["available"] >= 0 + assert response.headers["x-correlation-id"] + assert response.headers["x-tenant-id"] == get_settings().ragcore_tenant def test_attention_vehicles_filters_by_severity(client): @@ -72,6 +91,8 @@ def test_search_knowledge_grounded_and_respects_max_sources(client): body = response.json() assert body["evidence_state"] == "grounded" assert len(body["sources"]) == 1 + assert int(response.headers["x-sources-available"]) >= 1 + assert response.headers["x-sources-returned"] == "1" def test_mcp_tool_requests_are_audited(client, ops_client): @@ -118,7 +139,7 @@ def test_operations_summary_mints_correlation_id_when_none_supplied(client, ops_ ) assert response.status_code == 200 events = ops_client.get("/api/v1/audit", params={"action": "mcp_tool_request"}).json() - matching = [e for e in events if e["actor_label"] == "no-correlation-probe"] + matching = [e for e in events if e["actor_label"].endswith(":no-correlation-probe")] assert len(matching) >= 1 assert matching[0]["correlation_id"] # a fresh UUID was minted, not left empty diff --git a/contracts/mcp-tools.json b/contracts/mcp-tools.json index 14de04a..b952e1b 100644 --- a/contracts/mcp-tools.json +++ b/contracts/mcp-tools.json @@ -1,8 +1,19 @@ { "_note": "Fleet Ops's own published tool contract. The live ITWorx MCP Hub connector (ITWorx_MCP_Hub repo, connectors/mobilityops/) wraps these under its own dotted namespace (mobilityops.operations.summary, .attention.list, .vehicle.get, .knowledge.search) -- that naming is Hub-owned. deprecated_aliases below are Fleet Ops's own prior internal audit-label names, kept only so existing clients/dashboards referencing them don't break.", "provider_id": "mobilityops", - "version": "1.1.0", + "version": "1.2.0", "required_scope": "mobilityops.read", + "authentication": { + "service_header": "X-Service-Token", + "client_header": "X-Client-Id", + "client_pattern": "^itworx-mcp-hub:(readiness|mobilityops:[A-Za-z0-9][A-Za-z0-9._:-]{0,127})$", + "optional_tenant_header": "X-Tenant-Id", + "tenant_must_match": "northstar-mobility-demo" + }, + "observability": { + "response_headers": ["X-Correlation-Id", "X-Tenant-Id", "Cache-Control"], + "knowledge_response_headers": ["X-Sources-Available", "X-Sources-Returned"] + }, "tools": [ { "name": "fleet_ops_get_operations_summary",