Extract early config validation from runServeCmd into testable helpers

cmd/fleet/serve.go

@@ -191,11 +191,8 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
 	}
 
 	// Validate OTEL server options
-	if config.Logging.OtelLogsEnabled && !config.Logging.TracingEnabled {
-		initFatal(
-			errors.New("logging.otel_logs_enabled requires logging.tracing_enabled to be true"),
-			"OTEL logs require tracing for trace correlation",
-		)
+	if err := validateOTELLoggingConfig(config); err != nil {
+		initFatal(err, "OTEL logs require tracing for trace correlation")
 	}
 
 	// Init OTEL providers (traces, metrics, logs)
@@ -317,36 +314,19 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
 		createTestBuckets(cmd.Context(), &config, logger)
 	}
 
-	allowedHostIdentifiers := map[string]bool{
-		"provided": true,
-		"instance": true,
-		"uuid":     true,
-		"hostname": true,
-	}
-	if !allowedHostIdentifiers[config.Osquery.HostIdentifier] {
-		initFatal(fmt.Errorf("%s is not a valid value for osquery_host_identifier", config.Osquery.HostIdentifier), "set host identifier")
+	if err := validateOsqueryHostIdentifier(config); err != nil {
+		initFatal(err, "set host identifier")
 	}
 
 	config.ConditionalAccess.Validate(initFatal)
 
-	if len(config.Server.URLPrefix) > 0 {
-		// Massage provided prefix to match expected format
-		config.Server.URLPrefix = strings.TrimSuffix(config.Server.URLPrefix, "/")
-		if len(config.Server.URLPrefix) > 0 && !strings.HasPrefix(config.Server.URLPrefix, "/") {
-			config.Server.URLPrefix = "/" + config.Server.URLPrefix
-		}
-
-		if !allowedURLPrefixRegexp.MatchString(config.Server.URLPrefix) {
-			initFatal(
-				fmt.Errorf("prefix must match regexp \"%s\"", allowedURLPrefixRegexp.String()),
-				"setting server URL prefix",
-			)
-		}
+	if err := normalizeAndValidateServerURLPrefix(&config); err != nil {
+		initFatal(err, "setting server URL prefix")
 	}
 
 	// Handle server private key configuration - either direct or via AWS Secrets Manager
-	if config.Server.PrivateKey != "" && config.Server.PrivateKeySecretArn != "" {
-		initFatal(errors.New("cannot specify both private_key and private_key_secret_arn"), "validate private key configuration")
+	if err := validateServerPrivateKeyExclusive(config); err != nil {
+		initFatal(err, "validate private key configuration")
 	}
 
 	// Retrieve private key from AWS Secrets Manager if specified
@@ -364,11 +344,10 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
 		config.Server.PrivateKey = privateKey
 	}
 
+	if err := validateServerPrivateKeyLength(config); err != nil {
+		initFatal(err, "validate private key")
+	}
 	if len(config.Server.PrivateKey) > 0 {
-		if len(config.Server.PrivateKey) < 32 {
-			initFatal(errors.New("private key must be at least 32 bytes long"), "validate private key")
-		}
-
 		// We truncate to 32 bytes because AES-256 requires a 32 byte (256 bit) PK, but some
 		// infra setups generate keys that are longer than 32 bytes.
 		config.Server.PrivateKey = config.Server.PrivateKey[:32]

cmd/fleet/serve_validation.go

@@ -0,0 +1,74 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/fleetdm/fleet/v4/server/config"
+)
+
+// validateServerPrivateKeyExclusive enforces that the server private key may
+// be supplied either directly via private_key or via private_key_arn (AWS
+// Secrets Manager), but not both.
+func validateServerPrivateKeyExclusive(cfg config.FleetConfig) error {
+	if cfg.Server.PrivateKey != "" && cfg.Server.PrivateKeySecretArn != "" {
+		return errors.New("cannot specify both private_key and private_key_secret_arn")
+	}
+	return nil
+}
+
+// validateServerPrivateKeyLength enforces a 32-byte minimum length on the
+// server private key when one is configured. The key is truncated to 32
+// bytes after this check because AES-256 requires a 32-byte key; rejecting
+// shorter inputs prevents a silently invalid AES-256 setup.
+func validateServerPrivateKeyLength(cfg config.FleetConfig) error {
+	if len(cfg.Server.PrivateKey) > 0 && len(cfg.Server.PrivateKey) < 32 {
+		return errors.New("private key must be at least 32 bytes long")
+	}
+	return nil
+}
+
+// validateOsqueryHostIdentifier rejects osquery_host_identifier values
+// outside the supported set. The osquery agent uses this to determine which
+// identifier is reported as the host UUID.
+func validateOsqueryHostIdentifier(cfg config.FleetConfig) error {
+	allowed := map[string]struct{}{
+		"provided": {},
+		"instance": {},
+		"uuid":     {},
+		"hostname": {},
+	}
+	if _, ok := allowed[cfg.Osquery.HostIdentifier]; !ok {
+		return fmt.Errorf("%s is not a valid value for osquery_host_identifier", cfg.Osquery.HostIdentifier)
+	}
+	return nil
+}
+
+// validateOTELLoggingConfig enforces the dependency between OTEL logs and
+// tracing: log records carry trace IDs only when tracing is enabled, so
+// enabling logs without tracing produces correlation-broken telemetry.
+func validateOTELLoggingConfig(cfg config.FleetConfig) error {
+	if cfg.Logging.OtelLogsEnabled && !cfg.Logging.TracingEnabled {
+		return errors.New("logging.otel_logs_enabled requires logging.tracing_enabled to be true")
+	}
+	return nil
+}
+
+// normalizeAndValidateServerURLPrefix trims a trailing slash, ensures a
+// leading slash, and validates the resulting prefix against
+// allowedURLPrefixRegexp. The mutation is done in place because the
+// normalized value is consumed downstream as part of route registration.
+func normalizeAndValidateServerURLPrefix(cfg *config.FleetConfig) error {
+	if len(cfg.Server.URLPrefix) == 0 {
+		return nil
+	}
+	cfg.Server.URLPrefix = strings.TrimSuffix(cfg.Server.URLPrefix, "/")
+	if len(cfg.Server.URLPrefix) > 0 && !strings.HasPrefix(cfg.Server.URLPrefix, "/") {
+		cfg.Server.URLPrefix = "/" + cfg.Server.URLPrefix
+	}
+	if !allowedURLPrefixRegexp.MatchString(cfg.Server.URLPrefix) {
+		return fmt.Errorf("prefix must match regexp %q", allowedURLPrefixRegexp.String())
+	}
+	return nil
+}

cmd/fleet/serve_validation_test.go

@@ -0,0 +1,165 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/config"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateServerPrivateKeyExclusive(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		privateKey       string
+		privateKeyArn    string
+		wantErrSubstring string
+	}{
+		{"both empty", "", "", ""},
+		{"direct key only", "some-direct-key-value", "", ""},
+		{"arn only", "", "arn:aws:secretsmanager:us-east-1:123:secret:foo", ""},
+		{
+			"both set rejected",
+			"some-direct-key-value",
+			"arn:aws:secretsmanager:us-east-1:123:secret:foo",
+			"cannot specify both private_key and private_key_secret_arn",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := config.FleetConfig{
+				Server: config.ServerConfig{
+					PrivateKey:          tc.privateKey,
+					PrivateKeySecretArn: tc.privateKeyArn,
+				},
+			}
+			err := validateServerPrivateKeyExclusive(cfg)
+			if tc.wantErrSubstring == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), tc.wantErrSubstring)
+		})
+	}
+}
+
+func TestValidateServerPrivateKeyLength(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		privateKey       string
+		wantErrSubstring string
+	}{
+		{"empty key", "", ""},
+		{"key under minimum length", strings.Repeat("x", 16), "at least 32 bytes long"},
+		{"key one byte under minimum", strings.Repeat("x", 31), "at least 32 bytes long"},
+		{"key at exactly minimum length", strings.Repeat("x", 32), ""},
+		{"key over minimum length", strings.Repeat("x", 64), ""},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := config.FleetConfig{
+				Server: config.ServerConfig{
+					PrivateKey: tc.privateKey,
+				},
+			}
+			err := validateServerPrivateKeyLength(cfg)
+			if tc.wantErrSubstring == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), tc.wantErrSubstring)
+		})
+	}
+}
+
+func TestValidateOsqueryHostIdentifier(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		identifier       string
+		wantErrSubstring string
+	}{
+		{"provided is allowed", "provided", ""},
+		{"instance is allowed", "instance", ""},
+		{"uuid is allowed", "uuid", ""},
+		{"hostname is allowed", "hostname", ""},
+		{"empty string rejected", "", "is not a valid value for osquery_host_identifier"},
+		{"unknown value rejected", "serial", "is not a valid value for osquery_host_identifier"},
+		{"case-sensitive: UUID rejected", "UUID", "is not a valid value for osquery_host_identifier"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := config.FleetConfig{
+				Osquery: config.OsqueryConfig{HostIdentifier: tc.identifier},
+			}
+			err := validateOsqueryHostIdentifier(cfg)
+			if tc.wantErrSubstring == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), tc.wantErrSubstring)
+		})
+	}
+}
+
+func TestValidateOTELLoggingConfig(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		otelLogs         bool
+		tracing          bool
+		wantErrSubstring string
+	}{
+		{"both disabled", false, false, ""},
+		{"tracing only", false, true, ""},
+		{"both enabled", true, true, ""},
+		{"otel logs without tracing rejected", true, false, "logging.otel_logs_enabled requires logging.tracing_enabled"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := config.FleetConfig{
+				Logging: config.LoggingConfig{
+					OtelLogsEnabled: tc.otelLogs,
+					TracingEnabled:  tc.tracing,
+				},
+			}
+			err := validateOTELLoggingConfig(cfg)
+			if tc.wantErrSubstring == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), tc.wantErrSubstring)
+		})
+	}
+}
+
+func TestNormalizeAndValidateServerURLPrefix(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		input            string
+		wantNormalized   string
+		wantErrSubstring string
+	}{
+		{"empty prefix is left alone", "", "", ""},
+		{"slash-prefixed value passes unchanged", "/fleet", "/fleet", ""},
+		{"leading slash added", "fleet", "/fleet", ""},
+		{"trailing slash trimmed", "/fleet/", "/fleet", ""},
+		{"both fixes applied", "fleet/", "/fleet", ""},
+		{"nested path is allowed", "/api/v1", "/api/v1", ""},
+		{"single slash trims to empty and is rejected by regex", "/", "", "must match regexp"},
+		{"invalid character rejected", "/fleet space", "", "must match regexp"},
+		{"query-style suffix rejected", "/fleet?x=1", "", "must match regexp"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := config.FleetConfig{
+				Server: config.ServerConfig{URLPrefix: tc.input},
+			}
+			err := normalizeAndValidateServerURLPrefix(&cfg)
+			if tc.wantErrSubstring == "" {
+				require.NoError(t, err)
+				require.Equal(t, tc.wantNormalized, cfg.Server.URLPrefix)
+				return
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), tc.wantErrSubstring)
+		})
+	}
+}