Home-Network/home-network-k3s-impl
2
0 Fork 0
Code Issues 2 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

feat(auth): declare API paths per service, so --api-route can ship at all
gate / gate (pull_request) Failing after 1m44s
Details
gate / conformance (pull_request) Has been skipped
Details

Browse Source 
PR #28 fixed the cookie collision, the 5-minute session and the missing metrics
listener in one renderer function, and deliberately declined `--api-route`
because the config model could not express the difference between two apps.
This adds the hook it was missing and sets the flag only where a measurement
says to.

THE DEFECT. oauth2-proxy answers an unauthenticated request with a 302 to
Keycloak. Its AJAX guard would send 401 instead, but it fires only on an EXACT
`Accept: application/json`, which these apps' JavaScript does not consistently
send. So a browser XHR gets a 302, FOLLOWS IT INVISIBLY, has it satisfied
silently by the live SSO session, and resolves 200 carrying the app's HTML where
the JavaScript expected JSON. On a write it is worse: a browser following a 302
re-issues a POST as a GET and DROPS THE BODY, so the operator's change is
discarded and the UI reports success.

MEASURED, on two apps and two different paths -- which is the whole argument:
  jackett 2026-09-30: 33 x 302 vs 2 x 401 on /api/ paths, and 4 of 6 POSTs
          answered by the proxy and never reaching Jackett (upstream `-`).
  radarr  2026-10-01, 23h after PR #28 deployed (pod radarr-7d55cbffb9-42hmh,
          0 restarts): 232 unauthenticated requests in the pod's whole life and
          ALL 232 are the same call -- POST /signalr/messages/negotiate from a
          browser, 302, upstream `-`. A left-open tab whose session the cookie
          rename orphaned, polling forever, silently redirected every time and
          NEVER re-prompting. The reported symptom, still in production.

So radarr's broken path is /signalr/ and jackett's is /api/, and PIHOLE SERVES
NO /api/ AT ALL. One function renders all five sidecars, so the cheap fix is a
constant -- and a constant applies one app's measurement to four apps nobody
measured. `auth` was `additionalProperties: false`, so there was no per-app
hook. Adding one named, documented key is the change; the block stays closed.

  schemas/servicedefinition.schema.json  1.8.0 -> 1.9.0: optional
      `auth.apiRoutes`, a list (radarr needs two, and --api-route is natively
      repeatable) of anchored path regexes. Plus the shape invariant that a
      non-empty list requires mode: forward-auth, since no sidecar is rendered
      for oidc/native/none and declaring it there would be a SILENT NO-OP.
  catalog/render.py  emits one --api-route per entry, in order, and RAISES on
      an unanchored entry. The flag is SINGULAR -- verified against the pinned
      v7.8.1 binary's own --help; --api-routes does not exist and the proxy
      refuses to start with it.
  catalog/{jackett,radarr}.yaml  the two measured apps.
  catalog/{pihole,sonarr,qbittorrent}.yaml  comments recording that they get
      NOTHING, and why. Sonarr and qBittorrent were measured (0 unauthenticated
      requests in 23h) and left alone: that is evidence of no TRAFFIC, not of no
      need, and sonarr being nearly radarr's codebase is what makes the guess
      tempting and still a guess.
  apps/wave-0/jackett.yaml, apps/wave-1/radarr.yaml  regenerated. Only TWO
      goldens change; pihole/qbittorrent/sonarr are byte-identical to main,
      which is the per-app property proving itself on the real tree.

BREADTH IS THE DANGEROUS DIRECTION AND IT IS NOT SYMMETRIC. A missing
--api-route is a silent 302. A too-broad one answers a plain PAGE LOAD with
401, so a logged-out operator gets a bare error page and is never offered a
sign-in -- the service looks permanently broken. Each entry must therefore
match `^\^/[A-Za-z0-9._~/-]+(\.\*|\$)?$`: anchored at ^/ then literal path
characters. `^/`, `^/.*`, `.*`, `/api/` and `^/api/|^/` are all rejected, all
five provoked. Alternation is refused rather than supported because the flag is
repeatable, so one entry per route costs nothing.

NEW GATE CHECK: conformance/tests/l0-static/oauth2-proxy-api-routes-per-app
(whole-repo scope, so ci/l0.sh's --include-scope union runs it on every
invocation; no bespoke stage, which whole-repo-invariants-wired would fail).
It is a SIBLING of oauth2-proxy-cookie-names-distinct, not an extension of it:
that check's subject is cookie IDENTITY and shares neither inputs nor failure
mode with this, so bundling them would make its name a lie and let a future
api-route decision be expressed only by weakening a COOKIE check.

The check exists for one reason the other gates cannot cover: render-golden
compares the render to the goldens, so a RENDERER CONSTANT plus regenerated
goldens AGREE WITH EACH OTHER while both being wrong. Provoked: the regen exits
0 -- render-golden is happy -- and this check exits 1 with 15 failures naming
pihole, qbittorrent, radarr and sonarr individually. It also asserts the
anchoring pattern is the SAME in the schema and the renderer, because three
surfaces guard this and if they drift the loosest wins silently.

Tested in both directions on throwaway trees outside the repo: compliant tree
rc 0 / 43 assertions / 6 sidecars; pre-change tree rc 1 / 11 failures; and four
targeted mutations (hand-edited golden, widened regex, renderer constant,
deleted schema pattern) each firing their own assertion class and nothing else.

ALSO FOLDED IN (coordinator's request; defect is in the file I own). _selftest()
reached _cookie_name() through a bare `next(genexpr)` with NO default, so
deleting the --cookie-name emission -- the exact mutation that assertion exists
to catch -- raised StopIteration, which escaped _selftest() BEFORE its collected
`SELFTEST FAIL:` lines were printed. Right verdict, zero diagnostics, and it
masked every other assertion the same mutation broke (found by Desmond Oyelaran
while wiring the selftest into the render-golden stage; his PR #33 is not in this
base -- origin/main @ cdf3e14 drives the selftest from no stage, checked). Fixed
with `next(..., None)` plus an explicit check(), so a missing flag reports as a
NAMED assertion. Provoking it exposed three more instances of the same class in
the same block -- `side = fa_containers[1]`, `side["env"][0]`, and the fa_svc
lookup all raise rather than assert when the sidecar is absent -- so all four are
now None-tolerant, and the two fixtures added for --api-route were written that
way from the start. Measured on throwaway copies:
  --cookie-name deleted:  before rc 1 / 0 FAIL lines / StopIteration
                          after  rc 1 / 3 FAIL lines / no traceback
  sidecar not appended:   before rc 1 / 0 FAIL lines / IndexError
                          after  rc 1 / 20 FAIL lines / no traceback
Goldens are unchanged by this part; it touches _selftest() only.

NOT verified, in full in the handoff entry: no browser round-trip by anyone for
this flag -- that a 401 REPAIRS the operator's experience is reasoned from HTTP
semantics, not watched. `^/api/` on radarr is PR #30's measurement of radarr's
pre-fix pod, not mine; the current pod has zero /api/ traffic. Sonarr and
qBittorrent are UNMEASURED, not cleared. --cookie-refresh is still unexercised
(0 AuthSuccess on all five pods). Nothing was applied; the live pods are still
the pre-change spec.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
jakman85andClaude Opus 5 committed 2026-10-01 22:07:08 -05:00
1 parent cdf3e14e27
commit 99560c2d22
13 files changed
+1245 -17
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

catalog/render.py
+218 -15
View File
@@ -172,6 +172,45 @@ OAUTH2_PROXY_METRICS_PORT = 44180
DRI_VOLUME_NAME = "dri"
DRI_DEVICE_DIR = "/dev/dri"
# --- API routes: 401 instead of 302, PER APP ------------------------------------------
# oauth2-proxy answers an unauthenticated request with a 302 to Keycloak. Its built-in AJAX
# guard would send a 401 instead, but it fires only on an EXACT `Accept: application/json`,
# which these apps' JavaScript does not consistently send. So an unauthenticated XHR gets a
# 302, the browser follows it INVISIBLY, the live Keycloak SSO session satisfies it silently,
# and the call resolves 200 carrying the app's HTML where the JavaScript expected JSON — no
# prompt, no console error, nothing logged as a failure. On a WRITE it is worse: a browser
# following a 302 re-issues a POST as a GET and DROPS THE BODY, so the operator's change is
# discarded and the UI reports success. MEASURED, twice:
# jackett 2026-09-30: 33 x 302 against 2 x 401 on /api/ paths, and 4 of 6 POSTs answered by
# the proxy and never reaching the app (access-log upstream field `-`, not `/`).
# radarr 2026-10-01, 23h after the cookie fix rolled: 232 x 302, EVERY ONE a
# `POST /signalr/messages/negotiate` from the operator's own browser, upstream `-`.
# A left-open tab whose cookie the rename orphaned, polling forever, silently
# redirected every time, NEVER re-prompting. That is the reported symptom
# ("reloading never fixes it and I am never re-prompted") still happening in
# production, on a service nobody was looking at.
# `--api-route=<regex>` makes matching paths answer 401.
#
# WHY THIS IS NOT A RENDERER CONSTANT, AND MUST NEVER BECOME ONE. The path shape is a property
# of the APP, not of the forward-auth pattern: radarr's broken path is `/signalr/`, jackett's is
# `/api/`, and PIHOLE SERVES NO `/api/` AT ALL. One constant would apply one app's path shape to
# five, and the failure mode of a wrong one is not "no improvement" but "this service is
# permanently broken": a path a BROWSER NAVIGATES to, answered 401, shows a logged-out operator
# a bare error page and never offers a sign-in. So the routes are declared per service in
# `auth.apiRoutes` (schema 1.9.0) and rendered one `--api-route` per entry — the flag is
# repeatable, and it is SINGULAR: `--api-routes` does not exist in v7.8.1 and the proxy refuses
# to start with it (verified against the pinned binary's own --help, 2026-10-01:
# `--api-route strings return HTTP 401 instead of redirecting to authentication server...`).
#
# BREADTH RATCHET. Every entry must be anchored at `^/` and continue with literal path
# characters. That rejects `^/`, `^/.*`, `.*`, an unanchored `/api/`, and any alternation — the
# shapes that turn a page load into a 401. Alternation is refused rather than supported because
# the flag is repeatable, so one regex per route costs nothing and stays readable. The same
# pattern is in the schema (so ci/l0.sh's schema stage rejects it at the authoring surface) and
# enforced HERE (so ci/render-golden.sh fails even if the schema stage is skipped) — a breadth
# mistake must not need two independent gates to both be running.
OAUTH2_API_ROUTE_PATTERN = re.compile(r"^\^/[A-Za-z0-9._~/-]+(\.\*|\$)?$")
# =============================================================================
# Defaults (renderer-contract.md "Default posture") — materialize BEFORE mapping.
@@ -197,6 +236,12 @@ def apply_defaults(sd: dict) -> dict:
if "mode" not in auth:
auth["mode"] = "forward-auth" if s["ingressRoutes"] else "none"
auth.setdefault("groups", [])
# apiRoutes default posture is EMPTY, deliberately, and that is the whole point of the field
# (schema 1.9.0): an app gets --api-route only where someone MEASURED that it serves a machine
# API, so a newly-onboarded app renders no --api-route at all rather than inheriting another
# app's path shape. See OAUTH2_API_ROUTE_PATTERN above for why a default of `^/api/` would be
# wrong rather than merely generous.
auth.setdefault("apiRoutes", [])
s["auth"] = auth
update = dict(s.get("update") or {})
@@ -1054,7 +1099,12 @@ def _oauth2_sidecar_container(s: dict) -> dict:
--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."""
zone makes the instances evict each other's sessions.
--api-route is DECLARED per app, from auth.apiRoutes, and is the one flag here that is
neither a constant nor derivable: it names the paths THIS app serves as a machine API, and
those differ per app (radarr `/signalr/`, jackett `/api/`, pihole none). An app that declares
none renders no --api-route at all."""
route = s["ingressRoutes"][0]
host = route["host"]
upstream_port = route["port"]
@@ -1074,6 +1124,27 @@ def _oauth2_sidecar_container(s: dict) -> dict:
args += [
"--skip-provider-button=true",
f"--redirect-url=https://{host}/oauth2/callback",
]
# ── API ROUTES: 401 instead of 302, ON THIS APP'S OWN PATHS ONLY ───────────────────────
# One flag per declared route, in declared order. An app that declares NONE renders NO
# --api-route, which is the correct and common case — see OAUTH2_API_ROUTE_PATTERN above for
# the measurements, for why this cannot be a renderer constant, and for the breadth ratchet.
# Validated here as well as in the schema so a breadth mistake is caught by render-golden
# even on a run where the schema stage did not execute.
for route in s["auth"].get("apiRoutes", []):
if not OAUTH2_API_ROUTE_PATTERN.match(str(route)):
raise SystemExit(
f"render.py: {s['name']}: auth.apiRoutes entry {route!r} is not an anchored path "
f"regex. Each entry must start with '^/' and continue with literal path "
f"characters (e.g. '^/api/', '^/signalr/', '^/api/v3/.*'). Rejected shapes: '^/' "
f"and '^/.*' (match every page load), an unanchored '/api/' (matches mid-path), "
f"and alternation (the flag is repeatable — declare one entry per route). A "
f"too-broad --api-route answers a PAGE LOAD with 401, so a logged-out operator "
f"sees a bare error page and is never offered a sign-in: the service looks "
f"permanently broken rather than merely unauthenticated."
)
args.append(f"--api-route={route}")
args += [
# ── 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
@@ -2666,15 +2737,27 @@ def _selftest() -> int:
"auth": {"mode": "forward-auth", "groups": ["media-users"]},
}
)
fa_dep = next(d[1] for d in fa_docs if d[1]["kind"] == "Deployment")
fa_containers = fa_dep["spec"]["template"]["spec"]["containers"]
fa_dep = next((d[1] for d in fa_docs if d[1]["kind"] == "Deployment"), None)
check(fa_dep is not None,
"a forward-auth app rendered NO Deployment, so there is no pod to gate at all")
fa_containers = (((fa_dep or {}).get("spec") or {}).get("template") or {}) \
.get("spec", {}).get("containers") or []
check(
len(fa_containers) == 2
and fa_containers[0]["name"] == "fa"
and fa_containers[1]["name"] == "oauth2-proxy",
"forward-auth app must render the app container FIRST + an oauth2-proxy sidecar SECOND",
f"forward-auth app must render the app container FIRST + an oauth2-proxy sidecar SECOND; "
f"rendered {[c.get('name') for c in fa_containers]!r}",
)
side = fa_containers[1]
# EVERY assertion below reads `side`, so an absent sidecar must NOT be allowed to raise here.
# The whole point of the None-tolerant lookups in this block is that a missing sidecar (or a
# missing flag on it) reports as NAMED ASSERTIONS rather than as a traceback: an exception
# escaping _selftest() exits non-zero with the right verdict and ZERO diagnostics, and it
# hides every other assertion the same mutation broke. Measured by Desmond Oyelaran while
# wiring this selftest into the render-golden stage: deleting the --cookie-name emission gave
# rc 1 via an uncaught StopIteration and printed not one `SELFTEST FAIL:` line. Provoked
# again here with the sidecar removed entirely, which raised IndexError at this line.
side = fa_containers[1] if len(fa_containers) > 1 else {"args": [], "env": [], "ports": []}
check(
"--upstream=http://127.0.0.1:9117" in side["args"],
"sidecar upstream must be the co-located app UI over http on 127.0.0.1:<uiport>",
@@ -2743,12 +2826,101 @@ def _selftest() -> int:
"sidecar must declare BOTH the http and metrics container ports — an undeclared port "
"cannot be named by a Service port or a PodMonitor, so the flag alone is inert",
)
# API ROUTES (schema 1.9.0). Three properties, and the FIRST is the one that keeps the field
# honest: an app that declares nothing must render NO --api-route. `fa` above declares none.
check(
not any(a.startswith("--api-route") for a in side["args"]),
"a forward-auth app that declares no auth.apiRoutes must render NO --api-route. A "
"default or a renderer constant would answer PiHole's page loads with 401 — PiHole "
"serves no /api/ at all — so the service would look permanently broken to a "
"logged-out operator instead of signing them in",
)
# Same `next(..., None)` discipline as _cookie_name() below, for the same reason: if the
# sidecar stops being rendered at all, this must report as a named assertion rather than as a
# StopIteration that swallows every other SELFTEST FAIL line in the run.
_faapi_dep = next(
(
d[1]
for d in render(
{
"name": "faapi",
"source": {"image": "a:1"},
"ingressRoutes": [{"host": "faapi.home.keithjackson.dev", "port": 9119}],
"auth": {
"mode": "forward-auth",
"groups": ["media-users"],
"apiRoutes": ["^/api/", "^/signalr/"],
},
}
)
if d[1]["kind"] == "Deployment"
),
None,
)
fa_api_side = next(
(
c
for c in ((_faapi_dep or {}).get("spec", {}).get("template", {})
.get("spec", {}).get("containers") or [])
if c.get("name") == "oauth2-proxy"
),
None,
)
check(
fa_api_side is not None,
"a forward-auth app declaring auth.apiRoutes rendered NO oauth2-proxy sidecar, so the "
"routes have nowhere to land and the app is not gated at all (R-IDP-02)",
)
check(
fa_api_side is not None
and [a for a in fa_api_side["args"] if a.startswith("--api-route")]
== ["--api-route=^/api/", "--api-route=^/signalr/"],
"a declared auth.apiRoutes must render as one repeatable --api-route per entry, in "
"declared order, and NOTHING else. Note the flag is SINGULAR: --api-routes does not "
"exist in v7.8.1 and the proxy refuses to start with it",
)
# BREADTH. The renderer must REFUSE a regex that would match a page load, not merely
# discourage one, because the schema stage can be skipped and render-golden cannot.
for _bad in ("^/", "^/.*", ".*", "/api/", "^/api/|^/"):
try:
render(
{
"name": "fabad",
"source": {"image": "a:1"},
"ingressRoutes": [{"host": "fabad.home.keithjackson.dev", "port": 9120}],
"auth": {
"mode": "forward-auth",
"groups": ["media-users"],
"apiRoutes": [_bad],
},
}
)
_refused = False
except SystemExit:
_refused = True
check(
_refused,
f"the renderer ACCEPTED auth.apiRoutes entry {_bad!r}. A too-broad --api-route "
f"answers a plain page load with 401, so a logged-out operator sees a bare error "
f"page and is never offered a sign-in — the service looks permanently broken "
f"rather than merely unauthenticated, which is a worse failure than the 302 bug "
f"this flag exists to fix",
)
for _good in ("^/api/", "^/signalr/", "^/api/v3/.*", "^/api/v2.0/server/update$"):
check(
bool(OAUTH2_API_ROUTE_PATTERN.match(_good)),
f"the breadth ratchet rejected the legitimate route {_good!r}. A ratchet that "
f"blocks real API prefixes is a ratchet someone eventually deletes",
)
# 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(
# Same None-tolerant shape as the `faapi` fixture above and for the same reason: a missing
# sidecar must surface as a named assertion, never as a traceback that suppresses the
# collected SELFTEST FAIL lines.
_fa2_dep = next(
(
d[1]
for d in render(
{
@@ -2759,24 +2931,55 @@ def _selftest() -> int:
}
)
if d[1]["kind"] == "Deployment"
)["spec"]["template"]["spec"]["containers"]
if c["name"] == "oauth2-proxy"
),
None,
)
fa2_side = next(
(
c
for c in ((_fa2_dep or {}).get("spec", {}).get("template", {})
.get("spec", {}).get("containers") or [])
if c.get("name") == "oauth2-proxy"
),
{"args": []},
)
# `next(..., None)` — the DEFAULT IS LOAD-BEARING, not defensive style. With a bare
# `next(genexpr)` this raises StopIteration the moment a sidecar renders no --cookie-name,
# which is precisely the mutation this assertion exists to catch. The traceback then escapes
# _selftest() before its collected `SELFTEST FAIL:` lines are printed, so the run exits
# non-zero with the RIGHT VERDICT AND NO DIAGNOSTICS — and it masks every other assertion the
# same mutation broke. Measured by Desmond Oyelaran while wiring the selftest into the
# render-golden stage: deleting the --cookie-name emission gave rc 1 via an uncaught
# StopIteration and suppressed all of the messages. A missing flag must report AS AN
# ASSERTION, with its name in the output, which is what the None + explicit check below do.
def _cookie_name(container):
return next(a for a in container["args"] if a.startswith("--cookie-name="))
return next((a for a in container["args"] if a.startswith("--cookie-name=")), None)
_fa_cn, _fa2_cn = _cookie_name(side), _cookie_name(fa2_side)
check(
_cookie_name(side) != _cookie_name(fa2_side),
_fa_cn is not None and _fa2_cn is not None,
f"a forward-auth sidecar rendered NO --cookie-name at all (fa={_fa_cn!r}, "
f"fa2={_fa2_cn!r}). Omitting the flag is not neutral: oauth2-proxy then claims its "
f"built-in default `_oauth2_proxy`, which every other instance on the platform also "
f"claims, so they overwrite and ClearSession-delete each other's sessions",
)
check(
_fa_cn != _fa2_cn,
"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)",
)
# Index-safe for the same reason as `side` above: an absent sidecar has no env, and an
# IndexError here would exit non-zero with no diagnostics at all.
_fa_env = (side.get("env") or [{}])[0]
check(
side["env"][0]["valueFrom"]["secretKeyRef"]["name"] == "fa-oauth2-proxy",
(((_fa_env.get("valueFrom") or {}).get("secretKeyRef") or {}).get("name")
== "fa-oauth2-proxy"),
"sidecar creds must come by name from the per-app <app>-oauth2-proxy Secret (R-IDP-04)",
)
fa_svc = next(d[1] for d in fa_docs if d[1]["kind"] == "Service")
fa_svc_ports = {p["port"] for p in fa_svc["spec"]["ports"]}
fa_svc = next((d[1] for d in fa_docs if d[1]["kind"] == "Service"), None)
check(fa_svc is not None, "a forward-auth app must render a Service for the Gateway to reach")
fa_svc_ports = {p["port"] for p in (((fa_svc or {}).get("spec") or {}).get("ports") or [])}
check(
fa_svc_ports == {9117, 4180},
"forward-auth Service must expose the app port (east-west) AND :4180 (gateway->sidecar)",