🔒 fix: route RBAC introspection through kc-agent (#7993 Phase 6)#8185
Merged
clubanderson merged 1 commit intomainfrom Apr 15, 2026
Merged
🔒 fix: route RBAC introspection through kc-agent (#7993 Phase 6)#8185clubanderson merged 1 commit intomainfrom
clubanderson merged 1 commit intomainfrom
Conversation
Move CheckCanI / GetClusterPermissions / GetPermissionsSummary from the
backend pod ServiceAccount to kc-agent so SelfSubjectAccessReviews answer
"can the *user* do X?" instead of "can the *pod SA* do X?" when console
is deployed in-cluster.
This is the last remaining reach into the pod SA from a user-facing
read path. After this, no pkg/api/handlers/ code calls the shared
pkg/k8s.MultiClusterClient on behalf of a user — every such call goes
through kc-agent under the user's kubeconfig.
- pkg/agent/server_rbac.go: new /rbac/can-i, /rbac/permissions,
/permissions/summary handlers that delegate to the existing
MultiClusterClient methods (already user-scoped on kc-agent).
- pkg/api/handlers/rbac.go: delete CheckCanI / GetClusterPermissions /
GetPermissionsSummary backend handlers.
- pkg/api/server.go: drop the three route registrations.
- web/src/hooks/usePermissions.ts: useCanI + usePermissions now POST/GET
${LOCAL_AGENT_HTTP_URL}/rbac/can-i and /permissions/summary.
- web/src/hooks/useUsers.ts: useClusterPermissions now GETs
${LOCAL_AGENT_HTTP_URL}/rbac/permissions and bails in demo mode via
isBackendUnavailable().
- Tests updated to match the new fetch URLs.
Fixes #7993
Signed-off-by: Andrew Anderson <[email protected]>
kubestellar-prow
Bot
added
the
dco-signoff: yes
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/L
Contributor
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Contributor
There was a problem hiding this comment.
Pull request overview
Moves the remaining RBAC “introspection” endpoints (SSAR + permissions summaries) off the backend (which can run as the in-cluster pod ServiceAccount) and onto kc-agent (which runs under the user’s kubeconfig), and updates frontend hooks/tests to call the new local-agent routes.
Changes:
- Removed backend
/api/rbac/permissions,/api/permissions/summary, and/api/rbac/can-iroutes/handlers. - Added kc-agent HTTP handlers and route registrations for
/rbac/permissions,/permissions/summary, and/rbac/can-i. - Updated frontend hooks and tests to fetch these RBAC endpoints from
LOCAL_AGENT_HTTP_URL.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/mocks/handlers.ts | Keeps a legacy MSW stub and documents Phase 6 routing change. |
| web/src/hooks/useUsers.ts | Switches useClusterPermissions to fetch from kc-agent. |
| web/src/hooks/usePermissions.ts | Switches permissions summary + SSAR (useCanI) to kc-agent; introduces a named timeout constant. |
| web/src/hooks/tests/useUsers.test.ts | Updates tests to spy on fetch for the new kc-agent URL. |
| web/src/hooks/tests/usePermissions.test.ts | Updates URL assertions for permissions summary + can-i to the new kc-agent paths. |
| pkg/api/server.go | Removes backend route registrations for the migrated RBAC endpoints. |
| pkg/api/handlers/rbac.go | Deletes the three backend RBAC introspection handlers and leaves notes pointing to kc-agent. |
| pkg/agent/server_rbac.go | Adds new kc-agent RBAC/permissions handlers delegating to MultiClusterClient. |
| pkg/agent/server.go | Registers the new kc-agent RBAC/permissions routes. |
Comment on lines
+38
to
+55
| func (s *Server) handleCanIHTTP(w http.ResponseWriter, r *http.Request) { | ||
| s.setCORSHeaders(w, r) | ||
| w.Header().Set("Content-Type", "application/json") | ||
| if r.Method == http.MethodOptions { | ||
| w.WriteHeader(http.StatusOK) | ||
| return | ||
| } | ||
| if !s.validateToken(r) { | ||
| http.Error(w, "Unauthorized", http.StatusUnauthorized) | ||
| return | ||
| } | ||
| if r.Method != http.MethodPost { | ||
| writeJSONError(w, http.StatusMethodNotAllowed, "method not allowed") | ||
| return | ||
| } | ||
| if s.k8sClient == nil { | ||
| writeJSONError(w, http.StatusServiceUnavailable, "k8s client not initialized") | ||
| return |
Comment on lines
+702
to
+706
| const token = localStorage.getItem(STORAGE_KEY_TOKEN) | ||
| const response = await fetch(`${LOCAL_AGENT_HTTP_URL}/rbac/permissions${params}`, { | ||
| headers: { | ||
| 'Authorization': token ? `Bearer ${token}` : '', | ||
| 'Content-Type': 'application/json' }, |
| // permission checks (SelfSubjectAccessReview + permission summaries) must | ||
| // run under the user's kubeconfig, not the backend pod ServiceAccount. | ||
| // Otherwise the pod SA's permissions answer the question, not the caller's, | ||
| // which is both wrong and a privilege-escalation vector. |
Contributor
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Contributor
✅ Post-Merge Verification: passedCommit: |
clubanderson
added a commit
that referenced
this pull request
Apr 15, 2026
…header comment (#8188) Addresses three Copilot review comments on merged #8185: 1. handleCanIHTTP is a POST endpoint but setCORSHeaders was defaulting Access-Control-Allow-Methods to "GET, OPTIONS", so browser preflight requests for POST would fail. Generalized setCORSHeaders with a variadic methods parameter — back-compat preserved because callers that pass nothing still get defaultCORSAllowedMethods ("GET, OPTIONS"). handleCanIHTTP now passes http.MethodPost + http.MethodOptions. 2. useClusterPermissions re-implemented the agent auth-header logic and sent "Authorization: ''" when no token was stored, which the agent rejects. Refactored to reuse the existing agentAuthHeaders() helper (it correctly omits the header when no token is configured). 3. server_rbac.go file header comment called the pre-#8185 behavior a "privilege-escalation vector". Rewrote it to describe the actual impact: misleading RBAC display — CheckCanI ran under the pod SA, so the UI showed system permissions instead of the caller's. No real privilege escalation occurred (mutations already flowed through kc-agent under the user's identity). Added server_rbac_test.go covering the POST CORS preflight for handleCanIHTTP plus the setCORSHeaders back-compat default and explicit methods paths. Fixes #8188 Signed-off-by: Andy Anderson <[email protected]>
clubanderson
added a commit
that referenced
this pull request
Apr 15, 2026
…header comment (#8188) (#8198) Addresses three Copilot review comments on merged #8185: 1. handleCanIHTTP is a POST endpoint but setCORSHeaders was defaulting Access-Control-Allow-Methods to "GET, OPTIONS", so browser preflight requests for POST would fail. Generalized setCORSHeaders with a variadic methods parameter — back-compat preserved because callers that pass nothing still get defaultCORSAllowedMethods ("GET, OPTIONS"). handleCanIHTTP now passes http.MethodPost + http.MethodOptions. 2. useClusterPermissions re-implemented the agent auth-header logic and sent "Authorization: ''" when no token was stored, which the agent rejects. Refactored to reuse the existing agentAuthHeaders() helper (it correctly omits the header when no token is configured). 3. server_rbac.go file header comment called the pre-#8185 behavior a "privilege-escalation vector". Rewrote it to describe the actual impact: misleading RBAC display — CheckCanI ran under the pod SA, so the UI showed system permissions instead of the caller's. No real privilege escalation occurred (mutations already flowed through kc-agent under the user's identity). Added server_rbac_test.go covering the POST CORS preflight for handleCanIHTTP plus the setCORSHeaders back-compat default and explicit methods paths. Fixes #8188 Signed-off-by: Andy Anderson <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final phase of #7993. Move the last three user-facing RBAC introspection paths off the backend pod ServiceAccount and onto kc-agent so SelfSubjectAccessReviews answer can the user do X? rather than can the pod SA do X? when console is deployed in-cluster.
After this PR, no
pkg/api/handlers/code reaches the sharedpkg/k8s.MultiClusterClienton behalf of a user — every such call goes through kc-agent under the user's kubeconfig, which is the load-bearing rule from #7993.Why this only mattered in-cluster
k8sClientis loaded from~/.kube/config, so SSAR already runs as the user. The handlers were correct.k8sClientfalls through torest.InClusterConfig(), so SSAR runs as the pod SA. The answer reflects the pod SA's permissions, not the user's. Wrong answer.The wrongness was cosmetic, not a privilege escalation — actual destructive actions already route through kc-agent after Phases 1–4, so the apiserver enforces against the user's real identity. But a permission-gated button saying "allowed" when the user can't actually do the action is a confusing UX, and it was the last vestige of pod-SA k8s access in user-facing code.
Changes
Backend (deletes)
pkg/api/handlers/rbac.go: deleteCheckCanI,GetClusterPermissions,GetPermissionsSummaryhandlerspkg/api/server.go: drop the three route registrations (/rbac/can-i,/rbac/permissions,/permissions/summary)kc-agent (new)
pkg/agent/server_rbac.go: newhandleCanIHTTP,handleClusterPermissionsHTTP,handlePermissionsSummaryHTTPhandlers that delegate to the existingMultiClusterClientmethods. kc-agent'ss.k8sClientis loaded from the user's kubeconfig at startup, so calling these methods is already user-scoped — no logic duplication.pkg/agent/server.go: register the three new routes.Frontend
web/src/hooks/usePermissions.ts:useCanIandusePermissionsnow call${LOCAL_AGENT_HTTP_URL}/rbac/can-iand${LOCAL_AGENT_HTTP_URL}/permissions/summary. Per-request timeout extracted to a named constant.web/src/hooks/useUsers.ts:useClusterPermissionsrewritten to fetch${LOCAL_AGENT_HTTP_URL}/rbac/permissionsdirectly (instead of viaapi.get) and bail in demo mode viaisBackendUnavailable().globalThis.fetchand assert the new URLs.web/src/mocks/handlers.ts: legacy/api/permissions/summaryMSW stub kept as a no-op (frontend hooks short-circuit in demo mode anyway).Architectural notes
The shared
pkg/k8s.MultiClusterClientmethods (CheckCanI,CheckClusterAdminAccess,GetClusterPermissions,GetAllClusterPermissions,GetPermissionsSummary,GetAllPermissionsSummaries) stay — kc-agent uses them. The privileged-client lint guard checks for mutation patterns (Create|Update|Delete|Patch) inpkg/api/handlers/, so calling these read-ish methods from kc-agent doesn't trip it.Test plan
go build ./...— cleango test ./pkg/agent/... ./pkg/k8s/...— passgo test ./pkg/api/handlers/ -run "TestRBAC|TestCheckCanI|TestGetPermissions|TestGetClusterPermissions"— pass (the broader handlers test suite has the pre-existingTestRecordFocus_BadBody_Returns400mock failure unrelated to this PR)npm run build— pass with all post-build safety checks greenCloses
Fixes #7993