Found while fixing #16 (middleware skipped on error responses). SecurityMiddleware.call:
async def __call__(self, request: Request) -> Response:
try:
return await self._on_request(request)
except Exception:
# Security middleware must never crash the request pipeline.
return await self.app(request)_on_request implementations (InputSanitizationMiddleware, BruteForceProtection, CSRF, RequestLimits) typically call await self.app(request) themselves partway through, after their own checks pass. If that inner call raises (e.g. a 404 from routing, or any handler exception), the exception propagates up out of _on_request, gets caught by this try/except, and self.app(request) gets called a SECOND time — a full second invocation of the downstream chain (handler included) for a single incoming request. For a POST handler this means a real risk of double side effects (double DB write, etc.) whenever the handler raises after doing a side effect but before returning.
The comment's intent ("security middleware must never crash the request pipeline") is reasonable for bugs in the middleware's OWN detection logic (e.g. a bad regex throwing), but the current try/except can't distinguish "exception from our own logic" from "exception that already came from self.app(request), which we already awaited." It needs to distinguish those two cases — e.g. by giving _on_request a call_next()-style callback instead of letting subclasses call self.app directly, so the base class is the only place that ever calls self.app, exactly once, outside any retry logic.
Didn't fix as part of #16 since it's a different root cause requiring a contract change across every SecurityMiddleware subclass, not a routing/exception-handling fix in app.py.
Found while fixing #16 (middleware skipped on error responses). SecurityMiddleware.call:
_on_request implementations (InputSanitizationMiddleware, BruteForceProtection, CSRF, RequestLimits) typically call
await self.app(request)themselves partway through, after their own checks pass. If that inner call raises (e.g. a 404 from routing, or any handler exception), the exception propagates up out of _on_request, gets caught by this try/except, and self.app(request) gets called a SECOND time — a full second invocation of the downstream chain (handler included) for a single incoming request. For a POST handler this means a real risk of double side effects (double DB write, etc.) whenever the handler raises after doing a side effect but before returning.The comment's intent ("security middleware must never crash the request pipeline") is reasonable for bugs in the middleware's OWN detection logic (e.g. a bad regex throwing), but the current try/except can't distinguish "exception from our own logic" from "exception that already came from self.app(request), which we already awaited." It needs to distinguish those two cases — e.g. by giving _on_request a call_next()-style callback instead of letting subclasses call self.app directly, so the base class is the only place that ever calls self.app, exactly once, outside any retry logic.
Didn't fix as part of #16 since it's a different root cause requiring a contract change across every SecurityMiddleware subclass, not a routing/exception-handling fix in app.py.