fix(security): Stop blocking legitimate clients on their first request

internal/security/detector.go

@@ -2,6 +2,8 @@ package security
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 	"sync"
 	"time"
 )
@@ -165,49 +167,75 @@ func (d *Detector) trackRequest(ip string, event *IngestEvent) bool {
 	return window.count > d.rateThreshold
 }
 
-// ShouldAutoBlock determines if an IP should be automatically blocked
-func (d *Detector) ShouldAutoBlock(ip string, event *SecurityEvent) bool {
-	// Always auto-block scanners
+// ShouldAutoBlock reports whether an IP should be auto-blocked, and when it
+// should, a human-readable reason naming the rule and the counts or paths that
+// tripped it, so a blocked IP carries a trace of what led to the block.
+func (d *Detector) ShouldAutoBlock(ip string, event *SecurityEvent) (bool, string) {
 	if event.EventType == EventTypeScannerDetected {
-		return true
+		return true, event.Message
 	}
-
-	// Auto-block high request rates
 	if event.EventType == EventTypeHighRequestRate {
-		return true
+		return true, event.Message
 	}
 
+	// The window fields (including pathHits) are mutated by trackRequest under
+	// the lock, so read them while holding it rather than through a bare pointer.
 	d.mu.RLock()
-	window, exists := d.ipRequestCount[ip]
-	d.mu.RUnlock()
+	defer d.mu.RUnlock()
 
+	window, exists := d.ipRequestCount[ip]
 	if !exists {
-		return false
+		return false, ""
 	}
 
-	// Auto-block after too many 404s (probing for files/paths)
 	if window.notFoundHits >= d.notFoundThreshold {
-		return true
+		return true, fmt.Sprintf("%d not-found responses in the detection window; paths: %s",
+			window.notFoundHits, summarizePaths(window.pathHits))
 	}
-
-	// Auto-block after too many auth failures
 	if window.authFailures >= d.authFailureThreshold {
-		return true
+		return true, fmt.Sprintf("%d authentication failures in the detection window", window.authFailures)
 	}
-
-	// Auto-block if trying too many unique paths (scanning)
 	if len(window.pathHits) >= d.uniquePathsThreshold {
-		return true
+		return true, fmt.Sprintf("probed %d distinct paths in the detection window; paths: %s",
+			len(window.pathHits), summarizePaths(window.pathHits))
 	}
-
-	// Auto-block if hammering same path repeatedly
-	for _, hits := range window.pathHits {
+	for path, hits := range window.pathHits {
 		if hits >= d.repeatedHitsThreshold {
-			return true
+			return true, fmt.Sprintf("%d requests to %s in the detection window", hits, path)
 		}
 	}
 
-	return false
+	return false, ""
+}
+
+// summarizePaths renders the busiest few paths in a window for a block reason,
+// most-hit first so the output is stable regardless of map order.
+func summarizePaths(pathHits map[string]int) string {
+	type ph struct {
+		path string
+		hits int
+	}
+	items := make([]ph, 0, len(pathHits))
+	for p, h := range pathHits {
+		items = append(items, ph{p, h})
+	}
+	sort.Slice(items, func(i, j int) bool {
+		if items[i].hits != items[j].hits {
+			return items[i].hits > items[j].hits
+		}
+		return items[i].path < items[j].path
+	})
+
+	const maxPaths = 3
+	parts := make([]string, 0, maxPaths)
+	for i, it := range items {
+		if i >= maxPaths {
+			parts = append(parts, fmt.Sprintf("and %d more", len(items)-maxPaths))
+			break
+		}
+		parts = append(parts, fmt.Sprintf("%s (%d)", it.path, it.hits))
+	}
+	return strings.Join(parts, ", ")
 }
 
 // CleanupOldWindows removes expired rate tracking windows

internal/security/manager.go

@@ -92,9 +92,9 @@ func (m *Manager) IngestEvent(event *IngestEvent, autoBlockDuration time.Duratio
 	result.Event = secEvent
 
 	// Check if we should auto-block the IP
-	if m.detector.ShouldAutoBlock(event.SourceIP, secEvent) {
+	if blocked, reason := m.detector.ShouldAutoBlock(event.SourceIP, secEvent); blocked {
 		expiresAt := time.Now().Add(autoBlockDuration)
-		_, err := m.db.BlockIP(event.SourceIP, "Auto-blocked due to suspicious activity", &expiresAt, true)
+		_, err := m.db.BlockIP(event.SourceIP, "Auto-blocked: "+reason, &expiresAt, true)
 		if err == nil {
 			result.AutoBlocked = true
 			result.BlockedIP = event.SourceIP

internal/security/manager_test.go

@@ -1,6 +1,7 @@
 package security
 
 import (
+	"strings"
 	"testing"
 	"time"
 )
@@ -51,6 +52,88 @@ func TestIngestEventAutoBlocksOnRepeatedAuthFailures(t *testing.T) {
 	}
 }
 
+// A legitimate client that identifies as a general-purpose HTTP tool must not be
+// blocked on its first request. This reproduces the reported bug where one 404
+// from curl / a Go or Python script locked the caller out immediately.
+func TestIngestEventDoesNotBlockGenericHTTPClient(t *testing.T) {
+	for _, ua := range []string{"curl/8.4.0", "Go-http-client/1.1", "python-requests/2.31.0", "Wget/1.21"} {
+		m := newTestManager(t)
+		result, err := m.IngestEvent(&IngestEvent{
+			SourceIP:      "203.0.113.20",
+			RequestPath:   "/wp-login.php",
+			RequestMethod: "GET",
+			StatusCode:    404,
+			UserAgent:     ua,
+		}, time.Hour)
+		if err != nil {
+			t.Fatalf("IngestEvent(%s): %v", ua, err)
+		}
+		if result.AutoBlocked {
+			t.Errorf("UA %q: a single 404 must not auto-block a general-purpose client", ua)
+		}
+	}
+}
+
+// A tool that names itself as an attack scanner is still blocked on sight.
+func TestIngestEventBlocksNamedScanner(t *testing.T) {
+	m := newTestManager(t)
+	result, err := m.IngestEvent(&IngestEvent{
+		SourceIP:      "203.0.113.21",
+		RequestPath:   "/",
+		RequestMethod: "GET",
+		StatusCode:    200,
+		UserAgent:     "sqlmap/1.7.2#stable",
+	}, time.Hour)
+	if err != nil {
+		t.Fatalf("IngestEvent: %v", err)
+	}
+	if !result.AutoBlocked {
+		t.Fatal("a named scanner user agent must still be auto-blocked")
+	}
+}
+
+// The persisted block records what tripped it: the rule, the count, and the paths.
+func TestIngestEventBlockReasonTracesTrigger(t *testing.T) {
+	m := newTestManager(t)
+
+	var last *IngestResult
+	for i := 0; i < 10; i++ {
+		var err error
+		last, err = m.IngestEvent(&IngestEvent{
+			SourceIP:      "203.0.113.22",
+			RequestPath:   "/missing.php",
+			RequestMethod: "GET",
+			StatusCode:    404,
+			UserAgent:     "Mozilla/5.0",
+		}, time.Hour)
+		if err != nil {
+			t.Fatalf("IngestEvent: %v", err)
+		}
+	}
+	if !last.AutoBlocked {
+		t.Fatal("expected block after repeated 404 probing")
+	}
+
+	blocked, err := m.GetBlockedIPs()
+	if err != nil {
+		t.Fatalf("GetBlockedIPs: %v", err)
+	}
+	var reason string
+	for _, b := range blocked {
+		if b.IP == "203.0.113.22" {
+			reason = b.Reason
+		}
+	}
+	if reason == "" {
+		t.Fatal("blocked IP not found")
+	}
+	for _, want := range []string{"not-found", "/missing.php"} {
+		if !strings.Contains(reason, want) {
+			t.Errorf("block reason should trace the trigger, missing %q in %q", want, reason)
+		}
+	}
+}
+
 func TestIngestEventSkipsWhitelistedIP(t *testing.T) {
 	m := newTestManager(t)
 

internal/security/rules.go

@@ -122,7 +122,11 @@ var SuspiciousPaths = []string{
 	"/clientaccesspolicy.xml",
 }
 
-// Scanner user agent patterns
+// ScannerUserAgents match on sight and block immediately, so this list holds
+// only attack tools that name themselves. General-purpose HTTP clients (curl,
+// wget, python-requests, go-http-client) are deliberately absent: real scripts
+// and health checks use them, so matching them here blocked legitimate callers
+// on their first request. They are still caught by the volumetric thresholds.
 var ScannerUserAgents = []string{
 	"nikto",
 	"nmap",
@@ -144,12 +148,6 @@ var ScannerUserAgents = []string{
 	"joomscan",
 	"droopescan",
 	"zgrab",
-	"curl/",
-	"wget/",
-	"python-requests",
-	"go-http-client",
-	"httpx",
-	"nuclei",
 }
 
 var suspiciousPathPatterns []*regexp.Regexp