US08-01: Make the Trust Boundary Configurable and Authenticated (#96)

photo_pipeline/api/app.py

@@ -35,7 +35,13 @@ from photo_pipeline.api.routes import (
     uploads,
     workflow,
 )
-from photo_pipeline.api.security import DEFAULT_HEADERS, SecurityMiddleware, Session
+from photo_pipeline.api.security import (
+    DEFAULT_HEADERS,
+    FailureLimiter,
+    SecurityMiddleware,
+    Session,
+    trust_refusal,
+)
 
 # Registers the safety_score / analysis job handlers on import.
 import photo_pipeline.jobs.domain_handlers  # noqa: F401
@@ -84,9 +90,16 @@ def _install_error_handlers(app: FastAPI) -> None:
         return _envelope(500, "internal_error", "internal error")
 
 
+class ConfigurationRefused(RuntimeError):
+    """The configuration would serve the library to callers it cannot authenticate."""
+
+
 def create_app(config: Config | None = None) -> FastAPI:
     config = config or Config.from_env()
     configure_logging(config.log_level, config.log_format)
+    # Before anything is built, let alone bound to a port (US08-01).
+    if (why := trust_refusal(config)) is not None:
+        raise ConfigurationRefused(why)
 
     @asynccontextmanager
     async def lifespan(app: FastAPI):
@@ -114,6 +127,10 @@ def create_app(config: Config | None = None) -> FastAPI:
     # One session per process: the browser exchanges it for a cookie + CSRF token,
     # and every other origin is refused before a route ever runs (US07-02).
     app.state.session = Session.create()
+    app.state.access_limiter = FailureLimiter()
+    # Also set in the lifespan, but the bootstrap route reads it, and a caller can
+    # arrive before anything else has touched app.state.
+    app.state.config = config
     app.add_middleware(SecurityMiddleware, session=app.state.session, config=config)
     _install_error_handlers(app)
     app.include_router(session_routes.router, prefix="/api/v1")

photo_pipeline/api/routes/session.py

@@ -3,21 +3,52 @@
 It sets the ``HttpOnly``/``SameSite=Strict`` session cookie and returns the CSRF
 token in the body. A foreign page can call this — it just cannot read the answer,
 because the app sends no CORS headers — and the cookie it received is never attached
-to a request that foreign page initiates.
+to a request that foreign page initiated.
+
+When an access secret is configured (mandatory as soon as the app is reachable from
+another machine, US08-01) this is also the authentication gate: the secret buys the
+cookie, and every route behind it keeps asking for exactly the session and CSRF token
+it asked for before. Wrong secrets are counted, and a burst of them stops being
+answered — otherwise a proxy-exposed deployment could be guessed at indefinitely.
 """
 
 from __future__ import annotations
 
+import logging
+import secrets
+
 from fastapi import APIRouter, Request
 from fastapi.responses import JSONResponse
 
-from photo_pipeline.api.security import SESSION_COOKIE
+from photo_pipeline.api.security import ACCESS_SECRET_HEADER, SESSION_COOKIE
 
 router = APIRouter(tags=["session"])
 
+log = logging.getLogger(__name__)
+
+
+def _refuse(status: int, code: str, message: str) -> JSONResponse:
+    return JSONResponse(status_code=status, content={"error": {"code": code, "message": message}})
+
 
 @router.get("/session")
 def start_session(request: Request) -> JSONResponse:
+    config = request.app.state.config
+    secret = config.access_secret
+    if secret is not None:
+        limiter = request.app.state.access_limiter
+        if limiter.blocked():
+            return _refuse(429, "too_many_attempts", "too many failed attempts; retry later")
+        offered = request.headers.get(ACCESS_SECRET_HEADER, "")
+        if not secrets.compare_digest(offered, secret.get_secret_value()):
+            limiter.record_failure()
+            # The client address is the whole record: the offered secret, the issued
+            # session, and the request body all stay out of the log.
+            log.warning(
+                "access secret rejected", extra={"client": _client(request), "path": "/session"}
+            )
+            return _refuse(401, "access_denied", "a valid access secret is required")
+
     session = request.app.state.session
     response = JSONResponse({"csrf_token": session.csrf_token})
     response.set_cookie(
@@ -25,6 +56,13 @@ def start_session(request: Request) -> JSONResponse:
         session.id,
         httponly=True,
         samesite="strict",
+        # HTTPS outside means the cookie must never travel over a plain hop, even one
+        # this process cannot see. Loopback http keeps working unchanged.
+        secure=request.scope.get("state", {}).get("external_scheme") == "https",
         path="/",
     )
     return response
+
+
+def _client(request: Request) -> str:
+    return request.client.host if request.client else "unknown"

photo_pipeline/api/security.py

@@ -19,6 +19,14 @@ The defenses stack, because each one alone has a hole:
   only in the bootstrap response body, which a foreign page cannot read (no CORS) —
   so possessing it proves the caller is same-origin.
 
+Behind a reverse proxy (US08-01) the same stack holds with two substitutions: the
+allowed host set comes from configuration instead of being the loopback names, and
+the host/scheme the policy judges is the *external* one, which is only read from
+``X-Forwarded-*`` when the request actually arrived from a configured proxy. The
+loopback check was standing in for authentication, so naming a non-loopback host
+also makes an access secret mandatory — ``trust_refusal`` refuses to start without
+one, and the secret is what the bootstrap endpoint trades for the session cookie.
+
 ``evaluate`` is a pure function over the request metadata: the whole policy is one
 table that a unit test can enumerate, and the middleware only applies its verdict.
 """
@@ -26,6 +34,7 @@ table that a unit test can enumerate, and the middleware only applies its verdic
 from __future__ import annotations
 
 import secrets
+import time
 from collections.abc import Mapping
 from dataclasses import dataclass
 from urllib.parse import urlsplit
@@ -35,6 +44,7 @@ from starlette.responses import JSONResponse
 
 SESSION_COOKIE = "pp_session"
 CSRF_HEADER = "x-csrf-token"
+ACCESS_SECRET_HEADER = "x-access-secret"
 API_PREFIX = "/api/v1"
 SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
 # Reachable without a session: liveness/readiness (an orchestrator has no cookie)
@@ -96,6 +106,28 @@ def split_host(value: str) -> tuple[str, str]:
     return host, port
 
 
+def external_view(
+    *,
+    client: str | None,
+    headers: Mapping[str, str],
+    scheme: str,
+    trusted_proxies: frozenset[str],
+) -> tuple[str, str]:
+    """The ``(scheme, host)`` the caller used, as opposed to the one this hop saw.
+
+    Forwarded headers are a client-supplied claim. Believing them from anyone lets a
+    request declare its own origin — and origin is half of this module's evidence —
+    so they count only when the connection came from a configured proxy.
+    """
+    host = headers.get("host", "")
+    if client is None or client not in trusted_proxies:
+        return scheme, host
+    # A chain appends: the first entry is what the original client asked for.
+    forwarded_proto = headers.get("x-forwarded-proto", "").split(",")[0].strip().lower()
+    forwarded_host = headers.get("x-forwarded-host", "").split(",")[0].strip()
+    return forwarded_proto or scheme, forwarded_host or host
+
+
 def evaluate(
     *,
     method: str,
@@ -103,20 +135,26 @@ def evaluate(
     headers: Mapping[str, str],
     session: Session,
     allowed_hosts: frozenset[str] = LOOPBACK_HOSTS,
+    scheme: str = "http",
     max_request_bytes: int,
 ) -> Refusal | None:
-    """Why this request must be refused, or ``None`` when it may proceed."""
+    """Why this request must be refused, or ``None`` when it may proceed.
+
+    ``headers["host"]`` and ``scheme`` are the external ones (see ``external_view``);
+    the allowed origins are the allowed hosts under that scheme and port, so there is
+    no second list that can drift away from the first.
+    """
     host_header = headers.get("host", "")
     host, port = split_host(host_header)
     if host.lower() not in allowed_hosts:
-        return Refusal(403, "host_not_allowed", "request host is not a local address")
+        return Refusal(403, "host_not_allowed", "request host is not an allowed address")
 
     origin = headers.get("origin")
     if origin is not None and origin != "":
         parts = urlsplit(origin)
         origin_host, origin_port = split_host(parts.netloc)
         if (
-            parts.scheme not in ("http", "https")
+            parts.scheme != scheme
             or origin_host.lower() not in allowed_hosts
             or origin_port != port
         ):
@@ -142,6 +180,52 @@ def evaluate(
     return None
 
 
+def exposed_hosts(config) -> list[str]:
+    """Configured names by which this application is reachable from another machine."""
+    names = {str(config.host).lower()}
+    names.update(split_host(name)[0].lower() for name in config.allowed_hosts)
+    return sorted(names - LOOPBACK_HOSTS)
+
+
+def trust_refusal(config) -> str | None:
+    """Why this configuration must not serve at all, or ``None``.
+
+    Reaching the app used to prove ownership of it. The moment a configuration makes
+    it reachable from elsewhere that stops being true, so serving without a secret
+    would publish the library — refuse at startup rather than at the first request,
+    when the operator is no longer watching (US08-01).
+    """
+    exposed = exposed_hosts(config)
+    if exposed and config.access_secret is None:
+        return (
+            f"refusing to serve: {', '.join(exposed)} is reachable from outside this "
+            "machine, so PHOTO_PIPELINE_ACCESS_SECRET must be set"
+        )
+    return None
+
+
+class FailureLimiter:
+    """Bounded failed access-secret attempts, so the secret cannot be guessed online.
+
+    ponytail: one counter for the whole process rather than per client address —
+    behind a proxy every attempt arrives from the same address anyway. Per-caller
+    buckets if the app is ever exposed without one.
+    """
+
+    def __init__(self, limit: int = 5, window: float = 60.0) -> None:
+        self.limit = limit
+        self.window = window
+        self._failures: list[float] = []
+
+    def blocked(self) -> bool:
+        now = time.monotonic()
+        self._failures = [at for at in self._failures if now - at < self.window]
+        return len(self._failures) >= self.limit
+
+    def record_failure(self) -> None:
+        self._failures.append(time.monotonic())
+
+
 class SecurityMiddleware:
     """Pure-ASGI so the SSE stream keeps streaming (BaseHTTPMiddleware buffers)."""
 
@@ -150,7 +234,12 @@ class SecurityMiddleware:
         self.session = session
         self.config = config
         self.max_request_bytes = config.max_request_bytes
-        self.allowed_hosts = frozenset(LOOPBACK_HOSTS | {str(config.host).lower()})
+        self.allowed_hosts = frozenset(
+            LOOPBACK_HOSTS
+            | {str(config.host).lower()}
+            | {split_host(name)[0].lower() for name in config.allowed_hosts}
+        )
+        self.trusted_proxies = frozenset(config.trusted_proxies)
 
     async def __call__(self, scope, receive, send) -> None:
         if scope["type"] != "http":
@@ -161,12 +250,22 @@ class SecurityMiddleware:
         # policy never has to parse a Cookie header.
         lookup = dict(headers)
         lookup["cookie-session"] = _cookie(headers.get("cookie", ""), SESSION_COOKIE)
+        client = scope.get("client")
+        scheme, lookup["host"] = external_view(
+            client=client[0] if client else None,
+            headers=headers,
+            scheme=scope.get("scheme", "http"),
+            trusted_proxies=self.trusted_proxies,
+        )
+        # What the session cookie's Secure flag is decided from, one hop later.
+        scope.setdefault("state", {})["external_scheme"] = scheme
         refusal = evaluate(
             method=scope.get("method", "GET"),
             path=scope.get("path", "/"),
             headers=lookup,
             session=self.session,
             allowed_hosts=self.allowed_hosts,
+            scheme=scheme,
             max_request_bytes=self.max_request_bytes,
         )
         if refusal is None: