M41: harden trust boundaries and delivery

backend/app/api/deps.py

@@ -67,7 +67,7 @@ def require_operations_manager(
 
 @dataclass(frozen=True)
 class McpClientContext:
-    client_id: str
+    reported_client_id: str
     tenant: str
 
 
@@ -91,4 +91,4 @@ def require_mcp_service_token(
         )
     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)
+    return McpClientContext(reported_client_id=x_client_id, tenant=settings.ragcore_tenant)

backend/app/api/routers/auth.py

@@ -38,10 +38,11 @@ _login_limiter = (
 
 def _client_key(request: Request) -> str:
     # The API sits behind the web container's reverse proxy in every documented
-    # deployment; honour the first hop of X-Forwarded-For when present.
+    # deployment. The proxy appends/overwrites the socket peer as the final hop, so an
+    # attacker-controlled leading value must never select a fresh limiter bucket.
     forwarded = request.headers.get("x-forwarded-for", "")
     if forwarded:
-        return forwarded.split(",")[0].strip()
+        return forwarded.split(",")[-1].strip()
     return request.client.host if request.client else "unknown"
 
 
@@ -143,7 +144,7 @@ def _allowed_oidc_email(email: str) -> bool:
 def _resolve_oidc_user(db: Session, claims: dict[str, object]) -> User:
     subject = str(claims.get("sub") or "").strip()
     email = str(claims.get("email") or "").strip().lower()
-    if not subject or not email or claims.get("email_verified") is False:
+    if not subject or not email or claims.get("email_verified") is not True:
         raise HTTPException(status_code=401, detail="Verified OIDC email and subject are required")
     if not _allowed_oidc_email(email):
         raise HTTPException(status_code=403, detail="Email domain is not allowed")

backend/app/api/routers/knowledge.py

@@ -1,20 +1,32 @@
 from __future__ import annotations
 
+import hashlib
 import uuid
 from typing import Literal
 
-from fastapi import APIRouter, Depends, HTTPException
+from fastapi import APIRouter, Depends, HTTPException, Request, status
 from pydantic import BaseModel, Field
 from sqlalchemy import select
 from sqlalchemy.orm import Session
 
 from app.api.deps import get_current_user, get_db
+from app.core.config import get_settings
+from app.core.ratelimit import SlidingWindowLimiter
 from app.models.audit import AuditEvent
 from app.schemas import CurrentUser
 from app.services.audit import record_audit_event
 from app.services.knowledge import GroundedAnswer, KnowledgeHealth, get_knowledge_provider
 
 router = APIRouter(prefix="/api/v1/knowledge", tags=["knowledge"])
+settings = get_settings()
+_question_limiter = (
+    SlidingWindowLimiter(
+        max_requests=settings.knowledge_max_requests,
+        window_seconds=settings.knowledge_rate_limit_window_seconds,
+    )
+    if settings.knowledge_max_requests > 0
+    else None
+)
 
 SupportedLanguage = Literal["nl-BE", "en-GB", "fr-BE"]
 
@@ -32,9 +44,29 @@ class KnowledgeFeedbackRequest(BaseModel):
 @router.post("/questions", response_model=GroundedAnswer)
 def ask_question(
     body: AskQuestionRequest,
+    request: Request,
     db: Session = Depends(get_db),
     user: CurrentUser = Depends(get_current_user),
 ) -> GroundedAnswer:
+    if _question_limiter is not None:
+        forwarded = request.headers.get("x-forwarded-for", "")
+        client_ip = (
+            forwarded.split(",")[-1].strip()
+            if forwarded
+            else request.client.host if request.client else "unknown"
+        )
+        token = request.cookies.get(settings.session_cookie_name, "")
+        session_key = hashlib.sha256(token.encode("utf-8")).hexdigest()
+        retry_after = max(
+            _question_limiter.consume(f"ip:{client_ip}"),
+            _question_limiter.consume(f"session:{session_key}"),
+        )
+        if retry_after:
+            raise HTTPException(
+                status_code=status.HTTP_429_TOO_MANY_REQUESTS,
+                detail="Too many knowledge questions. Try again later.",
+                headers={"Retry-After": str(retry_after)},
+            )
     correlation_id = str(uuid.uuid4())
     provider = get_knowledge_provider()
     answer = provider.ask(body.question, correlation_id, body.language)

backend/app/api/routers/mcp_integrations.py

@@ -44,7 +44,7 @@ def get_correlation_id(
 def _audit_service_request(
     db: Session,
     *,
-    client_id: str,
+    reported_client_id: str,
     tool: str,
     status_label: str,
     correlation_id: str,
@@ -53,11 +53,19 @@ def _audit_service_request(
     record_audit_event(
         db,
         actor_type="service",
-        actor_label=client_id,
+        # The shared service token authenticates the Hub, not the caller identity that
+        # the Hub reports in a header. Keep attribution authoritative and retain the
+        # reported value only as explicitly non-authenticated diagnostic metadata.
+        actor_label="itworx-mcp-hub",
         action="mcp_tool_request",
         entity_type="mcp_tool",
         correlation_id=uuid.UUID(correlation_id),
-        metadata={"tool": tool, "status": status_label, **(metadata or {})},
+        metadata={
+            "tool": tool,
+            "status": status_label,
+            "reported_client_id": reported_client_id,
+            **(metadata or {}),
+        },
     )
     db.commit()
 
@@ -79,7 +87,7 @@ def operations_summary(
     _set_trace_headers(response, correlation_id, client.tenant)
     _audit_service_request(
         db,
-        client_id=client.client_id,
+        reported_client_id=client.reported_client_id,
         tool="fleet_ops_get_operations_summary",
         status_label="ok",
         correlation_id=correlation_id,
@@ -103,7 +111,7 @@ def attention_vehicles(
     _set_trace_headers(response, correlation_id, client.tenant)
     _audit_service_request(
         db,
-        client_id=client.client_id,
+        reported_client_id=client.reported_client_id,
         tool="fleet_ops_list_attention_vehicles",
         status_label="ok",
         correlation_id=correlation_id,
@@ -124,7 +132,7 @@ def vehicle_details(
     if vehicle is None:
         _audit_service_request(
             db,
-            client_id=client.client_id,
+            reported_client_id=client.reported_client_id,
             tool="fleet_ops_get_vehicle_details",
             status_label="not_found",
             correlation_id=correlation_id,
@@ -146,7 +154,7 @@ def vehicle_details(
 
     _audit_service_request(
         db,
-        client_id=client.client_id,
+        reported_client_id=client.reported_client_id,
         tool="fleet_ops_get_vehicle_details",
         status_label="ok",
         correlation_id=correlation_id,
@@ -182,7 +190,7 @@ def search_knowledge(
     response.headers["X-Sources-Returned"] = str(len(answer.sources))
     _audit_service_request(
         db,
-        client_id=client.client_id,
+        reported_client_id=client.reported_client_id,
         tool="fleet_ops_search_knowledge",
         status_label=answer.evidence_state,
         correlation_id=correlation_id,

backend/app/core/config.py

@@ -23,6 +23,8 @@ class Settings(BaseSettings):
     ragcore_api_token: str = ""
     ragcore_space_id: str = ""
     ragcore_http_timeout_seconds: float = 5.0
+    # Search fallback is only labelled grounded above this explicit retrieval threshold.
+    ragcore_min_search_score: float = 0.05
     n8n_webhook_url: str = "http://n8n:5678/webhook/mobilityops-return"
     n8n_webhook_trigger_token: str = "replace-me-n8n-webhook-trigger-token"
     n8n_callback_token: str = "replace-me-n8n-callback-token"
@@ -67,6 +69,8 @@ class Settings(BaseSettings):
     # Failed password logins per client IP before a temporary 429 (0 disables).
     login_max_failures: int = 10
     login_failure_window_seconds: int = 900
+    knowledge_max_requests: int = 30
+    knowledge_rate_limit_window_seconds: int = 60
     metrics_bearer_token: str = ""
     privacy_minimum_booking_retention_days: int = 30
     privacy_audit_retention_days: int = 2555
@@ -86,13 +90,11 @@ INSECURE_DEFAULT_SECRETS: tuple[tuple[str, str], ...] = (
 def insecure_default_secrets(settings: "Settings") -> list[str]:
     """Return the names of secret settings that still carry their placeholder value.
 
-    Only secrets that actually guard something in the given deployment are reported:
-    ``mcp_hub_service_token`` is irrelevant while MCP Hub registration is disabled.
+    MCP routes are always mounted, independently of the Hub reachability-status flag, so
+    their inbound token must always be non-placeholder in production.
     """
     insecure: list[str] = []
     for name, placeholder in INSECURE_DEFAULT_SECRETS:
-        if name == "mcp_hub_service_token" and not settings.mcp_hub_registration_enabled:
-            continue
         value = getattr(settings, name)
         if not value or value == placeholder or value.startswith("replace-me"):
             insecure.append(name)
@@ -112,4 +114,8 @@ def get_settings() -> Settings:
                 + ", ".join(insecure)
                 + ". Set real values in the environment (see .env.example)."
             )
+        if not settings.mobilityops_public_url.lower().startswith("https://"):
+            raise RuntimeError("Production MOBILITYOPS_PUBLIC_URL must use HTTPS.")
+        if not settings.session_cookie_secure:
+            raise RuntimeError("Production SESSION_COOKIE_SECURE must be true.")
     return settings

backend/app/core/ratelimit.py

@@ -47,3 +47,26 @@ class FailedAttemptLimiter:
     def reset(self, key: str) -> None:
         with self._lock:
             self._failures.pop(key, None)
+
+
+class SlidingWindowLimiter:
+    """Thread-safe request limiter where every accepted request consumes capacity."""
+
+    def __init__(self, *, max_requests: int, window_seconds: float) -> None:
+        self.max_requests = max_requests
+        self.window_seconds = window_seconds
+        self._requests: dict[str, deque[float]] = {}
+        self._lock = threading.Lock()
+
+    def consume(self, key: str) -> int:
+        """Record an accepted request, or return the seconds until capacity is available."""
+        now = time.monotonic()
+        with self._lock:
+            bucket = self._requests.setdefault(key, deque())
+            cutoff = now - self.window_seconds
+            while bucket and bucket[0] <= cutoff:
+                bucket.popleft()
+            if len(bucket) >= self.max_requests:
+                return max(1, int(bucket[0] + self.window_seconds - now + 0.999))
+            bucket.append(now)
+            return 0

backend/app/models/data_quality.py

@@ -2,7 +2,7 @@ import uuid
 from datetime import datetime
 from typing import TYPE_CHECKING
 
-from sqlalchemy import CheckConstraint, DateTime, ForeignKey, Index, String
+from sqlalchemy import CheckConstraint, DateTime, ForeignKey, Index, String, text
 from sqlalchemy.dialects.postgresql import JSONB, UUID
 from sqlalchemy.orm import Mapped, mapped_column, relationship
 
@@ -37,6 +37,14 @@ class DataQualityIssue(UUIDPrimaryKeyMixin, TimestampMixin, Base):
             name="ck_data_quality_status",
         ),
         Index("ix_data_quality_work_queue", "status", "due_at", "severity"),
+        Index(
+            "uq_data_quality_one_open_condition",
+            "rule_type",
+            "entity_type",
+            "entity_id",
+            unique=True,
+            postgresql_where=text("status = 'open'"),
+        ),
     )
 
     public_ref: Mapped[str] = mapped_column(String(20), unique=True, nullable=False)

backend/app/services/data_quality.py

@@ -5,7 +5,7 @@ from dataclasses import dataclass, field
 from datetime import UTC, datetime, timedelta
 from difflib import SequenceMatcher
 
-from sqlalchemy import select, update
+from sqlalchemy import func, select, update
 from sqlalchemy.orm import Session
 
 from app.core.errors import AppError
@@ -26,6 +26,7 @@ from app.services.vehicle_status import (
 REQUIRED_CUSTOMER_FIELDS = ("first_name", "last_name")
 REQUIRED_VEHICLE_FIELDS = ("registration_number", "make", "model", "location")
 DUPLICATE_THRESHOLD = 70
+DATA_QUALITY_SCAN_LOCK_ID = 6_138_493_717_091_029_491
 
 
 def issue_due_at(detected_at: datetime, severity: str) -> datetime:
@@ -356,6 +357,9 @@ def _scan_odometer_regressions(db: Session, scan: ScanResult) -> None:
 def run_scan(
     db: Session, *, actor_label: str | None = None, actor_type: str = "user"
 ) -> ScanResult:
+    # The check-then-insert work below spans several rules. Serialise whole scans at the
+    # database boundary so API and n8n triggers cannot both observe an empty condition.
+    db.scalar(select(func.pg_advisory_xact_lock(DATA_QUALITY_SCAN_LOCK_ID)))
     scan = ScanResult()
     _scan_duplicate_customers(db, scan)
     _scan_missing_required_fields(db, scan)
@@ -375,8 +379,13 @@ def run_scan(
     return scan
 
 
-def _load_open_issue(db: Session, public_ref: str) -> DataQualityIssue:
-    issue = db.scalar(select(DataQualityIssue).where(DataQualityIssue.public_ref == public_ref))
+def _load_open_issue(
+    db: Session, public_ref: str, *, lock: bool = True
+) -> DataQualityIssue:
+    statement = select(DataQualityIssue).where(DataQualityIssue.public_ref == public_ref)
+    if lock:
+        statement = statement.with_for_update()
+    issue = db.scalar(statement)
     if issue is None:
         raise AppError("ISSUE_NOT_FOUND", "Data quality issue not found.", status_code=404)
     if issue.status != "open":
@@ -707,8 +716,10 @@ def resolve_booking_overlap(
     return issue
 
 
-def _load_vehicle_status_conflict_issue(db: Session, public_ref: str) -> DataQualityIssue:
-    issue = _load_open_issue(db, public_ref)
+def _load_vehicle_status_conflict_issue(
+    db: Session, public_ref: str, *, lock: bool = True
+) -> DataQualityIssue:
+    issue = _load_open_issue(db, public_ref, lock=lock)
     if issue.rule_type != "vehicle_status_conflict":
         raise AppError(
             "NOT_A_STATUS_CONFLICT_ISSUE",
@@ -724,7 +735,7 @@ def preview_vehicle_status_recommendation(
     """Non-mutating: computes and returns the recommendation only. Never resolves the
     issue, never writes an audit event, never queues automation -- safe to call as often
     as the UI needs (e.g. every time the panel is opened) with zero side effects."""
-    issue = _load_vehicle_status_conflict_issue(db, public_ref)
+    issue = _load_vehicle_status_conflict_issue(db, public_ref, lock=False)
     vehicle = db.scalar(select(Vehicle).where(Vehicle.id == issue.entity_id))
     if vehicle is None:
         raise AppError(

backend/app/services/knowledge/ragcore.py

@@ -92,6 +92,17 @@ def _rank_sources_for_concepts(sources: list[SourceCard], concepts: set[str]) ->
     )
 
 
+def _retrieval_score(result: dict) -> float | None:
+    scores = result.get("scores")
+    if not isinstance(scores, dict):
+        return None
+    for name in ("rerank", "fused"):
+        value = scores.get(name)
+        if isinstance(value, int | float) and not isinstance(value, bool):
+            return float(value)
+    return None
+
+
 class RAGcoreKnowledgeProvider:
     """Adapter for the central RAGcore service, against its real `/v1/*` contract
     (see `docs/contracts/openapi.yaml` in the RAGcore checkout -- RAGcore is built and
@@ -338,6 +349,8 @@ class RAGcoreKnowledgeProvider:
 
         try:
             results = body.get("results", [])
+            if not isinstance(results, list):
+                raise TypeError("results must be a list")
             sources = [
                 SourceCard(
                     document_id=str(result["citation"]["document_id"]),
@@ -351,10 +364,21 @@ class RAGcoreKnowledgeProvider:
         except (TypeError, KeyError, ValueError):
             return unavailable
 
-        sources = _deduplicate_sources(sources)
+        all_sources = _deduplicate_sources(sources)
         concepts = _question_concepts(question)
-        sources = _rank_sources_for_concepts(sources, concepts)
-        if not sources:
+        qualified_sources = [
+            SourceCard(
+                document_id=str(result["citation"]["document_id"]),
+                title=result["citation"]["title"],
+                version=str(result["citation"]["document_version_id"]),
+                section=result["citation"].get("section") or "",
+                excerpt=result["citation"]["excerpt"],
+            )
+            for result in results
+            if (_retrieval_score(result) or 0.0) >= self._settings.ragcore_min_search_score
+        ]
+        sources = _rank_sources_for_concepts(_deduplicate_sources(qualified_sources), concepts)
+        if not all_sources:
             return GroundedAnswer(
                 answer="",
                 evidence_state="insufficient",
@@ -363,11 +387,19 @@ class RAGcoreKnowledgeProvider:
                 correlation_id=correlation_id,
             )
 
-        if not concepts:
+        damage_evidence = any(
+            term
+            in (
+                f"{source.document_id} {source.title} {source.section} {source.excerpt}"
+            ).casefold()
+            for source in sources
+            for term in _DOMAIN_CONCEPTS["damage"]
+        )
+        if not concepts or not sources or ("damage" in concepts and not damage_evidence):
             return GroundedAnswer(
                 answer="",
                 evidence_state="insufficient",
-                sources=sources,
+                sources=all_sources,
                 provider=self.name,
                 correlation_id=correlation_id,
             )