Fix: Address security alerts and bump version to v0.0.9

Author: fabriziosalmi

caddywaf.go

@@ -48,7 +48,7 @@ var (
 )
 
 // Add or update the version constant as needed
-const wafVersion = "v0.0.8" // update this value to the new release version when tagging
+const wafVersion = "v0.0.9" // update this value to the new release version when tagging
 
 // ==================== Initialization and Setup ====================
 

handler.go

@@ -182,7 +182,7 @@ func (m *Middleware) handleResponseBodyPhase(recorder *responseRecorder, r *http
 
 	for _, rule := range rules {
 		if rule.regex.MatchString(body) {
-			if m.processRuleMatch(recorder, r, &rule, body, state) {
+			if m.processRuleMatch(recorder, r, &rule, "RESPONSE_BODY", body, state) { // Pass RESPONSE_BODY as target
 				return
 			}
 		}
@@ -453,29 +453,31 @@ func (m *Middleware) handlePhase(w http.ResponseWriter, r *http.Request, phase i
 				continue
 			}
 
+			redactedValue := m.requestValueExtractor.RedactValueIfSensitive(target, value)
+
 			m.logger.Debug("Extracted value",
 				zap.String("rule_id", rule.ID),
 				zap.String("target", target),
-				zap.String("value", value),
+				zap.String("value", redactedValue),
 			)
 
 			if rule.regex.MatchString(value) {
 				m.logger.Debug("Rule matched",
 					zap.String("rule_id", rule.ID),
 					zap.String("target", target),
-					zap.String("value", value),
+					zap.String("value", redactedValue),
 				)
 
 				// FIXED: Correctly interpret processRuleMatch return value
 				var shouldContinue bool
 				if phase == 3 || phase == 4 {
 					if recorder, ok := w.(*responseRecorder); ok {
-						shouldContinue = m.processRuleMatch(recorder, r, &rule, value, state)
+						shouldContinue = m.processRuleMatch(recorder, r, &rule, target, value, state)
 					} else {
-						shouldContinue = m.processRuleMatch(w, r, &rule, value, state)
+						shouldContinue = m.processRuleMatch(w, r, &rule, target, value, state)
 					}
 				} else {
-					shouldContinue = m.processRuleMatch(w, r, &rule, value, state)
+					shouldContinue = m.processRuleMatch(w, r, &rule, target, value, state)
 				}
 
 				// If processRuleMatch returned false or state is now blocked, stop processing
@@ -496,7 +498,7 @@ func (m *Middleware) handlePhase(w http.ResponseWriter, r *http.Request, phase i
 				m.logger.Debug("Rule did not match",
 					zap.String("rule_id", rule.ID),
 					zap.String("target", target),
-					zap.String("value", value),
+					zap.String("value", redactedValue),
 				)
 			}
 		}

handler_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"mime/multipart"
+	"os"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
@@ -26,8 +27,9 @@ func TestBlockedRequestPhase1_DNSBlacklist(t *testing.T) {
 		dnsBlacklist: map[string]struct{}{
 			"malicious.domain": {},
 		},
-		ipBlacklist:     iptrie.NewTrie(),
-		CustomResponses: customResponse,
+		ipBlacklist:           iptrie.NewTrie(),
+		CustomResponses:       customResponse,
+		requestValueExtractor: NewRequestValueExtractor(logger, false),
 	}
 
 	w := httptest.NewRecorder()
@@ -60,6 +62,9 @@ func TestBlockedRequestPhase1_DNSBlacklist(t *testing.T) {
 }
 
 func TestBlockedRequestPhase1_GeoIPBlocking(t *testing.T) {
+	if _, err := os.Stat(geoIPdata); os.IsNotExist(err) {
+		t.Skip("GeoIP database not found, skipping test")
+	}
 	logger, err := zap.NewDevelopment()
 	assert.NoError(t, err)
 
@@ -77,7 +82,8 @@ func TestBlockedRequestPhase1_GeoIPBlocking(t *testing.T) {
 			GeoIPDBPath: geoIPdata, // Path to a test GeoIP database
 			geoIP:       geoIPBlock,
 		},
-		CustomResponses: customResponse,
+		CustomResponses:       customResponse,
+		requestValueExtractor: NewRequestValueExtractor(logger, false),
 	}
 
 	wlMiddleware := &Middleware{
@@ -90,7 +96,8 @@ func TestBlockedRequestPhase1_GeoIPBlocking(t *testing.T) {
 			GeoIPDBPath: geoIPdata, // Path to a test GeoIP database
 			geoIP:       geoIPBlock,
 		},
-		CustomResponses: customResponse,
+		CustomResponses:       customResponse,
+		requestValueExtractor: NewRequestValueExtractor(logger, false),
 	}
 
 	blackWhiteMw := &Middleware{
@@ -109,7 +116,8 @@ func TestBlockedRequestPhase1_GeoIPBlocking(t *testing.T) {
 			GeoIPDBPath: geoIPdata, // Path to a test GeoIP database
 			geoIP:       geoIPBlock,
 		},
-		CustomResponses: customResponse,
+		CustomResponses:       customResponse,
+		requestValueExtractor: NewRequestValueExtractor(logger, false),
 	}
 
 	req := httptest.NewRequest("GET", testURL, nil)
@@ -205,9 +213,10 @@ func TestBlockedRequestPhase1_IPBlocking(t *testing.T) {
 
 	t.Run("Allow unblocked CIDR", func(t *testing.T) {
 		middleware := &Middleware{
-			logger:          logger,
-			ipBlacklist:     blackList,
-			CustomResponses: customResponse,
+			logger:                logger,
+			ipBlacklist:           blackList,
+			CustomResponses:       customResponse,
+			requestValueExtractor: NewRequestValueExtractor(logger, false),
 		}
 
 		req := httptest.NewRequest("GET", testURL, nil)
@@ -222,9 +231,10 @@ func TestBlockedRequestPhase1_IPBlocking(t *testing.T) {
 
 	t.Run("Blocks blacklisted CIDR", func(t *testing.T) {
 		middleware := &Middleware{
-			logger:          logger,
-			ipBlacklist:     blackList,
-			CustomResponses: customResponse,
+			logger:                logger,
+			ipBlacklist:           blackList,
+			CustomResponses:       customResponse,
+			requestValueExtractor: NewRequestValueExtractor(logger, false),
 		}
 
 		req := httptest.NewRequest("GET", testURL, nil)

request.go

@@ -166,7 +166,7 @@ func (rve *RequestValueExtractor) extractSingleValue(target string, r *http.Requ
 	}
 
 	// Redact sensitive fields before returning the value (as before)
-	value := rve.redactValueIfSensitive(target, unredactedValue)
+	value := rve.RedactValueIfSensitive(target, unredactedValue)
 
 	// Log the extracted value (redacted if necessary)
 	rve.logger.Debug("Extracted value",
@@ -351,7 +351,7 @@ func (rve *RequestValueExtractor) extractValueForJSONPath(r *http.Request, jsonP
 }
 
 // Helper function to redact value if target is sensitive
-func (rve *RequestValueExtractor) redactValueIfSensitive(target string, value string) string {
+func (rve *RequestValueExtractor) RedactValueIfSensitive(target string, value string) string {
 	if rve.redactSensitiveData {
 		for _, sensitive := range sensitiveTargets {
 			if strings.Contains(strings.ToLower(target), sensitive) {

request_test.go

@@ -145,7 +145,7 @@ func TestRedactValueIfSensitive(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			rve := NewRequestValueExtractor(logger, tt.redactSensitive)
-			result := rve.redactValueIfSensitive(tt.target, tt.value)
+			result := rve.RedactValueIfSensitive(tt.target, tt.value)
 
 			if tt.expectedRedacted && result != "REDACTED" {
 				t.Errorf("Expected REDACTED but got %q", result)
@@ -364,10 +364,11 @@ func newMockLogger() *MockLogger {
 func TestProcessRuleMatch_HighScore(t *testing.T) {
 	logger := newMockLogger()
 	middleware := &Middleware{
-		logger:           logger.Logger,
-		AnomalyThreshold: 100, // High threshold
-		ruleHits:         sync.Map{},
-		muMetrics:        sync.RWMutex{},
+		logger:                logger.Logger,
+		AnomalyThreshold:      100, // High threshold
+		ruleHits:              sync.Map{},
+		muMetrics:             sync.RWMutex{},
+		requestValueExtractor: NewRequestValueExtractor(logger.Logger, false), // Initialize
 	}
 
 	rule := &Rule{
@@ -394,7 +395,7 @@ func TestProcessRuleMatch_HighScore(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	// Test blocking rule with high score
-	shouldContinue := middleware.processRuleMatch(w, req, rule, "value", state)
+	shouldContinue := middleware.processRuleMatch(w, req, rule, "header", "value", state)
 	assert.False(t, shouldContinue)
 	assert.Equal(t, http.StatusForbidden, w.Code)
 	assert.True(t, state.Blocked)

rules.go

@@ -1,26 +1,27 @@
 // rules.go
 package caddywaf
 
+
 import (
 	"encoding/json"
 	"fmt"
 	"net/http"
 	"os"
 	"regexp"
 	"sort"
-	"strings"
-
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 )
 
-func (m *Middleware) processRuleMatch(w http.ResponseWriter, r *http.Request, rule *Rule, value string, state *WAFState) bool {
+func (m *Middleware) processRuleMatch(w http.ResponseWriter, r *http.Request, rule *Rule, target, value string, state *WAFState) bool {
 	logID := r.Context().Value(ContextKeyLogId("logID")).(string)
 
+	redactedValue := m.requestValueExtractor.RedactValueIfSensitive(target, value)
+
 	m.logRequest(zapcore.DebugLevel, "Rule Matched", r, // More concise log message
 		zap.String("rule_id", rule.ID),
-		zap.String("target", strings.Join(rule.Targets, ",")),
-		zap.String("value", value),
+		zap.String("target", target), // Log the specific target that matched
+		zap.String("value", redactedValue),
 		zap.String("description", rule.Description),
 		zap.Int("score", rule.Score),
 		zap.Int("anomaly_threshold_config", m.AnomalyThreshold), // ADDED: Log configured anomaly threshold

rules_test.go

@@ -146,10 +146,11 @@ func TestProcessRuleMatch(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			m := &Middleware{
-				logger:           logger,
-				AnomalyThreshold: tt.anomalyThreshold,
-				ruleHits:         sync.Map{},
-				muMetrics:        sync.RWMutex{},
+				logger:                logger,
+				AnomalyThreshold:      tt.anomalyThreshold,
+				ruleHits:              sync.Map{},
+				muMetrics:             sync.RWMutex{},
+				requestValueExtractor: NewRequestValueExtractor(logger, false),
 			}
 
 			w := httptest.NewRecorder()
@@ -162,7 +163,7 @@ func TestProcessRuleMatch(t *testing.T) {
 				ResponseWritten: tt.responseWritten,
 			}
 
-			result := m.processRuleMatch(w, r, &tt.rule, "test-value", state)
+			result := m.processRuleMatch(w, r, &tt.rule, "ARGS", "test-value", state)
 			if result == tt.wantBlock {
 				t.Errorf("processRuleMatch() returned %v, want %v", result, !tt.wantBlock)
 			}