domverse/photoanalyzer
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity

US07-02: Harden API Authorization and Path Boundaries

Browse Source
This commit is contained in:
domverse
2026-08-16 23:06:41 +02:00
parent d143fee4d2
commit b2b996b37e
21 changed files with 1134 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
photo_pipeline/api/app.py
View File

@@ -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")

5
photo_pipeline/api/routes/analysis.py
View File

@@ -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:

30
photo_pipeline/api/routes/session.py Normal file
View File

@@ -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

4
photo_pipeline/api/routes/thumbnails.py
View File

@@ -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"},
)

192
photo_pipeline/api/security.py Normal file
View File

@@ -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 ``<img src=...>`` 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
``<img>`` or ``<script>`` against a media endpoint looks like.
* A **session cookie** (``SameSite=Strict``, ``HttpOnly``) is required by every
``/api/v1`` route except liveness/readiness and the bootstrap itself. Strict means
the browser never attaches it to a request another site initiated.
* A **CSRF token** must be echoed in a header on every mutation. It is handed out
only in the bootstrap response body, which a foreign page cannot read (no CORS) —
so possessing it proves the caller is same-origin.
``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.
"""
from __future__ import annotations
import secrets
from collections.abc import Mapping
from dataclasses import dataclass
from urllib.parse import urlsplit
from starlette.datastructures import Headers, MutableHeaders
from starlette.responses import JSONResponse
SESSION_COOKIE = "pp_session"
CSRF_HEADER = "x-csrf-token"
API_PREFIX = "/api/v1"
SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
# Reachable without a session: liveness/readiness (an orchestrator has no cookie)
# and the bootstrap that issues the session in the first place.
PUBLIC_PATHS = frozenset(
{f"{API_PREFIX}/health/live", f"{API_PREFIX}/health/ready", f"{API_PREFIX}/session"}
)
LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1", "[::1]"})
# Applied to every response. No inline script/style is used by the frontend, so the
# policy can stay strict; `frame-ancestors 'none'` and CORP keep other pages from
# embedding the app or its thumbnails.
DEFAULT_HEADERS = {
"x-content-type-options": "nosniff",
"x-frame-options": "DENY",
"referrer-policy": "no-referrer",
"cross-origin-resource-policy": "same-origin",
"cross-origin-opener-policy": "same-origin",
"content-security-policy": (
"default-src 'self'; img-src 'self' data:; style-src 'self'; script-src 'self'; "
"connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'"
),
}
@dataclass(frozen=True)
class Session:
"""One process, one session. A local app has exactly one user; a session store
would be bookkeeping without a second subject to distinguish.
ponytail: per-session rows if the app ever serves more than one operator.
"""
id: str
csrf_token: str
@classmethod
def create(cls) -> Session:
return cls(secrets.token_urlsafe(32), secrets.token_urlsafe(32))
@dataclass(frozen=True)
class Refusal:
status: int
code: str
message: str
def split_host(value: str) -> tuple[str, str]:
"""``"127.0.0.1:8000"`` -> ``("127.0.0.1", "8000")``; bracketed IPv6 aware."""
value = value.strip()
if value.startswith("["):
host, _, port = value.partition("]")
return host + "]", port.lstrip(":")
host, _, port = value.partition(":")
return host, port
def evaluate(
*,
method: str,
path: str,
headers: Mapping[str, str],
session: Session,
allowed_hosts: frozenset[str] = LOOPBACK_HOSTS,
max_request_bytes: int,
) -> Refusal | None:
"""Why this request must be refused, or ``None`` when it may proceed."""
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")
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")
or origin_host.lower() not in allowed_hosts
or origin_port != port
):
return Refusal(403, "origin_not_allowed", "request origin is not this application")
# Absent means a non-browser client; "none" is a user-initiated navigation.
fetch_site = headers.get("sec-fetch-site")
if fetch_site is not None and fetch_site not in ("same-origin", "none"):
return Refusal(403, "cross_site_blocked", "cross-site requests are not accepted")
length = headers.get("content-length")
if length and length.isdigit() and int(length) > max_request_bytes:
return Refusal(413, "payload_too_large", "request body exceeds the configured limit")
protected = path.startswith(API_PREFIX) and path not in PUBLIC_PATHS
if not protected:
return None
if headers.get("cookie-session") != session.id:
return Refusal(401, "unauthenticated", "a valid application session is required")
if method.upper() not in SAFE_METHODS and headers.get(CSRF_HEADER) != session.csrf_token:
return Refusal(403, "csrf_failed", "missing or invalid CSRF token")
return None
class SecurityMiddleware:
"""Pure-ASGI so the SSE stream keeps streaming (BaseHTTPMiddleware buffers)."""
def __init__(self, app, *, session: Session, config) -> None:
self.app = app
self.session = session
self.max_request_bytes = config.max_request_bytes
self.allowed_hosts = frozenset(LOOPBACK_HOSTS | {str(config.host).lower()})
async def __call__(self, scope, receive, send) -> None:
if scope["type"] != "http":
await self.app(scope, receive, send)
return
headers = Headers(scope=scope)
# The cookie is read here and handed to the pure policy as one value, so the
# policy never has to parse a Cookie header.
lookup = dict(headers)
lookup["cookie-session"] = _cookie(headers.get("cookie", ""), SESSION_COOKIE)
refusal = evaluate(
method=scope.get("method", "GET"),
path=scope.get("path", "/"),
headers=lookup,
session=self.session,
allowed_hosts=self.allowed_hosts,
max_request_bytes=self.max_request_bytes,
)
if refusal is not None:
response = JSONResponse(
status_code=refusal.status,
content={"error": {"code": refusal.code, "message": refusal.message}},
headers=DEFAULT_HEADERS,
)
await response(scope, receive, send)
return
async def send_with_headers(message):
if message["type"] == "http.response.start":
out = MutableHeaders(scope=message)
for name, value in DEFAULT_HEADERS.items():
out.setdefault(name, value)
await send(message)
await self.app(scope, receive, send_with_headers)
def _cookie(header: str, name: str) -> str:
for part in header.split(";"):
key, _, value = part.strip().partition("=")
if key == name:
return value
return ""

4
photo_pipeline/config.py
View File

@@ -30,6 +30,10 @@ class Config(BaseModel):
log_level: str = "INFO"
log_format: str = "json" # "json" or "text"
# Largest request body the API accepts. Every endpoint takes small JSON commands;
# anything larger is a mistake or an attempt to exhaust memory (US07-02).
max_request_bytes: int = 1_048_576
# Library boundary for path validation (os.pathsep-separated in the env var).
library_roots: tuple[Path, ...] = ()
thumbnail_cache_quota_bytes: int = 500_000_000

3
photo_pipeline/jobs/domain_handlers.py
View File

@@ -38,7 +38,8 @@ def _safety_score_item(asset_id: str, ctx: JobContext) -> None:
def _analysis_item(asset_id: str, ctx: JobContext) -> None:
from photo_pipeline.services.analysis import AnalysisService
AnalysisService(ctx.session_factory).run([asset_id])
roots = tuple(getattr(ctx.config, "library_roots", ()) or ())
AnalysisService(ctx.session_factory, library_roots=roots).run([asset_id])
def _upload_batch_item(batch_id: str, ctx: JobContext) -> None:

23
photo_pipeline/path_policy.py
View File

@@ -49,6 +49,29 @@ def resolve_within(root: Path, path: os.PathLike | str) -> Path:
return resolved
def resolve_in_roots(roots: Iterable[os.PathLike | str], path: os.PathLike | str) -> Path:
"""The resolved path, proven to be inside one of ``roots`` and not excluded.
Callers must use the **returned** path for whatever they do next: validating one
name and then opening another is the symlink race this exists to close (US07-02).
The message names no path — it reaches API responses.
With no roots configured there is no boundary to check; that is a property of the
configuration, not permission granted to this call.
"""
if is_excluded(path):
raise PathPolicyError("path is inside an excluded (_IGNORE/) tree")
roots = list(roots)
if not roots:
return Path(path)
for root in roots:
try:
return resolve_within(Path(root), path)
except PathPolicyError:
continue
raise PathPolicyError("path is outside the configured library roots")
def iter_supported_files(root: os.PathLike | str) -> Iterator[Path]:
"""Yield supported, non-excluded files under ``root`` in deterministic order.

22
photo_pipeline/services/analysis.py
View File

@@ -25,6 +25,7 @@ from typing import Protocol
from sqlalchemy import func, select
from sqlalchemy.orm import sessionmaker
from photo_pipeline import path_policy
from photo_pipeline.integrations import exiftool
from photo_pipeline.models import AnalysisResult, Asset, SafetyReview
from photo_pipeline.services import hashing
@@ -59,9 +60,16 @@ def _now() -> datetime:
class AnalysisService:
def __init__(self, session_factory: sessionmaker, *, provider: VisionProvider | None = None) -> None:
def __init__(
self,
session_factory: sessionmaker,
*,
provider: VisionProvider | None = None,
library_roots: tuple = (),
) -> None:
self._session_factory = session_factory
self._provider = provider
self._roots = tuple(library_roots)
def _sfw_asset_ids(self, session) -> set[str]:
"""Asset ids whose latest safety decision is ``sfw`` — the ONLY assets that
@@ -136,6 +144,18 @@ class AnalysisService:
if not path:
skipped += 1
continue
# Second gate, at the moment of use: the database says where the file
# was, the filesystem decides what that name means now. A link swapped
# under an asset after the scan would otherwise send bytes from outside
# the library — the one place that leaves this machine (US07-02).
try:
path = str(path_policy.resolve_in_roots(self._roots, path))
except path_policy.PathPolicyError as error:
self._store(
asset_id, status="error", result=None, error=str(error), tokens=0, raw=""
)
errors += 1
continue
try:
result = provider.analyze(path, album_hint=_album_hint(path))
except Exception as error: # provider/validation failure is per-asset

29
photo_pipeline/services/thumbnails.py
View File

@@ -129,7 +129,10 @@ class ThumbnailService:
raise ThumbnailUnavailable(f"asset {asset_id} has no readable file")
source = str(source)
if source == asset.current_path:
self._validate_path(source) # archive roots lie outside the library
# Render the *resolved* path the check approved: revalidating and then
# reopening the original name would let a symlink swapped in between
# the two steps decide which bytes are served (US07-02).
source = str(self._validate_path(source)) # archive roots lie outside
# Rendering happens outside the DB session (no transaction held during I/O).
try:
@@ -195,20 +198,16 @@ class ThumbnailService:
session.commit()
# ── path safety ──────────────────────────────────────────────────────────
def _validate_path(self, current_path: str) -> None:
path = Path(current_path)
if path_policy.is_excluded(path):
raise PathNotAllowed(f"excluded path: {current_path}")
roots = self._config.library_roots
if not roots:
return
for root in roots:
try:
path_policy.resolve_within(Path(root), path)
return
except path_policy.PathPolicyError:
continue
raise PathNotAllowed(f"path outside configured roots: {current_path}")
def _validate_path(self, current_path: str) -> Path:
"""The resolved path to read, or ``PathNotAllowed``.
The message names no path: a refusal is returned to the browser, and where
the library lives is not the caller's business (US07-02).
"""
try:
return path_policy.resolve_in_roots(self._config.library_roots, current_path)
except path_policy.PathPolicyError as error:
raise PathNotAllowed(str(error)) from None
# ── cache key + rendering ──────────────────────────────────────────────────
@staticmethod