🔒 fix: audit setCORSHeaders callers to pass explicit methods (#8201)#8212
Merged
clubanderson merged 1 commit intomainfrom Apr 15, 2026
Merged
🔒 fix: audit setCORSHeaders callers to pass explicit methods (#8201)#8212clubanderson merged 1 commit intomainfrom
clubanderson merged 1 commit intomainfrom
Conversation
Copilot review on merged PR #8198 flagged that setCORSHeaders still defaults Access-Control-Allow-Methods to "GET, OPTIONS" unless callers pass an explicit method list, and that several existing kc-agent handlers serving POST/DELETE (e.g. /serviceaccounts) were still calling setCORSHeaders(w, r) without methods, so their preflight responses advertised only GET/OPTIONS and browsers rejected cross-origin POST/DELETE. This PR audits every setCORSHeaders caller in pkg/agent and updates the non-GET handlers to pass an explicit method list. The default of "GET, OPTIONS" is preserved for back-compat and remains correct for the read-only handlers. Handlers updated to advertise correct Allow-Methods on preflight: server_http.go - handleNamespacesHTTP GET, POST, DELETE, OPTIONS - handleServiceAccountsHTTP GET, POST, DELETE, OPTIONS - handleServiceExportsHTTP POST, DELETE, OPTIONS - handleRoleBindingsHTTP GET, POST, DELETE, OPTIONS - handleScaleHTTP POST, OPTIONS (consolidated from inline) - handleDeployWorkloadHTTP POST, OPTIONS (consolidated from inline) - handleDeleteWorkloadHTTP POST, OPTIONS (consolidated from inline) - handleAutoUpdateConfig GET, POST, OPTIONS (was OPTIONS-only) - handleAutoUpdateTrigger POST, OPTIONS (was OPTIONS-only) - handleAutoUpdateCancel POST, OPTIONS (was OPTIONS-only) - handleKubeconfigRemoveHTTP POST, OPTIONS (consolidated from inline) server_helm.go - handleHelmRollback POST, OPTIONS - handleHelmUninstall POST, OPTIONS - handleHelmUpgrade POST, OPTIONS server_gitops.go - handleDetectDrift POST, OPTIONS - handleGitopsSync POST, OPTIONS server_console_cr.go - handleConsoleCRManagedWorkloads POST, PUT, DELETE, OPTIONS - handleConsoleCRClusterGroups POST, PUT, DELETE, OPTIONS - handleConsoleCRWorkloadDeployments POST, DELETE, OPTIONS - handleConsoleCRWorkloadDeploymentStatus PUT, OPTIONS server_argocd.go - handleArgoCDSync POST, OPTIONS (consolidated from inline) server_gpu_health.go - handleGPUHealthCronJob POST, DELETE, OPTIONS server_operations.go - handleLocalClusters GET, POST, DELETE, OPTIONS - handleLocalClusterLifecycle POST, OPTIONS - handleVClusterCreate POST, OPTIONS - handleVClusterConnect POST, OPTIONS - handleVClusterDisconnect POST, OPTIONS - handleVClusterDelete POST, OPTIONS GET-only handlers (gpu-nodes, nodes, pods, events, deployments, replicasets, statefulsets, daemonsets, cronjobs, ingresses, services, configmaps, secrets, jobs, hpas, pvcs, roles, resourcequotas, limitranges, resolvedeps, clusterhealth, autoupdatestatus, kagenti/*, kagent-crds/*, prometheus query, vCluster list/check, cloudCLIStatus, localClusterTools, rbac/permissions, permissions/summary) keep the default and remain correct. Also updates the setCORSHeaders doc comment with an explicit audit rule so future handlers serving non-GET methods don't silently regress. Adds a regression test (TestHandleServiceAccounts_CORSMethodsHeader) that pins the fix on the specific handler Copilot called out. Fixes #8201 Signed-off-by: Andy Anderson <[email protected]>
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 |
kubestellar-prow
Bot
added
the
dco-signoff: yes
Contributor
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/L
✅ Deploy Preview for kubestellarconsole canceled.
|
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
Audits kc-agent HTTP handlers to ensure CORS preflight responses advertise the full set of non-GET methods actually served by each route, preventing browser CORS failures caused by the setCORSHeaders default of "GET, OPTIONS".
Changes:
- Updated non-GET handlers across agent servers to pass explicit
Access-Control-Allow-MethodsviasetCORSHeaders(..., methods...). - Consolidated a few prior inline
Access-Control-Allow-Methodsoverrides intosetCORSHeaderscalls for consistency. - Added a regression test pinning the
/serviceaccountsCORS methods header behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/server_http.go | Pass explicit method lists to setCORSHeaders for mutating handlers (namespaces, serviceaccounts, serviceexports, rolebindings, scale/deploy/delete workload, auto-update endpoints, kubeconfig remove) and documents the audit rule. |
| pkg/agent/server_operations.go | Updates local cluster + vCluster mutating endpoints to advertise correct CORS methods (GET/POST/DELETE or POST-only). |
| pkg/agent/server_helm.go | Ensures Helm POST-only endpoints advertise POST in CORS preflight via setCORSHeaders. |
| pkg/agent/server_gitops.go | Ensures GitOps POST-only endpoints advertise POST in CORS preflight via setCORSHeaders. |
| pkg/agent/server_console_cr.go | Ensures Console CR write endpoints advertise POST/PUT/DELETE as appropriate in CORS preflight via setCORSHeaders. |
| pkg/agent/server_argocd.go | Replaces inline CORS methods override with explicit setCORSHeaders method list for POST-only sync. |
| pkg/agent/server_gpu_health.go | Ensures GPU health CronJob install/uninstall advertises POST/DELETE in CORS preflight via setCORSHeaders. |
| pkg/agent/server_rbac_test.go | Adds TestHandleServiceAccounts_CORSMethodsHeader regression test for the audited /serviceaccounts handler. |
Contributor
❌ Post-Merge Verification: failedCommit: |
Contributor
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
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
Copilot review on merged PR #8198 flagged:
This PR audits every
setCORSHeaderscaller inpkg/agentand updates each non-GET handler to pass an explicit method list. The default of"GET, OPTIONS"is preserved for back-compat and remains correct for read-only handlers.Handlers Updated
server_http.gohandleNamespacesHTTP→ GET, POST, DELETE, OPTIONShandleServiceAccountsHTTP→ GET, POST, DELETE, OPTIONS (Copilot-flagged)handleServiceExportsHTTP→ POST, DELETE, OPTIONShandleRoleBindingsHTTP→ GET, POST, DELETE, OPTIONShandleScaleHTTP→ POST, OPTIONS (consolidated from inline override)handleDeployWorkloadHTTP→ POST, OPTIONS (consolidated from inline override)handleDeleteWorkloadHTTP→ POST, OPTIONS (consolidated from inline override)handleAutoUpdateConfig→ GET, POST, OPTIONS (was set only inside the OPTIONS branch)handleAutoUpdateTrigger→ POST, OPTIONS (was set only inside the OPTIONS branch)handleAutoUpdateCancel→ POST, OPTIONS (was set only inside the OPTIONS branch)handleKubeconfigRemoveHTTP→ POST, OPTIONS (consolidated from inline override)server_helm.gohandleHelmRollback→ POST, OPTIONShandleHelmUninstall→ POST, OPTIONShandleHelmUpgrade→ POST, OPTIONSserver_gitops.gohandleDetectDrift→ POST, OPTIONShandleGitopsSync→ POST, OPTIONSserver_console_cr.gohandleConsoleCRManagedWorkloads→ POST, PUT, DELETE, OPTIONShandleConsoleCRClusterGroups→ POST, PUT, DELETE, OPTIONShandleConsoleCRWorkloadDeployments→ POST, DELETE, OPTIONShandleConsoleCRWorkloadDeploymentStatus→ PUT, OPTIONSserver_argocd.gohandleArgoCDSync→ POST, OPTIONS (consolidated from inline override added in [Copilot Review] 7 comment(s) on merged PR #8038 #8040)server_gpu_health.gohandleGPUHealthCronJob→ POST, DELETE, OPTIONSserver_operations.gohandleLocalClusters→ GET, POST, DELETE, OPTIONShandleLocalClusterLifecycle→ POST, OPTIONShandleVClusterCreate→ POST, OPTIONShandleVClusterConnect→ POST, OPTIONShandleVClusterDisconnect→ POST, OPTIONShandleVClusterDelete→ POST, OPTIONSGET-only handlers (gpu-nodes, nodes, pods, events, deployments, replicasets, statefulsets, daemonsets, cronjobs, ingresses, networkpolicies, services, configmaps, secrets, jobs, hpas, pvcs, roles, resourcequotas, limitranges, resolve-deps, cluster-health, autoUpdateStatus, kagenti/, kagent-crds/, prometheus query, vCluster list/check, cloudCLIStatus, localClusterTools, rbac/permissions, permissions/summary) keep the default and remain correct.
The
setCORSHeadersdoc comment now carries an explicit audit rule so future handlers serving non-GET methods don't silently regress.A new regression test
TestHandleServiceAccounts_CORSMethodsHeaderpins the fix for the specific handler Copilot called out.Test plan
go build ./...passesgo vet ./...passesgo test ./pkg/agent/ -run "TestSetCORSHeaders|TestHandleCanIHTTP_CORSMethodsHeader|TestHandleServiceAccounts_CORSMethodsHeader"passescd web && npm run buildpassesFixes #8201