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.