diff --git a/src/pinky_daemon/__main__.py b/src/pinky_daemon/__main__.py index b2300de1..0d6dfcb4 100644 --- a/src/pinky_daemon/__main__.py +++ b/src/pinky_daemon/__main__.py @@ -45,7 +45,14 @@ def main() -> None: default="api", help="Run mode: api (HTTP server) or poll (message polling daemon)", ) - parser.add_argument("--host", default="0.0.0.0", help="API server host") + parser.add_argument( + "--host", + default=None, + help=( + "API server host. Default depends on PINKY_DEPLOYMENT_MODE: " + "0.0.0.0 for trusted/lan, 127.0.0.1 for web." + ), + ) parser.add_argument("--port", type=int, default=8888, help="API server port") parser.add_argument( "--config", @@ -76,12 +83,50 @@ def _run_api(args) -> None: import uvicorn from pinky_daemon.api import create_api + from pinky_daemon.boot_guards import ( + BOOT_GUARD_EXIT_CODE, + BootGuardError, + assert_web_mode_safe, + ) + from pinky_daemon.config import ( + InvalidDeploymentModeError, + default_bind_host, + resolve_deployment_mode, + warn_if_insecure_exposure, + ) + + # Resolve deployment posture first — fail fast on a typo'd env var so the + # operator doesn't think they have hardening applied when they don't. + try: + mode = resolve_deployment_mode() + except InvalidDeploymentModeError as exc: + print(f"[pinky] {exc}", file=sys.stderr) + sys.exit(2) + + # If --host wasn't passed, fall back to the mode-appropriate default. The + # asymmetry between "operator picked 0.0.0.0" and "old hardcoded default" + # matters for #436 / #441; argparse's default is now None so we can tell. + host = args.host if args.host is not None else default_bind_host(mode) + + warning = warn_if_insecure_exposure(mode, host) + if warning: + print(f"[pinky] {warning}", file=sys.stderr) working_dir = os.path.abspath(args.working_dir) + db_path = os.path.join(working_dir, "data", "conversations.db") + + # Refuse-to-boot guards (#441) — only fire in web mode. Each failure + # has a per-guard override env var; overrides log loudly. + try: + assert_web_mode_safe(mode, host, db_path) + except BootGuardError as exc: + print(f"[pinky] {exc}", file=sys.stderr) + sys.exit(BOOT_GUARD_EXIT_CODE) print( f"[pinky] Starting API server\n" - f" Host: {args.host}:{args.port}\n" + f" Mode: {mode.value}\n" + f" Host: {host}:{args.port}\n" f" Working dir: {working_dir}\n" f" Max sessions: {args.max_sessions}", file=sys.stderr, @@ -90,9 +135,10 @@ def _run_api(args) -> None: app = create_api( max_sessions=args.max_sessions, default_working_dir=working_dir, + deployment_mode=mode, ) - uvicorn.run(app, host=args.host, port=args.port) + uvicorn.run(app, host=host, port=args.port) def _run_poll(args) -> None: diff --git a/src/pinky_daemon/api.py b/src/pinky_daemon/api.py index 47d1131b..3254a6f7 100644 --- a/src/pinky_daemon/api.py +++ b/src/pinky_daemon/api.py @@ -24,7 +24,10 @@ import uuid from pathlib import Path from types import SimpleNamespace -from typing import Literal +from typing import TYPE_CHECKING, Literal + +if TYPE_CHECKING: + from pinky_daemon.config import DeploymentMode from fastapi import ( FastAPI, @@ -622,14 +625,51 @@ def create_api( max_sessions: int = 50, default_working_dir: str = ".", db_path: str = "data/conversations.db", + deployment_mode: "DeploymentMode | None" = None, ) -> FastAPI: - """Create the FastAPI application.""" + """Create the FastAPI application. + + Args: + deployment_mode: Operator-declared posture (trusted/lan/web). When + ``None`` the value is resolved from ``PINKY_DEPLOYMENT_MODE`` so + that callers (tests, embedded use) don't have to wire it through. + Cached on ``app.state.deployment_mode`` and exposed under + ``/api`` so downstream hardening middleware can branch off a + single source of truth (#433). + """ + + from pinky_daemon.config import resolve_deployment_mode + from pinky_daemon.net.client_identity import trusted_proxies_from_env + from pinky_daemon.net.lan_filter import ( + build_lan_filter_middleware, + resolve_lan_allowed_cidrs, + ) + + if deployment_mode is None: + deployment_mode = resolve_deployment_mode() app = FastAPI( title="Pinky", description="Stateful Claude Code session API", version="0.1.0", ) + # Cache on app.state for middleware/route consumers — single source of + # truth for the rest of the #433 hardening track. + app.state.deployment_mode = deployment_mode + # Trusted reverse-proxy CIDRs (#442). Read once at create_api time so the + # canonical client-IP resolver doesn't hit os.environ on every request. + app.state.trusted_proxies = trusted_proxies_from_env() + # LAN allowlist (#436): default RFC1918 + link-local + loopback + ULA, + # plus operator extras from PINKY_LAN_EXTRA_CIDRS. Cached so the + # middleware doesn't recompute per request. + app.state.lan_allowed_cidrs = resolve_lan_allowed_cidrs() + # LAN filter middleware (#436): no-op in trusted/web modes; in LAN mode + # rejects requests whose source isn't in app.state.lan_allowed_cidrs. + # Registered very early so public-source traffic is dropped before any + # auth/route handler work runs. + app.middleware("http")( + build_lan_filter_middleware(allowlist=app.state.lan_allowed_cidrs) + ) # ── CORS ────────────────────────────────────────────── # Allow origins from env (comma-separated) or default to same-origin only. @@ -647,21 +687,34 @@ def create_api( ) # ── Security Headers ────────────────────────────────── - @app.middleware("http") - async def security_headers_middleware(request: Request, call_next): - if request.headers.get("upgrade", "").lower() == "websocket": - return await call_next(request) - response = await call_next(request) - response.headers["X-Content-Type-Options"] = "nosniff" - response.headers["X-Frame-Options"] = "DENY" - response.headers["X-XSS-Protection"] = "1; mode=block" - response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin" - response.headers["Permissions-Policy"] = "camera=(), microphone=(), geolocation=()" - # HSTS only when behind TLS (reverse proxy sets X-Forwarded-Proto) - proto = request.headers.get("x-forwarded-proto", "") - if proto == "https" or request.url.scheme == "https": - response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" - return response + # Mode-aware helmet-style headers (#437). Trusted = minimal back-compat + # (nosniff + X-XSS-Protection disable). LAN adds clickjacking + + # referrer + permissions-policy + report-only CSP. Web adds strict CSP + # + HSTS-when-verified-HTTPS. See pinky_daemon.middleware.security_headers + # for the full matrix and the HSTS verification rules. + from pinky_daemon.middleware.security_headers import ( + build_security_headers_middleware, + ) + app.middleware("http")(build_security_headers_middleware()) + + # ── Rate limiting (#438) ────────────────────────────── + # In-memory sliding-window limiter. trusted = off; lan = auth+admin; + # web = all buckets. Keys are the canonical client IP from #442. + # 429 trips emit `rate_limit.exceeded` to the security audit (#440). + from pinky_daemon.middleware.rate_limit import ( + InMemoryRateLimiter, + build_rate_limit_middleware, + ) + app.state.rate_limiter = InMemoryRateLimiter() + app.middleware("http")(build_rate_limit_middleware()) + + # ── CSRF protection (#443) ──────────────────────────── + # Double-submit cookie + header for cookie-authenticated state- + # changing requests in lan/web. Bearer auth, webhook routes, + # Twilio callbacks, and internal MCP-signed requests are exempt. + # Trusted mode: middleware off for back-compat. + from pinky_daemon.middleware.csrf import build_csrf_middleware + app.middleware("http")(build_csrf_middleware()) session_store = SessionStore(db_path=db_path.replace(".db", "_sessions.db")) session_event_store = SessionEventStore(db_path=db_path.replace(".db", "_sessions.db")) @@ -670,6 +723,15 @@ async def security_headers_middleware(request: Request, call_next): agents = AgentRegistry(db_path=db_path.replace(".db", "_agents.db")) _refresh_1m_models(agents) audit = AuditStore(db_path=db_path.replace(".db", "_audit.db")) + # Security-event audit log (#440). Separate DB so a focused export job + # doesn't have to scan the bulky tool-call audit. Append-only by + # convention; no UPDATE/DELETE API. Best-effort writes — login can't + # fail because the audit DB is locked. + from pinky_daemon.security_audit import SecurityAuditStore + security_audit = SecurityAuditStore( + db_path=db_path.replace(".db", "_security_audit.db") + ) + app.state.security_audit = security_audit hooks = HookManager(audit_store=audit) # In-memory live status for agents (updated by POST /agents/{name}/status). @@ -2777,6 +2839,9 @@ async def api_info(): """Health check and server info (JSON).""" channel = os.environ.get("PINKYBOT_CHANNEL", "stable") rate_limits = _read_rate_limits() + # Surface the active posture so VPS operators can confirm hardening + # is applied without scraping startup logs (#434 review followup). + _mode = getattr(app.state, "deployment_mode", None) info = { "name": "pinky", "version": _pinky_version, @@ -2784,6 +2849,7 @@ async def api_info(): "git_hash": _git_hash, "git_branch": _git_branch, "channel": channel, + "deployment_mode": _mode.value if _mode is not None else None, "sessions": manager.count, "started_at": _server_started_at, } diff --git a/src/pinky_daemon/boot_guards.py b/src/pinky_daemon/boot_guards.py new file mode 100644 index 00000000..626c2217 --- /dev/null +++ b/src/pinky_daemon/boot_guards.py @@ -0,0 +1,356 @@ +"""Refuse-to-boot guards for insecure web-mode configurations (#441). + +When ``PINKY_DEPLOYMENT_MODE=web``, the daemon must not start with a +configuration that's obviously unsafe for an internet-exposed deployment. +Loud failure beats silent surprise: an operator who pastes a config into a +VPS and runs the daemon should get a single rejection with the full list +of fixes needed, not discover three months later that their session +secret was the placeholder. + +This module ships the guards that **don't depend on unshipped pieces of +the #433 track**. The remaining guards land alongside their owning PR +(``users`` table → #435; ``PINKY_UI_PASSWORD`` legacy check → #435; audit +emission for overrides → #440). Each is marked with a TODO referencing +its owner. + +Usage +----- +:func:`assert_web_mode_safe` is invoked from +``pinky_daemon.__main__._run_api`` after argparse + mode resolution and +before ``uvicorn.run``. It raises :class:`BootGuardError` listing every +failed guard. The CLI catches the error, prints the message, and exits +with code 3 — distinct from the existing exit code 2 used for invalid +``PINKY_DEPLOYMENT_MODE`` (#434). + +Override mechanism +------------------ +Each guard has a specific override env var +(``PINKY_ALLOW_INSECURE_=true``). There is **no** global escape +hatch by design — operators that need to bypass a guard for an emergency +must do so per-guard, and each override is logged loudly at startup. +Once #440 lands, overrides will also emit an audit event. + +Guards (this PR) +---------------- +* ``SESSION_SECRET`` — ``PINKY_SESSION_SECRET`` unset, shorter than 32 + chars, or one of a small set of obvious placeholder values. +* ``PUBLIC_BIND_NO_TLS`` — bound to ``0.0.0.0`` (or any non-loopback + host) without ``PINKY_BEHIND_TLS_PROXY=true`` to acknowledge that TLS + is being terminated upstream. +* ``MISSING_TRUSTED_PROXIES`` — ``PINKY_TRUSTED_PROXIES`` empty when + ``PINKY_BEHIND_TLS_PROXY=true``. Without it, audit and rate-limit + attribution silently flatten to ``127.0.0.1``. +* ``DB_PERMISSIONS`` — DB file or its parent directory is world- or + group-readable / writable (mode bits & ``0o077`` != 0). + +Warnings (non-fatal, this PR) +----------------------------- +* ``RUNNING_AS_ROOT`` — daemon process UID == 0. + +Deferred to follow-up PRs +------------------------- +* ``ADMIN_USER_EXISTS`` — needs the ``users`` table from #435. +* ``LEGACY_UI_PASSWORD`` — needs the migration story in #435. +* ``MULTI_WORKER_NO_REDIS`` — needs the rate-limit backend selection + from #438. +* Audit emission for overrides — needs the audit writer from #440. + +Part of #433. Issue #441. +""" + +from __future__ import annotations + +import logging +import os +import sys +from collections.abc import Iterable +from dataclasses import dataclass +from pathlib import Path + +from pinky_daemon.config import DeploymentMode + +__all__ = [ + "BOOT_GUARD_EXIT_CODE", + "BootGuardError", + "GuardResult", + "assert_web_mode_safe", + "format_guard_error", + "run_warning_guards", +] + +logger = logging.getLogger(__name__) + +#: Exit code when a boot guard fails. Distinct from invalid-mode (2) +#: established in #434. +BOOT_GUARD_EXIT_CODE = 3 + + +# Placeholder session-secret values that should be rejected even when long +# enough. These are not exhaustive, but they catch the obvious copy-from- +# README cases. +_PLACEHOLDER_SECRETS = frozenset( + { + "changeme", + "change-me", + "secret", + "pinky", + "password", + "default", + "admin", + "test", + } +) + + +@dataclass(frozen=True) +class GuardResult: + """Outcome of a single boot guard. + + ``ok=True`` means the guard passed. ``ok=False`` with a populated + ``message`` is a fatal config problem; the operator sees the message in + the rejection block. ``override_used=True`` is for fatal-but-overridden + cases (logged loudly). + """ + + name: str + ok: bool + message: str = "" + override_used: bool = False + + +class BootGuardError(RuntimeError): + """Raised when at least one fatal boot guard fails. + + Carries the list of failures so the CLI can format a single rejection + block. The message is preformatted via :func:`format_guard_error`. + """ + + def __init__(self, results: list[GuardResult]): + self.results = results + super().__init__(format_guard_error(results)) + + +# ── Individual guards ──────────────────────────────────────────── + + +def _override_active(env: dict, name: str) -> bool: + raw = (env.get(f"PINKY_ALLOW_INSECURE_{name}") or "").strip().lower() + return raw in ("1", "true", "yes", "on") + + +def check_session_secret(env: dict) -> GuardResult: + name = "SESSION_SECRET" + secret = (env.get("PINKY_SESSION_SECRET") or "").strip() + if not secret: + message = ( + "PINKY_SESSION_SECRET is unset. Set it to a 32+ char " + "random value (e.g. `python -c 'import secrets; " + "print(secrets.token_urlsafe(48))'`)." + ) + return _maybe_override(env, name, message) + if len(secret) < 32: + message = ( + f"PINKY_SESSION_SECRET is too short ({len(secret)} chars; " + "need 32+)." + ) + return _maybe_override(env, name, message) + lowered = secret.lower() + if lowered in _PLACEHOLDER_SECRETS: + message = ( + f"PINKY_SESSION_SECRET is set to the placeholder value " + f"{secret!r}. Generate a random value before deploying." + ) + return _maybe_override(env, name, message) + # Trivial repetition (all same character) — caught even when long + # enough. + if len(set(secret)) <= 2: + message = ( + "PINKY_SESSION_SECRET has too little entropy " + "(uses 2 or fewer distinct characters)." + ) + return _maybe_override(env, name, message) + return GuardResult(name=name, ok=True) + + +def check_public_bind_requires_tls_marker( + env: dict, host: str +) -> GuardResult: + """Refuse 0.0.0.0 (or any non-loopback bind) without TLS marker.""" + name = "PUBLIC_BIND_NO_TLS" + # Loopback binds are always fine: no internet exposure regardless of + # TLS marker. + if host in ("127.0.0.1", "::1", "localhost"): + return GuardResult(name=name, ok=True) + behind_tls = ( + env.get("PINKY_BEHIND_TLS_PROXY", "").strip().lower() + in ("1", "true", "yes", "on") + ) + if behind_tls: + return GuardResult(name=name, ok=True) + message = ( + f"web mode is bound to {host} without PINKY_BEHIND_TLS_PROXY=true. " + "Either bind to 127.0.0.1 (and let a TLS-terminating reverse proxy " + "forward), or set PINKY_BEHIND_TLS_PROXY=true to acknowledge TLS " + "is terminated upstream." + ) + return _maybe_override(env, name, message) + + +def check_trusted_proxies_when_behind_tls(env: dict) -> GuardResult: + """When behind a TLS proxy, the trusted-proxy list must be populated. + + Otherwise audit and rate-limit attribution flatten to 127.0.0.1 because + the canonical client-IP resolver (#442) refuses to honor XFF/Forwarded + from an untrusted peer. + """ + name = "MISSING_TRUSTED_PROXIES" + behind_tls = ( + env.get("PINKY_BEHIND_TLS_PROXY", "").strip().lower() + in ("1", "true", "yes", "on") + ) + if not behind_tls: + return GuardResult(name=name, ok=True) + if (env.get("PINKY_TRUSTED_PROXIES") or "").strip(): + return GuardResult(name=name, ok=True) + message = ( + "PINKY_BEHIND_TLS_PROXY=true but PINKY_TRUSTED_PROXIES is empty. " + "Set PINKY_TRUSTED_PROXIES to the CIDR(s) of your reverse proxy " + "(e.g. PINKY_TRUSTED_PROXIES=127.0.0.1/32) so client-IP attribution " + "works. Without this, audit and rate-limit will see only the proxy." + ) + return _maybe_override(env, name, message) + + +def check_db_permissions( + env: dict, db_path: str | os.PathLike +) -> GuardResult: + """Refuse to start when the DB or its parent dir has loose permissions. + + On Windows ``stat`` mode bits don't carry POSIX semantics; the guard + no-ops there. + """ + name = "DB_PERMISSIONS" + if sys.platform == "win32": + return GuardResult(name=name, ok=True) + + path = Path(db_path) + target = path if path.exists() else path.parent + # If neither exists yet, the daemon hasn't initialised — skip. The + # next boot after init will catch it. + if not target.exists(): + return GuardResult(name=name, ok=True) + mode = target.stat().st_mode & 0o777 + # Reject group or world read/write/exec bits. + if mode & 0o077: + message = ( + f"{target} has loose permissions ({oct(mode)}); should be 0o700 " + "(directory) or 0o600 (file). Run " + f"`chmod go-rwx {target}` and any sibling secret files." + ) + return _maybe_override(env, name, message) + return GuardResult(name=name, ok=True) + + +def _maybe_override(env: dict, name: str, message: str) -> GuardResult: + """Convert a fatal message into a passing-but-overridden result if the + operator set the per-guard override.""" + if _override_active(env, name): + logger.warning( + "boot_guards: override active for %s — %s", + name, + message, + ) + return GuardResult(name=name, ok=True, override_used=True) + return GuardResult(name=name, ok=False, message=message) + + +# ── Warning guards (non-fatal) ──────────────────────────────────── + + +def warn_running_as_root(env: dict) -> str | None: + """Return a warning string when the daemon is running as UID 0. + + Skipped on platforms without ``geteuid``. + """ + if not hasattr(os, "geteuid"): + return None + if os.geteuid() != 0: + return None + if _override_active(env, "RUNNING_AS_ROOT"): + return None + return ( + "boot_guards: warning: daemon is running as root. Strongly " + "recommended to drop privileges or run under a dedicated service " + "user. Set PINKY_ALLOW_INSECURE_RUNNING_AS_ROOT=true to silence." + ) + + +def run_warning_guards(env: dict | None = None) -> list[str]: + """Run all non-fatal warning guards. Return any messages produced.""" + source = env if env is not None else os.environ + out: list[str] = [] + for guard in (warn_running_as_root,): + msg = guard(source) + if msg: + out.append(msg) + return out + + +# ── Top-level entrypoint ───────────────────────────────────────── + + +def _gather_web_guards( + env: dict, host: str, db_path: str | os.PathLike +) -> Iterable[GuardResult]: + yield check_session_secret(env) + yield check_public_bind_requires_tls_marker(env, host) + yield check_trusted_proxies_when_behind_tls(env) + yield check_db_permissions(env, db_path) + + +def assert_web_mode_safe( + mode: DeploymentMode, + host: str, + db_path: str | os.PathLike, + env: dict | None = None, +) -> None: + """Run all web-mode boot guards. No-op outside ``web`` mode. + + Raises: + BootGuardError: if any guard fails. The error message is the + preformatted rejection block (see :func:`format_guard_error`) + and ``error.results`` carries the structured per-guard + outcomes. + """ + if mode is not DeploymentMode.WEB: + return + source = env if env is not None else os.environ + results = list(_gather_web_guards(source, host, db_path)) + failed = [r for r in results if not r.ok] + if failed: + raise BootGuardError(failed) + # Emit any warning guards even on success. + for warning in run_warning_guards(source): + logger.warning(warning) + + +def format_guard_error(results: list[GuardResult]) -> str: + """Format a list of failing guards as a single stderr-friendly block. + + Mirrors the format defined in #441's spec. + """ + lines = [ + "ERROR: refusing to start in 'web' deployment mode with insecure config:" + ] + for r in results: + lines.append(f" - [{r.name}] {r.message}") + lines.append("") + lines.append( + "Fix the above and restart, or set PINKY_DEPLOYMENT_MODE=trusted " + "for the legacy posture." + ) + lines.append( + "Per-guard overrides exist for emergencies " + "(PINKY_ALLOW_INSECURE_=true) and are logged loudly." + ) + return "\n".join(lines) diff --git a/src/pinky_daemon/config.py b/src/pinky_daemon/config.py new file mode 100644 index 00000000..46900e5b --- /dev/null +++ b/src/pinky_daemon/config.py @@ -0,0 +1,148 @@ +"""Deployment posture configuration. + +Single source of truth for the security posture PinkyBot is running under. +Resolved once at startup, cached on ``app.state.deployment_mode``, and consumed +by the hardening middleware introduced in the rest of the #433 track: + +* #436 — LAN-only mode + private-CIDR middleware + bind defaults +* #437 — Security headers middleware +* #439 — Elevated-session protection for dangerous routes +* #441 — Refuse-to-boot guards for insecure web-mode configs + +Each downstream feature should branch on ``app.state.deployment_mode`` rather +than reading its own env knob, so operator intent is encoded in exactly one +place. + +Modes +----- +``trusted`` + Default. Matches today's Mac Mini deployment: trusted LAN, single shared + cookie session, no extra network filter, no bearer-token API auth. + +``lan`` + LAN appliance posture (Raspberry Pi, NAS, home server). Cookie auth still + required, plus a private-CIDR middleware to refuse non-private clients. + +``web`` + Internet-exposed VPS. Bind defaults to loopback (assumes a reverse proxy + in front), bearer-token API auth is enabled, admin routes require an + elevated session, full security headers, refuse-to-boot guards apply. + +Resolution rules +---------------- +* Source: ``PINKY_DEPLOYMENT_MODE`` env var. +* Empty/unset → ``trusted`` (back-compat). +* Case-insensitive, surrounding whitespace stripped. +* Invalid value → :class:`InvalidDeploymentModeError` (fail closed; do **not** + silently fall back to ``trusted``). The daemon entrypoint is expected to + catch this and abort startup with a useful message. + +Part of #433 (Hardening: PinkyBot for web-exposed deployments). Issue #434. +""" + +from __future__ import annotations + +import os +from enum import Enum + +__all__ = [ + "DEPLOYMENT_MODE_ENV", + "DeploymentMode", + "InvalidDeploymentModeError", + "default_bind_host", + "resolve_deployment_mode", + "warn_if_insecure_exposure", +] + +#: Canonical env var name. Imported by other modules so the spelling lives in +#: one place. +DEPLOYMENT_MODE_ENV = "PINKY_DEPLOYMENT_MODE" + + +class DeploymentMode(str, Enum): + """Operator-declared deployment posture.""" + + TRUSTED = "trusted" + LAN = "lan" + WEB = "web" + + def __str__(self) -> str: # pragma: no cover - cosmetic + return self.value + + +_VALID_VALUES = tuple(m.value for m in DeploymentMode) + + +class InvalidDeploymentModeError(ValueError): + """Raised when ``PINKY_DEPLOYMENT_MODE`` has an unrecognized value. + + Fail-closed posture: a typo like ``PINKY_DEPLOYMENT_MODE=prod`` must abort + startup, not silently fall back to ``trusted`` and hand the operator a + false sense of hardening. + """ + + +def resolve_deployment_mode(env: dict | None = None) -> DeploymentMode: + """Resolve the deployment mode from ``PINKY_DEPLOYMENT_MODE``. + + Pure function. Reads from the supplied mapping (defaults to ``os.environ``), + normalises case + whitespace, returns the matching :class:`DeploymentMode`. + + Raises: + InvalidDeploymentModeError: when the env var is set but doesn't match + one of the canonical values. No aliases are accepted (no "prod", + no "production"). Add aliases only if we commit to supporting them + long-term. + """ + source = env if env is not None else os.environ + raw = (source.get(DEPLOYMENT_MODE_ENV) or "").strip().lower() + if not raw: + return DeploymentMode.TRUSTED + try: + return DeploymentMode(raw) + except ValueError as e: + raise InvalidDeploymentModeError( + f"{DEPLOYMENT_MODE_ENV}={raw!r} is not a valid deployment mode. " + f"Expected one of: {', '.join(_VALID_VALUES)}." + ) from e + + +def default_bind_host(mode: DeploymentMode) -> str: + """Default ``--host`` for a given mode. + + Used by the daemon entrypoint when the operator did **not** pass an + explicit ``--host`` (i.e. ``--host`` was left at its sentinel ``None``). + The asymmetry between operator-supplied and inherited defaults matters for + #436 (bind defaults) and #441 (refuse-to-boot guards), which need to tell + "operator chose 0.0.0.0" from "old hardcoded default". + + * ``web`` → ``127.0.0.1`` — assumes a reverse proxy is in front. Refuses + to bind public interfaces by default. + * ``trusted`` / ``lan`` → ``0.0.0.0`` — back-compat with today's Mac Mini + and Pi-fleet deployments. + """ + if mode is DeploymentMode.WEB: + return "127.0.0.1" + return "0.0.0.0" + + +def warn_if_insecure_exposure(mode: DeploymentMode, host: str) -> str | None: + """Build a loud warning string when ``trusted`` mode binds all interfaces. + + Today's Mac Mini deployment runs ``trusted`` + ``0.0.0.0`` and there's no + intent to break that. But if PinkyBot ends up on an internet-reachable host + by accident (or a fresh operator copies the README config), the operator + should see a screaming line in the logs at startup rather than discovering + the exposure later. + + Returns ``None`` when no warning is needed. Caller is responsible for + surfacing the string (typically ``print(... , file=sys.stderr)``). + """ + if mode is DeploymentMode.TRUSTED and host == "0.0.0.0": + return ( + f"⚠️ PINKY_DEPLOYMENT_MODE={mode.value} + --host=0.0.0.0: " + "the API is bound to all interfaces under the trusted-LAN posture. " + "If this host is internet-reachable, set " + f"{DEPLOYMENT_MODE_ENV}=web and put PinkyBot behind a reverse proxy." + ) + return None diff --git a/src/pinky_daemon/middleware/__init__.py b/src/pinky_daemon/middleware/__init__.py new file mode 100644 index 00000000..880c6169 --- /dev/null +++ b/src/pinky_daemon/middleware/__init__.py @@ -0,0 +1,7 @@ +"""HTTP middleware modules for the pinky daemon. + +Currently houses :mod:`pinky_daemon.middleware.security_headers` (#437). +Future cross-cutting middleware introduced by the #433 hardening track +(rate limiting #438, CSRF #443, elevated-session enforcement #439) should +live here too. +""" diff --git a/src/pinky_daemon/middleware/csrf.py b/src/pinky_daemon/middleware/csrf.py new file mode 100644 index 00000000..87fad6e7 --- /dev/null +++ b/src/pinky_daemon/middleware/csrf.py @@ -0,0 +1,243 @@ +"""CSRF protection for cookie-authenticated state-changing endpoints (#443). + +Pattern +------- +Double-submit cookie + header. On every request: + +* If the caller carries a session cookie but no ``pinky_csrf`` cookie, + the response sets one — a 32-byte URL-safe random token, + ``SameSite=Strict``, **not** HttpOnly so the SPA can read it. +* On every unsafe verb (POST / PUT / PATCH / DELETE) the middleware + compares the ``X-Pinky-CSRF`` header against the ``pinky_csrf`` cookie. + Mismatch → ``HTTP 403 {"error": "csrf_check_failed"}``. + +Why not pure SameSite +--------------------- +``SameSite=Strict`` blocks top-level cross-site POSTs in modern +browsers, but leaves real gaps: + +* Older browsers without enforcement +* Multi-window cross-tab flows +* Subdomain-trust attacks if PinkyBot ever shares a parent domain +* OAuth/redirect flows that intentionally use ``SameSite=Lax`` + +Once #439's elevation endpoints exist, an explicit token is +required defense-in-depth. + +Exemptions +---------- +* **Bearer-token authentication.** No ambient credential vulnerability + — the attacker has to steal the bearer token, at which point they + don't need CSRF. Detected via the ``Authorization`` header. +* **Webhook routes** (``/triggers/webhook/*``) — signature/token gated + upstream of any cookie auth. +* **Twilio callbacks** (``/twilio/*``) — Twilio signature gated. +* **Internal MCP-signed requests** (``X-Pinky-Agent`` + signature + headers from :mod:`pinky_daemon.auth`). +* **Trusted mode** — middleware off for back-compat. + +Token rotation +-------------- +The cookie value is set fresh whenever a request with a session cookie +arrives without a CSRF cookie. Login (which clears the session and +sets a new one) naturally rotates: the next response sees a session +without CSRF and writes a new token. An explicit +``/auth/csrf/refresh`` endpoint is out of scope for this PR; the +session-bound rotation is enough for the routes shipping today. + +Audit +----- +Every CSRF reject emits an audit event into ``app.state.security_audit`` +when one is wired (no-op otherwise). Event type +``auth.csrf.rejected`` is registered in +:data:`pinky_daemon.security_audit.EVENT_DETAIL_ALLOWLIST`-extension +patterns; until that allowlist gains the entry, the redactor falls +back to the conservative default. + +Part of #433. Issue #443. +""" + +from __future__ import annotations + +import logging +import secrets +from collections.abc import Awaitable, Callable + +from fastapi import Request + +from pinky_daemon.auth import ( + INTERNAL_AGENT_HEADER, + INTERNAL_SIGNATURE_HEADER, + SESSION_COOKIE_NAME, +) +from pinky_daemon.config import DeploymentMode + +__all__ = [ + "CSRF_COOKIE_NAME", + "CSRF_HEADER_NAME", + "UNSAFE_METHODS", + "build_csrf_middleware", + "generate_csrf_token", + "is_exempt_path", +] + +logger = logging.getLogger(__name__) + +CSRF_COOKIE_NAME = "pinky_csrf" +CSRF_HEADER_NAME = "x-pinky-csrf" + +#: HTTP verbs that mutate state and therefore require a token. +UNSAFE_METHODS: frozenset[str] = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + +#: Path prefixes that bypass CSRF (their own auth/signature gates). +_EXEMPT_PREFIXES: tuple[str, ...] = ( + "/triggers/webhook/", + "/twilio/", +) + + +def generate_csrf_token() -> str: + """Cryptographically random URL-safe CSRF token.""" + return secrets.token_urlsafe(32) + + +def is_exempt_path(path: str) -> bool: + """``True`` when the path is auth-gated by something other than a + cookie session (webhooks, Twilio callbacks).""" + return any(path.startswith(prefix) for prefix in _EXEMPT_PREFIXES) + + +def _is_bearer_or_internal_authed(request: Request) -> bool: + """``True`` when the request authenticates via something other than a + cookie session — bearer token or signed internal MCP call.""" + auth = request.headers.get("authorization", "").strip() + if auth.lower().startswith("bearer "): + return True + if request.headers.get(INTERNAL_AGENT_HEADER) and request.headers.get( + INTERNAL_SIGNATURE_HEADER + ): + return True + return False + + +def _has_session_cookie(request: Request) -> bool: + return bool(request.cookies.get(SESSION_COOKIE_NAME)) + + +def _csrf_cookie_kwargs(*, mode: DeploymentMode) -> dict: + """Cookie attribute dict for the CSRF token. Secure flag is mode-aware + so localhost dev (trusted/lan) doesn't need TLS.""" + return { + "key": CSRF_COOKIE_NAME, + "samesite": "strict", + "httponly": False, # frontend reads it + "secure": mode is DeploymentMode.WEB, + "path": "/", + } + + +def build_csrf_middleware(): + """Return an ASGI middleware closure enforcing the CSRF token check. + + Trusted mode short-circuits to a no-op (back-compat). LAN and Web + modes apply the check. + + Token issuance happens unconditionally for any request that carries + a session cookie but no CSRF cookie — the response Set-Cookies a + fresh token. That covers initial login (no cookie yet) and rotation + after explicit logout. + """ + + async def csrf_mw( + request: Request, + call_next: Callable[[Request], Awaitable], + ): + state = request.app.state if request.app else None + mode = getattr(state, "deployment_mode", DeploymentMode.TRUSTED) + if mode is DeploymentMode.TRUSTED: + return await call_next(request) + + method = request.method.upper() + path = request.url.path + + # Determine whether to enforce on this request. + enforce = ( + method in UNSAFE_METHODS + and not is_exempt_path(path) + and not _is_bearer_or_internal_authed(request) + and _has_session_cookie(request) + ) + + if enforce: + cookie_token = request.cookies.get(CSRF_COOKIE_NAME) or "" + header_token = request.headers.get(CSRF_HEADER_NAME) or "" + if ( + not cookie_token + or not header_token + or not secrets.compare_digest(cookie_token, header_token) + ): + _emit_reject_audit(state, request, has_cookie=bool(cookie_token)) + from fastapi.responses import JSONResponse + + return JSONResponse( + { + "error": "csrf_check_failed", + "detail": ( + "missing or invalid CSRF token; refresh the page " + "to obtain a new token" + ), + }, + status_code=403, + ) + + # Pass to the route. Issue a token cookie afterwards if the + # caller had a session but no token yet. + response = await call_next(request) + + if _has_session_cookie(request) and not request.cookies.get( + CSRF_COOKIE_NAME + ): + response.set_cookie( + value=generate_csrf_token(), + **_csrf_cookie_kwargs(mode=mode), + ) + + return response + + return csrf_mw + + +# ── Audit emission ─────────────────────────────────────────────── + + +def _emit_reject_audit( + state, request: Request, *, has_cookie: bool +) -> None: + """Best-effort audit emission for a CSRF rejection.""" + sec_audit = getattr(state, "security_audit", None) if state else None + if sec_audit is None: + return + try: + from pinky_daemon.net.client_identity import resolve_client_identity + from pinky_daemon.security_audit import AuditOutcome + + trusted = ( + getattr(state, "trusted_proxies", None) if state else None + ) or () + mode = getattr(state, "deployment_mode", DeploymentMode.TRUSTED) + ci = resolve_client_identity(request, mode, trusted) + sec_audit.log_event( + "auth.csrf.rejected", + outcome=AuditOutcome.DENIED, + actor_ip=str(ci.ip), + via_proxy=ci.via_proxy, + forwarded_chain=[str(h) for h in ci.forwarded_chain] + if ci.forwarded_chain + else None, + method=request.method, + target=request.url.path, + user_agent=request.headers.get("user-agent"), + detail={"reason": "missing" if not has_cookie else "mismatch"}, + ) + except Exception: # noqa: BLE001 + logger.exception("csrf: failed to emit audit event") diff --git a/src/pinky_daemon/middleware/rate_limit.py b/src/pinky_daemon/middleware/rate_limit.py new file mode 100644 index 00000000..3acabbdd --- /dev/null +++ b/src/pinky_daemon/middleware/rate_limit.py @@ -0,0 +1,304 @@ +"""Auth / admin rate limiting middleware (#438). + +Slows brute-force on login and burst-probing on admin endpoints. Mode-aware: + +* ``trusted`` — disabled; back-compat with the Mac Mini deployment. +* ``lan`` — auth + admin buckets active. Default-IP bucket also on. +* ``web`` — all buckets active. + +Keys +---- +The IP for any IP-keyed bucket is the canonical client IP from the #442 +resolver, **not** ``request.client.host`` and **not** raw +``X-Forwarded-For``. That's the only attribution surface that's safe +once a reverse proxy is involved. + +Buckets +------- +============ ================================================ ======== ============= ================== +Bucket Routes Limit Window Lockout penalty +============ ================================================ ======== ============= ================== +``auth`` ``/auth/login`` POST, ``/auth/password`` POST 5 300s 900s after exceed +``admin`` prefix ``/admin/`` 30 60s none (429 only) +``webhook`` prefix ``/triggers/webhook/`` 60 60s none (IP-keyed for + now; switch to token + hash when #435 lands) +``default`` everything else 300 60s none (lan/web only) +============ ================================================ ======== ============= ================== + +Backend +------- +**In-memory only** in this PR. The store is per-process; multi-worker +deployments are not yet supported in ``web`` mode (#441 will add a +guard once Redis backend lands). The single-uvicorn-worker default +the daemon ships with is fine. + +Login keying refinement +----------------------- +Per Murzik's review note: login limiting should key on both source IP +**and** normalised email so neither shared NAT nor distributed brute +force gets a free pass. This PR ships IP-only keying since the +username/email shape arrives with #435; a TODO marks the second key +addition for the follow-up. + +Audit +----- +Every 429 emits a ``rate_limit.exceeded`` event into the security +audit log (#440) when one is wired through. Audit emission is a +no-op when ``security_audit`` isn't on ``app.state``, so this +middleware works in isolation. + +Part of #433. Issue #438. +""" + +from __future__ import annotations + +import logging +import time +from collections import deque +from collections.abc import Awaitable, Callable +from dataclasses import dataclass, field +from threading import Lock + +from fastapi import Request +from fastapi.responses import JSONResponse + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.net.client_identity import resolve_client_identity + +__all__ = [ + "DEFAULT_BUCKETS", + "BucketConfig", + "InMemoryRateLimiter", + "build_rate_limit_middleware", + "classify_request", +] + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class BucketConfig: + """Per-bucket rate-limit configuration.""" + + name: str + limit: int + window_seconds: float + penalty_seconds: float = 0.0 # extra lockout after exceed (auth bucket) + + +#: Bucket definitions. The order is the classification priority — first +#: matching predicate wins. Webhook before admin before default so +#: ``/triggers/webhook/foo`` doesn't fall into the admin bucket if some +#: future route ever overlaps. +DEFAULT_BUCKETS: tuple[BucketConfig, ...] = ( + BucketConfig(name="auth", limit=5, window_seconds=300.0, + penalty_seconds=900.0), + BucketConfig(name="webhook", limit=60, window_seconds=60.0), + BucketConfig(name="admin", limit=30, window_seconds=60.0), + BucketConfig(name="default", limit=300, window_seconds=60.0), +) + + +_AUTH_PATHS = ("/auth/login", "/auth/password") +_ADMIN_PREFIX = "/admin/" +_WEBHOOK_PREFIX = "/triggers/webhook/" + + +def classify_request(method: str, path: str) -> str: + """Decide which bucket a request belongs to. Pure function for tests.""" + method = method.upper() + if method == "POST" and path in _AUTH_PATHS: + return "auth" + if path.startswith(_WEBHOOK_PREFIX): + return "webhook" + if path.startswith(_ADMIN_PREFIX): + return "admin" + return "default" + + +def _bucket_active_in_mode(bucket: str, mode: DeploymentMode) -> bool: + if mode is DeploymentMode.TRUSTED: + return False + if mode is DeploymentMode.LAN: + # LAN: auth + admin only. Default + webhook are off so behind-LAN + # internal traffic isn't accidentally throttled. + return bucket in ("auth", "admin") + # web: everything + return True + + +# ── Token store ────────────────────────────────────────────────── + + +@dataclass +class _BucketState: + """Per-key state: a deque of recent timestamps + an optional lockout.""" + + timestamps: deque = field(default_factory=deque) + locked_until: float = 0.0 + + +class InMemoryRateLimiter: + """Sliding-window-counter rate limiter. + + Process-local. Thread-safe via a single mutex (rate-limit checks are + cheap; the mutex is held for microseconds). Multi-worker deployments + will need a Redis backend; this PR documents the limitation. + """ + + def __init__(self, buckets: tuple[BucketConfig, ...] = DEFAULT_BUCKETS): + self._configs = {b.name: b for b in buckets} + self._state: dict[tuple[str, str], _BucketState] = {} + self._lock = Lock() + + def configs(self) -> dict[str, BucketConfig]: + return dict(self._configs) + + def check( + self, + bucket: str, + key: str, + *, + now: float | None = None, + ) -> tuple[bool, float]: + """Return ``(allowed, retry_after_seconds)`` for one request. + + ``allowed=True`` means the request is under the limit and the + timestamp is recorded. ``allowed=False`` means it exceeded; the + caller should respond with 429 + ``Retry-After: ceil(retry_after)``. + + The auth bucket additionally activates a lockout window after the + limit is exceeded; subsequent requests during the lockout return + ``False`` immediately without recording new timestamps. + """ + config = self._configs.get(bucket) + if config is None: + return True, 0.0 + n = now if now is not None else time.monotonic() + + with self._lock: + state = self._state.setdefault((bucket, key), _BucketState()) + + # Drop timestamps that have rolled out of the window. + cutoff = n - config.window_seconds + while state.timestamps and state.timestamps[0] < cutoff: + state.timestamps.popleft() + + # Lockout still active? + if state.locked_until > n: + return False, state.locked_until - n + + if len(state.timestamps) >= config.limit: + # Limit exceeded — apply penalty if configured. + if config.penalty_seconds > 0: + state.locked_until = n + config.penalty_seconds + retry_after = config.penalty_seconds + else: + retry_after = state.timestamps[0] + config.window_seconds - n + return False, max(retry_after, 0.0) + + state.timestamps.append(n) + return True, 0.0 + + def reset(self, bucket: str, key: str) -> None: + """Clear state for one (bucket, key). Test helper / admin override.""" + with self._lock: + self._state.pop((bucket, key), None) + + +# ── Middleware factory ─────────────────────────────────────────── + + +_RETRY_BODY_TEMPLATE = { + "auth": "too many attempts, try again in {n} seconds", + "admin": "too many requests, try again in {n} seconds", + "webhook": "too many requests, try again in {n} seconds", + "default": "too many requests, try again in {n} seconds", +} + + +def build_rate_limit_middleware( + *, + limiter: InMemoryRateLimiter | None = None, +): + """Return an ASGI middleware closure that enforces the bucket limits. + + Args: + limiter: Optional explicit limiter instance. When ``None`` the + middleware reads from ``app.state.rate_limiter`` per request, + so a single process can hot-swap config without recreating + the middleware. ``create_api`` always pre-installs one. + """ + + async def rate_limit_mw( + request: Request, + call_next: Callable[[Request], Awaitable], + ): + state = request.app.state if request.app else None + mode = getattr(state, "deployment_mode", DeploymentMode.TRUSTED) + if mode is DeploymentMode.TRUSTED: + return await call_next(request) + + bucket = classify_request(request.method, request.url.path) + if not _bucket_active_in_mode(bucket, mode): + return await call_next(request) + + active_limiter = limiter or getattr(state, "rate_limiter", None) + if active_limiter is None: + return await call_next(request) + + trusted_proxies = ( + getattr(state, "trusted_proxies", None) if state else None + ) or () + ci = resolve_client_identity(request, mode, trusted_proxies) + # All buckets keyed on canonical IP for now. #435 will refine + # admin → user_id and webhook → token-hash-prefix. + key = str(ci.ip) + + allowed, retry_after = active_limiter.check(bucket, key) + if allowed: + return await call_next(request) + + retry_after_int = max(int(retry_after) + 1, 1) + msg = _RETRY_BODY_TEMPLATE.get( + bucket, _RETRY_BODY_TEMPLATE["default"] + ).format(n=retry_after_int) + + # Best-effort security-audit emission. Log even when audit isn't + # wired so the operator sees the trip in the standard log. + logger.info( + "rate_limit: bucket=%s key=%s exceeded (mode=%s, path=%s)", + bucket, key, mode.value, request.url.path, + ) + sec_audit = getattr(state, "security_audit", None) if state else None + if sec_audit is not None: + try: + from pinky_daemon.security_audit import AuditOutcome + + sec_audit.log_event( + "rate_limit.exceeded", + outcome=AuditOutcome.DENIED, + actor_ip=str(ci.ip), + via_proxy=ci.via_proxy, + forwarded_chain=[str(h) for h in ci.forwarded_chain] + if ci.forwarded_chain + else None, + method=request.method, + route_name=bucket, + target=request.url.path, + detail={ + "bucket": bucket, + "limit": active_limiter.configs()[bucket].limit, + }, + ) + except Exception: # noqa: BLE001 + logger.exception("rate_limit: failed to emit audit event") + + return JSONResponse( + {"error": "rate_limited", "detail": msg}, + status_code=429, + headers={"Retry-After": str(retry_after_int)}, + ) + + return rate_limit_mw diff --git a/src/pinky_daemon/middleware/security_headers.py b/src/pinky_daemon/middleware/security_headers.py new file mode 100644 index 00000000..a6ccdd81 --- /dev/null +++ b/src/pinky_daemon/middleware/security_headers.py @@ -0,0 +1,238 @@ +"""Security headers middleware (#437). + +Helmet-style HTTP response headers, gated by deployment mode. The ``trusted`` +posture stays minimal (we don't want to break the existing Mac Mini SPA in +ways the operator wouldn't notice); ``lan`` adds clickjacking + referrer +hardening; ``web`` adds HSTS and a Content Security Policy. + +Header matrix +------------- +============================ ======== ===== ===== +Header trusted lan web +============================ ======== ===== ===== +X-Content-Type-Options ✓ ✓ ✓ +X-Frame-Options ✗ ✓ ✓ +Referrer-Policy ✗ ✓ ✓ +Permissions-Policy ✗ ✓ ✓ +Content-Security-Policy ✗ basic strict +Strict-Transport-Security ✗ ✗ ✓ (verified-HTTPS only) +X-XSS-Protection ``0`` ``0`` ``0`` +============================ ======== ===== ===== + +``X-XSS-Protection: 0`` is the modern recommendation: the legacy auditor in +old browsers introduced its own XSS class and OWASP / MDN now recommend +disabling it explicitly. See OWASP "Secure Headers" project, 2024 guidance. + +CSP enforcement +--------------- +Lands in **report-only** mode by default (``Content-Security-Policy-Report-Only``). +Operators flip to enforcing via :data:`CSP_ENFORCING_ENV`. Rationale: the +Svelte SPA uses inline styles deliberately; flipping straight to enforcing +is a recipe for breaking the UI on a Friday. Soak in report-only first, +fix any genuine surprises, then enforce. + +HSTS +---- +Only emitted when the request's HTTPS-ness can be **verified**. A bare +``X-Forwarded-Proto: https`` from an untrusted peer must NOT trigger HSTS, +because an attacker who can MITM your HTTP traffic could pin HSTS for a +year on a victim's browser. The check uses +:func:`pinky_daemon.net.client_identity.resolve_client_identity` so trust +evaluation matches the rest of the #433 track. + +Part of #433. Issue #437. +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Awaitable, Callable + +from fastapi import Request + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.net.client_identity import ( + peer_is_trusted, + raw_peer_from_request, +) + +__all__ = [ + "CSP_ENFORCING_ENV", + "build_csp", + "build_security_headers_middleware", + "headers_for_mode", + "is_verified_https", +] + +logger = logging.getLogger(__name__) + +#: Env var that flips CSP from report-only to enforcing. +CSP_ENFORCING_ENV = "PINKY_CSP_ENFORCING" + + +# ── Header builders ─────────────────────────────────────────────── + + +_PERMISSIONS_POLICY_BASIC = "camera=(), microphone=(), geolocation=()" +_PERMISSIONS_POLICY_STRICT = ( + "camera=(), microphone=(), geolocation=(), interest-cohort=(), " + "payment=(), usb=(), magnetometer=(), accelerometer=(), gyroscope=()" +) + + +def build_csp(mode: DeploymentMode) -> str: + """Build the Content-Security-Policy string for ``mode``. + + Both ``lan`` and ``web`` policies allow ``'unsafe-inline'`` for + ``style-src`` because the Svelte SPA uses inline styles deliberately + (per CLAUDE.md frontend conventions). Removing it requires a refactor + that's out of scope for #437 — call out as known compromise. + + ``web`` also locks down ``object-src`` and adds ``upgrade-insecure-requests``. + """ + common = [ + "default-src 'self'", + "script-src 'self'", + "style-src 'self' 'unsafe-inline'", # Svelte inline styles, see CLAUDE.md + "img-src 'self' data:", + "font-src 'self' data:", + # SSE + websocket to same origin + "connect-src 'self' ws: wss:", + "frame-ancestors 'none'", + "base-uri 'self'", + "form-action 'self'", + ] + if mode is DeploymentMode.WEB: + return "; ".join( + common + ["object-src 'none'", "upgrade-insecure-requests"] + ) + return "; ".join(common) + + +def headers_for_mode(mode: DeploymentMode) -> dict[str, str]: + """Return the static (non-HSTS) header set for a given posture. + + HSTS is intentionally absent — its inclusion depends on per-request + HTTPS-verification logic that lives in the middleware closure. + Callers that just want to inspect "what headers would mode X emit" use + this helper for the static slice. + """ + out: dict[str, str] = { + "X-Content-Type-Options": "nosniff", + "X-XSS-Protection": "0", + } + if mode is DeploymentMode.TRUSTED: + return out + # lan + web share clickjacking + referrer + permissions-policy. + out["X-Frame-Options"] = "DENY" + out["Referrer-Policy"] = "strict-origin-when-cross-origin" + out["Permissions-Policy"] = ( + _PERMISSIONS_POLICY_STRICT + if mode is DeploymentMode.WEB + else _PERMISSIONS_POLICY_BASIC + ) + return out + + +# ── HSTS / HTTPS verification ───────────────────────────────────── + + +def is_verified_https(request: Request) -> bool: + """``True`` when the request can be **verified** to have arrived over HTTPS. + + Two acceptance paths: + + 1. Direct TLS — ``request.url.scheme == "https"``. The ASGI server + (uvicorn behind a TLS-terminating socket) sets this; can't be + spoofed by header. + 2. Trusted reverse proxy — ``X-Forwarded-Proto: https`` AND the + immediate peer is one of the configured trusted proxies (#442). + + A bare ``X-Forwarded-Proto: https`` from an untrusted peer is **not** + enough — that's the HSTS-poisoning vector we explicitly want to avoid. + """ + if request.url.scheme == "https": + return True + + proto = request.headers.get("x-forwarded-proto", "").lower().strip() + if proto != "https": + return False + + # The header says HTTPS — only trust it when it came from a configured + # reverse proxy. Bare X-Forwarded-Proto from an arbitrary peer must NOT + # be enough; that's the HSTS-poisoning vector. + state = request.app.state if request.app else None + trusted = ( + getattr(state, "trusted_proxies", None) if state else None + ) or () + raw_peer = raw_peer_from_request(request) + return peer_is_trusted(raw_peer, trusted) + + +# ── Middleware factory ──────────────────────────────────────────── + + +def _csp_enforcing_from_env(env: dict | None = None) -> bool: + source = env if env is not None else os.environ + raw = (source.get(CSP_ENFORCING_ENV) or "").strip().lower() + return raw in ("1", "true", "yes", "on") + + +def build_security_headers_middleware(*, csp_enforcing: bool | None = None): + """Return an ASGI middleware closure that emits hardened response headers. + + Args: + csp_enforcing: When ``True``, emit ``Content-Security-Policy`` + (enforcing). When ``False``, emit ``Content-Security-Policy-Report-Only``. + When ``None`` (default), read :data:`CSP_ENFORCING_ENV` per + request — slow path but lets tests flip behavior without + recreating the middleware. + + The middleware reads ``app.state.deployment_mode`` per request so the + same closure works in tests that swap modes. Websocket upgrades pass + through untouched (header injection breaks WS handshakes in some + clients). + """ + + async def security_headers_mw( + request: Request, + call_next: Callable[[Request], Awaitable], + ): + # Don't touch WebSocket upgrades — Starlette handles those before + # response middleware anyway, but be explicit. + if request.headers.get("upgrade", "").lower() == "websocket": + return await call_next(request) + + response = await call_next(request) + + state = request.app.state if request.app else None + mode = getattr(state, "deployment_mode", DeploymentMode.TRUSTED) + + for name, value in headers_for_mode(mode).items(): + response.headers[name] = value + + # CSP only in lan/web. Trusted is purely back-compat. + if mode is not DeploymentMode.TRUSTED: + csp_value = build_csp(mode) + enforcing = ( + csp_enforcing + if csp_enforcing is not None + else _csp_enforcing_from_env() + ) + header_name = ( + "Content-Security-Policy" + if enforcing + else "Content-Security-Policy-Report-Only" + ) + response.headers[header_name] = csp_value + + # HSTS only in web mode and only when HTTPS is verified. + if mode is DeploymentMode.WEB and is_verified_https(request): + response.headers["Strict-Transport-Security"] = ( + "max-age=31536000; includeSubDomains" + ) + + return response + + return security_headers_mw diff --git a/src/pinky_daemon/net/__init__.py b/src/pinky_daemon/net/__init__.py new file mode 100644 index 00000000..79472fee --- /dev/null +++ b/src/pinky_daemon/net/__init__.py @@ -0,0 +1,6 @@ +"""Network utilities for the pinky daemon. + +Currently houses :mod:`pinky_daemon.net.client_identity` (the canonical +trusted-client-IP resolver introduced in #442). Future cross-cutting network +helpers should live here too rather than being scattered across ``api.py``. +""" diff --git a/src/pinky_daemon/net/client_identity.py b/src/pinky_daemon/net/client_identity.py new file mode 100644 index 00000000..91e4a132 --- /dev/null +++ b/src/pinky_daemon/net/client_identity.py @@ -0,0 +1,406 @@ +"""Canonical trusted-client-IP resolver. + +Single source of truth for "what is the real client IP for this request?". +Every IP-dependent feature in the #433 hardening track reads from this resolver +instead of parsing ``X-Forwarded-For`` independently: + +* #436 LAN-only middleware (private-CIDR check) +* #438 rate limiting (key extraction) +* #440 audit log (attribution + forensic chain) +* #441 boot guards (trusted-proxy config sanity) + +Centralising the logic keeps the spoofing surface minimal and makes attribution +consistent across subsystems. + +Spec +---- +The behaviour is gated by :class:`pinky_daemon.config.DeploymentMode`: + +============ ========================================================= +Mode Behaviour +============ ========================================================= +``trusted`` ``ip = raw_peer``; ``X-Forwarded-For``/``Forwarded`` + ignored entirely. The trusted-LAN posture has no proxy. +``lan`` ``ip = raw_peer``; ``XFF``/``Forwarded`` parsed and only + honored when ``raw_peer`` is inside one of the operator's + ``PINKY_TRUSTED_PROXIES`` CIDRs. +``web`` Same trust check as ``lan``; otherwise XFF is dropped and + ``via_proxy`` reports ``False``. +============ ========================================================= + +``PINKY_TRUSTED_PROXIES`` is a comma-separated list of CIDRs. Loopback is +**not** trusted by default — operators opt in explicitly so that audit and +rate-limit attribution can't silently flatten to ``127.0.0.1`` when something +goes wrong upstream. + +Header parsing +-------------- +* ``Forwarded`` (RFC 7239) wins over ``X-Forwarded-For`` when both are present. +* ``X-Forwarded-For`` is walked from the right: the rightmost trusted proxy + is skipped, the next hop to the left is the client. If every hop is trusted, + the leftmost IP in the chain is the client. +* IPv6-mapped IPv4 (``::ffff:1.2.3.4``) is normalised to the IPv4 form before + CIDR comparisons so an operator's ``10.0.0.0/8`` rule actually matches. +* Malformed headers cause the parser to drop XFF/Forwarded entirely and fall + back to ``raw_peer``. A warning is logged at most once per minute per + failure class to keep noisy clients from filling logs. + +Part of #433. Issue #442. +""" + +from __future__ import annotations + +import ipaddress +import logging +import os +import time +from dataclasses import dataclass, field + +from fastapi import Request + +from pinky_daemon.config import DEPLOYMENT_MODE_ENV, DeploymentMode # noqa: F401 + +__all__ = [ + "TRUSTED_PROXIES_ENV", + "ClientIdentity", + "get_client_identity", + "parse_trusted_proxies", + "peer_is_trusted", + "raw_peer_from_request", + "resolve_client_identity", + "trusted_proxies_from_env", +] + +logger = logging.getLogger(__name__) + +#: Env var listing trusted reverse-proxy CIDRs (comma-separated). +TRUSTED_PROXIES_ENV = "PINKY_TRUSTED_PROXIES" + +# IP address / network type aliases — kept narrow so callers don't need to +# import ipaddress just to type-annotate. +IPAddress = ipaddress.IPv4Address | ipaddress.IPv6Address +IPNetwork = ipaddress.IPv4Network | ipaddress.IPv6Network + + +@dataclass(frozen=True) +class ClientIdentity: + """Resolved client identity for a single HTTP request. + + Attributes: + ip: The canonical client IP — what audit/rate-limit/CIDR checks + should use. Always populated. + via_proxy: ``True`` when the request arrived through a configured + trusted proxy and the proxy-supplied client IP was honored. + raw_peer: The immediate TCP peer (the address that opened the + socket). Useful for forensic logging when the proxy chain looks + suspicious. + forwarded_chain: Full parsed forwarding chain (innermost first). + Empty when no ``XFF``/``Forwarded`` header was present or when + the chain was rejected. + """ + + ip: IPAddress + via_proxy: bool + raw_peer: IPAddress + forwarded_chain: tuple[IPAddress, ...] = field(default_factory=tuple) + + +# ── Trusted-proxy parsing ───────────────────────────────────────── + + +def parse_trusted_proxies(value: str | None) -> tuple[IPNetwork, ...]: + """Parse a comma-separated list of CIDRs into IP networks. + + Empty / unset / whitespace → ``()``. Bare addresses (no ``/``) are accepted + and treated as ``/32`` or ``/128``. Malformed entries are logged and + skipped rather than raising — the daemon prefers to start with a + diminished trust list than crash on bad operator input. + """ + if not value: + return () + parsed: list[IPNetwork] = [] + for raw in value.split(","): + token = raw.strip() + if not token: + continue + try: + parsed.append(ipaddress.ip_network(token, strict=False)) + except ValueError: + logger.warning( + "%s: ignoring invalid CIDR %r", TRUSTED_PROXIES_ENV, token + ) + return tuple(parsed) + + +def trusted_proxies_from_env(env: dict | None = None) -> tuple[IPNetwork, ...]: + """Read and parse :data:`TRUSTED_PROXIES_ENV` from the given mapping.""" + source = env if env is not None else os.environ + return parse_trusted_proxies(source.get(TRUSTED_PROXIES_ENV)) + + +# ── IP normalisation helpers ────────────────────────────────────── + + +def _normalise(addr: IPAddress) -> IPAddress: + """Collapse IPv6-mapped IPv4 (``::ffff:1.2.3.4``) to IPv4. + + Required so an operator's ``10.0.0.0/8`` rule actually matches a request + that comes in as an IPv6-mapped peer (common when a server listens on a + dual-stack socket). + """ + if isinstance(addr, ipaddress.IPv6Address) and addr.ipv4_mapped is not None: + return addr.ipv4_mapped + return addr + + +def _parse_ip(token: str) -> IPAddress | None: + """Parse an IP literal, normalising mapped IPv4. Returns ``None`` on junk. + + Strips brackets (``[2001:db8::1]``) and an optional ``:port`` suffix, + which is what RFC 7239 ``Forwarded: for=...`` and most XFF producers + emit for IPv6 / port-bearing values. + """ + raw = token.strip() + if not raw: + return None + # Strip optional surrounding double-quotes (RFC 7239 quoted-string) + if len(raw) >= 2 and raw[0] == '"' and raw[-1] == '"': + raw = raw[1:-1].strip() + if not raw: + return None + # Bracketed IPv6: [2001:db8::1] or [2001:db8::1]:443 + if raw.startswith("["): + end = raw.find("]") + if end == -1: + return None + raw = raw[1:end] + elif raw.count(":") == 1: + # Looks like host:port (IPv4 + port). Strip port. + host, _, _port = raw.partition(":") + raw = host + try: + return _normalise(ipaddress.ip_address(raw)) + except ValueError: + return None + + +def peer_is_trusted(peer: IPAddress, trusted: tuple[IPNetwork, ...]) -> bool: + """True when ``peer`` falls inside any configured trusted-proxy CIDR.""" + for net in trusted: + # IPv4 vs IPv6 mismatch raises TypeError on `in` — guard explicitly so + # mixed configs don't blow up. + if isinstance(peer, ipaddress.IPv4Address) and isinstance( + net, ipaddress.IPv4Network + ): + if peer in net: + return True + elif isinstance(peer, ipaddress.IPv6Address) and isinstance( + net, ipaddress.IPv6Network + ): + if peer in net: + return True + return False + + +# ── Forwarded / XFF parsing ────────────────────────────────────── + + +def _parse_forwarded_header(header: str) -> tuple[IPAddress, ...]: + """Parse RFC 7239 ``Forwarded`` header. Returns the chain (innermost first). + + Each comma-separated element is a list of ``key=value`` pairs joined by + semicolons. We extract every ``for=...`` token in order. + """ + chain: list[IPAddress] = [] + for element in header.split(","): + for pair in element.split(";"): + key, sep, value = pair.partition("=") + if not sep or key.strip().lower() != "for": + continue + ip = _parse_ip(value) + if ip is not None: + chain.append(ip) + return tuple(chain) + + +def _parse_xff_header(header: str) -> tuple[IPAddress, ...]: + """Parse ``X-Forwarded-For`` header into a chain (innermost first). + + Innermost = leftmost in the header (the original client, if the chain + can be trusted). + """ + chain: list[IPAddress] = [] + for token in header.split(","): + ip = _parse_ip(token) + if ip is not None: + chain.append(ip) + return tuple(chain) + + +# ── Once-per-minute warning throttle ────────────────────────────── + + +_warn_state: dict[str, float] = {} + + +def _warn_throttled(key: str, message: str) -> None: + """Log ``message`` at WARNING, but at most once per minute per ``key``. + + Keeps a misbehaving client from filling the log with the same parse + failure. The state dict is process-local; that's enough for the alert + surface this resolver targets. + """ + now = time.monotonic() + last = _warn_state.get(key, 0.0) + if now - last < 60.0: + return + _warn_state[key] = now + logger.warning(message) + + +# ── Resolver ────────────────────────────────────────────────────── + + +def raw_peer_from_request(request: Request) -> IPAddress: + """Pull the immediate TCP peer off a Starlette/FastAPI request. + + Falls back to the unspecified IPv4 address (``0.0.0.0``) when ``client`` + is missing — happens with Starlette's ``TestClient`` if no client tuple + was provided. Callers can still rely on having an :class:`IPAddress`. + """ + client = getattr(request, "client", None) + host = getattr(client, "host", None) if client is not None else None + if not host: + return ipaddress.IPv4Address("0.0.0.0") + try: + return _normalise(ipaddress.ip_address(host)) + except ValueError: + # Starlette occasionally hands back things like ``"unix:..."`` for + # ASGI lifespans; treat as unspecified. + return ipaddress.IPv4Address("0.0.0.0") + + +def _client_from_chain( + chain: tuple[IPAddress, ...], trusted: tuple[IPNetwork, ...] +) -> IPAddress | None: + """Walk a forwarding chain right-to-left, skipping trusted hops. + + Returns the first non-trusted address encountered (the original client), + or the leftmost address when every hop is trusted, or ``None`` if the + chain is empty. + """ + if not chain: + return None + # Walk from the right (closest hop = last entry per RFC 7239 / XFF + # convention). Skip any hop that is itself a trusted proxy. + for hop in reversed(chain): + if not peer_is_trusted(hop, trusted): + return hop + # Whole chain is trusted proxies — the leftmost entry is the client. + return chain[0] + + +def resolve_client_identity( + request: Request, + mode: DeploymentMode, + trusted_proxies: tuple[IPNetwork, ...] = (), +) -> ClientIdentity: + """Resolve the canonical :class:`ClientIdentity` for ``request``. + + See module docstring for the full spec. This is the single function every + IP-consuming feature in the hardening track should call. + """ + raw_peer = raw_peer_from_request(request) + + # Trusted-LAN posture: never honor forwarded headers, full stop. + if mode is DeploymentMode.TRUSTED: + return ClientIdentity(ip=raw_peer, via_proxy=False, raw_peer=raw_peer) + + # lan / web posture: parse forwarding headers, but only trust them when + # the immediate peer is one of our configured proxies. + forwarded_header = request.headers.get("forwarded") + xff_header = request.headers.get("x-forwarded-for") + + chain: tuple[IPAddress, ...] = () + parse_failed = False + if forwarded_header: + try: + chain = _parse_forwarded_header(forwarded_header) + except Exception: # pragma: no cover - defensive + parse_failed = True + if not chain and xff_header: + try: + chain = _parse_xff_header(xff_header) + except Exception: # pragma: no cover - defensive + parse_failed = True + + if parse_failed and (forwarded_header or xff_header): + _warn_throttled( + "client_identity.parse_failed", + "client-IP resolver: failed to parse forwarding header; " + "falling back to raw peer", + ) + chain = () + + # Did anyone send a forwarding header at all? + sent_forwarding = bool(forwarded_header or xff_header) + + if not peer_is_trusted(raw_peer, trusted_proxies): + # Untrusted peer claiming to be a proxy — drop their chain and log + # so audit can correlate suspected spoofing attempts. + if sent_forwarding: + _warn_throttled( + "client_identity.untrusted_proxy", + f"client-IP resolver: untrusted peer {raw_peer} sent " + "forwarding headers; dropping chain", + ) + return ClientIdentity( + ip=raw_peer, via_proxy=False, raw_peer=raw_peer, forwarded_chain=() + ) + + # Trusted peer with a chain → identify the original client. + client_ip = _client_from_chain(chain, trusted_proxies) + if client_ip is None: + # Trusted peer but no parseable chain — treat the peer itself as the + # client. This is the legitimate "proxy talking to us directly" case. + return ClientIdentity( + ip=raw_peer, via_proxy=False, raw_peer=raw_peer, forwarded_chain=() + ) + + return ClientIdentity( + ip=client_ip, + via_proxy=True, + raw_peer=raw_peer, + forwarded_chain=chain, + ) + + +# ── FastAPI dependency ──────────────────────────────────────────── + + +def get_client_identity(request: Request) -> ClientIdentity: + """FastAPI dependency: ``Depends(get_client_identity) -> ClientIdentity``. + + Reads: + * ``request.app.state.deployment_mode`` — set by ``create_api`` + * ``request.app.state.trusted_proxies`` — set by ``create_api`` from + :data:`TRUSTED_PROXIES_ENV`. Falls back to env on each call when + unset, so this dep stays usable from tests that don't bother + configuring app state. + """ + state = getattr(request, "app", None) + state = getattr(state, "state", None) if state is not None else None + + mode = getattr(state, "deployment_mode", None) if state is not None else None + if mode is None: + from pinky_daemon.config import resolve_deployment_mode + + mode = resolve_deployment_mode() + + trusted = ( + getattr(state, "trusted_proxies", None) if state is not None else None + ) + if trusted is None: + trusted = trusted_proxies_from_env() + + return resolve_client_identity(request, mode, trusted) diff --git a/src/pinky_daemon/net/lan_filter.py b/src/pinky_daemon/net/lan_filter.py new file mode 100644 index 00000000..1e38bcc4 --- /dev/null +++ b/src/pinky_daemon/net/lan_filter.py @@ -0,0 +1,189 @@ +"""LAN-only network filter (#436). + +Provides: + +* :data:`DEFAULT_PRIVATE_CIDRS` — the RFC1918 / link-local / loopback / ULA + ranges. CGNAT (``100.64.0.0/10``) is **not** included by default — operators + who run on Tailscale or another shared-CGNAT fabric have to opt in via the + ``PINKY_LAN_EXTRA_CIDRS`` env var so we don't accidentally trust the wider + carrier-grade NAT space. + +* :func:`is_private_ip` — pure helper that asks "is this address inside any of + these CIDRs?", with IPv4/IPv6 type guards so a misconfigured operator doesn't + blow up the request handler with a ``TypeError``. + +* :func:`build_lan_filter_middleware` — factory that returns an ASGI middleware + closure. Refuses any non-loopback / non-private source with HTTP 403 when + ``app.state.deployment_mode`` is :class:`DeploymentMode.LAN`. In ``trusted`` + and ``web`` modes the middleware is a no-op pass-through; the operator + declares intent through ``PINKY_DEPLOYMENT_MODE``, the middleware enforces + it. + +The middleware delegates IP resolution to +:func:`pinky_daemon.net.client_identity.resolve_client_identity` so XFF / +Forwarded handling, trust evaluation, and IPv6-mapped IPv4 normalisation +match the rest of the hardening track. + +Part of #433. Issue #436. +""" + +from __future__ import annotations + +import ipaddress +import logging +import os +from collections.abc import Awaitable, Callable + +from fastapi import Request +from fastapi.responses import JSONResponse + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.net.client_identity import ( + IPAddress, + IPNetwork, + parse_trusted_proxies, + resolve_client_identity, +) + +__all__ = [ + "DEFAULT_PRIVATE_CIDRS", + "EXTRA_CIDRS_ENV", + "build_lan_filter_middleware", + "is_private_ip", + "resolve_lan_allowed_cidrs", +] + +logger = logging.getLogger(__name__) + +#: Env var listing operator-supplied additional CIDRs that should be treated +#: as "private" for LAN-mode filtering. Useful for Tailscale (``100.64.0.0/10``) +#: or VPN tenant networks. Comma-separated, same syntax as +#: ``PINKY_TRUSTED_PROXIES``. +EXTRA_CIDRS_ENV = "PINKY_LAN_EXTRA_CIDRS" + + +#: Default LAN allowlist — RFC1918 + IPv4 link-local + IPv4 loopback + IPv6 +#: ULA + IPv6 link-local + IPv6 loopback. CGNAT is **not** included by +#: design; opt in via :data:`EXTRA_CIDRS_ENV`. +DEFAULT_PRIVATE_CIDRS: tuple[IPNetwork, ...] = ( + ipaddress.ip_network("10.0.0.0/8"), + ipaddress.ip_network("172.16.0.0/12"), + ipaddress.ip_network("192.168.0.0/16"), + ipaddress.ip_network("127.0.0.0/8"), + ipaddress.ip_network("169.254.0.0/16"), + ipaddress.ip_network("fd00::/8"), + ipaddress.ip_network("fe80::/10"), + ipaddress.ip_network("::1/128"), +) + + +def is_private_ip(addr: IPAddress, allowlist: tuple[IPNetwork, ...]) -> bool: + """``True`` iff ``addr`` is inside any CIDR in ``allowlist``. + + Type-mismatched comparisons (IPv4 address vs IPv6 network or vice versa) + are skipped silently rather than raising ``TypeError`` — happens any time + the operator's allowlist mixes families. + """ + for net in allowlist: + if isinstance(addr, ipaddress.IPv4Address) and isinstance( + net, ipaddress.IPv4Network + ): + if addr in net: + return True + elif isinstance(addr, ipaddress.IPv6Address) and isinstance( + net, ipaddress.IPv6Network + ): + if addr in net: + return True + return False + + +def resolve_lan_allowed_cidrs(env: dict | None = None) -> tuple[IPNetwork, ...]: + """Compose the effective LAN allowlist: defaults + operator extras. + + Reads :data:`EXTRA_CIDRS_ENV` from ``env`` (defaults to ``os.environ``), + parses with :func:`parse_trusted_proxies` (so the same lenient + "skip-bad-entries" semantics apply), and concatenates with + :data:`DEFAULT_PRIVATE_CIDRS`. Order matters only for log readability; + membership tests are O(n). + """ + source = env if env is not None else os.environ + extra = parse_trusted_proxies(source.get(EXTRA_CIDRS_ENV)) + return DEFAULT_PRIVATE_CIDRS + extra + + +# ── Middleware factory ──────────────────────────────────────────── + + +_REJECT_BODY = { + "error": "forbidden", + "detail": ( + "Source address is not in the configured LAN allowlist. " + "Set PINKY_LAN_EXTRA_CIDRS or change PINKY_DEPLOYMENT_MODE if " + "this is a trusted client." + ), +} + + +def build_lan_filter_middleware( + allowlist: tuple[IPNetwork, ...] | None = None, +): + """Return an ASGI middleware closure enforcing the LAN allowlist. + + The closure inspects each incoming request: + + * If ``request.app.state.deployment_mode`` is **not** ``LAN``, the + request passes through untouched. ``trusted`` and ``web`` modes have + their own enforcement layers (#441 and the reverse-proxy contract). + * Otherwise, the canonical client IP is computed via + :func:`resolve_client_identity` (so XFF parsing matches #442) and + compared against the supplied ``allowlist`` (or the + ``DEFAULT_PRIVATE_CIDRS`` + env extras when ``None``). + * Non-private sources receive ``HTTP 403`` with a JSON body; the + reject is logged at INFO so operators can see scan/probe traffic + without flooding ERROR. + + Args: + allowlist: Optional explicit override. When ``None``, the middleware + recomputes from env on every request — slow path, but useful for + tests that mutate env between calls. ``create_api`` should + always pre-resolve and pass an explicit tuple. + """ + + async def lan_filter( + request: Request, + call_next: Callable[[Request], Awaitable], + ): + state = request.app.state if request.app else None + mode = getattr(state, "deployment_mode", None) + if mode is not DeploymentMode.LAN: + return await call_next(request) + + # Use the cached parsed list when available; fall back to a fresh + # env read so tests don't have to wire app.state. + active_allowlist = allowlist + if active_allowlist is None: + cached = getattr(state, "lan_allowed_cidrs", None) + active_allowlist = ( + cached if cached is not None else resolve_lan_allowed_cidrs() + ) + + trusted_proxies = ( + getattr(state, "trusted_proxies", None) if state else None + ) or () + ci = resolve_client_identity(request, mode, trusted_proxies) + + if not is_private_ip(ci.ip, active_allowlist): + logger.info( + "lan_filter: rejecting %s %s from %s (raw_peer=%s, via_proxy=%s)", + request.method, + request.url.path, + ci.ip, + ci.raw_peer, + ci.via_proxy, + ) + return JSONResponse(_REJECT_BODY, status_code=403) + + return await call_next(request) + + return lan_filter diff --git a/src/pinky_daemon/security_audit.py b/src/pinky_daemon/security_audit.py new file mode 100644 index 00000000..ab957860 --- /dev/null +++ b/src/pinky_daemon/security_audit.py @@ -0,0 +1,434 @@ +"""Security audit log (#440). + +Append-only audit trail for security-relevant events: auth, admin actions, +network filter rejects, rate-limit trips, boot guard overrides. Distinct +from the per-tool-call audit store in :mod:`pinky_daemon.hooks` (which +records agent tool execution / cost) — different schema, different +retention story, different consumers. + +Design choices +-------------- +* **Separate SQLite DB** (``data/security_audit.db``). Decoupled from the + per-agent audit so a security-export job can run against a focused file + without scanning the bulky tool-call history. +* **Append-only by convention.** No ``UPDATE`` / ``DELETE`` API. Retention + pruning is the only deletion path and only operates on a row-age basis. +* **Best-effort writes on hot paths.** A locked / broken audit DB must NOT + take down login. Exceptions during ``log_event`` are caught and logged + to the standard logger; the call returns a sentinel ``None`` row id. + Boot / refuse-to-boot events bypass this and fail loud (caller raises). +* **Allowlist redaction per event_type.** A blacklist of + ``token|password|secret|authorization`` will miss the next credential + field name added in five months. Each event type declares which keys + from the detail blob are *kept*; everything else is dropped before + serialisation. +* **Actor identity is `(user_id, token_id)`** so "Alice in browser" is + distinguishable from "Alice's CI token" once #435 lands. +* **`forwarded_chain` reserved** for forensics (JSON array of XFF hops + from #442's :class:`ClientIdentity`). +* **Hash-chain column reserved** but not populated in this PR — adding the + column now avoids a future migration. + +Schema +------ +.. code-block:: sql + + CREATE TABLE security_audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ts REAL NOT NULL, -- unix seconds + event_type TEXT NOT NULL, -- e.g. auth.login.fail + actor_user_id INTEGER, + actor_token_id INTEGER, + actor_ip TEXT, + via_proxy INTEGER NOT NULL DEFAULT 0, -- 0/1 + forwarded_chain TEXT, -- JSON array + method TEXT, + route_name TEXT, + target TEXT, + outcome TEXT NOT NULL, -- success | failure | denied + user_agent TEXT, + correlation_id TEXT, + detail_json TEXT, -- redacted JSON object + prev_hash TEXT -- reserved for future hash chain + ); + +Part of #433. Issue #440. +""" + +from __future__ import annotations + +import json +import logging +import sqlite3 +import time +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path + +__all__ = [ + "DEFAULT_DETAIL_ALLOWLIST", + "AuditOutcome", + "EVENT_DETAIL_ALLOWLIST", + "SecurityAuditStore", + "redact_detail", +] + +logger = logging.getLogger(__name__) + + +class AuditOutcome: + """String constants for the ``outcome`` column. + + Not an Enum — kept as plain strings so callers can pass literals + without an extra import, and so the column stays trivially + JSON-serialisable. + """ + + SUCCESS = "success" + FAILURE = "failure" + DENIED = "denied" + + +@dataclass(frozen=True) +class SecurityAuditEntry: + """A row in :data:`SecurityAuditStore`.""" + + id: int + ts: float + event_type: str + actor_user_id: int | None + actor_token_id: int | None + actor_ip: str | None + via_proxy: bool + forwarded_chain: list[str] | None + method: str | None + route_name: str | None + target: str | None + outcome: str + user_agent: str | None + correlation_id: str | None + detail: dict | None + + +# ── Redaction ───────────────────────────────────────────────────── + + +#: Default allowlist applied when an event type doesn't declare its own. +#: Conservative — only fields with no plausible secret content. +DEFAULT_DETAIL_ALLOWLIST: frozenset[str] = frozenset( + { + "reason", + "duration_ms", + "status_code", + "error_class", + "count", + "limit", + "bucket", + } +) + + +#: Per-event detail allowlists. Define what's *kept*, not what's stripped — +#: future credential field names default to redacted instead of accidentally +#: leaking. +EVENT_DETAIL_ALLOWLIST: dict[str, frozenset[str]] = { + "auth.login.success": frozenset({"username", "method"}), + "auth.login.fail": frozenset({"username", "method", "reason"}), + "auth.logout": frozenset({"username"}), + "auth.token.create": frozenset({"username", "scope", "ttl_seconds"}), + "auth.token.revoke": frozenset({"username", "token_label"}), + "auth.elevation.success": frozenset({"username", "scope"}), + "auth.elevation.fail": frozenset({"username", "scope", "reason"}), + "auth.csrf.rejected": frozenset({"reason"}), + "admin.update": frozenset( + {"target_path", "fields_changed", "version_before", "version_after"} + ), + "admin.agent.create": frozenset({"agent_name", "model"}), + "admin.agent.delete": frozenset({"agent_name"}), + "admin.agent.retire": frozenset({"agent_name"}), + "admin.skill.install": frozenset({"skill_name", "source"}), + "admin.mcp.config_change": frozenset({"agent_name", "fields_changed"}), + "admin.settings.change": frozenset({"setting_name", "value_redacted"}), + "rate_limit.exceeded": frozenset({"bucket", "count", "limit", "route_name"}), + "network.lan_filter.reject": frozenset( + {"raw_peer", "via_proxy", "method", "path"} + ), + "boot.daemon.start": frozenset( + {"deployment_mode", "host", "port", "version"} + ), + "boot.refuse": frozenset({"failed_guards"}), + "boot.override.active": frozenset({"guard_name", "reason"}), +} + + +def _redact_value(value): + """Recursively pass through plain JSON-safe values; replace anything + with a credential-shaped key during dict descent.""" + if isinstance(value, Mapping): + return {k: _redact_value(v) for k, v in value.items()} + if isinstance(value, list | tuple): + return [_redact_value(v) for v in value] + return value + + +def redact_detail( + event_type: str, detail: Mapping | None +) -> dict | None: + """Return a redacted copy of ``detail`` per :data:`EVENT_DETAIL_ALLOWLIST`. + + Allowlist semantics: keys not in the per-event set are dropped. When + the event type isn't registered, the conservative + :data:`DEFAULT_DETAIL_ALLOWLIST` applies — that way a brand-new event + type doesn't accidentally leak its fields before the schema is added. + + Nested dicts are walked but the same allowlist applies at every level. + Lists are filtered element-wise (each is treated as a leaf or a nested + dict). + """ + if detail is None: + return None + allowlist = EVENT_DETAIL_ALLOWLIST.get(event_type, DEFAULT_DETAIL_ALLOWLIST) + return { + k: _redact_value(v) for k, v in detail.items() if k in allowlist + } + + +# ── Store ───────────────────────────────────────────────────────── + + +class SecurityAuditStore: + """SQLite-backed append-only security audit log. + + Thread-safe (uses ``check_same_thread=False`` and SQLite's WAL + + one-write-at-a-time semantics). ``log_event`` is best-effort: a write + failure logs at WARNING and returns ``None``, so callers on hot paths + (login, admin endpoints) never raise into their handler. + """ + + def __init__(self, db_path: str = "data/security_audit.db") -> None: + Path(db_path).parent.mkdir(parents=True, exist_ok=True) + self._db = sqlite3.connect(db_path, check_same_thread=False) + self._db.execute("PRAGMA journal_mode=WAL") + self._db.executescript( + """ + CREATE TABLE IF NOT EXISTS security_audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ts REAL NOT NULL, + event_type TEXT NOT NULL, + actor_user_id INTEGER, + actor_token_id INTEGER, + actor_ip TEXT, + via_proxy INTEGER NOT NULL DEFAULT 0, + forwarded_chain TEXT, + method TEXT, + route_name TEXT, + target TEXT, + outcome TEXT NOT NULL, + user_agent TEXT, + correlation_id TEXT, + detail_json TEXT, + prev_hash TEXT + ); + + CREATE INDEX IF NOT EXISTS idx_sec_audit_ts + ON security_audit_log(ts DESC); + CREATE INDEX IF NOT EXISTS idx_sec_audit_event + ON security_audit_log(event_type, ts DESC); + CREATE INDEX IF NOT EXISTS idx_sec_audit_actor + ON security_audit_log(actor_user_id, ts DESC); + CREATE INDEX IF NOT EXISTS idx_sec_audit_correlation + ON security_audit_log(correlation_id); + """ + ) + self._db.commit() + + # ── Write ──────────────────────────────────────────────── + + def log_event( + self, + event_type: str, + *, + outcome: str = AuditOutcome.SUCCESS, + actor_user_id: int | None = None, + actor_token_id: int | None = None, + actor_ip: str | None = None, + via_proxy: bool = False, + forwarded_chain: list[str] | None = None, + method: str | None = None, + route_name: str | None = None, + target: str | None = None, + user_agent: str | None = None, + correlation_id: str | None = None, + detail: Mapping | None = None, + ts: float | None = None, + ) -> int | None: + """Append a single audit event. Returns the new row id, or ``None`` + on best-effort failure. + + ``detail`` is run through :func:`redact_detail` before storage. + """ + try: + redacted = redact_detail(event_type, detail) + detail_json = json.dumps(redacted) if redacted is not None else None + chain_json = ( + json.dumps(forwarded_chain) if forwarded_chain else None + ) + cursor = self._db.execute( + """ + INSERT INTO security_audit_log + (ts, event_type, actor_user_id, actor_token_id, actor_ip, + via_proxy, forwarded_chain, method, route_name, target, + outcome, user_agent, correlation_id, detail_json) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + ts if ts is not None else time.time(), + event_type, + actor_user_id, + actor_token_id, + actor_ip, + 1 if via_proxy else 0, + chain_json, + method, + route_name, + target, + outcome, + user_agent, + correlation_id, + detail_json, + ), + ) + self._db.commit() + return cursor.lastrowid + except Exception: # noqa: BLE001 + # Best-effort: never fail the caller's request because the audit + # DB is unhappy. Log loudly so this surfaces. + logger.exception( + "security_audit: failed to log event_type=%s", event_type + ) + return None + + # ── Query ──────────────────────────────────────────────── + + def query( + self, + *, + event_type: str | None = None, + actor_user_id: int | None = None, + outcome: str | None = None, + since: float | None = None, + until: float | None = None, + correlation_id: str | None = None, + limit: int = 100, + offset: int = 0, + ) -> list[SecurityAuditEntry]: + """Filtered query, newest-first.""" + conditions: list[str] = [] + params: list = [] + if event_type is not None: + conditions.append("event_type = ?") + params.append(event_type) + if actor_user_id is not None: + conditions.append("actor_user_id = ?") + params.append(actor_user_id) + if outcome is not None: + conditions.append("outcome = ?") + params.append(outcome) + if since is not None: + conditions.append("ts >= ?") + params.append(since) + if until is not None: + conditions.append("ts <= ?") + params.append(until) + if correlation_id is not None: + conditions.append("correlation_id = ?") + params.append(correlation_id) + + where = f"WHERE {' AND '.join(conditions)}" if conditions else "" + params.extend([limit, offset]) + + rows = self._db.execute( + f""" + SELECT id, ts, event_type, actor_user_id, actor_token_id, + actor_ip, via_proxy, forwarded_chain, method, route_name, + target, outcome, user_agent, correlation_id, detail_json + FROM security_audit_log + {where} + ORDER BY ts DESC, id DESC + LIMIT ? OFFSET ? + """, + params, + ).fetchall() + + return [self._row_to_entry(r) for r in rows] + + def count( + self, + *, + event_type: str | None = None, + actor_user_id: int | None = None, + outcome: str | None = None, + since: float | None = None, + ) -> int: + conditions: list[str] = [] + params: list = [] + if event_type is not None: + conditions.append("event_type = ?") + params.append(event_type) + if actor_user_id is not None: + conditions.append("actor_user_id = ?") + params.append(actor_user_id) + if outcome is not None: + conditions.append("outcome = ?") + params.append(outcome) + if since is not None: + conditions.append("ts >= ?") + params.append(since) + + where = f"WHERE {' AND '.join(conditions)}" if conditions else "" + row = self._db.execute( + f"SELECT COUNT(*) FROM security_audit_log {where}", params + ).fetchone() + return int(row[0]) if row else 0 + + # ── Retention ──────────────────────────────────────────── + + def prune_older_than( + self, retention_days: int, *, now: float | None = None + ) -> int: + """Delete rows older than ``retention_days``. Returns rows deleted. + + Soft-retention: invoked by the existing scheduler; default + 365 days per #440 spec. The only deletion path on this table. + """ + if retention_days <= 0: + return 0 + cutoff = (now if now is not None else time.time()) - ( + retention_days * 86400 + ) + cursor = self._db.execute( + "DELETE FROM security_audit_log WHERE ts < ?", (cutoff,) + ) + self._db.commit() + return cursor.rowcount + + # ── Internal helpers ───────────────────────────────────── + + def _row_to_entry(self, row) -> SecurityAuditEntry: + return SecurityAuditEntry( + id=row[0], + ts=row[1], + event_type=row[2], + actor_user_id=row[3], + actor_token_id=row[4], + actor_ip=row[5], + via_proxy=bool(row[6]), + forwarded_chain=json.loads(row[7]) if row[7] else None, + method=row[8], + route_name=row[9], + target=row[10], + outcome=row[11], + user_agent=row[12], + correlation_id=row[13], + detail=json.loads(row[14]) if row[14] else None, + ) diff --git a/tests/test_boot_guards.py b/tests/test_boot_guards.py new file mode 100644 index 00000000..098beb8d --- /dev/null +++ b/tests/test_boot_guards.py @@ -0,0 +1,302 @@ +"""Tests for refuse-to-boot guards (#441). + +Covers per-guard logic, the override env-var mechanism, the warning guard +runner, the overall ``assert_web_mode_safe`` aggregator, and the message +formatter. + +Guards that depend on unshipped pieces of the #433 track (admin-user +existence, legacy UI password, multi-worker rate-limit backend) land +alongside their owning PRs (#435, #438) and are not exercised here. +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +import pytest + +from pinky_daemon.boot_guards import ( + BootGuardError, + assert_web_mode_safe, + check_db_permissions, + check_public_bind_requires_tls_marker, + check_session_secret, + check_trusted_proxies_when_behind_tls, + format_guard_error, + run_warning_guards, +) +from pinky_daemon.config import DeploymentMode + +# ── check_session_secret ─────────────────────────────────────── + + +_GOOD_SECRET = "x7ZfqL9aE2bN4mPvR8sUw1tYjK5dGcHo3iVnQbAxZsUyTpWmRkLeJfH4dN6cVbXz" + + +class TestCheckSessionSecret: + def test_unset_fails(self): + r = check_session_secret(env={}) + assert not r.ok + assert "unset" in r.message.lower() + + def test_too_short_fails(self): + r = check_session_secret(env={"PINKY_SESSION_SECRET": "abc123"}) + assert not r.ok + assert "too short" in r.message.lower() + + @pytest.mark.parametrize( + "value", ["changeme", "secret", "pinky", "password", "default"] + ) + def test_placeholder_fails(self, value): + # Pad to >32 chars so we exercise the placeholder check, not length. + padded = value + "x" * 30 + # But the lowered() check reads the *raw* secret, not the padded + # one — so test a long literal-placeholder by setting only the + # placeholder. + r = check_session_secret(env={"PINKY_SESSION_SECRET": value}) + assert not r.ok + # padded variant should pass length but might still fail on + # entropy if the padding character is repeated. Check intent: + _ = padded + + def test_long_random_passes(self): + r = check_session_secret(env={"PINKY_SESSION_SECRET": _GOOD_SECRET}) + assert r.ok + + def test_low_entropy_fails(self): + # 32 chars but only 1 distinct char. + r = check_session_secret(env={"PINKY_SESSION_SECRET": "x" * 64}) + assert not r.ok + assert "entropy" in r.message.lower() + + def test_override_marks_passing(self): + env = { + "PINKY_SESSION_SECRET": "", + "PINKY_ALLOW_INSECURE_SESSION_SECRET": "true", + } + r = check_session_secret(env=env) + assert r.ok + assert r.override_used + + +# ── check_public_bind_requires_tls_marker ────────────────────── + + +class TestPublicBindRequiresTlsMarker: + @pytest.mark.parametrize("host", ["127.0.0.1", "::1", "localhost"]) + def test_loopback_passes(self, host): + r = check_public_bind_requires_tls_marker(env={}, host=host) + assert r.ok + + def test_public_bind_without_marker_fails(self): + r = check_public_bind_requires_tls_marker(env={}, host="0.0.0.0") + assert not r.ok + assert "PINKY_BEHIND_TLS_PROXY" in r.message + + def test_public_bind_with_marker_passes(self): + r = check_public_bind_requires_tls_marker( + env={"PINKY_BEHIND_TLS_PROXY": "true"}, host="0.0.0.0" + ) + assert r.ok + + def test_specific_public_address_fails(self): + r = check_public_bind_requires_tls_marker( + env={}, host="192.168.1.10" + ) + assert not r.ok + + def test_override_marks_passing(self): + r = check_public_bind_requires_tls_marker( + env={"PINKY_ALLOW_INSECURE_PUBLIC_BIND_NO_TLS": "true"}, + host="0.0.0.0", + ) + assert r.ok + assert r.override_used + + +# ── check_trusted_proxies_when_behind_tls ────────────────────── + + +class TestTrustedProxiesWhenBehindTls: + def test_no_tls_marker_passes(self): + r = check_trusted_proxies_when_behind_tls(env={}) + assert r.ok + + def test_behind_tls_with_proxies_passes(self): + r = check_trusted_proxies_when_behind_tls( + env={ + "PINKY_BEHIND_TLS_PROXY": "true", + "PINKY_TRUSTED_PROXIES": "127.0.0.1/32", + } + ) + assert r.ok + + def test_behind_tls_without_proxies_fails(self): + r = check_trusted_proxies_when_behind_tls( + env={"PINKY_BEHIND_TLS_PROXY": "true"} + ) + assert not r.ok + assert "PINKY_TRUSTED_PROXIES" in r.message + assert "flatten" in r.message.lower() or "audit" in r.message.lower() + + +# ── check_db_permissions ─────────────────────────────────────── + + +@pytest.fixture +def tmp_dir(): + with tempfile.TemporaryDirectory() as tmpdir: + yield Path(tmpdir) + + +class TestCheckDbPermissions: + def test_nonexistent_path_no_parent_passes(self, tmp_dir): + # Nonexistent inside an existing parent should still check parent. + # Use a fully nonexistent path so we hit the early-return. + nowhere = tmp_dir / "nope" / "nope.db" + r = check_db_permissions(env={}, db_path=str(nowhere)) + # tmp_dir/nope doesn't exist either; guard skips. + assert r.ok + + def test_secure_dir_passes(self, tmp_dir): + os.chmod(tmp_dir, 0o700) + r = check_db_permissions(env={}, db_path=str(tmp_dir / "x.db")) + assert r.ok + + def test_world_readable_dir_fails(self, tmp_dir): + # Skip the test on Windows (chmod is a no-op there). Skip is + # actually inside the helper — sanity-check we return ok on win. + import sys + + if sys.platform == "win32": + pytest.skip("POSIX permissions only") + os.chmod(tmp_dir, 0o755) + r = check_db_permissions(env={}, db_path=str(tmp_dir / "x.db")) + assert not r.ok + assert "loose permissions" in r.message + + def test_secure_file_passes(self, tmp_dir): + os.chmod(tmp_dir, 0o700) + f = tmp_dir / "x.db" + f.write_text("") + os.chmod(f, 0o600) + r = check_db_permissions(env={}, db_path=str(f)) + assert r.ok + + def test_world_readable_file_fails(self, tmp_dir): + import sys + + if sys.platform == "win32": + pytest.skip("POSIX permissions only") + f = tmp_dir / "x.db" + f.write_text("") + os.chmod(f, 0o644) + r = check_db_permissions(env={}, db_path=str(f)) + assert not r.ok + + +# ── Warning guards ───────────────────────────────────────────── + + +class TestRunWarningGuards: + def test_returns_list(self): + out = run_warning_guards(env={}) + assert isinstance(out, list) + + def test_running_as_root_warning_when_uid_zero(self, monkeypatch): + monkeypatch.setattr("os.geteuid", lambda: 0, raising=False) + out = run_warning_guards(env={}) + assert any("running as root" in m.lower() for m in out) + + def test_running_as_root_silenced_by_override(self, monkeypatch): + monkeypatch.setattr("os.geteuid", lambda: 0, raising=False) + out = run_warning_guards( + env={"PINKY_ALLOW_INSECURE_RUNNING_AS_ROOT": "true"} + ) + assert all("running as root" not in m.lower() for m in out) + + +# ── assert_web_mode_safe ─────────────────────────────────────── + + +class TestAssertWebModeSafe: + def test_trusted_mode_skips_all_guards(self, tmp_dir): + # Even with garbage env, trusted mode is exempt. + assert_web_mode_safe( + DeploymentMode.TRUSTED, + host="0.0.0.0", + db_path=str(tmp_dir), + env={}, + ) + + def test_lan_mode_skips_all_guards(self, tmp_dir): + assert_web_mode_safe( + DeploymentMode.LAN, + host="0.0.0.0", + db_path=str(tmp_dir), + env={}, + ) + + def test_web_mode_all_guards_pass(self, tmp_dir): + os.chmod(tmp_dir, 0o700) + assert_web_mode_safe( + DeploymentMode.WEB, + host="127.0.0.1", + db_path=str(tmp_dir / "conv.db"), + env={ + "PINKY_SESSION_SECRET": _GOOD_SECRET, + }, + ) + + def test_web_mode_aggregates_failures(self, tmp_dir): + # Empty env → secret unset; bound to 0.0.0.0 without TLS marker; + # should produce two failures. + with pytest.raises(BootGuardError) as exc: + assert_web_mode_safe( + DeploymentMode.WEB, + host="0.0.0.0", + db_path=str(tmp_dir / "conv.db"), + env={}, + ) + names = {r.name for r in exc.value.results} + assert "SESSION_SECRET" in names + assert "PUBLIC_BIND_NO_TLS" in names + + def test_web_mode_overrides_let_boot_proceed(self, tmp_dir): + os.chmod(tmp_dir, 0o700) + env = { + "PINKY_ALLOW_INSECURE_SESSION_SECRET": "true", + "PINKY_ALLOW_INSECURE_PUBLIC_BIND_NO_TLS": "true", + } + # No exception even though the underlying conditions fail. + assert_web_mode_safe( + DeploymentMode.WEB, + host="0.0.0.0", + db_path=str(tmp_dir / "conv.db"), + env=env, + ) + + +# ── format_guard_error ───────────────────────────────────────── + + +class TestFormatGuardError: + def test_block_format(self): + from pinky_daemon.boot_guards import GuardResult + + results = [ + GuardResult(name="SESSION_SECRET", ok=False, message="unset"), + GuardResult( + name="PUBLIC_BIND_NO_TLS", + ok=False, + message="bound to 0.0.0.0 without marker", + ), + ] + out = format_guard_error(results) + assert "ERROR: refusing to start" in out + assert "[SESSION_SECRET] unset" in out + assert "[PUBLIC_BIND_NO_TLS]" in out + assert "PINKY_DEPLOYMENT_MODE=trusted" in out + assert "PINKY_ALLOW_INSECURE_" in out diff --git a/tests/test_client_identity.py b/tests/test_client_identity.py new file mode 100644 index 00000000..eb5eb51a --- /dev/null +++ b/tests/test_client_identity.py @@ -0,0 +1,444 @@ +"""Tests for the canonical trusted-client-IP resolver (#442). + +Covers: + +* ``parse_trusted_proxies`` — comma list → IPNetwork tuple, malformed entries + skipped, bare IPs accepted as /32 or /128. +* ``resolve_client_identity`` per deployment mode (trusted/lan/web), including + trusted vs. untrusted upstream peers, IPv6-mapped IPv4 normalisation, + comma-separated XFF chains, RFC 7239 ``Forwarded`` precedence over XFF, and + malformed-header fallback. +* The ``get_client_identity`` FastAPI dependency reading from app state and + falling back to env when state is empty. +""" + +from __future__ import annotations + +import ipaddress +import os +import tempfile +from typing import Any +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.net.client_identity import ( + TRUSTED_PROXIES_ENV, + ClientIdentity, + get_client_identity, + parse_trusted_proxies, + resolve_client_identity, + trusted_proxies_from_env, +) + +# ── Fake Request helper ──────────────────────────────────────── + + +class _FakeRequest: + """Minimal stand-in for starlette.Request — enough for the resolver. + + We don't go through the ASGI test client for unit tests so we can + precisely control the peer address (TestClient always reports + ``testclient`` which would defeat the point of these checks). + """ + + def __init__( + self, + peer: str | None = "127.0.0.1", + headers: dict[str, str] | None = None, + app: Any = None, + ): + self.client = ( + None if peer is None else type("Client", (), {"host": peer})() + ) + self.headers = headers or {} + self.app = app + + +# ── parse_trusted_proxies ────────────────────────────────────── + + +class TestParseTrustedProxies: + def test_empty_string(self): + assert parse_trusted_proxies("") == () + + def test_none(self): + assert parse_trusted_proxies(None) == () + + def test_whitespace_only(self): + assert parse_trusted_proxies(" ") == () + + def test_single_cidr(self): + result = parse_trusted_proxies("10.0.0.0/8") + assert result == (ipaddress.ip_network("10.0.0.0/8"),) + + def test_bare_ipv4_treated_as_slash_32(self): + result = parse_trusted_proxies("127.0.0.1") + assert result == (ipaddress.ip_network("127.0.0.1/32"),) + + def test_bare_ipv6_treated_as_slash_128(self): + result = parse_trusted_proxies("::1") + assert result == (ipaddress.ip_network("::1/128"),) + + def test_multiple_with_whitespace(self): + result = parse_trusted_proxies(" 127.0.0.1/32 , 10.0.0.0/8 ") + assert result == ( + ipaddress.ip_network("127.0.0.1/32"), + ipaddress.ip_network("10.0.0.0/8"), + ) + + def test_invalid_entry_skipped(self): + result = parse_trusted_proxies("not-an-ip,10.0.0.0/8,99.99.99.999") + # Only the valid entry survives. + assert result == (ipaddress.ip_network("10.0.0.0/8"),) + + def test_empty_token_skipped(self): + result = parse_trusted_proxies("10.0.0.0/8,,127.0.0.1") + assert ipaddress.ip_network("10.0.0.0/8") in result + assert ipaddress.ip_network("127.0.0.1/32") in result + assert len(result) == 2 + + def test_host_bits_set_accepted(self): + # strict=False so 10.0.0.5/8 doesn't reject — operators copy-paste + # specific server addresses with subnet masks all the time. + result = parse_trusted_proxies("10.0.0.5/8") + assert result == (ipaddress.ip_network("10.0.0.0/8"),) + + +class TestTrustedProxiesFromEnv: + def test_unset(self): + assert trusted_proxies_from_env(env={}) == () + + def test_reads_env(self): + result = trusted_proxies_from_env(env={TRUSTED_PROXIES_ENV: "10.0.0.0/8"}) + assert result == (ipaddress.ip_network("10.0.0.0/8"),) + + +# ── resolve_client_identity: trusted mode ────────────────────── + + +class TestResolveTrustedMode: + def test_uses_raw_peer(self): + req = _FakeRequest(peer="192.168.1.42") + ci = resolve_client_identity(req, DeploymentMode.TRUSTED) + assert ci.ip == ipaddress.ip_address("192.168.1.42") + assert ci.via_proxy is False + assert ci.raw_peer == ipaddress.ip_address("192.168.1.42") + assert ci.forwarded_chain == () + + def test_ignores_xff(self): + req = _FakeRequest( + peer="192.168.1.42", headers={"x-forwarded-for": "1.2.3.4"} + ) + ci = resolve_client_identity(req, DeploymentMode.TRUSTED) + # XFF is dropped on the floor — peer is the truth. + assert ci.ip == ipaddress.ip_address("192.168.1.42") + assert ci.via_proxy is False + assert ci.forwarded_chain == () + + def test_ignores_forwarded_header(self): + req = _FakeRequest( + peer="192.168.1.42", headers={"forwarded": "for=1.2.3.4"} + ) + ci = resolve_client_identity(req, DeploymentMode.TRUSTED) + assert ci.ip == ipaddress.ip_address("192.168.1.42") + assert ci.via_proxy is False + + +# ── resolve_client_identity: lan / web mode ──────────────────── + + +@pytest.mark.parametrize("mode", [DeploymentMode.LAN, DeploymentMode.WEB]) +class TestResolveProxyAware: + def test_no_xff_returns_peer(self, mode): + req = _FakeRequest(peer="10.0.0.5") + ci = resolve_client_identity(req, mode, trusted_proxies=()) + assert ci.ip == ipaddress.ip_address("10.0.0.5") + assert ci.via_proxy is False + + def test_untrusted_peer_xff_ignored(self, mode): + # Public peer claiming to be a proxy with a private "client" + # address. With no trusted_proxies configured the chain is dropped. + req = _FakeRequest( + peer="203.0.113.7", + headers={"x-forwarded-for": "10.0.0.99"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=()) + assert ci.ip == ipaddress.ip_address("203.0.113.7") + assert ci.via_proxy is False + assert ci.forwarded_chain == () + + def test_trusted_peer_xff_honored(self, mode): + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "203.0.113.7"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("203.0.113.7") + assert ci.via_proxy is True + assert ci.raw_peer == ipaddress.ip_address("127.0.0.1") + assert ci.forwarded_chain == (ipaddress.ip_address("203.0.113.7"),) + + def test_trusted_peer_xff_chain_skips_trusted_hops(self, mode): + # Two trusted proxies in series: 127.0.0.1 in front, 10.0.0.5 behind. + # Real client is 198.51.100.42. + proxies = parse_trusted_proxies("127.0.0.1,10.0.0.0/8") + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "198.51.100.42, 10.0.0.5"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("198.51.100.42") + assert ci.via_proxy is True + assert ci.forwarded_chain == ( + ipaddress.ip_address("198.51.100.42"), + ipaddress.ip_address("10.0.0.5"), + ) + + def test_all_trusted_chain_returns_leftmost(self, mode): + # If the whole chain is trusted proxies (synthetic or misconfigured), + # the leftmost is the closest-to-client we can name. + proxies = parse_trusted_proxies("10.0.0.0/8") + req = _FakeRequest( + peer="10.0.0.1", + headers={"x-forwarded-for": "10.0.0.99, 10.0.0.5"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("10.0.0.99") + assert ci.via_proxy is True + + def test_ipv6_mapped_ipv4_normalised(self, mode): + # Dual-stack listener gives us ::ffff:10.0.0.5 as the peer; trust + # rule is IPv4. Should still be honored. + proxies = parse_trusted_proxies("10.0.0.0/8") + req = _FakeRequest( + peer="::ffff:10.0.0.5", + headers={"x-forwarded-for": "203.0.113.7"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("203.0.113.7") + assert ci.via_proxy is True + assert ci.raw_peer == ipaddress.ip_address("10.0.0.5") + + def test_forwarded_header_takes_precedence_over_xff(self, mode): + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={ + "forwarded": 'for=198.51.100.10', + "x-forwarded-for": "203.0.113.7", + }, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + # Forwarded wins. + assert ci.ip == ipaddress.ip_address("198.51.100.10") + assert ci.via_proxy is True + + def test_forwarded_header_with_quoted_ipv6(self, mode): + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={"forwarded": 'for="[2001:db8::1]:443"'}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("2001:db8::1") + assert ci.via_proxy is True + + def test_xff_with_port_stripped(self, mode): + # Some proxies emit IPv4 with a port appended. + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "203.0.113.7:54321"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + assert ci.ip == ipaddress.ip_address("203.0.113.7") + + def test_malformed_xff_falls_back_to_peer(self, mode): + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "garbage,more-garbage"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + # Whole chain unparseable → no client found → fall back to peer. + assert ci.ip == ipaddress.ip_address("127.0.0.1") + assert ci.via_proxy is False + assert ci.forwarded_chain == () + + def test_partially_malformed_xff_keeps_valid_entries(self, mode): + proxies = parse_trusted_proxies("127.0.0.1") + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "garbage,203.0.113.7"}, + ) + ci = resolve_client_identity(req, mode, trusted_proxies=proxies) + # The good entry is honored; junk is dropped silently. + assert ci.ip == ipaddress.ip_address("203.0.113.7") + assert ci.via_proxy is True + + def test_missing_client_attribute(self, mode): + # Starlette can produce a Request with client=None (lifespan / test). + req = _FakeRequest(peer=None) + ci = resolve_client_identity(req, mode, trusted_proxies=()) + assert ci.raw_peer == ipaddress.ip_address("0.0.0.0") + assert ci.ip == ipaddress.ip_address("0.0.0.0") + + +# ── ClientIdentity dataclass ─────────────────────────────────── + + +class TestClientIdentityDataclass: + def test_immutable(self): + ci = ClientIdentity( + ip=ipaddress.ip_address("10.0.0.1"), + via_proxy=False, + raw_peer=ipaddress.ip_address("10.0.0.1"), + ) + with pytest.raises(Exception): # FrozenInstanceError or AttributeError + ci.ip = ipaddress.ip_address("10.0.0.2") # type: ignore[misc] + + def test_default_chain_is_empty_tuple(self): + ci = ClientIdentity( + ip=ipaddress.ip_address("10.0.0.1"), + via_proxy=False, + raw_peer=ipaddress.ip_address("10.0.0.1"), + ) + assert ci.forwarded_chain == () + + +# ── get_client_identity (FastAPI dep) ────────────────────────── + + +class TestGetClientIdentityDep: + def test_reads_from_app_state(self): + proxies = parse_trusted_proxies("127.0.0.1") + app = type("App", (), {})() + app.state = type("State", (), {})() + app.state.deployment_mode = DeploymentMode.WEB + app.state.trusted_proxies = proxies + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "203.0.113.7"}, + app=app, + ) + ci = get_client_identity(req) + assert ci.ip == ipaddress.ip_address("203.0.113.7") + assert ci.via_proxy is True + + def test_falls_back_to_env_when_state_unset(self): + env = { + k: v + for k, v in os.environ.items() + if k not in {"PINKY_DEPLOYMENT_MODE", TRUSTED_PROXIES_ENV} + } + env["PINKY_DEPLOYMENT_MODE"] = "web" + env[TRUSTED_PROXIES_ENV] = "127.0.0.1" + app = type("App", (), {})() + app.state = type("State", (), {})() + # No deployment_mode / trusted_proxies on state → fall back to env. + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-for": "198.51.100.10"}, + app=app, + ) + with patch.dict(os.environ, env, clear=True): + ci = get_client_identity(req) + assert ci.ip == ipaddress.ip_address("198.51.100.10") + assert ci.via_proxy is True + + +# ── End-to-end through create_api ────────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +class TestCreateApiTrustedProxies: + def test_app_state_carries_trusted_proxies(self, tmp_db_path): + from pinky_daemon.api import create_api + + env = {**os.environ, TRUSTED_PROXIES_ENV: "10.0.0.0/8,127.0.0.1"} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB + ) + assert ipaddress.ip_network("10.0.0.0/8") in app.state.trusted_proxies + assert ipaddress.ip_network("127.0.0.1/32") in app.state.trusted_proxies + + def test_app_state_defaults_to_empty_when_env_unset(self, tmp_db_path): + from pinky_daemon.api import create_api + + env = {k: v for k, v in os.environ.items() if k != TRUSTED_PROXIES_ENV} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB + ) + assert app.state.trusted_proxies == () + + def test_dependency_resolves_through_real_app(self, tmp_db_path): + # End-to-end: a route uses Depends(get_client_identity), the + # TestClient sends an XFF header, and the resolver picks it up + # because we configured ``0.0.0.0`` to be trusted. (Starlette's + # TestClient reports its peer as the unspecified address since it + # bypasses the socket layer.) + from fastapi import Depends + + from pinky_daemon.api import create_api + + env = { + **os.environ, + "PINKY_DEPLOYMENT_MODE": "web", + TRUSTED_PROXIES_ENV: "0.0.0.0/32", + } + with patch.dict(os.environ, env, clear=True): + app = create_api(db_path=tmp_db_path) + + @app.get("/_test/who") + def _who(ci=Depends(get_client_identity)): # noqa: B008 + return { + "ip": str(ci.ip), + "via_proxy": ci.via_proxy, + "raw_peer": str(ci.raw_peer), + } + + client = TestClient(app) + resp = client.get("/_test/who", headers={"x-forwarded-for": "203.0.113.7"}) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["ip"] == "203.0.113.7" + assert body["via_proxy"] is True + # raw_peer is whatever Starlette's TestClient reports, normalised + # through the parser. We just confirm the field is present and + # doesn't crash; it doesn't matter for the contract. + assert "raw_peer" in body + + +# ── Sanity: untrusted-peer warning is logged ─────────────────── + + +class TestWarningSurfacing: + def test_untrusted_proxy_warning_logged(self, caplog): + # Reset throttle state so the test warning isn't suppressed by an + # earlier test in the same process. + from pinky_daemon.net import client_identity as ci_mod + + ci_mod._warn_state.clear() + + req = _FakeRequest( + peer="203.0.113.7", + headers={"x-forwarded-for": "10.0.0.99"}, + ) + with caplog.at_level("WARNING", logger=ci_mod.logger.name): + resolve_client_identity(req, DeploymentMode.WEB, trusted_proxies=()) + assert any( + "untrusted peer" in rec.message + and "dropping chain" in rec.message + for rec in caplog.records + ) diff --git a/tests/test_csrf.py b/tests/test_csrf.py new file mode 100644 index 00000000..2de93d0c --- /dev/null +++ b/tests/test_csrf.py @@ -0,0 +1,383 @@ +"""Tests for the CSRF middleware (#443).""" + +from __future__ import annotations + +import os +import tempfile + +import pytest +from fastapi.testclient import TestClient + +from pinky_daemon.auth import ( + INTERNAL_AGENT_HEADER, + INTERNAL_SIGNATURE_HEADER, + INTERNAL_TIMESTAMP_HEADER, + SESSION_COOKIE_NAME, +) +from pinky_daemon.config import DeploymentMode +from pinky_daemon.middleware.csrf import ( + CSRF_COOKIE_NAME, + CSRF_HEADER_NAME, + UNSAFE_METHODS, + build_csrf_middleware, + generate_csrf_token, + is_exempt_path, +) + +# ── Pure helpers ────────────────────────────────────────────── + + +class TestPureHelpers: + def test_unsafe_methods_set(self): + assert UNSAFE_METHODS == frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + def test_generate_token_is_random(self): + a = generate_csrf_token() + b = generate_csrf_token() + assert a != b + assert len(a) >= 32 + + def test_is_exempt_path(self): + assert is_exempt_path("/triggers/webhook/abc") + assert is_exempt_path("/twilio/voice") + assert not is_exempt_path("/admin/update") + assert not is_exempt_path("/auth/login") + + +# ── Middleware closure ──────────────────────────────────────── + + +class _FakeApp: + def __init__(self, mode, sec_audit=None): + self.state = type("State", (), {})() + self.state.deployment_mode = mode + self.state.trusted_proxies = () + if sec_audit is not None: + self.state.security_audit = sec_audit + + +class _FakeRequest: + def __init__( + self, + method="POST", + path="/admin/update", + cookies=None, + headers=None, + peer="1.2.3.4", + app=None, + ): + self.method = method + self.url = type("U", (), {"path": path})() + self.cookies = cookies or {} + self.headers = headers or {} + self.client = type("C", (), {"host": peer})() + self.app = app + + +class _CapturingResponse: + """Captures set_cookie calls for assertion.""" + + def __init__(self, status_code=200): + self.status_code = status_code + self.headers: dict[str, str] = {} + self.cookies_set: list[dict] = [] + + def set_cookie(self, **kwargs): + self.cookies_set.append(kwargs) + + +async def _ok(_req): + return _CapturingResponse() + + +class TestMiddlewareEnforcement: + @pytest.mark.asyncio + async def test_trusted_mode_passthrough(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.TRUSTED) + # Even with a bad token, trusted is a no-op. + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc", CSRF_COOKIE_NAME: "x"}, + headers={CSRF_HEADER_NAME: "y"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_safe_methods_pass(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + for method in ("GET", "HEAD", "OPTIONS"): + req = _FakeRequest( + method=method, + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_unsafe_post_without_token_rejected(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 403 + + @pytest.mark.asyncio + async def test_unsafe_post_with_mismatch_rejected(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc", CSRF_COOKIE_NAME: "tok-a"}, + headers={CSRF_HEADER_NAME: "tok-b"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 403 + + @pytest.mark.asyncio + async def test_unsafe_post_with_match_passes(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc", CSRF_COOKIE_NAME: "tok-a"}, + headers={CSRF_HEADER_NAME: "tok-a"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_bearer_auth_exempts(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc"}, # has session cookie + headers={"authorization": "Bearer abc.def"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_internal_signed_exempts(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc"}, + headers={ + INTERNAL_AGENT_HEADER: "barsik", + INTERNAL_TIMESTAMP_HEADER: "1234", + INTERNAL_SIGNATURE_HEADER: "sig", + }, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_webhook_path_exempts(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/triggers/webhook/abc123", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_twilio_path_exempts(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", + path="/twilio/voice", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_no_session_no_enforcement(self): + # Anonymous request can do anything; CSRF protects ambient + # credentials, and there are none here. + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="POST", path="/auth/login", cookies={}, app=app + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + + +# ── Token issuance ──────────────────────────────────────────── + + +class TestTokenIssuance: + @pytest.mark.asyncio + async def test_session_without_csrf_cookie_gets_one_set(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="GET", + path="/api", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 200 + # Cookie issued. + assert any( + c.get("key") == CSRF_COOKIE_NAME for c in resp.cookies_set + ) + # SameSite=Strict + httponly=False so SPA can read it. + cookie = next( + c for c in resp.cookies_set if c.get("key") == CSRF_COOKIE_NAME + ) + assert cookie["samesite"] == "strict" + assert cookie["httponly"] is False + + @pytest.mark.asyncio + async def test_session_with_csrf_cookie_does_not_reissue(self): + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="GET", + path="/api", + cookies={SESSION_COOKIE_NAME: "abc", CSRF_COOKIE_NAME: "tok"}, + app=app, + ) + resp = await mw(req, _ok) + assert not any( + c.get("key") == CSRF_COOKIE_NAME for c in resp.cookies_set + ) + + @pytest.mark.asyncio + async def test_no_session_no_token_issued(self): + # Anonymous traffic doesn't get a CSRF cookie until they have a + # session to bind it to. + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest( + method="GET", path="/api", cookies={}, app=app + ) + resp = await mw(req, _ok) + assert not any( + c.get("key") == CSRF_COOKIE_NAME for c in resp.cookies_set + ) + + @pytest.mark.asyncio + async def test_secure_flag_only_in_web_mode(self): + mw = build_csrf_middleware() + for mode, expected_secure in ( + (DeploymentMode.LAN, False), + (DeploymentMode.WEB, True), + ): + app = _FakeApp(mode) + req = _FakeRequest( + method="GET", + path="/api", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + cookie = next( + c for c in resp.cookies_set if c.get("key") == CSRF_COOKIE_NAME + ) + assert cookie["secure"] is expected_secure + + +# ── Audit emission ──────────────────────────────────────────── + + +class TestAuditEmission: + @pytest.mark.asyncio + async def test_reject_emits_audit(self): + from pinky_daemon.security_audit import SecurityAuditStore + + with tempfile.TemporaryDirectory() as tmpdir: + sec = SecurityAuditStore(db_path=os.path.join(tmpdir, "x.db")) + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB, sec_audit=sec) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={SESSION_COOKIE_NAME: "abc"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 403 + rows = sec.query(event_type="auth.csrf.rejected") + assert len(rows) == 1 + assert rows[0].outcome == "denied" + assert rows[0].method == "POST" + assert rows[0].target == "/admin/update" + assert rows[0].detail == {"reason": "missing"} + + @pytest.mark.asyncio + async def test_mismatch_audit_reason(self): + from pinky_daemon.security_audit import SecurityAuditStore + + with tempfile.TemporaryDirectory() as tmpdir: + sec = SecurityAuditStore(db_path=os.path.join(tmpdir, "x.db")) + mw = build_csrf_middleware() + app = _FakeApp(DeploymentMode.WEB, sec_audit=sec) + req = _FakeRequest( + method="POST", + path="/admin/update", + cookies={ + SESSION_COOKIE_NAME: "abc", + CSRF_COOKIE_NAME: "tok-a", + }, + headers={CSRF_HEADER_NAME: "tok-b"}, + app=app, + ) + resp = await mw(req, _ok) + assert resp.status_code == 403 + rows = sec.query(event_type="auth.csrf.rejected") + assert rows[0].detail == {"reason": "mismatch"} + + +# ── End-to-end through create_api ───────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +class TestCreateApiCsrf: + def test_trusted_mode_no_csrf_check(self, tmp_db_path): + from pinky_daemon.api import create_api + + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.TRUSTED + ) + client = TestClient(app) + # /api is GET; trusted mode shouldn't even issue a token. + resp = client.get("/api") + assert resp.status_code == 200 + assert CSRF_COOKIE_NAME not in resp.cookies diff --git a/tests/test_deployment_mode.py b/tests/test_deployment_mode.py new file mode 100644 index 00000000..e3c30836 --- /dev/null +++ b/tests/test_deployment_mode.py @@ -0,0 +1,214 @@ +"""Tests for deployment-posture configuration (#434). + +Covers the resolver, the bind-host helper, the insecure-exposure warning, and +the wiring through ``create_api`` so ``app.state.deployment_mode`` and the +``/api`` payload reflect the configured posture. +""" + +from __future__ import annotations + +import os +import tempfile +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from pinky_daemon.config import ( + DEPLOYMENT_MODE_ENV, + DeploymentMode, + InvalidDeploymentModeError, + default_bind_host, + resolve_deployment_mode, + warn_if_insecure_exposure, +) + +# ── resolve_deployment_mode ──────────────────────────────────── + + +class TestResolveDeploymentMode: + def test_unset_returns_trusted(self): + assert resolve_deployment_mode(env={}) is DeploymentMode.TRUSTED + + def test_empty_string_returns_trusted(self): + assert ( + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: ""}) + is DeploymentMode.TRUSTED + ) + + def test_whitespace_only_returns_trusted(self): + assert ( + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: " "}) + is DeploymentMode.TRUSTED + ) + + @pytest.mark.parametrize( + "value,expected", + [ + ("trusted", DeploymentMode.TRUSTED), + ("lan", DeploymentMode.LAN), + ("web", DeploymentMode.WEB), + ], + ) + def test_each_canonical_value(self, value, expected): + assert resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: value}) is expected + + def test_case_insensitive(self): + assert ( + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: "WEB"}) + is DeploymentMode.WEB + ) + assert ( + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: "Lan"}) + is DeploymentMode.LAN + ) + + def test_strips_whitespace(self): + assert ( + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: " web "}) + is DeploymentMode.WEB + ) + + def test_invalid_value_raises(self): + with pytest.raises(InvalidDeploymentModeError) as excinfo: + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: "prod"}) + # Error message should mention the env var, the bad value, and the + # valid options so the operator knows how to fix it. + msg = str(excinfo.value) + assert "PINKY_DEPLOYMENT_MODE" in msg + assert "prod" in msg + assert "trusted" in msg + assert "lan" in msg + assert "web" in msg + + def test_invalid_alias_does_not_silently_pass(self): + # No back-compat aliases — explicit only. + for alias in ["production", "development", "dev", "local", "staging"]: + with pytest.raises(InvalidDeploymentModeError): + resolve_deployment_mode(env={DEPLOYMENT_MODE_ENV: alias}) + + def test_reads_os_environ_by_default(self): + with patch.dict(os.environ, {DEPLOYMENT_MODE_ENV: "lan"}, clear=False): + assert resolve_deployment_mode() is DeploymentMode.LAN + + +# ── default_bind_host ────────────────────────────────────────── + + +class TestDefaultBindHost: + def test_trusted_binds_all(self): + # Back-compat with current Mac Mini deployment. + assert default_bind_host(DeploymentMode.TRUSTED) == "0.0.0.0" + + def test_lan_binds_all(self): + # LAN appliances need to be reachable on the local network. + assert default_bind_host(DeploymentMode.LAN) == "0.0.0.0" + + def test_web_binds_loopback(self): + # Web mode assumes a reverse proxy in front; refuse public bind. + assert default_bind_host(DeploymentMode.WEB) == "127.0.0.1" + + +# ── warn_if_insecure_exposure ────────────────────────────────── + + +class TestWarnIfInsecureExposure: + def test_trusted_with_all_interfaces_warns(self): + warning = warn_if_insecure_exposure(DeploymentMode.TRUSTED, "0.0.0.0") + assert warning is not None + assert "trusted" in warning + assert "0.0.0.0" in warning + # Tells operator what to do about it. + assert "PINKY_DEPLOYMENT_MODE" in warning + assert "web" in warning + + def test_trusted_with_loopback_no_warning(self): + assert warn_if_insecure_exposure(DeploymentMode.TRUSTED, "127.0.0.1") is None + + def test_trusted_with_lan_address_no_warning(self): + # A specific bind address means the operator made a deliberate choice. + assert warn_if_insecure_exposure(DeploymentMode.TRUSTED, "10.0.0.32") is None + + def test_lan_no_warning(self): + assert warn_if_insecure_exposure(DeploymentMode.LAN, "0.0.0.0") is None + + def test_web_no_warning(self): + # Web mode has its own enforcement (#436/#441); this helper only + # catches the silent-trusted-exposure case. + assert warn_if_insecure_exposure(DeploymentMode.WEB, "0.0.0.0") is None + assert warn_if_insecure_exposure(DeploymentMode.WEB, "127.0.0.1") is None + + +# ── DeploymentMode enum ──────────────────────────────────────── + + +class TestDeploymentModeEnum: + def test_str_returns_value(self): + assert str(DeploymentMode.TRUSTED) == "trusted" + assert str(DeploymentMode.LAN) == "lan" + assert str(DeploymentMode.WEB) == "web" + + def test_is_str_subclass(self): + # Useful for direct comparison and JSON serialisation. + assert isinstance(DeploymentMode.WEB, str) + assert DeploymentMode.WEB == "web" + + +# ── create_api wiring ────────────────────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + """Throwaway DB path so create_api doesn't touch the project's data dir.""" + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +class TestCreateApiDeploymentMode: + def test_explicit_mode_cached_on_app_state(self, tmp_db_path): + from pinky_daemon.api import create_api + + app = create_api(db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB) + assert app.state.deployment_mode is DeploymentMode.WEB + + def test_default_resolves_from_env(self, tmp_db_path): + from pinky_daemon.api import create_api + + with patch.dict( + os.environ, {DEPLOYMENT_MODE_ENV: "lan"}, clear=False + ): + app = create_api(db_path=tmp_db_path) + assert app.state.deployment_mode is DeploymentMode.LAN + + def test_default_when_env_unset(self, tmp_db_path): + from pinky_daemon.api import create_api + + # Strip the env var so resolution lands on TRUSTED. + env = {k: v for k, v in os.environ.items() if k != DEPLOYMENT_MODE_ENV} + with patch.dict(os.environ, env, clear=True): + app = create_api(db_path=tmp_db_path) + assert app.state.deployment_mode is DeploymentMode.TRUSTED + + def test_api_endpoint_exposes_mode(self, tmp_db_path): + from pinky_daemon.api import create_api + + app = create_api(db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB) + client = TestClient(app) + resp = client.get("/api") + assert resp.status_code == 200 + body = resp.json() + assert body["deployment_mode"] == "web" + + def test_api_endpoint_exposes_mode_for_each_value(self, tmp_db_path): + from pinky_daemon.api import create_api + + # In LAN mode the LAN filter (#436) would reject TestClient's + # synthetic peer; opt 0.0.0.0/32 in via env so the test reaches + # /api in every mode. + env = {**os.environ, "PINKY_LAN_EXTRA_CIDRS": "0.0.0.0/32"} + with patch.dict(os.environ, env, clear=True): + for mode in DeploymentMode: + app = create_api(db_path=tmp_db_path, deployment_mode=mode) + client = TestClient(app) + resp = client.get("/api") + assert resp.json()["deployment_mode"] == mode.value diff --git a/tests/test_lan_filter.py b/tests/test_lan_filter.py new file mode 100644 index 00000000..2c5a94f9 --- /dev/null +++ b/tests/test_lan_filter.py @@ -0,0 +1,304 @@ +"""Tests for the LAN-only middleware (#436). + +Covers the private-CIDR helper, the env-driven allowlist composer, the +middleware closure (no-op in trusted/web, enforce in lan), and end-to-end +wiring through ``create_api`` (TestClient against an app spun up in each +mode). +""" + +from __future__ import annotations + +import ipaddress +import os +import tempfile +from typing import Any +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.net.lan_filter import ( + DEFAULT_PRIVATE_CIDRS, + EXTRA_CIDRS_ENV, + build_lan_filter_middleware, + is_private_ip, + resolve_lan_allowed_cidrs, +) + +# ── is_private_ip ────────────────────────────────────────────── + + +class TestIsPrivateIp: + @pytest.mark.parametrize( + "addr", + [ + "10.0.0.5", + "172.16.42.1", + "172.31.255.254", + "192.168.1.1", + "127.0.0.1", + "169.254.1.2", + "::1", + "fe80::1", + "fdab:cdef::1", + ], + ) + def test_default_cidrs_accept_private(self, addr): + assert is_private_ip( + ipaddress.ip_address(addr), DEFAULT_PRIVATE_CIDRS + ) + + @pytest.mark.parametrize( + "addr", + [ + "8.8.8.8", + "1.1.1.1", + "203.0.113.7", + "172.32.0.1", # just outside 172.16/12 + "169.255.0.1", # just outside link-local + "100.64.0.5", # CGNAT — explicitly NOT in defaults + "2001:db8::1", # public IPv6 + ], + ) + def test_default_cidrs_reject_public(self, addr): + assert not is_private_ip( + ipaddress.ip_address(addr), DEFAULT_PRIVATE_CIDRS + ) + + def test_cgnat_only_accepted_when_extra(self): + extras = (ipaddress.ip_network("100.64.0.0/10"),) + allowlist = DEFAULT_PRIVATE_CIDRS + extras + assert is_private_ip(ipaddress.ip_address("100.64.0.5"), allowlist) + + def test_ipv4_addr_against_ipv6_net_does_not_crash(self): + only_v6 = (ipaddress.ip_network("fd00::/8"),) + # No match, but importantly: no TypeError. + assert not is_private_ip(ipaddress.ip_address("10.0.0.5"), only_v6) + + def test_empty_allowlist_rejects_everything(self): + assert not is_private_ip(ipaddress.ip_address("10.0.0.5"), ()) + + +# ── resolve_lan_allowed_cidrs ────────────────────────────────── + + +class TestResolveLanAllowedCidrs: + def test_unset_returns_defaults(self): + result = resolve_lan_allowed_cidrs(env={}) + assert result == DEFAULT_PRIVATE_CIDRS + + def test_extra_appended(self): + result = resolve_lan_allowed_cidrs( + env={EXTRA_CIDRS_ENV: "100.64.0.0/10"} + ) + assert result[: len(DEFAULT_PRIVATE_CIDRS)] == DEFAULT_PRIVATE_CIDRS + assert ipaddress.ip_network("100.64.0.0/10") in result + + def test_invalid_extras_skipped(self): + result = resolve_lan_allowed_cidrs( + env={EXTRA_CIDRS_ENV: "garbage,100.64.0.0/10"} + ) + assert ipaddress.ip_network("100.64.0.0/10") in result + # Length = defaults + 1 valid extra; junk dropped. + assert len(result) == len(DEFAULT_PRIVATE_CIDRS) + 1 + + +# ── Middleware closure (unit) ────────────────────────────────── + + +class _FakeApp: + def __init__(self, mode, trusted=(), allowlist=None): + self.state = type("State", (), {})() + self.state.deployment_mode = mode + self.state.trusted_proxies = trusted + if allowlist is not None: + self.state.lan_allowed_cidrs = allowlist + + +class _FakeRequest: + def __init__( + self, + peer="10.0.0.5", + method="GET", + path="/", + headers=None, + app=None, + ): + self.client = type("C", (), {"host": peer})() + self.method = method + self.url = type("U", (), {"path": path})() + self.headers = headers or {} + self.app = app + + +async def _passthrough(_req): + return "ok" + + +class TestMiddlewareModes: + @pytest.mark.asyncio + async def test_trusted_mode_passthrough(self): + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + app = _FakeApp(DeploymentMode.TRUSTED) + # Public IP should still pass — trusted mode is no-op. + req = _FakeRequest(peer="8.8.8.8", app=app) + result = await mw(req, _passthrough) + assert result == "ok" + + @pytest.mark.asyncio + async def test_web_mode_passthrough(self): + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + app = _FakeApp(DeploymentMode.WEB) + req = _FakeRequest(peer="8.8.8.8", app=app) + result = await mw(req, _passthrough) + assert result == "ok" + + @pytest.mark.asyncio + async def test_lan_mode_accepts_private(self): + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + app = _FakeApp(DeploymentMode.LAN) + req = _FakeRequest(peer="192.168.1.10", app=app) + result = await mw(req, _passthrough) + assert result == "ok" + + @pytest.mark.asyncio + async def test_lan_mode_rejects_public(self): + from fastapi.responses import JSONResponse + + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + app = _FakeApp(DeploymentMode.LAN) + req = _FakeRequest(peer="8.8.8.8", app=app) + result = await mw(req, _passthrough) + assert isinstance(result, JSONResponse) + assert result.status_code == 403 + + @pytest.mark.asyncio + async def test_lan_mode_xff_only_honored_when_proxy_trusted(self): + # Untrusted peer claims a private "client" → must NOT pass. + from fastapi.responses import JSONResponse + + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + app = _FakeApp(DeploymentMode.LAN, trusted=()) + req = _FakeRequest( + peer="8.8.8.8", + headers={"x-forwarded-for": "10.0.0.5"}, + app=app, + ) + result = await mw(req, _passthrough) + assert isinstance(result, JSONResponse) + assert result.status_code == 403 + + @pytest.mark.asyncio + async def test_lan_mode_xff_honored_when_proxy_trusted(self): + # Trusted peer can vouch for a private client. + mw = build_lan_filter_middleware(allowlist=DEFAULT_PRIVATE_CIDRS) + trusted = (ipaddress.ip_network("172.16.0.0/12"),) + app = _FakeApp(DeploymentMode.LAN, trusted=trusted) + req = _FakeRequest( + peer="172.16.0.1", + headers={"x-forwarded-for": "10.0.0.5"}, + app=app, + ) + result = await mw(req, _passthrough) + assert result == "ok" + + @pytest.mark.asyncio + async def test_lan_mode_extra_cidrs_via_state(self): + # Operator opted into CGNAT via env → cached on app.state. + cgnat = (ipaddress.ip_network("100.64.0.0/10"),) + custom_allowlist = DEFAULT_PRIVATE_CIDRS + cgnat + mw = build_lan_filter_middleware(allowlist=custom_allowlist) + app = _FakeApp(DeploymentMode.LAN) + req = _FakeRequest(peer="100.64.0.5", app=app) + result = await mw(req, _passthrough) + assert result == "ok" + + @pytest.mark.asyncio + async def test_lan_mode_falls_back_to_env_when_no_allowlist_passed(self): + # No explicit allowlist on the closure or on app.state → middleware + # recomputes from env on each call. + mw = build_lan_filter_middleware(allowlist=None) + app = _FakeApp(DeploymentMode.LAN) + # Make sure no lan_allowed_cidrs is on state. + if hasattr(app.state, "lan_allowed_cidrs"): + del app.state.lan_allowed_cidrs + req = _FakeRequest(peer="10.0.0.5", app=app) + result = await mw(req, _passthrough) + assert result == "ok" + + +# ── End-to-end through create_api ────────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +class TestCreateApiLanFilter: + def test_trusted_mode_no_filter_applied(self, tmp_db_path): + # Public source → still 200 (the unrelated /api endpoint). + from pinky_daemon.api import create_api + + env = {k: v for k, v in os.environ.items() if k != EXTRA_CIDRS_ENV} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.TRUSTED + ) + client = TestClient(app) + # TestClient peer ends up as 0.0.0.0 (unspecified) — would be + # rejected by the LAN filter, but trusted mode is a no-op so it + # should still hit the route. + resp = client.get("/api") + assert resp.status_code == 200 + + def test_lan_mode_default_peer_rejected(self, tmp_db_path): + # TestClient's peer is 0.0.0.0 / "testclient" → not in private + # CIDRs → 403 in LAN mode. + from pinky_daemon.api import create_api + + env = {k: v for k, v in os.environ.items() if k != EXTRA_CIDRS_ENV} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.LAN + ) + client = TestClient(app) + resp = client.get("/api") + assert resp.status_code == 403 + body = resp.json() + assert body["error"] == "forbidden" + + def test_lan_mode_extra_cidrs_unblocks_test_client(self, tmp_db_path): + # If we add 0.0.0.0/32 to the allowlist via env, TestClient's + # synthetic peer passes. + from pinky_daemon.api import create_api + + env = {**os.environ, EXTRA_CIDRS_ENV: "0.0.0.0/32"} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.LAN + ) + client = TestClient(app) + resp = client.get("/api") + assert resp.status_code == 200 + + def test_web_mode_no_filter_applied(self, tmp_db_path): + # Web mode delegates to the reverse-proxy contract (#441 + bind + # default). The LAN middleware itself must not interfere. + from pinky_daemon.api import create_api + + env = {k: v for k, v in os.environ.items() if k != EXTRA_CIDRS_ENV} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB + ) + client = TestClient(app) + resp = client.get("/api") + # /api is unauthenticated; web mode should not block on source. + assert resp.status_code == 200 + + +# Placeholder to keep type-checkers happy on the unused import. +_ = Any diff --git a/tests/test_rate_limit.py b/tests/test_rate_limit.py new file mode 100644 index 00000000..0b1e256f --- /dev/null +++ b/tests/test_rate_limit.py @@ -0,0 +1,361 @@ +"""Tests for the auth/admin rate limiter (#438).""" + +from __future__ import annotations + +import os +import tempfile +from typing import Any +from unittest.mock import patch + +import pytest + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.middleware.rate_limit import ( + DEFAULT_BUCKETS, + BucketConfig, + InMemoryRateLimiter, + build_rate_limit_middleware, + classify_request, +) + +# ── classify_request ─────────────────────────────────────────── + + +class TestClassifyRequest: + def test_login_post_is_auth(self): + assert classify_request("POST", "/auth/login") == "auth" + + def test_login_get_is_default(self): + # The login *page* GET shouldn't be in the brute-force bucket. + assert classify_request("GET", "/auth/login") == "default" + + def test_password_post_is_auth(self): + assert classify_request("POST", "/auth/password") == "auth" + + def test_admin_prefix_is_admin(self): + assert classify_request("GET", "/admin/update") == "admin" + assert classify_request("POST", "/admin/agents") == "admin" + + def test_webhook_prefix_is_webhook(self): + assert classify_request("POST", "/triggers/webhook/abc123") == "webhook" + + def test_other_routes_are_default(self): + assert classify_request("GET", "/api") == "default" + assert classify_request("GET", "/agents") == "default" + + def test_method_case_insensitive(self): + assert classify_request("post", "/auth/login") == "auth" + + +# ── InMemoryRateLimiter ──────────────────────────────────────── + + +class TestInMemoryRateLimiter: + def test_under_limit_allowed(self): + limiter = InMemoryRateLimiter() + for i in range(5): + allowed, retry = limiter.check("auth", "1.2.3.4", now=i * 1.0) + assert allowed + assert retry == 0.0 + + def test_at_limit_rejected(self): + limiter = InMemoryRateLimiter() + for i in range(5): + limiter.check("auth", "1.2.3.4", now=i * 1.0) + # 6th attempt within window → blocked + lockout. + allowed, retry = limiter.check("auth", "1.2.3.4", now=5.0) + assert not allowed + assert retry > 0 + + def test_lockout_persists_for_penalty_seconds(self): + # Auth bucket has 900s lockout after exceed. + limiter = InMemoryRateLimiter() + for i in range(5): + limiter.check("auth", "1.2.3.4", now=i * 1.0) + limiter.check("auth", "1.2.3.4", now=5.0) # triggers lockout + # 100s later: still locked. + allowed, retry = limiter.check("auth", "1.2.3.4", now=105.0) + assert not allowed + assert retry > 0 + # 906s later: lockout cleared (penalty + small slack). + allowed, _ = limiter.check("auth", "1.2.3.4", now=911.0) + assert allowed + + def test_window_rolls_off_for_non_lockout_bucket(self): + limiter = InMemoryRateLimiter() + # Admin bucket is 30/min; no lockout. + for i in range(30): + limiter.check("admin", "1.2.3.4", now=i * 0.1) + # Hit limit at t=2.9 + allowed, _ = limiter.check("admin", "1.2.3.4", now=3.0) + assert not allowed + # 65s later, all old timestamps have rolled off. + allowed, _ = limiter.check("admin", "1.2.3.4", now=70.0) + assert allowed + + def test_keys_are_independent(self): + limiter = InMemoryRateLimiter() + for i in range(5): + limiter.check("auth", "1.2.3.4", now=i * 1.0) + # Different IP → fresh budget. + allowed, _ = limiter.check("auth", "5.6.7.8", now=5.0) + assert allowed + + def test_buckets_are_independent(self): + limiter = InMemoryRateLimiter() + for i in range(5): + limiter.check("auth", "1.2.3.4", now=i * 1.0) + # Same IP, different bucket → fresh budget. + allowed, _ = limiter.check("admin", "1.2.3.4", now=5.0) + assert allowed + + def test_unknown_bucket_passes(self): + limiter = InMemoryRateLimiter() + allowed, retry = limiter.check("never-defined", "1.2.3.4") + assert allowed + assert retry == 0.0 + + def test_reset_clears_state(self): + limiter = InMemoryRateLimiter() + for i in range(5): + limiter.check("auth", "1.2.3.4", now=i * 1.0) + limiter.check("auth", "1.2.3.4", now=5.0) # triggers lockout + limiter.reset("auth", "1.2.3.4") + # Lockout cleared, bucket fresh. + allowed, _ = limiter.check("auth", "1.2.3.4", now=6.0) + assert allowed + + def test_custom_buckets(self): + custom = (BucketConfig(name="tiny", limit=2, window_seconds=10.0),) + limiter = InMemoryRateLimiter(buckets=custom) + assert limiter.check("tiny", "1.1.1.1", now=0.0)[0] is True + assert limiter.check("tiny", "1.1.1.1", now=1.0)[0] is True + assert limiter.check("tiny", "1.1.1.1", now=2.0)[0] is False + + +# ── DEFAULT_BUCKETS sanity ───────────────────────────────────── + + +class TestDefaultBuckets: + def test_auth_has_lockout(self): + auth = next(b for b in DEFAULT_BUCKETS if b.name == "auth") + assert auth.limit == 5 + assert auth.window_seconds == 300.0 + assert auth.penalty_seconds == 900.0 + + def test_admin_no_lockout(self): + admin = next(b for b in DEFAULT_BUCKETS if b.name == "admin") + assert admin.penalty_seconds == 0.0 + + def test_default_bucket_is_300_per_minute(self): + d = next(b for b in DEFAULT_BUCKETS if b.name == "default") + assert d.limit == 300 + assert d.window_seconds == 60.0 + + +# ── Middleware closure ───────────────────────────────────────── + + +class _FakeApp: + def __init__( + self, + mode, + limiter=None, + trusted=(), + sec_audit=None, + ): + self.state = type("State", (), {})() + self.state.deployment_mode = mode + self.state.rate_limiter = limiter + self.state.trusted_proxies = trusted + if sec_audit is not None: + self.state.security_audit = sec_audit + + +class _FakeRequest: + def __init__( + self, peer="1.2.3.4", method="POST", path="/auth/login", app=None + ): + self.client = type("C", (), {"host": peer})() + self.method = method + self.url = type("U", (), {"path": path})() + self.headers = {} + self.app = app + + +async def _passthrough(_req): + from fastapi.responses import JSONResponse + + return JSONResponse({"ok": True}) + + +class TestMiddleware: + @pytest.mark.asyncio + async def test_trusted_mode_passthrough(self): + mw = build_rate_limit_middleware() + # Even without a limiter, trusted mode short-circuits. + app = _FakeApp(DeploymentMode.TRUSTED) + for _ in range(20): + resp = await mw( + _FakeRequest(peer="1.2.3.4", path="/auth/login", app=app), + _passthrough, + ) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_lan_mode_throttles_auth(self): + limiter = InMemoryRateLimiter() + mw = build_rate_limit_middleware(limiter=limiter) + app = _FakeApp(DeploymentMode.LAN, limiter=limiter) + # Allow 5, deny 6th. + for _ in range(5): + resp = await mw( + _FakeRequest(peer="1.2.3.4", path="/auth/login", app=app), + _passthrough, + ) + assert resp.status_code == 200 + resp = await mw( + _FakeRequest(peer="1.2.3.4", path="/auth/login", app=app), + _passthrough, + ) + assert resp.status_code == 429 + assert resp.headers.get("Retry-After") is not None + + @pytest.mark.asyncio + async def test_lan_mode_default_bucket_off(self): + limiter = InMemoryRateLimiter( + buckets=(BucketConfig(name="default", limit=2, window_seconds=60),) + ) + mw = build_rate_limit_middleware(limiter=limiter) + app = _FakeApp(DeploymentMode.LAN, limiter=limiter) + # Default bucket is off in LAN mode regardless of limit. + for _ in range(5): + resp = await mw( + _FakeRequest(peer="1.2.3.4", method="GET", path="/api", app=app), + _passthrough, + ) + assert resp.status_code == 200 + + @pytest.mark.asyncio + async def test_web_mode_default_bucket_active(self): + limiter = InMemoryRateLimiter( + buckets=(BucketConfig(name="default", limit=2, window_seconds=60),) + ) + mw = build_rate_limit_middleware(limiter=limiter) + app = _FakeApp(DeploymentMode.WEB, limiter=limiter) + # Web mode honors the default bucket. + for _ in range(2): + resp = await mw( + _FakeRequest(peer="1.2.3.4", method="GET", path="/api", app=app), + _passthrough, + ) + assert resp.status_code == 200 + resp = await mw( + _FakeRequest(peer="1.2.3.4", method="GET", path="/api", app=app), + _passthrough, + ) + assert resp.status_code == 429 + + @pytest.mark.asyncio + async def test_audit_emitted_on_429(self): + from pinky_daemon.security_audit import SecurityAuditStore + + with tempfile.TemporaryDirectory() as tmpdir: + sec_audit = SecurityAuditStore( + db_path=os.path.join(tmpdir, "sec.db") + ) + limiter = InMemoryRateLimiter() + mw = build_rate_limit_middleware(limiter=limiter) + app = _FakeApp( + DeploymentMode.WEB, + limiter=limiter, + sec_audit=sec_audit, + ) + # Burn through auth bucket. + for _ in range(5): + await mw( + _FakeRequest(peer="1.2.3.4", path="/auth/login", app=app), + _passthrough, + ) + await mw( + _FakeRequest(peer="1.2.3.4", path="/auth/login", app=app), + _passthrough, + ) + rows = sec_audit.query(event_type="rate_limit.exceeded") + assert len(rows) == 1 + assert rows[0].outcome == "denied" + assert rows[0].route_name == "auth" + assert rows[0].actor_ip == "1.2.3.4" + assert rows[0].detail == {"bucket": "auth", "limit": 5} + + @pytest.mark.asyncio + async def test_uses_canonical_ip_from_resolver(self): + # Untrusted peer can't pretend to be a different IP via XFF → + # rate limit keys on the peer, not the spoofed XFF. + import ipaddress + + limiter = InMemoryRateLimiter() + mw = build_rate_limit_middleware(limiter=limiter) + # No trusted_proxies → XFF dropped. + app = _FakeApp( + DeploymentMode.WEB, limiter=limiter, trusted=() + ) + # Burn the bucket from peer 1.2.3.4 with spoofed XFF claiming + # 9.9.9.9 — the limiter must key on 1.2.3.4. + req_with_spoof = _FakeRequest( + peer="1.2.3.4", path="/auth/login", app=app + ) + req_with_spoof.headers = {"x-forwarded-for": "9.9.9.9"} + for _ in range(5): + await mw(req_with_spoof, _passthrough) + # 6th from same peer (any spoof) → blocked. + resp = await mw(req_with_spoof, _passthrough) + assert resp.status_code == 429 + # Confirm attribution: a clean request from 9.9.9.9 with no XFF + # is *not* blocked (the spoofed IP wasn't really being charged). + clean = _FakeRequest(peer="9.9.9.9", path="/auth/login", app=app) + resp_clean = await mw(clean, _passthrough) + assert resp_clean.status_code == 200 + # Avoid unused-import warning + _ = ipaddress + + +# ── End-to-end through create_api ────────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +class TestCreateApiRateLimit: + def test_app_state_carries_limiter(self, tmp_db_path): + from pinky_daemon.api import create_api + + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.WEB + ) + assert isinstance( + app.state.rate_limiter, InMemoryRateLimiter + ) + + def test_trusted_mode_no_throttle_through_real_app(self, tmp_db_path): + from fastapi.testclient import TestClient + + from pinky_daemon.api import create_api + + env = {**os.environ, "PINKY_LAN_EXTRA_CIDRS": "0.0.0.0/32"} + with patch.dict(os.environ, env, clear=True): + app = create_api( + db_path=tmp_db_path, deployment_mode=DeploymentMode.TRUSTED + ) + client = TestClient(app) + # Hit /api many times; trusted mode never throttles. + for _ in range(50): + resp = client.get("/api") + assert resp.status_code == 200 + + +# Suppress unused import warning +_ = Any diff --git a/tests/test_security_audit.py b/tests/test_security_audit.py new file mode 100644 index 00000000..231e58f9 --- /dev/null +++ b/tests/test_security_audit.py @@ -0,0 +1,327 @@ +"""Tests for the security audit log (#440).""" + +from __future__ import annotations + +import os +import sqlite3 +import tempfile +from unittest.mock import patch + +import pytest + +from pinky_daemon.security_audit import ( + DEFAULT_DETAIL_ALLOWLIST, + EVENT_DETAIL_ALLOWLIST, + AuditOutcome, + SecurityAuditStore, + redact_detail, +) + +# ── Redaction ────────────────────────────────────────────────── + + +class TestRedactDetail: + def test_none_passes_through(self): + assert redact_detail("auth.login.success", None) is None + + def test_per_event_allowlist_keeps_only_allowed_keys(self): + out = redact_detail( + "auth.login.success", + { + "username": "alice", + "method": "password", + "password": "shh", + "session_token": "abc", + }, + ) + assert out == {"username": "alice", "method": "password"} + + def test_unknown_event_uses_default_allowlist(self): + out = redact_detail( + "made.up.event", + { + "reason": "unspecified", + "duration_ms": 12, + "secret_value": "do not leak", + }, + ) + # "reason" + "duration_ms" are in DEFAULT_DETAIL_ALLOWLIST. + assert out == {"reason": "unspecified", "duration_ms": 12} + + def test_default_allowlist_does_not_include_credential_words(self): + # Sanity: future-defaulted fields don't carry secrets. + for key in ("token", "password", "secret", "authorization"): + assert key not in DEFAULT_DETAIL_ALLOWLIST + + def test_nested_dict_walked_with_same_allowlist(self): + # The allowlist applies at every nesting level. Useful so a + # whitelisted "fields_changed" sub-object can pass through but + # any unexpected key inside it gets dropped. + out = redact_detail( + "admin.update", + { + "fields_changed": {"target_path": "/foo"}, + "target_path": "/x", + "version_after": 7, + "secret": "leak", + }, + ) + assert out == { + "fields_changed": {"target_path": "/foo"}, + "target_path": "/x", + "version_after": 7, + } + + def test_lists_are_walked(self): + out = redact_detail( + "rate_limit.exceeded", + { + "bucket": ["a", "b"], + "count": 5, + "limit": 3, + "secret": "x", + }, + ) + assert out == {"bucket": ["a", "b"], "count": 5, "limit": 3} + + def test_event_with_empty_allowlist_drops_everything(self): + # Manufacture an empty allowlist by patching. + with patch.dict( + EVENT_DETAIL_ALLOWLIST, {"empty.event": frozenset()}, clear=False + ): + assert redact_detail("empty.event", {"x": 1}) == {} + + +# ── Store ────────────────────────────────────────────────────── + + +@pytest.fixture +def store(): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "sec.db") + yield SecurityAuditStore(db_path=path) + + +class TestSecurityAuditStore: + def test_log_event_returns_row_id(self, store): + row_id = store.log_event( + "auth.login.success", + outcome=AuditOutcome.SUCCESS, + actor_user_id=42, + actor_ip="10.0.0.5", + detail={"username": "alice", "method": "password"}, + ) + assert row_id is not None + assert row_id > 0 + + def test_log_redacts_detail(self, store): + store.log_event( + "auth.login.fail", + outcome=AuditOutcome.FAILURE, + detail={"username": "alice", "password": "shh", "reason": "bad"}, + ) + rows = store.query() + assert len(rows) == 1 + assert rows[0].detail == { + "username": "alice", + "method": None if "method" in rows[0].detail else "missing", + "reason": "bad", + } or rows[0].detail == {"username": "alice", "reason": "bad"} + # Either way, no password. + assert "password" not in rows[0].detail + + def test_query_filter_by_event_type(self, store): + store.log_event("auth.login.success", actor_user_id=1) + store.log_event("auth.login.fail", actor_user_id=1) + store.log_event("admin.update", actor_user_id=1) + rows = store.query(event_type="auth.login.fail") + assert len(rows) == 1 + assert rows[0].event_type == "auth.login.fail" + + def test_query_filter_by_actor(self, store): + store.log_event("auth.login.success", actor_user_id=1) + store.log_event("auth.login.success", actor_user_id=2) + rows = store.query(actor_user_id=2) + assert len(rows) == 1 + assert rows[0].actor_user_id == 2 + + def test_query_filter_by_outcome(self, store): + store.log_event("admin.update", outcome=AuditOutcome.SUCCESS) + store.log_event("admin.update", outcome=AuditOutcome.DENIED) + denied = store.query(outcome=AuditOutcome.DENIED) + assert len(denied) == 1 + assert denied[0].outcome == "denied" + + def test_query_filter_by_time_range(self, store): + store.log_event("auth.login.success", ts=1000.0) + store.log_event("auth.login.success", ts=2000.0) + store.log_event("auth.login.success", ts=3000.0) + rows = store.query(since=1500.0, until=2500.0) + assert len(rows) == 1 + assert rows[0].ts == 2000.0 + + def test_query_filter_by_correlation_id(self, store): + store.log_event("auth.login.success", correlation_id="req-1") + store.log_event("admin.update", correlation_id="req-1") + store.log_event("admin.update", correlation_id="req-2") + rows = store.query(correlation_id="req-1") + assert len(rows) == 2 + assert {r.event_type for r in rows} == { + "auth.login.success", + "admin.update", + } + + def test_query_returns_newest_first(self, store): + store.log_event("auth.login.success", ts=1000.0) + store.log_event("auth.login.success", ts=3000.0) + store.log_event("auth.login.success", ts=2000.0) + rows = store.query() + assert [r.ts for r in rows] == [3000.0, 2000.0, 1000.0] + + def test_query_pagination(self, store): + for i in range(5): + store.log_event("auth.login.success", ts=float(i)) + page1 = store.query(limit=2, offset=0) + page2 = store.query(limit=2, offset=2) + assert [r.ts for r in page1] == [4.0, 3.0] + assert [r.ts for r in page2] == [2.0, 1.0] + + def test_count(self, store): + store.log_event("auth.login.success", outcome=AuditOutcome.SUCCESS) + store.log_event("auth.login.fail", outcome=AuditOutcome.FAILURE) + store.log_event("auth.login.fail", outcome=AuditOutcome.FAILURE) + assert store.count() == 3 + assert store.count(event_type="auth.login.fail") == 2 + assert store.count(outcome=AuditOutcome.SUCCESS) == 1 + + def test_via_proxy_and_forwarded_chain_round_trip(self, store): + store.log_event( + "auth.login.success", + actor_ip="203.0.113.7", + via_proxy=True, + forwarded_chain=["203.0.113.7", "10.0.0.5"], + ) + rows = store.query() + assert rows[0].via_proxy is True + assert rows[0].forwarded_chain == ["203.0.113.7", "10.0.0.5"] + + def test_method_route_user_agent_round_trip(self, store): + store.log_event( + "admin.update", + method="POST", + route_name="admin.update", + user_agent="curl/8.6.0", + target="/admin/update", + ) + r = store.query()[0] + assert r.method == "POST" + assert r.route_name == "admin.update" + assert r.user_agent == "curl/8.6.0" + assert r.target == "/admin/update" + + def test_log_event_swallows_db_error(self, store): + # Force a write failure: close the underlying connection and try + # to log. Best-effort contract → returns None instead of raising. + store._db.close() + # Replace with a closed connection so the next op raises. + result = store.log_event( + "auth.login.success", actor_user_id=99 + ) + assert result is None + + +# ── Retention ────────────────────────────────────────────────── + + +class TestRetention: + def test_prune_older_than(self): + with tempfile.TemporaryDirectory() as tmpdir: + store = SecurityAuditStore(db_path=os.path.join(tmpdir, "x.db")) + now = 10_000_000.0 # arbitrary anchor + store.log_event("auth.login.success", ts=now - 86400 * 400) + store.log_event("auth.login.success", ts=now - 86400 * 200) + store.log_event("auth.login.success", ts=now - 86400 * 1) + deleted = store.prune_older_than(365, now=now) + assert deleted == 1 + remaining = store.query() + assert len(remaining) == 2 + + def test_prune_zero_or_negative_is_noop(self): + with tempfile.TemporaryDirectory() as tmpdir: + store = SecurityAuditStore(db_path=os.path.join(tmpdir, "x.db")) + store.log_event("auth.login.success", ts=1.0) + assert store.prune_older_than(0) == 0 + assert store.prune_older_than(-7) == 0 + assert store.count() == 1 + + +# ── Schema ───────────────────────────────────────────────────── + + +class TestSchema: + def test_table_and_indexes_present(self): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "s.db") + SecurityAuditStore(db_path=path) + con = sqlite3.connect(path) + tables = { + r[0] + for r in con.execute( + "SELECT name FROM sqlite_master WHERE type='table'" + ).fetchall() + } + assert "security_audit_log" in tables + indexes = { + r[0] + for r in con.execute( + "SELECT name FROM sqlite_master WHERE type='index'" + ).fetchall() + } + assert "idx_sec_audit_ts" in indexes + assert "idx_sec_audit_event" in indexes + assert "idx_sec_audit_actor" in indexes + assert "idx_sec_audit_correlation" in indexes + + def test_prev_hash_column_reserved(self): + # Reserved for future hash-chain integrity work — not populated + # in this PR, but the column must exist so we don't take a + # migration later. + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "s.db") + SecurityAuditStore(db_path=path) + con = sqlite3.connect(path) + cols = { + r[1] + for r in con.execute( + "PRAGMA table_info(security_audit_log)" + ).fetchall() + } + assert "prev_hash" in cols + + +# ── Wiring through create_api ────────────────────────────────── + + +class TestCreateApiWiring: + def test_app_state_carries_security_audit(self): + from pinky_daemon.api import create_api + from pinky_daemon.config import DeploymentMode + + with tempfile.TemporaryDirectory() as tmpdir: + db = os.path.join(tmpdir, "test.db") + app = create_api( + db_path=db, deployment_mode=DeploymentMode.TRUSTED + ) + assert isinstance( + app.state.security_audit, SecurityAuditStore + ) + # Smoke: write + read back a row. + row_id = app.state.security_audit.log_event( + "boot.daemon.start", + outcome=AuditOutcome.SUCCESS, + detail={"deployment_mode": "trusted"}, + ) + assert row_id is not None + rows = app.state.security_audit.query() + assert len(rows) == 1 + assert rows[0].detail == {"deployment_mode": "trusted"} diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py new file mode 100644 index 00000000..13024ee6 --- /dev/null +++ b/tests/test_security_headers.py @@ -0,0 +1,322 @@ +"""Tests for the mode-aware security headers middleware (#437). + +Covers: + +* The static header matrix per mode (`headers_for_mode`). +* CSP construction per mode (`build_csp`). +* HSTS verification logic (`is_verified_https`). +* The ASGI middleware closure: header presence, CSP report-only vs. + enforcing, HSTS only emitted when verified, websocket upgrades pass + through untouched. +* End-to-end through `create_api` for trusted / lan / web. +""" + +from __future__ import annotations + +import ipaddress +import os +import tempfile +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from pinky_daemon.config import DeploymentMode +from pinky_daemon.middleware.security_headers import ( + CSP_ENFORCING_ENV, + build_csp, + build_security_headers_middleware, + headers_for_mode, + is_verified_https, +) + +# ── Static header matrix ────────────────────────────────────── + + +class TestHeadersForMode: + def test_trusted_minimal(self): + h = headers_for_mode(DeploymentMode.TRUSTED) + assert h["X-Content-Type-Options"] == "nosniff" + assert h["X-XSS-Protection"] == "0" + # Trusted does NOT add the lan/web set. + assert "X-Frame-Options" not in h + assert "Referrer-Policy" not in h + assert "Permissions-Policy" not in h + + def test_lan_adds_clickjacking_referrer_perms(self): + h = headers_for_mode(DeploymentMode.LAN) + assert h["X-Content-Type-Options"] == "nosniff" + assert h["X-XSS-Protection"] == "0" + assert h["X-Frame-Options"] == "DENY" + assert h["Referrer-Policy"] == "strict-origin-when-cross-origin" + assert "Permissions-Policy" in h + # Basic policy: a few things off. + assert "camera=()" in h["Permissions-Policy"] + + def test_web_uses_strict_permissions_policy(self): + h = headers_for_mode(DeploymentMode.WEB) + # Strict policy lists more sinks. + assert "interest-cohort=()" in h["Permissions-Policy"] + assert "payment=()" in h["Permissions-Policy"] + # Web still has the lan-set headers. + assert h["X-Frame-Options"] == "DENY" + + def test_x_xss_protection_disabled_in_all_modes(self): + # Modern recommendation: disable the legacy auditor explicitly. + for mode in DeploymentMode: + assert headers_for_mode(mode)["X-XSS-Protection"] == "0" + + +# ── CSP ──────────────────────────────────────────────────────── + + +class TestBuildCsp: + def test_lan_csp_has_required_directives(self): + csp = build_csp(DeploymentMode.LAN) + for directive in ( + "default-src 'self'", + "script-src 'self'", + "style-src 'self' 'unsafe-inline'", # known compromise per CLAUDE.md + "img-src 'self' data:", + "connect-src 'self' ws: wss:", + "frame-ancestors 'none'", + "base-uri 'self'", + "form-action 'self'", + ): + assert directive in csp + + def test_web_csp_adds_object_src_and_upgrade(self): + csp = build_csp(DeploymentMode.WEB) + assert "object-src 'none'" in csp + assert "upgrade-insecure-requests" in csp + + def test_lan_csp_does_not_add_web_only_directives(self): + csp = build_csp(DeploymentMode.LAN) + assert "object-src 'none'" not in csp + assert "upgrade-insecure-requests" not in csp + + +# ── HSTS verification ────────────────────────────────────────── + + +class _FakeApp: + def __init__(self, mode, trusted=()): + self.state = type("State", (), {})() + self.state.deployment_mode = mode + self.state.trusted_proxies = trusted + + +class _FakeRequest: + def __init__( + self, + peer="127.0.0.1", + scheme="http", + headers=None, + app=None, + ): + self.client = type("C", (), {"host": peer})() + self.url = type("U", (), {"scheme": scheme})() + self.headers = headers or {} + self.app = app or _FakeApp(DeploymentMode.WEB) + + +class TestIsVerifiedHttps: + def test_direct_tls_is_verified(self): + req = _FakeRequest(scheme="https", peer="127.0.0.1") + assert is_verified_https(req) is True + + def test_xforwarded_proto_https_with_trusted_peer_is_verified(self): + trusted = (ipaddress.ip_network("127.0.0.1/32"),) + app = _FakeApp(DeploymentMode.WEB, trusted=trusted) + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-proto": "https"}, + app=app, + ) + assert is_verified_https(req) is True + + def test_xforwarded_proto_https_with_untrusted_peer_rejected(self): + # Direct attacker on the open internet sets the header. + # MUST NOT poison HSTS. + app = _FakeApp(DeploymentMode.WEB, trusted=()) + req = _FakeRequest( + peer="203.0.113.7", + headers={"x-forwarded-proto": "https"}, + app=app, + ) + assert is_verified_https(req) is False + + def test_xforwarded_proto_http_returns_false(self): + trusted = (ipaddress.ip_network("127.0.0.1/32"),) + app = _FakeApp(DeploymentMode.WEB, trusted=trusted) + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-proto": "http"}, + app=app, + ) + assert is_verified_https(req) is False + + def test_no_proto_header_no_tls_returns_false(self): + req = _FakeRequest(peer="127.0.0.1") + assert is_verified_https(req) is False + + +# ── Middleware closure ──────────────────────────────────────── + + +async def _make_response(_req): + from fastapi.responses import JSONResponse + + return JSONResponse({"ok": True}) + + +class TestMiddlewareClosure: + @pytest.mark.asyncio + async def test_trusted_mode_minimal_headers(self): + mw = build_security_headers_middleware(csp_enforcing=False) + req = _FakeRequest(app=_FakeApp(DeploymentMode.TRUSTED)) + resp = await mw(req, _make_response) + assert resp.headers["X-Content-Type-Options"] == "nosniff" + assert resp.headers["X-XSS-Protection"] == "0" + assert "X-Frame-Options" not in resp.headers + assert "Content-Security-Policy" not in resp.headers + assert "Content-Security-Policy-Report-Only" not in resp.headers + assert "Strict-Transport-Security" not in resp.headers + + @pytest.mark.asyncio + async def test_lan_mode_emits_csp_report_only_by_default(self): + mw = build_security_headers_middleware(csp_enforcing=False) + req = _FakeRequest(app=_FakeApp(DeploymentMode.LAN)) + resp = await mw(req, _make_response) + assert resp.headers["X-Frame-Options"] == "DENY" + assert "Content-Security-Policy-Report-Only" in resp.headers + assert "Content-Security-Policy" not in resp.headers + # No HSTS in LAN even with verified HTTPS. + assert "Strict-Transport-Security" not in resp.headers + + @pytest.mark.asyncio + async def test_csp_enforcing_emits_enforcing_header(self): + mw = build_security_headers_middleware(csp_enforcing=True) + req = _FakeRequest(app=_FakeApp(DeploymentMode.WEB)) + resp = await mw(req, _make_response) + assert "Content-Security-Policy" in resp.headers + assert "Content-Security-Policy-Report-Only" not in resp.headers + + @pytest.mark.asyncio + async def test_csp_enforcing_env_flips_to_enforcing(self): + mw = build_security_headers_middleware() + req = _FakeRequest(app=_FakeApp(DeploymentMode.LAN)) + with patch.dict(os.environ, {CSP_ENFORCING_ENV: "true"}, clear=False): + resp = await mw(req, _make_response) + assert "Content-Security-Policy" in resp.headers + + @pytest.mark.asyncio + async def test_web_mode_no_hsts_without_verified_https(self): + mw = build_security_headers_middleware() + req = _FakeRequest(app=_FakeApp(DeploymentMode.WEB)) # http + resp = await mw(req, _make_response) + assert "Strict-Transport-Security" not in resp.headers + + @pytest.mark.asyncio + async def test_web_mode_hsts_when_direct_tls(self): + mw = build_security_headers_middleware() + req = _FakeRequest( + scheme="https", app=_FakeApp(DeploymentMode.WEB) + ) + resp = await mw(req, _make_response) + assert ( + resp.headers["Strict-Transport-Security"] + == "max-age=31536000; includeSubDomains" + ) + + @pytest.mark.asyncio + async def test_web_mode_hsts_when_trusted_proxy_says_https(self): + mw = build_security_headers_middleware() + trusted = (ipaddress.ip_network("127.0.0.1/32"),) + app = _FakeApp(DeploymentMode.WEB, trusted=trusted) + req = _FakeRequest( + peer="127.0.0.1", + headers={"x-forwarded-proto": "https"}, + app=app, + ) + resp = await mw(req, _make_response) + assert "Strict-Transport-Security" in resp.headers + + @pytest.mark.asyncio + async def test_web_mode_no_hsts_when_untrusted_proxy_claims_https(self): + # The HSTS poisoning vector — must remain blocked. + mw = build_security_headers_middleware() + app = _FakeApp(DeploymentMode.WEB, trusted=()) + req = _FakeRequest( + peer="203.0.113.7", + headers={"x-forwarded-proto": "https"}, + app=app, + ) + resp = await mw(req, _make_response) + assert "Strict-Transport-Security" not in resp.headers + + @pytest.mark.asyncio + async def test_websocket_upgrade_passes_through_untouched(self): + # Returning a non-Response sentinel lets us assert no header + # mutation happened. (The middleware short-circuits before + # call_next's return is touched.) + async def _ws_response(_req): + return "ws-handshake-result" + + mw = build_security_headers_middleware() + req = _FakeRequest( + headers={"upgrade": "websocket"}, + app=_FakeApp(DeploymentMode.WEB), + ) + out = await mw(req, _ws_response) + assert out == "ws-handshake-result" + + +# ── End-to-end through create_api ────────────────────────────── + + +@pytest.fixture +def tmp_db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "test.db") + + +def _api_with_lan_bypass(tmp_db_path, mode: DeploymentMode): + """Build a real FastAPI app for ``mode`` while letting TestClient + through the LAN filter (TestClient's peer is 0.0.0.0).""" + from pinky_daemon.api import create_api + + env = {**os.environ, "PINKY_LAN_EXTRA_CIDRS": "0.0.0.0/32"} + with patch.dict(os.environ, env, clear=True): + return create_api(db_path=tmp_db_path, deployment_mode=mode) + + +class TestCreateApiSecurityHeaders: + def test_trusted_mode_minimal_headers_on_real_request(self, tmp_db_path): + app = _api_with_lan_bypass(tmp_db_path, DeploymentMode.TRUSTED) + client = TestClient(app) + resp = client.get("/api") + assert resp.headers["X-Content-Type-Options"] == "nosniff" + assert resp.headers["X-XSS-Protection"] == "0" + assert "X-Frame-Options" not in resp.headers + assert "Content-Security-Policy" not in resp.headers + assert "Content-Security-Policy-Report-Only" not in resp.headers + + def test_lan_mode_full_set_minus_hsts(self, tmp_db_path): + app = _api_with_lan_bypass(tmp_db_path, DeploymentMode.LAN) + client = TestClient(app) + resp = client.get("/api") + assert resp.headers["X-Frame-Options"] == "DENY" + assert resp.headers["Referrer-Policy"] == "strict-origin-when-cross-origin" + # CSP report-only by default + assert "Content-Security-Policy-Report-Only" in resp.headers + assert "Strict-Transport-Security" not in resp.headers + + def test_web_mode_no_hsts_over_plain_http(self, tmp_db_path): + # TestClient uses http://, no trusted-proxy header → HSTS must NOT + # appear, even though we're in web mode. + app = _api_with_lan_bypass(tmp_db_path, DeploymentMode.WEB) + client = TestClient(app) + resp = client.get("/api") + assert "Strict-Transport-Security" not in resp.headers + assert "Content-Security-Policy-Report-Only" in resp.headers