Move instance name validation to handlers

pkg/manager/operations.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"llamactl/pkg/instance"
-	"llamactl/pkg/validation"
 	"log"
 )
 
@@ -58,12 +57,7 @@ func (im *instanceManager) CreateInstance(name string, options *instance.Options
 		return nil, fmt.Errorf("instance options cannot be nil")
 	}
 
-	name, err := validation.ValidateInstanceName(name)
+	err := options.BackendOptions.ValidateInstanceOptions()
-	if err != nil {
-		return nil, err
-	}
-
-	err = options.BackendOptions.ValidateInstanceOptions()
 	if err != nil {
 		return nil, err
 	}

pkg/manager/remote.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"llamactl/pkg/config"
 	"llamactl/pkg/instance"
-	"llamactl/pkg/validation"
 	"net/http"
 	"net/url"
 	"sync"
@@ -139,16 +138,9 @@ func parseRemoteResponse(resp *http.Response, result any) error {
 
 // --- Remote CRUD operations ---
 
-// CreateInstance creates a new instance on a remote node.
+// createInstance creates a new instance on a remote node.
 func (rm *remoteManager) createInstance(ctx context.Context, node *config.NodeConfig, name string, opts *instance.Options) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
+	escapedName := url.PathEscape(name)
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
-
-	// URL-encode the validated instance name to safely include it in the URL path
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/", apiBasePath, escapedName)
 
@@ -165,16 +157,10 @@ func (rm *remoteManager) createInstance(ctx context.Context, node *config.NodeCo
 	return &inst, nil
 }
 
-// GetInstance retrieves an instance by name from a remote node.
+// getInstance retrieves an instance by name from a remote node.
 func (rm *remoteManager) getInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/", apiBasePath, escapedName)
 	resp, err := rm.makeRemoteRequest(ctx, node, "GET", path, nil)
@@ -190,16 +176,10 @@ func (rm *remoteManager) getInstance(ctx context.Context, node *config.NodeConfi
 	return &inst, nil
 }
 
-// UpdateInstance updates an existing instance on a remote node.
+// updateInstance updates an existing instance on a remote node.
 func (rm *remoteManager) updateInstance(ctx context.Context, node *config.NodeConfig, name string, opts *instance.Options) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/", apiBasePath, escapedName)
 
@@ -216,16 +196,10 @@ func (rm *remoteManager) updateInstance(ctx context.Context, node *config.NodeCo
 	return &inst, nil
 }
 
-// DeleteInstance deletes an instance from a remote node.
+// deleteInstance deletes an instance from a remote node.
 func (rm *remoteManager) deleteInstance(ctx context.Context, node *config.NodeConfig, name string) error {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/", apiBasePath, escapedName)
 	resp, err := rm.makeRemoteRequest(ctx, node, "DELETE", path, nil)
@@ -236,16 +210,10 @@ func (rm *remoteManager) deleteInstance(ctx context.Context, node *config.NodeCo
 	return parseRemoteResponse(resp, nil)
 }
 
-// StartInstance starts an instance on a remote node.
+// startInstance starts an instance on a remote node.
 func (rm *remoteManager) startInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/start", apiBasePath, escapedName)
 	resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil)
@@ -261,16 +229,10 @@ func (rm *remoteManager) startInstance(ctx context.Context, node *config.NodeCon
 	return &inst, nil
 }
 
-// StopInstance stops an instance on a remote node.
+// stopInstance stops an instance on a remote node.
 func (rm *remoteManager) stopInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/stop", apiBasePath, escapedName)
 	resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil)
@@ -286,16 +248,9 @@ func (rm *remoteManager) stopInstance(ctx context.Context, node *config.NodeConf
 	return &inst, nil
 }
 
-// RestartInstance restarts an instance on a remote node.
+// restartInstance restarts an instance on a remote node.
 func (rm *remoteManager) restartInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) {
-	// Validate instance name to prevent injection attacks
+	escapedName := url.PathEscape(name)
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return nil, err
-	}
-
-	// URL-encode the validated instance name to safely include it in the URL path
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/restart", apiBasePath, escapedName)
 	resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil)
@@ -311,16 +266,10 @@ func (rm *remoteManager) restartInstance(ctx context.Context, node *config.NodeC
 	return &inst, nil
 }
 
-// GetInstanceLogs retrieves logs for an instance from a remote node.
+// getInstanceLogs retrieves logs for an instance from a remote node.
 func (rm *remoteManager) getInstanceLogs(ctx context.Context, node *config.NodeConfig, name string, numLines int) (string, error) {
-	// Validate instance name to prevent injection attacks
-	validatedName, err := validation.ValidateInstanceName(name)
-	if err != nil {
-		return "", err
-	}
 
-	// URL-encode the validated instance name to safely include it in the URL path
+	escapedName := url.PathEscape(name)
-	escapedName := url.PathEscape(validatedName)
 
 	path := fmt.Sprintf("%s%s/logs?lines=%d", apiBasePath, escapedName, numLines)
 	resp, err := rm.makeRemoteRequest(ctx, node, "GET", path, nil)

pkg/server/handlers_instances.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"llamactl/pkg/instance"
 	"llamactl/pkg/manager"
+	"llamactl/pkg/validation"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -55,8 +56,10 @@ func (h *Handler) ListInstances() http.HandlerFunc {
 func (h *Handler) CreateInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
@@ -66,7 +69,7 @@ func (h *Handler) CreateInstance() http.HandlerFunc {
 			return
 		}
 
-		inst, err := h.InstanceManager.CreateInstance(name, &options)
+		inst, err := h.InstanceManager.CreateInstance(validatedName, &options)
 		if err != nil {
 			http.Error(w, "Failed to create instance: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -95,12 +98,14 @@ func (h *Handler) CreateInstance() http.HandlerFunc {
 func (h *Handler) GetInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		inst, err := h.InstanceManager.GetInstance(name)
+		inst, err := h.InstanceManager.GetInstance(validatedName)
 		if err != nil {
 			http.Error(w, "Invalid instance: "+err.Error(), http.StatusBadRequest)
 			return
@@ -130,8 +135,10 @@ func (h *Handler) GetInstance() http.HandlerFunc {
 func (h *Handler) UpdateInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
@@ -141,7 +148,7 @@ func (h *Handler) UpdateInstance() http.HandlerFunc {
 			return
 		}
 
-		inst, err := h.InstanceManager.UpdateInstance(name, &options)
+		inst, err := h.InstanceManager.UpdateInstance(validatedName, &options)
 		if err != nil {
 			http.Error(w, "Failed to update instance: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -169,12 +176,14 @@ func (h *Handler) UpdateInstance() http.HandlerFunc {
 func (h *Handler) StartInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		inst, err := h.InstanceManager.StartInstance(name)
+		inst, err := h.InstanceManager.StartInstance(validatedName)
 		if err != nil {
 			// Check if error is due to maximum running instances limit
 			if _, ok := err.(manager.MaxRunningInstancesError); ok {
@@ -208,12 +217,14 @@ func (h *Handler) StartInstance() http.HandlerFunc {
 func (h *Handler) StopInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		inst, err := h.InstanceManager.StopInstance(name)
+		inst, err := h.InstanceManager.StopInstance(validatedName)
 		if err != nil {
 			http.Error(w, "Failed to stop instance: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -241,12 +252,14 @@ func (h *Handler) StopInstance() http.HandlerFunc {
 func (h *Handler) RestartInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		inst, err := h.InstanceManager.RestartInstance(name)
+		inst, err := h.InstanceManager.RestartInstance(validatedName)
 		if err != nil {
 			http.Error(w, "Failed to restart instance: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -273,12 +286,14 @@ func (h *Handler) RestartInstance() http.HandlerFunc {
 func (h *Handler) DeleteInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		if err := h.InstanceManager.DeleteInstance(name); err != nil {
+		if err := h.InstanceManager.DeleteInstance(validatedName); err != nil {
 			http.Error(w, "Failed to delete instance: "+err.Error(), http.StatusInternalServerError)
 			return
 		}
@@ -302,8 +317,10 @@ func (h *Handler) DeleteInstance() http.HandlerFunc {
 func (h *Handler) GetInstanceLogs() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
@@ -319,7 +336,7 @@ func (h *Handler) GetInstanceLogs() http.HandlerFunc {
 		}
 
 		// Use the instance manager which handles both local and remote instances
-		logs, err := h.InstanceManager.GetInstanceLogs(name, numLines)
+		logs, err := h.InstanceManager.GetInstanceLogs(validatedName, numLines)
 		if err != nil {
 			http.Error(w, "Failed to get logs: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -345,12 +362,14 @@ func (h *Handler) GetInstanceLogs() http.HandlerFunc {
 func (h *Handler) ProxyToInstance() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := chi.URLParam(r, "name")
-		if name == "" {
+
-			http.Error(w, "Instance name cannot be empty", http.StatusBadRequest)
+		validatedName, err := validation.ValidateInstanceName(name)
+		if err != nil {
+			http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		inst, err := h.InstanceManager.GetInstance(name)
+		inst, err := h.InstanceManager.GetInstance(validatedName)
 		if err != nil {
 			http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError)
 			return
@@ -358,7 +377,7 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc {
 
 		// Check if this is a remote instance
 		if inst.IsRemote() {
-			h.RemoteInstanceProxy(w, r, name, inst)
+			h.RemoteInstanceProxy(w, r, validatedName, inst)
 			return
 		}
 
@@ -375,7 +394,7 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc {
 		}
 
 		// Strip the "/api/v1/instances/<name>/proxy" prefix from the request URL
-		prefix := fmt.Sprintf("/api/v1/instances/%s/proxy", name)
+		prefix := fmt.Sprintf("/api/v1/instances/%s/proxy", validatedName)
 		r.URL.Path = strings.TrimPrefix(r.URL.Path, prefix)
 
 		// Update the last request time for the instance