Commit ea10adc
authored
feat: probe-based informer gating + per-user RBAC for namespace picker, REST, and MCP (#609)
Per-user RBAC across the cache, the namespace picker, REST, and MCP. Three changes that grew together once live testing showed they had to ship as one to be coherent.
Each typed kind now goes cluster-wide, namespace-scoped, or off based on a real `list?limit=1` probe against the SA / kubeconfig identity — not an SSAR. List probes are authoritative because they ARE the operation the informer will perform; SSAR is one indirection too many and disagrees with reality on clusters using webhook authorizers (GKE IAM in particular). Mixed-scope users keep the access they have, with listers routed to the matching factory; dynamic/CRD watches probe cluster-wide first and only fall back per-GVR after a real 403. Reflector 403/Unauthorized errors now surface in diagnostics so RBAC drift is visible instead of looping silently.
Adds Role, ClusterRole, RoleBinding, ClusterRoleBinding to the probe/cache path with informers, listers, counts, and handlers — closes the access-control blind spot in the typed cache.
The header switcher used to call `PerformNamespaceSwitch`, which tore down and rebuilt the process-wide informer cache scoped to the picked namespace. In any shared deploy, one logged-in user could narrow every other user's view in real time.
The cache is never reshaped by user action. The pick is stored as a per-user preference (`Server.nsPreferences`, keyed by username + context) and intersected into reads on the way out:
- Switch is ~1s instead of 5–30s — no informer rebuild
- One user's pick doesn't affect anyone else's view
- Cluster-only kinds (Nodes, PVs, StorageClasses, etc.) stay visible for cluster-admins regardless of the pick — they have no namespace dimension to filter on
- Local single-user mode still persists picks across restarts via `settings.ActiveNamespaces`
- On context switch, all users' picks are dropped — they reference the previous cluster's namespaces
Every read path now agrees on the same boundary the apiserver would enforce.
- **MCP read tools** (`list_resources`, `get_resource`, `get_topology`, `get_events`, `get_pod_logs`, `list_namespaces`, `get_changes`, `get_workload_logs`, `get_audit`, `get_dashboard`) intersect requested namespaces with the user's RBAC-allowed set. New helper in `internal/mcp/permissions.go`. MCP write tools / exec / logs continue to impersonate via `DynamicClientFromContext`, so the apiserver enforces full RBAC there directly.
- **Picker source** (`/api/cluster/namespace-scope.accessibleNamespaces`) returns only what the calling user can access; `set-active-namespace` 403s when the pick isn't in that set (alice picking bob's namespace → 403, not silent empty results).
- **REST cluster-scoped reads** (`/api/resources/{nodes,namespaces,…}`, `/api/metrics/top/nodes`, `/api/nodes/.../{metrics,history}`, dashboard CRDs) gate per-kind via `canRead` SAR. Cluster-wide pod visibility does NOT imply cluster-scoped reads — that's the load-bearing distinction (a user with `list pods` everywhere still can't read Nodes / Namespace metadata / cluster-scoped CRDs / ClusterRoles).
- **Namespace metadata** specifically requires `list/get namespaces` SAR — `AllowedNamespaces` (which is populated from `list pods` cluster-wide) is not a sufficient fallback. Restricted users keep the picker via the synthesized `{name, status}` view at `/api/namespaces` and the `list_namespaces` MCP tool.
- **Topology** strips cluster-scoped node kinds (Karpenter NodePool/NodeClaim/NodeClass, GatewayClass, PV, StorageClass, ClusterNetworkPolicy, CiliumClusterwideNetworkPolicy) per-user via SAR, in both REST `handleTopology` and MCP `handleGetTopology`. The strip table is a denylist — it covers every cluster-scoped kind the topology builder creates that we identified in review, and the maintenance signal lives on `topology.NodeKind` so new kinds get caught. Architectural follow-up (per-node GVR/scope metadata so the strip is scope-driven) is non-blocking.
- **Cross-cluster authorization race closed.** `permCache` entries are stamped with the K8s context name at write time; `Get` returns nil when the stamp doesn't match the current context, so an old-cluster `canI` cache result can't authorize a new-cluster read in the window between context switch and cache invalidation. `s.finalizePostContextSwitch` runs as an `OnContextSwitch` callback so the invalidation order is consistent across REST and MCP.
Per-kind RBAC inside a namespace ("can list pods but not secrets in `team-a`") is deliberately deferred to a follow-up. Same boundary REST already enforced; fits 90% of K8s RBAC setups in practice.
User-controlled strings (`X-Forwarded-User`, URL-derived kind/group/resource, kubeconfig fields) flow into `log.Printf` along several auth paths. Sanitized via `k8s.SanitizeForLog` (CRLF strip — CodeQL doesn't model `%q` escaping). Closes the four open CodeQL `go/log-injection` alerts on this PR plus the sister sites in impersonation, namespace discovery, and SAR error paths.
Three layers, in order of rigor:
1. **Unit** (`go test ./internal/...`, `cd pkg && go test ./...`) — probe scenarios incl. forced-namespace cluster-only kinds, filter primitives, cluster-only kind detection, topology strip semantics with nil/empty edge cases, namespace SAR gates, cross-user `canI` cache isolation, cross-cluster permCache invalidation.
2. **Handler integration** (`internal/mcp/tools_filter_test.go`) — each MCP read handler exercised with fake K8s + `InitTestResourceCache` against no-auth / cluster-admin / namespace-restricted user contexts; asserts result content matches expected per-user filtering. Catches regressions where new read tools forget to filter.
3. **Live multi-user RBAC** (`scripts/test-rbac.sh`) — boots Radar with `--auth-mode=proxy`, applies test RoleBindings for five users (alice, bob, carol, dave, erin) covering namespace-scoped, cluster-wide-pods-only, multi-namespace, and nodes-only RBAC shapes, verifies REST + MCP both filter per-user, picker is per-user, cluster-only kinds hidden, denied namespaces rejected, cluster-scoped CRDs gated via discovery. 24/24 pass on a kind cluster. RBAC cleaned up on exit. Run with `--yes` to skip the safety prompt.
Visual layer covered separately against a real cluster (#655 tracks three UX polish items found during that pass — none security-relevant).
`cd web && npm run tsc` clean.
- `CLAUDE.md`: K8s Caching section rewritten for the new model (per-user filter, MCP RBAC parity, where state lives).
- `docs/configuration.md`: namespace picker described as a per-user view filter; doesn't affect other users.
- `docs/in-cluster.md`: voluntary scoping note plus an explicit note that MCP enforces the same RBAC boundary as REST; pod SA is the ceiling for both.
- `docs/mcp.md`: "RBAC-aware" rewritten to be specific about each deploy shape (local kubeconfig / in-cluster auth / in-cluster no-auth). Removed misleading "Local-only" line.
- Mirrored in `radar-docs` (PR skyhook-dev/radar-docs#8).1 parent 22ee2f5 commit ea10adc
52 files changed
Lines changed: 5792 additions & 762 deletions
File tree
- docs
- internal
- k8s
- mcp
- server
- settings
- packages/k8s-ui/src/types
- pkg
- auth
- k8score
- topology
- scripts
- web/src
- api
- components
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
334 | 334 | | |
335 | 335 | | |
336 | 336 | | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
337 | 341 | | |
338 | 342 | | |
339 | 343 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
109 | 109 | | |
110 | 110 | | |
111 | 111 | | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
112 | 118 | | |
113 | 119 | | |
114 | 120 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
179 | 179 | | |
180 | 180 | | |
181 | 181 | | |
182 | | - | |
| 182 | + | |
183 | 183 | | |
184 | | - | |
| 184 | + | |
185 | 185 | | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
190 | 192 | | |
191 | 193 | | |
192 | 194 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
198 | 198 | | |
199 | 199 | | |
200 | 200 | | |
201 | | - | |
202 | | - | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
203 | 205 | | |
204 | 206 | | |
205 | 207 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
52 | | - | |
| 51 | + | |
53 | 52 | | |
54 | 53 | | |
55 | 54 | | |
| |||
122 | 121 | | |
123 | 122 | | |
124 | 123 | | |
125 | | - | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
126 | 128 | | |
127 | 129 | | |
128 | 130 | | |
129 | 131 | | |
130 | | - | |
| 132 | + | |
131 | 133 | | |
132 | 134 | | |
133 | 135 | | |
134 | 136 | | |
135 | 137 | | |
136 | 138 | | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
162 | 145 | | |
163 | 146 | | |
164 | 147 | | |
165 | 148 | | |
166 | | - | |
| 149 | + | |
167 | 150 | | |
168 | | - | |
169 | | - | |
170 | 151 | | |
171 | 152 | | |
172 | 153 | | |
| |||
222 | 203 | | |
223 | 204 | | |
224 | 205 | | |
225 | | - | |
| 206 | + | |
226 | 207 | | |
227 | 208 | | |
228 | 209 | | |
| |||
645 | 626 | | |
646 | 627 | | |
647 | 628 | | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
648 | 633 | | |
649 | 634 | | |
650 | 635 | | |
| |||
931 | 916 | | |
932 | 917 | | |
933 | 918 | | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
934 | 955 | | |
935 | 956 | | |
936 | 957 | | |
| |||
0 commit comments