M13: harden MCP trust boundary
This commit is contained in:
@@ -2314,3 +2314,18 @@ evidence yet."
|
|||||||
focused Unraid contracts **19 passed** and the full suite **208 passed, 1 warning**.
|
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
|
- Exact next action: harden MCP per-client authorization and evidence completeness, then
|
||||||
replace inferred n8n status with explicit heartbeat/execution telemetry.
|
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.
|
||||||
|
|||||||
+25
-4
@@ -1,6 +1,9 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import hmac
|
||||||
|
import re
|
||||||
from collections.abc import Generator
|
from collections.abc import Generator
|
||||||
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from fastapi import Depends, Header, HTTPException, Request, status
|
from fastapi import Depends, Header, HTTPException, Request, status
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -60,12 +63,30 @@ def require_operations_manager(
|
|||||||
return user
|
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(
|
def require_mcp_service_token(
|
||||||
x_service_token: str = Header(..., alias="X-Service-Token"),
|
x_service_token: str = Header(..., alias="X-Service-Token"),
|
||||||
x_client_id: str = Header(default="unknown-mcp-client", alias="X-Client-Id"),
|
x_client_id: str = Header(..., alias="X-Client-Id", min_length=1, max_length=180),
|
||||||
) -> str:
|
x_tenant_id: str | None = Header(default=None, alias="X-Tenant-Id", max_length=120),
|
||||||
if x_service_token != settings.mcp_hub_service_token:
|
) -> McpClientContext:
|
||||||
|
if not hmac.compare_digest(x_service_token, settings.mcp_hub_service_token):
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid service token"
|
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)
|
||||||
|
|||||||
@@ -3,11 +3,11 @@ from __future__ import annotations
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import date
|
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 import select
|
||||||
from sqlalchemy.orm import Session
|
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.config import get_settings
|
||||||
from app.core.errors import AppError
|
from app.core.errors import AppError
|
||||||
from app.models.booking import Booking
|
from app.models.booking import Booking
|
||||||
@@ -42,7 +42,8 @@ def get_correlation_id(
|
|||||||
|
|
||||||
|
|
||||||
def _audit_service_request(
|
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:
|
) -> None:
|
||||||
record_audit_event(
|
record_audit_event(
|
||||||
db,
|
db,
|
||||||
@@ -51,43 +52,53 @@ def _audit_service_request(
|
|||||||
action="mcp_tool_request",
|
action="mcp_tool_request",
|
||||||
entity_type="mcp_tool",
|
entity_type="mcp_tool",
|
||||||
correlation_id=uuid.UUID(correlation_id),
|
correlation_id=uuid.UUID(correlation_id),
|
||||||
metadata={"tool": tool, "status": status_label},
|
metadata={"tool": tool, "status": status_label, **(metadata or {})},
|
||||||
)
|
)
|
||||||
db.commit()
|
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)
|
@router.get("/operations-summary", response_model=OperationsSummaryOut)
|
||||||
def operations_summary(
|
def operations_summary(
|
||||||
|
response: Response,
|
||||||
db: Session = Depends(get_db),
|
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),
|
correlation_id: str = Depends(get_correlation_id),
|
||||||
) -> OperationsSummaryOut:
|
) -> OperationsSummaryOut:
|
||||||
metrics = compute_metrics(db)
|
metrics = compute_metrics(db)
|
||||||
|
_set_trace_headers(response, correlation_id, client.tenant)
|
||||||
_audit_service_request(
|
_audit_service_request(
|
||||||
db,
|
db,
|
||||||
client_id=client_id,
|
client_id=client.client_id,
|
||||||
tool="fleet_ops_get_operations_summary",
|
tool="fleet_ops_get_operations_summary",
|
||||||
status_label="ok",
|
status_label="ok",
|
||||||
correlation_id=correlation_id,
|
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])
|
@router.get("/attention-vehicles", response_model=list[AttentionVehicleOut])
|
||||||
def attention_vehicles(
|
def attention_vehicles(
|
||||||
|
response: Response,
|
||||||
minimum_severity: str = Query(default="medium", pattern="^(low|medium|high)$"),
|
minimum_severity: str = Query(default="medium", pattern="^(low|medium|high)$"),
|
||||||
date_filter: date | None = Query(default=None, alias="date"),
|
date_filter: date | None = Query(default=None, alias="date"),
|
||||||
limit: int = Query(default=20, ge=1, le=50),
|
limit: int = Query(default=20, ge=1, le=50),
|
||||||
db: Session = Depends(get_db),
|
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),
|
correlation_id: str = Depends(get_correlation_id),
|
||||||
) -> list[AttentionVehicleOut]:
|
) -> list[AttentionVehicleOut]:
|
||||||
results = list_attention_vehicles(
|
results = list_attention_vehicles(
|
||||||
db, minimum_severity=minimum_severity, on_or_before=date_filter, limit=limit
|
db, minimum_severity=minimum_severity, on_or_before=date_filter, limit=limit
|
||||||
)
|
)
|
||||||
|
_set_trace_headers(response, correlation_id, client.tenant)
|
||||||
_audit_service_request(
|
_audit_service_request(
|
||||||
db,
|
db,
|
||||||
client_id=client_id,
|
client_id=client.client_id,
|
||||||
tool="fleet_ops_list_attention_vehicles",
|
tool="fleet_ops_list_attention_vehicles",
|
||||||
status_label="ok",
|
status_label="ok",
|
||||||
correlation_id=correlation_id,
|
correlation_id=correlation_id,
|
||||||
@@ -98,15 +109,17 @@ def attention_vehicles(
|
|||||||
@router.get("/vehicles/{vehicle_ref}", response_model=McpVehicleDetailOut)
|
@router.get("/vehicles/{vehicle_ref}", response_model=McpVehicleDetailOut)
|
||||||
def vehicle_details(
|
def vehicle_details(
|
||||||
vehicle_ref: str,
|
vehicle_ref: str,
|
||||||
|
response: Response,
|
||||||
db: Session = Depends(get_db),
|
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),
|
correlation_id: str = Depends(get_correlation_id),
|
||||||
) -> McpVehicleDetailOut:
|
) -> McpVehicleDetailOut:
|
||||||
|
_set_trace_headers(response, correlation_id, client.tenant)
|
||||||
vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref))
|
vehicle = db.scalar(select(Vehicle).where(Vehicle.public_ref == vehicle_ref))
|
||||||
if vehicle is None:
|
if vehicle is None:
|
||||||
_audit_service_request(
|
_audit_service_request(
|
||||||
db,
|
db,
|
||||||
client_id=client_id,
|
client_id=client.client_id,
|
||||||
tool="fleet_ops_get_vehicle_details",
|
tool="fleet_ops_get_vehicle_details",
|
||||||
status_label="not_found",
|
status_label="not_found",
|
||||||
correlation_id=correlation_id,
|
correlation_id=correlation_id,
|
||||||
@@ -128,7 +141,7 @@ def vehicle_details(
|
|||||||
|
|
||||||
_audit_service_request(
|
_audit_service_request(
|
||||||
db,
|
db,
|
||||||
client_id=client_id,
|
client_id=client.client_id,
|
||||||
tool="fleet_ops_get_vehicle_details",
|
tool="fleet_ops_get_vehicle_details",
|
||||||
status_label="ok",
|
status_label="ok",
|
||||||
correlation_id=correlation_id,
|
correlation_id=correlation_id,
|
||||||
@@ -150,18 +163,29 @@ def vehicle_details(
|
|||||||
@router.post("/search-knowledge", response_model=GroundedAnswer)
|
@router.post("/search-knowledge", response_model=GroundedAnswer)
|
||||||
def search_knowledge(
|
def search_knowledge(
|
||||||
body: McpKnowledgeSearchRequest,
|
body: McpKnowledgeSearchRequest,
|
||||||
|
response: Response,
|
||||||
db: Session = Depends(get_db),
|
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),
|
correlation_id: str = Depends(get_correlation_id),
|
||||||
) -> GroundedAnswer:
|
) -> GroundedAnswer:
|
||||||
provider = get_knowledge_provider()
|
provider = get_knowledge_provider()
|
||||||
answer = provider.ask(body.question, correlation_id, language=body.locale)
|
answer = provider.ask(body.question, correlation_id, language=body.locale)
|
||||||
|
source_count_available = len(answer.sources)
|
||||||
answer.sources = answer.sources[: body.max_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(
|
_audit_service_request(
|
||||||
db,
|
db,
|
||||||
client_id=client_id,
|
client_id=client.client_id,
|
||||||
tool="fleet_ops_search_knowledge",
|
tool="fleet_ops_search_knowledge",
|
||||||
status_label=answer.evidence_state,
|
status_label=answer.evidence_state,
|
||||||
correlation_id=correlation_id,
|
correlation_id=correlation_id,
|
||||||
|
metadata={
|
||||||
|
"tenant": client.tenant,
|
||||||
|
"locale": body.locale,
|
||||||
|
"sources_available": source_count_available,
|
||||||
|
"sources_returned": len(answer.sources),
|
||||||
|
},
|
||||||
)
|
)
|
||||||
return answer
|
return answer
|
||||||
|
|||||||
@@ -75,7 +75,7 @@ def test_demo_reset_preserves_integration_telemetry(ops_client, client):
|
|||||||
"/api/v1/integrations/mcp/operations-summary",
|
"/api/v1/integrations/mcp/operations-summary",
|
||||||
headers={
|
headers={
|
||||||
"X-Service-Token": settings.mcp_hub_service_token,
|
"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
|
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
|
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()
|
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)
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ def _headers(token: str | None = None, client_id: str = "test-mcp-client"):
|
|||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
return {
|
return {
|
||||||
"X-Service-Token": token if token is not None else settings.mcp_hub_service_token,
|
"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
|
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):
|
def test_operations_summary_returns_metrics(client):
|
||||||
response = client.get("/api/v1/integrations/mcp/operations-summary", headers=_headers())
|
response = client.get("/api/v1/integrations/mcp/operations-summary", headers=_headers())
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
body = response.json()
|
body = response.json()
|
||||||
assert "metrics" in body
|
assert "metrics" in body
|
||||||
assert body["metrics"]["available"] >= 0
|
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):
|
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()
|
body = response.json()
|
||||||
assert body["evidence_state"] == "grounded"
|
assert body["evidence_state"] == "grounded"
|
||||||
assert len(body["sources"]) == 1
|
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):
|
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
|
assert response.status_code == 200
|
||||||
events = ops_client.get("/api/v1/audit", params={"action": "mcp_tool_request"}).json()
|
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 len(matching) >= 1
|
||||||
assert matching[0]["correlation_id"] # a fresh UUID was minted, not left empty
|
assert matching[0]["correlation_id"] # a fresh UUID was minted, not left empty
|
||||||
|
|
||||||
|
|||||||
@@ -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.",
|
"_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",
|
"provider_id": "mobilityops",
|
||||||
"version": "1.1.0",
|
"version": "1.2.0",
|
||||||
"required_scope": "mobilityops.read",
|
"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": [
|
"tools": [
|
||||||
{
|
{
|
||||||
"name": "fleet_ops_get_operations_summary",
|
"name": "fleet_ops_get_operations_summary",
|
||||||
|
|||||||
Reference in New Issue
Block a user