fix(conf): harden settings validation for security and correctness (#2876)

* fix(conf): harden settings validation for security and correctness

Tighten input validation across settings to reject invalid values that
were previously accepted silently:

- Reject zero and negative realtime intervals, cap at 24h
- Strip HTML tags from site name (defense in depth against stored XSS)
- Reject path traversal (..) and absolute paths in audio export path
- Validate EQ filter frequency > 0 and Q > 0 for both global and
  per-source equalizers, with upper bounds
- Add dynamic threshold cross-field validation: min <= trigger,
  validHours > 0 when enabled, both in [0,1] range
- Default empty BirdNET locale to "en" instead of leaving it blank
- Validate weather provider against known values
- Reject negative retention maxAge (parsed duration) and negative
  minClips regardless of retention policy
- Fix pre-existing goconst lint issue in test_helpers.go

* fix(api): update test settings to satisfy stricter validation

The validation hardening requires positive realtime interval, non-empty
BirdNET locale, and relative export paths. Update the API v2 test
helper to provide valid defaults so existing PATCH tests continue to
pass with the stricter checks.

* fix(conf): address review feedback on validation hardening

- Move MinClips check above empty-policy early return so negative
  values are rejected regardless of retention policy
- Rename misleading test case from "windows absolute rejected" to
  "windows-style path treated as relative on unix"

* fix(conf): add per-stream EQ validation and update test expectations

Add EQ filter validation for per-stream equalizer settings (was missing
after per-source and global EQ were covered). Update malicious input
test to accept 400 responses for path traversal (now correctly rejected
at validation time). Update export path assertion in settings update
test to match relative path default.

* fix(conf): use require.NoError in validation tests and remove unused import

- Replace ignored ValidateSettings return with require.NoError so
  tests abort on unexpected validation failures
- Add SessionDuration to createMinimalValidSettings for completeness
- Remove unused math import from validate_audio.go

* fix(conf): reject NaN floats and null bytes in validation

Address review feedback from Gemini peer review:

- Reject NaN values in EQ filter frequency/Q and dynamic threshold
  trigger/min fields. NaN comparisons always return false in Go, so
  NaN would bypass all range checks. YAML supports .nan literals.
- Reject null bytes in export path. While Go's os package rejects
  them at runtime, catching them at validation time gives a clear
  error message.
- Add tests for NaN and null byte edge cases.

* fix(api): remove synctest from settings concurrent tests

handleSettingsChanges spawns a background goroutine with time.Sleep
between control channel sends. When this runs inside a synctest bubble,
the sleeping goroutine outlives the test function and triggers the
deadlock detector: "main bubble goroutine has exited but blocked
goroutines remain".

Replace synctest.Test with regular WaitGroup.Go for the two affected
scenarios. These tests verify data consistency under concurrent access,
not timing behavior, so synctest provides no benefit here.

* fix(conf): treat zero realtime interval as default for upgrade compat

Existing configs may have interval: 0 explicitly set. Rather than
failing validation on upgrade, coerce 0 to the default (15s) with a
warning log, matching the SpeciesConfig.Interval pattern where 0 means
"use default". Negative values remain rejected.

Author: tphakala

internal/api/v2/settings_concurrent_test.go

@@ -21,7 +21,6 @@ import (
 	"net/http/httptest"
 	"sync"
 	"testing"
-	"testing/synctest"
 
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
@@ -260,78 +259,66 @@ func TestRaceConditionScenarios(t *testing.T) {
 			description: "Verify reads during writes return consistent data",
 			scenario: func(t *testing.T, controller *Controller) {
 				t.Helper()
-				// Use synctest.Test for deterministic read/write concurrency (Go 1.25)
-				// This ensures predictable execution order without timing dependencies
-				synctest.Test(t, func(t *testing.T) {
-					t.Helper()
-					var wg sync.WaitGroup
+				// Cannot use synctest.Test here: handleSettingsChanges spawns a
+				// background goroutine with time.Sleep that outlives the test
+				// function, causing synctest's deadlock detector to fire.
+				var wg sync.WaitGroup
 
-					// Start a write using WaitGroup.Go() (Go 1.25)
+				wg.Go(func() {
+					update := map[string]any{
+						"summaryLimit": 999,
+					}
+					_ = makeSettingsUpdate(t, controller, "dashboard", update)
+				})
+
+				for range 10 {
 					wg.Go(func() {
-						update := map[string]any{
-							"summaryLimit": 999,
-						}
-						_ = makeSettingsUpdate(t, controller, "dashboard", update)
+						req := httptest.NewRequest(http.MethodGet, "/api/v2/settings/dashboard", http.NoBody)
+						rec := httptest.NewRecorder()
+						ctx := controller.Echo.NewContext(req, rec)
+						ctx.SetParamNames("section")
+						ctx.SetParamValues("dashboard")
+
+						err := controller.GetSectionSettings(ctx)
+						require.NoError(t, err)
+						assert.Equal(t, http.StatusOK, rec.Code)
+
+						var response map[string]any
+						err = json.Unmarshal(rec.Body.Bytes(), &response)
+						require.NoError(t, err)
 					})
+				}
 
-					// Perform multiple reads concurrently with the write
-					for range 10 {
-						wg.Go(func() {
-							req := httptest.NewRequest(http.MethodGet, "/api/v2/settings/dashboard", http.NoBody)
-							rec := httptest.NewRecorder()
-							ctx := controller.Echo.NewContext(req, rec)
-							ctx.SetParamNames("section")
-							ctx.SetParamValues("dashboard")
-
-							err := controller.GetSectionSettings(ctx)
-							require.NoError(t, err)
-							assert.Equal(t, http.StatusOK, rec.Code)
-
-							// Parse response to verify it's valid JSON
-							var response map[string]any
-							err = json.Unmarshal(rec.Body.Bytes(), &response)
-							require.NoError(t, err)
-						})
-					}
-
-					wg.Wait()
-					synctest.Wait() // Ensure all bubble goroutines complete
-				})
+				wg.Wait()
 			},
 		},
 		{
 			name:        "Conflicting updates to nested fields",
 			description: "Verify nested field updates don't corrupt parent objects",
 			scenario: func(t *testing.T, controller *Controller) {
 				t.Helper()
-				// Use synctest.Test for deterministic nested field updates (Go 1.25)
-				synctest.Test(t, func(t *testing.T) {
-					t.Helper()
-					var wg sync.WaitGroup
-
-					// Update different nested fields concurrently using WaitGroup.Go() (Go 1.25)
-					wg.Go(func() {
-						update := map[string]any{
-							"thumbnails": map[string]any{
-								"summary": true,
-							},
-						}
-						_ = makeSettingsUpdate(t, controller, "dashboard", update)
-					})
+				var wg sync.WaitGroup
 
-					wg.Go(func() {
-						update := map[string]any{
-							"thumbnails": map[string]any{
-								"recent": false,
-							},
-						}
-						_ = makeSettingsUpdate(t, controller, "dashboard", update)
-					})
+				wg.Go(func() {
+					update := map[string]any{
+						"thumbnails": map[string]any{
+							"summary": true,
+						},
+					}
+					_ = makeSettingsUpdate(t, controller, "dashboard", update)
+				})
 
-					wg.Wait()
-					synctest.Wait() // Ensure all bubble goroutines complete
+				wg.Go(func() {
+					update := map[string]any{
+						"thumbnails": map[string]any{
+							"recent": false,
+						},
+					}
+					_ = makeSettingsUpdate(t, controller, "dashboard", update)
 				})
 
+				wg.Wait()
+
 				// Verify both fields were updated
 				settings := controller.Settings
 				assert.NotNil(t, settings.Realtime.Dashboard.Thumbnails)

internal/api/v2/settings_malicious_test.go

@@ -49,7 +49,7 @@ func TestMaliciousInputData(t *testing.T) {
 					"path": "../../../etc/passwd",
 				},
 			},
-			description: "Should accept path but file operations should validate",
+			description: "Should reject path traversal at validation time",
 		},
 		{
 			name:    "Command injection attempt",
@@ -139,17 +139,24 @@ func TestMaliciousInputData(t *testing.T) {
 			ctx.SetParamNames("section")
 			ctx.SetParamValues(tt.section)
 
-			// The update should succeed (we accept the data)
+			// The update should either succeed or reject malicious input
 			err = controller.UpdateSectionSettings(ctx)
 			if err != nil {
-				// Some malicious inputs might be rejected, which is also fine
+				// Some malicious inputs might be rejected via error return, which is fine
 				if httpErr, ok := errors.AsType[*echo.HTTPError](err); ok && httpErr.Code == http.StatusBadRequest {
-					t.Logf("Input rejected as expected: %v", err)
+					t.Logf("Input rejected via error: %v", err)
 					return
 				}
 			}
 
-			// If accepted, verify the response is valid
+			// Validation may also reject via response code (HandleError writes 400
+			// to recorder without returning an error). Both 200 and 400 are acceptable
+			// outcomes for malicious input: 200 means the data was safely accepted,
+			// 400 means it was correctly rejected at validation time.
+			if rec.Code == http.StatusBadRequest {
+				t.Logf("Input rejected at validation: status %d", rec.Code)
+				return
+			}
 			assert.Equal(t, http.StatusOK, rec.Code)
 			t.Logf("%s: %s", tt.name, tt.description)
 		})

internal/api/v2/settings_update_test.go

@@ -419,7 +419,7 @@ func TestAudioExportPartialUpdate(t *testing.T) {
 	settings := controller.Settings
 	assert.Equal(t, "mp3", settings.Realtime.Audio.Export.Type)     // Changed
 	assert.True(t, settings.Realtime.Audio.Export.Enabled)          // Preserved
-	assert.Equal(t, "/clips", settings.Realtime.Audio.Export.Path)  // Preserved
+	assert.Equal(t, "clips", settings.Realtime.Audio.Export.Path)   // Preserved
 	assert.Equal(t, "192k", settings.Realtime.Audio.Export.Bitrate) // Preserved
 }
 

internal/api/v2/test_helpers.go

@@ -43,6 +43,7 @@ func getTestSettings(t *testing.T) *conf.Settings {
 	settings := &conf.Settings{}
 
 	// Initialize with valid defaults
+	settings.Realtime.Interval = 15                // Must be positive after hardening
 	settings.Realtime.Dashboard.SummaryLimit = 100 // Valid range: 10-1000
 	settings.Realtime.Dashboard.Thumbnails.Summary = true
 	settings.Realtime.Dashboard.Thumbnails.Recent = true
@@ -63,6 +64,7 @@ func getTestSettings(t *testing.T) *conf.Settings {
 	settings.BirdNET.Longitude = testNewYorkLongitude
 	settings.BirdNET.Sensitivity = 1.0
 	settings.BirdNET.Threshold = 0.8
+	settings.BirdNET.Locale = "en"
 	settings.BirdNET.RangeFilter.Model = "latest"
 	settings.BirdNET.RangeFilter.Threshold = 0.03
 
@@ -73,7 +75,7 @@ func getTestSettings(t *testing.T) *conf.Settings {
 	}}
 	settings.Realtime.Audio.Export.Enabled = true
 	settings.Realtime.Audio.Export.Type = "wav"
-	settings.Realtime.Audio.Export.Path = "/clips"
+	settings.Realtime.Audio.Export.Path = "clips"
 	settings.Realtime.Audio.Export.Bitrate = "192k"
 	settings.Realtime.Audio.Export.Length = 15
 

internal/conf/audio_source_validation_test.go

@@ -217,7 +217,7 @@ func TestAudioSourceConfig_Validate_PerSourceEQ(t *testing.T) {
 		Equalizer: &EqualizerSettings{
 			Enabled: true,
 			Filters: []EqualizerFilter{
-				{Type: "HighPass", Frequency: 100},
+				{Type: "HighPass", Frequency: 100, Q: 0.707},
 			},
 		},
 	}

internal/conf/test_helpers.go

@@ -29,7 +29,7 @@ func GetTestSettings() *Settings {
 	settings.Realtime.Dashboard.Thumbnails.Summary = false
 	settings.Realtime.Dashboard.Thumbnails.Recent = true
 	settings.Realtime.Dashboard.Thumbnails.ImageProvider = "avicommons"
-	settings.Realtime.Dashboard.Thumbnails.FallbackPolicy = "none"
+	settings.Realtime.Dashboard.Thumbnails.FallbackPolicy = RetentionPolicyNone
 
 	// Other realtime settings
 	settings.Realtime.Interval = 15

internal/conf/validate.go

@@ -29,6 +29,15 @@ var validRetentionPolicies = []string{
 	RetentionPolicyUsage,
 }
 
+// htmlTagPattern matches HTML tags for sanitization.
+var htmlTagPattern = regexp.MustCompile(`<[^>]*>`)
+
+// sanitizeStringField strips HTML tags from a string field as defense in depth.
+// Returns the sanitized string.
+func sanitizeStringField(s string) string {
+	return htmlTagPattern.ReplaceAllString(s, "")
+}
+
 // Precompiled regular expressions for validation
 var (
 	// birdweatherIDPattern validates Birdweather ID format (24 alphanumeric characters)
@@ -153,6 +162,15 @@ func firstValidationError(result ValidationResult, validationType string) error
 func ValidateSettings(settings *Settings) error {
 	ve := ValidationError{}
 
+	// Sanitize Main.Name: strip HTML tags as defense in depth
+	settings.Main.Name = sanitizeStringField(settings.Main.Name)
+
+	// Default empty BirdNET locale to "en" so downstream label loading
+	// always has a valid locale to work with.
+	if settings.BirdNET.Locale == "" {
+		settings.BirdNET.Locale = "en"
+	}
+
 	// Validate BirdNET settings
 	if err := validateBirdNETSettings(&settings.BirdNET); err != nil {
 		ve.Errors = append(ve.Errors, err.Error())