Compare commits
1 Commits
us/US07-06
...
us/US07-02
| Author | SHA1 | Date | |
|---|---|---|---|
| b2b996b37e |
19
README.md
19
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 `<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
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
30
photo_pipeline/api/routes/session.py
Normal 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
|
||||
@@ -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
192
photo_pipeline/api/security.py
Normal 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 ""
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
280
tests/e2e/test_security.py
Normal 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()
|
||||
136
tests/integration/test_security_boundaries.py
Normal file
136
tests/integration/test_security_boundaries.py
Normal 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())]
|
||||
@@ -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"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
169
tests/unit/test_security_policy.py
Normal file
169
tests/unit/test_security_policy.py
Normal 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
|
||||
Reference in New Issue
Block a user