Skip to content

[CI validation only — do not merge] Combined hardening stack (#472-#479) - #480

Draft
olegbrok wants to merge 8 commits into
mainfrom
validate/hardening-stack-ci
Draft

[CI validation only — do not merge] Combined hardening stack (#472-#479)#480
olegbrok wants to merge 8 commits into
mainfrom
validate/hardening-stack-ci

Conversation

@olegbrok

Copy link
Copy Markdown
Collaborator

Purpose

CI validation only — do not merge.

This is a synthetic draft PR that exists solely to run the CI + CodeQL workflows against the full cumulative diff of the 8-PR hardening stack (#472#479).

The individual stacked PRs target their parent branch (not `main`), which means `ci.yml` and `codeql.yml` — gated on `pull_request: branches: [main]` — don't fire on them. This draft surfaces the full-stack result so we can see the combined diff go green (or not) before merging the stack PR-by-PR.

What's in it

All 8 commits from `feat/csrf-stacked`:

When this gets closed

🤖 Opened by Barsik

Oleg added 8 commits May 13, 2026 00:21
…_mode (#434)

PR-1 of the #433 web-exposed hardening track. Establishes the single env-var
switch downstream middleware will branch on (#436 LAN filter, #437 headers,
#439 admin elevation, #441 boot guards, etc.) instead of each feature carrying
its own knob.

What
----
* New `pinky_daemon.config` module:
  - `DeploymentMode` enum: `trusted` (default, current Mac Mini), `lan`,
    `web`.
  - `resolve_deployment_mode()` reads `PINKY_DEPLOYMENT_MODE`, normalises
    case + whitespace, **fails closed** on invalid values
    (`InvalidDeploymentModeError`) — a typo like `prod` aborts startup
    rather than silently falling back to `trusted`.
  - `default_bind_host(mode)`: `0.0.0.0` for trusted/lan, `127.0.0.1` for
    web (assumes a reverse proxy in front).
  - `warn_if_insecure_exposure(mode, host)`: loud stderr warning when
    `trusted` + `0.0.0.0`, so accidental VPS exposure is obvious in logs
    while back-compat is preserved.

* `pinky_daemon.__main__`:
  - `--host` default flipped to `None` so we can tell "operator picked
    0.0.0.0" from "old hardcoded default" — needed by #436/#441.
  - On startup: resolve mode → resolve host → warn → log mode + host.
  - `InvalidDeploymentModeError` → `sys.exit(2)` with a useful message.

* `pinky_daemon.api.create_api`:
  - New `deployment_mode` kwarg; if `None`, resolved from env so embedded
    callers and tests don't have to wire it through.
  - Cached on `app.state.deployment_mode` — single source of truth for
    the rest of #433.
  - `/api` payload exposes `deployment_mode` so VPS operators can confirm
    posture without scraping logs.

Tests (26 new)
--------------
* Resolver: empty/unset → trusted; each canonical value; case-insensitive;
  whitespace stripped; invalid value raises with a useful message; aliases
  like "production" / "dev" don't silently pass.
* `default_bind_host`: trusted/lan → 0.0.0.0, web → 127.0.0.1.
* `warn_if_insecure_exposure`: warns only on trusted + 0.0.0.0; quiet on
  lan/web and on specific bind addresses.
* `create_api`: explicit mode cached on `app.state`; defaults resolve from
  env; `/api` exposes the mode for each posture.

Existing test_api.py: 286 tests still green.

Closes part of #433.
Refs #434.

🤖 Authored by Barsik
PR-2 of the #433 web-exposed hardening track. Establishes one canonical
"what is the real client IP for this request?" answer that #436 (LAN
filter), #438 (rate limit key), #440 (audit attribution), and #441 (boot
guards) all read from instead of each parsing X-Forwarded-For
independently.

Stacks on #472 (PINKY_DEPLOYMENT_MODE).

What
----
* New `pinky_daemon.net` subpackage for cross-cutting network helpers.
* `pinky_daemon.net.client_identity`:
  - `ClientIdentity` frozen dataclass: `ip`, `via_proxy`, `raw_peer`,
    `forwarded_chain` — what every IP-consuming feature operates on.
  - `parse_trusted_proxies(value)` — comma-separated CIDRs → tuple of
    IPNetwork. Bare IPs accepted as /32 or /128. Invalid entries logged
    and skipped (don't crash startup on operator typos in the list).
  - `resolve_client_identity(request, mode, trusted_proxies)`:
    * `trusted` → ip = raw_peer; XFF / Forwarded ignored entirely.
    * `lan` / `web` → XFF + RFC 7239 Forwarded parsed, but only
      honored when `raw_peer` is in `trusted_proxies`. Otherwise the
      chain is dropped and a throttled WARNING is emitted (suspected
      header spoofing).
    * Forwarded wins over X-Forwarded-For.
    * IPv6-mapped IPv4 (`::ffff:1.2.3.4`) normalised to v4 before CIDR
      comparison so dual-stack listeners actually match v4 trust rules.
    * Right-to-left walk of the chain, skipping trusted hops; falls back
      to leftmost if the entire chain is trusted.
    * Malformed XFF / Forwarded → fall back to raw_peer; warning
      throttled to once per minute per failure class.
  - `get_client_identity(request)` — FastAPI dependency. Reads
    `app.state.deployment_mode` and `app.state.trusted_proxies`; falls
    back to env when state isn't set so tests don't have to wire it.
* `create_api`:
  - Reads `PINKY_TRUSTED_PROXIES` once at startup, caches parsed CIDRs
    on `app.state.trusted_proxies`.
  - Loopback is **not** trusted by default — operator opt-in only,
    otherwise audit/rate-limit attribution silently flattens to
    127.0.0.1 when something upstream goes wrong.

Tests (47 new)
--------------
* `parse_trusted_proxies`: empty/None/whitespace; bare v4/v6; multiple
  with whitespace; invalid entries skipped; host-bits-set accepted.
* `trusted` mode: peer wins, XFF + Forwarded both ignored.
* `lan` and `web` modes (parametrised): no-XFF; untrusted-peer XFF
  dropped; trusted-peer XFF honored; multi-hop chain skips trusted
  hops; all-trusted-chain returns leftmost; IPv6-mapped IPv4
  normalisation; Forwarded precedence; quoted bracketed IPv6 in
  Forwarded; XFF with `host:port` form; fully malformed → fallback to
  peer; partial malformed → keep good entries; missing `client`
  attribute → 0.0.0.0.
* `ClientIdentity` immutability and default empty-tuple chain.
* `get_client_identity` reads from app.state and falls back to env.
* End-to-end through `create_api` + TestClient.
* Untrusted-peer warning is logged.

286 existing api tests + 26 deployment-mode tests still green.

Closes part of #433. Refs #442.

🤖 Authored by Barsik
PR-3 of the #433 web-exposed hardening track. Stacked on #473 (consumes
ClientIdentity from #442).

What
----
* New `pinky_daemon.net.lan_filter`:
  - `DEFAULT_PRIVATE_CIDRS` — RFC1918 + IPv4 link-local + IPv4 loopback
    + IPv6 ULA + IPv6 link-local + IPv6 loopback. CGNAT
    (100.64.0.0/10) is **not** included by design — operators on
    Tailscale opt in via `PINKY_LAN_EXTRA_CIDRS`.
  - `is_private_ip(addr, allowlist)` — pure helper with v4/v6 type
    guards so a mixed-family allowlist doesn't crash with TypeError.
  - `resolve_lan_allowed_cidrs(env)` — composes defaults + parsed
    extras from `PINKY_LAN_EXTRA_CIDRS` (lenient: malformed entries
    logged + skipped).
  - `build_lan_filter_middleware(allowlist)` — ASGI middleware factory:
    * No-op pass-through in `trusted` and `web` modes.
    * In `lan` mode, computes the canonical client IP via
      `resolve_client_identity` (so XFF parsing matches #442) and
      returns 403 when the IP isn't in the allowlist.
    * Logs reject at INFO with method, path, ip, raw_peer, via_proxy
      so scan/probe traffic is visible without flooding ERROR.

* `create_api`:
  - Reads `PINKY_LAN_EXTRA_CIDRS` once, caches the composed allowlist
    on `app.state.lan_allowed_cidrs`.
  - Registers the LAN filter middleware **early** (before auth, before
    route handlers) so public-source traffic in LAN mode is dropped
    before any password-check work happens.

Murzik review followups (from #436)
-----------------------------------
* [x] Defer trusted-IP resolution to #442 — middleware uses
  `resolve_client_identity` end-to-end; no XFF parsing here.
* [x] LAN middleware ordering: very early (registered first in
  create_api).
* [x] Never trust XFF in LAN mode unless peer ∈ trusted_proxies —
  inherited from #442 via the resolver call.
* [x] CIDR list: RFC1918 / link-local / ULA only. CGNAT explicitly
  excluded; opt-in via env.
* [x] Test coverage: IPv6-mapped IPv4 (covered by #442 tests +
  is_private_ip type-guard test), comma-separated XFF (#442 tests),
  malformed XFF (#442 tests).

Out of scope (deferred)
-----------------------
* Bind defaults — landed in #472 (#434).
* Refuse-to-boot when web + 0.0.0.0 without TLS marker — owned by
  #441.

Tests (34 new)
--------------
* `is_private_ip`: 9 private addresses accepted, 7 public/just-outside
  addresses rejected, CGNAT rejected by default + accepted with
  extras, mixed-family no-crash, empty allowlist rejects everything.
* `resolve_lan_allowed_cidrs`: defaults when unset, extras appended,
  invalid extras skipped.
* Middleware closure: trusted/web pass-through, LAN accepts private,
  LAN rejects public, LAN drops untrusted-peer XFF, LAN honors
  trusted-peer XFF, LAN honors operator-extra CIDRs, LAN falls back
  to env when no allowlist on state.
* End-to-end through `create_api` (TestClient) for trusted (no
  filter), LAN (default-rejects testclient peer), LAN with extras
  (testclient peer allowed), web (no filter).

Adjusted `test_deployment_mode::test_api_endpoint_exposes_mode_for_each_value`
to opt 0.0.0.0/32 into the LAN allowlist so the loop reaches /api in
every mode.

Closes part of #433. Refs #436.

🤖 Authored by Barsik
PR-4 of the #433 web-exposed hardening track. Replaces the inline
helmet-style middleware in api.py with a mode-aware module that adds CSP,
HSTS-when-verified, and a stricter permissions-policy on top of what was
already shipped.

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``
============================  ========  =====  =====

Stacked on #474#473#472.

What
----
* `pinky_daemon.middleware.security_headers`:
  - `headers_for_mode(mode)` — static header set per posture.
  - `build_csp(mode)` — CSP string (web adds object-src 'none' +
    upgrade-insecure-requests). 'unsafe-inline' for style-src is
    documented compromise per CLAUDE.md.
  - `is_verified_https(request)` — direct TLS OR
    `X-Forwarded-Proto: https` from a peer ∈ trusted_proxies. Bare
    `X-Forwarded-Proto: https` from any other peer is NOT enough —
    closes the HSTS-poisoning vector.
  - `build_security_headers_middleware(csp_enforcing=None)` — ASGI
    middleware closure. Reads `app.state.deployment_mode` per request.
    CSP defaults to `Content-Security-Policy-Report-Only`; flip to
    `Content-Security-Policy` via `PINKY_CSP_ENFORCING=1` env or
    `csp_enforcing=True` arg. Soak in report-only first.
* `pinky_daemon.net.client_identity`: promoted `peer_is_trusted` and
  `raw_peer_from_request` from underscore-private to public — needed
  by the security_headers HSTS check and by future #438 / #439 work.
* `create_api`: replaced inline middleware with
  `build_security_headers_middleware()`.

Murzik review followups (from #437)
-----------------------------------
* [x] HSTS only emitted when HTTPS is **verified** through a trusted
  proxy or direct TLS — bare `X-Forwarded-Proto` from untrusted peers
  ignored.
* [x] CSP runs in report-only by default; operator opts into enforcing
  via env.
* [x] CSP allows `data:` for img-src, `'unsafe-inline'` for style-src
  (Svelte inline-styles compromise documented in module docstring).
* [x] `connect-src 'self' ws: wss:` for SSE/websocket.
* [x] `frame-ancestors 'none'` (redundant with X-Frame-Options DENY,
  fine).
* [x] X-XSS-Protection: 0 in all modes (modern OWASP guidance).
* [x] Twilio callbacks unaffected (server-to-server, no CSP relevance).

Tests (24 new)
--------------
* `headers_for_mode`: trusted minimal, lan + web headers, X-XSS=0
  everywhere.
* `build_csp`: lan/web required directives, web-only extras.
* `is_verified_https`: direct TLS, trusted-peer XFP, untrusted-peer
  XFP rejected, XFP=http rejected, no header rejected.
* Middleware closure: trusted minimal, lan CSP report-only, CSP
  enforcing kwarg + env, web HSTS only when verified, web HSTS NOT
  when untrusted-peer claims https, websocket upgrade pass-through.
* End-to-end `create_api`: trusted minimal, lan full set + report-only
  CSP, web no HSTS over plain http.

286 existing api tests still green.

Closes part of #433. Refs #437.

🤖 Authored by Barsik
…441)

PR-5 of the #433 web-exposed hardening track. Loud-failure boot-time
checks for the obviously-unsafe configurations an internet-exposed
deployment must not run with. Stacked on #475#474#473#472.

Scope of this PR
================
Ships the guards that don't depend on unshipped pieces of the track.
The remaining guards land alongside their owning PRs (admin user,
legacy UI password → #435; multi-worker rate-limit backend → #438;
audit emission for overrides → #440). Each deferral is marked with a
TODO referencing the owning PR.

Guards (fatal, web mode only)
-----------------------------
* SESSION_SECRET — PINKY_SESSION_SECRET unset, < 32 chars, one of a
  small placeholder set (changeme/secret/pinky/...), or trivially
  low-entropy (≤ 2 distinct chars).
* PUBLIC_BIND_NO_TLS — bound to 0.0.0.0 / a non-loopback host without
  PINKY_BEHIND_TLS_PROXY=true to acknowledge upstream TLS termination.
* MISSING_TRUSTED_PROXIES — PINKY_BEHIND_TLS_PROXY=true with
  PINKY_TRUSTED_PROXIES empty; otherwise audit + rate-limit attribution
  silently flatten to 127.0.0.1 because #442's resolver refuses to
  honor XFF from an untrusted peer.
* DB_PERMISSIONS — DB file or its parent directory has loose
  permissions (group/other bits set). No-op on Windows.

Warnings (non-fatal, all modes)
-------------------------------
* RUNNING_AS_ROOT — daemon process UID == 0.

Override mechanism
------------------
Per-guard PINKY_ALLOW_INSECURE_<NAME>=true env vars. **No global
escape hatch by design** — operators who 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.

Wiring
------
* assert_web_mode_safe(mode, host, db_path) called from
  pinky_daemon.__main__._run_api after argparse + mode resolution and
  before uvicorn.run.
* Failure → preformatted rejection block to stderr; sys.exit(3).
  Distinct exit code from invalid-mode (#434 uses 2) so wrapper
  scripts can route on it.

Tests (34 new)
--------------
* check_session_secret: unset / short / placeholder (parametrised) /
  long-random-passes / low-entropy / override.
* check_public_bind_requires_tls_marker: loopback variants pass /
  public bind no marker fails / public bind with marker passes /
  specific public address fails / override.
* check_trusted_proxies_when_behind_tls: no marker passes / marker
  with proxies passes / marker without proxies fails.
* check_db_permissions: nonexistent skips / secure dir + file pass /
  world-readable dir + file fail.
* run_warning_guards: empty list when nothing to warn / running as
  root warning / silenced by override.
* assert_web_mode_safe: trusted/lan skip all / web all-pass / web
  aggregates failures / overrides let boot proceed.
* format_guard_error: contains all expected sections.

Closes part of #433. Refs #441.

🤖 Authored by Barsik
PR-6 of the #433 web-exposed hardening track. Introduces a dedicated
security-event audit log distinct from the per-tool-call audit in
hooks.AuditStore. Stacked on #476#475#474#473#472.

Why a new store
===============
hooks.AuditStore tracks tool execution (agent_name, session_id,
tool_name, cost) — useful for cost/usage attribution. #440's audit log
tracks security events (auth, admin, network reject, rate-limit, boot
guards) with a different schema and a different retention story:

* Security audit benefits from a focused export job that doesn't have
  to scan the bulky tool-call history.
* Append-only by convention; no UPDATE/DELETE API.
* Best-effort writes: a locked DB must NOT take down login.

Schema
======
security_audit_log:
  id, ts, event_type,
  actor_user_id, actor_token_id, actor_ip,
  via_proxy, forwarded_chain (JSON),
  method, route_name, target,
  outcome, user_agent, correlation_id,
  detail_json,
  prev_hash       -- reserved for future hash-chain integrity

Indexes on (ts), (event_type, ts), (actor_user_id, ts),
(correlation_id) — query patterns are time-windowed by event/actor
filters or correlation-id traces across multi-hop requests.

Redaction
=========
Allowlist-shaped per event_type. Define what's *kept*, not what's
stripped, so a future credential field name added to login flow
defaults to redacted instead of accidentally leaking.

EVENT_DETAIL_ALLOWLIST registers the allowed keys for known event
types (auth.login.success/fail, auth.token.create/revoke, admin.*,
rate_limit.exceeded, network.lan_filter.reject, boot.*). Unknown
event types fall back to DEFAULT_DETAIL_ALLOWLIST — a conservative set
({reason, duration_ms, status_code, error_class, count, limit, bucket}).

Best-effort writes
==================
log_event catches all exceptions, logs at WARNING via the standard
logger, returns None instead of raising. Boot-time refuse-to-boot
events that need fail-loud semantics call assert_web_mode_safe (#441)
which raises BootGuardError; the audit log there is informational
only.

Wiring
======
SecurityAuditStore is constructed in create_api alongside the existing
AuditStore and cached on app.state.security_audit. Routes will reach
it via Depends() once #435 / #438 / #439 / #443 land their consumers.

Out of scope (deferred)
=======================
* GET /audit endpoints (admin-only) — depend on #435 user accounts +
  #439 elevated-session protection. Land alongside those PRs.
* GET /audit/export (CSV/JSON) — same dependency chain.
* Retention cron wiring — store ships prune_older_than(); the
  scheduler wiring lands as a one-line follow-up once review confirms
  the desired default (365d).
* Hash-chain population — column is reserved for the integrity work
  but not populated in this PR.
* Override-emits-audit for boot guards (#441) — needs this PR + an
  obvious wiring point. Land in a follow-up that depends on both.

Tests (25 new)
==============
* redact_detail: None pass-through, per-event allowlist, default
  allowlist for unknown events, default doesn't include credential
  words, nested dict walked, lists walked, empty allowlist drops
  everything.
* SecurityAuditStore: log returns id, redacts on write, query filters
  (event_type, actor, outcome, time range, correlation_id), newest-first
  ordering, pagination, count, via_proxy + forwarded_chain round-trip,
  method/route/user_agent round-trip, best-effort returns None on
  closed DB.
* prune_older_than: deletes older rows, no-op on zero/negative.
* Schema: table + all four indexes present, prev_hash column reserved.
* Wiring: app.state.security_audit is a SecurityAuditStore instance;
  smoke-write through it.

Closes part of #433. Refs #440.

🤖 Authored by Barsik
PR-7 of the #433 web-exposed hardening track. Slows brute-force on
login and burst-probing on admin endpoints. Stacked on #477#476#475#474#473#472.

Buckets
=======
============  ================================  =====  ======  =================
Bucket        Routes                            Limit  Window  Penalty
============  ================================  =====  ======  =================
auth          POST /auth/login + /auth/password 5      300s    900s lockout
admin         /admin/*                          30     60s     none
webhook       /triggers/webhook/*               60     60s     none
default       everything else                   300    60s     none (lan/web)
============  ================================  =====  ======  =================

Mode behaviour
--------------
* trusted — disabled (back-compat with Mac Mini)
* lan     — auth + admin only
* web     — all four buckets

IP attribution
--------------
Keys come from the canonical client IP from #442's resolver. raw
X-Forwarded-For is NOT trusted; an attacker on the public internet
can't dodge the auth bucket by spoofing XFF (test
test_uses_canonical_ip_from_resolver verifies this end-to-end).

Audit
-----
Every 429 emits a `rate_limit.exceeded` event into the security audit
(#440) with bucket, limit, ip, via_proxy, forwarded_chain, method,
target. No-op when security_audit isn't on app.state.

Implementation
==============
* InMemoryRateLimiter — sliding-window-counter; thread-safe via single
  mutex (microsecond hold time). Per-process state.
* BucketConfig dataclass — limit, window, optional penalty_seconds.
* classify_request(method, path) — pure function so route → bucket
  mapping is unit-testable.
* build_rate_limit_middleware(limiter=None) — closure; reads
  app.state.rate_limiter when no explicit limiter is passed so a
  process can hot-swap config.

Wiring
======
create_api installs InMemoryRateLimiter on app.state.rate_limiter and
registers the middleware. Order matters — runs after security headers
so 429 responses still get headers; before route handlers so admin
work doesn't kick off when limiter says no.

Out of scope (deferred)
=======================
* Login keying on (IP, normalized email) — needs the auth user model
  from #435. Today: IP only. TODO marker in the module.
* Per-user admin bucket — same dependency on #435.
* Webhook keying on token-hash-prefix — needs the token model from
  #435. Today: IP only.
* Redis backend for multi-worker — process-local only; #441 should
  add a guard once the backend lands.

Murzik review followups (from #438) — addressed
================================================
* [x] IP-keyed buckets use #442 resolver, NOT slowapi.get_remote_address
* [-] Login keys on (IP, email) — IP-only this PR; deferred to #435
* [x] Lockout window implemented (penalty_seconds on auth bucket)
* [x] Process-local backend documented; multi-worker guard deferred to #441
* [-] Webhook by token-prefix — IP-only this PR; deferred to #435
* [-] Admin per-user — IP-only this PR; deferred to #435
* [x] 429s emit audit events to #440

Tests (27 new)
==============
* classify_request: login POST/GET, password POST, admin prefix,
  webhook prefix, default, case-insensitive method.
* InMemoryRateLimiter: under-limit allowed, at-limit rejected, lockout
  persists for penalty seconds, window rolls off for non-lockout
  bucket, keys independent, buckets independent, unknown bucket
  passes, reset clears state, custom buckets.
* DEFAULT_BUCKETS: auth has lockout, admin no lockout, default 300/60.
* Middleware: trusted pass-through, LAN throttles auth, LAN default
  bucket off, web default bucket active, audit emitted on 429,
  canonical-IP keying defeats XFF spoofing.
* End-to-end create_api: app.state has limiter, trusted-mode no
  throttle through TestClient.

503 tests green across the full hardening suite + existing api tests.

Closes part of #433. Refs #438.

🤖 Authored by Barsik
PR-8 of the #433 web-exposed hardening track. Defense-in-depth on top
of SameSite=Strict for cookie-authenticated state-changing routes in
lan/web modes. Stacked on #478#477#476#475#474#473#472.

Pattern
=======
Double-submit cookie + header:

1. Server issues `pinky_csrf` cookie alongside any session cookie that
   doesn't already have one. Value = 32-byte URL-safe random,
   SameSite=Strict, NOT HttpOnly (frontend reads it).
2. Frontend reads cookie, sends as `X-Pinky-CSRF` header on every
   state-changing request.
3. Middleware on unsafe verbs (POST/PUT/PATCH/DELETE) compares header
   vs cookie via secrets.compare_digest. Mismatch → 403
   {"error":"csrf_check_failed"}.

Why not just SameSite=Strict
============================
* Older browsers without enforcement
* Multi-window cross-tab leakage
* Subdomain-trust attacks if PinkyBot ever shares a parent domain
* OAuth/redirect flows that intentionally use SameSite=Lax

#439's `/auth/elevate` and other admin POSTs are exactly the
high-value targets CSRF protects.

Mode behaviour
==============
* trusted — disabled (back-compat)
* lan + web — enforced

In LAN the cookie is set with Secure=False (localhost dev doesn't have
TLS); in WEB it's Secure=True.

Exemptions
==========
* Bearer-token auth (Authorization: Bearer ...) — no ambient
  credential vulnerability
* Internal MCP-signed requests (X-Pinky-Agent + X-Pinky-Signature)
* `/triggers/webhook/*` (token-gated upstream)
* `/twilio/*` (Twilio signature gated)
* Anonymous requests (no session cookie → nothing to forge)

Token rotation
==============
Session-bound. Whenever a request carries a session cookie but no CSRF
cookie, the response sets a fresh CSRF token. Login (which sets a new
session) naturally rotates: the next response sees a session without
CSRF and writes a new token. Explicit `/auth/csrf/refresh` is out of
scope for this PR.

Audit
=====
Every CSRF rejection emits `auth.csrf.rejected` into the security
audit log (#440) with ip, via_proxy, forwarded_chain, method, target,
user_agent, and a redacted detail blob {"reason": "missing"|"mismatch"}.
The redaction allowlist was extended in security_audit.py to register
the new event type.

Tests (20 new)
==============
* Pure helpers: UNSAFE_METHODS, generate_csrf_token randomness,
  is_exempt_path.
* Middleware enforcement: trusted pass-through, safe methods
  (GET/HEAD/OPTIONS) pass, unsafe POST without/with mismatch/with
  match, bearer auth exempts, internal-signed exempts, webhook + twilio
  paths exempt, no-session no-enforcement.
* Token issuance: session without CSRF gets one (samesite=strict,
  httponly=false), session with CSRF doesn't reissue, no session no
  token, Secure flag only in web mode.
* Audit: reject emits event with method/target/reason; mismatch
  distinguishable from missing.
* E2E create_api: trusted mode no token issuance + no enforcement.

523 tests green across the full hardening suite + existing api tests.

Closes part of #433. Refs #443.

🤖 Authored by Barsik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant