feat(auth): declare API paths per service, so --api-route can ship at all #34
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Home-Network/home-network-k3s-impl#34
Reference in new issue
Block a user
Second half of #28, which 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, and I have preserved that reasoning rather than worked around it.The defect
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 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 — 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, on two apps and two different paths — which is the whole argument:
/api/paths, and 4 of 6 POSTs answered by the proxy and never reaching Jackett (access-log upstream field-, not/)radarr-7d55cbffb9-42hmh, 0 restarts)POST /signalr/messages/negotiate, browser UA, 302, upstream-That Radarr number is the live proof this change is needed: a left-open browser tab whose session the cookie rename orphaned, polling SignalR forever, silently redirected every time, and never re-prompting the operator. 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.
So radarr's broken path is
/signalr/, jackett's is/api/, and PiHole serves no/api/at all. One function renders all five sidecars, so the cheap fix is a renderer constant — and a constant applies one app's measurement to four apps nobody measured. Theauthblock wasadditionalProperties: false, so there was no per-app hook. Creating the hook is the change; setting the flag is the easy part.What this does
schemas/servicedefinition.schema.json(1.8.0 → 1.9.0)auth.apiRoutes: a list of anchored path regexes,uniqueItems, default[]; plus the invariant that a non-empty list requiresmode: forward-authci/l0.shvalidatescatalog/*.yamlagainst the schema on every gate runcatalog/render.py--api-route=<r>per entry, in order; raises on an unanchored entryrender-goldenruns on every gate invocation even when the schema stage skipsconformance/tests/l0-static/oauth2-proxy-api-routes-per-app/(new)The
authblock staysadditionalProperties: false— one named, documented key, not an opened door. That property is what made "there is no override hook" a fact rather than a guess for the two engineers who hit it before me.A list, not a single string, because Radarr needs two and
--api-routeis natively repeatable. A single string would have forced alternation into the value, and alternation is how breadth gets smuggled past a prefix-only reading of a regex.The flag is
--api-route, singular — verified against the pinned binary rather than taken from the brief:podman run --rm quay.io/oauth2-proxy/oauth2-proxy:v7.8.1 --help→--api-route strings return HTTP 401 instead of redirecting to authentication server if token is not valid. Format: path_regex. There is no--api-routesin the help output.Which apps, and the measurement behind each
auth.apiRoutes^/api/^/api/,^/signalr/^/signalr/is my measurement of the live post-fix pod (232/232).^/api/is #30's measurement of radarr's pre-fix pod (62 ×/api/v3/command, 46 ×/api/v3/queue/details) — this app's own evidence, by another engineer, not an analogy. The current pod has logged zero/api/traffic, so I did not re-measure it./api/; it is the reason the field is per-appSonarr and qBittorrent were measured and deliberately left alone. Sonarr is nearly Radarr's codebase, which is exactly what makes
^/api/+^/signalr/a tempting guess and still a guess. Nobody has opened either UI since #28 rolled, so their logs are empty — evidence of no traffic, not of no need. Each catalog file now carries a comment saying so, with the numbers and what to measure first.Breadth is the dangerous direction, and it is not symmetric
A missing
--api-routeis a silent 302 — bad, invisible, recoverable. A too broad one answers a plain page load with 401: a logged-out operator gets a bare error page and is never offered a sign-in, so the service looks permanently broken and no reload recovers it. That is a worse outcome than the defect the flag fixes.So every entry must match
^\^/[A-Za-z0-9._~/-]+(\.\*|\$)?$— anchored at^/, then literal path characters. Rejected and all five provoked:^/,^/.*,.*,/api/(unanchored),^/api/|^/. Alternation is refused rather than supported, because the flag is repeatable so one entry per route costs nothing. The same pattern lives in all three surfaces and assertion (5) of the new check fails if they drift apart — if they do, the loosest wins silently.Goldens
Two goldens change, not five, and that is the per-app property proving itself on the real tree:
pihole,qbittorrentandsonarrare byte-identical tomain. No L1 fixture needed regenerating — all threeconformance/tests/l1-rendering/*/fixtures/svcdef*.yamlare forward-auth with noapiRoutes, so they render no flag. #28 touched 5 goldens and 3 fixtures because it changed flags on every sidecar; this changes flags only where a measurement says so.The new L0 check, and why it is a sibling rather than an extension
Not folded into
oauth2-proxy-cookie-names-distinct. That check's subject is cookie identity — its inputs are names and domains, its failure mode is mutual session eviction, its register is the shared proxy's claim on the default name.--api-routeshares none of those, so bundling them would make its name a lie and let a future api-route decision be expressed only by weakening a cookie check. That is #28's own stated discipline, applied to my change rather than quietly dropped.And "nowhere" was wrong for exactly one reason:
render-goldencompares 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, i.e. render-golden is happy, and the new check exits 1 with 15 failures naming pihole, qbittorrent, radarr and sonarr individually. Assertion (2) compares each rendered sidecar against its own ServiceDefinition, which is the only reading that fails it.Tested in both directions on throwaway trees outside the repo, never by mutating the branch:
git archive origin/main+ the test)--api-route=^/items.patterndeletedDeclares
conformance.home.k3s/scope: whole-repo, soci/l0.sh's--include-scopeunion runs it on every gate invocation. No bespoke stage was added —whole-repo-invariants-wiredwould fail that. Selection confirmed: it is listed among 21 tests (was 20) with a requirement filter that matches nothing.Also folded in: the selftest could exit 1 with ZERO diagnostics
Not originally mine — @Desmond found it while wiring
catalog/render.py --selftestinto therender-goldenstage (#33, which touches onlyci/render-golden.shand is not in this base;origin/mainatcdf3e14drives the selftest from no stage, which I checked rather than assumed). The coordinator asked me to fold the fix in because the defect is in the file I own._selftest()reached_cookie_name()through a barenext(genexpr)with no default. Delete the--cookie-nameemission — exactly the mutation that assertion exists to catch — and it raisesStopIteration, which escapes_selftest()before its collectedSELFTEST FAIL:lines are printed. Right verdict, no diagnostics, and it masks every other assertion the same mutation broke.Provoking it exposed three more instances of the same failure class in the same block, which is why this is not literally one line:
side = fa_containers[1],side["env"][0][...]and thefa_svclookup all raise rather than assert when the sidecar is absent entirely. All four are now None-tolerant, and the two fixtures I added for--api-routewere written that way from the start.--cookie-nameemission deletedSELFTEST FAILlines, 1StopIterationSELFTEST FAILlines, 0 tracebacksIndexErrorSELFTEST FAILlines, 0 tracebacksGoldens are unchanged by this part; it touches
_selftest()only. The two PRs are independent but complementary: #33 makes the stage real, this makes its output readable when it fires. Worth noting for reviewers of #33 —.gitea/workflows/gate.yml:138runs./ci/gate.sh || [ "$?" -eq 3 ], so rc 1 is not tolerated in CI the way rc 3 is.Not in this PR, deliberately: the metrics-ingress NetworkPolicy
@Ines verified that #28's metrics listener genuinely serves (port-forward:
http=200, 10,208 bytes,oauth2_proxy_requests_total{code="302"} 2) but that the scrape will be dropped — nsjackett's default-deny has noingresskey at all, andjackett-allow-gateway-ingressadmitsfromEntities: [ingress]only. The fix belongs in each app's own rendered policy, i.e. in my file and my goldens, and the coordinator asked whether to bundle it. My answer is: queue it as the next PR, not drop it. Reasons, in the order that decided it:observability.metricsEndpointalready renders the monitoring-namespace ingress allow. What the model cannot express is a second metrics endpoint belonging to a sidecar the renderer injected rather than to the declared workload. Keying a bespoke policy off the pod labelauth-mode: forward-authworks today but hardcodes "forward-auth implies a metrics port on 44180" instead of deriving it — the "list, don't derive" anti-patternoauth2-proxy-cookie-names-distinctassertion (5) forbids, introduced one directory from the check that forbids it. Worth arguing about on its own.--api-routeheadline, least of all in a PR that already needs a human for a different sensitive-path reason.decisions/README.mdwas two branches appending to the same hunk.wget: download timed outon:44180and:4180, not the Flux sibling ordering. Shipping a security-adjacent rule on wholly inherited evidence as a footnote is what §2b exists to stop.So: the sidecars' metrics are still unscrapable and this PR does not change that. Nobody should read it otherwise.
Gate
Toolbox run (
DEV_KUBECONFIG=/dev/null+ anENGINEshim mounting the main checkout's.gitand the sibling spec repo at/home-network-k3s):./ci/gate.sh→ rc 3 — UNDETERMINED, NOT green.spec-immutability,decisions-log,lint,l0,render-golden,l1,approval-policygitea-settings(noSETTINGS_API_TOKENlocally) and L2/L3 conformance (CLUSTER_AVAILABLEunset)R-DNS-03 R-GITOPS-02 R-IDP-02 R-IDP-06 R-NET-01 R-NET-05 R-NET-06 R-NET-07 R-PLAT-02 R-TEST-01 R-TEST-02 R-TEST-03 R-TEST-04spec-immutabilityis green, not skipped.catalog/render.py --selftest: PASS.CI, read from
/statusesrather than from a tickBoth contexts report, and only one of them runs. Identical on
99560c2d,df1b25b6andf1d16f44:That second line is §2b's trap in the wild — a skipped Gitea job reports
state=success, so its green tick certifies nothing. Read the description, not the colour.The duration is a range on purpose — it differs by a second or two per run, and the load-bearing fields are the state and the description, which are stable across all three. Worth recording that the same context changed its answer over time: on the first push it briefly read
state=pending desc='Blocked by required conditions', still waiting on the gate job it depends on, and resolved tosuccessonly once that settled. Sample a status once and you can see either a pending or a green tick for a job that never executed.And a correction worth more than the rest of this section: my local
approval-policy: greenwas meaningless.ci/approval-policy.sh:47readsGITHUB_EVENT_NAME, unset outside CI, so it short-circuits to"not a pull_request event (event='none') — approval policy N/A". A clean local gate structurally cannot catch a sensitive-path problem. Re-running withGITHUB_EVENT_NAME=pull_request PR_NUMBER=34makes it evaluate and fail closed on the missing local token, while every other stage stays green — which bounds the CI failure to that one stage:I could not read CI's own job log: this Gitea answers 404 on
/actions/runsand on/actions/jobs/{task_id}/logs, leaving only/actions/tasks. So attribution of the CI red toapproval-policyrests on that local reproduction plus the sensitive-path rule, not on CI's log — weaker evidence than #28 had, and named rather than papered over.Branch protection re-measured live (§2b), not inherited from CLAUDE.md's table:
protected=True,user_can_push=False,enable_status_check=True,status_check_contexts=['gate / gate (pull_request)'],required_approvals=1; runnerid=1 incluster-dind status=online. I did not provoke a rejected push tomain, souser_can_pushis the API's word only.Two things to expect, both correct behaviour
approval-policywill FAIL this PR in CI. The diff touchesconformance/**, whichinfra/gitea/approval-policy.jsonlists as sensitive — "the tests and the harness; changing them changes what green MEANS" — so it is fail-closed pending a non-author approving review. #28 was blocked the same way. That is CLAUDE.md §1 working, not a defect.required_approvals=1applies to every PR here regardless of path, so this needs a human either way.And read descriptions, not colours: a skipped Gitea job reports
state=successwith the description"Has been skipped". Both status lines are quoted in the handoff entry once CI reports.What this does NOT establish
No browser round-trip, by anyone, for this flag. The 302/401 split, the dropped POST bodies and the orphaned SignalR poll are all read from access logs; that a 401 repairs the operator's experience is reasoned from HTTP semantics, not watched. Also:
^/api/on radarr is #30's measurement, not mine; sonarr and qBittorrent are unmeasured, not cleared;--cookie-refresh=2m30sis still unexercised (0AuthSuccesson all five pods); nothing was applied — cluster access was read-only throughout. Full list indecisions/2026-10-01-oauth2-proxy-api-route-per-app.md.Merging rolls jackett and radarr only. No cookie flag changed, so nobody is signed out — unlike #28. Both use
strategy: Recreateon RWO Longhorn volumes, so there is a brief gap per app, and radarr/sonarr lose direct:9117access to Jackett while its pod restarts.The human test — how to tell a too-broad regex from a working one
Do step 1 first, before anything else. From a signed-out private window, open
https://radarr.home.keithjackson.dev/andhttps://jackett.home.keithjackson.dev/UI/Dashboard. Each must bounce through Keycloak and land on the app's UI. If either shows a bare401 Unauthorizedinstead of a sign-in, the regex is too broad — stop and revert. That is the one failure this change can introduce, and it is the only test that can see it: a working regex is invisible on a page load, a broken one is total and immediate.signalr. Those negotiate calls are 302 today and the UI never complains; they must now be 200 while signed in._oauth2_proxy_radarrcookie and let the tab poll — negotiate must answer 401 and Radarr must show a problem. Then reload:/matches neither regex, so it must sign you back in silently. That round trip is the entire fix — fail loudly on the API, recover on the page load.--cookie-refresh=2m30spast the 300 s cliff. A failure there points at the realm's refresh/SSO-idle timeouts, not at this change.🤖 Generated with Claude Code
df1b25b680tof1d16f44b8f1d16f44b8to2983378496