Compare commits

...

1 Commits

Author SHA1 Message Date
b2b996b37e US07-02: Harden API Authorization and Path Boundaries 2026-08-16 23:06:41 +02:00
21 changed files with 1134 additions and 34 deletions

View File

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

View File

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

View File

@@ -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 });

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

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:

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

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

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

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

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:

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.

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

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

View File

@@ -1,4 +1,15 @@
"""Make the repository root importable for the pipeline test suites."""
"""Make the repository root importable for the pipeline test suites, and give every
suite the application session the API requires since US07-02.
The suites drive the API the way the browser does — module-level ``httpx`` calls and
``TestClient`` — so instead of threading a cookie through several hundred call sites,
both clients bootstrap the session themselves exactly like ``frontend/js/api.js``:
fetch ``/api/v1/session`` once, then send the cookie plus the CSRF header, and
re-bootstrap once on 401 (a restarted server issues a new session).
Security tests deliberately bypass this by constructing their own ``httpx.Client``;
only the module-level helpers are wrapped.
"""
import sys
from pathlib import Path
@@ -6,3 +17,85 @@ from pathlib import Path
REPO = Path(__file__).resolve().parents[1]
if str(REPO) not in sys.path:
sys.path.insert(0, str(REPO))
import httpx
import httpx._api # httpx.get/post resolve `request` in this module
import pytest
from starlette.testclient import TestClient
from photo_pipeline.api.security import CSRF_HEADER, SESSION_COOKIE
_SESSIONS: dict[str, tuple[str, str]] = {} # base url -> (session id, csrf token)
def _base(url) -> str:
parts = httpx.URL(str(url))
return f"{parts.scheme}://{parts.netloc.decode()}"
def _bootstrap(base: str) -> tuple[str, str]:
if base not in _SESSIONS:
response = httpx.Client(timeout=10).get(f"{base}/api/v1/session")
_SESSIONS[base] = (
response.cookies.get(SESSION_COOKIE, ""),
response.json().get("csrf_token", ""),
)
return _SESSIONS[base]
def session_client(base_url: str, **kwargs) -> httpx.Client:
"""An ``httpx.Client`` that has bootstrapped its own application session.
For suites that keep one client for a whole journey; it holds its own cookie, so
a client made after a server restart picks up the new session by construction.
"""
client = httpx.Client(base_url=base_url, **kwargs)
client.headers[CSRF_HEADER] = client.get("/api/v1/session").json()["csrf_token"]
return client
def _authorized(url, headers, cookies):
session_id, token = _bootstrap(_base(url))
headers = dict(headers or {})
headers.setdefault(CSRF_HEADER, token)
cookies = dict(cookies or {})
cookies.setdefault(SESSION_COOKIE, session_id)
return headers, cookies
@pytest.fixture(autouse=True, scope="session")
def _api_session():
real_request, real_stream = httpx._api.request, httpx._api.stream
real_client_request, real_client_init = TestClient.request, TestClient.__init__
def request(method, url, *, headers=None, cookies=None, **kwargs):
sent, jar = _authorized(url, headers, cookies)
response = real_request(method, url, headers=sent, cookies=jar, **kwargs)
if response.status_code == 401:
_SESSIONS.pop(_base(url), None)
sent, jar = _authorized(url, headers, cookies)
response = real_request(method, url, headers=sent, cookies=jar, **kwargs)
return response
def stream(method, url, *, headers=None, cookies=None, **kwargs):
sent, jar = _authorized(url, headers, cookies)
return real_stream(method, url, headers=sent, cookies=jar, **kwargs)
def client_init(self, app, *args, base_url="http://127.0.0.1", **kwargs):
# The default "http://testserver" is not a local host, which is exactly what
# the Host check refuses; in-process tests are still same-origin callers.
real_client_init(self, app, *args, base_url=base_url, **kwargs)
def client_request(self, method, url, *, headers=None, **kwargs):
if CSRF_HEADER not in self.headers:
response = real_client_request(self, "GET", "/api/v1/session")
self.headers[CSRF_HEADER] = response.json()["csrf_token"]
return real_client_request(self, method, url, headers=headers, **kwargs)
httpx._api.request, httpx.request = request, request
httpx._api.stream, httpx.stream = stream, stream
TestClient.request, TestClient.__init__ = client_request, client_init
yield
httpx._api.request, httpx.request = real_request, real_request
httpx._api.stream, httpx.stream = real_stream, real_stream
TestClient.request, TestClient.__init__ = real_client_request, real_client_init

View File

@@ -22,6 +22,8 @@ import numpy as np
import pytest
from PIL import Image
from tests.conftest import session_client
REPO = Path(__file__).resolve().parents[2]
@@ -92,7 +94,7 @@ class ServerController:
pytest.fail(f"server exited: {err.decode(errors='replace')}")
try:
if httpx.get(f"{self.base}/api/v1/health/ready", timeout=1).status_code == 200:
self.client = httpx.Client(base_url=self.base, timeout=10)
self.client = session_client(self.base, timeout=10)
return
except httpx.HTTPError:
time.sleep(0.2)

View File

@@ -17,6 +17,9 @@ import httpx
import numpy as np
import pytest
from PIL import Image
from playwright.sync_api import expect
from tests.conftest import session_client
REPO = Path(__file__).resolve().parents[2]
@@ -111,7 +114,7 @@ def server(tmp_path):
proc.terminate()
pytest.fail("server never became ready")
client = httpx.Client(base_url=base, timeout=5)
client = session_client(base, timeout=5)
def cluster_by_method(method):
clusters = client.get("/api/v1/duplicates/clusters").json()["items"]
@@ -148,9 +151,9 @@ def test_fuzzy_decision_requires_confirmation(page, server):
page.get_by_test_id("confirm").wait_for()
assert "open" in page.get_by_test_id("cluster-state").inner_text()
page.get_by_test_id("confirm-yes").click()
page.wait_for_function(
"document.querySelector('[data-testid=cluster-state]').innerText.includes('dismissed')"
)
# A locator assertion, not wait_for_function: the app's CSP forbids eval, and
# Playwright's polling predicate is evaluated as a string in the page (US07-02).
expect(page.get_by_test_id("cluster-state")).to_contain_text("dismissed")
def test_stale_version_shows_conflict(page, server):
@@ -176,9 +179,9 @@ def test_decision_persists_after_reload(page, server):
page.goto(f"{server.base}/app/#/duplicates/{cluster['id']}")
page.get_by_test_id("not-duplicate").click()
page.get_by_test_id("confirm-yes").click()
page.wait_for_function(
"document.querySelector('[data-testid=cluster-state]').innerText.includes('dismissed')"
)
# A locator assertion, not wait_for_function: the app's CSP forbids eval, and
# Playwright's polling predicate is evaluated as a string in the page (US07-02).
expect(page.get_by_test_id("cluster-state")).to_contain_text("dismissed")
page.reload()
page.get_by_test_id("cluster-state").wait_for()
assert "dismissed" in page.get_by_test_id("cluster-state").inner_text()

280
tests/e2e/test_security.py Normal file
View File

@@ -0,0 +1,280 @@
"""US07-02 black-box security tests: authorization and path boundaries.
Everything here talks to a real ``photo_pipeline serve`` child process over HTTP with
its own ``httpx.Client``, deliberately outside the session helper the rest of the
suite uses — an attacker does not get a bootstrapped client.
The threat is a page in the user's browser, not a remote attacker: the app listens on
127.0.0.1, so any site the user visits can send requests to it and can point an
``<img>`` at its media endpoints. Each journey below is one of those attempts.
"""
from __future__ import annotations
import json
from contextlib import contextmanager
from pathlib import Path
import httpx
import pytest
from tests.e2e._pipeline_harness import Server, image, seed_library
TIMEOUT = 20
SENTINEL_KEY = "immich-sentinel-9f3a2b"
SESSION_COOKIE = "pp_session"
CSRF_HEADER = "X-CSRF-Token"
# A GET, a mutation, and a media endpoint: the three shapes the policy must cover.
PROTECTED = [
("GET", "/api/v1/workflow", None),
("POST", "/api/v1/albums/proposals", {}),
("GET", "/api/v1/assets/{asset}/thumbnail?size=256", None),
]
@pytest.fixture(scope="module")
def stack(tmp_path_factory):
"""One server, one album of two photos, and a credential sentinel in its config."""
tmp_path = tmp_path_factory.mktemp("security")
seeded = seed_library(tmp_path, {"a": 1, "b": 2}, {"a": "sfw", "b": "sfw"})
outside = tmp_path / "outside"
outside.mkdir()
image(outside / "secret.jpg", 99)
server = Server(
seeded,
extra_env={
"PHOTO_PIPELINE_IMMICH_API_KEY": SENTINEL_KEY,
"PHOTO_PIPELINE_IMMICH_SERVER_URL": "http://127.0.0.1:1",
},
).start()
yield server, seeded, outside
server.stop()
@contextmanager
def anonymous(server):
with httpx.Client(base_url=server.base, timeout=TIMEOUT) as client:
yield client
@contextmanager
def authenticated(server):
"""A browser that has loaded the app: session cookie in the jar, token in a header."""
with httpx.Client(base_url=server.base, timeout=TIMEOUT) as client:
client.headers[CSRF_HEADER] = client.get("/api/v1/session").json()["csrf_token"]
yield client
def call(client, method, path, body, asset):
return client.request(method, path.format(asset=asset), json=body)
def test_the_api_refuses_every_caller_without_a_session(stack):
server, seeded, _ = stack
with anonymous(server) as client:
for method, path, body in PROTECTED:
response = call(client, method, path, body, seeded.asset_ids["a"])
assert response.status_code == 401, path
assert response.json()["error"]["code"] == "unauthenticated"
# Liveness and readiness stay open: an orchestrator holds no session.
assert client.get("/api/v1/health/ready").status_code == 200
assert client.get("/api/v1/health/live").status_code == 200
def test_a_guessed_session_cookie_is_refused(stack):
server, _, _ = stack
with anonymous(server) as client:
client.cookies.set(SESSION_COOKIE, "guessed", domain="127.0.0.1")
response = client.get("/api/v1/workflow")
assert response.status_code == 401
assert response.json()["error"]["code"] == "unauthenticated"
def test_the_bootstrap_issues_a_strict_httponly_cookie(stack):
server, _, _ = stack
with anonymous(server) as client:
response = client.get("/api/v1/session")
cookie = response.headers["set-cookie"].lower()
assert "httponly" in cookie and "samesite=strict" in cookie and "path=/" in cookie
assert response.json()["csrf_token"]
# The token is in the body, which no other origin may read: no CORS header
# grants access to it.
assert "access-control-allow-origin" not in response.headers
def test_a_session_without_the_csrf_token_may_read_but_not_mutate(stack):
server, _, _ = stack
with authenticated(server) as client:
del client.headers[CSRF_HEADER]
assert client.get("/api/v1/albums/proposals").status_code == 200
response = client.post("/api/v1/albums/proposals", json={})
assert response.status_code == 403
assert response.json()["error"]["code"] == "csrf_failed"
response = client.post(
"/api/v1/albums/proposals", json={}, headers={CSRF_HEADER: "guessed"}
)
assert response.status_code == 403
# And nothing was created behind the refusal.
with authenticated(server) as client:
assert client.get("/api/v1/albums/proposals").json()["items"] == []
def test_a_foreign_origin_cannot_mutate_even_with_a_session(stack):
server, _, _ = stack
with authenticated(server) as client:
for origin in ("http://evil.example", "http://127.0.0.1:1", "null"):
response = client.post(
"/api/v1/albums/proposals", json={}, headers={"Origin": origin}
)
assert response.status_code == 403, origin
assert response.json()["error"]["code"] == "origin_not_allowed"
# This app's own origin is accepted, so the check is not simply refusing all.
allowed = client.post(
"/api/v1/albums/proposals", json={}, headers={"Origin": server.base}
)
assert allowed.status_code == 200
def test_a_rebinding_host_is_refused(stack):
"""A name that resolves to 127.0.0.1 makes the browser treat the attacker's page
as same-origin. The Host header still carries that name, so it is checked."""
server, _, _ = stack
with authenticated(server) as client:
response = client.get("/api/v1/workflow", headers={"Host": "photos.evil.example"})
assert response.status_code == 403
assert response.json()["error"]["code"] == "host_not_allowed"
def test_media_cannot_be_embedded_by_another_page(stack):
server, seeded, _ = stack
url = f"/api/v1/assets/{seeded.asset_ids['a']}/thumbnail?size=256"
with authenticated(server) as client:
# What an <img> on another site produces: no Origin, but a cross-site marker.
blocked = client.get(url, headers={"Sec-Fetch-Site": "cross-site"})
assert blocked.status_code == 403
assert blocked.json()["error"]["code"] == "cross_site_blocked"
served = client.get(url, headers={"Sec-Fetch-Site": "same-origin"})
assert served.status_code == 200
assert served.headers["content-type"] == "image/webp"
assert served.headers["cross-origin-resource-policy"] == "same-origin"
# Photos must never land in a shared cache.
assert served.headers["cache-control"].startswith("private")
def test_every_response_carries_the_default_headers_and_no_cors(stack):
server, seeded, _ = stack
with authenticated(server) as client:
responses = [
client.get("/api/v1/workflow"),
client.get(f"/api/v1/assets/{seeded.asset_ids['a']}/thumbnail?size=256"),
client.get("/app/"),
client.get("/api/v1/does-not-exist"),
]
for response in responses:
assert response.headers["x-content-type-options"] == "nosniff"
assert response.headers["x-frame-options"] == "DENY"
assert response.headers["referrer-policy"] == "no-referrer"
assert "frame-ancestors 'none'" in response.headers["content-security-policy"]
assert "access-control-allow-origin" not in response.headers
assert "access-control-allow-credentials" not in response.headers
def test_a_traversal_attempt_addresses_nothing(stack):
"""Identifiers are database keys, not paths: traversal has nowhere to land."""
server, _, _ = stack
with authenticated(server) as client:
for path in (
"/api/v1/albums/proposals/..%2F..%2F..%2Fetc%2Fpasswd",
"/api/v1/albums/proposals/../../../etc/passwd",
"/api/v1/assets/..%2F..%2Fetc%2Fpasswd/thumbnail?size=256",
"/api/v1/assets/%2Fetc%2Fpasswd/thumbnail?size=256",
):
response = client.get(path)
assert response.status_code in (404, 422), path
assert "root:" not in response.text, path
def test_an_oversized_request_is_refused_before_it_is_parsed(stack):
server, _, _ = stack
with authenticated(server) as client:
response = client.post(
"/api/v1/albums/proposals",
content=json.dumps({"albums": ["x" * 2_000_000]}),
headers={"Content-Type": "application/json"},
)
assert response.status_code == 413
assert response.json()["error"]["code"] == "payload_too_large"
def test_a_malformed_request_reports_the_field_and_nothing_else(stack):
server, _, _ = stack
with authenticated(server) as client:
for content in ("{", '{"albums": 5}', ""):
response = client.post(
"/api/v1/albums/proposals",
content=content,
headers={"Content-Type": "application/json"},
)
assert response.status_code == 422, content
error = response.json()["error"]
assert error["code"] == "invalid_request"
assert "Traceback" not in response.text and "photo_pipeline/" not in response.text
def test_errors_reveal_neither_the_credential_nor_an_internal(stack):
"""The API key is configured but must appear nowhere; failures additionally say
nothing about where the library lives or how the server is built.
Successful responses are a different matter: path previews are the point of the
rename and archive views, and the operator is the one who owns those paths.
"""
server, seeded, _ = stack
with authenticated(server) as client:
responses = [
client.post("/api/v1/upload-preflight", json={}),
client.get("/api/v1/workflow"),
client.get("/api/v1/upload-batches"),
client.get("/api/v1/assets/unknown-asset/thumbnail?size=256"),
client.get("/api/v1/archive-plans/unknown-plan"),
client.post("/api/v1/archive-locations", json={"name": "x", "root": "/nope"}),
client.get("/api/v1/does-not-exist"),
]
for response in responses:
assert SENTINEL_KEY not in response.text
assert "Traceback" not in response.text
if response.status_code >= 400:
assert str(seeded.lib) not in response.text
assert "photo_pipeline/" not in response.text
assert "sqlite" not in response.text.lower()
def test_a_symlink_swapped_under_an_asset_cannot_be_served(stack):
"""TOCTOU on the media path: the file the database points at is replaced by a
link to something outside the library between the scan and the request."""
server, seeded, outside = stack
secret = outside / "secret.jpg"
original = seeded.lib / "b.jpg"
original.unlink()
original.symlink_to(secret)
with authenticated(server) as client:
response = client.get(f"/api/v1/assets/{seeded.asset_ids['b']}/thumbnail?size=256")
assert response.status_code == 403
assert response.json()["error"]["code"] == "path_not_allowed"
# The refusal names no filesystem location, and no bytes escaped with it.
assert str(outside) not in response.text and str(seeded.lib) not in response.text
assert secret.read_bytes()[:16] not in response.content
def test_the_frontend_shell_stays_reachable_without_a_session(stack):
"""It must load before any JavaScript can ask for a session."""
server, _, _ = stack
with anonymous(server) as client:
response = client.get("/app/", headers={"Sec-Fetch-Site": "none"})
assert response.status_code == 200
assert response.headers["content-type"].startswith("text/html")
assert Path("frontend/index.html").exists()

View File

@@ -0,0 +1,136 @@
"""US07-02: what an unhandled failure says, and where the library ends.
Two boundaries that only show up below the HTTP surface:
* Exception text is where internals leak — absolute paths, SQL, and occasionally a
credential passed to the call that blew up. The first tests drive the real
application with a route that raises such an exception (no production route does)
and assert the client sees only a code.
* The path the database recorded is not the path the filesystem will open a moment
later. Analysis is the one stage whose bytes leave this machine, so it resolves
the source against the library roots immediately before the provider call.
"""
from __future__ import annotations
import logging
import uuid
from datetime import datetime, timezone
import pytest
from PIL import Image
from starlette.testclient import TestClient
from photo_pipeline.api.app import create_app
from photo_pipeline.config import Config
from photo_pipeline.db import create_db_engine, create_session_factory, run_migrations
from photo_pipeline.models import AnalysisResult, Asset
from photo_pipeline.services.analysis import AnalysisService
from photo_pipeline.services.safety import SafetyService
BOOM = "sqlite:///Users/someone/Pictures/private.db failed with key sk-secret-123"
@pytest.fixture
def client(tmp_path):
app = create_app(Config(data_dir=tmp_path / "data"))
@app.get("/api/v1/boom")
def boom():
raise RuntimeError(BOOM)
with TestClient(app, raise_server_exceptions=False) as test_client:
yield test_client
def test_an_unhandled_error_returns_a_bare_envelope(client, caplog):
with caplog.at_level(logging.ERROR):
response = client.get("/api/v1/boom")
assert response.status_code == 500
assert response.json() == {"error": {"code": "internal_error", "message": "internal error"}}
assert BOOM not in response.text and "Traceback" not in response.text
# The operator still gets the whole story, on the server side.
assert BOOM in caplog.text
def test_a_refusal_response_still_carries_the_default_headers(client):
"""A 500 escaping the middleware's response path would also escape its headers."""
response = client.get("/api/v1/boom")
assert response.headers["x-content-type-options"] == "nosniff"
assert response.headers["x-frame-options"] == "DENY"
# ── the library boundary, revalidated at the moment of use ───────────────────
class RecordingProvider:
def __init__(self):
self.calls = []
def analyze(self, path, *, album_hint):
self.calls.append(path)
return {"description": "a photo", "tags": []}
def _library(tmp_path):
lib = tmp_path / "lib"
lib.mkdir()
outside = tmp_path / "outside"
outside.mkdir()
for path in (lib / "inside.jpg", outside / "private.jpg"):
Image.new("RGB", (8, 8), "blue").save(path)
(tmp_path / "data").mkdir()
config = Config.from_env({"PHOTO_PIPELINE_DATA_DIR": str(tmp_path / "data")})
run_migrations(config.database_url)
return lib, outside, create_session_factory(create_db_engine(config.database_url))
def _sfw_asset(sf, path):
asset = Asset(
id=str(uuid.uuid4()),
original_path=str(path),
current_path=str(path),
discovered_at=datetime.now(timezone.utc),
hash_version=1,
)
with sf() as session:
session.add(asset)
session.commit()
SafetyService(sf).decide(asset.id, "sfw", write_exif=False)
return asset.id
def test_analysis_will_not_send_a_file_that_left_the_library(tmp_path):
"""A link swapped under an asset after the scan points at something the user
never put in the library. Those bytes must not reach the vision provider — it is
the one place in the pipeline where content leaves this machine."""
lib, outside, sf = _library(tmp_path)
inside = lib / "inside.jpg"
asset_id = _sfw_asset(sf, inside)
inside.unlink()
inside.symlink_to(outside / "private.jpg")
provider = RecordingProvider()
result = AnalysisService(sf, provider=provider, library_roots=(lib,)).run([asset_id])
assert provider.calls == [], "the provider must never have been constructed a request"
assert result == {"analyzed": 0, "skipped": 0, "errors": 1}
with sf() as session:
row = session.get(AnalysisResult, asset_id)
# The failure is visible and names no path.
assert row.status == "error"
assert "outside the configured library roots" in row.error_message
assert str(outside) not in row.error_message
def test_analysis_still_reads_a_file_that_stayed_inside(tmp_path):
"""The guard must resolve real paths, not refuse everything."""
lib, _, sf = _library(tmp_path)
asset_id = _sfw_asset(sf, lib / "inside.jpg")
provider = RecordingProvider()
result = AnalysisService(sf, provider=provider, library_roots=(lib,)).run([asset_id])
assert result["analyzed"] == 1
assert provider.calls == [str((lib / "inside.jpg").resolve())]

View File

@@ -147,6 +147,11 @@
"US07-01": [
"tests/unit/test_legacy_archive.py",
"tests/integration/test_legacy_import.py"
],
"US07-02": [
"tests/unit/test_security_policy.py",
"tests/integration/test_security_boundaries.py",
"tests/e2e/test_security.py"
]
}
}

View File

@@ -81,3 +81,42 @@ def test_symlink_within_root_is_allowed(tmp_path):
pytest.skip("cannot create symlink on this platform")
found = path_policy.discover([root])
assert target in found and link in found
def test_resolve_in_roots_returns_the_path_the_caller_must_use(tmp_path):
"""The resolved path is the answer, not a yes/no: a caller that revalidates one
name and then opens another has an open symlink race (US07-02)."""
root = tmp_path / "lib"
(root / "sub").mkdir(parents=True)
target = root / "sub" / "real.jpg"
target.write_bytes(b"x")
link = root / "alias.jpg"
os.symlink(target, link)
assert path_policy.resolve_in_roots([root], link) == target.resolve()
assert path_policy.resolve_in_roots([root], target) == target.resolve()
def test_resolve_in_roots_refuses_escapes_without_naming_them(tmp_path):
root = tmp_path / "lib"
root.mkdir()
outside = tmp_path / "outside.jpg"
outside.write_bytes(b"x")
link = root / "alias.jpg"
os.symlink(outside, link)
for candidate in (link, outside, root / "_IGNORE" / "a.jpg"):
with pytest.raises(path_policy.PathPolicyError) as raised:
path_policy.resolve_in_roots([root], candidate)
assert str(outside) not in str(raised.value)
def test_resolve_in_roots_checks_every_configured_root(tmp_path):
first, second = tmp_path / "one", tmp_path / "two"
first.mkdir()
second.mkdir()
photo = second / "b.jpg"
photo.write_bytes(b"x")
assert path_policy.resolve_in_roots([first, second], photo) == photo.resolve()
# No configured boundary means nothing to check against.
assert path_policy.resolve_in_roots([], photo) == photo

View File

@@ -0,0 +1,169 @@
"""US07-02: the request-admission policy, enumerated.
``evaluate`` decides every refusal the API can make before a route runs, so the
whole local-web threat model is one table here: who may call, from where, with what
proof. The middleware and the endpoints are covered black box in
``tests/e2e/test_security.py``; this file pins the rules themselves, including the
combinations a browser can produce but a test client rarely does.
"""
from __future__ import annotations
import pytest
from photo_pipeline.api.security import (
CSRF_HEADER,
PUBLIC_PATHS,
Session,
evaluate,
split_host,
)
SESSION = Session(id="session-id", csrf_token="csrf-token")
HOST = "127.0.0.1:8000"
LIMIT = 1024
def check(method="GET", path="/api/v1/workflow", **headers):
"""Evaluate a request that is authenticated and same-origin unless overridden."""
sent = {
"host": HOST,
"cookie-session": SESSION.id,
CSRF_HEADER: SESSION.csrf_token,
}
sent.update({name.replace("_", "-"): value for name, value in headers.items()})
sent = {name: value for name, value in sent.items() if value is not None}
return evaluate(
method=method,
path=path,
headers=sent,
session=SESSION,
max_request_bytes=LIMIT,
)
def test_an_authenticated_same_origin_request_is_admitted():
assert check() is None
assert check(method="POST", origin="http://127.0.0.1:8000") is None
assert check(sec_fetch_site="same-origin") is None
@pytest.mark.parametrize("host", ["evil.example", "evil.example:8000", "192.168.1.10:8000", ""])
def test_a_non_loopback_host_is_refused(host):
"""DNS rebinding: the browser thinks it is talking to the attacker's name, which
resolves to 127.0.0.1. The name is the evidence, so the name is checked."""
refusal = check(host=host)
assert (refusal.status, refusal.code) == (403, "host_not_allowed")
@pytest.mark.parametrize("host", ["127.0.0.1:8000", "localhost:8000", "[::1]:8000", "localhost"])
def test_loopback_hosts_are_accepted(host):
assert check(host=host) is None
@pytest.mark.parametrize(
"origin",
[
"http://evil.example",
"https://evil.example:8000",
"http://127.0.0.1:9999", # another local app is still another origin
"http://localhost.evil.example:8000",
"null",
"file://",
],
)
def test_a_foreign_origin_is_refused(origin):
refusal = check(method="POST", origin=origin)
assert (refusal.status, refusal.code) == (403, "origin_not_allowed")
@pytest.mark.parametrize("origin", ["http://127.0.0.1:8000", "http://localhost:8000"])
def test_this_applications_origin_is_accepted(origin):
assert check(method="POST", origin=origin) is None
@pytest.mark.parametrize("site", ["cross-site", "same-site"])
def test_a_cross_site_fetch_is_refused_even_without_an_origin(site):
"""What ``<img src="http://127.0.0.1:8000/...">`` on another page looks like."""
refusal = check(path="/api/v1/assets/a1/thumbnail", sec_fetch_site=site)
assert (refusal.status, refusal.code) == (403, "cross_site_blocked")
def test_a_user_initiated_navigation_is_accepted():
assert check(sec_fetch_site="none") is None
def test_a_request_without_a_session_is_unauthenticated():
for method, path in [
("GET", "/api/v1/workflow"),
("POST", "/api/v1/jobs"),
("GET", "/api/v1/assets/a1/thumbnail"),
]:
refusal = check(method=method, path=path, cookie_session=None)
assert (refusal.status, refusal.code) == (401, "unauthenticated"), path
def test_a_forged_session_is_unauthenticated():
refusal = check(cookie_session="guessed")
assert (refusal.status, refusal.code) == (401, "unauthenticated")
@pytest.mark.parametrize("path", sorted(PUBLIC_PATHS))
def test_health_and_the_bootstrap_stay_reachable_without_a_session(path):
assert check(path=path, cookie_session=None) is None
def test_the_static_shell_needs_no_session():
"""It has to load before any JavaScript can ask for one."""
assert check(path="/app/index.html", cookie_session=None) is None
@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"])
def test_a_mutation_without_a_valid_csrf_token_is_refused(method):
for token in (None, "guessed"):
refusal = check(method=method, path="/api/v1/jobs", **{CSRF_HEADER: token})
assert (refusal.status, refusal.code) == (403, "csrf_failed")
@pytest.mark.parametrize("method", ["GET", "HEAD", "OPTIONS"])
def test_reads_need_no_csrf_token(method):
assert check(method=method, **{CSRF_HEADER: None}) is None
def test_an_oversized_body_is_refused_before_it_is_read():
refusal = check(method="POST", path="/api/v1/jobs", content_length=str(LIMIT + 1))
assert (refusal.status, refusal.code) == (413, "payload_too_large")
assert check(method="POST", path="/api/v1/jobs", content_length=str(LIMIT)) is None
def test_the_host_check_precedes_authentication():
"""A refusal must not tell a foreign caller whether its session guess was right."""
refusal = check(host="evil.example", cookie_session="guessed")
assert refusal.code == "host_not_allowed"
def test_refusals_name_no_path_secret_or_internal():
refusals = [
check(host="evil.example"),
check(method="POST", origin="http://evil.example"),
check(cookie_session=None),
check(method="POST", **{CSRF_HEADER: None}),
]
for refusal in refusals:
assert SESSION.id not in refusal.message
assert SESSION.csrf_token not in refusal.message
assert "/" not in refusal.message
@pytest.mark.parametrize(
"value,expected",
[
("127.0.0.1:8000", ("127.0.0.1", "8000")),
("localhost", ("localhost", "")),
("[::1]:8000", ("[::1]", "8000")),
("[::1]", ("[::1]", "")),
("", ("", "")),
],
)
def test_split_host(value, expected):
assert split_host(value) == expected