Extract common validation helpers in config package (#2481)

Centralizes duplicated validation logic across config operations into
reusable helper functions with consistent error messaging.

Changes:
- Add pkg/config/validation.go with reusable validators:
  * validateFilePath() - file existence and path cleaning
  * validateFileExists() - file existence check without cleaning
  * readFile() - consistent file reading with error handling
  * validateJSONFile() - JSON extension and content validation
  * validateURLScheme() - URL scheme validation (http/https)
  * makeAbsolutePath() - relative to absolute path conversion
- Add error message constants for consistent formatting
- Add comprehensive unit tests in validation_test.go
- Refactor cacert.go to use validation helpers
- Refactor registry.go to use validation helpers
- Remove duplicated validation logic and unused imports

Benefits:
- Eliminates code duplication in file and URL validation
- Consistent error messages across all config operations
- Easier maintenance with centralized validation logic
- Better testability with isolated validator tests

All existing tests pass with no breaking changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>

Author: JAORMX

pkg/config/cacert.go

@@ -3,7 +3,6 @@ package config
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"github.com/stacklok/toolhive/pkg/certs"
 )
@@ -17,18 +16,16 @@ import (
 //
 // The function returns an error if any validation fails or if updating the configuration fails.
 func setCACert(provider Provider, certPath string) error {
-	// Clean the path
-	certPath = filepath.Clean(certPath)
-
-	// Validate that the file exists and is readable
-	if _, err := os.Stat(certPath); err != nil {
-		return fmt.Errorf("CA certificate file not found or not accessible: %w", err)
+	// Validate and clean the file path
+	cleanPath, err := validateFilePath(certPath)
+	if err != nil {
+		return fmt.Errorf("CA certificate %w", err)
 	}
 
-	// Read and validate the certificate
-	certContent, err := os.ReadFile(certPath)
+	// Read the certificate
+	certContent, err := readFile(cleanPath)
 	if err != nil {
-		return fmt.Errorf("failed to read CA certificate file: %w", err)
+		return fmt.Errorf("CA certificate %w", err)
 	}
 
 	// Validate the certificate format
@@ -38,7 +35,7 @@ func setCACert(provider Provider, certPath string) error {
 
 	// Update the configuration
 	err = provider.UpdateConfig(func(c *Config) {
-		c.CACertificatePath = certPath
+		c.CACertificatePath = cleanPath
 	})
 	if err != nil {
 		return fmt.Errorf("failed to update configuration: %w", err)

pkg/config/registry.go

@@ -1,10 +1,7 @@
 package config
 
 import (
-	"encoding/json"
 	"fmt"
-	neturl "net/url"
-	"os"
 	"path/filepath"
 	"strings"
 
@@ -36,23 +33,13 @@ func DetectRegistryType(input string) (registryType string, cleanPath string) {
 
 // setRegistryURL validates and sets a registry URL using the provided provider
 func setRegistryURL(provider Provider, registryURL string, allowPrivateRegistryIp bool) error {
-	parsedURL, err := neturl.Parse(registryURL)
+	// Validate URL scheme
+	_, err := validateURLScheme(registryURL, allowPrivateRegistryIp)
 	if err != nil {
 		return fmt.Errorf("invalid registry URL: %w", err)
 	}
 
-	if allowPrivateRegistryIp {
-		// we validate either https or http URLs
-		if parsedURL.Scheme != networking.HttpScheme && parsedURL.Scheme != networking.HttpsScheme {
-			return fmt.Errorf("registry URL must start with http:// or https:// when allowing private IPs")
-		}
-	} else {
-		// we just allow https
-		if parsedURL.Scheme != networking.HttpsScheme {
-			return fmt.Errorf("registry URL must start with https:// when not allowing private IPs")
-		}
-	}
-
+	// Check for private IP addresses if not allowed
 	if !allowPrivateRegistryIp {
 		registryClient, err := networking.NewHttpClientBuilder().Build()
 		if err != nil {
@@ -79,33 +66,21 @@ func setRegistryURL(provider Provider, registryURL string, allowPrivateRegistryI
 
 // setRegistryFile validates and sets a local registry file using the provided provider
 func setRegistryFile(provider Provider, registryPath string) error {
-	// Validate that the file exists and is readable
-	if _, err := os.Stat(registryPath); err != nil {
-		return fmt.Errorf("local registry file not found or not accessible: %w", err)
-	}
-
-	// Basic validation - check if it's a JSON file
-	if !strings.HasSuffix(strings.ToLower(registryPath), ".json") {
-		return fmt.Errorf("registry file must be a JSON file (*.json)")
-	}
-
-	// Try to read and parse the file to validate it's a valid registry
-	// #nosec G304: File path is user-provided but validated above
-	registryContent, err := os.ReadFile(registryPath)
+	// Validate file path exists
+	cleanPath, err := validateFilePath(registryPath)
 	if err != nil {
-		return fmt.Errorf("failed to read registry file: %w", err)
+		return fmt.Errorf("local registry %w", err)
 	}
 
-	// Basic JSON validation
-	var registry map[string]interface{}
-	if err := json.Unmarshal(registryContent, &registry); err != nil {
-		return fmt.Errorf("invalid JSON format in registry file: %w", err)
+	// Validate JSON file
+	if err := validateJSONFile(cleanPath); err != nil {
+		return fmt.Errorf("registry file: %w", err)
 	}
 
 	// Make the path absolute
-	absPath, err := filepath.Abs(registryPath)
+	absPath, err := makeAbsolutePath(cleanPath)
 	if err != nil {
-		return fmt.Errorf("failed to resolve absolute path: %w", err)
+		return fmt.Errorf("registry file: %w", err)
 	}
 
 	// Update the configuration

pkg/config/validation.go

@@ -0,0 +1,113 @@
+package config
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	neturl "net/url"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/stacklok/toolhive/pkg/networking"
+)
+
+// Error message templates for consistent error formatting
+const (
+	errFileNotFound        = "file not found or not accessible: %w"
+	errFileRead            = "failed to read file: %w"
+	errInvalidJSON         = "invalid JSON format: %w"
+	errInvalidURL          = "invalid URL format: %w"
+	errInvalidURLScheme    = "URL must start with %s://"
+	errJSONExtensionOnly   = "file must be a JSON file (*.json)"
+	errAbsolutePathResolve = "failed to resolve absolute path: %w"
+)
+
+// validateFilePath validates that a file path exists and is accessible.
+// It also cleans the file path using filepath.Clean.
+// Returns the cleaned path and an error if the file doesn't exist or isn't accessible.
+func validateFilePath(path string) (string, error) {
+	cleanPath := filepath.Clean(path)
+
+	if _, err := os.Stat(cleanPath); err != nil {
+		return "", fmt.Errorf(errFileNotFound, err)
+	}
+
+	return cleanPath, nil
+}
+
+// validateFileExists checks if a file exists and is accessible without cleaning the path.
+// This is useful when the path has already been cleaned.
+func validateFileExists(path string) error {
+	if _, err := os.Stat(path); err != nil {
+		return fmt.Errorf(errFileNotFound, err)
+	}
+	return nil
+}
+
+// readFile reads the contents of a file and returns the data.
+// This is a wrapper around os.ReadFile with consistent error messaging.
+func readFile(path string) ([]byte, error) {
+	// #nosec G304: File path is user-provided but should be validated by caller
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf(errFileRead, err)
+	}
+	return data, nil
+}
+
+// validateJSONFile validates that a file contains valid JSON.
+// It checks the file extension and attempts to parse the content.
+func validateJSONFile(path string) error {
+	// Check file extension
+	if !strings.HasSuffix(strings.ToLower(path), ".json") {
+		return errors.New(errJSONExtensionOnly)
+	}
+
+	// Read and validate JSON content
+	data, err := readFile(path)
+	if err != nil {
+		return err
+	}
+
+	// Basic JSON validation - unmarshal into generic map
+	var jsonData map[string]interface{}
+	if err := json.Unmarshal(data, &jsonData); err != nil {
+		return fmt.Errorf(errInvalidJSON, err)
+	}
+
+	return nil
+}
+
+// validateURLScheme validates that a URL has the correct scheme (http or https).
+// If allowInsecure is false, only https is allowed.
+// If allowInsecure is true, both http and https are allowed.
+func validateURLScheme(rawURL string, allowInsecure bool) (*neturl.URL, error) {
+	parsedURL, err := neturl.Parse(rawURL)
+	if err != nil {
+		return nil, fmt.Errorf(errInvalidURL, err)
+	}
+
+	if allowInsecure {
+		// Allow both http and https
+		if parsedURL.Scheme != networking.HttpScheme && parsedURL.Scheme != networking.HttpsScheme {
+			return nil, fmt.Errorf("URL must start with http:// or https://")
+		}
+	} else {
+		// Only allow https
+		if parsedURL.Scheme != networking.HttpsScheme {
+			return nil, fmt.Errorf(errInvalidURLScheme, networking.HttpsScheme)
+		}
+	}
+
+	return parsedURL, nil
+}
+
+// makeAbsolutePath converts a relative path to an absolute path.
+func makeAbsolutePath(path string) (string, error) {
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		return "", fmt.Errorf(errAbsolutePathResolve, err)
+	}
+	return absPath, nil
+}