From ee5ebf08deb8fad4951da50125dff4a3e621b5c9 Mon Sep 17 00:00:00 2001 From: busya Date: Thu, 4 Jun 2026 16:15:40 +0300 Subject: [PATCH] fix(backend): migrate trace middleware to raw ASGI for contextvar isolation BaseHTTPMiddleware (Starlette 0.50.0) uses anyio.create_task_group() internally, creating separate asyncio tasks for dispatch vs call_next. ContextVars set in dispatch() were not visible to outer middleware like log_requests. Converting to raw ASGI middleware ensures trace_id is seeded in the root task context, visible to ALL middleware layers. Key changes: - Replace BaseHTTPMiddleware with raw ASGI __call__(self, scope, receive, send) - UUID v4 validation: check parsed.version == 4 explicitly instead of relying on uuid.UUID(hex=..., version=4) which silently mutates non-v4 UUIDs - Add @RATIONALE and @REJECTED tags per semantics-core protocol - Update app.py comment to document the architectural decision --- backend/src/app.py | 9 ++- backend/src/core/middleware/trace.py | 105 ++++++++++++++------------- 2 files changed, 62 insertions(+), 52 deletions(-) diff --git a/backend/src/app.py b/backend/src/app.py index 5d432de2..b560da8e 100755 --- a/backend/src/app.py +++ b/backend/src/app.py @@ -122,8 +122,13 @@ app = FastAPI( version="1.0.0", lifespan=lifespan, ) -# TraceContextMiddleware MUST be the outermost middleware so trace_id -# is seeded before any other middleware or handler runs. +# TraceContextMiddleware is a raw ASGI middleware (not BaseHTTPMiddleware). +# This is by design — BaseHTTPMiddleware (Starlette 0.50.0) uses +# anyio.create_task_group() internally, creating separate asyncio tasks for +# dispatch vs call_next. ContextVars set in dispatch() are NOT visible to +# outer middleware layers (like log_requests). Raw ASGI middleware runs in +# the root task context, making trace_id visible to ALL middleware layers. +# See trace.py @RATIONALE for details. from .core.middleware.trace import TraceContextMiddleware # noqa: E402 app.add_middleware(TraceContextMiddleware) # #endregion FastAPI_App diff --git a/backend/src/core/middleware/trace.py b/backend/src/core/middleware/trace.py index 06a8f9a7..75e350a8 100644 --- a/backend/src/core/middleware/trace.py +++ b/backend/src/core/middleware/trace.py @@ -1,81 +1,86 @@ # #region TraceContextMiddlewareModule [C:3] [TYPE Module] [SEMANTICS fastapi, middleware, trace, context, request] -# @BRIEF FastAPI/Starlette middleware that seeds a trace_id for every incoming HTTP request. +# @BRIEF Raw ASGI middleware that seeds a trace_id for every incoming HTTP request. # Optionally extracts X-Trace-ID header for cross-service trace propagation. +# Implemented as raw ASGI middleware (not BaseHTTPMiddleware) to avoid +# contextvar isolation caused by BaseHTTPMiddleware's internal task switching +# via anyio.create_task_group() — see [RCA TRACE-001] for details. # @LAYER Core # @RELATION DEPENDS_ON -> [CotLoggerModule] # @RELATION CALLED_BY -> [AppModule] -# @INVARIANT X-Trace-ID header is validated as UUID4 format to prevent log injection / trace poisoning. -# @PRE FastAPI app instance with Starlette middleware support. -# @POST Every request gets a trace_id via seed_trace_id(). Existing X-Trace-ID header is -# preserved and used as the trace_id when present and valid. -# @SIDE_EFFECT Sets ContextVar _trace_id for the duration of the request. +# @INVARIANT X-Trace-ID header UUID is validated as version 4 explicitly (not via +# uuid.UUID(hex=..., version=4) which silently mutates non-v4 UUIDs). +# Non-UTF8 header bytes are caught via UnicodeDecodeError. See [FIX TRACE-002]. +# @PRE FastAPI app instance with ASGI middleware support. +# @POST Every HTTP request gets a trace_id via seed_trace_id(). +# If a valid UUID4 X-Trace-ID header is present, that value is used. +# Non-v4 UUIDs, non-UUID values, and malformed UTF-8 headers fall back to +# seed_trace_id() (graceful degradation). +# @SIDE_EFFECT Sets ContextVar _trace_id for the current asyncio task context. +# Lifetime is bounded by the asyncio task — no explicit cleanup needed since +# each request runs in its own task. +# @RATIONALE BaseHTTPMiddleware (Starlette 0.50.0) uses anyio.create_task_group() internally, +# which creates separate asyncio tasks for dispatch and call_next. ContextVars set +# in dispatch() are not visible to outer middlewares (like log_requests) because +# they run in different async contexts. Converting to raw ASGI middleware ensures +# trace_id is seeded in the root task context before any BaseHTTPMiddleware runs, +# making it visible to ALL middleware layers. See [RCA TRACE-001]. +# @REJECTED Keeping BaseHTTPMiddleware with workaround (e.g. manually passing trace_id via +# request.state) was rejected — it would require modifying every middleware that +# needs trace_id access. Raw ASGI middleware is the minimal fix. +# @REJECTED uuid.UUID(hex=..., version=4) validation rejected in [FIX TRACE-002] — the +# version=N parameter silently MUTATES the UUID's version nibble instead of +# validating it. A UUID v1 passed as X-Trace-ID would be corrupted to a different +# UUID, breaking cross-service trace correlation. We now parse without version +# and check parsed.version == 4 explicitly. import uuid -from starlette.middleware.base import BaseHTTPMiddleware -from starlette.requests import Request -from starlette.responses import Response -from starlette.types import ASGIApp +from starlette.types import ASGIApp, Receive, Scope, Send from ..cot_logger import seed_trace_id, set_trace_id -# #region TraceContextMiddleware [C:2] [TYPE Class] [SEMANTICS middleware, trace, context, dispatch] -# @BRIEF Starlette BaseHTTPMiddleware that seeds a trace_id per request. -class TraceContextMiddleware(BaseHTTPMiddleware): - """FastAPI/Starlette middleware that seeds a trace_id for every request. - - If the incoming request carries an ``X-Trace-ID`` header, that value is - validated as UUID4 and used as the trace_id (enabling cross-service trace - chaining). Invalid or non-UUID values are ignored and a new UUID4 is - generated instead. - - Usage:: - - from backend.src.core.middleware.trace import TraceContextMiddleware - app.add_middleware(TraceContextMiddleware) - """ +# #region TraceContextMiddleware [C:2] [TYPE Class] [SEMANTICS middleware, trace, context, asgi] +# @BRIEF Raw ASGI middleware that seeds a trace_id per request. Runs in the root task +# context, ensuring all downstream BaseHTTPMiddleware layers see the trace_id. +class TraceContextMiddleware: # #region TraceContextMiddleware.__init__ [TYPE Function] - # @BRIEF Standard BaseHTTPMiddleware initialiser. + # @BRIEF Store the inner ASGI app. def __init__(self, app: ASGIApp) -> None: - super().__init__(app) + self.app = app # #endregion TraceContextMiddleware.__init__ - # #region TraceContextMiddleware.dispatch [C:2] [TYPE Function] [SEMANTICS dispatch, trace, seed, header, call_next] - # @BRIEF Dispatch handler that seeds trace_id before passing to the next middleware. - async def dispatch( - self, request: Request, call_next - ) -> Response: - """Intercept every request, seed the trace_id, and forward. + # #region TraceContextMiddleware.__call__ [C:2] [TYPE Function] [SEMANTICS asgi, call, trace, seed, header] + # @BRIEF ASGI __call__ that seeds trace_id in the root task context before forwarding. + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return - If the client sent an ``X-Trace-ID`` header with a valid UUID4 value, - use it; otherwise ``seed_trace_id()`` generates a new UUID4. - Invalid X-Trace-ID values are silently ignored to prevent log - injection / trace poisoning (see [SEC:H-5]). + # Extract X-Trace-ID from raw ASGI scope headers + incoming_trace_id = None + for header_name, header_value in scope.get("headers", []): + if header_name.lower() == b"x-trace-id": + incoming_trace_id = header_value.decode("utf-8", errors="replace") + break - Args: - request: The incoming Starlette Request. - call_next: The next middleware or route handler. - - Returns: - Response from the downstream handler. - """ - incoming_trace_id = request.headers.get("X-Trace-ID") if incoming_trace_id: try: - uuid.UUID(hex=incoming_trace_id, version=4) - set_trace_id(incoming_trace_id) + parsed = uuid.UUID(hex=incoming_trace_id) + if parsed.version == 4: + set_trace_id(incoming_trace_id) + else: + seed_trace_id() except (ValueError, AttributeError): seed_trace_id() else: seed_trace_id() - response = await call_next(request) - return response + await self.app(scope, receive, send) - # #endregion TraceContextMiddleware.dispatch + # #endregion TraceContextMiddleware.__call__ # #endregion TraceContextMiddleware