Compare commits

...

1 Commits

Author SHA1 Message Date
a7a5e6d63e US08-01: Make the Trust Boundary Configurable and Authenticated 2026-08-18 21:58:53 +02:00
10 changed files with 626 additions and 13 deletions

View File

@@ -69,6 +69,34 @@ not a loopback name (DNS rebinding), when `Origin` is any other origin, when
thumbnail), or when the body exceeds `PHOTO_PIPELINE_MAX_REQUEST_BYTES`. There is no 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. CORS middleware at all, so no other origin can read a response.
### Reaching it through a hostname or proxy (US08-01)
| variable | meaning |
|---|---|
| `PHOTO_PIPELINE_ALLOWED_HOSTS` | comma-separated extra names the app answers to; empty means loopback only |
| `PHOTO_PIPELINE_ACCESS_SECRET` | traded for the session cookie at `GET /api/v1/session` via `X-Access-Secret` |
| `PHOTO_PIPELINE_TRUSTED_PROXIES` | comma-separated peer addresses whose `X-Forwarded-Proto`/`X-Forwarded-Host` are believed |
Being reachable *was* the authentication: whoever could open `127.0.0.1:8000` owned
the library. So naming any non-loopback host — or binding to one, `0.0.0.0` included
— makes the access secret mandatory, and `serve` refuses to start without it rather
than publishing the library. Loopback-only deployments need no secret and behave
exactly as before.
```bash
curl -sc /tmp/pp.jar -H "X-Access-Secret: $PHOTO_PIPELINE_ACCESS_SECRET" \
https://photos.example.com/api/v1/session
```
The browser asks for the secret once per tab and keeps it in `sessionStorage`.
Wrong secrets are rate-limited (5 per minute) and logged with the caller's address
only. `Host` and `Origin` are judged against the configured names; the *external*
scheme and host come from the forwarded headers only when the request arrived from a
`PHOTO_PIPELINE_TRUSTED_PROXIES` address, so a client cannot declare its own origin,
and the session cookie is marked `Secure` when that external scheme is HTTPS. Health
endpoints stay reachable without the secret so an orchestrator can restart the
container; nothing else does.
## Testing ## Testing
One offline command runs the whole suite (unit, integration, and browser One offline command runs the whole suite (unit, integration, and browser

View File

@@ -7,9 +7,28 @@ export const BASE = "/api/v1";
// what makes it proof that the caller is this app and not another page. // what makes it proof that the caller is this app and not another page.
let csrfToken = null; let csrfToken = null;
// A deployment reachable through a proxy trades an operator secret for that cookie.
// Kept per tab: sessionStorage dies with the tab, and the secret never enters a URL.
const SECRET_KEY = "pp_access_secret";
async function bootstrap(secret) {
return fetch(BASE + "/session", {
credentials: "same-origin",
headers: secret ? { "X-Access-Secret": secret } : {},
});
}
async function session() { async function session() {
if (csrfToken === null) { if (csrfToken === null) {
const response = await fetch(BASE + "/session", { credentials: "same-origin" }); let response = await bootstrap(sessionStorage.getItem(SECRET_KEY));
if (response.status === 401) {
sessionStorage.removeItem(SECRET_KEY);
const secret = prompt("Access secret");
if (secret) {
response = await bootstrap(secret);
if (response.ok) sessionStorage.setItem(SECRET_KEY, secret);
}
}
const body = await response.json().catch(() => null); const body = await response.json().catch(() => null);
csrfToken = (body && body.csrf_token) || null; csrfToken = (body && body.csrf_token) || null;
} }

View File

@@ -209,15 +209,25 @@ def main(argv: Sequence[str] | None = None) -> int:
lock.release() lock.release()
return 0 return 0
import sys
import uvicorn import uvicorn
from photo_pipeline.api.app import create_app from photo_pipeline.api.app import ConfigurationRefused, create_app
# An exposed deployment without an access secret must not reach the port at all,
# and the operator needs a sentence, not a traceback (US08-01).
try:
app = create_app(config)
except ConfigurationRefused as error:
print(str(error), file=sys.stderr)
return 4
lock = LibraryLock(config, "api") lock = LibraryLock(config, "api")
if (held := _acquire(lock, allow_legacy=args.allow_legacy)) is not None: if (held := _acquire(lock, allow_legacy=args.allow_legacy)) is not None:
return held return held
try: try:
uvicorn.run(create_app(config), host=config.host, port=config.port) uvicorn.run(app, host=config.host, port=config.port)
finally: finally:
lock.release() lock.release()
return 0 return 0

View File

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

View File

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

View File

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

View File

@@ -7,6 +7,9 @@ real external call needs them.
pydantic-settings would do this too, but a prefix-scan over the declared fields pydantic-settings would do this too, but a prefix-scan over the declared fields
is a few lines and one fewer dependency. is a few lines and one fewer dependency.
Tuple-valued settings are lists in one variable: library roots are ``os.pathsep``
separated because they are paths, everything else is comma separated.
""" """
from __future__ import annotations from __future__ import annotations
@@ -20,6 +23,7 @@ from pydantic import BaseModel, ConfigDict, SecretStr
ENV_PREFIX = "PHOTO_PIPELINE_" ENV_PREFIX = "PHOTO_PIPELINE_"
ENV_FILE_VAR = f"{ENV_PREFIX}ENV_FILE" ENV_FILE_VAR = f"{ENV_PREFIX}ENV_FILE"
DEFAULT_ENV_FILE = Path(".env") DEFAULT_ENV_FILE = Path(".env")
COMMA_LIST_FIELDS = frozenset({"allowed_hosts", "trusted_proxies"})
# The archived CLI's variable names, so the configuration file an operator already # The archived CLI's variable names, so the configuration file an operator already
# has keeps working. The vision provider reads the OpenAI SDK's names, and the # has keeps working. The vision provider reads the OpenAI SDK's names, and the
@@ -85,6 +89,19 @@ class Config(BaseModel):
log_level: str = "INFO" log_level: str = "INFO"
log_format: str = "json" # "json" or "text" log_format: str = "json" # "json" or "text"
# Trust boundary (US08-01). Empty means loopback only, which is what the app did
# before there was a setting: a request whose Host is not a loopback name is
# refused, and no secret is needed because nothing outside this machine can call.
# Naming a real hostname here is what makes the app reachable through a reverse
# proxy, and it is exactly then that ``access_secret`` becomes mandatory.
allowed_hosts: tuple[str, ...] = ()
# Addresses whose ``X-Forwarded-Proto``/``X-Forwarded-Host`` may be believed. A
# client that is not the proxy can otherwise declare its own origin.
trusted_proxies: tuple[str, ...] = ()
# Exchanged for the session cookie at the bootstrap endpoint. Once set it is
# required even on loopback, so a development setup cannot half-enable it.
access_secret: SecretStr | None = None
# Largest request body the API accepts. Every endpoint takes small JSON commands; # 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). # anything larger is a mistake or an attempt to exhaust memory (US07-02).
max_request_bytes: int = 1_048_576 max_request_bytes: int = 1_048_576
@@ -131,5 +148,10 @@ class Config(BaseModel):
raw = env.get(ENV_PREFIX + name.upper()) raw = env.get(ENV_PREFIX + name.upper())
if not raw: if not raw:
continue continue
data[name] = raw.split(os.pathsep) if name == "library_roots" else raw if name == "library_roots":
data[name] = raw.split(os.pathsep)
elif name in COMMA_LIST_FIELDS:
data[name] = [part.strip() for part in raw.split(",") if part.strip()]
else:
data[name] = raw
return cls(**data) return cls(**data)

View File

@@ -0,0 +1,253 @@
"""US08-01: the configurable trust boundary and its authentication gate.
Until now, reaching the app proved ownership of it: it answered only to loopback
names. A container behind a reverse proxy answers to a real hostname, so these tests
pin the two halves that replace that proof — the app refuses to start exposed without
an access secret, and the secret is the only way to obtain the session every other
route already required (US07-02, unchanged and re-asserted here).
The suite's ``conftest`` bootstraps a session for any ``TestClient`` automatically,
which is precisely what an unauthenticated caller does not get; ``raw_client``
pre-seeds a placeholder CSRF header to opt out of that convenience.
"""
from __future__ import annotations
import pytest
from starlette.testclient import TestClient
from photo_pipeline.api.app import ConfigurationRefused, create_app
from photo_pipeline.api.security import ACCESS_SECRET_HEADER, CSRF_HEADER, SESSION_COOKIE
from photo_pipeline.config import Config
SECRET = "operator-secret-value"
HOSTNAME = "photos.example.com"
# What Starlette reports as the peer address of an in-process request.
TESTCLIENT_ADDRESS = "testclient"
# One of each route class: a read, a mutation, and a media endpoint.
PROTECTED = [
("GET", "/api/v1/workflow", None),
("POST", "/api/v1/albums/proposals", {}),
("GET", "/api/v1/assets/unknown-asset/thumbnail?size=256", None),
]
def config(tmp_path, **overrides) -> Config:
return Config(data_dir=tmp_path / "data", **overrides)
def raw_client(app, base_url="http://127.0.0.1") -> TestClient:
client = TestClient(app, base_url=base_url)
client.headers[CSRF_HEADER] = "placeholder"
return client
def exchange(client, secret=SECRET, headers=None):
return client.get("/api/v1/session", headers={ACCESS_SECRET_HEADER: secret, **(headers or {})})
# ── startup: exposure without a secret is refused, loopback is unchanged ──────
@pytest.mark.parametrize(
"exposure,exposed",
[({"allowed_hosts": (HOSTNAME,)}, HOSTNAME), ({"host": "0.0.0.0"}, "0.0.0.0")],
)
def test_an_exposed_configuration_refuses_to_serve_without_a_secret(tmp_path, exposure, exposed):
with pytest.raises(ConfigurationRefused) as refused:
create_app(config(tmp_path, **exposure))
assert "PHOTO_PIPELINE_ACCESS_SECRET" in str(refused.value)
# The message names what is exposed, so the operator knows which setting did it.
assert exposed in str(refused.value)
def test_the_serve_command_reports_the_refusal_instead_of_binding(tmp_path, monkeypatch, capsys):
"""Exit before the port, the lock, and the database, with a sentence not a trace."""
from photo_pipeline.__main__ import main
monkeypatch.setenv("PHOTO_PIPELINE_DATA_DIR", str(tmp_path / "data"))
monkeypatch.setenv("PHOTO_PIPELINE_ALLOWED_HOSTS", HOSTNAME)
monkeypatch.delenv("PHOTO_PIPELINE_ACCESS_SECRET", raising=False)
assert main(["serve"]) == 4
assert "PHOTO_PIPELINE_ACCESS_SECRET" in capsys.readouterr().err
def test_an_exposed_configuration_with_a_secret_starts(tmp_path):
app = create_app(config(tmp_path, allowed_hosts=(HOSTNAME,), access_secret=SECRET))
with raw_client(app, base_url=f"http://{HOSTNAME}") as client:
assert exchange(client).status_code == 200
def test_a_loopback_configuration_still_needs_no_secret(tmp_path):
"""An unset trust boundary must behave exactly as it did before this story."""
with raw_client(create_app(config(tmp_path))) as client:
response = client.get("/api/v1/session")
assert response.status_code == 200
assert response.json()["csrf_token"]
assert "secure" not in response.headers["set-cookie"].lower()
# ── the exchange: secret in, session out ─────────────────────────────────────
@pytest.fixture
def gated(tmp_path):
app = create_app(
config(
tmp_path,
allowed_hosts=(HOSTNAME,),
access_secret=SECRET,
trusted_proxies=(TESTCLIENT_ADDRESS,),
)
)
with raw_client(app, base_url=f"http://{HOSTNAME}") as client:
yield client
def test_the_secret_buys_the_session_and_the_session_buys_the_routes(gated):
response = exchange(gated)
assert response.status_code == 200
cookie = response.headers["set-cookie"].lower()
assert "httponly" in cookie and "samesite=strict" in cookie
gated.headers[CSRF_HEADER] = response.json()["csrf_token"]
# The session and CSRF requirements behind the gate are the ones US07-02 set.
assert gated.get("/api/v1/workflow").status_code == 200
assert gated.post("/api/v1/albums/proposals", json={}).status_code == 200
refused = gated.post("/api/v1/albums/proposals", json={}, headers={CSRF_HEADER: "guessed"})
assert refused.json()["error"]["code"] == "csrf_failed"
@pytest.mark.parametrize("offered", ["", "wrong-secret", SECRET + "x", SECRET.upper()])
def test_a_wrong_secret_buys_nothing(gated, offered):
response = exchange(gated, secret=offered)
assert response.status_code == 401
assert response.json()["error"]["code"] == "access_denied"
assert "set-cookie" not in response.headers
def test_a_refusal_never_echoes_the_secret_or_the_session(gated, caplog):
with caplog.at_level("WARNING"):
response = exchange(gated, secret="wrong-secret")
assert SECRET not in response.text and "wrong-secret" not in response.text
assert SECRET not in caplog.text
# Logged as an event with its caller, without the session it did not get.
assert "access secret rejected" in caplog.text
def test_guessing_is_rate_limited(gated):
codes = [exchange(gated, secret=f"guess-{n}").status_code for n in range(6)]
assert codes.count(401) == 5 and codes[-1] == 429
assert gated.get("/api/v1/session").status_code == 429
# The right secret is refused too while the limiter holds: that is the point.
blocked = exchange(gated)
assert blocked.status_code == 429
assert SECRET not in blocked.text
def test_every_route_class_is_unreachable_without_the_secret(gated):
for method, path, body in PROTECTED:
response = gated.request(method, path, json=body)
assert response.status_code == 401, path
assert response.json()["error"]["code"] == "unauthenticated", path
# Health stays open: an orchestrator restarting the container holds no secret.
assert gated.get("/api/v1/health/live").status_code == 200
assert gated.get("/api/v1/health/ready").status_code == 200
def test_a_session_from_another_process_is_not_replayable(tmp_path):
"""Sessions live in the process, so a cookie captured from a previous one — a
restarted container, or a second deployment — must not open this one."""
settings = dict(allowed_hosts=(HOSTNAME,), access_secret=SECRET)
first, second = (create_app(config(tmp_path / str(n), **settings)) for n in (1, 2))
with raw_client(first, base_url=f"http://{HOSTNAME}") as client:
exchange(client)
stolen = client.cookies[SESSION_COOKIE]
with raw_client(second, base_url=f"http://{HOSTNAME}") as client:
client.cookies.set(SESSION_COOKIE, stolen, domain=HOSTNAME)
response = client.get("/api/v1/workflow")
assert response.status_code == 401
assert response.json()["error"]["code"] == "unauthenticated"
def test_a_cross_site_request_is_still_refused_behind_the_gate(gated):
gated.headers[CSRF_HEADER] = exchange(gated).json()["csrf_token"]
refused = gated.post(
"/api/v1/albums/proposals", json={}, headers={"Origin": "https://evil.example"}
)
assert refused.json()["error"]["code"] == "origin_not_allowed"
embedded = gated.get(
"/api/v1/assets/unknown-asset/thumbnail?size=256", headers={"Sec-Fetch-Site": "cross-site"}
)
assert embedded.json()["error"]["code"] == "cross_site_blocked"
def test_an_unconfigured_host_is_refused_even_with_a_valid_session(gated):
gated.headers[CSRF_HEADER] = exchange(gated).json()["csrf_token"]
for host in ("other.example.com", "192.168.1.10"):
response = gated.get("/api/v1/workflow", headers={"Host": host})
assert response.status_code == 403, host
assert response.json()["error"]["code"] == "host_not_allowed", host
# ── forwarded headers: believed from the proxy, ignored from anyone else ──────
def test_a_trusted_proxys_https_makes_the_cookie_secure(gated):
"""The proxy speaks HTTPS outward and HTTP to this app, so only the header knows."""
assert "secure" in exchange(gated, headers={"X-Forwarded-Proto": "https"}).headers[
"set-cookie"
].lower()
assert "secure" not in exchange(gated).headers["set-cookie"].lower()
def test_the_external_scheme_is_part_of_the_accepted_origin(gated):
gated.headers[CSRF_HEADER] = exchange(gated).json()["csrf_token"]
allowed = gated.post(
"/api/v1/albums/proposals",
json={},
headers={"X-Forwarded-Proto": "https", "Origin": f"https://{HOSTNAME}"},
)
assert allowed.status_code == 200
# The scheme is part of the origin: the same name over plain HTTP is not it.
refused = gated.post(
"/api/v1/albums/proposals",
json={},
headers={"X-Forwarded-Proto": "https", "Origin": f"http://{HOSTNAME}"},
)
assert refused.json()["error"]["code"] == "origin_not_allowed"
def test_a_trusted_proxys_forwarded_host_is_the_host_that_is_judged(tmp_path):
"""The proxy terminates the operator's hostname and dials this app by address."""
app = create_app(
config(
tmp_path,
allowed_hosts=(HOSTNAME,),
access_secret=SECRET,
trusted_proxies=(TESTCLIENT_ADDRESS,),
)
)
with raw_client(app, base_url="http://10.0.0.5") as client:
forwarded = {"X-Forwarded-Host": HOSTNAME}
assert exchange(client, headers=forwarded).status_code == 200
# Without the header the address it was dialled by is not an allowed name.
assert exchange(client).json()["error"]["code"] == "host_not_allowed"
def test_forwarded_headers_from_an_untrusted_client_are_ignored(tmp_path):
"""Otherwise any caller could declare the hostname and scheme of its choosing."""
app = create_app(config(tmp_path, allowed_hosts=(HOSTNAME,), access_secret=SECRET))
with raw_client(app, base_url="http://evil.example") as client:
forged = exchange(client, headers={"X-Forwarded-Host": HOSTNAME})
assert forged.json()["error"]["code"] == "host_not_allowed"
with raw_client(app, base_url=f"http://{HOSTNAME}") as client:
# A forged scheme would flip the cookie's Secure flag on a plain connection,
# which is how a cookie gets set and then never sent again.
response = exchange(client, headers={"X-Forwarded-Proto": "https"})
assert response.status_code == 200
assert "secure" not in response.headers["set-cookie"].lower()

View File

@@ -174,10 +174,13 @@
"US07-07": [ "US07-07": [
"tests/e2e/test_release_gate.py", "tests/e2e/test_release_gate.py",
"tests/e2e/test_release_journey.py" "tests/e2e/test_release_journey.py"
],
"US08-01": [
"tests/unit/test_security_policy.py",
"tests/integration/test_trusted_hosts.py"
] ]
}, },
"planned": [ "planned": [
"US08-01",
"US08-02", "US08-02",
"US08-03", "US08-03",
"US08-04", "US08-04",

View File

@@ -13,18 +13,31 @@ import pytest
from photo_pipeline.api.security import ( from photo_pipeline.api.security import (
CSRF_HEADER, CSRF_HEADER,
LOOPBACK_HOSTS,
PUBLIC_PATHS, PUBLIC_PATHS,
FailureLimiter,
Session, Session,
evaluate, evaluate,
exposed_hosts,
external_view,
split_host, split_host,
trust_refusal,
) )
from photo_pipeline.config import Config
SESSION = Session(id="session-id", csrf_token="csrf-token") SESSION = Session(id="session-id", csrf_token="csrf-token")
HOST = "127.0.0.1:8000" HOST = "127.0.0.1:8000"
LIMIT = 1024 LIMIT = 1024
HOSTNAME = "photos.example.com"
def check(method="GET", path="/api/v1/workflow", **headers): def check(
method="GET",
path="/api/v1/workflow",
allowed_hosts=LOOPBACK_HOSTS,
scheme="http",
**headers,
):
"""Evaluate a request that is authenticated and same-origin unless overridden.""" """Evaluate a request that is authenticated and same-origin unless overridden."""
sent = { sent = {
"host": HOST, "host": HOST,
@@ -38,6 +51,8 @@ def check(method="GET", path="/api/v1/workflow", **headers):
path=path, path=path,
headers=sent, headers=sent,
session=SESSION, session=SESSION,
allowed_hosts=allowed_hosts,
scheme=scheme,
max_request_bytes=LIMIT, max_request_bytes=LIMIT,
) )
@@ -155,6 +170,115 @@ def test_refusals_name_no_path_secret_or_internal():
assert "/" not in refusal.message assert "/" not in refusal.message
# ── US08-01: the same table with a configured trust boundary ─────────────────
CONFIGURED = frozenset(LOOPBACK_HOSTS | {HOSTNAME})
def test_a_configured_host_is_accepted_and_its_neighbours_are_not():
assert check(host=HOSTNAME, allowed_hosts=CONFIGURED) is None
for host in ("other.example.com", f"evil-{HOSTNAME}", "192.168.1.10"):
refusal = check(host=host, allowed_hosts=CONFIGURED)
assert (refusal.status, refusal.code) == (403, "host_not_allowed"), host
def test_the_loopback_default_refuses_a_host_nobody_configured():
"""The default set is what the app enforced before there was a setting."""
refusal = check(host=HOSTNAME)
assert (refusal.status, refusal.code) == (403, "host_not_allowed")
def test_the_origin_must_match_the_external_scheme():
for scheme in ("http", "https"):
assert (
check(
method="POST",
host=HOSTNAME,
origin=f"{scheme}://{HOSTNAME}",
allowed_hosts=CONFIGURED,
scheme=scheme,
)
is None
)
# An HTTPS deployment whose caller claims plain HTTP is a different origin.
refusal = check(
method="POST",
host=HOSTNAME,
origin=f"http://{HOSTNAME}",
allowed_hosts=CONFIGURED,
scheme="https",
)
assert (refusal.status, refusal.code) == (403, "origin_not_allowed")
def view(client, *, trusted=(), **headers):
sent = {name.replace("_", "-"): value for name, value in headers.items()}
return external_view(
client=client,
headers={"host": HOST, **sent},
scheme="http",
trusted_proxies=frozenset(trusted),
)
def test_forwarded_headers_are_ignored_without_a_trusted_proxy():
forged = {"x_forwarded_proto": "https", "x_forwarded_host": HOSTNAME}
assert view("10.0.0.9", **forged) == ("http", HOST)
assert view(None, **forged) == ("http", HOST)
# Configuring *a* proxy does not trust a caller that is not it.
assert view("10.0.0.9", trusted=("10.0.0.1",), **forged) == ("http", HOST)
def test_a_trusted_proxy_defines_the_external_scheme_and_host():
assert view(
"10.0.0.1", trusted=("10.0.0.1",), x_forwarded_proto="https", x_forwarded_host=HOSTNAME
) == ("https", HOSTNAME)
# A chain: the first entry is what the original client asked for.
assert view(
"10.0.0.1",
trusted=("10.0.0.1",),
x_forwarded_proto="https, http",
x_forwarded_host=f"{HOSTNAME}, inner.internal",
) == ("https", HOSTNAME)
# Trusted but silent: this hop's own view stands.
assert view("10.0.0.1", trusted=("10.0.0.1",)) == ("http", HOST)
@pytest.mark.parametrize(
"settings,exposed",
[
({}, []),
({"host": "127.0.0.1"}, []),
({"allowed_hosts": ("localhost", "127.0.0.1")}, []),
({"allowed_hosts": (f"{HOSTNAME}:8443",)}, [HOSTNAME]),
({"host": "0.0.0.0", "allowed_hosts": (HOSTNAME,)}, ["0.0.0.0", HOSTNAME]),
],
)
def test_exposed_hosts_names_only_what_another_machine_can_reach(settings, exposed):
assert exposed_hosts(Config(**settings)) == exposed
def test_an_exposed_configuration_without_a_secret_must_not_serve():
refusal = trust_refusal(Config(allowed_hosts=(HOSTNAME,)))
assert HOSTNAME in refusal and "PHOTO_PIPELINE_ACCESS_SECRET" in refusal
assert trust_refusal(Config(allowed_hosts=(HOSTNAME,), access_secret="s")) is None
# Loopback-only, with and without a secret, is unchanged.
assert trust_refusal(Config()) is None
assert trust_refusal(Config(access_secret="s")) is None
def test_failed_attempts_are_bounded_per_window():
limiter = FailureLimiter(limit=2, window=60.0)
assert not limiter.blocked()
limiter.record_failure()
assert not limiter.blocked()
limiter.record_failure()
assert limiter.blocked()
# Attempts age out, so a locked-out operator is not locked out forever.
limiter._failures = [-120.0, -120.0]
assert not limiter.blocked()
@pytest.mark.parametrize( @pytest.mark.parametrize(
"value,expected", "value,expected",
[ [