From c9c5939f0abb56c2f8b03741c669737bbc8dc757 Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 18:34:48 +0000 Subject: [PATCH] security: bound and redact inbound requests (#327) --- README.md | 8 ++ src/main.py | 19 +++++ src/request_boundary.py | 93 +++++++++++++++++++++ tests/test_request_boundary.py | 147 +++++++++++++++++++++++++++++++++ 4 files changed, 267 insertions(+) create mode 100644 src/request_boundary.py create mode 100644 tests/test_request_boundary.py diff --git a/README.md b/README.md index 90aec5c..0e411d3 100644 --- a/README.md +++ b/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 diff --git a/src/main.py b/src/main.py index 1fca8af..68bd21a 100644 --- a/src/main.py +++ b/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.""" diff --git a/src/request_boundary.py b/src/request_boundary.py new file mode 100644 index 0000000..34cd81f --- /dev/null +++ b/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}) diff --git a/tests/test_request_boundary.py b/tests/test_request_boundary.py new file mode 100644 index 0000000..f4d65e4 --- /dev/null +++ b/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 -- 2.43.0