chore: move allowed origins and hosts validation into the httpapi package

Author: hugodutka

cmd/server/server.go

@@ -6,11 +6,9 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
-	"net/url"
 	"os"
 	"sort"
 	"strings"
-	"unicode"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -60,79 +58,6 @@ func parseAgentType(firstArg string, agentTypeVar string) (AgentType, error) {
 	return AgentTypeCustom, nil
 }
 
-// Validate allowed hosts don't contain whitespace, commas, schemes, or ports.
-// Viper/Cobra use different separators (space for env vars, comma for flags),
-// so these characters likely indicate user error.
-func validateAllowedHosts(input []string) error {
-	if len(input) == 0 {
-		return fmt.Errorf("the list must not be empty")
-	}
-	// First pass: whitespace & comma checks (surface these errors first)
-	for _, item := range input {
-		for _, r := range item {
-			if unicode.IsSpace(r) {
-				return fmt.Errorf("'%s' contains whitespace characters, which are not allowed", item)
-			}
-		}
-		if strings.Contains(item, ",") {
-			return fmt.Errorf("'%s' contains comma characters, which are not allowed", item)
-		}
-	}
-	// Second pass: scheme check
-	for _, item := range input {
-		if strings.Contains(item, "http://") || strings.Contains(item, "https://") {
-			return fmt.Errorf("'%s' must not include http:// or https://", item)
-		}
-	}
-	// Third pass: port check (but allow IPv6 literals without ports)
-	for _, item := range input {
-		trimmed := strings.TrimSpace(item)
-		colonCount := strings.Count(trimmed, ":")
-		// If bracketed, rely on url.Parse to detect a port in "]:<port>" form.
-		if strings.HasPrefix(trimmed, "[") {
-			if u, err := url.Parse("http://" + trimmed); err == nil {
-				if u.Port() != "" {
-					return fmt.Errorf("'%s' must not include a port", item)
-				}
-			}
-			continue
-		}
-		// Unbracketed IPv6: multiple colons and no brackets; treat as valid (no ports allowed here)
-		if colonCount >= 2 {
-			continue
-		}
-		// IPv4 or hostname: if URL parsing finds a port or there's a single colon, it's invalid
-		if u, err := url.Parse("http://" + trimmed); err == nil {
-			if u.Port() != "" {
-				return fmt.Errorf("'%s' must not include a port", item)
-			}
-		}
-		if colonCount == 1 {
-			return fmt.Errorf("'%s' must not include a port", item)
-		}
-	}
-	return nil
-}
-
-// Validate allowed origins don't contain whitespace or commas.
-// Origins must include a scheme, validated later by the HTTP layer.
-func validateAllowedOrigins(input []string) error {
-	if len(input) == 0 {
-		return fmt.Errorf("the list must not be empty")
-	}
-	for _, item := range input {
-		for _, r := range item {
-			if unicode.IsSpace(r) {
-				return fmt.Errorf("'%s' contains whitespace characters, which are not allowed", item)
-			}
-		}
-		if strings.Contains(item, ",") {
-			return fmt.Errorf("'%s' contains comma characters, which are not allowed", item)
-		}
-	}
-	return nil
-}
-
 func runServer(ctx context.Context, logger *slog.Logger, argsToPass []string) error {
 	agent := argsToPass[0]
 	agentTypeValue := viper.GetString(FlagType)
@@ -247,17 +172,6 @@ func CreateServerCmd() *cobra.Command {
 		Short: "Run the server",
 		Long:  fmt.Sprintf("Run the server with the specified agent (one of: %s)", strings.Join(agentNames, ", ")),
 		Args:  cobra.MinimumNArgs(1),
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			allowedHosts := viper.GetStringSlice(FlagAllowedHosts)
-			if err := validateAllowedHosts(allowedHosts); err != nil {
-				return xerrors.Errorf("failed to validate allowed hosts: %w", err)
-			}
-			allowedOrigins := viper.GetStringSlice(FlagAllowedOrigins)
-			if err := validateAllowedOrigins(allowedOrigins); err != nil {
-				return xerrors.Errorf("failed to validate allowed origins: %w", err)
-			}
-			return nil
-		},
 		Run: func(cmd *cobra.Command, args []string) {
 			// The --exit flag is used for testing validation of flags in the test suite
 			if viper.GetBool(FlagExit) {
@@ -280,7 +194,7 @@ func CreateServerCmd() *cobra.Command {
 		{FlagTermWidth, "W", uint16(80), "Width of the emulated terminal", "uint16"},
 		{FlagTermHeight, "H", uint16(1000), "Height of the emulated terminal", "uint16"},
 		// localhost is the default host for the server. Port is ignored during matching.
-		{FlagAllowedHosts, "a", []string{"localhost"}, "HTTP allowed hosts (hostnames only, no ports). Use '*' for all, comma-separated list via flag, space-separated list via AGENTAPI_ALLOWED_HOSTS env var", "stringSlice"},
+		{FlagAllowedHosts, "a", []string{"localhost", "127.0.0.1", "[::1]"}, "HTTP allowed hosts (hostnames only, no ports). Use '*' for all, comma-separated list via flag, space-separated list via AGENTAPI_ALLOWED_HOSTS env var", "stringSlice"},
 		// localhost:3284 is the default origin when you open the chat interface in your browser. localhost:3000 and 3001 are used during development.
 		{FlagAllowedOrigins, "o", []string{"http://localhost:3284", "http://localhost:3000", "http://localhost:3001"}, "HTTP allowed origins. Use '*' for all, comma-separated list via flag, space-separated list via AGENTAPI_ALLOWED_ORIGINS env var", "stringSlice"},
 	}

cmd/server/server_test.go

@@ -155,7 +155,7 @@ func TestServerCmd_AllArgs_Defaults(t *testing.T) {
 		{"chat-base-path default", FlagChatBasePath, "/chat", func() any { return viper.GetString(FlagChatBasePath) }},
 		{"term-width default", FlagTermWidth, uint16(80), func() any { return viper.GetUint16(FlagTermWidth) }},
 		{"term-height default", FlagTermHeight, uint16(1000), func() any { return viper.GetUint16(FlagTermHeight) }},
-		{"allowed-hosts default", FlagAllowedHosts, []string{"localhost"}, func() any { return viper.GetStringSlice(FlagAllowedHosts) }},
+		{"allowed-hosts default", FlagAllowedHosts, []string{"localhost", "127.0.0.1", "[::1]"}, func() any { return viper.GetStringSlice(FlagAllowedHosts) }},
 		{"allowed-origins default", FlagAllowedOrigins, []string{"http://localhost:3284", "http://localhost:3000", "http://localhost:3001"}, func() any { return viper.GetStringSlice(FlagAllowedOrigins) }},
 	}
 
@@ -341,37 +341,6 @@ func TestServerCmd_AllowedHosts(t *testing.T) {
 			args:     []string{},
 			expected: []string{"localhost", "example.com"},
 		},
-		{
-			name:        "env: host with comma (invalid)",
-			env:         map[string]string{"AGENTAPI_ALLOWED_HOSTS": "localhost:3284,example.com"},
-			args:        []string{},
-			expectedErr: "contains comma characters",
-		},
-		{
-			name:        "env: host with port (invalid)",
-			env:         map[string]string{"AGENTAPI_ALLOWED_HOSTS": "localhost:3284"},
-			args:        []string{},
-			expectedErr: "must not include a port",
-		},
-        {
-            name:     "env: ipv6 literal",
-            env:      map[string]string{"AGENTAPI_ALLOWED_HOSTS": "2001:db8::1"},
-            args:     []string{},
-            expected: []string{"2001:db8::1"},
-        },
-        {
-            name:     "env: ipv6 bracketed literal",
-            env:      map[string]string{"AGENTAPI_ALLOWED_HOSTS": "[2001:db8::1]"},
-            args:     []string{},
-            expected: []string{"[2001:db8::1]"},
-        },
-        {
-            name:        "env: ipv6 with port (invalid)",
-            env:         map[string]string{"AGENTAPI_ALLOWED_HOSTS": "[2001:db8::1]:443"},
-            args:        []string{},
-            expectedErr: "must not include a port",
-        },
-
 		// CLI flag scenarios (comma-separated format)
 		{
 			name:     "flag: single valid host",
@@ -394,30 +363,10 @@ func TestServerCmd_AllowedHosts(t *testing.T) {
 			expected: []string{"localhost"},
 		},
 		{
-			name:        "flag: host with space in comma-separated list (invalid)",
-			args:        []string{"--allowed-hosts", "localhost:3284,example .com"},
-			expectedErr: "contains whitespace characters",
-		},
-		{
-			name:        "flag: host with port (invalid)",
-			args:        []string{"--allowed-hosts", "localhost:3284"},
-			expectedErr: "must not include a port",
+			name:     "flag: ipv6 bracketed literal",
+			args:     []string{"--allowed-hosts", "[2001:db8::1]"},
+			expected: []string{"[2001:db8::1]"},
 		},
-        {
-            name:     "flag: ipv6 literal",
-            args:     []string{"--allowed-hosts", "2001:db8::1"},
-            expected: []string{"2001:db8::1"},
-        },
-        {
-            name:     "flag: ipv6 bracketed literal",
-            args:     []string{"--allowed-hosts", "[2001:db8::1]"},
-            expected: []string{"[2001:db8::1]"},
-        },
-        {
-            name:        "flag: ipv6 with port (invalid)",
-            args:        []string{"--allowed-hosts", "[2001:db8::1]:443"},
-            expectedErr: "must not include a port",
-        },
 
 		// Mixed scenarios (env + flag precedence)
 		{
@@ -426,26 +375,6 @@ func TestServerCmd_AllowedHosts(t *testing.T) {
 			args:     []string{"--allowed-hosts", "override.com"},
 			expected: []string{"override.com"},
 		},
-		{
-			name:        "mixed: flag overrides env but flag is invalid",
-			env:         map[string]string{"AGENTAPI_ALLOWED_HOSTS": "localhost"},
-			args:        []string{"--allowed-hosts", "invalid .com"},
-			expectedErr: "contains whitespace characters",
-		},
-
-		// Empty hosts are not allowed
-		{
-			name:        "empty host",
-			args:        []string{"--allowed-hosts", ""},
-			expectedErr: "the list must not be empty",
-		},
-
-		// Default behavior
-		{
-			name:     "default hosts when neither env nor flag provided",
-			args:     []string{},
-			expected: []string{"localhost"},
-		},
 	}
 
 	for _, tt := range tests {
@@ -506,12 +435,6 @@ func TestServerCmd_AllowedOrigins(t *testing.T) {
 			args:     []string{},
 			expected: []string{"https://example.com", "http://localhost:3000"},
 		},
-		{
-			name:        "env: origin with comma (invalid)",
-			env:         map[string]string{"AGENTAPI_ALLOWED_ORIGINS": "https://example.com,http://localhost:3000"},
-			args:        []string{},
-			expectedErr: "contains comma characters",
-		},
 
 		// CLI flag scenarios (comma-separated format)
 		{
@@ -539,11 +462,6 @@ func TestServerCmd_AllowedOrigins(t *testing.T) {
 			args:     []string{"--allowed-origins", "https://example.com\n"},
 			expected: []string{"https://example.com"},
 		},
-		{
-			name:        "flag: origin with space in comma-separated list (invalid)",
-			args:        []string{"--allowed-origins", "https://example.com,http://localhost :3000"},
-			expectedErr: "contains whitespace characters",
-		},
 
 		// Mixed scenarios (env + flag precedence)
 		{
@@ -552,26 +470,6 @@ func TestServerCmd_AllowedOrigins(t *testing.T) {
 			args:     []string{"--allowed-origins", "https://override.com"},
 			expected: []string{"https://override.com"},
 		},
-		{
-			name:        "mixed: flag overrides env but flag is invalid",
-			env:         map[string]string{"AGENTAPI_ALLOWED_ORIGINS": "https://env-example.com"},
-			args:        []string{"--allowed-origins", "invalid origin"},
-			expectedErr: "contains whitespace characters",
-		},
-
-		// Empty origins are not allowed
-		{
-			name:        "empty origin",
-			args:        []string{"--allowed-origins", ""},
-			expectedErr: "the list must not be empty",
-		},
-
-		// Default behavior
-		{
-			name:     "default origins when neither env nor flag provided",
-			args:     []string{},
-			expected: []string{"http://localhost:3284", "http://localhost:3000", "http://localhost:3001"},
-		},
 	}
 
 	for _, tt := range tests {