fix(api): scope 429 OpenAPI injection to rate-limited routes
Previous commit advertised 429 on every operation. Only routes
decorated with @limiter.limit can actually return slowapi's 429 —
currently just POST /api/v1/auth/login. Documenting it elsewhere is
dishonest and would mislead clients into expecting a response the
server cannot produce.
Walk slowapi's _route_limits / _dynamic_route_limits registries to
identify decorated endpoints, match them to FastAPI routes by
{module}.{name}, and only inject 429 on those.
Existing per-route 429 declarations (e.g. SSE connection-cap on
events streams via sse_limits) are untouched.
This commit is contained in:
@@ -258,15 +258,32 @@ if DECNET_PROFILE_REQUESTS:
|
|||||||
app.include_router(api_router, prefix="/api/v1")
|
app.include_router(api_router, prefix="/api/v1")
|
||||||
|
|
||||||
|
|
||||||
def _custom_openapi() -> dict:
|
def _rate_limited_endpoint_names() -> set[str]:
|
||||||
"""Inject the global 429 response into every operation.
|
"""Return ``{module}.{qualname}`` for every endpoint slowapi tracks.
|
||||||
|
|
||||||
SlowAPI middleware can short-circuit any request with 429 once the
|
slowapi tags each ``@limiter.limit(...)``-decorated function under
|
||||||
per-route or per-IP rate limit fires, so the OpenAPI spec must
|
these two registries. We do NOT advertise 429 on undecorated routes:
|
||||||
advertise 429 on every operation — otherwise schemathesis flags
|
SlowAPIMiddleware only consults the registries, so an endpoint
|
||||||
legitimate rate-limit responses as schema-noncompliant.
|
without a decorator can never legitimately return 429, and
|
||||||
|
documenting it would mislead clients.
|
||||||
|
"""
|
||||||
|
names: set[str] = set()
|
||||||
|
names.update(getattr(limiter, "_route_limits", {}).keys())
|
||||||
|
names.update(getattr(limiter, "_dynamic_route_limits", {}).keys())
|
||||||
|
return names
|
||||||
|
|
||||||
|
|
||||||
|
def _custom_openapi() -> dict:
|
||||||
|
"""Inject 429 into the OpenAPI spec for rate-limited operations only.
|
||||||
|
|
||||||
|
SlowAPI returns 429 from ``@limiter.limit(...)``-decorated routes
|
||||||
|
(currently just ``POST /api/v1/auth/login``). Without 429 in the
|
||||||
|
spec, schemathesis flags legitimate rate-limit responses as
|
||||||
|
status-code-nonconformant. We restrict the injection to actually
|
||||||
|
rate-limited endpoints so the spec stays honest.
|
||||||
"""
|
"""
|
||||||
from fastapi.openapi.utils import get_openapi
|
from fastapi.openapi.utils import get_openapi
|
||||||
|
from fastapi.routing import APIRoute
|
||||||
|
|
||||||
if app.openapi_schema:
|
if app.openapi_schema:
|
||||||
return app.openapi_schema
|
return app.openapi_schema
|
||||||
@@ -276,6 +293,12 @@ def _custom_openapi() -> dict:
|
|||||||
routes=app.routes,
|
routes=app.routes,
|
||||||
description=app.description,
|
description=app.description,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
rate_limited = _rate_limited_endpoint_names()
|
||||||
|
if not rate_limited:
|
||||||
|
app.openapi_schema = schema
|
||||||
|
return schema
|
||||||
|
|
||||||
too_many = {
|
too_many = {
|
||||||
"description": "Rate limit exceeded",
|
"description": "Rate limit exceeded",
|
||||||
"content": {
|
"content": {
|
||||||
@@ -288,13 +311,29 @@ def _custom_openapi() -> dict:
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
for path_item in schema.get("paths", {}).values():
|
|
||||||
|
# Build {(path, method): endpoint_qualname} from the live route table.
|
||||||
|
route_index: dict[tuple[str, str], str] = {}
|
||||||
|
for route in app.routes:
|
||||||
|
if not isinstance(route, APIRoute):
|
||||||
|
continue
|
||||||
|
endpoint = route.endpoint
|
||||||
|
# slowapi unwraps decorated functions but keeps the original
|
||||||
|
# __module__/__name__ via functools.wraps, so this matches.
|
||||||
|
qualname = f"{endpoint.__module__}.{endpoint.__name__}"
|
||||||
|
for method in route.methods or ():
|
||||||
|
route_index[(route.path, method.lower())] = qualname
|
||||||
|
|
||||||
|
for path, path_item in schema.get("paths", {}).items():
|
||||||
for method, op in path_item.items():
|
for method, op in path_item.items():
|
||||||
if method.lower() not in {
|
if method.lower() not in {
|
||||||
"get", "post", "put", "patch", "delete", "options", "head",
|
"get", "post", "put", "patch", "delete", "options", "head",
|
||||||
}:
|
}:
|
||||||
continue
|
continue
|
||||||
|
qualname = route_index.get((path, method.lower()))
|
||||||
|
if qualname and qualname in rate_limited:
|
||||||
op.setdefault("responses", {}).setdefault("429", too_many)
|
op.setdefault("responses", {}).setdefault("429", too_many)
|
||||||
|
|
||||||
app.openapi_schema = schema
|
app.openapi_schema = schema
|
||||||
return schema
|
return schema
|
||||||
|
|
||||||
|
|||||||
@@ -1,24 +1,50 @@
|
|||||||
"""OpenAPI must advertise 429 on every operation.
|
"""OpenAPI must advertise 429 on every slowapi-rate-limited operation.
|
||||||
|
|
||||||
SlowAPI can return 429 from any rate-limited route at any time. If the
|
Other endpoints may also advertise 429 for their own reasons (e.g. the
|
||||||
schema doesn't list it, schemathesis (and any other contract-driven
|
SSE connection cap in ``decnet.web.sse_limits``); the test does not
|
||||||
client) treats a legitimate rate-limit response as a contract violation.
|
forbid those — it only enforces the slowapi side.
|
||||||
"""
|
"""
|
||||||
from decnet.web.api import app
|
from decnet.web.api import app, _rate_limited_endpoint_names
|
||||||
|
from fastapi.routing import APIRoute
|
||||||
|
|
||||||
|
|
||||||
def test_every_operation_documents_429() -> None:
|
def _route_qualname_index() -> dict[tuple[str, str], str]:
|
||||||
|
idx: dict[tuple[str, str], str] = {}
|
||||||
|
for route in app.routes:
|
||||||
|
if not isinstance(route, APIRoute):
|
||||||
|
continue
|
||||||
|
qn = f"{route.endpoint.__module__}.{route.endpoint.__name__}"
|
||||||
|
for method in route.methods or ():
|
||||||
|
idx[(route.path, method.lower())] = qn
|
||||||
|
return idx
|
||||||
|
|
||||||
|
|
||||||
|
def test_429_documented_on_rate_limited_endpoints_only() -> None:
|
||||||
schema = app.openapi()
|
schema = app.openapi()
|
||||||
paths = schema.get("paths", {})
|
paths = schema.get("paths", {})
|
||||||
assert paths, "OpenAPI schema is empty — router not mounted"
|
assert paths, "OpenAPI schema is empty — router not mounted"
|
||||||
|
|
||||||
|
rate_limited = _rate_limited_endpoint_names()
|
||||||
|
assert rate_limited, "no @limiter.limit-decorated endpoints found"
|
||||||
|
|
||||||
|
qualname_for = _route_qualname_index()
|
||||||
|
|
||||||
http_methods = {"get", "post", "put", "patch", "delete", "options", "head"}
|
http_methods = {"get", "post", "put", "patch", "delete", "options", "head"}
|
||||||
missing: list[str] = []
|
missing: list[str] = []
|
||||||
|
|
||||||
for path, item in paths.items():
|
for path, item in paths.items():
|
||||||
for method, op in item.items():
|
for method, op in item.items():
|
||||||
if method.lower() not in http_methods:
|
if method.lower() not in http_methods:
|
||||||
continue
|
continue
|
||||||
if "429" not in op.get("responses", {}):
|
qn = qualname_for.get((path, method.lower()))
|
||||||
|
if qn in rate_limited and "429" not in op.get("responses", {}):
|
||||||
missing.append(f"{method.upper()} {path}")
|
missing.append(f"{method.upper()} {path}")
|
||||||
|
|
||||||
assert not missing, f"Operations missing 429 response: {missing[:5]}"
|
assert not missing, f"rate-limited ops missing 429: {missing}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_login_endpoint_documents_429() -> None:
|
||||||
|
"""Sanity check the one endpoint we know is rate-limited."""
|
||||||
|
schema = app.openapi()
|
||||||
|
op = schema["paths"]["/api/v1/auth/login"]["post"]
|
||||||
|
assert "429" in op["responses"]
|
||||||
|
|||||||
Reference in New Issue
Block a user