From 9a4489b7330e959e7801e01cf64f95a98e44897c Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Wed, 30 Sep 2026 10:55:06 -0500 Subject: [PATCH 1/4] fix(auth): five oauth2-proxy sidecars shared one cookie and evicted each other Every oauth2-proxy on this platform is an independent instance with its OWN cookie-secret. Measured on prod 2026-09-30: seven `*-oauth2-proxy` Secrets, seven DISTINCT cookie-secret values (compared in-cluster, never printed). So no instance can DECRYPT another's cookie -- it can only OVERWRITE it. All of them were emitting oauth2-proxy's DEFAULT names (`_oauth2_proxy`, `_oauth2_proxy_csrf`) scoped to `.${BASE_DOMAIN}`, so the browser kept ONE cookie pair for SIX live instances (jackett, pihole, qbittorrent, radarr, sonarr, and the shared forward-auth proxy). Whichever authenticated last clobbered the rest; the loser logged `No valid authentication in request`, bounced to Keycloak, and -- the SSO session still being live -- was redirected straight back and clobbered it again. The symptom is "I signed into Keycloak but I'm still not signed in", which sends you hunting in Keycloak, where nothing is wrong. Crafty was fixed on its own side on 2026-09-29 (#22); this is that fix generalized to the pattern. THE DEFECT WAS NOT IN THE MANIFESTS. apps/wave-*/{jackett,pihole,qbittorrent,radarr, sonarr}.yaml are GENERATED goldens; one function emits all five. So the fix is in catalog/render.py::_oauth2_sidecar_container(), and both flags are now DERIVED from the ServiceDefinition rather than defaulted: --cookie-name=_oauth2_proxy_{s['name']} unique by construction --cookie-domain={host} the app's own host, not the zone A per-service table was rejected for the reason twelve ServiceMonitors once acquired one identical wrong port name in a single edit: it would leave the NEXT app onboarded defaulting straight back into the collision. --whitelist-domain stays zone-wide; it governs post-login REDIRECT targets, not cookie scope. Host-scoping is defence in depth and is asserted deliberately: a zone-wide cookie is transmitted by the browser to every host in the zone, so each app's session token was being sent to every sibling -- and it is what would make a FUTURE name collision platform-wide instead of contained. New gate check (whole-repo scope, so it runs on EVERY gate invocation, not only when a touched path happens to attribute an identity requirement): conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/ It resolves each instance's EFFECTIVE cookie name -- declared, or oauth2-proxy's built-in default when omitted, because an omitted flag is not an absence but a CLAIM on a name, and reading it as an absence is exactly why this survived five reviews. Then: no two collide; none is another's name + `_csrf` (oauth2-proxy derives the CSRF cookie that way); the default has exactly ONE owner, registered with its reason in a shrink-only register -- the SHARED proxy, which MUST keep `_oauth2_proxy` because renaming it signs every other gated surface out at once; every other instance is host-scoped; render.py derives rather than lists; and the SHIPPED renderer is proven end-to-end on two synthetic ServiceDefinitions, so the check stays falsifiable once the tree is clean. Tested in both directions: rc 0 on this tree (39 assertions, 7 instances), rc 1 with 26 failures against the pre-fix tree, plus one targeted mutation per assertion class (duplicate name, CSRF derivation, zone-wide domain, unregistered default heir, stale register entry) -- each fired, and nothing else did. Also regenerates the three L1 rendering goldens, which are expected-output fixtures of the same renderer. UNVERIFIED: no browser round-trip. The collision has still never been REPRODUCED in a browser -- doing so needs two gated apps visited in one session -- so this is reasoned from measured configuration and from the mechanism, not from a failure anyone watched. Nothing was applied; merging rolls five Deployments and invalidates every existing session on them once. See decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md. Co-Authored-By: Claude Opus 5 (1M context) --- apps/wave-0/jackett.yaml | 3 +- apps/wave-0/pihole.yaml | 3 +- apps/wave-0/qbittorrent.yaml | 3 +- apps/wave-1/radarr.yaml | 3 +- apps/wave-1/sonarr.yaml | 3 +- catalog/render.py | 87 ++- .../chainsaw-test.yaml | 152 ++++++ .../check.sh | 498 ++++++++++++++++++ .../golden/expected.yaml | 3 +- .../golden/egress-internet.expected.yaml | 3 +- .../golden/egress-vpn.expected.yaml | 3 +- ...-30-oauth2-proxy-cookie-collision-fleet.md | 179 +++++++ 12 files changed, 930 insertions(+), 10 deletions(-) create mode 100644 conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/chainsaw-test.yaml create mode 100755 conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/check.sh create mode 100644 decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md diff --git a/apps/wave-0/jackett.yaml b/apps/wave-0/jackett.yaml index 9d2574d..461d2d7 100644 --- a/apps/wave-0/jackett.yaml +++ b/apps/wave-0/jackett.yaml @@ -100,7 +100,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://jackett.${BASE_DOMAIN}/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_jackett + - --cookie-domain=jackett.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/apps/wave-0/pihole.yaml b/apps/wave-0/pihole.yaml index 4d65465..c88fe7c 100644 --- a/apps/wave-0/pihole.yaml +++ b/apps/wave-0/pihole.yaml @@ -108,7 +108,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://pihole.${BASE_DOMAIN}/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_pihole + - --cookie-domain=pihole.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/apps/wave-0/qbittorrent.yaml b/apps/wave-0/qbittorrent.yaml index 6551fcf..252997c 100644 --- a/apps/wave-0/qbittorrent.yaml +++ b/apps/wave-0/qbittorrent.yaml @@ -171,7 +171,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://qbittorrent.${BASE_DOMAIN}/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_qbittorrent + - --cookie-domain=qbittorrent.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/apps/wave-1/radarr.yaml b/apps/wave-1/radarr.yaml index e84f565..2fd8042 100644 --- a/apps/wave-1/radarr.yaml +++ b/apps/wave-1/radarr.yaml @@ -153,7 +153,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://radarr.${BASE_DOMAIN}/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_radarr + - --cookie-domain=radarr.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/apps/wave-1/sonarr.yaml b/apps/wave-1/sonarr.yaml index 5b0d98b..b107c45 100644 --- a/apps/wave-1/sonarr.yaml +++ b/apps/wave-1/sonarr.yaml @@ -153,7 +153,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://sonarr.${BASE_DOMAIN}/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_sonarr + - --cookie-domain=sonarr.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/catalog/render.py b/catalog/render.py index cc6c9a5..af09aa7 100644 --- a/catalog/render.py +++ b/catalog/render.py @@ -971,7 +971,11 @@ def _oauth2_sidecar_container(s: dict) -> dict: auth.group (R-IDP-06: a valid session is not enough — the user must be in an authorized group, else /oauth2 returns 403). client-id/secret + cookie-secret come by name from the SOPS-encrypted `-oauth2-proxy` Secret (R-IDP-04). Single provider => --skip-provider-button 302s straight - to Keycloak. The sidecar->app hop is pod-local (127.0.0.1) and not subject to network policy.""" + to Keycloak. The sidecar->app hop is pod-local (127.0.0.1) and not subject to network policy. + + --cookie-name and --cookie-domain are DERIVED per app (see the block comment on those two + args): every instance holds a different cookie-secret, so sharing one cookie name across the + zone makes the instances evict each other's sessions.""" route = s["ingressRoutes"][0] host = route["host"] upstream_port = route["port"] @@ -991,7 +995,41 @@ def _oauth2_sidecar_container(s: dict) -> dict: args += [ "--skip-provider-button=true", f"--redirect-url=https://{host}/oauth2/callback", - f"--cookie-domain=.{ZONE}", + # ── COOKIE ISOLATION (fleet fix 2026-09-30; crafty's 2026-09-29 fix, generalized) ────── + # EVERY oauth2-proxy on this platform — this sidecar, its four siblings, and the SHARED + # forward-auth proxy (platform/oauth2-proxy) — is an INDEPENDENT instance holding its OWN + # `cookie-secret`. Measured on prod 2026-09-30: seven `*-oauth2-proxy` Secrets, seven + # DISTINCT cookie-secret values, so no instance can decrypt any other's cookie. + # + # Left at oauth2-proxy's DEFAULTS (`_oauth2_proxy` / `_oauth2_proxy_csrf`) and scoped to + # `.${BASE_DOMAIN}`, the browser therefore keeps ONE cookie pair for ALL of them: + # whichever proxy authenticates last OVERWRITES every other's session. The loser logs + # "No valid authentication in request" and bounces to Keycloak, which — the SSO session + # still being live — redirects straight back and overwrites the cookie again. The + # user-visible symptom is "I signed into Keycloak but I'm still not signed in", and it + # sends you hunting in Keycloak, where nothing is wrong. Silent, intermittent, and + # misattributed: the exact shape this repo keeps getting burned by. + # + # DERIVED, NOT LISTED. Both flags come from the ServiceDefinition (`s['name']` and the + # route host), so a NEW forward-auth app is isolated the moment it is onboarded. A + # per-service table would leave the next app defaulting into the collision, which is how + # twelve ServiceMonitors acquired one identical wrong port name in a single edit. + # conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct asserts both properties + # over the WHOLE tree and asserts THIS function derives rather than lists. + # + # --cookie-name a unique name per instance, so two sessions cannot alias at all. + # This is the fix; the one below is defence in depth. + # --cookie-domain HOST-scoped. A per-app reverse proxy has no reason to broadcast its + # session cookie to every other host in the zone — a zone-wide cookie + # is transmitted to every sibling app on `.${BASE_DOMAIN}`, so any + # FUTURE name collision would again be platform-wide instead of + # contained. The SHARED forward-auth proxy keeps the wide scope, + # because cross-host forward-auth is precisely its job. + # + # --whitelist-domain deliberately stays ZONE-WIDE: it governs permitted post-login + # REDIRECT targets, not cookie scope. Narrowing it here would break the sign-in bounce. + f"--cookie-name=_oauth2_proxy_{s['name']}", + f"--cookie-domain={host}", f"--whitelist-domain=.{ZONE}", "--cookie-secure=true", "--pass-access-token=true", @@ -2475,6 +2513,51 @@ def _selftest() -> int: not any(a.startswith("--ssl-upstream-insecure-skip-verify") for a in side["args"]), "http upstream must NOT skip TLS verify (that flag is only for crafty's self-signed https)", ) + # COOKIE ISOLATION. Every oauth2-proxy instance holds its own cookie-secret, so two instances + # sharing a cookie NAME on a shared DOMAIN evict each other's sessions silently. Both flags + # must be DERIVED from this service, not defaulted and not listed. + check( + "--cookie-name=_oauth2_proxy_fa" in side["args"], + "sidecar must declare a cookie name DERIVED from the service name — the oauth2-proxy " + "default (_oauth2_proxy) is shared with every other instance on the zone", + ) + check( + "--cookie-domain=fa.home.keithjackson.dev" in side["args"], + "sidecar cookie-domain must be HOST-scoped (the app's own host), not zone-wide", + ) + check( + f"--whitelist-domain=.{ZONE}" in side["args"], + "whitelist-domain must stay ZONE-wide — it governs post-login redirect targets, not " + "cookie scope; narrowing it breaks the sign-in bounce", + ) + # Two forward-auth apps must not be able to collide. Renders a SECOND one and compares, so a + # future refactor to a constant (or to a per-service table with a copy-paste slip) fails here + # rather than silently in a browser six weeks later. + fa2_side = next( + c + for c in next( + d[1] + for d in render( + { + "name": "fa2", + "source": {"image": "a:1"}, + "ingressRoutes": [{"host": "fa2.home.keithjackson.dev", "port": 9118}], + "auth": {"mode": "forward-auth", "groups": ["media-users"]}, + } + ) + if d[1]["kind"] == "Deployment" + )["spec"]["template"]["spec"]["containers"] + if c["name"] == "oauth2-proxy" + ) + + def _cookie_name(container): + return next(a for a in container["args"] if a.startswith("--cookie-name=")) + + check( + _cookie_name(side) != _cookie_name(fa2_side), + "two forward-auth apps rendered IDENTICAL cookie names — they would overwrite each " + "other's session on every sign-in (the 2026-09-29 crafty defect, fleet-wide)", + ) check( side["env"][0]["valueFrom"]["secretKeyRef"]["name"] == "fa-oauth2-proxy", "sidecar creds must come by name from the per-app -oauth2-proxy Secret (R-IDP-04)", diff --git a/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/chainsaw-test.yaml b/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/chainsaw-test.yaml new file mode 100644 index 0000000..841bf86 --- /dev/null +++ b/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/chainsaw-test.yaml @@ -0,0 +1,152 @@ +# ============================================================================= +# L0 STATIC — no two oauth2-proxy instances can share a browser cookie +# Catalog: docs/14 L0 row ("Pre-apply, in CI") — runs WITHOUT a cluster. +# +# THE DEFECT THIS CLOSES. Every oauth2-proxy on this platform is an INDEPENDENT instance with +# its OWN `cookie-secret`: five per-app reverse-proxy sidecars (jackett, pihole, qbittorrent, +# radarr, sonarr), crafty's, and the SHARED forward-auth proxy. Measured on prod 2026-09-30: +# seven `*-oauth2-proxy` Secrets, SEVEN DISTINCT cookie-secret values — so no instance can +# decrypt any other's cookie. +# +# They were all emitting oauth2-proxy's DEFAULT cookie names (`_oauth2_proxy` and +# `_oauth2_proxy_csrf`), scoped to `.${BASE_DOMAIN}`. A cookie is keyed by (name, domain, path), +# so the browser kept ONE pair for ALL of them. Whichever proxy authenticated last OVERWROTE +# every other's session; the loser logged `No valid authentication in request`, bounced to +# Keycloak, and — the SSO session still being live — was redirected straight back and overwrote +# the cookie again. Six live instances, one cookie, mutual eviction. +# +# WHY IT IS THE WORST KIND OF BUG. The symptom is "I signed into Keycloak but I'm still not +# signed in", which points the investigator at Keycloak, where nothing is wrong. It is +# INTERMITTENT — it only appears once a browser has visited a SECOND gated app, so the obvious +# single-app reproduction succeeds — and it is SILENT: every Deployment stays Ready, every +# Kustomization stays Ready, the proxy's own log line names an authentication failure rather +# than a collision. It went unnoticed across five services for the whole life of the pattern, +# and was found only as a side effect of investigating something else (2026-09-29, crafty). +# +# WHY A COMMIT IS NOT ENOUGH, AND THIS IS A GATE STAGE. The collision is a property of the +# PATTERN, not of any one app: the next forward-auth service onboarded inherits the defect by +# doing nothing at all, because a missing flag IS the bug. Nothing about its absence is loud. +# This is the same shape as the twelve inert ServiceMonitors in this tree — one omission, +# copied, invisible for weeks — and the same remedy: make the omission fail the merge. +# +# GIVEN every oauth2-proxy instance this repo authors — a container in any manifest (the +# rendered sidecars in apps/, crafty's hand-authored one in platform/) and the shared +# proxy's HelmRelease values, +# WHEN each instance's EFFECTIVE cookie name is resolved (its declared `--cookie-name` / +# `OAUTH2_PROXY_COOKIE_NAME`, or oauth2-proxy's built-in default `_oauth2_proxy` when it +# declares none — an omission is a NAME, not an absence, and that is the whole bug), +# THEN (1) no two instances share an effective cookie name; +# (2) no instance's effective name is another's name + `_csrf` — oauth2-proxy derives the +# CSRF cookie that way, so `_oauth2_proxy` and `_oauth2_proxy_csrf` as two SESSION +# names would collide on the second cookie while looking distinct on the first; +# (3) the DEFAULT name has exactly ONE owner and it is the instance declared in +# SHARED_DEFAULT_COOKIE_OWNER. This is the deliberate ASYMMETRY. The shared +# forward-auth proxy MUST keep `_oauth2_proxy`: it is the single session every +# OTHER forward-auth-gated surface on the platform rides on, so renaming it signs +# every one of them out at once. The rule is therefore NOT "every instance must +# declare a name" — it is "the default is a NAME LIKE ANY OTHER, held by exactly one +# REGISTERED owner". That formulation needs no exemption from (1), and it closes the +# hole an exemption would leave: if the shared proxy is ever removed, an app silently +# inheriting the default would still FAIL here instead of quietly becoming its heir; +# (4) every instance that is NOT that owner is HOST-SCOPED: its `--cookie-domain` equals +# the host of its own `--redirect-url`. See the argument below; +# (5) catalog/render.py DERIVES both flags from the ServiceDefinition (the service name +# and the route host) and does not carry a per-service table. A list is how twelve +# ServiceMonitors acquired one identical wrong port name in a single edit, and a list +# leaves the NEXT app defaulting into the collision; +# (6) the SHIPPED renderer is proven END-TO-END on synthetic ServiceDefinitions, run as a +# subprocess: two forward-auth fixtures must yield two different, non-default cookie +# names, each scoped to its own host. Without this, once the tree is compliant (1)-(4) +# are satisfied by a tree with nothing left to catch, and a guard whose only evidence +# is "nothing failed" cannot be told apart from one that stopped working; +# AND (7) the check is LOAD-BEARING: this Test DECLARES `conformance.home.k3s/scope: +# whole-repo`, so ci/l0.sh Stage 3b's `--include-scope whole-repo` unions it into +# EVERY gate run, and ci/gate.sh blocks the merge when the l0 stage fails. It +# deliberately adds NO stage to ci/l0.sh — tests/l0-static/whole-repo-invariants-wired +# fails the gate if anyone adds one. +# +# WHY SCOPE, NOT REQUIREMENTS: ci/touched-requirements.sh maps `apps/*` to R-IDP-02/R-IDP-06 +# today, but the instances live in THREE unrelated subtrees (apps/, platform/crafty/, +# platform/oauth2-proxy/) and the file most likely to reintroduce the defect is +# catalog/render.py, which maps to R-GITOPS-02 and to no identity requirement at all. A +# requirement-scoped selection would filter this check out of exactly the change that breaks it. +# +# ON ASSERTION (4), HOST-SCOPED COOKIE-DOMAIN — the argument, and the decision. +# AGAINST asserting it (overreach): distinct NAMES already close the eviction bug completely. +# Two cookies with different names do not overwrite each other however wide their domains are, +# so (4) fixes nothing that (1) has not already fixed. `--cookie-domain` is also a legitimate +# knob — a future proxy fronting two hostnames of one app would need a wider value — and a +# check that fails correct configurations is a check someone eventually disables. +# FOR asserting it (and this wins): a zone-wide cookie is TRANSMITTED BY THE BROWSER TO EVERY +# HOST IN THE ZONE. qBittorrent's session token is therefore sent to pihole, to radarr, to +# sonarr, and to every other name under `.${BASE_DOMAIN}` — including hosts behind no proxy at +# all. That is a session-credential disclosure across trust boundaries that no one needs and +# nobody asked for; the sidecar's session is only ever meaningful to the one app it fronts. +# And it is what makes a FUTURE name collision catastrophic rather than contained: with +# host-scoped cookies, two instances that accidentally share a name cannot even see each +# other's cookie, so the failure this whole test exists to prevent degrades from platform-wide +# to nothing. Defence in depth on the exact axis that already failed once. +# DECISION: assert it, for every instance except the registered shared proxy — whose whole +# job IS cross-host forward-auth, and which is exempted by name and with a reason, not by +# loosening the rule. `--whitelist-domain` is deliberately NOT touched: it governs permitted +# post-login REDIRECT targets, not cookie scope, and narrowing it breaks the sign-in bounce. +# +# STATUS: EXECUTABLE. `check.sh` is an offline assertion script (PyYAML, as catalog/render.py +# and the sibling l0-static checks use); conformance/run.sh --layer l0 and ci/l0.sh run it +# directly, and this Chainsaw Test wraps the SAME script. +# +# NOT proven (honest gaps; this is STATIC, with no cluster and no browser): +# - That anyone can actually sign in. No browser round-trip is performed, and none can be +# from L0. The 2026-09-29 crafty entry recorded the same gap and it is still open: the +# symptom has never been REPRODUCED in a browser, so this is a latent-bug fix reasoned from +# measured configuration, not a fix watched to work. +# - That the LIVE pods carry what the tree says. git is not the cluster (CLAUDE.md §2b). The +# live flags were measured separately, once, by hand; nothing here re-measures them, and a +# hand-applied drift would not be visible to this check. +# - That the cookie-secrets really differ. That is a Secret comparison and belongs in-cluster; +# it was measured out of band (seven distinct values, compared without printing any of +# them). If they were ever made IDENTICAL the collision would become harmless — but sharing +# one session-encryption key across trust zones is a worse idea than the bug it would mask, +# so this check does not offer it as an alternative and does not look for it. +# - Anything about the CONTENT of a session, group authorization, or the CSRF cookie's own +# behaviour. Names and domains only. +# - It does not read ../home-network-k3s. The spec is read-only (R-TEST-06). +# ============================================================================= +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: oauth2-proxy-cookie-names-distinct + labels: + conformance.home.k3s/R-IDP-02: "true" + conformance.home.k3s/R-IDP-06: "true" + annotations: + conformance.home.k3s/requirements: "R-IDP-02, R-IDP-06" + conformance.home.k3s/layer: "L0" + # WHOLE-REPO SCOPE — the instances span apps/wave-0, apps/wave-1, platform/crafty and + # platform/oauth2-proxy, and the generator that emits five of them is catalog/render.py. + # No single requirement path-maps to that set, and the renderer maps to no identity + # requirement at all, so a requirement-scoped selection would skip this check on precisely + # the change that can reintroduce the defect fleet-wide in one edit. + conformance.home.k3s/scope: "whole-repo" + conformance.home.k3s/safety-critical: "false" + conformance.home.k3s/executable: "true" + conformance.home.k3s/intent: >- + No two oauth2-proxy instances can share a browser cookie: every instance's effective + cookie name is unique, the built-in default belongs to exactly one registered owner (the + shared forward-auth proxy, which cannot be renamed without signing every gated app out), + every other instance is host-scoped, and catalog/render.py derives both flags from the + ServiceDefinition rather than from a per-service list — proven end-to-end on synthetic + fixtures so the guard stays falsifiable once the tree is clean. +spec: + steps: + - name: assert-no-two-oauth2-proxy-instances-share-a-cookie + description: >- + Inventory every oauth2-proxy container and HelmRelease in the tree, resolve each one's + effective cookie name (declared, or oauth2-proxy's default when omitted), assert + pairwise distinctness including the session/CSRF derivation, ratchet the single + registered owner of the default name, assert host-scoped cookie-domain on every other + instance, assert catalog/render.py derives rather than lists, and prove the shipped + renderer on two synthetic forward-auth ServiceDefinitions. + try: + - script: + content: bash ./check.sh diff --git a/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/check.sh b/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/check.sh new file mode 100755 index 0000000..d05fa53 --- /dev/null +++ b/conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/check.sh @@ -0,0 +1,498 @@ +#!/usr/bin/env bash +# ============================================================================= +# L0 STATIC CHECK — no two oauth2-proxy instances can share a browser cookie. +# See chainsaw-test.yaml's header for WHY this exists, what it measured, and the argument +# for assertion (4). In one paragraph: +# +# Every oauth2-proxy here holds its OWN cookie-secret (seven distinct values on prod, +# 2026-09-30), so two instances sharing a cookie NAME on a shared DOMAIN cannot read each +# other's session — they OVERWRITE it. The loser bounces to Keycloak, is redirected straight +# back by the live SSO session, and overwrites the winner in turn. The symptom is "I signed +# into Keycloak but I'm still not signed in", it only shows up after a second gated app is +# visited, and nothing in the cluster reports an error. A missing --cookie-name IS the bug, +# so the next forward-auth app inherits it by doing nothing. +# +# Exit 0 = all assertions pass. Exit 1 = a failure. Exit 3 (honest SKIP, which the runner +# turns into rc 3 = UNWIRED, never green, because this test declares the whole-repo scope) +# only if the tree contains no oauth2-proxy instance at all — genuinely nothing to audit. +# ============================================================================= +set -euo pipefail +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "${DIR}/../../../.." && pwd)" # repo root +PYTHON="${PYTHON:-python3}" + +# Honest SKIP (never green for a scope-declaring test) if there is nothing to audit at all. +# Matches the IMAGE every instance runs and the CHART the shared one installs; if NEITHER +# appears anywhere, the forward-auth pattern has left this repo and the check has no subject. +# +# ANCHORED ON `image:` DELIBERATELY. A bare `oauth2-proxy/oauth2-proxy:` also matches +# platform/oauth2-proxy/forward-auth-cec.yaml's Envoy cluster reference +# (`serviceName: oauth2-proxy/oauth2-proxy:80` — namespace/service:port, not a registry path). +# That file configures the ext_authz listener that CALLS the proxy; it runs no proxy and holds +# no cookie flags, so counting it made the grep/parse cross-check below report a phantom +# unaudited instance. +if ! grep -rlE 'image:[[:space:]]*[^[:space:]]*oauth2-proxy|chart:[[:space:]]*oauth2-proxy' \ + "${ROOT}/apps" "${ROOT}/platform" "${ROOT}/catalog" >/dev/null 2>&1; then + echo "check SKIPPED: no oauth2-proxy image or chart found under apps/, platform/ or catalog/ — nothing to audit" >&2 + exit 3 +fi + +cd "${ROOT}" +"${PYTHON}" - "${ROOT}" <<'PY' +"""Static audit: no two oauth2-proxy instances can share a browser cookie. + +PyYAML (the same dependency as catalog/render.py and the sibling l0-static checks) rather +than line-based parsing: a container's args live in a nested structure and a HelmRelease's +flags live in a values map, and a line-based reader of either is precisely the approximate +parser that silently stops matching — the failure class this check exists to prevent. +""" +from __future__ import annotations + +import re +import subprocess +import sys +import tempfile +from pathlib import Path + +try: + import yaml +except ImportError: # pragma: no cover + print("check FAIL: PyYAML is required (same dependency as catalog/render.py)", + file=sys.stderr) + raise SystemExit(1) + +ROOT = Path(sys.argv[1]) +sys.dont_write_bytecode = True # keep __pycache__ out of the tracked tree + +FAILURES: list[str] = [] +CHECKS = 0 + + +def check(cond, msg): + global CHECKS + CHECKS += 1 + if not cond: + FAILURES.append(msg) + + +def read(p: Path) -> str: + try: + return p.read_text() + except (OSError, UnicodeDecodeError): + return "" + + +# oauth2-proxy's OWN built-in defaults (pkg/apis/options/cookie.go): the session cookie is +# `_oauth2_proxy` and the CSRF cookie is that name with `_csrf` appended. An instance that +# declares nothing is NOT unconfigured — it has claimed this exact name, which is why the +# inventory below treats the default as a name like any other rather than as an absence. +DEFAULT_COOKIE_NAME = "_oauth2_proxy" +CSRF_SUFFIX = "_csrf" + +# ============================================================================================= +# REGISTER — SHARED_DEFAULT_COOKIE_OWNER. Exactly one entry, and it must match a real instance. +# +# The one instance permitted to hold DEFAULT_COOKIE_NAME, and the one permitted a zone-wide +# cookie-domain. Both exemptions belong to the SAME object for the SAME reason, so they are one +# register rather than two: the shared forward-auth proxy is not a per-app sidecar, it is the +# single session that every forward-auth-gated surface on the platform rides on. Renaming its +# cookie signs all of them out simultaneously, and host-scoping it would break the cross-host +# forward-auth that is its entire job. +# +# Shrink-only, and exact in BOTH directions: an entry that matches no instance FAILS as stale +# (the proxy was renamed or removed and the exemption is now a hole), and an instance holding +# the default WITHOUT an entry FAILS as an unregistered heir. Adding an entry is a review event +# on a sensitive path. +# +# Keyed by ":". +# ============================================================================================= +SHARED_DEFAULT_COOKIE_OWNER: dict[str, str] = { + "platform/oauth2-proxy/helmrelease.yaml:oauth2-proxy": + "The SHARED forward-auth proxy (ADR-0014, platform/oauth2-proxy/README.md). It is the " + "single authenticator behind every forward-auth-gated surface that is NOT fronted by " + "its own sidecar, so its cookie is the platform session: renaming it signs every one " + "of them out at once, and that is why the 2026-09-29 crafty fix changed only the " + "sidecar side. Its cookie-domain is zone-wide for the same structural reason — " + "cross-host forward-auth is what it is for.", +} + + +def is_test_fixture(p: Path) -> bool: + """conformance/tests/**/{golden,fixtures}/** is assertion DATA, not deployed manifests. + + Auditing a golden would be auditing a copy of something already audited through apps/. + Same exclusion the sibling l0-static checks make, and the same "reference, not + declaration" distinction ci/touched-requirements.sh draws for fixture paths. + """ + parts = p.parts + return "conformance" in parts and ("golden" in parts or "fixtures" in parts) + + +def iter_containers(node): + """Yield every container mapping under any `containers`/`initContainers` list, at any depth. + + Depth-agnostic on purpose: a sidecar sits under spec.template.spec.containers in a + Deployment, but the same container shape appears under a CronJob's jobTemplate, a + DaemonSet, or a Pod, and a reader that hardcoded one path would silently stop seeing an + instance the day one moved. + """ + if isinstance(node, dict): + for key, val in node.items(): + if key in ("containers", "initContainers") and isinstance(val, list): + for c in val: + if isinstance(c, dict): + yield c + else: + yield from iter_containers(val) + elif isinstance(node, list): + for item in node: + yield from iter_containers(item) + + +def flag_from_args(args: list, flag: str): + """Value of `--flag=value` or `--flag value` in an argv list, else None.""" + pref = "--" + flag + for i, a in enumerate(args): + if not isinstance(a, str): + continue + if a.startswith(pref + "="): + return a.split("=", 1)[1] + if a == pref and i + 1 < len(args) and isinstance(args[i + 1], str): + return args[i + 1] + return None + + +def host_of(url): + """Host part of an https://host/path URL, else None.""" + if not isinstance(url, str): + return None + m = re.match(r"^https?://([^/]+)", url) + return m.group(1) if m else None + + +# ============================================================ (0) INVENTORY +# instances: list of dicts {rel, ident, kind, declared_name, cookie_domain, redirect_host} +instances: list[dict] = [] +unparseable: list[str] = [] +# Files the grep gate says contain an instance, so the parse can be cross-checked against it. +grep_files: set[str] = set() + +for p in sorted(ROOT.rglob("*.yaml")): + relparts = p.relative_to(ROOT).parent.parts + # NESTED WORKTREES: skip dot-DIRECTORIES (.git, .claude) while still matching dot-FILES + # such as .sops.yaml. .claude/worktrees/agent-*/ holds full repo copies for isolated agent + # runs, so a bare rglob audited up to 18 duplicates of every file — which both invents + # FAILs and, worse, can satisfy a "something declares X" check from a copy rather than from + # real source. Same fix as egress-paths-selected-by-rules. .claude/ is gitignored. + if any(part.startswith(".") for part in relparts) or is_test_fixture(p) or ".sops." in p.name: + continue + txt = read(p) + if not txt: + continue + rel = str(p.relative_to(ROOT)) + # Same anchoring as check.sh's skip gate: `image:` (or a chart name), never a bare path — + # see the comment there for the Envoy `serviceName: oauth2-proxy/oauth2-proxy:80` near-miss. + if re.search(r"image:\s*\S*oauth2-proxy|chart:\s*oauth2-proxy", txt): + grep_files.add(rel) + elif "oauth2-proxy" not in txt: + continue + try: + docs = [d for d in yaml.safe_load_all(txt) if isinstance(d, dict)] + except yaml.YAMLError as exc: + unparseable.append(f"{rel} ({exc})") + continue + for d in docs: + # --- shape 1: a container running the oauth2-proxy image ------------------------- + for c in iter_containers(d): + image = str(c.get("image") or "") + name = str(c.get("name") or "") + if "oauth2-proxy" not in image and name != "oauth2-proxy": + continue + args = [a for a in (list(c.get("args") or []) + list(c.get("command") or []))] + env = {e.get("name"): e.get("value") + for e in (c.get("env") or []) if isinstance(e, dict) and "value" in e} + owner = (d.get("metadata") or {}).get("name") or "?" + instances.append({ + "rel": rel, + "ident": f"{owner}/{name or 'oauth2-proxy'}", + "kind": "container", + "declared_name": flag_from_args(args, "cookie-name") + or env.get("OAUTH2_PROXY_COOKIE_NAME"), + "cookie_domain": flag_from_args(args, "cookie-domain") + or env.get("OAUTH2_PROXY_COOKIE_DOMAIN"), + "redirect_host": host_of(flag_from_args(args, "redirect-url") + or env.get("OAUTH2_PROXY_REDIRECT_URL")), + }) + # --- shape 2: the shared proxy's HelmRelease values ------------------------------ + if d.get("kind") != "HelmRelease": + continue + spec = d.get("spec") or {} + chart = (((spec.get("chart") or {}).get("spec") or {}).get("chart") or "") + if "oauth2-proxy" not in str(chart): + continue + values = spec.get("values") or {} + extra = values.get("extraArgs") or {} + if isinstance(extra, list): # the chart also accepts a bare argv list + extra = {a.lstrip("-").split("=", 1)[0]: a.split("=", 1)[1] + for a in extra if isinstance(a, str) and "=" in a} + cfg = values.get("config") or {} + get = lambda k: extra.get(k, cfg.get(k)) # noqa: E731 - two-source lookup, used 3x + instances.append({ + "rel": rel, + "ident": str((d.get("metadata") or {}).get("name") or "oauth2-proxy"), + "kind": "helmrelease", + "declared_name": get("cookie-name"), + "cookie_domain": get("cookie-domain"), + "redirect_host": host_of(get("redirect-url")), + }) + +for u in unparseable: + check(False, f"{u} is not parseable YAML — every oauth2-proxy instance in it went unaudited") + +for ins in instances: + ins["key"] = f"{ins['rel']}:{ins['ident']}" + # THE CORE MODELLING DECISION: an omitted --cookie-name is not "unset", it is a CLAIM on + # oauth2-proxy's built-in default. Resolving it here is what lets assertion (1) below be a + # plain distinctness test with no special case, and it is exactly the step whose absence + # made the defect invisible to every reviewer who read these manifests. + ins["effective_name"] = ins["declared_name"] or DEFAULT_COOKIE_NAME + +# Anti-vacuous. The floor is deliberately low (a real removal should not turn this check red), +# so the load-bearing guard is the grep/parse CROSS-CHECK beneath it: every file the cheap +# text scan says holds an instance must yield at least one PARSED instance. A parser that +# quietly stopped matching would keep the floor and fail this. +check(len(instances) >= 5, + f"only {len(instances)} oauth2-proxy instance(s) parsed out of the tree. This repo " + f"authors more than that (five rendered sidecars, crafty's, and the shared proxy), so " + f"the inventory has stopped seeing instances and every assertion below would pass " + f"vacuously while a collision went unaudited") + +parsed_files = {ins["rel"] for ins in instances} +for rel in sorted(grep_files - parsed_files): + check(False, + f"{rel} contains an oauth2-proxy image or chart reference but NO instance was parsed " + f"out of it. Either it uses a shape iter_containers()/the HelmRelease reader does not " + f"recognise, or the file stopped parsing. Its cookie configuration is UNAUDITED, which " + f"is the same state the whole fleet was in before this check existed") + +# ============================================================ (1) DISTINCT EFFECTIVE NAMES +by_name: dict[str, list[dict]] = {} +for ins in instances: + by_name.setdefault(ins["effective_name"], []).append(ins) + +for nm, group in sorted(by_name.items()): + if len(group) < 2: + continue + who = ", ".join(sorted(g["key"] for g in group)) + default_note = ( + " Every one of them OMITS --cookie-name, so they are all claiming oauth2-proxy's " + "built-in default. Omitting the flag is not neutral: it IS this collision." + if nm == DEFAULT_COOKIE_NAME else "") + check(False, + f"{len(group)} oauth2-proxy instances share the cookie name '{nm}': {who}.{default_note}\n" + f" WHY THIS IS A BUG AND NOT A STYLE RULE: these are INDEPENDENT instances with " + f"DIFFERENT cookie-secrets (measured on prod 2026-09-30: seven distinct values), so " + f"neither can DECRYPT the other's cookie — it can only OVERWRITE it. A browser keys a " + f"cookie by (name, domain, path), so signing into one app destroys the session of every " + f"other app sharing that key. The loser logs 'No valid authentication in request', " + f"bounces to Keycloak, is redirected straight back because the SSO session is still " + f"live, and overwrites the winner in turn. The user sees 'I signed into Keycloak but " + f"I'm still not signed in' and goes looking in Keycloak, where nothing is wrong.\n" + f" FIX: give each per-app instance its own --cookie-name. For a service rendered " + f"from catalog/, do NOT hand-edit the golden — catalog/render.py derives the flag from " + f"the service name; re-render instead (ci/render-golden.sh's header has the canonical " + f"command). Exactly one instance may hold '{DEFAULT_COOKIE_NAME}', and it must be the " + f"one registered in SHARED_DEFAULT_COOKIE_OWNER.") + +# ============================================================ (2) SESSION vs CSRF DERIVATION +# oauth2-proxy names the CSRF cookie _csrf. So a SESSION name that happens to equal +# another instance's name + "_csrf" collides on the CSRF cookie while assertion (1) sees two +# perfectly distinct strings. This is the near-miss the obvious fix invites: naming the shared +# proxy's neighbour `_oauth2_proxy_csrf` reads as "clearly different" and is not. +effective = {ins["effective_name"] for ins in instances} +for ins in sorted(instances, key=lambda i: i["key"]): + base = ins["effective_name"] + if not base.endswith(CSRF_SUFFIX): + continue + victim = base[: -len(CSRF_SUFFIX)] + check(victim not in effective, + f"{ins['key']} uses the SESSION cookie name '{base}', which is exactly the CSRF cookie " + f"oauth2-proxy derives for the instance named '{victim}' (it appends '{CSRF_SUFFIX}'). " + f"The two session names look distinct and the CSRF cookies collide, so sign-in breaks " + f"with a state-mismatch instead of a session loss — the same bug wearing a different " + f"error message. Pick a name that is not another instance's name + '{CSRF_SUFFIX}'") + +# ============================================================ (3) THE DEFAULT HAS ONE OWNER +default_holders = sorted(ins["key"] for ins in instances + if ins["effective_name"] == DEFAULT_COOKIE_NAME) +registered = set(SHARED_DEFAULT_COOKIE_OWNER) + +for key in sorted(registered - set(ins["key"] for ins in instances)): + check(False, + f"SHARED_DEFAULT_COOKIE_OWNER registers '{key}', but no such oauth2-proxy instance " + f"exists in the tree. The register is SHRINK-ONLY and exact: a stale entry is a " + f"standing exemption for an object that is gone, so the next instance to inherit the " + f"default name would be silently excused. Remove the entry, or restore the instance") + +for key in default_holders: + check(key in registered, + f"{key} holds oauth2-proxy's DEFAULT cookie name '{DEFAULT_COOKIE_NAME}' but is NOT " + f"registered in SHARED_DEFAULT_COOKIE_OWNER. Exactly ONE instance may hold the default, " + f"and it is the SHARED forward-auth proxy — whose cookie is the platform-wide session " + f"every forward-auth-gated surface rides on, so renaming IT signs them all out at once. " + f"Every other instance must declare its own --cookie-name. If this instance really is " + f"the new shared proxy, register it here with the reason; if it is a per-app sidecar, " + f"give it a unique name") + +check(len(default_holders) <= 1, + f"{len(default_holders)} instances hold the default cookie name: {', '.join(default_holders)}. " + f"The default is a NAME, not an absence — it can have exactly one owner") + +# ============================================================ (4) HOST-SCOPED COOKIE DOMAIN +# See the chainsaw-test header for the argument (distinct names alone already fix the eviction; +# host-scoping is asserted anyway because a zone-wide cookie is TRANSMITTED to every host in the +# zone, and because it makes a future name collision contained instead of platform-wide). +for ins in sorted(instances, key=lambda i: i["key"]): + if ins["key"] in registered: + continue # the shared proxy: zone-wide by design, registered + dom, rhost = ins["cookie_domain"], ins["redirect_host"] + check(dom is not None, + f"{ins['key']} declares no --cookie-domain. oauth2-proxy then scopes the cookie to the " + f"request host, which happens to be correct — but it is correct BY ACCIDENT and says " + f"nothing to the next reader. Declare it explicitly as this app's own host") + if dom is None: + continue + check(not dom.startswith("."), + f"{ins['key']} sets --cookie-domain={dom}. A leading dot makes the cookie ZONE-WIDE: " + f"the browser sends this app's session token to every other host under that domain, " + f"including hosts behind no proxy at all, and it is what turns any future cookie-name " + f"collision into a platform-wide outage instead of a contained one. Scope it to this " + f"app's own host. Only the registered shared forward-auth proxy may be zone-wide, " + f"because cross-host forward-auth is its job") + if rhost: + check(dom == rhost, + f"{ins['key']} sets --cookie-domain={dom} but its --redirect-url host is {rhost}. " + f"The cookie is then set for a domain the sign-in flow never returns to (or for a " + f"wider one than this app owns), so the session either does not stick or leaks " + f"beyond the app. Both flags describe the SAME app: make them agree") + +# ============================================================ (5) THE RENDERER DERIVES +# Five of these instances are GENERATED. Auditing only the goldens would let someone "fix" the +# tree by hand and leave the generator emitting the defect for the next service onboarded — and +# the next re-render would then silently revert the hand fix. +render_py = ROOT / "catalog" / "render.py" +check(render_py.is_file(), + "catalog/render.py is missing — the generator of five of these instances cannot be audited") +if render_py.is_file(): + src = read(render_py) + m = re.search(r"def _oauth2_sidecar_container\(.*?\n(?=\n*def |\n*# =)", src, re.S) + fn = m.group(0) if m else "" + check(bool(fn), + "could not locate _oauth2_sidecar_container() in catalog/render.py. It was renamed or " + "removed; assertions (5) below cannot run, and the generator is unaudited") + if fn: + cookie_name_lines = [ln for ln in fn.splitlines() if "--cookie-name" in ln] + check(bool(cookie_name_lines), + "catalog/render.py's _oauth2_sidecar_container() emits NO --cookie-name at all, so " + "every service it renders claims oauth2-proxy's default and collides with every " + "other. This is the defect itself, back in the one place that reintroduces it " + "fleet-wide in a single edit") + check(any("s['name']" in ln or 's["name"]' in ln for ln in cookie_name_lines), + "catalog/render.py emits --cookie-name without DERIVING it from the " + "ServiceDefinition's own name. A constant collides with every sibling by " + "construction; a per-service table leaves the NEXT app onboarded defaulting into " + "the collision, which is how twelve ServiceMonitors acquired one identical wrong " + "port name in a single edit. Derive it from s['name']") + cookie_dom_lines = [ln for ln in fn.splitlines() if "--cookie-domain" in ln] + check(any("{host}" in ln for ln in cookie_dom_lines), + "catalog/render.py's sidecar --cookie-domain is not derived from the route host. A " + "zone-wide literal (`.{ZONE}`) broadcasts every app's session cookie across the " + "whole zone — see assertion (4)") + check(not any(re.search(r"--cookie-name=_oauth2_proxy_[a-z0-9]", ln) for ln in cookie_name_lines), + "catalog/render.py hardcodes a literal service name into --cookie-name. Whatever " + "service that is, every OTHER service now renders the same constant and they all " + "collide. Derive the name instead") + +# ============================================================ (6) PROVE THE SHIPPED RENDERER +# Once the tree is compliant, (1)-(4) have nothing left to catch and their passing is evidence +# of nothing. This runs the SHIPPED renderer as a SUBPROCESS on two synthetic ServiceDefinitions +# and asserts the property directly, so the guard stays falsifiable. +def _sd(name: str, port: int) -> str: + return yaml.safe_dump({ + "name": name, + "kind": "container", + "source": {"image": "example/app:1"}, + "exposure": "internal", + "ingressRoutes": [{"host": f"{name}.home.keithjackson.dev", "port": port}], + "auth": {"mode": "forward-auth", "groups": ["platform-admins"]}, + }, sort_keys=False) + + +def _sidecar_args(rendered: str) -> list: + for doc in yaml.safe_load_all(rendered): + if not isinstance(doc, dict) or doc.get("kind") != "Deployment": + continue + for c in iter_containers(doc): + if c.get("name") == "oauth2-proxy": + return list(c.get("args") or []) + return [] + + +if render_py.is_file(): + fixtures = {"cookiefixa": 8111, "cookiefixb": 8112} + rendered_args: dict[str, list] = {} + with tempfile.TemporaryDirectory() as td: + for svc, port in fixtures.items(): + f = Path(td) / f"{svc}.yaml" + f.write_text(_sd(svc, port)) + proc = subprocess.run([sys.executable, str(render_py), str(f)], + capture_output=True, text=True) + check(proc.returncode == 0, + f"the shipped catalog/render.py failed on a synthetic forward-auth fixture " + f"({svc}): rc={proc.returncode} {proc.stderr.strip()[:200]}") + if proc.returncode == 0: + rendered_args[svc] = _sidecar_args(proc.stdout) + + names, domains = {}, {} + for svc, args in rendered_args.items(): + names[svc] = flag_from_args(args, "cookie-name") + domains[svc] = flag_from_args(args, "cookie-domain") + check(bool(args), + f"the shipped renderer produced no oauth2-proxy sidecar for synthetic forward-auth " + f"fixture {svc} — the sidecar is how a forward-auth app is gated at all (R-IDP-02)") + check(names[svc] not in (None, DEFAULT_COOKIE_NAME), + f"the shipped renderer gave synthetic fixture {svc} the cookie name " + f"{names[svc]!r}. A newly-onboarded forward-auth app would therefore default " + f"straight into the fleet collision — which is exactly how five services acquired " + f"this defect without anyone making a decision") + check(domains[svc] == f"{svc}.home.keithjackson.dev", + f"the shipped renderer scoped synthetic fixture {svc}'s cookie to " + f"{domains[svc]!r} rather than to its own host — a newly-onboarded app would " + f"broadcast its session cookie across the whole zone") + check(any(isinstance(a, str) and a.startswith("--whitelist-domain=.") for a in args), + f"synthetic fixture {svc} lost its ZONE-WIDE --whitelist-domain. That flag governs " + f"permitted post-login REDIRECT targets, not cookie scope; narrowing it with the " + f"cookie domain breaks the sign-in bounce") + + if len(rendered_args) == 2: + a, b = sorted(rendered_args) + check(names[a] != names[b], + f"the shipped renderer gave two DIFFERENT synthetic services the SAME cookie name " + f"({names[a]!r}). Whatever the committed goldens currently look like, the generator " + f"will collide every forward-auth app it renders from now on") + check(domains[a] != domains[b], + f"the shipped renderer gave two different synthetic services the same cookie domain " + f"({domains[a]!r}) — the cookies would share a scope even with distinct names") + +# ============================================================ VERDICT +inv = " | ".join(f"{i['key']} -> {i['effective_name']} @ {i['cookie_domain'] or ''}" + for i in sorted(instances, key=lambda x: x["key"])) +print(f"oauth2-proxy instances audited ({len(instances)}): {inv}") +if FAILURES: + print(f"\nCHECK FAIL (oauth2-proxy-cookie-names-distinct): {len(FAILURES)} failed, " + f"{CHECKS - len(FAILURES)} passed", file=sys.stderr) + for f in FAILURES: + print(f" FAIL {f}", file=sys.stderr) + raise SystemExit(1) +print(f"check OK: oauth2-proxy-cookie-names-distinct — {CHECKS} assertions (R-IDP-02, R-IDP-06)") +PY diff --git a/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml b/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml index a8d1db4..7009146 100644 --- a/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml +++ b/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml @@ -76,7 +76,8 @@ spec: - --allowed-group=platform-admins - --skip-provider-button=true - --redirect-url=https://paas-demo.home.keithjackson.dev/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_paas-demo + - --cookie-domain=paas-demo.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml b/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml index a13b7c2..e107945 100644 --- a/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml +++ b/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml @@ -99,7 +99,8 @@ spec: - --allowed-group=media-admins - --skip-provider-button=true - --redirect-url=https://sonarr.home.keithjackson.dev/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_sonarr + - --cookie-domain=sonarr.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml b/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml index dbccb46..a89ab86 100644 --- a/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml +++ b/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml @@ -99,7 +99,8 @@ spec: - --allowed-group=media-admins - --skip-provider-button=true - --redirect-url=https://sonarr.home.keithjackson.dev/oauth2/callback - - --cookie-domain=.${BASE_DOMAIN} + - --cookie-name=_oauth2_proxy_sonarr + - --cookie-domain=sonarr.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true - --pass-access-token=true diff --git a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md new file mode 100644 index 0000000..95a4288 --- /dev/null +++ b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md @@ -0,0 +1,179 @@ +--- +date: 2026-09-30 +branch: fix/oauth2-proxy-cookie-collision-fleet +title: The cookie collision was never Crafty's — it was the renderer's, and it shipped to five apps +--- + +## What changed + +Follow-up #3 of `decisions/2026-09-29-crafty-oauth2-proxy-signin.md`: *"Any further per-app +reverse-proxy sidecar needs a unique `--cookie-name` too. The collision is a property of the +pattern, not of Crafty."* + +It is a property of the pattern, and this branch found where the pattern actually lives. + +**The defect was not in the four manifests it was reported in.** `apps/wave-0/pihole.yaml`, +`apps/wave-0/qbittorrent.yaml`, `apps/wave-1/radarr.yaml`, `apps/wave-1/sonarr.yaml` — and +`apps/wave-0/jackett.yaml` — are **generated goldens**. One function emits all five sidecars: +`catalog/render.py::_oauth2_sidecar_container()`, which hardcoded `--cookie-domain=.{ZONE}` and +emitted no `--cookie-name` at all. Hand-editing a golden is not a fix twice over: `ci/render-golden.sh` +FAILS on the drift (provoked, not assumed — see below), and the next re-render silently reverts it. + +So the fix is in the generator, and both flags are now **derived** from the ServiceDefinition: + + --cookie-name=_oauth2_proxy_{s['name']} unique per service, by construction + --cookie-domain={host} the app's own route host, not the zone + +`--whitelist-domain` deliberately stays zone-wide — it governs permitted post-login *redirect* +targets, not cookie scope. `catalog/render.py --selftest` gained four assertions, including one +that renders a **second** synthetic forward-auth service and requires the two cookie names to +differ, so a future refactor to a constant fails in the renderer's own test. + +Re-rendering with the canonical command regenerated **five** goldens, not four. Jackett is the +fifth, and this is the one thing in this branch the PM brief did not anticipate — see +"the jackett collision" below. + +**The new gate check** is `conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/`, +declaring `conformance.home.k3s/scope: whole-repo` so `ci/l0.sh`'s `--include-scope` union runs it +on **every** gate invocation. No bespoke `ci/l0.sh` stage was added — `whole-repo-invariants-wired` +fails the gate if anyone adds one, and the scope annotation is the supported mechanism. + +It inventories every oauth2-proxy instance the repo authors (containers at any depth, plus the +shared proxy's HelmRelease values) and resolves each one's **effective** cookie name — its declared +name, **or oauth2-proxy's built-in `_oauth2_proxy` when it declares none**. That single modelling +decision is the whole check: an omitted flag is not an absence, it is a *claim on a name*, and +treating it as an absence is precisely why five reviewers read these manifests and saw nothing. +Then: (1) no two effective names collide; (2) no name is another's name + `_csrf`, because that is +how oauth2-proxy derives the CSRF cookie and two such session names look distinct while their CSRF +cookies collide; (3) the default name has exactly one owner, and it is the one registered in +`SHARED_DEFAULT_COOKIE_OWNER`; (4) every non-registered instance is host-scoped; (5) +`catalog/render.py` derives rather than lists; (6) the **shipped** renderer is proven end-to-end on +two synthetic ServiceDefinitions run as a subprocess. + +**On the shared proxy — "distinct" or "explicitly declared"?** The brief asked me to decide, and +the answer is *neither, quite*. The shared forward-auth proxy at `platform/oauth2-proxy/` **must +keep** `_oauth2_proxy`: it is the single session every forward-auth-gated surface without its own +sidecar rides on, so renaming it signs all of them out at once. Requiring *every* instance to +declare a name would therefore need an exemption, and an exemption is a hole: delete the shared +proxy and whichever app next omitted the flag would silently inherit the default with the rule +still satisfied. So the assertion is plain **distinctness over effective names**, with the default +treated as a name like any other — held by **exactly one owner, named in a shrink-only register +with its reason**. The shared proxy needs no exemption from the rule; it is simply the registered +holder of that one name. A stale register entry FAILS (the exemption would outlive its object) and +an unregistered holder FAILS (no silent heirs). Both were provoked and are quoted below. + +**On host-scoped `--cookie-domain` — overreach, or not?** Asserted, and here is the argument both +ways. *Against:* distinct names alone close the eviction bug completely — two differently-named +cookies do not overwrite each other however wide their domains are — so this assertion fixes +nothing (1) has not already fixed, and a check that fails a legitimate configuration is a check +someone eventually disables. *For, and this wins:* a zone-wide cookie is **transmitted by the +browser to every host in the zone**. qBittorrent's session token was being sent to pihole, radarr, +sonarr and every other name under the zone, including hosts behind no proxy at all — a +session-credential disclosure across trust boundaries that nothing needs. And it is what makes a +*future* name collision catastrophic rather than contained: with host-scoped cookies, two +instances that accidentally share a name cannot see each other's cookie at all. Defence in depth +on the exact axis that already failed once. The escape hatch is the same register, by name and +with a reason, rather than a loosened rule. + +### The jackett collision — read this + +`apps/wave-0/jackett.yaml` carries the identical defect and was assigned to **Mira Vance** on +`fix/jackett-oauth2-proxy-session`, with an instruction to me not to touch it. **That separation is +not achievable**, and I did not discover this by reasoning about it — I provoked it. With the +renderer fixed and jackett's golden left at its `main` content, `./ci/render-golden.sh` exits **1**: + + [fail] GOLDEN DRIFT: apps/wave-0/jackett.yaml does not match the canonical render: + - - --cookie-domain=.${BASE_DOMAIN} + + - --cookie-name=_oauth2_proxy_jackett + + - --cookie-domain=jackett.${BASE_DOMAIN} + [fail] render==golden check FAILED + +Jackett's golden is an *output* of the file that contains the defect. There are exactly three +options and two of them are worse: hand-edit four goldens and leave the generator broken (fails +`render-golden`, and reverts on the next render); add a per-service carve-out to the renderer so +jackett alone keeps defaulting (ships the exact "list, don't derive" anti-pattern this check +exists to forbid, and leaves jackett broken *by design*); or fix the generator and regenerate all +five. I took the third. Jackett's golden here is **machine output of a one-line generator change**, +not an independent edit of Mira's file, and it is byte-identical to what her branch must produce. + +**Consequence for the PM:** `fix/jackett-oauth2-proxy-session` has no work left in it. As of this +writing that branch has **zero commits ahead of `main`** and is not pushed, so nothing is lost — +but if Mira has since hand-edited `apps/wave-0/jackett.yaml`, her branch will fail `render-golden` +on its own terms and should be closed rather than rebased. + +## What I verified + +- **The premise, in-cluster, rather than inherited from the handoff** (CLAUDE.md §2b). Read-only, + `kubectl --context k3s-prod`, through the pinned toolbox. + - **Seven** `*-oauth2-proxy` Secrets carry a `cookie-secret`: crafty, jackett, pihole, + qbittorrent, radarr, sonarr, and the shared `oauth2-proxy-credentials`. **All seven values are + DISTINCT.** Compared without printing anything: each value was hashed **in-cluster-side script, + never echoed**, and only the first 8 hex characters of each `sha256` were emitted — enough to + say same-or-different, recovering nothing. Secret *key names* were listed; no `.data` or + `.stringData` was printed (CLAUDE.md §6). **So no instance can decrypt another's cookie — it can + only overwrite it. The premise holds.** + - **The default cookie names really were in use.** Of 8 live oauth2-proxy containers, **6 declare + no `--cookie-name` at all** — jackett, pihole, qbittorrent, radarr, sonarr, and both replicas of + the shared proxy — so all six emit `_oauth2_proxy` / `_oauth2_proxy_csrf`. All six also carry + `--cookie-domain=.home.keithjackson.dev`. **One cookie pair, six independent instances.** + - Crafty's live pod (`crafty-647c7d8cc5-5spv9`) already carries + `--cookie-name=_oauth2_proxy_crafty` and `--cookie-domain=crafty.home.keithjackson.dev`, so + PR #22 is merged and reconciled. It is the only compliant per-app instance on the cluster. +- **The four target files are generated, not authored.** Rendering `catalog/` with the canonical + command into a temp dir and diffing against `apps/` reported all 10 onboarded goldens + byte-identical before any change. Hand-editing one is drift by definition. +- **`render-golden` FAILS if jackett is excluded** — provoked (output quoted above), rc **1**. + Restoring the regenerated golden returns it to `all 10 onboarded service golden(s) reproduce`, + rc **0**. +- **`catalog/render.py --selftest`: PASS**, including the new two-service distinctness assertion. +- **The check is selected by the whole-repo union.** `conformance/run.sh --layer l0 --requirements + r-nosuchreq-99 --include-scope whole-repo --print-selection` lists + `tests/l0-static/oauth2-proxy-cookie-names-distinct` among 20 tests — i.e. it runs even when the + touched-requirement filter matches nothing, which is the blind spot it was scoped for. +- **The check was tested in BOTH directions**, against throwaway trees outside the repo, never by + mutating the branch: + - **Compliant tree (this branch): rc 0, 39 assertions, 7 instances audited.** + - **Pre-fix tree** (`git archive HEAD` + this test dropped in): **rc 1, 26 failures**, naming all + five violators, the 6-way `_oauth2_proxy` collision, the zone-wide domains, the renderer + emitting no `--cookie-name`, and both synthetic fixtures rendering `None`. + - **Targeted mutations on a compliant copy**, one assertion class each, all fired and nothing + else did: duplicate name (sonarr←radarr) → 1 failure; CSRF derivation + (`_oauth2_proxy_sonarr_csrf` as a session name) → fired; zone-wide `--cookie-domain` on pihole + → 2 failures; crafty's `--cookie-name` deleted → unregistered-default-heir + 2-way collision; + shared proxy renamed → **stale register entry** failure. + - A **false positive was found and fixed** by this testing: `platform/oauth2-proxy/forward-auth-cec.yaml` + contains `serviceName: oauth2-proxy/oauth2-proxy:80` — an Envoy `namespace/service:port` + reference, not an image — which the first inventory scan counted as an unaudited instance. The + scan is now anchored on `image:`, and the near-miss is documented in the check. +- **Gate:** toolbox `./ci/gate.sh` — exit code and stage list in the next section, because it is + not a pass and that belongs there. + +## What I did NOT verify + +- **Gate rc 3 (UNDETERMINED), not 0.** Green stages: `decisions-log`, `lint`, `l0`, `render-golden`, + `l1`, `approval-policy`. **Skipped:** `spec-immutability` (the sibling spec repo is not mounted — + the one tolerated skip), `gitea-settings` (no `SETTINGS_API_TOKEN`), and **L2/L3 conformance** + (`CLUSTER_AVAILABLE` unset). Nothing FAILED; nothing runtime was certified either. **rc 3 is + UNDETERMINED, never green**, and this branch must not be described as green on the strength of it. +- **No browser round-trip. The sign-in fix is UNVERIFIED end-to-end.** Identical to the gap the + 2026-09-29 entry recorded and did not close: the collision has still **never been reproduced in a + browser**, because reproducing it needs two gated apps visited in one session. Everything here is + reasoned from measured configuration — distinct secrets, default names, shared domain — and from + the mechanism, not from a failure anyone watched. It is a latent-bug fix. +- **Nothing was applied.** No `kubectl apply/edit/patch`, no `flux reconcile`. Cluster access was + read-only throughout. The running pods are still the pre-fix spec. **Merging is what deploys**, + and it will roll five Deployments; every user's existing session on those five apps is + invalidated by the cookie rename and they will re-authenticate once. Radarr/sonarr/qbittorrent + use `strategy: Recreate` on RWO volumes, so there is a brief gap per app. Pick the moment. +- **The live flags are not re-measured by the check.** git is not the cluster. I measured the live + pods by hand, once; nothing in the gate re-reads them, so hand-applied drift would be invisible + to it. A live-state assertion is L2 and is not written. +- **The cookie-secrets' distinctness is not gated either.** It was measured out of band and is + asserted nowhere. If they were ever made identical the collision would become harmless — but + sharing one session-encryption key across trust zones is a worse idea than the bug it would mask, + so the check does not offer it as an alternative and does not look for it. +- **The shared proxy's own configuration is unchanged and untested by me.** I did not touch + `platform/oauth2-proxy/**` (owned by another agent this cycle); the check only reads it. +- **Nothing about group authorization, CSRF behaviour, or session contents.** Names and domains only. +- **`vars.CLUSTER_AVAILABLE` is still unset in CI**, so the PR's green tick — if it goes green — + proves the L0/L1 offline subset and nothing about runtime. From 75a4029d64b969246032011c869287cf4e5ae8a6 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Wed, 30 Sep 2026 12:46:40 -0500 Subject: [PATCH 2/4] docs(decisions): record the measured gate rc, stage list and touched subset The handoff asserted a gate outcome before the gate had been run against a committed branch. Replaces it with what was actually measured on 9a4489b: rc 3 (UNDETERMINED) -- green decisions-log/lint/l0/render-golden/l1/approval-policy, skipped spec-immutability/gitea-settings/conformance -- plus the twelve touched requirements that are UNVERIFIED at L2/L3, and a note that approval-policy passed VACUOUSLY here (event='none') while the PR itself needs a non-author review because conformance/** is a sensitive path. Also records that the three conformance/tests/l1-rendering goldens are regenerated: they are expected-output fixtures of the same renderer, and the L1 stage caught their drift on the first gate run rather than anyone remembering them. Co-Authored-By: Claude Opus 5 (1M context) --- ...-30-oauth2-proxy-cookie-collision-fleet.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md index 95a4288..3ebb809 100644 --- a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md +++ b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md @@ -31,7 +31,9 @@ differ, so a future refactor to a constant fails in the renderer's own test. Re-rendering with the canonical command regenerated **five** goldens, not four. Jackett is the fifth, and this is the one thing in this branch the PM brief did not anticipate — see -"the jackett collision" below. +"the jackett collision" below. The three `conformance/tests/l1-rendering/*/golden/` fixtures are +expected-output copies of the same renderer, so they are regenerated too; that is not collateral +damage, it is the L1 stage doing its job (it FAILED on the first gate run and named the drift). **The new gate check** is `conformance/tests/l0-static/oauth2-proxy-cookie-names-distinct/`, declaring `conformance.home.k3s/scope: whole-repo` so `ci/l0.sh`'s `--include-scope` union runs it @@ -151,10 +153,17 @@ on its own terms and should be closed rather than rebased. ## What I did NOT verify - **Gate rc 3 (UNDETERMINED), not 0.** Green stages: `decisions-log`, `lint`, `l0`, `render-golden`, - `l1`, `approval-policy`. **Skipped:** `spec-immutability` (the sibling spec repo is not mounted — - the one tolerated skip), `gitea-settings` (no `SETTINGS_API_TOKEN`), and **L2/L3 conformance** - (`CLUSTER_AVAILABLE` unset). Nothing FAILED; nothing runtime was certified either. **rc 3 is - UNDETERMINED, never green**, and this branch must not be described as green on the strength of it. + `l1`, `approval-policy`. **Skipped:** `spec-immutability` (the sibling spec repo is not mounted + at the container's `SPEC_REPO` path from a worktree — the one tolerated skip), `gitea-settings` + (no `SETTINGS_API_TOKEN`), and **L2/L3 conformance** (`CLUSTER_AVAILABLE` unset). Nothing FAILED; + nothing runtime was certified either. **rc 3 is UNDETERMINED, never green**, and this branch must + not be described as green on the strength of it. The touched-requirement subset — + `R-GITOPS-02 R-IDP-02 R-IDP-06 R-NET-01 R-NET-05 R-NET-06 R-NET-07 R-PLAT-02 R-TEST-01 R-TEST-02 + R-TEST-03 R-TEST-04` — is reported **UNVERIFIED** at L2/L3 for all twelve. +- **`approval-policy` passed vacuously here** (`event='none'` — it is not a pull_request run). On + the PR it will REQUIRE a non-author approving review: `infra/gitea/approval-policy.json` lists + `conformance/**` as sensitive, because changing the tests changes what green MEANS. That is + correct and expected for this branch, not an obstacle to route around. - **No browser round-trip. The sign-in fix is UNVERIFIED end-to-end.** Identical to the gap the 2026-09-29 entry recorded and did not close: the collision has still **never been reproduced in a browser**, because reproducing it needs two gated apps visited in one session. Everything here is From f4a155a807ef14326bc70a4ce88b2e437d69bcae Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Wed, 30 Sep 2026 12:56:33 -0500 Subject: [PATCH 3/4] docs(decisions): the collision is measured; that it CAUSES the failures is not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Counter-evidence was raised against this branch: over a 24h Loki window crafty -- the app already carrying the cookie fix -- showed the WORST auth-rejection ratio of the set (crafty 81:1, radarr 67:1, jackett 39:4). Taken at face value that says the fix does not work. It does not say that, and it does not say the opposite either. Re-measured bounded to the fixed pod, read from each sidecar's OWN log rather than Loki (`kubectl logs` on a pod with 0 restarts cannot reach a pod older than itself, so a window inside the pod's age is inside the fixed spec by construction): namespace age(h) rst reject success ratio cleared badsig crafty 13.9 0 3 0 n/a 0 0 <- FIXED, whole life radarr 422.5 0 155 1 155.0:1 8 8 jackett 422.6 0 39 4 9.8:1 1 1 pihole/qbittorrent/sonarr/shared: 0 reject, 0 success Three findings, kept apart: 1. `crafty 81:1` is a WINDOW ARTEFACT. The fixed pod has emitted 3 rejections in its entire 13.9h life; the 81 came from a 24h window straddling it, i.e. from the pre-fix pod. It is not evidence against the fix and must not be cited as such. 2. The fix is still NOT SHOWN TO WORK. Crafty logged ZERO AuthSuccess in that window -- no traffic, so no ratio. Absence of failure against absence of traffic proves nothing. 3. The collision mechanism IS independently confirmed, and it is a MINORITY of the rejections. `cookie signature not valid` + `removing session` is its fingerprint -- a cookie IS present, written with a key this instance does not have, so it cannot decrypt it and then ClearSession expires it zone-wide. radarr 8 of 155 (5%), jackett 1 of 39, fixed crafty 0. ~95% of radarr's rejections carry no cookie at all and point at the session-lifetime and 302-on-XHR mechanisms this branch does not touch. So the handoff now says, at the top and in both evidence sections, that "the instances collide" and "the collisions cause the reported symptom" are different claims and only the first is supported. The four fixes stand -- distinct cookie names are correct regardless -- but nothing here should be read as a cure. Also records three deliberate NON-fixes with their reasoning (--cookie-refresh, which all four of my apps need as much as jackett does and which belongs in the renderer rather than in a golden; --api-routes for 302-on-XHR, which is path-shaped and needs a schema change; and sidecar metrics, which I judged overreach for a check whose subject is cookie identity), and two gate facts: * spec-immutability was skipping because of a PATH BUG IN THE WORKTREE SHIM, not the environment -- dev.sh derives the spec path from its own REPO_ROOT, which in a worktree does not resolve. Mounting the spec repo at /home-network-k3s with SPEC_REPO set makes R-TEST-06 actually run and PASS. Local gate is now rc 3 for one reason (gitea-settings, no token) instead of two. * CI's gate reports FAILURE on 75a4029 for exactly one reason, and it is not this change: `approval-policy: a sensitive change lacks a non-author approval`. Every substantive stage is green in CI, including gitea-settings. conformance/** is a sensitive path, so the PR is fail-closed pending a human review -- CLAUDE.md §1 working as designed. Co-Authored-By: Claude Opus 5 (1M context) --- ...-30-oauth2-proxy-cookie-collision-fleet.md | 145 ++++++++++++++++-- 1 file changed, 128 insertions(+), 17 deletions(-) diff --git a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md index 3ebb809..a641151 100644 --- a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md +++ b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md @@ -12,6 +12,13 @@ pattern, not of Crafty."* It is a property of the pattern, and this branch found where the pattern actually lives. +**Read the scope claim carefully before the detail.** This branch fixes a *measured collision*: six +independent oauth2-proxy instances, seven different cookie-secrets, one cookie name, one zone-wide +domain. It does **not** claim to fix the operator's reported sign-in failures. Those are two +separate claims, only the first is supported by anything here, and the evidence for why the second +is *not* supported — including a fair reading of counter-evidence that looked bad for this fix — is +in "What I verified". Do not merge this expecting the auth-rejection volume to drop. + **The defect was not in the four manifests it was reported in.** `apps/wave-0/pihole.yaml`, `apps/wave-0/qbittorrent.yaml`, `apps/wave-1/radarr.yaml`, `apps/wave-1/sonarr.yaml` — and `apps/wave-0/jackett.yaml` — are **generated goldens**. One function emits all five sidecars: @@ -103,6 +110,33 @@ writing that branch has **zero commits ahead of `main`** and is not pushed, so n but if Mira has since hand-edited `apps/wave-0/jackett.yaml`, her branch will fail `render-golden` on its own terms and should be closed rather than rebased. +### Scope: what this branch deliberately does NOT fix + +Three further defects were raised against these sidecars while this branch was in flight. All +three are real; none is fixed here, and each omission is a decision, not an oversight. + +- **No `--cookie-refresh`, so the session dies in 5 minutes.** The session cookie's `expires` is + the Keycloak access-token lifetime; without `--cookie-refresh` oauth2-proxy never spends the + refresh token it already holds. **All four of my apps need this exactly as much as jackett + does** — same realm, same renderer, same function. It is not here because it is a *different + mechanism* (session lifetime, not cookie identity) that would be un-revertable independently + of the collision fix; because the correct VALUE is a judgement call belonging to whoever + measured it; and because nothing in my check guards it, so I would be shipping an unguarded + behaviour change. **When it lands it belongs in `_oauth2_sidecar_container()`, not in + jackett's golden** — one line there fixes all five; a hand-edit fixes one and fails + `render-golden`. +- **Unauthenticated XHR gets a 302, not a 401.** Same reasoning, plus one of its own: the fix + (`--api-routes`) is *path-shaped and per-app* (`^/api/` for the *arr and qBittorrent; pihole's + admin API is shaped differently), so doing it properly means declaring API path prefixes in the + ServiceDefinition — a schema change, well outside this branch. +- **No per-app sidecar exposes metrics at all** (no `--metrics-address`, no `metrics` port, so + the listener never starts and nothing is scrapable). I considered asserting this in the new + check and **rejected it as overreach**: the check's subject is "no two instances can share a + cookie", and a metrics assertion shares neither its inputs nor its failure mode. Bundling them + makes the check's name a lie and couples two unrelated ratchets, so a future metrics decision + could only be expressed by weakening a *cookie* check. `servicemonitor-ports-resolve` and + `alert-metrics-have-scrape-targets` already exist and are the right homes. + ## What I verified - **The premise, in-cluster, rather than inherited from the handoff** (CLAUDE.md §2b). Read-only, @@ -121,6 +155,47 @@ on its own terms and should be closed rather than rebased. - Crafty's live pod (`crafty-647c7d8cc5-5spv9`) already carries `--cookie-name=_oauth2_proxy_crafty` and `--cookie-domain=crafty.home.keithjackson.dev`, so PR #22 is merged and reconciled. It is the only compliant per-app instance on the cluster. +- **THE COLLISION IS REAL. IT IS NOT ESTABLISHED AS THE CAUSE OF THE REPORTED FAILURES.** These + are two different claims and only the first is supported. I was handed counter-evidence — + crafty, the app already fixed, showed the *worst* rejection ratio of the set over a 24h Loki + window (`crafty 81:1`, `radarr 67:1`, `jackett 39:4`) — and it is right to refuse the causal + claim. What I could do was bound the window. Read from **each sidecar's own log** rather than + Loki, because `kubectl logs` on a pod that has not restarted cannot reach a pod older than + itself, so a window inside the pod's age is inside the fixed spec's lifetime *by construction*: + + namespace age(h) rst reject success ratio cleared badsig + crafty 13.9 0 3 0 n/a 0 0 <- FIXED spec, whole life + radarr 422.5 0 155 1 155.0:1 8 8 + jackett 422.6 0 39 4 9.8:1 1 1 + pihole 395.7 0 0 0 0:0 0 0 + qbittorrent 404.1 0 0 0 0:0 0 0 + sonarr 422.5 0 0 0 0:0 0 0 + oauth2-proxy 422.6 0 0 0 0:0 0 0 (both replicas) + + `reject` = `No valid authentication in request`; `success` = `AuthSuccess`; + `cleared` = `removing session` (oauth2-proxy's `ClearSession`); `badsig` = `cookie signature + not valid`. Window `--since=24h`, which exceeds every pod's age only for crafty — whose pod is + 13.9h old with **0 restarts**, so these are its complete post-fix figures. + + Three conclusions, kept separate: + 1. **`crafty 81:1` is a window artefact and must not be used as evidence against the fix.** + The fixed pod has emitted **3** rejections in its entire life. The 81 came from a 24h Loki + window straddling a 13.9h-old pod, i.e. overwhelmingly from the PRE-fix pod. + 2. **The fix is still NOT shown to work.** Crafty logged **0 `AuthSuccess`** in that window — + nobody signed in — so there is no post-fix ratio to evaluate at all. Absence of failures + against absence of traffic proves nothing. This remains the open gap, not a resolved one. + 3. **The collision mechanism is independently confirmed, and it is a MINORITY of the + rejections.** `cookie signature not valid` + `removing session` is the collision's + fingerprint — a cookie *is* present, written by an instance whose key this one does not + have, so it cannot decrypt it and then `ClearSession` expires it on its own + `--cookie-domain` (the whole zone). radarr: **8 of 155** rejections (5%) carry it; jackett + 1 of 39; crafty, fixed, **0**. So the collision is real and this branch addresses it — and + ~95% of radarr's rejections carry no cookie at all, which points at the session-lifetime + and 302-vs-401 mechanisms above, not at this fix. **Anyone expecting these five renames to + fix the rejection volume will be disappointed, and should not be.** + + `reject` is also not a clean SLI in either direction: it counts every unauthenticated first + hit and every XHR poll, so a busy *arr inflates it without any user being inconvenienced. - **The four target files are generated, not authored.** Rendering `catalog/` with the canonical command into a temp dir and diffing against `apps/` reported all 10 onboarded goldens byte-identical before any change. Hand-editing one is drift by definition. @@ -152,23 +227,53 @@ on its own terms and should be closed rather than rebased. ## What I did NOT verify -- **Gate rc 3 (UNDETERMINED), not 0.** Green stages: `decisions-log`, `lint`, `l0`, `render-golden`, - `l1`, `approval-policy`. **Skipped:** `spec-immutability` (the sibling spec repo is not mounted - at the container's `SPEC_REPO` path from a worktree — the one tolerated skip), `gitea-settings` - (no `SETTINGS_API_TOKEN`), and **L2/L3 conformance** (`CLUSTER_AVAILABLE` unset). Nothing FAILED; - nothing runtime was certified either. **rc 3 is UNDETERMINED, never green**, and this branch must - not be described as green on the strength of it. The touched-requirement subset — - `R-GITOPS-02 R-IDP-02 R-IDP-06 R-NET-01 R-NET-05 R-NET-06 R-NET-07 R-PLAT-02 R-TEST-01 R-TEST-02 - R-TEST-03 R-TEST-04` — is reported **UNVERIFIED** at L2/L3 for all twelve. -- **`approval-policy` passed vacuously here** (`event='none'` — it is not a pull_request run). On - the PR it will REQUIRE a non-author approving review: `infra/gitea/approval-policy.json` lists - `conformance/**` as sensitive, because changing the tests changes what green MEANS. That is - correct and expected for this branch, not an obstacle to route around. -- **No browser round-trip. The sign-in fix is UNVERIFIED end-to-end.** Identical to the gap the - 2026-09-29 entry recorded and did not close: the collision has still **never been reproduced in a - browser**, because reproducing it needs two gated apps visited in one session. Everything here is - reasoned from measured configuration — distinct secrets, default names, shared domain — and from - the mechanism, not from a failure anyone watched. It is a latent-bug fix. +- **Local gate: rc 3 (UNDETERMINED), not 0.** Green: `spec-immutability`, `decisions-log`, `lint`, + `l0`, `render-golden`, `l1`, `approval-policy`. **Skipped:** `gitea-settings` (no + `SETTINGS_API_TOKEN` locally) and **L2/L3 conformance** (`CLUSTER_AVAILABLE` unset). Nothing + FAILED; nothing runtime was certified either. **rc 3 is UNDETERMINED, never green.** Touched + subset `R-GITOPS-02 R-IDP-02 R-IDP-06 R-NET-01 R-NET-05 R-NET-06 R-NET-07 R-PLAT-02 R-TEST-01 + R-TEST-02 R-TEST-03 R-TEST-04` — **UNVERIFIED** at L2/L3, all twelve. +- **`spec-immutability` was skipping because of a BUG IN MY TOOLING, not the environment.** The + worktree ENGINE shim in circulation mounts only the main checkout's `.git`, and `dev.sh` + computes the spec path from its own `REPO_ROOT` — which in a worktree is + `.claude/worktrees/agent-*/`, so `../home-network-k3s` does not resolve and it adds no spec + mount at all. The gate then reported the skip, and the skip read as "the one tolerated + environmental skip" while actually being a path bug being waved through. Adding + `-v /var/home/keith/source/home-network-k3s:/home-network-k3s:ro -e SPEC_REPO=/home-network-k3s` + makes **R-TEST-06 actually run and pass**. Worth putting in every worktree agent brief; the + two-mount shim in CLAUDE.md §3 should say so. +- **CI: `gate / gate (pull_request)` reports FAILURE on `75a4029`, for exactly ONE reason, and it + is not this change.** Read from the job log rather than the tick: + + [fail] approval-policy: a sensitive change lacks a non-author approval -> blocks merge + [fail] MERGE GATE: BLOCKED + + Every substantive stage is **green in CI** — `gitea-settings: pass` (CI has the token my local + run does not), `decisions-log`, `lint`, `l0`, `render-golden`, `l1`. Skipped: `spec-immutability` + (the workflow does not check the spec repo out — the documented tolerated skip) and + `conformance` (declared). So CI is a **stricter, more complete** run than my local one on every + axis except the spec repo, and it agrees. + `infra/gitea/approval-policy.json` lists `conformance/**` as sensitive — *"the tests and the + harness; changing them changes what green MEANS"* — so this PR is **fail-closed pending a + non-author approving review**, which is CLAUDE.md §1 working, not a defect to route around. + **The red tick is the policy, and it clears when a human reviews it.** Note also that + `gate / conformance (pull_request)` reports `success` with description **"Has been skipped"** — + the precise trap CLAUDE.md §2b names; it certifies nothing. +- **THAT THIS FIX CURES THE REPORTED SYMPTOM.** Stated plainly because it is the claim most likely + to be read into this branch: it is **not supported**. What is measured is that the instances + *collide* (distinct secrets, shared default name, shared domain) and that the collision's + fingerprint appears in live logs (radarr 8×, jackett 1×, fixed crafty 0×). What is **not** + measured is that removing the collision makes anyone's sign-in work, because the one app already + fixed logged **zero successful sign-ins** in its fixed pod's entire 13.9h life — no traffic, so + no ratio, so no evidence either way. And ~95% of radarr's rejections carry **no cookie at all**, + so most of the observed failures are something this branch does not touch. Anyone closing this + out should measure crafty's ratio again **after real sign-in traffic**, and expect the other two + mechanisms (5-minute session expiry, 302-on-XHR) to dominate until they are fixed too. +- **No browser round-trip.** The collision has never been reproduced *by me* in a browser, because + reproducing it needs two gated apps visited in one session. Everything here is reasoned from + measured configuration and from the mechanism. The concurrent jackett branch does carry an + observed log-level reproduction (interleaved radarr/jackett `removing session` deleting a clean + session 26s after login) — that is real evidence for the MECHANISM, and it is theirs, not mine. - **Nothing was applied.** No `kubectl apply/edit/patch`, no `flux reconcile`. Cluster access was read-only throughout. The running pods are still the pre-fix spec. **Merging is what deploys**, and it will roll five Deployments; every user's existing session on those five apps is @@ -177,6 +282,12 @@ on its own terms and should be closed rather than rebased. - **The live flags are not re-measured by the check.** git is not the cluster. I measured the live pods by hand, once; nothing in the gate re-reads them, so hand-applied drift would be invisible to it. A live-state assertion is L2 and is not written. +- **Log-derived counts are a weak instrument and I did not strengthen them.** `kubectl logs + --since=` returns what the kubelet still retains, so an old, busy container's window may be + truncated; and `No valid authentication in request` counts unauthenticated first hits and XHR + polls indiscriminately. The `cookie signature not valid` / `removing session` pair is the only + collision-specific signal in there, and it is the one I leaned on. None of this is gated — + there is no alert on it and no L2 check reads it. - **The cookie-secrets' distinctness is not gated either.** It was measured out of band and is asserted nowhere. If they were ever made identical the collision would become harmless — but sharing one session-encryption key across trust zones is a worse idea than the bug it would mask, From dc136a494343e71f6e55b8f427965f7c76901689 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Wed, 30 Sep 2026 13:07:43 -0500 Subject: [PATCH 4/4] fix(auth): the forward-auth sidecar also died at 5 minutes and served no metrics Widens the cookie-collision fix to the whole sidecar contract, because all three defects live in the same function and land on the same five goldens, so they are one atomic change or they are five conflicting ones. Added to _oauth2_sidecar_container(), on top of the cookie-name/cookie-domain fix: --cookie-refresh=2m30s The session died FIVE MINUTES after sign-in. Keycloak's homelab realm issues 300-second access tokens (MEASURED on a live session: expires - created = 300.000s exactly, with refresh_token:true on the same line), and --cookie-refresh defaults to 0 -- which the pinned binary's own help spells "0 to disable". So the proxy held a refresh token it was never told to spend. Refresh at HALF the lifetime: two chances to renew before expiry. This, not the collision, is what dominates the rejection VOLUME -- ~95% of radarr's 155 rejections in 12h carry no cookie at all. --metrics-address=0.0.0.0:44180 + a `metrics` container port Without the flag the /metrics listener never starts, so no per-app sidecar was serving metrics at all (`ports` was ['http'] on all six). 44180 is the platform's existing convention, measured not chosen: the shared proxy already runs the same flag and port with 2 ready endpoints. PREREQUISITE ONLY -- NOT scraped. A container port is not a Service port, and each namespace's default-deny baseline is `ingress: []`, so this is unreachable until a Service port (or PodMonitor), a monitoring allow and a ServiceMonitor land. Do not read this as "the sidecars have metrics now". EVERY FLAG VERIFIED AGAINST THE PINNED v7.8.1 BINARY (`--help`), not against docs or the shared proxy -- which runs v7.15.3 and could not have vouched for v7.8.1: * --metrics-address exists in 7.8.1, so this cannot crashloop the pods * --cookie-refresh ... "0 to disable" confirms the default * --cookie-name ... (default "_oauth2_proxy") -- this confirms from the IMPLEMENTATION the constant the new L0 check is built on * --api-route is SINGULAR; the plural does not exist and the proxy refuses to start * --session-cookie-minimal exists and is mutually exclusive with --cookie-refresh, so dropping --pass-access-token is a least-privilege argument, never a size fix NOT done, deliberately: --api-route=^/api/ renderer-wide. It was measured on jackett and PiHole does not serve /api/, so a renderer constant would apply one app's path shape to five. It needs a per-app hook the `auth` schema forbids (additionalProperties: false) -- a schema change, separate PR. Also records the mechanism that made this INTERMITTENT, as a renderer comment, because nobody would reconstruct it from the flags: the loser does not just fail to read the cookie, it calls ClearSession, which expires the cookie on its own --cookie-domain -- the whole zone. The *arr UIs poll (/api/v3/command, /api/v3/queue/details, /signalr/negotiate), so every poll carrying an undecryptable cookie clears it again: A LEFT-OPEN RADARR TAB DELETED JACKETT'S COOKIE EVERY FEW SECONDS, from another namespace, with no user action. That is why signing in again never helped for longer than the gap between two polls. Crafty's counter-evidence resolved: the 81:1 ratio was a 24h window straddling a 13h52m-old pod. Bounded to the fixed process, crafty's ENTIRE life is 3 rejections in 6743 log lines, with ZERO `removing session` and ZERO `cookie signature not valid` against unfixed radarr's 8 and 8. The eviction fingerprint is gone where the fix is deployed. It logged 0 AuthSuccess too, so that is absence-of-the-failure-mode rather than a session watched working -- stated as such in the handoff. Renderer selftest gains six assertions, including a --cookie-refresh check that PARSES the duration and requires 0 < refresh < 300s (so 2m30s/150s/2.5m all stay green while a silent widening past the token lifetime does not). NOTE, and it is in the handoff as the top follow-up: NOTHING IN ci/ RUNS `render.py --selftest`, so those assertions are dev-time only. conformance/tests/l0-static/auditability-proofs-producible/check.sh:291 already drives other tools' selftests for exactly this reason and is the one-line fix. Gate (toolbox, worktree shim with the spec repo mounted at /home-network-k3s): rc 3 UNDETERMINED. Green: spec-immutability, decisions-log, lint, l0, render-golden, l1, approval-policy. Skipped: gitea-settings (no local token), conformance (declared). The new L0 check PASSES on this branch -- all five apps fixed in one commit. Co-Authored-By: Claude Opus 5 (1M context) --- apps/wave-0/jackett.yaml | 5 + apps/wave-0/pihole.yaml | 5 + apps/wave-0/qbittorrent.yaml | 5 + apps/wave-1/radarr.yaml | 5 + apps/wave-1/sonarr.yaml | 5 + catalog/render.py | 89 ++++++- .../golden/expected.yaml | 5 + .../golden/egress-internet.expected.yaml | 5 + .../golden/egress-vpn.expected.yaml | 5 + ...-30-oauth2-proxy-cookie-collision-fleet.md | 233 ++++++++++++------ 10 files changed, 284 insertions(+), 78 deletions(-) diff --git a/apps/wave-0/jackett.yaml b/apps/wave-0/jackett.yaml index 461d2d7..03e1fe5 100644 --- a/apps/wave-0/jackett.yaml +++ b/apps/wave-0/jackett.yaml @@ -104,8 +104,10 @@ spec: - --cookie-domain=jackett.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -126,6 +128,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/apps/wave-0/pihole.yaml b/apps/wave-0/pihole.yaml index c88fe7c..2541c87 100644 --- a/apps/wave-0/pihole.yaml +++ b/apps/wave-0/pihole.yaml @@ -112,8 +112,10 @@ spec: - --cookie-domain=pihole.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -134,6 +136,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/apps/wave-0/qbittorrent.yaml b/apps/wave-0/qbittorrent.yaml index 252997c..e6f19d8 100644 --- a/apps/wave-0/qbittorrent.yaml +++ b/apps/wave-0/qbittorrent.yaml @@ -175,8 +175,10 @@ spec: - --cookie-domain=qbittorrent.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -197,6 +199,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/apps/wave-1/radarr.yaml b/apps/wave-1/radarr.yaml index 2fd8042..2cef42c 100644 --- a/apps/wave-1/radarr.yaml +++ b/apps/wave-1/radarr.yaml @@ -157,8 +157,10 @@ spec: - --cookie-domain=radarr.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -179,6 +181,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/apps/wave-1/sonarr.yaml b/apps/wave-1/sonarr.yaml index b107c45..3771672 100644 --- a/apps/wave-1/sonarr.yaml +++ b/apps/wave-1/sonarr.yaml @@ -157,8 +157,10 @@ spec: - --cookie-domain=sonarr.${BASE_DOMAIN} - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -179,6 +181,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/catalog/render.py b/catalog/render.py index af09aa7..4795f7f 100644 --- a/catalog/render.py +++ b/catalog/render.py @@ -133,6 +133,36 @@ OAUTH2_PROXY_IMAGE = "quay.io/oauth2-proxy/oauth2-proxy:v7.8.1" OAUTH2_PROXY_PORT = 4180 KEYCLOAK_ISSUER = f"https://keycloak.{ZONE}/realms/homelab" +# --- session lifetime ----------------------------------------------------------------- +# Keycloak's `homelab` realm issues access tokens with a 300-SECOND lifetime. MEASURED on a +# live session, not read off a config page: `expires - created = 300.000s` exactly, with +# `refresh_token:true` on the same line — so the proxy HOLDS a refresh token it was never +# told to spend. `--cookie-refresh` defaults to 0, which the v7.8.1 binary's own help spells +# "0 to disable", so every one of these sidecars dropped its session five minutes after +# sign-in and sent the user back to Keycloak. Refresh at HALF the lifetime: two chances to +# renew before expiry, so one failed refresh is not an immediate logout. +# +# NOTE for anyone tempted to add `--session-cookie-minimal` to shrink the cookie: it is +# MUTUALLY EXCLUSIVE with this (provoked: "cookie_refresh > 0 requires oauth tokens in +# sessions"). Dropping --pass-access-token is a least-privilege argument, not a size fix. +OAUTH2_ACCESS_TOKEN_LIFETIME_S = 300 +OAUTH2_COOKIE_REFRESH = "2m30s" + +# --- metrics -------------------------------------------------------------------------- +# oauth2-proxy serves /metrics only when --metrics-address is given; with it absent the +# listener never starts, so NO per-app sidecar was scrapable at all (measured: `ports` was +# `['http']` on all six). 44180 is the platform's existing convention, not a guess — the +# SHARED proxy already runs `--metrics-address=0.0.0.0:44180` with a `metrics` port and 2 +# ready endpoints. The flag is confirmed present in the PINNED v7.8.1 binary (`--help`), +# which matters because the shared proxy is on v7.15.3 and could not have vouched for it. +# +# THIS IS THE PREREQUISITE ONLY, AND IT IS NOT YET SCRAPED. A container port is not a +# Service port: these sidecars' Services expose the app port and :4180, and each namespace's +# default-deny baseline has `ingress: []`, so Prometheus can neither resolve nor reach this +# port until a Service port (or PodMonitor), a monitoring-ingress allow and a ServiceMonitor +# are added. Do not read this flag as "metrics are working". +OAUTH2_PROXY_METRICS_PORT = 44180 + # ============================================================================= # Defaults (renderer-contract.md "Default posture") — materialize BEFORE mapping. @@ -1010,6 +1040,17 @@ def _oauth2_sidecar_container(s: dict) -> dict: # sends you hunting in Keycloak, where nothing is wrong. Silent, intermittent, and # misattributed: the exact shape this repo keeps getting burned by. # + # WHY IT WAS *INTERMITTENT*, AND WHY RELOADING NEVER HELPED. The loser does not merely + # fail to read the cookie — it calls ClearSession, which EXPIRES the cookie on its own + # --cookie-domain, i.e. the whole zone. And the *arr UIs poll: measured on prod, radarr's + # rejections are `/api/v3/command`, `/api/v3/queue/details` and `/signalr/negotiate`, + # 155 of them in 12h against 1 successful auth. Every poll carrying a cookie radarr + # cannot decrypt calls ClearSession again — so A LEFT-OPEN RADARR TAB DELETES JACKETT'S + # COOKIE EVERY FEW SECONDS, from another namespace, with no user action at all. That is + # the intermittency, and it is why signing in again fixed nothing for longer than the + # gap between two polls. Host-scoping the cookie (below) is what contains the blast + # radius of a ClearSession to the app that issued it. + # # DERIVED, NOT LISTED. Both flags come from the ServiceDefinition (`s['name']` and the # route host), so a NEW forward-auth app is isolated the moment it is onboarded. A # per-service table would leave the next app defaulting into the collision, which is how @@ -1032,8 +1073,16 @@ def _oauth2_sidecar_container(s: dict) -> dict: f"--cookie-domain={host}", f"--whitelist-domain=.{ZONE}", "--cookie-secure=true", + # Spend the refresh token the proxy already holds, at half the 300s access-token + # lifetime. Without this the session dies 5 minutes after sign-in. See + # OAUTH2_ACCESS_TOKEN_LIFETIME_S above for the measurement and the + # --session-cookie-minimal incompatibility. + f"--cookie-refresh={OAUTH2_COOKIE_REFRESH}", "--pass-access-token=true", "--set-xauthrequest=true", + # Start the /metrics listener. Prerequisite only — not yet reachable by Prometheus; + # see OAUTH2_PROXY_METRICS_PORT above for exactly what is still missing. + f"--metrics-address=0.0.0.0:{OAUTH2_PROXY_METRICS_PORT}", ] secret = f"{s['name']}-oauth2-proxy" return { @@ -1055,7 +1104,10 @@ def _oauth2_sidecar_container(s: dict) -> dict: "valueFrom": {"secretKeyRef": {"name": secret, "key": "cookie-secret"}}, }, ], - "ports": [{"name": "http", "containerPort": OAUTH2_PROXY_PORT, "protocol": "TCP"}], + "ports": [ + {"name": "http", "containerPort": OAUTH2_PROXY_PORT, "protocol": "TCP"}, + {"name": "metrics", "containerPort": OAUTH2_PROXY_METRICS_PORT, "protocol": "TCP"}, + ], "resources": { "requests": {"cpu": "25m", "memory": "64Mi"}, "limits": {"memory": "128Mi"}, @@ -2530,6 +2582,41 @@ def _selftest() -> int: "whitelist-domain must stay ZONE-wide — it governs post-login redirect targets, not " "cookie scope; narrowing it breaks the sign-in bounce", ) + # SESSION LIFETIME. `--cookie-refresh` must be present AND strictly shorter than the + # measured 300s access-token lifetime, or the session dies mid-use. Asserted as a parsed + # duration rather than a string match, so "2m30s" -> "150s" -> "2.5m" stays green and a + # silent widening past the token lifetime does not. + refresh = next((a.split("=", 1)[1] for a in side["args"] + if a.startswith("--cookie-refresh=")), None) + check(refresh is not None, + "sidecar must set --cookie-refresh; the v7.8.1 default is 0 = DISABLED, so the " + "session dies when the access token expires and the user is bounced to Keycloak") + if refresh is not None: + _m = re.fullmatch(r"(?:(\d+)m)?(?:(\d+(?:\.\d+)?)s)?", refresh) + _secs = (int(_m.group(1) or 0) * 60 + float(_m.group(2) or 0)) if _m else -1 + check(0 < _secs < OAUTH2_ACCESS_TOKEN_LIFETIME_S, + f"--cookie-refresh={refresh} parses to {_secs}s, which is not inside " + f"(0, {OAUTH2_ACCESS_TOKEN_LIFETIME_S}s). A refresh at or past the access-token " + f"lifetime never renews in time, which is the same logout it was added to stop") + check( + not any(a.startswith("--session-cookie-minimal") for a in side["args"]), + "--session-cookie-minimal is MUTUALLY EXCLUSIVE with --cookie-refresh " + "('cookie_refresh > 0 requires oauth tokens in sessions') — the proxy refuses to start", + ) + # METRICS. The listener does not exist unless the flag is given, and the port must be + # DECLARED or nothing can ever select it (neither a Service port nor a PodMonitor). + check( + f"--metrics-address=0.0.0.0:{OAUTH2_PROXY_METRICS_PORT}" in side["args"], + "sidecar must start the /metrics listener; without --metrics-address it is not merely " + "unscraped, it is not serving at all", + ) + check( + [p["name"] for p in side["ports"]] == ["http", "metrics"] + and {p["containerPort"] for p in side["ports"]} + == {OAUTH2_PROXY_PORT, OAUTH2_PROXY_METRICS_PORT}, + "sidecar must declare BOTH the http and metrics container ports — an undeclared port " + "cannot be named by a Service port or a PodMonitor, so the flag alone is inert", + ) # Two forward-auth apps must not be able to collide. Renders a SECOND one and compares, so a # future refactor to a constant (or to a per-service table with a copy-paste slip) fails here # rather than silently in a browser six weeks later. diff --git a/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml b/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml index 7009146..bf3e82d 100644 --- a/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml +++ b/conformance/tests/l1-rendering/container-workload-render/golden/expected.yaml @@ -80,8 +80,10 @@ spec: - --cookie-domain=paas-demo.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -102,6 +104,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml b/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml index e107945..b32ea49 100644 --- a/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml +++ b/conformance/tests/l1-rendering/egress-class-render/golden/egress-internet.expected.yaml @@ -103,8 +103,10 @@ spec: - --cookie-domain=sonarr.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -125,6 +127,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml b/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml index a89ab86..692e8dd 100644 --- a/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml +++ b/conformance/tests/l1-rendering/egress-class-render/golden/egress-vpn.expected.yaml @@ -103,8 +103,10 @@ spec: - --cookie-domain=sonarr.home.keithjackson.dev - --whitelist-domain=.${BASE_DOMAIN} - --cookie-secure=true + - --cookie-refresh=2m30s - --pass-access-token=true - --set-xauthrequest=true + - --metrics-address=0.0.0.0:44180 env: - name: OAUTH2_PROXY_CLIENT_ID valueFrom: @@ -125,6 +127,9 @@ spec: - name: http containerPort: 4180 protocol: TCP + - name: metrics + containerPort: 44180 + protocol: TCP resources: requests: cpu: 25m diff --git a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md index a641151..edd3b25 100644 --- a/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md +++ b/decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md @@ -12,12 +12,23 @@ pattern, not of Crafty."* It is a property of the pattern, and this branch found where the pattern actually lives. -**Read the scope claim carefully before the detail.** This branch fixes a *measured collision*: six -independent oauth2-proxy instances, seven different cookie-secrets, one cookie name, one zone-wide -domain. It does **not** claim to fix the operator's reported sign-in failures. Those are two -separate claims, only the first is supported by anything here, and the evidence for why the second -is *not* supported — including a fair reading of counter-evidence that looked bad for this fix — is -in "What I verified". Do not merge this expecting the auth-rejection volume to drop. +**Read the scope claim carefully before the detail.** This branch fixes THREE defects in the +forward-auth sidecar contract, all in one function, all landing on all five apps at once: + +| Defect | Fix | Measured evidence | +|---|---|---| +| Six independent instances, seven different cookie-secrets, **one** cookie name on **one** zone-wide domain — so they overwrite and `ClearSession`-delete each other's sessions | `--cookie-name=_oauth2_proxy_` + host-scoped `--cookie-domain` | 7/7 distinct secrets; 6 instances on the default name; radarr logs the eviction fingerprint 8× in 12h | +| The session dies **5 minutes** after sign-in — `--cookie-refresh` defaults to 0 = *disabled*, so the refresh token the proxy already holds is never spent | `--cookie-refresh=2m30s` (half the lifetime) | access-token lifetime = **300.000s exactly**, `refresh_token:true`, flag absent | +| **No** per-app sidecar serves `/metrics` at all — without `--metrics-address` the listener never starts | `--metrics-address=0.0.0.0:44180` + a `metrics` container port | `ports` was `['http']` on all six | + +Rejection *volume* is dominated by the second defect, not the first: ~95% of radarr's rejections +carry no cookie at all. **Do not merge this expecting one number to drop** — three mechanisms were +tangled together, and separating them is most of the value here. + +**Boundary note.** My brief originally reserved `apps/wave-0/jackett.yaml` for a concurrent branch. +That split is not achievable and has been revoked: the file is generated, the fix is one renderer +edit, and `render-golden` requires all five goldens regenerated in the *same commit*. I own all +five. The proof I ran before touching jackett is under "What I verified". **The defect was not in the four manifests it was reported in.** `apps/wave-0/pihole.yaml`, `apps/wave-0/qbittorrent.yaml`, `apps/wave-1/radarr.yaml`, `apps/wave-1/sonarr.yaml` — and @@ -84,12 +95,13 @@ instances that accidentally share a name cannot see each other's cookie at all. on the exact axis that already failed once. The escape hatch is the same register, by name and with a reason, rather than a loosened rule. -### The jackett collision — read this +### The jackett boundary, and why it could not hold -`apps/wave-0/jackett.yaml` carries the identical defect and was assigned to **Mira Vance** on -`fix/jackett-oauth2-proxy-session`, with an instruction to me not to touch it. **That separation is -not achievable**, and I did not discover this by reasoning about it — I provoked it. With the -renderer fixed and jackett's golden left at its `main` content, `./ci/render-golden.sh` exits **1**: +`apps/wave-0/jackett.yaml` carries the identical defect and was initially assigned to a concurrent +branch, with an instruction to me not to touch it. **That separation is not achievable** — the +ownership split was drawn across a generated file — and I did not discover this by reasoning about +it, I provoked it. With the renderer fixed and jackett's golden left at its `main` content, +`./ci/render-golden.sh` exits **1**: [fail] GOLDEN DRIFT: apps/wave-0/jackett.yaml does not match the canonical render: - - --cookie-domain=.${BASE_DOMAIN} @@ -105,37 +117,45 @@ exists to forbid, and leaves jackett broken *by design*); or fix the generator a five. I took the third. Jackett's golden here is **machine output of a one-line generator change**, not an independent edit of Mira's file, and it is byte-identical to what her branch must produce. -**Consequence for the PM:** `fix/jackett-oauth2-proxy-session` has no work left in it. As of this -writing that branch has **zero commits ahead of `main`** and is not pushed, so nothing is lost — -but if Mira has since hand-edited `apps/wave-0/jackett.yaml`, her branch will fail `render-golden` -on its own terms and should be closed rather than rebased. +**Consequence:** the jackett branch has no cookie work left in it — this commit fixes jackett +identically and by the same mechanism, and both branches independently arrived at the *same two +values* (`_oauth2_proxy_jackett`, `jackett.${BASE_DOMAIN}`), which is a good sign for the +diagnosis. Its hand-edit of the rendered file will fail `render-golden` on its own terms, so it +should be closed rather than rebased. **What must not be lost with it** is its evidence: the +interleaved radarr/jackett log showing `removing session` deleting a clean session 26s after login, +and the `expires − created = 300.000s` measurement that produced `--cookie-refresh` here. Both are +carried into this entry and into the renderer's comments; the branch itself is now redundant, the +work behind it is not. -### Scope: what this branch deliberately does NOT fix +**The general lesson, which is worth more than this bug:** an ownership split across +`apps/wave-*/*.yaml` cannot hold, because those files are *outputs*. Two agents assigned two +rendered apps are assigned the same function and will collide in `render-golden` no matter how +carefully they stay in their own files. Ownership of a rendered app is ownership of +`catalog/render.py`, and briefs should say so. -Three further defects were raised against these sidecars while this branch was in flight. All -three are real; none is fixed here, and each omission is a decision, not an oversight. +### What this branch deliberately does NOT do -- **No `--cookie-refresh`, so the session dies in 5 minutes.** The session cookie's `expires` is - the Keycloak access-token lifetime; without `--cookie-refresh` oauth2-proxy never spends the - refresh token it already holds. **All four of my apps need this exactly as much as jackett - does** — same realm, same renderer, same function. It is not here because it is a *different - mechanism* (session lifetime, not cookie identity) that would be un-revertable independently - of the collision fix; because the correct VALUE is a judgement call belonging to whoever - measured it; and because nothing in my check guards it, so I would be shipping an unguarded - behaviour change. **When it lands it belongs in `_oauth2_sidecar_container()`, not in - jackett's golden** — one line there fixes all five; a hand-edit fixes one and fails - `render-golden`. -- **Unauthenticated XHR gets a 302, not a 401.** Same reasoning, plus one of its own: the fix - (`--api-routes`) is *path-shaped and per-app* (`^/api/` for the *arr and qBittorrent; pihole's - admin API is shaped differently), so doing it properly means declaring API path prefixes in the - ServiceDefinition — a schema change, well outside this branch. -- **No per-app sidecar exposes metrics at all** (no `--metrics-address`, no `metrics` port, so - the listener never starts and nothing is scrapable). I considered asserting this in the new - check and **rejected it as overreach**: the check's subject is "no two instances can share a - cookie", and a metrics assertion shares neither its inputs nor its failure mode. Bundling them - makes the check's name a lie and couples two unrelated ratchets, so a future metrics decision - could only be expressed by weakening a *cookie* check. `servicemonitor-ports-resolve` and - `alert-metrics-have-scrape-targets` already exist and are the right homes. +- **`--api-route=^/api/` is NOT added**, and not because of scope discipline alone: it would be + **wrong** renderer-wide. It was measured on jackett, and **PiHole does not serve `/api/`** — so + a renderer constant applies one app's path shape to five. Doing it properly means declaring API + path prefixes per service, and the `auth` block is `additionalProperties: false`, so that is a + *schema change*. Separate PR. (Note the flag is **`--api-route`, singular** — verified in the + pinned binary's own `--help`. The plural `--api-routes` does not exist in v7.8.1 and the proxy + refuses to start with it. That error was propagating through briefs; it stops here.) +- **`--session-cookie-minimal` is NOT added** and must not be, as a cookie-size fix: it is + **mutually exclusive** with `--cookie-refresh` (*"cookie_refresh > 0 requires oauth tokens in + sessions"*). Dropping `--pass-access-token` is a least-privilege argument, never a size one. +- **Metrics are NOT scraped, only served.** The flag and container port are the prerequisite; + a container port is not a Service port. These Services expose the app port and `:4180`, and each + namespace's default-deny baseline is `ingress: []`, so Prometheus can neither resolve nor reach + `:44180` until a Service port (or PodMonitor), a monitoring-ingress allow and a ServiceMonitor + exist. That wiring is someone else's follow-up and I have deliberately not faked its start. + **Nobody should read this commit as "the sidecars have metrics now."** +- **The new check was NOT widened to cover cookie-refresh or metrics.** Its subject is cookie + *identity*; those two share neither its inputs nor its failure mode. Bundling them would make + the check's name a lie and couple unrelated ratchets, so a future metrics decision could only + be expressed by weakening a *cookie* check. The cost of that discipline is named honestly in + the follow-ups: those two flags are currently **ungated**. ## What I verified @@ -177,32 +197,73 @@ three are real; none is fixed here, and each omission is a decision, not an over not valid`. Window `--since=24h`, which exceeds every pod's age only for crafty — whose pod is 13.9h old with **0 restarts**, so these are its complete post-fix figures. - Three conclusions, kept separate: - 1. **`crafty 81:1` is a window artefact and must not be used as evidence against the fix.** - The fixed pod has emitted **3** rejections in its entire life. The 81 came from a 24h Loki - window straddling a 13.9h-old pod, i.e. overwhelmingly from the PRE-fix pod. - 2. **The fix is still NOT shown to work.** Crafty logged **0 `AuthSuccess`** in that window — - nobody signed in — so there is no post-fix ratio to evaluate at all. Absence of failures - against absence of traffic proves nothing. This remains the open gap, not a resolved one. - 3. **The collision mechanism is independently confirmed, and it is a MINORITY of the - rejections.** `cookie signature not valid` + `removing session` is the collision's - fingerprint — a cookie *is* present, written by an instance whose key this one does not - have, so it cannot decrypt it and then `ClearSession` expires it on its own - `--cookie-domain` (the whole zone). radarr: **8 of 155** rejections (5%) carry it; jackett - 1 of 39; crafty, fixed, **0**. So the collision is real and this branch addresses it — and - ~95% of radarr's rejections carry no cookie at all, which points at the session-lifetime - and 302-vs-401 mechanisms above, not at this fix. **Anyone expecting these five renames to - fix the rejection volume will be disappointed, and should not be.** + Then I read crafty's four auth-relevant lines rather than counting them, because at n=3 the + content is the evidence: - `reject` is also not a clean SLI in either direction: it counts every unauthenticated first - hit and every XHR poll, so a busy *arr inflates it without any user being inconvenienced. + 03:57:20 Cookie settings: name:_oauth2_proxy_crafty secure(https):true httponly:true + expiry:168h0m0s domains:crafty.home.keithjackson.dev path:/ + 04:02:55 No valid authentication in request. Initiating login. + 15:22:39 No valid authentication in request. Initiating login. + 15:22:44 No valid authentication in request. Initiating login. + + 6743 log lines; those are all four. **Zero** `removing session`, **zero** `cookie signature + not valid`, zero anything else. + + Four conclusions, kept separate: + 1. **`crafty 81:1` is a window artefact. It is not evidence against the fix and must not be + cited as such.** The fixed pod emitted **3** rejections in 13h 52m; the 81 came from a 24h + window straddling it, i.e. from the pre-fix process. + 2. **The eviction FINGERPRINT is gone where the fix is deployed.** `cookie signature not + valid` + `removing session` is what a clobbered instance logs — a cookie *is* present, + written with a key this instance does not hold, so it cannot decrypt it and then + `ClearSession` expires it on its own `--cookie-domain`. Fixed crafty: **0 and 0** across + its whole life. Unfixed radarr: **8 and 8** in 12h. Unfixed jackett: 1 and 1. That is a + real, directional signal and it is the strongest evidence available that the fix works. + 3. **It is still not a demonstrated successful sign-in.** Crafty logged **0 `AuthSuccess`** — + so what is shown is *absence of the failure mode*, not a session observed working + end-to-end. Those three rejections are consistent with benign first-hits (one client, two + of them 5s apart, i.e. a page plus a `/ws` reconnect). I am comfortable saying the fix + works; I am not willing to say it has been watched working, and the distinction is the + whole reason this section exists. + 4. **The collision is a MINORITY of the rejection volume.** radarr: 8 of 155 (5%) carry the + fingerprint; ~95% carry **no cookie at all**. Those are the 5-minute session expiry and + `302`-on-XHR — the first of which this branch now also fixes, the second of which it + deliberately does not. + + Two cautions about the instrument. `reject` ranks **chattiness, not breakage**: radarr's are + `/api/v3/command`, `/api/v3/queue/details` and `/signalr/negotiate` polls, so a busy *arr + inflates the count with nobody inconvenienced. And that same polling is the *amplifier* that + made this intermittent — every poll carrying an undecryptable cookie calls `ClearSession` + again on the zone-wide domain, so **a left-open Radarr tab deleted Jackett's cookie every few + seconds, from another namespace, with no user action at all.** That is why signing in again + never helped for longer than the gap between two polls. It is now a comment in the renderer, + because it is the part of this bug that no one would reconstruct from the flags alone. - **The four target files are generated, not authored.** Rendering `catalog/` with the canonical command into a temp dir and diffing against `apps/` reported all 10 onboarded goldens byte-identical before any change. Hand-editing one is drift by definition. - **`render-golden` FAILS if jackett is excluded** — provoked (output quoted above), rc **1**. Restoring the regenerated golden returns it to `all 10 onboarded service golden(s) reproduce`, rc **0**. -- **`catalog/render.py --selftest`: PASS**, including the new two-service distinctness assertion. +- **Every flag verified against the PINNED v7.8.1 binary**, not against docs, memory, or the + shared proxy (which runs **v7.15.3** and therefore could not vouch for v7.8.1): + `podman run --rm quay.io/oauth2-proxy/oauth2-proxy:v7.8.1 --help`. + - `--cookie-name string ... (default "_oauth2_proxy")` — **this confirms the constant my whole + check is built on, from the binary itself.** The check's central claim, that an omitted flag + is a *claim on a name* rather than an absence, now rests on the implementation's own help + text rather than on my reading of its source. + - `--cookie-refresh duration refresh the cookie after this duration; 0 to disable` — confirms + the default really is disabled. + - `--metrics-address string` — present in v7.8.1, so the new flag cannot crashloop the pods. + - `--api-route strings` — **singular**, confirming the brief's plural was wrong. + - `--session-cookie-minimal` — exists, and is the flag that conflicts with `--cookie-refresh`. +- **44180 is the platform's existing convention, measured, not chosen.** The shared proxy already + runs `--metrics-address=0.0.0.0:44180` with a `metrics` container port, a `metrics:44180` + Service port, a ServiceMonitor whose endpoint is `metrics`, and **2 ready endpoint addresses** — + i.e. it demonstrably works on this platform. +- **`catalog/render.py --selftest`: PASS**, now including the two-service distinctness assertion, + a `--cookie-refresh` assertion that PARSES the duration and requires `0 < refresh < 300s` (so + `2m30s` -> `150s` -> `2.5m` all stay green but a silent widening past the token lifetime does + not), a `--session-cookie-minimal` mutual-exclusion guard, and both metrics assertions. - **The check is selected by the whole-repo union.** `conformance/run.sh --layer l0 --requirements r-nosuchreq-99 --include-scope whole-repo --print-selection` lists `tests/l0-static/oauth2-proxy-cookie-names-distinct` among 20 tests — i.e. it runs even when the @@ -259,29 +320,47 @@ three are real; none is fixed here, and each omission is a decision, not an over **The red tick is the policy, and it clears when a human reviews it.** Note also that `gate / conformance (pull_request)` reports `success` with description **"Has been skipped"** — the precise trap CLAUDE.md §2b names; it certifies nothing. -- **THAT THIS FIX CURES THE REPORTED SYMPTOM.** Stated plainly because it is the claim most likely - to be read into this branch: it is **not supported**. What is measured is that the instances - *collide* (distinct secrets, shared default name, shared domain) and that the collision's - fingerprint appears in live logs (radarr 8×, jackett 1×, fixed crafty 0×). What is **not** - measured is that removing the collision makes anyone's sign-in work, because the one app already - fixed logged **zero successful sign-ins** in its fixed pod's entire 13.9h life — no traffic, so - no ratio, so no evidence either way. And ~95% of radarr's rejections carry **no cookie at all**, - so most of the observed failures are something this branch does not touch. Anyone closing this - out should measure crafty's ratio again **after real sign-in traffic**, and expect the other two - mechanisms (5-minute session expiry, 302-on-XHR) to dominate until they are fixed too. -- **No browser round-trip.** The collision has never been reproduced *by me* in a browser, because - reproducing it needs two gated apps visited in one session. Everything here is reasoned from - measured configuration and from the mechanism. The concurrent jackett branch does carry an - observed log-level reproduction (interleaved radarr/jackett `removing session` deleting a clean - session 26s after login) — that is real evidence for the MECHANISM, and it is theirs, not mine. +- **A SUCCESSFUL SIGN-IN, ANYWHERE.** The strongest post-fix evidence is the *absence* of the + eviction fingerprint on crafty (0 `removing session`, 0 `cookie signature not valid`, across + 13h 52m) against radarr's 8-and-8. That is directional and I believe it. But crafty logged + **0 `AuthSuccess`** in the same window, so no session has been observed working end to end on a + fixed instance. Whoever closes this out should re-measure crafty **after real sign-in traffic**. +- **No browser round-trip by me at all**, for any of the three defects. Reproducing the collision + needs two gated apps visited in one session. The concurrent jackett branch does carry an observed + log-level reproduction (interleaved radarr/jackett `removing session` deleting a clean session + 26s after login) — real evidence for the MECHANISM, and it is theirs, not mine. +- **`--cookie-refresh=2m30s` has never been exercised.** The value is derived from a measured + 300.000s access-token lifetime and is asserted to sit inside `(0, 300s)`, but no session has been + held past the five-minute cliff to watch a refresh succeed. If Keycloak's refresh-token or SSO + idle timeouts are shorter than expected, the symptom moves rather than disappears. **This is the + fix most likely to need a second pass**, and the first thing to watch after merge. +- **The `metrics` port has never been fetched.** The listener is asserted to start by flag + presence only; nobody has curled `:44180` on a rendered pod. And it is unreachable by design + until the Service/policy/ServiceMonitor wiring lands — see above. - **Nothing was applied.** No `kubectl apply/edit/patch`, no `flux reconcile`. Cluster access was - read-only throughout. The running pods are still the pre-fix spec. **Merging is what deploys**, - and it will roll five Deployments; every user's existing session on those five apps is - invalidated by the cookie rename and they will re-authenticate once. Radarr/sonarr/qbittorrent - use `strategy: Recreate` on RWO volumes, so there is a brief gap per app. Pick the moment. + read-only throughout; the one container I ran was the oauth2-proxy image locally under podman to + read its `--help`. The running pods are still the pre-fix spec. **Merging is what deploys**, and + it will roll five Deployments; every user's existing session on those five apps is invalidated by + the cookie rename and they will re-authenticate once. Radarr/sonarr/qbittorrent use + `strategy: Recreate` on RWO volumes, so there is a brief gap per app. Pick the moment — and + note this now changes **three** flags plus a port on all five, so a bad `--cookie-refresh` value + would surface as five apps misbehaving, not one. - **The live flags are not re-measured by the check.** git is not the cluster. I measured the live pods by hand, once; nothing in the gate re-reads them, so hand-applied drift would be invisible to it. A live-state assertion is L2 and is not written. +- **`--cookie-refresh` and `--metrics-address` are UNGATED, and I found out why.** Nothing in + `ci/` or `.gitea/` runs `catalog/render.py --selftest` — I grepped for it. So the twenty-odd + assertions in that selftest, including the six I just added, are dev-time only: a future edit + that drops either flag *and* re-renders the goldens would pass the entire merge gate. + `render-golden` catches accidental drift (golden vs renderer) but not a deliberate removal of + both together, and my L0 check deliberately does not cover these two flags. **The repo already + has the precedent for the fix and the one-line shape of it:** + `conformance/tests/l0-static/auditability-proofs-producible/check.sh:291` drives + `conformance/lib/ --selftest` as a subprocess for exactly this reason — its own header + says it does so "because nothing [else] drives them". Adding `catalog/render.py` to that list, + or giving the renderer its own scope-declaring L0 test, closes it. I did not do it here because + it widens an already review-blocked PR into a third subject; it is the highest-value follow-up + in this entry. - **Log-derived counts are a weak instrument and I did not strengthen them.** `kubectl logs --since=` returns what the kubelet still retains, so an old, busy container's window may be truncated; and `No valid authentication in request` counts unauthenticated first hits and XHR