diff --git a/README.md b/README.md index 4234512..5abe4d0 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,25 @@ python -m photo_pipeline serve # start the API + static review UI (127.0.0.1 Configuration comes from `PHOTO_PIPELINE_*` environment variables (see `photo_pipeline/config.py`); secrets are referenced, never logged. +### API access (US07-02) + +The app listens on loopback, so its attacker is another page in the same browser. +Every `/api/v1` route except `health/live`, `health/ready`, and `session` requires +the application session, and every mutation requires its CSRF token as well: + +```bash +BASE=http://127.0.0.1:8000 +TOKEN=$(curl -sc /tmp/pp.jar $BASE/api/v1/session | python -c 'import json,sys; print(json.load(sys.stdin)["csrf_token"])') +curl -sb /tmp/pp.jar -H "X-CSRF-Token: $TOKEN" -X POST $BASE/api/v1/albums/proposals -d '{}' -H 'Content-Type: application/json' +``` + +The session is per server process — restarting `serve` invalidates it, and the +browser client re-bootstraps by itself. Requests are also refused when the `Host` is +not a loopback name (DNS rebinding), when `Origin` is any other origin, when +`Sec-Fetch-Site` says the request came from another site (an `` pointed at a +thumbnail), or when the body exceeds `PHOTO_PIPELINE_MAX_REQUEST_BYTES`. There is no +CORS middleware at all, so no other origin can read a response. + ## Testing One offline command runs the whole suite (unit, integration, and browser diff --git a/frontend/js/api.js b/frontend/js/api.js index f870d2b..05e11eb 100644 --- a/frontend/js/api.js +++ b/frontend/js/api.js @@ -2,14 +2,43 @@ // cancellation. Every method accepts an optional { signal } from cancellable(). export const BASE = "/api/v1"; +// The API refuses every request without the session cookie, and every mutation +// without this token echoed back. The token is readable only same-origin, which is +// what makes it proof that the caller is this app and not another page. +let csrfToken = null; + +async function session() { + if (csrfToken === null) { + const response = await fetch(BASE + "/session", { credentials: "same-origin" }); + const body = await response.json().catch(() => null); + csrfToken = (body && body.csrf_token) || null; + } + return csrfToken || ""; +} + +async function send(path, { signal, ...options }) { + return fetch(BASE + path, { + credentials: "same-origin", + signal, + ...options, + headers: { + "Content-Type": "application/json", + "X-CSRF-Token": await session(), + ...(options.headers || {}), + }, + }); +} + async function request(path, { signal, ...options } = {}) { let response; try { - response = await fetch(BASE + path, { - headers: { "Content-Type": "application/json" }, - signal, - ...options, - }); + response = await send(path, { signal, ...options }); + // A restarted server issues a new session; re-bootstrap once rather than + // stranding an open tab on 401. + if (response.status === 401) { + csrfToken = null; + response = await send(path, { signal, ...options }); + } } catch (error) { // A caller-cancelled fetch is not a failure; tag it so views can ignore it. if (error.name === "AbortError") { diff --git a/frontend/js/tests/unit.js b/frontend/js/tests/unit.js index 1293797..206b71e 100644 --- a/frontend/js/tests/unit.js +++ b/frontend/js/tests/unit.js @@ -29,6 +29,10 @@ function jsonResponse(status, body) { const tick = (ms = 0) => new Promise((r) => setTimeout(r, ms)); async function run() { + // The client fetches its CSRF token once, lazily (US07-02). Do that against the + // real server first, so the stubbed fetch below only ever sees the call under test. + await api.workflow().catch(() => {}); + // ── store ──────────────────────────────────────────────────────────────── { const store = createStore({ n: 0 }); diff --git a/photo_pipeline/api/app.py b/photo_pipeline/api/app.py index 9329fa1..79ac1a7 100644 --- a/photo_pipeline/api/app.py +++ b/photo_pipeline/api/app.py @@ -8,11 +8,15 @@ and exposes the versioned ``/api/v1`` surface; US01-02 ships only health. from __future__ import annotations +import logging from contextlib import asynccontextmanager from pathlib import Path -from fastapi import FastAPI +from fastapi import FastAPI, Request +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse from fastapi.staticfiles import StaticFiles +from starlette.exceptions import HTTPException as StarletteHTTPException from photo_pipeline.api.routes import ( albums, @@ -25,10 +29,12 @@ from photo_pipeline.api.routes import ( library, renames, safety, + session as session_routes, thumbnails, uploads, workflow, ) +from photo_pipeline.api.security import DEFAULT_HEADERS, SecurityMiddleware, Session # Registers the safety_score / analysis job handlers on import. import photo_pipeline.jobs.domain_handlers # noqa: F401 @@ -39,6 +45,41 @@ from photo_pipeline.services.upload_batches import UploadBatchService FRONTEND_DIR = Path(__file__).resolve().parents[2] / "frontend" +log = logging.getLogger(__name__) + + +def _envelope(status: int, code: str, message: str) -> JSONResponse: + return JSONResponse( + status_code=status, + content={"error": {"code": code, "message": message}}, + headers=DEFAULT_HEADERS, + ) + + +def _install_error_handlers(app: FastAPI) -> None: + """One JSON error envelope everywhere, and nothing behind it. + + An unhandled exception carries the library's absolute paths, SQL, and sometimes + a credential in its text; the client gets a code, the operator gets the traceback + in the server log (US07-02). + """ + + @app.exception_handler(StarletteHTTPException) + async def _http_error(request: Request, exc: StarletteHTTPException): + return _envelope(exc.status_code, "http_error", str(exc.detail)) + + @app.exception_handler(RequestValidationError) + async def _validation_error(request: Request, exc: RequestValidationError): + # Field locations only: the echoed input can be the caller's own data, but it + # is also what ends up in shared logs and screenshots. + fields = sorted(".".join(str(part) for part in error["loc"]) for error in exc.errors()) + return _envelope(422, "invalid_request", f"invalid request fields: {', '.join(fields)}") + + @app.exception_handler(Exception) + async def _unhandled(request: Request, exc: Exception): + log.exception("unhandled error serving %s", request.url.path) + return _envelope(500, "internal_error", "internal error") + def create_app(config: Config | None = None) -> FastAPI: config = config or Config.from_env() @@ -62,6 +103,12 @@ def create_app(config: Config | None = None) -> FastAPI: app.state.engine = None app = FastAPI(title="Photo Pipeline", version="0.1.0", lifespan=lifespan) + # 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.add_middleware(SecurityMiddleware, session=app.state.session, config=config) + _install_error_handlers(app) + app.include_router(session_routes.router, prefix="/api/v1") app.include_router(health.router, prefix="/api/v1") app.include_router(inventory.router, prefix="/api/v1") app.include_router(duplicates.router, prefix="/api/v1") diff --git a/photo_pipeline/api/routes/analysis.py b/photo_pipeline/api/routes/analysis.py index 03d12d7..4e365d8 100644 --- a/photo_pipeline/api/routes/analysis.py +++ b/photo_pipeline/api/routes/analysis.py @@ -18,7 +18,10 @@ router = APIRouter(tags=["analysis"]) def _service(request: Request) -> AnalysisService: - return AnalysisService(request.app.state.session_factory) + return AnalysisService( + request.app.state.session_factory, + library_roots=tuple(request.app.state.config.library_roots), + ) def _error(status: int, code: str, message: str) -> JSONResponse: diff --git a/photo_pipeline/api/routes/session.py b/photo_pipeline/api/routes/session.py new file mode 100644 index 0000000..9d95456 --- /dev/null +++ b/photo_pipeline/api/routes/session.py @@ -0,0 +1,30 @@ +"""Session bootstrap: the one endpoint reachable without a session (US07-02). + +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. +""" + +from __future__ import annotations + +from fastapi import APIRouter, Request +from fastapi.responses import JSONResponse + +from photo_pipeline.api.security import SESSION_COOKIE + +router = APIRouter(tags=["session"]) + + +@router.get("/session") +def start_session(request: Request) -> JSONResponse: + session = request.app.state.session + response = JSONResponse({"csrf_token": session.csrf_token}) + response.set_cookie( + SESSION_COOKIE, + session.id, + httponly=True, + samesite="strict", + path="/", + ) + return response diff --git a/photo_pipeline/api/routes/thumbnails.py b/photo_pipeline/api/routes/thumbnails.py index fa1aa76..e221dd8 100644 --- a/photo_pipeline/api/routes/thumbnails.py +++ b/photo_pipeline/api/routes/thumbnails.py @@ -31,5 +31,7 @@ def get_thumbnail(asset_id: str, request: Request, size: int = Query(512)): return FileResponse( path, media_type="image/webp", - headers={"Cache-Control": "public, max-age=31536000, immutable"}, + # private: the URL is versioned and immutable, but these bytes are the user's + # photos and must never sit in a shared cache (US07-02). + headers={"Cache-Control": "private, max-age=31536000, immutable"}, ) diff --git a/photo_pipeline/api/security.py b/photo_pipeline/api/security.py new file mode 100644 index 0000000..908273b --- /dev/null +++ b/photo_pipeline/api/security.py @@ -0,0 +1,192 @@ +"""Local-web attack surface: session, CSRF, Origin/Host checks, default headers. + +The app binds to 127.0.0.1, so the attacker is not a remote client but another page +in the user's browser (concept §15, "Local web attack"): any site can issue requests +to ``http://127.0.0.1:8000`` and can embed ```` against media endpoints. +The defenses stack, because each one alone has a hole: + +* **Host** must be a loopback name — a DNS rebinding host that resolves to 127.0.0.1 + passes the browser's origin rules but not this check. +* **Origin**, when the browser sends one, must be this exact origin (scheme, host, + port). There is no CORS middleware at all, so a foreign page can never *read* a + response even if it manages to send a request. +* **Sec-Fetch-Site** rejects cross-site loads that carry no Origin, which is what an + ```` or ``