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

Merge pull request 'feat(auth): declare API paths per service, so --api-route can ship at all' (#34) from feat/oauth2-proxy-api-route-per-app into main
gate / gate (push) Has been cancelled
Details
gate / conformance (push) Has been cancelled
Details

Browse Source 
Reviewed-on: #34
Reviewed-by: Keith Jackson <[email protected]>
This commit was merged in pull request #34.
This commit is contained in:
jakman85 committed 2026-10-02 04:31:56 +00:00
2 parents 8214642d87 2983378496
commit 8c12a1e525
13 files changed
+1387 -17
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

decisions/2026-10-01-oauth2-proxy-api-route-per-app.md
+482
View File
@@ -0,0 +1,482 @@
---
date: 2026-10-01
branch: feat/oauth2-proxy-api-route-per-app
title: The config model had no way to say "this app's API lives here" — so --api-route could not ship. Now it can, per app.
---
## What changed
Second half of PR #28 (`decisions/2026-09-30-oauth2-proxy-cookie-collision-fleet.md`), which
fixed the cookie collision, the 5-minute session and the missing metrics listener in one renderer
function and **deliberately declined `--api-route`** for a reason I have preserved rather than
worked around: that flag was measured on **one** app, and `PiHole serves no /api/ at all`, so a
renderer-wide constant would apply one app's path shape to five. The `auth` block is
`additionalProperties: false`, so there was no per-app override hook. **Creating the hook was the
work; setting the flag is the easy part.**
**Three surfaces, in order of what they protect:**
| Surface | Change | What it guards that the others cannot |
|---|---|---|
| `schemas/servicedefinition.schema.json` (**1.8.0 → 1.9.0**) | new optional `auth.apiRoutes`: array of anchored path regexes, `uniqueItems`, default `[]` | rejects a too-broad regex **where a human types it** — `ci/l0.sh` validates `catalog/*.yaml` against this schema on every gate run |
| `catalog/render.py` | emits one `--api-route=<r>` per declared entry, in order; **raises** on an unanchored entry | `ci/render-golden.sh` runs on every gate invocation even when the schema stage skips |
| `conformance/tests/l0-static/oauth2-proxy-api-routes-per-app/` (new) | cross-checks each rendered sidecar against its own ServiceDefinition | catches a **renderer constant**, which neither of the above can see |
**The `auth` block stays `additionalProperties: false`.** One named, documented key was added; the
door is not opened. That property is the reason the gap existed, and it is worth keeping — it is
what made "there is no override hook" a *fact* rather than a *guess* for the two engineers who hit
it before me.
**Rebased mid-flight.** `origin/main` advanced from `08deb20` to `cdf3e14` while I was working
(PR #27 and PR #32, the `hardware.gpu: intel-quicksync` capability). **That change also took schema
`1.8.0`**, so mine is renumbered to **1.9.0** and chained behind it; both version entries are
present in the description, and `catalog/render.py` had a two-sided conflict in the constants block
which is resolved by keeping both (the `DRI_*` constants and `OAUTH2_API_ROUTE_PATTERN` are
unrelated). Re-rendering after the rebase still changes **only** jackett and radarr — plex's new
golden reproduces byte-identically, so the two changes do not interact.
**Shape:** a **list** of regexes, not a single string, because Radarr needs two
(`^/api/` and `^/signalr/`) and `--api-route` is natively repeatable. A single string would have
forced alternation into the value, and alternation is exactly how breadth gets smuggled past a
prefix-only reading of a regex.
**The anchoring ratchet.** Each entry must match `^\^/[A-Za-z0-9._~/-]+(\.\*|\$)?$` — anchored at
`^/`, then literal path characters, with an optional trailing `.*` or `$`. The same pattern is in
the schema, in the renderer, and in the new check, and assertion (5) of the check **fails if they
drift apart**, because if they do the loosest one wins silently. Rejected shapes, all five
provoked: `^/`, `^/.*`, `.*`, `/api/` (unanchored), `^/api/|^/` (alternation). Alternation is
**refused rather than supported**: the flag is repeatable, so one entry per route costs nothing.
**Why breadth is the dangerous direction, and it is not symmetric.** A *missing* `--api-route` is 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, which is why it is gated on three surfaces rather than documented in one.
### Also folded in: the selftest could exit 1 with ZERO diagnostics
Not my change originally — Desmond Oyelaran found it while wiring `catalog/render.py --selftest`
into the `render-golden` stage (his PR #33, which touches only `ci/render-golden.sh` and is **not**
in my base; `origin/main` at `cdf3e14` does not yet drive the selftest from any gate stage). The
coordinator asked me to fold the fix into this PR because the defect is in the file I own.
`_selftest()` reached `_cookie_name()` through a bare `next(genexpr)` with **no default**. Delete
the `--cookie-name` emission — exactly the mutation that assertion exists to catch — and it raises
`StopIteration`, which escapes `_selftest()` **before its collected `SELFTEST FAIL:` lines are
printed**. Right verdict, no diagnostics, and it masks every other assertion the same mutation
broke. Fixed with `next(..., None)` plus an explicit `check()` so a missing flag reports **as a
named assertion**.
Provoking it turned up **three more instances of the same failure class** in the same block, which
is why this is not literally a one-line change: `side = fa_containers[1]`,
`side["env"][0][...]`, and the `fa_svc` lookup 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-route`
were written the same way from the start. Measured, on throwaway copies:
| Mutation | Before | After |
|---|---|---|
| `--cookie-name` emission deleted (origin/main's renderer) | rc 1, **0** `SELFTEST FAIL` lines, 1 `StopIteration` | rc 1, **3** `SELFTEST FAIL` lines, 0 tracebacks |
| the oauth2-proxy sidecar not appended at all | rc 1, **0** lines, `IndexError` | rc 1, **20** `SELFTEST FAIL` lines, 0 tracebacks |
Note the consequence of his stage change for anyone reading this later: once PR #33 lands,
`.gitea/workflows/gate.yml:138` runs `./ci/gate.sh || [ "$?" -eq 3 ]`, so **rc 1 is not tolerated
in CI the way rc 3 is** — a selftest failure will block. Which is the right outcome, and it is why
a selftest that prints nothing was worth fixing before that lands rather than after.
### Declined, with reasons: the metrics-ingress NetworkPolicy
The coordinator asked for my judgement on bundling Ines Dubois's finding — that #28's metrics
listener genuinely serves (she measured `http=200`, 10,208 bytes,
`oauth2_proxy_requests_total{code="302"} 2` via port-forward) but the scrape will be **dropped**,
because namespace `jackett`'s default-deny has **no `ingress` key at all** and
`jackett-allow-gateway-ingress` admits `fromEntities: [ingress]` only. The fix belongs in each
app's own rendered policy, i.e. in my file and my goldens. **I declined to bundle it**, and asked
for it as the immediate next PR. Five reasons, in the order that decided it:
1. **The real reason is that it is a MODEL question, not a policy to bolt on.** Schema 1.6.0
already owns this: `observability.metricsEndpoint` renders the Service port **and** the
monitoring-namespace ingress allow. What the model cannot currently express is a **second**
metrics endpoint on the same pod belonging to a *sidecar the renderer injected*, rather than to
the workload the operator declared. Keying a bespoke policy off the pod label
`homelab.keithjackson.dev/auth-mode: forward-auth` works today, but it hardcodes "forward-auth
implies a metrics port on 44180" into a policy instead of **deriving** it from the sidecar
declaration that is already the single source of that port — the exact "list, don't derive"
anti-pattern `oauth2-proxy-cookie-names-distinct` assertion (5) exists to forbid, and which I
would be introducing one directory away from the check that forbids it. That deserves its own
review, and it is squarely the kind of decision I was brought in to make deliberately.
2. **Different subject, and it is a trust-boundary change.** Mine is the auth *response code* on
API paths (R-IDP-02). That is an ingress allow (R-NET-01/02, R-OBS-01) admitting a new
namespace to a new port. Bundling a security-relevant ingress rule into a PR whose headline is
`--api-route` is how such a rule gets waved through on the strength of the other half — and this
PR already needs a human for a *different* sensitive-path reason, so the second subject would be
reviewed in the shadow of the first.
3. **The conflict argument does not actually apply to my diff.** It assumes I am regenerating five
goldens; **I am regenerating two** (jackett, radarr), because the flag renders only where
declared. A metrics-ingress policy renders a *different document* — a CiliumNetworkPolicy — in
all five. Different hunks, in files git merges independently; and goldens regenerate
deterministically from the renderer, so the follow-up rebases onto `main` cleanly. The N-1
rebase problem `decisions/README.md` records was two branches appending to **the same hunk**,
which this is not. I checked my own diff before answering rather than accepting the premise.
4. **Every fact in the finding is hers and I have measured none of it** — not the 200/10,208 bytes,
not the `wget: download timed out` on `:44180` *and* `:4180`, not the Flux sibling ordering that
makes `oauth2-proxy` and `apps-wave-0` unordered. Shipping a security-adjacent rule on wholly
inherited evidence, as a footnote in someone else's PR, is precisely what CLAUDE.md §2b exists
to stop. In its own PR I would measure it first.
5. **Sequencing: the PodMonitor is in HER PR #29, not mine.** If I ship the policy and her
PodMonitor is reworked or delayed, I have opened an ingress allow for a scraper that selects
nothing — a hole with no consumer. The policy is only useful paired with the PodMonitor, which
argues for it landing adjacent to that, not here. It also drags in a
`KNOWN_EXTERNAL_MONITORS` entry in `conformance/tests/l0-static/servicemonitor-ports-resolve/`,
a second conformance interaction on top of the new check this PR already adds.
Diff size is the tiebreaker rather than the argument: **1,245 insertions across 13 files**,
including a ~400-line new conformance check. I would rather hand a reviewer one subject they can
actually falsify. **Queued, not dropped** — and if the next person takes it, the model note in (1)
is the part worth arguing about, not the YAML.
### Which apps got which routes, and the measurement behind each
| App | `auth.apiRoutes` | Evidence |
|---|---|---|
| **jackett** | `^/api/` | 2026-09-30, its own access log: **33 × 302 against 2 × 401** on `/api/` paths, and **4 of 6 POSTs answered by the proxy and never reaching Jackett** (upstream field `-`, not `/`), including both `/api/v2.0/server/update` calls |
| **radarr** | `^/api/`, `^/signalr/` | `^/signalr/`: measured **by me, on the live post-fix pod** — see below. `^/api/`: measured on radarr by the PR #30 author, on the **pre-fix** pod |
| **pihole** | *(none — deliberate)* | serves no `/api/`; **0** unauthenticated requests in its current pod's 23 h |
| **sonarr** | *(none — not measured)* | 13,643 log lines, **0** unauthenticated requests. No traffic, not a negative result |
| **qbittorrent** | *(none — not measured)* | 13,649 log lines, **0** unauthenticated requests. Same |
**`^/signalr/` on radarr is the live proof this change was needed**, and it is the strongest
evidence in this entry. 23 h after PR #28 deployed, on `radarr-7d55cbffb9-42hmh` (0 restarts), the
sidecar logged **232 unauthenticated requests across the pod's entire lifetime, and all 232 are the
same call**: `POST /signalr/messages/negotiate`, browser UA, **302**, upstream `-` (Radarr never saw
one of them). That is a **left-open browser tab** whose session the cookie rename orphaned, polling
SignalR forever, silently redirected every time, and **never re-prompting the operator**. It 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. With 401 the UI's connection breaks **loudly**; the
operator reloads; `/` matches neither regex, so the reload signs them back in.
**`^/api/` on radarr: included, and here is the honest basis.** The current pod has logged **zero**
`/api/` traffic, so I did **not** re-measure it. It rests on a measurement of **radarr** recorded in
the merged PR #30 handoff — its 156 unauthenticated requests were `/api/v3/command` (62),
`/api/v3/queue/details` (46) and `/signalr/messages/negotiate` (43), i.e. **108 of 156 were
`/api/v3` XHRs**. That is this app's own evidence, by another engineer, not an analogy to another
app. I include it because `/api/v3/` is a pure JSON namespace a browser never navigates to (Radarr
serves its UI at `/`, assets under `/Content/` and `/MediaCover/`), so the breadth risk is nil and
the measured benefit is 70% of that app's rejection volume.
**Sonarr and qBittorrent were measured and deliberately left alone.** Sonarr is nearly the same
codebase as Radarr, which is exactly what makes `^/api/` + `^/signalr/` a tempting guess and still a
guess. I read both pods' logs: **zero** unauthenticated requests, nothing but health probes, because
nobody has opened either UI since the cookie fix rolled. **That is evidence of no traffic, not
evidence of no need.** Each catalog file now carries a comment saying so, with the numbers and what
to measure before adding the routes.
### Goldens and fixtures regenerated
Rendered with the canonical command
(`python3 catalog/render.py --all catalog -o apps --wave-layout --no-image-automation`).
**Two goldens changed, not five** — and that is the per-app property proving itself on the real
tree rather than in a fixture:
apps/wave-0/jackett.yaml + --api-route=^/api/
apps/wave-1/radarr.yaml + --api-route=^/api/
+ --api-route=^/signalr/
`apps/wave-0/pihole.yaml`, `apps/wave-0/qbittorrent.yaml` and `apps/wave-1/sonarr.yaml` are
**byte-identical to `main`**. **No L1 fixture needed regenerating**: all three
`conformance/tests/l1-rendering/*/fixtures/svcdef*.yaml` declare `auth.mode: forward-auth` with no
`apiRoutes`, so they render no flag and their goldens are unchanged. PR #28 touched 5 goldens and 3
L1 fixtures because it changed flags on *every* sidecar; this one changes flags only where a
measurement says so, which is the whole point.
### The L0 decision (and it is a decision, not a default)
**Not** in `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-route` shares none of those. Bundling them would
make that check's name a lie and couple two unrelated ratchets, so a future api-route decision could
only be expressed by weakening a *cookie* check. That is PR #28's own stated discipline, applied to
my change rather than quietly dropped.
**A sibling check, `oauth2-proxy-api-routes-per-app`** (whole-repo scope, so it runs on every gate
invocation via `ci/l0.sh`'s `--include-scope` union — no bespoke stage, which
`whole-repo-invariants-wired` would fail). **Nowhere was the wrong answer for exactly one reason:**
the existing gates cannot see a renderer constant. `render-golden` compares the render to the
goldens, so a constant **plus regenerated goldens agree with each other while both being wrong** —
I provoked that and it regenerates with **rc 0**. The new check's assertion (2) compares each
rendered sidecar against its **own ServiceDefinition**, which is the only reading that fails it.
## What I verified
- **`--api-route` is SINGULAR, read off 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-routes`** in the help output. The PM asked
me to verify rather than trust, and that is the result — the singular form is confirmed and the
help text also confirms the *semantics* I am relying on (401 instead of a redirect).
- **The premise, in-cluster, read-only, through the pinned toolbox, `--context k3s-prod` named
explicitly** (CLAUDE.md §2b and §5). PR #28 **is** deployed: all five sidecars carry
`--cookie-name=_oauth2_proxy_<app>`, `--cookie-domain=<app>.home.keithjackson.dev`,
`--cookie-refresh=2m30s` and `--metrics-address=0.0.0.0:44180`. Pods, all **0 restarts**:
`jackett-6877fcd9f5-ht9d9`, `pihole-…`, `qbittorrent-5547bdb9bd-zq557`,
`radarr-7d55cbffb9-42hmh`, `sonarr-ddf485775-8ckwt` (started 2026-09-30T21:41–21:42Z).
- **The full post-fix log census, parsed rather than grepped** (oauth2-proxy's access-log format,
with `/ping` health probes excluded), over each pod's whole life:
app lines unauth AuthSuccess removing session cookie sig invalid
jackett 13,649 0 0 0 0
pihole 13,649 0 0 0 0
qbittorrent 13,649 0 0 0 0
radarr 14,099 232 0 0 0
sonarr 13,643 0 0 0 0
Three findings, kept separate:
1. **The collision fix works.** Jackett was 39 unauthenticated before; it is **0** in 13,612
non-probe lines. The eviction fingerprint (`removing session` / `cookie signature not valid`)
is **0 on all five**, where radarr previously had 8 and 8.
2. **Radarr's 232 are one request.** 100% `POST /signalr/messages/negotiate`, 100% **302**, 100%
upstream `-`, browser UA (`Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537…`). **Zero `/api/`
traffic of any kind** on this pod.
3. **0 `AuthSuccess` anywhere**, so nobody has signed in since the restart and `--cookie-refresh`
is **still** unexercised — unchanged from PR #28's own caveat.
- **Only two goldens change, and three are byte-identical to `main`.** `git status` after the
canonical render lists exactly `apps/wave-0/jackett.yaml` and `apps/wave-1/radarr.yaml`.
`render-golden` is green.
- **`catalog/render.py --selftest`: PASS**, with new assertions that (a) the `fa` fixture, which
declares nothing, renders **no** `--api-route`; (b) a fixture declaring two routes renders exactly
those two, in order; (c) the renderer **refuses** all five breadth shapes; (d) four legitimate
routes (`^/api/`, `^/signalr/`, `^/api/v3/.*`, `^/api/v2.0/server/update$`) are **accepted** — a
ratchet that blocks real API prefixes is a ratchet someone eventually deletes.
- **The new check: rc 0, 43 assertions, 6 sidecars audited**, with its inventory printed so the
reviewer can see what it read:
`jackett -> ['^/api/'] | pihole -> (none) | qbittorrent -> (none) | radarr -> ['^/api/',
'^/signalr/'] | sonarr -> (none) | crafty -> (none)`.
- **The check was tested in BOTH directions, on throwaway copies outside the repo**, never by
mutating the branch. Each mutation fired its own assertion class and nothing else:
- **Pre-change tree** (`git archive origin/main` + the test dropped in): **rc 1, 11 failures** —
no service declares routes, the renderer emits none, the renderer does not read `auth.apiRoutes`,
the pattern is absent from both renderer and schema, and the shipped renderer accepts all five
breadth shapes.
- **pihole's golden hand-edited** to carry a route it never declared: **rc 1, 2 failures** (the
catalog↔golden mismatch, and "declares nothing must render nothing").
- **radarr's golden widened** to `--api-route=^/`: **rc 1, 2 failures** (unanchored, plus the
mismatch).
- **Renderer turned into a constant and EVERY golden regenerated to match: 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. **This is the case that justifies the
check's existence** and I provoked it rather than argued it.
- **Schema `items.pattern` deleted**: **rc 1, 1 failure** — the three-surface drift guard.
- **Unmutated control: rc 0.**
- **The selftest diagnostics fix was PROVOKED, not reasoned.** Both mutations above were run
against throwaway copies of the renderer outside the repo (`origin/main`'s `render.py` for the
"before" column, mine for the "after"), never by mutating the branch. The pre-fix StopIteration
and the post-fix 3-and-20 named assertions are quoted from those runs. `--selftest` on the
unmutated branch: **PASS**. Goldens after the fix: **unchanged** (it touches only `_selftest()`).
- **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-api-routes-per-app` among **21** tests (it was 20), i.e. it runs
even when the touched-requirement filter matches nothing.
- **THE SCHEMA GUARD IS PROVEN WITH THE PINNED VALIDATOR `ci/l0.sh` ACTUALLY DRIVES**, not by
reading the pattern. `./schemas/validate.sh` (`check-jsonschema, version 0.29.4`) against seven
throwaway ServiceDefinitions written outside the repo:
good ["^/api/", "^/signalr/"] rc 0
bad-root ["^/"] rc 1 apiRoutes ... does not match
bad-dotstar ["^/.*"] rc 1 apiRoutes ... does not match
bad-unanchored ["/api/"] rc 1 apiRoutes ... does not match
bad-alt ["^/api/|^/"] rc 1 apiRoutes ... does not match
bad-everything [".*"] rc 1 apiRoutes ... does not match
bad-mode ["^/api/"] on mode: oidc rc 1 const 'forward-auth'
So a too-broad regex is rejected **where a human types it**, on every gate run, by the real tool
— and the shape invariant fires too.
- **THE RENDERER'S REFUSAL REACHES `render-golden` AS A READABLE FAILURE, NOT A TRACEBACK.** With
`^/signalr/` swapped for `^/` in a *copy* of the catalog (outside the repo), both invocation
shapes exit **rc 1 with 0 tracebacks** — the single-SD form and `--all <dir> -o <dir>
--wave-layout --no-image-automation`, which is the one `ci/render-golden.sh` drives:
render.py: radarr: auth.apiRoutes entry '^/' is not an anchored path regex. Each entry
must start with '^/' and continue with literal path characters (e.g. '^/api/',
'^/signalr/', '^/api/v3/.*'). ... A too-broad --api-route answers a PAGE LOAD with 401, so
a logged-out operator sees a bare error page and is never offered a sign-in: the service
looks permanently broken rather than merely unauthenticated.
So the breadth guard is gated by `render-golden` even on a run where the schema stage skipped,
and the operator is told *why* rather than handed a stack trace.
- **The schema's new shape invariant.** A non-empty `auth.apiRoutes` requires
`auth.mode: forward-auth`, because the flag renders onto the per-app sidecar and no sidecar exists
for `oidc`/`native`/`none` — declaring it elsewhere would be a **silent no-op**, which is the
failure class this repo keeps paying for.
- **Gate:** toolbox `./ci/gate.sh` with the two-mount ENGINE shim — exit code and stage list in the
next section, because it is not a pass and that belongs there.
- **No mutating cluster access.** `kubectl get/logs/jsonpath` only, `--context k3s-prod` named on
every call. No `apply`/`edit`/`patch`, no `flux reconcile`. No Secret `.data` or `.stringData` was
read or printed; I did not need to touch a Secret at all this session. The one container I ran
locally was the pinned oauth2-proxy image, to read its `--help`.
- **`~/.git-credentials` was never read.** PR creation used `git credential fill`, which does not
echo the value.
## What I did NOT verify
- **NO BROWSER ROUND-TRIP. BY ANYONE. FOR THIS FLAG.** This is the most important line here. 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. Nobody has added a Jackett indexer or recovered a Radarr tab through this
configuration. The human test below is the first real evidence this change will ever have.
- **Local gate: rc 3 (UNDETERMINED), not 0.** Green: `spec-immutability`, `decisions-log`, `lint`,
`l0`, `render-golden`, `l1`. **Skipped:** `gitea-settings` (no `SETTINGS_API_TOKEN` locally) and
**L2/L3 conformance** (`CLUSTER_AVAILABLE` unset). Nothing FAILED; nothing runtime was certified
either. **rc 3 is UNDETERMINED, never green.**
- **MY LOCAL `approval-policy: green` WAS MEANINGLESS, AND I ALMOST REPORTED IT AS EVIDENCE.**
`ci/approval-policy.sh:47` reads `GITHUB_EVENT_NAME`, which is unset outside CI, so the stage
short-circuits at line 50: `ok "not a pull_request event (event='none') — approval policy N/A"`.
**A clean local gate therefore CANNOT catch a sensitive-path problem — structurally, not by
accident.** Flagged to me by the coordinator from Ines Dubois's round, and I verified it by
re-running the gate with `GITHUB_EVENT_NAME=pull_request PR_NUMBER=34`: the stage then
**evaluates** and fails closed on the missing token —
[fail] no SETTINGS_API_TOKEN — cannot read this PR's changed files or its reviews.
[fail] stage approval-policy: FAIL
[fail] approval-policy: a sensitive change lacks a non-author approval -> blocks merge
[fail] MERGE GATE: BLOCKED (rc 1, not 3)
— while **every other stage stays green** (`spec-immutability`, `decisions-log`, `lint`, `l0`,
`render-golden`, `l1`). That is the useful result: it bounds the CI failure to this one stage
rather than leaving "something is red" as the whole report. Fail-closed is correct here and
deliberate: unlike `gitea-settings`, where a missing credential is a lock-out, a skip of this
stage would be a bypass (CLAUDE.md §1).
- **CI, read from each commit's `/statuses` rather than from a tick.** Both contexts report, and
**only one of them runs**. Identical on `99560c2d`, `df1b25b6` and `f1d16f44`:
gate / gate (pull_request) state=failure desc='Failing after ~1m43-1m45s'
gate / conformance (pull_request) state=success desc='Has been skipped'
**That second line is §2b's trap in the wild**: a *skipped* Gitea job reports
`state=success`. The conformance job did not execute, so its green tick certifies nothing —
read the description, not the colour. The duration is given as a range on purpose: it differs
by a second or two per run, and pinning one number inside the file whose commit triggers the
next run is a self-reference, not a measurement. The load-bearing fields are the **state** and
the **description**, and those 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 to `success` only once that job settled. So sampling a status
once can show either a pending **or** a green tick for a job that never executed.
**I could NOT read the CI job log to confirm the failing stage from CI's
own output**: this Gitea's `/api/v1/repos/{owner}/{repo}/actions/runs` endpoint answers
**404** (only `/actions/tasks` exists, which lists tasks without logs), and
`/actions/jobs/{task_id}/logs` answers `404 Not found` for the task id. So my attribution of the
CI failure to `approval-policy` rests on the local event-set reproduction above plus the
sensitive-path rule, **not** on CI's own log. That is weaker evidence than PR #28 had and I am
naming it rather than borrowing its wording.
- **Branch protection and the runner, RE-MEASURED this session against the live API** (§2b),
not inherited from CLAUDE.md §2's table:
GET /repos/Home-Network/home-network-k3s-impl/branches/main
protected True
user_can_push False
enable_status_check True
status_check_contexts ['gate / gate (pull_request)']
required_approvals 1
GET /admin/actions/runners
id=1 name=incluster-dind status=online
So `required_approvals=1` applies to **every** PR here regardless of path — this needs a human
even without the sensitive-path rule. I did **not** provoke a rejected push to `main` this
session; `user_can_push: False` is the API's word, which is exactly the weaker form of evidence
CLAUDE.md §2 warns about, and I did not strengthen it.
- **`^/api/` ON RADARR IS NOT MY MEASUREMENT.** It is PR #30's, taken on the **pre-fix** pod. The
current pod has logged **zero** `/api/` traffic in 23 h, so I could not reproduce it. I judge the
breadth risk nil (`/api/v3/` is pure JSON; the UI is at `/`), but if someone wants a strictly
self-measured configuration, drop `^/api/` and keep `^/signalr/` — the 232 are entirely SignalR.
- **SONARR AND QBITTORRENT ARE UNMEASURED, NOT CLEARED.** Zero traffic in 23 h is zero evidence.
qBittorrent's WebUI drives everything through `/api/v2/` and is *plausibly* affected the same way
Jackett is; Sonarr is nearly Radarr's codebase. **Both are left alone on purpose** — plausibility
is what put a renderer constant on the table in the first place. Expect to add `^/api/` to
qBittorrent and `^/api/` + `^/signalr/` to Sonarr once someone opens those UIs and records the
split. **This PR does not fix them.**
- **PiHole is asserted to serve no `/api/` on inherited evidence, not mine.** Its log shows 0
unauthenticated requests, which says nothing about what paths it serves. The claim comes from
PR #28/#30. Note the pinned `update.range` is `>=2024.0.0`, and PiHole v6 *does* introduce a REST
API at `/api/` — so if PiHole is ever upgraded across that line, this needs re-measuring. It is
still correct to declare nothing today, because declaring an unmeasured route is the mistake.
- **`--cookie-refresh=2m30s` has STILL never been exercised** — 0 `AuthSuccess` on all five pods.
PR #28 named this as the fix most likely to need a second pass and it remains unobserved. My
change does not touch it, but a sign-in during the human test below will exercise both at once,
so a failure there must be attributed carefully.
- **The regexes are not validated against the apps' real route tables.** I assert `/api/v3/` and
`/signalr/` are JSON/RPC namespaces a browser never navigates to from Radarr's and Jackett's
observed traffic and from how those UIs are built — **not** from reading their routing code or
enumerating their endpoints. A path I have not thought of that matches `^/api/` and *is*
browser-navigated would 401.
- **`OPTIONS` preflight and non-browser clients are unconsidered.** `--skip-auth-preflight` exists
and is not set; nothing in these logs shows a CORS preflight, and Sonarr/Radarr talk to Jackett
**directly on `:9117`**, bypassing the proxy entirely — so I do not expect impact. Unverified.
- **The live pods do not carry this change and nothing re-measures that they will.** git is not the
cluster (§2b). **Merging is what deploys**, and it will roll **jackett** and **radarr** only
(`strategy: Recreate` on RWO Longhorn volumes, so a brief gap per app, and radarr/sonarr lose
direct `:9117` access to Jackett while its pod restarts). Existing sessions survive — no cookie
flag changed — so unlike PR #28 nobody is signed out.
- **No L2/L3 assertion reads any of this.** There is no runtime check that a `/api/` request to a
gated app answers 401, and I did not write one; `CLUSTER_AVAILABLE` is unset so it could not run
in CI anyway. The whole guard is static.
- **`catalog/render.py --selftest` is UNGATED IN MY BASE, and I did not wire it.** Desmond's PR #33
does that (`ci/render-golden.sh`) and is in review, not merged — `origin/main` at `cdf3e14`
contains no stage that drives it, which I checked rather than assumed (`grep -n selftest
ci/render-golden.sh` → no match). **So until #33 lands, my new selftest assertions are
dev-time only**, and the *gated* half of my breadth ratchet is the schema `pattern` (via
`ci/l0.sh`'s schema stage), the renderer's own `raise` (via `render-golden`), and the new L0
check. I deliberately did not wire the selftest myself: #33 already does it, and duplicating it
here would collide with his branch in the one file he owns. Note the two PRs are **independent
but complementary** — his makes the stage real, mine makes its output readable when it fires.
- **I did NOT verify PR #33's own claims.** The 106-assertion count, the
selftest→stage→`gate.sh`→`BLOCKED` propagation chain, and the `--cookie-refresh 2m30s → 10m`
mutation being caught by the selftest but not by `render-golden` are all his measurements,
relayed to me by the coordinator. I reproduced only the StopIteration defect itself, because that
is the part I was asked to fix. If his branch is reworked, nothing in mine depends on it.
- **The new check does not read the spec repo** and asserts nothing about `../home-network-k3s`
(R-TEST-06, read-only). `schemas/renderer-contract.md` is updated in **this** repo only; the
canonical spec `docs/13-service-catalog.md` is **not** updated, because it is immutable from here.
Someone with spec write access should mirror `auth.apiRoutes` into docs/13 and docs/15.
- **THE OAUTH2-PROXY SIDECARS' METRICS ARE STILL UNSCRAPABLE, AND THIS PR DOES NOT FIX IT.** See
the declined-bundle section above. #28 shipped `--metrics-address` as a prerequisite and said
plainly that it was not scraped; Ines has now measured that the listener **serves** and that the
scrape is **dropped by each app namespace's default-deny**. Nothing in my change touches that.
Anyone reading this entry as "the fleet gates are observable now" is wrong, and the next PR is
where that changes.
- **Alternation is refused, and that is a judgement I may have to revisit.** No app in this catalog
needs it, and the flag is repeatable — but if some future app genuinely needs a regex the ratchet
rejects, the fix is to widen the pattern **with a reason** on all three surfaces at once, not to
delete the ratchet on one.
- **CI status: two contexts, and only one of them ran.** Quoted in the PR body and in the report to
the PM. `approval-policy` **fails by design** until a non-author approving review exists, because
the diff touches `conformance/**`, which `infra/gitea/approval-policy.json` lists as sensitive
("the tests and the harness; changing them changes what green MEANS"). That is CLAUDE.md §1
working, not a defect. `required_approvals=1` applies to every PR here regardless of path, so this
needs a human either way.
### The human test — and how to tell a too-broad regex from a working one
After merge, wait for **new** pod names at 2/2 in `jackett` and `radarr`.
1. **The breadth test FIRST, before anything else, from a signed-out browser** (private window).
Open `https://radarr.home.keithjackson.dev/` and `https://jackett.home.keithjackson.dev/UI/Dashboard`.
**Each must bounce through Keycloak and land on the app's UI.** If either shows a bare
`401 Unauthorized` instead 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.
2. **Jackett, the original report:** signed in, *+ Add indexer* → pick one → *Configure* → *Okay*.
**It must save.** Previously 4 of 6 POSTs never reached Jackett at all.
3. **Radarr, the live symptom:** open Radarr and leave the tab open. DevTools → Network, filter
`signalr`. Today those negotiate calls are **302** and the UI never complains. They must now be
**200** while signed in — and if the session is ever lost, **401**, with the UI showing a visible
connection error instead of polling silently forever.
4. **The loud-failure test:** in DevTools, delete the `_oauth2_proxy_radarr` cookie and let the tab
poll. The negotiate call must answer **401** and Radarr must *show* a problem. Then **reload** —
`/` matches neither regex, so the reload must sign you back in silently. That round trip is the
entire fix: fail loudly on the API, recover on the page load.
5. **The six-minute test (PR #28's open item, still open):** stay on a dashboard for >6 minutes,
then save something. That exercises `--cookie-refresh=2m30s` past the 300 s access-token cliff.
If it fails, suspect the realm's refresh/SSO-idle timeouts — not this change.
6. **If something still fails, it must now fail LOUDLY.** A silent success-looking no-op means the
change did not take: check the running pod's args for `--api-route` before assuming anything
else.