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) <[email protected]>
This commit is contained in:
1 parent
0fbf544d5e
commit
9a4489b733
12 files changed
+930
-10
No files matched your search
@@ -100,7 +100,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://jackett.${BASE_DOMAIN}/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -108,7 +108,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://pihole.${BASE_DOMAIN}/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -171,7 +171,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://qbittorrent.${BASE_DOMAIN}/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -153,7 +153,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://radarr.${BASE_DOMAIN}/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -153,7 +153,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://sonarr.${BASE_DOMAIN}/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
+85
-2
@@ -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,
|
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
|
else /oauth2 returns 403). client-id/secret + cookie-secret come by name from the SOPS-encrypted
|
||||||
`<app>-oauth2-proxy` Secret (R-IDP-04). Single provider => --skip-provider-button 302s straight
|
`<app>-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]
|
route = s["ingressRoutes"][0]
|
||||||
host = route["host"]
|
host = route["host"]
|
||||||
upstream_port = route["port"]
|
upstream_port = route["port"]
|
||||||
@@ -991,7 +995,41 @@ def _oauth2_sidecar_container(s: dict) -> dict:
|
|||||||
args += [
|
args += [
|
||||||
"--skip-provider-button=true",
|
"--skip-provider-button=true",
|
||||||
f"--redirect-url=https://{host}/oauth2/callback",
|
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}",
|
f"--whitelist-domain=.{ZONE}",
|
||||||
"--cookie-secure=true",
|
"--cookie-secure=true",
|
||||||
"--pass-access-token=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"]),
|
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)",
|
"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(
|
check(
|
||||||
side["env"][0]["valueFrom"]["secretKeyRef"]["name"] == "fa-oauth2-proxy",
|
side["env"][0]["valueFrom"]["secretKeyRef"]["name"] == "fa-oauth2-proxy",
|
||||||
"sidecar creds must come by name from the per-app <app>-oauth2-proxy Secret (R-IDP-04)",
|
"sidecar creds must come by name from the per-app <app>-oauth2-proxy Secret (R-IDP-04)",
|
||||||
|
|||||||
@@ -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
|
||||||
@@ -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 "<relpath>:<instance id>".
|
||||||
|
# =============================================================================================
|
||||||
|
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 <cookie-name>_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 '<host>'}"
|
||||||
|
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
|
||||||
@@ -76,7 +76,8 @@ spec:
|
|||||||
- --allowed-group=platform-admins
|
- --allowed-group=platform-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://paas-demo.home.keithjackson.dev/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
+2
-1
@@ -99,7 +99,8 @@ spec:
|
|||||||
- --allowed-group=media-admins
|
- --allowed-group=media-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://sonarr.home.keithjackson.dev/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -99,7 +99,8 @@ spec:
|
|||||||
- --allowed-group=media-admins
|
- --allowed-group=media-admins
|
||||||
- --skip-provider-button=true
|
- --skip-provider-button=true
|
||||||
- --redirect-url=https://sonarr.home.keithjackson.dev/oauth2/callback
|
- --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}
|
- --whitelist-domain=.${BASE_DOMAIN}
|
||||||
- --cookie-secure=true
|
- --cookie-secure=true
|
||||||
- --pass-access-token=true
|
- --pass-access-token=true
|
||||||
|
|||||||
@@ -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.
|
||||||
Reference in new issue
Block a user