fix(rbac): bound SSE change-authorizer with a per-connection SAR memo

[hisco]

Problem

Follow-up to #1300. The SSE live-stream change authorizer built a plain closure that called canReadUser per change frame. canReadUser only writes a SubjectAccessReview result back to the shared permission cache when that user's entry still exists — and the SSE path primes the entry only once at subscribe, so it TTLs out after ~2min. From then on, for every long-lived client:

• each qualifying change frame ran a fresh, uncached SAR,
• serially inside the single broadcast goroutine,
• with no timeout.

Effect: one slow/hung apiserver SAR stalls change broadcasts for all connected clients; sustained change traffic multiplies apiserver SAR load by client × distinct (kind, namespace). This hits exactly the deployment mode the RBAC work targets — auth-enabled, multi-user (Radar Hub), long-lived streams. Auth-off (local kubeconfig, single user) is unaffected — the authorizer short-circuits on nil user.

Flagged in the #1300 review; #1300 merged before it was addressed, so this is a standalone follow-up.

Fix

newSSEChangeAuthorizer wraps the per-frame decision in a connection-lived TTL memo (memoizedAuthorizer), so each (context, verb, group, resource, namespace) is resolved at most once per TTL instead of once per frame:

• Fresh SAR, cache-bypassing. The memo's base runs a new canReadUserSAR — a bounded SAR that skips the shared permission cache — so SSE staleness is bounded to the memo's own 2min TTL, not stacked with the shared-cache TTL (~4min worst case).
• Context-scoped key. A kubeconfig context switch leaves SSE connections open (they get a context_changed frame, not a disconnect). The memo key includes the current context name, so post-switch frames miss and re-run against the new apiserver — a still-open stream can't authorize new-cluster frames with the previous cluster's decisions. Mirrors the shared cache's own context stamping.
• Fail closed on mid-SAR switch. If the context changes while a SAR is in flight, that verdict was decided against a different apiserver than the key names, so the frame fails closed (deny) and nothing is cached — a switch-back within the TTL can't serve a wrong-cluster decision.
• Bounded SAR. Runs under a 5s context so a hung apiserver call can't wedge the broadcast loop.
• Brief negative cache for transient failures. A non-authoritative result (no client, SAR error, or 5s timeout) is a fail-closed deny cached for a short 10s window — not the full 2min TTL (a blip must not deny a readable kind for the whole window), and not zero (a degraded-but-alive apiserver must not re-pay the 5s SAR on every frame for the same kind, serially in the single broadcast goroutine). Authoritative allow/deny still caches for the full TTL.
• Bounded memo. Expired entries are swept before the per-connection memo grows past a soft cap, so a long-lived all-namespace stream on a CRD-heavy cluster can't accumulate one entry per observed tuple without bound.

canReadUser is refactored to delegate its SAR tail to canReadUserSAR; its REST behavior is unchanged (reads the shared cache, writes back only when the entry exists). It deliberately still never caches a transient failure — unlike the SSE memo's brief negative cache — because the REST path runs one check per request goroutine, not serially on the shared broadcast loop, so caching a failure there would only add deny-latency with no throughput win.

Tests

• memoizedAuthorizer: within-TTL memoization, past-TTL re-check, context-switch decision isolation, fail-closed + no-cache on mid-SAR switch, and brief negative caching (served within the 10s window, then re-checks and caches the recovered allow for the full TTL).
• sweepExpiredAuthMemo: reclaims only expired entries, leaves live ones intact.
• newSSEChangeAuthorizer auth-off passthrough.

End-to-end against a real kind cluster in --auth-mode proxy, driving a single long-lived viewer stream (RoleBinding without secrets access) across the ~2min cache-expiry boundary:

	Secret frames	Deployment frames
Phase A (within TTL)	0	9
Phase B (post-TTL, same connection)	0	24 (new frames delivered)

Viewer received zero Secret frames across the whole run while still receiving new Deployment frames after the shared cache expired — proving the memo carries correct per-kind filtering past the bug window without over-denying readable kinds.

Deferred (tracked, not in this PR)

Two residuals are intentionally left as follow-ups — neither is a regression vs main, and both are gated on real-world evidence rather than built speculatively:

• Outage-storm latency. During an apiserver outage, the first frame for each distinct cold kind still pays one 5s SAR timeout (the 10s negative cache only dedups repeats of the same kind). A circuit breaker — trip on repeated failures, fail closed instantly for a cooldown — is the fuller fix and stays authoritative. Build if the watcher is observed stalling under a degraded apiserver.
• Revocation-visibility lag. A de-authorized user keeps receiving change frames for ≤2min (RoleBinding change, the documented TTL propagation window) or until reconnect (group-membership change, since connection identity is captured at subscribe). By-design today; matches the shared permission cache's cadence.

Not pursued: gating on SelfSubjectRulesReview (one call for all kinds) instead of per-kind SubjectAccessReview. It isn't authoritative on clusters with webhook/external authorizers (e.g. GKE IAM, which Radar Hub targets) — it returns an Incomplete rule set — so it would over-deny or mismatch the real decision. Per-kind SAR is the authoritative primitive; the memo bounds its cost.

https://claude.ai/code/session_01XNhMe6Zdqwj5EoBazNDnmW

---

Note

High Risk
Changes authorization on the live SSE broadcast path (RBAC/SAR gating); mistakes could leak change frames or over-deny, though behavior is fail-closed and covered by new tests.

Overview
Fixes long-lived SSE streams that were issuing an uncached SubjectAccessReview on every change frame after the shared permission cache expired (~2 min), serially in the single broadcast goroutine and without a timeout—stalling all clients and amplifying apiserver load.

SSE subscribe now wires newSSEChangeAuthorizer, which memoizes per-frame (context, verb, group, resource, namespace) decisions for 2 minutes on a connection-local cache. Lookups call canReadUserSAR (bypassing the shared cache) under a 5s context so hung SARs cannot wedge broadcasts. Memo keys include the kubeconfig context name so open streams after a context switch re-authorize against the new cluster; mid-SAR context changes fail closed and are not cached. Transient SAR failures are denied briefly (10s negative TTL) without poisoning the full TTL.

canReadUser is refactored to delegate SAR to canReadUserSAR (allowed, authoritative) and only writes authoritative verdicts back to the shared cache—same REST behavior, but transient errors are no longer memoized as long denials.

Unit tests cover memo TTL, context isolation, switch-during-SAR, and negative caching.

^{Reviewed by Cursor Bugbot for commit 47485b3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 49b8626. Configure here.}

[nadaverell]

Adversarial review (self + codex cross-model) of the full branch delta. Approve — this correctly fixes the finding from the #1300 review, and strictly improves on main in every scenario.

Vetted green

• The flagged bug is closed: per-frame uncached SARs are gone; each (context, verb, group, resource, namespace) resolves at most once per 2-min TTL per connection, and the 5s SAR bound means a hung apiserver call can no longer wedge the broadcast loop indefinitely.
• Authoritative-verdict split is right: transient failures (no client, SAR error, timeout) fail closed for the in-flight frame but are never cached — in the memo or the shared cache (canReadUser skips SetCanI likewise). Bugbot's finding, fixed properly in 8ec8bbc with a test.
• Context-switch key scoping + mid-SAR non-caching: post-switch frames miss and re-run against the new apiserver; a switch-straddling SAR result is not cached. Both pinned by tests.
• REST behavior preserved: the canReadUser → canReadUserSAR refactor keeps the exact same read-cache/write-back semantics on request paths.
• Verified locally: new tests pass; Authorize is only reachable from the broadcast path; auth-off is a strict passthrough.
• The PR's e2e (viewer across the TTL boundary: 0 Secret frames, Deployment frames keep flowing) demonstrates exactly the failure mode the original finding described, fixed without over-denial.

Non-blocking hardenings (fine as follow-ups; none is a regression vs main)

1. Fail closed on mid-SAR context change. When contextName() != ctxName after base(), the code correctly skips caching but still returns the SAR result — a verdict possibly decided against the other cluster can release the frame. Returning false there instead is 2 lines and strictly safer. (Context: SSE frames during a switch window aren't cluster-stamped anyway — the same async informer-shutdown window #1301 documents — so this narrows an existing residue rather than closing it fully.)
2. Short negative-result TTL. Non-authoritative results are never cached, so with a degraded apiserver every frame re-pays up to 5s per cold tuple per connection, serially in the single broadcast goroutine (N clients compound). Caching failures for ~10s would bound that to one stall per tuple per 10s while still recovering fast. This is the highest-value residual: today's worst case (unhealthy apiserver + many auth clients) can still meaningfully stall the watcher.
3. Opportunistic memo eviction. Expired entries are overwritten but never removed, so the per-connection map grows with every observed context × kind × namespace tuple over the connection's lifetime. A sweep when the map exceeds a threshold (or on context switch) keeps long-lived all-namespace streams on CRD-heavy clusters bounded.

Also noting for the record: same-name context identity (reconnect/credential refresh under an unchanged context name serving memoized decisions for up to the TTL) matches the shared permission cache's existing username\x00contextName keying — consistent with the system's context-identity model, TTL-bounded, not introduced here.