fix: cap and type-check the unauthenticated consent POST body (SEC-01)

src/aegis_gitea_mcp/oauth/callback.py

@@ -16,14 +16,21 @@ import html
 from typing import Any
 from urllib.parse import urlsplit
 
+from mcp.server.transport_security import RequestBodyLimitMiddleware
+from starlette.datastructures import Headers
 from starlette.requests import Request
 from starlette.responses import HTMLResponse, RedirectResponse, Response
-from starlette.routing import Route
+from starlette.routing import Route, request_response
+from starlette.types import ASGIApp, Receive, Scope, Send
 
 from aegis_gitea_mcp.oauth.provider import CONSENT_PATH, GiteaOAuthProvider
 from aegis_gitea_mcp.oauth.upstream import CALLBACK_PATH
 
 COOKIE_TTL_SECONDS = 600
+#: Largest request body the consent form may send (three short fields).
+MAX_CONSENT_BODY_BYTES = 16 * 1024
+_MAX_FORM_FIELDS = 8
+_MAX_FORM_PART_BYTES = 4096
 _INVALID_MESSAGE = (
     "This sign-in request is invalid or has expired. Start the sign-in again from your MCP client."
 )
@@ -100,6 +107,28 @@ def _text(value: Any, limit: int = 200) -> str:
     return value[:limit] if isinstance(value, str) else ""
 
 
+class _UrlencodedFormOnly:
+    """ASGI guard for the consent POST, applied before any body is read.
+
+    The route is reachable without authentication, so it must not parse
+    arbitrary bodies: anything but ``application/x-www-form-urlencoded`` gets
+    415 without reading a byte, and the body is capped at
+    :data:`MAX_CONSENT_BODY_BYTES` (413 beyond it).
+    """
+
+    def __init__(self, app: ASGIApp) -> None:
+        self._limited = RequestBodyLimitMiddleware(app, MAX_CONSENT_BODY_BYTES)
+
+    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
+        if scope["type"] == "http":
+            content_type = Headers(scope=scope).get("content-type", "")
+            if content_type.split(";")[0].strip().lower() != "application/x-www-form-urlencoded":
+                response = Response("Unsupported media type", status_code=415)
+                await response(scope, receive, send)
+                return
+        await self._limited(scope, receive, send)
+
+
 def build_browser_routes(provider: GiteaOAuthProvider) -> list[Route]:
     """Return the consent and callback routes for ``provider``."""
     cookies = _Cookies(provider.base_url)
@@ -138,7 +167,12 @@ def build_browser_routes(provider: GiteaOAuthProvider) -> list[Route]:
         origin = request.headers.get("origin")
         if origin is not None and origin != expected_origin:
             return _invalid()
-        form = await request.form()
+        try:
+            form = await request.form(
+                max_files=0, max_fields=_MAX_FORM_FIELDS, max_part_size=_MAX_FORM_PART_BYTES
+            )
+        except Exception:
+            return _invalid()
         decision = provider.finish_consent(
             _text(form.get("request")),
             _text(form.get("csrf")),
@@ -168,6 +202,12 @@ def build_browser_routes(provider: GiteaOAuthProvider) -> list[Route]:
 
     return [
         Route(CONSENT_PATH, consent_page, methods=["GET"]),
-        Route(CONSENT_PATH, consent_submit, methods=["POST"]),
+        Route(
-        Route(CALLBACK_PATH, callback, methods=["GET"]),
+            CONSENT_PATH, _UrlencodedFormOnly(request_response(consent_submit)), methods=["POST"]
+        ),
+        Route(
+            CALLBACK_PATH,
+            RequestBodyLimitMiddleware(request_response(callback), MAX_CONSENT_BODY_BYTES),
+            methods=["GET"],
+        ),
     ]

tests/security/helpers.py

@@ -0,0 +1,77 @@
+"""Helpers shared by the security tests."""
+
+from __future__ import annotations
+
+import json
+from collections.abc import Iterable
+from dataclasses import dataclass, field
+from typing import Any
+
+from starlette.types import ASGIApp, Message, Scope
+
+
+@dataclass
+class AsgiResult:
+    """What one raw ASGI call produced and how much of the request body it consumed."""
+
+    status: int = 0
+    body: bytes = b""
+    headers: dict[str, str] = field(default_factory=dict)
+    bytes_read: int = 0
+    receive_calls: int = 0
+
+    def json(self) -> Any:
+        return json.loads(self.body)
+
+
+async def asgi_request(
+    app: ASGIApp,
+    method: str,
+    path: str,
+    *,
+    headers: dict[str, str] | None = None,
+    chunks: Iterable[bytes] = (),
+    host: str = "mcp.example.com",
+    scheme: str = "https",
+    client: tuple[str, int] = ("198.51.100.7", 4000),
+) -> AsgiResult:
+    """Drive ``app`` with a counting ``receive`` so tests can assert how much body was read."""
+    result = AsgiResult()
+    pending = list(chunks)
+    raw_headers = [(b"host", host.encode())]
+    for name, value in (headers or {}).items():
+        raw_headers.append((name.lower().encode(), value.encode()))
+    scope: Scope = {
+        "type": "http",
+        "asgi": {"version": "3.0"},
+        "http_version": "1.1",
+        "method": method,
+        "scheme": scheme,
+        "path": path,
+        "raw_path": path.encode(),
+        "query_string": b"",
+        "root_path": "",
+        "headers": raw_headers,
+        "client": client,
+        "server": (host, 443),
+    }
+
+    async def receive() -> Message:
+        result.receive_calls += 1
+        if not pending:
+            return {"type": "http.disconnect"}
+        chunk = pending.pop(0)
+        result.bytes_read += len(chunk)
+        return {"type": "http.request", "body": chunk, "more_body": bool(pending)}
+
+    async def send(message: Message) -> None:
+        if message["type"] == "http.response.start":
+            result.status = message["status"]
+            result.headers = {
+                key.decode().lower(): value.decode() for key, value in message["headers"]
+            }
+        elif message["type"] == "http.response.body":
+            result.body += message.get("body", b"")
+
+    await app(scope, receive, send)
+    return result

tests/security/test_oauth_input_bounds.py

@@ -0,0 +1,89 @@
+"""Input bounds of the unauthenticated OAuth routes (SEC-01, SEC-09)."""
+
+from __future__ import annotations
+
+import pytest
+
+from aegis_gitea_mcp.oauth.callback import MAX_CONSENT_BODY_BYTES
+from tests.security.helpers import asgi_request
+from tests.test_oauth_helpers import BASE, build_oauth_app
+
+CHUNK = 64 * 1024
+
+
+@pytest.fixture(autouse=True)
+def _env(mock_env: None) -> None:
+    """The audit logger and the rate limiter read the process-wide settings."""
+
+
+def chunks_of(total: int) -> list[bytes]:
+    return [b"a" * CHUNK for _ in range(total // CHUNK)]
+
+
+async def test_consent_post_over_the_cap_is_413_without_reading_the_body() -> None:
+    app = build_oauth_app().app
+    result = await asgi_request(
+        app,
+        "POST",
+        "/oauth/consent",
+        headers={
+            "content-type": "application/x-www-form-urlencoded",
+            "content-length": str(1024 * 1024),
+            "origin": BASE,
+        },
+        chunks=chunks_of(1024 * 1024),
+    )
+    assert result.status == 413
+    assert result.bytes_read == 0
+
+
+async def test_consent_post_without_declared_length_stops_reading_at_the_cap() -> None:
+    app = build_oauth_app().app
+    result = await asgi_request(
+        app,
+        "POST",
+        "/oauth/consent",
+        headers={"content-type": "application/x-www-form-urlencoded", "origin": BASE},
+        chunks=chunks_of(1024 * 1024),
+    )
+    assert result.status == 413
+    assert result.bytes_read <= MAX_CONSENT_BODY_BYTES + CHUNK
+
+
+async def test_consent_multipart_is_415_without_reading_any_body() -> None:
+    app = build_oauth_app().app
+    result = await asgi_request(
+        app,
+        "POST",
+        "/oauth/consent",
+        headers={"content-type": "multipart/form-data; boundary=x", "origin": BASE},
+        chunks=chunks_of(512 * 1024),
+    )
+    assert result.status == 415
+    assert result.bytes_read == 0
+
+
+async def test_consent_json_body_is_415() -> None:
+    app = build_oauth_app().app
+    result = await asgi_request(
+        app,
+        "POST",
+        "/oauth/consent",
+        headers={"content-type": "application/json", "origin": BASE},
+        chunks=[b"{}"],
+    )
+    assert result.status == 415
+
+
+async def test_small_urlencoded_consent_post_still_reaches_the_handler() -> None:
+    app = build_oauth_app().app
+    result = await asgi_request(
+        app,
+        "POST",
+        "/oauth/consent",
+        headers={"content-type": "application/x-www-form-urlencoded", "origin": BASE},
+        chunks=[b"request=unknown&csrf=x&decision=approve"],
+    )
+    # Reaches the handler, which answers its fixed 400 page for an unknown request.
+    assert result.status == 400
+    assert b"invalid or has expired" in result.body