refactor(kubevirt): Enforce access control in toolset API

Remove direct RESTConfig() exposure from Kubernetes type and add
access-controlled methods (ResourcesListByGVR, RESTConfigForGVK) that
validate resource access through AccessControlRESTMapper before allowing
operations. Update all kubevirt tools to use controlled methods instead
of creating their own uncontrolled dynamic clients.

This prevents toolsets from bypassing the denied resources configuration
and ensures all API access goes through the access control layer.

Assisted-By: Claude <[email protected]>
Signed-off-by: Lee Yarwood <[email protected]>

Author: lyarwood

pkg/kubernetes/kubernetes.go

@@ -2,7 +2,6 @@ package kubernetes
 
 import (
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/rest"
 
 	"github.com/containers/kubernetes-mcp-server/pkg/helm"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -31,14 +30,6 @@ func (k *Kubernetes) AccessControlClientset() *AccessControlClientset {
 	return k.manager.accessControlClientSet
 }
 
-// RESTConfig returns the Kubernetes REST configuration
-func (k *Kubernetes) RESTConfig() *rest.Config {
-	if k.manager == nil {
-		return nil
-	}
-	return k.manager.cfg
-}
-
 var Scheme = scheme.Scheme
 var ParameterCodec = runtime.NewParameterCodec(Scheme)
 

pkg/kubernetes/resources.go

@@ -14,6 +14,7 @@ import (
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/client-go/rest"
 )
 
 const (
@@ -45,7 +46,56 @@ func (k *Kubernetes) ResourcesList(ctx context.Context, gvk *schema.GroupVersion
 	return k.manager.dynamicClient.Resource(*gvr).Namespace(namespace).List(ctx, options.ListOptions)
 }
 
+// ResourcesListByGVR lists resources using a GroupVersionResource directly.
+// Access control is enforced through the RESTMapper.
+// Use this when you need to query arbitrary resources not covered by ResourcesList.
+func (k *Kubernetes) ResourcesListByGVR(ctx context.Context, gvr schema.GroupVersionResource, namespace string, options metav1.ListOptions) (*unstructured.UnstructuredList, error) {
+	if k.manager == nil {
+		return nil, fmt.Errorf("kubernetes manager not initialized")
+	}
+
+	// Validate access by converting GVR to GVK through the access control mapper
+	gvk, err := k.manager.accessControlRESTMapper.KindFor(gvr)
+	if err != nil {
+		return nil, err // Access denied or resource not found
+	}
+
+	// Also validate through resourceFor to ensure consistent access control
+	validatedGVR, err := k.resourceFor(&gvk)
+	if err != nil {
+		return nil, err
+	}
+
+	return k.manager.dynamicClient.Resource(*validatedGVR).Namespace(namespace).List(ctx, options)
+}
+
+// RESTConfigForGVK returns a copy of the REST config for creating custom clients.
+// Access control is enforced by validating the provided GVK.
+// This is useful for accessing subresources that aren't supported by the dynamic client.
+// The caller is responsible for configuring GroupVersion, APIPath, and NegotiatedSerializer
+// as needed for their specific use case.
+func (k *Kubernetes) RESTConfigForGVK(gvk *schema.GroupVersionKind) (*rest.Config, error) {
+	if k.manager == nil {
+		return nil, fmt.Errorf("kubernetes manager not initialized")
+	}
+
+	// Validate access control for the resource kind
+	if gvk != nil {
+		_, err := k.resourceFor(gvk)
+		if err != nil {
+			return nil, err // Access denied or resource not found
+		}
+	}
+
+	// Return a copy of the REST config to avoid modifying the original
+	return rest.CopyConfig(k.manager.cfg), nil
+}
+
 func (k *Kubernetes) ResourcesGet(ctx context.Context, gvk *schema.GroupVersionKind, namespace, name string) (*unstructured.Unstructured, error) {
+	if k.manager == nil {
+		return nil, fmt.Errorf("kubernetes manager not initialized")
+	}
+
 	gvr, err := k.resourceFor(gvk)
 	if err != nil {
 		return nil, err
@@ -73,6 +123,10 @@ func (k *Kubernetes) ResourcesCreateOrUpdate(ctx context.Context, resource strin
 }
 
 func (k *Kubernetes) ResourcesDelete(ctx context.Context, gvk *schema.GroupVersionKind, namespace, name string) error {
+	if k.manager == nil {
+		return fmt.Errorf("kubernetes manager not initialized")
+	}
+
 	gvr, err := k.resourceFor(gvk)
 	if err != nil {
 		return err

pkg/toolsets/kubevirt/vm/create/tool.go

@@ -12,7 +12,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/utils/ptr"
 )
 
@@ -464,18 +463,6 @@ func getDefaultContainerDisks() []DataSourceInfo {
 
 // searchDataSources searches for DataSource resources in the cluster
 func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSourceInfo, error) {
-	// Get dynamic client for querying DataSources
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return nil, fmt.Errorf("REST config is nil")
-	}
-
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		// Return just the built-in containerdisk images
-		return getDefaultContainerDisks(), nil
-	}
-
 	// DataSource GVR for CDI
 	dataSourceGVR := schema.GroupVersionResource{
 		Group:    "cdi.kubevirt.io",
@@ -484,7 +471,7 @@ func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSource
 	}
 
 	// Collect DataSources from well-known namespaces and all namespaces
-	results := collectDataSources(params, dynamicClient, dataSourceGVR)
+	results := collectDataSources(params, dataSourceGVR)
 
 	// Add common containerdisk images
 	results = append(results, getDefaultContainerDisks()...)
@@ -504,7 +491,7 @@ func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSource
 }
 
 // collectDataSources collects DataSources from well-known namespaces and all namespaces
-func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource) []DataSourceInfo {
+func collectDataSources(params api.ToolHandlerParams, gvr schema.GroupVersionResource) []DataSourceInfo {
 	var results []DataSourceInfo
 
 	// Try to list DataSources from well-known namespaces first
@@ -514,14 +501,14 @@ func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Inte
 	}
 
 	for _, ns := range wellKnownNamespaces {
-		dsInfos, err := listDataSourcesFromNamespace(params, dynamicClient, gvr, ns)
+		dsInfos, err := listDataSourcesFromNamespace(params, gvr, ns)
 		if err == nil {
 			results = append(results, dsInfos...)
 		}
 	}
 
 	// List DataSources from all namespaces
-	list, err := dynamicClient.Resource(gvr).List(params.Context, metav1.ListOptions{})
+	list, err := params.ResourcesListByGVR(params.Context, gvr, "", metav1.ListOptions{})
 	if err != nil {
 		// If we found DataSources from well-known namespaces but couldn't list all, return what we have
 		if len(results) > 0 {
@@ -587,9 +574,9 @@ func deduplicateAndMergeDataSources(existing []DataSourceInfo, items []unstructu
 }
 
 // listDataSourcesFromNamespace lists DataSources from a specific namespace
-func listDataSourcesFromNamespace(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource, namespace string) ([]DataSourceInfo, error) {
+func listDataSourcesFromNamespace(params api.ToolHandlerParams, gvr schema.GroupVersionResource, namespace string) ([]DataSourceInfo, error) {
 	var results []DataSourceInfo
-	list, err := dynamicClient.Resource(gvr).Namespace(namespace).List(params.Context, metav1.ListOptions{})
+	list, err := params.ResourcesListByGVR(params.Context, gvr, namespace, metav1.ListOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -629,16 +616,6 @@ func searchPreferences(params api.ToolHandlerParams, namespace string) []Prefere
 		return []PreferenceInfo{}
 	}
 
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return []PreferenceInfo{}
-	}
-
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		return []PreferenceInfo{}
-	}
-
 	var results []PreferenceInfo
 
 	// Search for cluster-wide VirtualMachineClusterPreferences
@@ -648,7 +625,7 @@ func searchPreferences(params api.ToolHandlerParams, namespace string) []Prefere
 		Resource: "virtualmachineclusterpreferences",
 	}
 
-	clusterList, err := dynamicClient.Resource(clusterPreferenceGVR).List(params.Context, metav1.ListOptions{})
+	clusterList, err := params.ResourcesListByGVR(params.Context, clusterPreferenceGVR, "", metav1.ListOptions{})
 	if err == nil {
 		for _, item := range clusterList.Items {
 			results = append(results, PreferenceInfo{
@@ -664,7 +641,7 @@ func searchPreferences(params api.ToolHandlerParams, namespace string) []Prefere
 		Resource: "virtualmachinepreferences",
 	}
 
-	namespacedList, err := dynamicClient.Resource(namespacedPreferenceGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{})
+	namespacedList, err := params.ResourcesListByGVR(params.Context, namespacedPreferenceGVR, namespace, metav1.ListOptions{})
 	if err == nil {
 		for _, item := range namespacedList.Items {
 			results = append(results, PreferenceInfo{
@@ -683,16 +660,6 @@ func searchInstancetypes(params api.ToolHandlerParams, namespace string) []Insta
 		return []InstancetypeInfo{}
 	}
 
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return []InstancetypeInfo{}
-	}
-
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		return []InstancetypeInfo{}
-	}
-
 	var results []InstancetypeInfo
 
 	// Search for cluster-wide VirtualMachineClusterInstancetypes
@@ -702,7 +669,7 @@ func searchInstancetypes(params api.ToolHandlerParams, namespace string) []Insta
 		Resource: "virtualmachineclusterinstancetypes",
 	}
 
-	clusterList, err := dynamicClient.Resource(clusterInstancetypeGVR).List(params.Context, metav1.ListOptions{})
+	clusterList, err := params.ResourcesListByGVR(params.Context, clusterInstancetypeGVR, "", metav1.ListOptions{})
 	if err == nil {
 		for _, item := range clusterList.Items {
 			results = append(results, InstancetypeInfo{
@@ -719,7 +686,7 @@ func searchInstancetypes(params api.ToolHandlerParams, namespace string) []Insta
 		Resource: "virtualmachineinstancetypes",
 	}
 
-	namespacedList, err := dynamicClient.Resource(namespacedInstancetypeGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{})
+	namespacedList, err := params.ResourcesListByGVR(params.Context, namespacedInstancetypeGVR, namespace, metav1.ListOptions{})
 	if err == nil {
 		for _, item := range namespacedList.Items {
 			results = append(results, InstancetypeInfo{

pkg/toolsets/kubevirt/vm/delete/tool.go

@@ -5,9 +5,7 @@ import (
 
 	"github.com/containers/kubernetes-mcp-server/pkg/api"
 	"github.com/google/jsonschema-go/jsonschema"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/utils/ptr"
 )
 
@@ -56,30 +54,15 @@ func deleteVM(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
 		return api.NewToolCallResult("", err), nil
 	}
 
-	// Get dynamic client
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil
+	// Define the VirtualMachine GVK
+	gvk := schema.GroupVersionKind{
+		Group:   "kubevirt.io",
+		Version: "v1",
+		Kind:    "VirtualMachine",
 	}
 
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil
-	}
-
-	// Define the VirtualMachine GVR
-	gvr := schema.GroupVersionResource{
-		Group:    "kubevirt.io",
-		Version:  "v1",
-		Resource: "virtualmachines",
-	}
-
-	// Delete the VM
-	err = dynamicClient.Resource(gvr).Namespace(namespace).Delete(
-		params.Context,
-		name,
-		metav1.DeleteOptions{},
-	)
+	// Delete the VM using the access-controlled method
+	err = params.ResourcesDelete(params.Context, &gvk, namespace, name)
 	if err != nil {
 		return api.NewToolCallResult("", fmt.Errorf("failed to delete VirtualMachine: %w", err)), nil
 	}

pkg/toolsets/kubevirt/vm/pause/tool.go

@@ -68,10 +68,17 @@ func pause(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
 		return api.NewToolCallResult("", err), nil
 	}
 
-	// Get REST config
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil
+	// Validate access to VirtualMachineInstance resource
+	vmiGVK := schema.GroupVersionKind{
+		Group:   "kubevirt.io",
+		Version: "v1",
+		Kind:    "VirtualMachineInstance",
+	}
+
+	// Get REST config with access control validation
+	restConfig, err := params.RESTConfigForGVK(&vmiGVK)
+	if err != nil {
+		return api.NewToolCallResult("", fmt.Errorf("failed to get REST config: %w", err)), nil
 	}
 
 	// Create a REST client for the KubeVirt subresource API

pkg/toolsets/kubevirt/vm/start/tool.go

@@ -6,10 +6,8 @@ import (
 	"github.com/containers/kubernetes-mcp-server/pkg/api"
 	"github.com/containers/kubernetes-mcp-server/pkg/output"
 	"github.com/google/jsonschema-go/jsonschema"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/utils/ptr"
 )
 
@@ -58,29 +56,15 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
 		return api.NewToolCallResult("", err), nil
 	}
 
-	// Get dynamic client
-	restConfig := params.RESTConfig()
-	if restConfig == nil {
-		return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil
+	// Define the VirtualMachine GVK
+	gvk := schema.GroupVersionKind{
+		Group:   "kubevirt.io",
+		Version: "v1",
+		Kind:    "VirtualMachine",
 	}
 
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil
-	}
-
-	// Get the current VM
-	gvr := schema.GroupVersionResource{
-		Group:    "kubevirt.io",
-		Version:  "v1",
-		Resource: "virtualmachines",
-	}
-
-	vm, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(
-		params.Context,
-		name,
-		metav1.GetOptions{},
-	)
+	// Get the current VM using access-controlled method
+	vm, err := params.ResourcesGet(params.Context, &gvk, namespace, name)
 	if err != nil {
 		return api.NewToolCallResult("", fmt.Errorf("failed to get VirtualMachine: %w", err)), nil
 	}
@@ -90,15 +74,15 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
 		return api.NewToolCallResult("", fmt.Errorf("failed to set runStrategy: %w", err)), nil
 	}
 
-	// Update the VM
-	updatedVM, err := dynamicClient.Resource(gvr).Namespace(namespace).Update(
-		params.Context,
-		vm,
-		metav1.UpdateOptions{},
-	)
+	// Update the VM using access-controlled method
+	updatedVMs, err := params.ResourcesCreateOrUpdate(params.Context, mustMarshalYAML(vm))
 	if err != nil {
 		return api.NewToolCallResult("", fmt.Errorf("failed to update VirtualMachine: %w", err)), nil
 	}
+	if len(updatedVMs) == 0 {
+		return api.NewToolCallResult("", fmt.Errorf("no VirtualMachine returned after update")), nil
+	}
+	updatedVM := updatedVMs[0]
 
 	// Format the output
 	marshalledYaml, err := output.MarshalYaml(updatedVM)
@@ -108,3 +92,12 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
 
 	return api.NewToolCallResult("# VirtualMachine started successfully\n"+marshalledYaml, nil), nil
 }
+
+// mustMarshalYAML marshals an unstructured object to YAML string
+func mustMarshalYAML(obj *unstructured.Unstructured) string {
+	yaml, err := output.MarshalYaml(obj)
+	if err != nil {
+		panic(fmt.Sprintf("failed to marshal object to YAML: %v", err))
+	}
+	return yaml
+}