fix(auth): five oauth2-proxy sidecars shared one cookie, died at 5 minutes, and served no metrics #28

Merged
jakman85 merged 4 commits from fix/oauth2-proxy-cookie-collision-fleet into main 2026-09-30 21:39:41 +00:00
4 Commits
Author SHA1 Message Date
jakman85andClaude Opus 5 dc136a4943 fix(auth): the forward-auth sidecar also died at 5 minutes and served no metrics
gate / gate (pull_request) Successful in 1m42s
gate / conformance (pull_request) Has been skipped
Widens the cookie-collision fix to the whole sidecar contract, because all three
defects live in the same function and land on the same five goldens, so they are one
atomic change or they are five conflicting ones.

Added to _oauth2_sidecar_container(), on top of the cookie-name/cookie-domain fix:

  --cookie-refresh=2m30s
      The session died FIVE MINUTES after sign-in. Keycloak's homelab realm issues
      300-second access tokens (MEASURED on a live session: expires - created =
      300.000s exactly, with refresh_token:true on the same line), and
      --cookie-refresh defaults to 0 -- which the pinned binary's own help spells
      "0 to disable". So the proxy held a refresh token it was never told to spend.
      Refresh at HALF the lifetime: two chances to renew before expiry.
      This, not the collision, is what dominates the rejection VOLUME -- ~95% of
      radarr's 155 rejections in 12h carry no cookie at all.

  --metrics-address=0.0.0.0:44180 + a `metrics` container port
      Without the flag the /metrics listener never starts, so no per-app sidecar was
      serving metrics at all (`ports` was ['http'] on all six). 44180 is the
      platform's existing convention, measured not chosen: the shared proxy already
      runs the same flag and port with 2 ready endpoints.
      PREREQUISITE ONLY -- NOT scraped. A container port is not a Service port, and
      each namespace's default-deny baseline is `ingress: []`, so this is unreachable
      until a Service port (or PodMonitor), a monitoring allow and a ServiceMonitor
      land. Do not read this as "the sidecars have metrics now".

EVERY FLAG VERIFIED AGAINST THE PINNED v7.8.1 BINARY (`--help`), not against docs or
the shared proxy -- which runs v7.15.3 and could not have vouched for v7.8.1:
  * --metrics-address exists in 7.8.1, so this cannot crashloop the pods
  * --cookie-refresh ... "0 to disable" confirms the default
  * --cookie-name ... (default "_oauth2_proxy") -- this confirms from the
    IMPLEMENTATION the constant the new L0 check is built on
  * --api-route is SINGULAR; the plural does not exist and the proxy refuses to start
  * --session-cookie-minimal exists and is mutually exclusive with --cookie-refresh,
    so dropping --pass-access-token is a least-privilege argument, never a size fix

NOT done, deliberately: --api-route=^/api/ renderer-wide. It was measured on jackett
and PiHole does not serve /api/, so a renderer constant would apply one app's path
shape to five. It needs a per-app hook the `auth` schema forbids
(additionalProperties: false) -- a schema change, separate PR.

Also records the mechanism that made this INTERMITTENT, as a renderer comment, because
nobody would reconstruct it from the flags: the loser does not just fail to read the
cookie, it calls ClearSession, which expires the cookie on its own --cookie-domain --
the whole zone. The *arr UIs poll (/api/v3/command, /api/v3/queue/details,
/signalr/negotiate), so every poll carrying an undecryptable cookie clears it again:
A LEFT-OPEN RADARR TAB DELETED JACKETT'S COOKIE EVERY FEW SECONDS, from another
namespace, with no user action. That is why signing in again never helped for longer
than the gap between two polls.

Crafty's counter-evidence resolved: the 81:1 ratio was a 24h window straddling a
13h52m-old pod. Bounded to the fixed process, crafty's ENTIRE life is 3 rejections in
6743 log lines, with ZERO `removing session` and ZERO `cookie signature not valid`
against unfixed radarr's 8 and 8. The eviction fingerprint is gone where the fix is
deployed. It logged 0 AuthSuccess too, so that is absence-of-the-failure-mode rather
than a session watched working -- stated as such in the handoff.

Renderer selftest gains six assertions, including a --cookie-refresh check that PARSES
the duration and requires 0 < refresh < 300s (so 2m30s/150s/2.5m all stay green while a
silent widening past the token lifetime does not). NOTE, and it is in the handoff as the
top follow-up: NOTHING IN ci/ RUNS `render.py --selftest`, so those assertions are
dev-time only. conformance/tests/l0-static/auditability-proofs-producible/check.sh:291
already drives other tools' selftests for exactly this reason and is the one-line fix.

Gate (toolbox, worktree shim with the spec repo mounted at /home-network-k3s): rc 3
UNDETERMINED. Green: spec-immutability, decisions-log, lint, l0, render-golden, l1,
approval-policy. Skipped: gitea-settings (no local token), conformance (declared).
The new L0 check PASSES on this branch -- all five apps fixed in one commit.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-09-30 13:07:43 -05:00
jakman85andClaude Opus 5 f4a155a807 docs(decisions): the collision is measured; that it CAUSES the failures is not
gate / gate (pull_request) Failing after 1m43s
gate / conformance (pull_request) Has been skipped
Counter-evidence was raised against this branch: over a 24h Loki window crafty -- the
app already carrying the cookie fix -- showed the WORST auth-rejection ratio of the set
(crafty 81:1, radarr 67:1, jackett 39:4). Taken at face value that says the fix does not
work. It does not say that, and it does not say the opposite either.

Re-measured bounded to the fixed pod, read from each sidecar's OWN log rather than Loki
(`kubectl logs` on a pod with 0 restarts cannot reach a pod older than itself, so a
window inside the pod's age is inside the fixed spec by construction):

    namespace      age(h) rst  reject success    ratio  cleared  badsig
    crafty           13.9   0       3       0      n/a        0       0   <- FIXED, whole life
    radarr          422.5   0     155       1  155.0:1        8       8
    jackett         422.6   0      39       4    9.8:1        1       1
    pihole/qbittorrent/sonarr/shared: 0 reject, 0 success

Three findings, kept apart:

1. `crafty 81:1` is a WINDOW ARTEFACT. The fixed pod has emitted 3 rejections in its
   entire 13.9h life; the 81 came from a 24h window straddling it, i.e. from the pre-fix
   pod. It is not evidence against the fix and must not be cited as such.
2. The fix is still NOT SHOWN TO WORK. Crafty logged ZERO AuthSuccess in that window --
   no traffic, so no ratio. Absence of failure against absence of traffic proves nothing.
3. The collision mechanism IS independently confirmed, and it is a MINORITY of the
   rejections. `cookie signature not valid` + `removing session` is its fingerprint -- a
   cookie IS present, written with a key this instance does not have, so it cannot decrypt
   it and then ClearSession expires it zone-wide. radarr 8 of 155 (5%), jackett 1 of 39,
   fixed crafty 0. ~95% of radarr's rejections carry no cookie at all and point at the
   session-lifetime and 302-on-XHR mechanisms this branch does not touch.

So the handoff now says, at the top and in both evidence sections, that "the instances
collide" and "the collisions cause the reported symptom" are different claims and only
the first is supported. The four fixes stand -- distinct cookie names are correct
regardless -- but nothing here should be read as a cure.

Also records three deliberate NON-fixes with their reasoning (--cookie-refresh, which all
four of my apps need as much as jackett does and which belongs in the renderer rather than
in a golden; --api-routes for 302-on-XHR, which is path-shaped and needs a schema change;
and sidecar metrics, which I judged overreach for a check whose subject is cookie
identity), and two gate facts:

  * spec-immutability was skipping because of a PATH BUG IN THE WORKTREE SHIM, not the
    environment -- dev.sh derives the spec path from its own REPO_ROOT, which in a worktree
    does not resolve. Mounting the spec repo at /home-network-k3s with SPEC_REPO set makes
    R-TEST-06 actually run and PASS. Local gate is now rc 3 for one reason (gitea-settings,
    no token) instead of two.
  * CI's gate reports FAILURE on 75a4029 for exactly one reason, and it is not this change:
    `approval-policy: a sensitive change lacks a non-author approval`. Every substantive
    stage is green in CI, including gitea-settings. conformance/** is a sensitive path, so
    the PR is fail-closed pending a human review -- CLAUDE.md §1 working as designed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-09-30 12:56:33 -05:00
jakman85andClaude Opus 5 75a4029d64 docs(decisions): record the measured gate rc, stage list and touched subset
gate / gate (pull_request) Failing after 1m44s
gate / conformance (pull_request) Has been skipped
The handoff asserted a gate outcome before the gate had been run against a
committed branch. Replaces it with what was actually measured on 9a4489b:
rc 3 (UNDETERMINED) -- green decisions-log/lint/l0/render-golden/l1/approval-policy,
skipped spec-immutability/gitea-settings/conformance -- plus the twelve touched
requirements that are UNVERIFIED at L2/L3, and a note that approval-policy passed
VACUOUSLY here (event='none') while the PR itself needs a non-author review because
conformance/** is a sensitive path.

Also records that the three conformance/tests/l1-rendering goldens are regenerated:
they are expected-output fixtures of the same renderer, and the L1 stage caught their
drift on the first gate run rather than anyone remembering them.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-09-30 12:46:40 -05:00
jakman85andClaude Opus 5 9a4489b733 fix(auth): five oauth2-proxy sidecars shared one cookie and evicted each other
gate / gate (pull_request) Failing after 1m43s
gate / conformance (pull_request) Has been skipped
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]>
2026-09-30 10:55:06 -05:00