security: bound and redact inbound requests (#327)

README.md

@@ -90,6 +90,14 @@ shared writable storage. `X-Forwarded-For` is ignored unless the immediate peer
 inside `STACKCHAIN_TRUSTED_PROXY_CIDRS`; list only networks you operate. Without
 that setting, a reverse proxy is safely treated as one shared source.
 
+Inbound API mutations are admitted through a body-size boundary before FastAPI
+parses JSON: sign-in is capped at 16 KiB and other `POST`, `PUT`, and `PATCH`
+requests under `/api/v1/` are capped at 64 KiB. Both declared and streamed bodies
+are counted. Oversized requests receive a compact, non-cacheable HTTP 413 response;
+validation errors expose field locations and messages but never echo submitted
+values or validation context. Preserve these limits at the reverse proxy or enforce
+equal or tighter upstream limits.
+
 Each signed cookie includes an opaque session identifier whose hash and expiry are
 kept in the SQLite session registry. Keep that registry on persistent, writable
 storage shared by all dashboard workers. Operators name a device at sign-in and can

src/main.py

@@ -11,6 +11,7 @@ from typing import Any, Literal
 from urllib.parse import urlencode
 
 from fastapi import FastAPI, Header, HTTPException, Path as PathParam, Query, Request, Response
+from fastapi.exceptions import RequestValidationError
 from fastapi.responses import JSONResponse, RedirectResponse
 from fastapi.staticfiles import StaticFiles
 from pydantic import BaseModel, Field, PositiveInt, field_validator, model_validator
@@ -31,6 +32,7 @@ from src.gitea_proxy import (
 from src.idempotency import IdempotencyLedger, IdempotencyLedgerBusy
 from src.login_attempt_store import LoginAttemptStore, LoginAttemptStoreError, client_source
 from src.models import Issue, Milestone, PullRequest, Repo, User
+from src.request_boundary import RequestBodyLimitMiddleware, request_body_limit
 from src.suggestion_engine import compute
 from src.views import router as frontend_router
 
@@ -60,6 +62,7 @@ async def lifespan(_app: FastAPI):
 
 
 app = FastAPI(title="Stackchain Dashboard", lifespan=lifespan)
+app.add_middleware(RequestBodyLimitMiddleware, limit_for=request_body_limit)
 CONTEXT_TIMEOUT_SECONDS = 5.0
 EVENT_STREAM_TIMEOUT_SECONDS = 5.0
 READINESS_TIMEOUT_SECONDS = 5.0
@@ -601,6 +604,22 @@ CONTENT_SECURITY_POLICY = "; ".join(
 )
 
 
+@app.exception_handler(RequestValidationError)
+async def redact_request_validation_error(
+    _request: Request, error: RequestValidationError
+) -> JSONResponse:
+    """Return useful validation locations without reflecting submitted values."""
+    details = [
+        {key: item[key] for key in ("type", "loc", "msg") if key in item}
+        for item in error.errors()
+    ]
+    return JSONResponse(
+        {"detail": details},
+        status_code=422,
+        headers={"Cache-Control": "no-store"},
+    )
+
+
 @app.middleware("http")
 async def enforce_browser_security_boundary(request: Request, call_next):
     """Apply one browser trust boundary, including to auth short-circuits."""

src/request_boundary.py

@@ -0,0 +1,93 @@
+from collections.abc import Callable
+
+from starlette.types import ASGIApp, Message, Receive, Scope, Send
+
+
+SESSION_BODY_LIMIT = 16 * 1024
+API_MUTATION_BODY_LIMIT = 64 * 1024
+MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH"})
+
+
+def request_body_limit(method: str, path: str) -> int | None:
+    """Return the admission limit for request bodies that FastAPI will parse."""
+    normalized_method = method.upper()
+    if normalized_method == "POST" and path == "/api/v1/session":
+        return SESSION_BODY_LIMIT
+    if normalized_method in MUTATION_METHODS and path.startswith("/api/v1/"):
+        return API_MUTATION_BODY_LIMIT
+    return None
+
+
+class RequestBodyLimitMiddleware:
+    """Reject oversized API bodies before framework parsing or endpoint work."""
+
+    def __init__(
+        self,
+        app: ASGIApp,
+        limit_for: Callable[[str, str], int | None] = request_body_limit,
+    ) -> None:
+        self.app = app
+        self.limit_for = limit_for
+
+    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
+        if scope["type"] != "http":
+            await self.app(scope, receive, send)
+            return
+
+        path = scope.get("path", "")
+        root_path = scope.get("root_path", "").rstrip("/")
+        if root_path and (path == root_path or path.startswith(root_path + "/")):
+            path = path[len(root_path) :] or "/"
+        limit = self.limit_for(scope.get("method", "GET"), path)
+        if limit is None:
+            await self.app(scope, receive, send)
+            return
+
+        declared_length = self._content_length(scope)
+        if declared_length is not None and declared_length > limit:
+            await self._reject(send)
+            return
+
+        messages: list[Message] = []
+        received = 0
+        while True:
+            message = await receive()
+            messages.append(message)
+            if message["type"] != "http.request":
+                break
+            received += len(message.get("body", b""))
+            if received > limit:
+                await self._reject(send)
+                return
+            if not message.get("more_body", False):
+                break
+
+        async def replay() -> Message:
+            if messages:
+                return messages.pop(0)
+            return {"type": "http.request", "body": b"", "more_body": False}
+
+        await self.app(scope, replay, send)
+
+    @staticmethod
+    def _content_length(scope: Scope) -> int | None:
+        for name, value in scope.get("headers", []):
+            if name.lower() != b"content-length":
+                continue
+            try:
+                parsed = int(value)
+            except ValueError:
+                return None
+            return max(0, parsed)
+        return None
+
+    @staticmethod
+    async def _reject(send: Send) -> None:
+        body = b'{"detail":"Request body too large"}'
+        headers = [
+            (b"content-type", b"application/json"),
+            (b"content-length", str(len(body)).encode("ascii")),
+            (b"cache-control", b"no-store"),
+        ]
+        await send({"type": "http.response.start", "status": 413, "headers": headers})
+        await send({"type": "http.response.body", "body": body})

tests/test_request_boundary.py

@@ -0,0 +1,147 @@
+import httpx
+import pytest
+
+from src import main
+from src.request_boundary import RequestBodyLimitMiddleware
+
+
+@pytest.fixture
+def access_control(monkeypatch, tmp_path):
+    monkeypatch.setenv("STACKCHAIN_DASHBOARD_AUTH_MODE", "operator")
+    monkeypatch.setenv(
+        "STACKCHAIN_DASHBOARD_ACCESS_TOKEN", "correct horse battery staple"
+    )
+    monkeypatch.setenv(
+        "STACKCHAIN_DASHBOARD_SESSION_SECRET",
+        "a-separate-session-signing-secret-with-enough-entropy",
+    )
+    monkeypatch.setenv("STACKCHAIN_SESSION_DB", str(tmp_path / "sessions.sqlite3"))
+    monkeypatch.setenv(
+        "STACKCHAIN_LOGIN_ATTEMPT_DB", str(tmp_path / "login-attempts.sqlite3")
+    )
+
+
+@pytest.mark.anyio
+async def test_declared_oversized_sign_in_is_rejected_before_throttle(
+    access_control, monkeypatch
+):
+    throttle_touched = False
+
+    def attempt_store():
+        nonlocal throttle_touched
+        throttle_touched = True
+        raise AssertionError("oversized request reached sign-in throttling")
+
+    monkeypatch.setattr(main, "_login_attempt_store", attempt_store)
+    body = b'{"access_token":"' + (b"x" * 20_000) + b'"}'
+    transport = httpx.ASGITransport(app=main.app)
+
+    async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
+        response = await client.post(
+            "/api/v1/session",
+            content=body,
+            headers={"Content-Type": "application/json"},
+        )
+
+    assert response.status_code == 413
+    assert response.json() == {"detail": "Request body too large"}
+    assert response.headers["cache-control"] == "no-store"
+    assert response.headers["x-content-type-options"] == "nosniff"
+    assert len(response.content) < 128
+    assert throttle_touched is False
+
+
+@pytest.mark.anyio
+async def test_streamed_oversized_sign_in_without_content_length_is_rejected(
+    access_control, monkeypatch
+):
+    throttle_touched = False
+
+    def attempt_store():
+        nonlocal throttle_touched
+        throttle_touched = True
+        raise AssertionError("oversized stream reached sign-in throttling")
+
+    async def chunks():
+        yield b'{"access_token":"'
+        yield b"x" * 10_000
+        yield b"y" * 10_000
+        yield b'"}'
+
+    monkeypatch.setattr(main, "_login_attempt_store", attempt_store)
+    request = httpx.Request(
+        "POST",
+        "https://test/api/v1/session",
+        content=chunks(),
+        headers={"Content-Type": "application/json"},
+    )
+    request.headers.pop("transfer-encoding", None)
+    transport = httpx.ASGITransport(app=main.app)
+
+    response = await transport.handle_async_request(request)
+    await response.aread()
+
+    assert response.status_code == 413
+    assert response.json() == {"detail": "Request body too large"}
+    assert response.headers["cache-control"] == "no-store"
+    assert throttle_touched is False
+
+
+@pytest.mark.anyio
+async def test_validation_error_never_reflects_submitted_values(access_control):
+    canary = "DO-NOT-REFLECT-ACCESS-TOKEN-CANARY"
+    transport = httpx.ASGITransport(app=main.app)
+
+    async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
+        response = await client.post(
+            "/api/v1/session",
+            json={"access_token": canary, "device_label": canary * 3},
+        )
+
+    assert response.status_code == 422
+    assert canary not in response.text
+    details = response.json()["detail"]
+    assert details
+    assert all(set(item) <= {"type", "loc", "msg"} for item in details)
+
+
+def test_request_limits_are_route_specific_and_cover_api_mutations():
+    assert main.request_body_limit("POST", "/api/v1/session") == 16 * 1024
+    assert (
+        main.request_body_limit("POST", "/api/v1/repos/stackchain/project/issues")
+        == 64 * 1024
+    )
+    assert main.request_body_limit("GET", "/api/v1/context") is None
+    assert main.request_body_limit("POST", "/unrelated") is None
+
+
+@pytest.mark.anyio
+async def test_request_limit_uses_application_path_under_domain_subpath():
+    downstream_called = False
+    sent = []
+
+    async def downstream(_scope, _receive, _send):
+        nonlocal downstream_called
+        downstream_called = True
+
+    async def receive():
+        return {"type": "http.request", "body": b"", "more_body": False}
+
+    async def send(message):
+        sent.append(message)
+
+    middleware = RequestBodyLimitMiddleware(downstream)
+    await middleware(
+        {
+            "type": "http",
+            "method": "POST",
+            "root_path": "/dashboard",
+            "path": "/api/v1/session",
+            "headers": [(b"content-length", b"20000")],
+        },
+        receive,
+        send,
+    )
+
+    assert sent[0]["status"] == 413
+    assert downstream_called is False