diff --git a/internal/api/router.go b/internal/api/router.go index ce4dbb029..5d01a7ad9 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -3818,7 +3818,8 @@ func (r *Router) capturePublicURLFromRequest(req *http.Request) { } current := strings.TrimRight(strings.TrimSpace(r.config.PublicURL), "/") - if current != "" && current == normalizedCandidate { + if current != "" { + // If explicitly configured, never overwrite from request r.publicURLDetected = true r.publicURLMu.Unlock() return diff --git a/internal/api/updates.go b/internal/api/updates.go index a8ad3161f..683ff1fe5 100644 --- a/internal/api/updates.go +++ b/internal/api/updates.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "net" "net/http" "os" "strings" @@ -140,7 +139,7 @@ func (h *UpdateHandlers) HandleUpdateStatus(w http.ResponseWriter, r *http.Reque } // Extract client IP for rate limiting - clientIP := getClientIP(r) + clientIP := GetClientIP(r) // Check rate limit (5 seconds minimum between requests per client) h.statusMu.Lock() @@ -199,7 +198,7 @@ func (h *UpdateHandlers) HandleUpdateStream(w http.ResponseWriter, r *http.Reque w.Header().Set("Access-Control-Allow-Origin", "*") // Generate client ID - clientIP := getClientIP(r) + clientIP := GetClientIP(r) clientID := fmt.Sprintf("%s-%d", clientIP, time.Now().UnixNano()) // Register client with SSE broadcaster @@ -256,34 +255,6 @@ func (h *UpdateHandlers) doCleanupRateLimits(now time.Time) { } } -// getClientIP extracts the client IP from the request -func getClientIP(r *http.Request) string { - // Check X-Forwarded-For header first - xff := r.Header.Get("X-Forwarded-For") - if xff != "" { - // Take the first IP if multiple are present - if ip := net.ParseIP(xff); ip != nil { - return xff - } - } - - // Check X-Real-IP header - xri := r.Header.Get("X-Real-IP") - if xri != "" { - if ip := net.ParseIP(xri); ip != nil { - return xri - } - } - - // Fall back to RemoteAddr - host, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - return r.RemoteAddr - } - - return host -} - // HandleGetUpdatePlan returns update plan for current deployment func (h *UpdateHandlers) HandleGetUpdatePlan(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/internal/api/updates_test.go b/internal/api/updates_test.go index 8de6c76fa..506baa980 100644 --- a/internal/api/updates_test.go +++ b/internal/api/updates_test.go @@ -295,36 +295,6 @@ func TestHandleGetUpdateHistoryEntry(t *testing.T) { } } -func TestGetClientIP(t *testing.T) { - // Re-include the IP tests as they were useful - tests := []struct { - name string - remoteAddr string - headers map[string]string - expected string - }{ - {"RemoteAddr", "1.2.3.4:1234", nil, "1.2.3.4"}, - {"XFF", "1.1.1.1:1234", map[string]string{"X-Forwarded-For": "2.2.2.2"}, "2.2.2.2"}, - {"X-Real-IP", "1.1.1.1:1234", map[string]string{"X-Real-IP": "3.3.3.3"}, "3.3.3.3"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := httptest.NewRequest("GET", "/", nil) - r.RemoteAddr = tt.remoteAddr - for k, v := range tt.headers { - r.Header.Set(k, v) - } - - // getClientIP is strict internal but exposed via tests in same package - ip := getClientIP(r) - if ip != tt.expected { - t.Errorf("Expected %s, got %s", tt.expected, ip) - } - }) - } -} - func TestDoCleanupRateLimits(t *testing.T) { h := NewUpdateHandlers(nil, nil) now := time.Now() diff --git a/internal/monitoring/metrics_history.go b/internal/monitoring/metrics_history.go index 3b8095a1d..dedb1ee56 100644 --- a/internal/monitoring/metrics_history.go +++ b/internal/monitoring/metrics_history.go @@ -135,15 +135,16 @@ func (mh *MetricsHistory) appendMetric(metrics []MetricPoint, point MetricPoint) // Remove old points beyond retention time cutoffTime := time.Now().Add(-mh.retentionTime) - startIdx := 0 + found := false for i, p := range metrics { if p.Timestamp.After(cutoffTime) { - startIdx = i + metrics = metrics[i:] + found = true break } } - if startIdx > 0 { - metrics = metrics[startIdx:] + if !found { + metrics = metrics[:0] } // Ensure we don't exceed max data points @@ -367,15 +368,10 @@ func (mh *MetricsHistory) Cleanup() { // cleanupMetrics removes points older than cutoff time func (mh *MetricsHistory) cleanupMetrics(metrics []MetricPoint, cutoffTime time.Time) []MetricPoint { - startIdx := 0 for i, p := range metrics { if p.Timestamp.After(cutoffTime) { - startIdx = i - break + return metrics[i:] } } - if startIdx > 0 { - return metrics[startIdx:] - } - return metrics + return metrics[:0] } diff --git a/internal/monitoring/metrics_history_test.go b/internal/monitoring/metrics_history_test.go index 019c636fc..c83e8507c 100644 --- a/internal/monitoring/metrics_history_test.go +++ b/internal/monitoring/metrics_history_test.go @@ -203,7 +203,7 @@ func TestCleanupMetrics(t *testing.T) { {Value: 20.0, Timestamp: now.Add(-2 * time.Hour)}, }, cutoffTime: now.Add(-time.Hour), - wantLen: 2, // cleanupMetrics doesn't remove all - keeps slice if nothing after cutoff + wantLen: 0, }, { name: "boundary - point exactly at cutoff", diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index c3bfcc50c..ebcb1133e 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -87,6 +87,20 @@ func (n *NotificationManager) SendEnhancedWebhook(webhook EnhancedWebhookConfig, } webhook.URL = renderedURL + // Validate webhook URL to prevent SSRF/DNS rebinding attacks + if err := n.ValidateWebhookURL(webhook.URL); err != nil { + return fmt.Errorf("webhook URL validation failed: %w", err) + } + + // Check rate limit + if !n.checkWebhookRateLimit(webhook.URL) { + log.Warn(). + Str("webhook", webhook.Name). + Str("url", webhook.URL). + Msg("Webhook request dropped due to rate limiting") + return fmt.Errorf("rate limit exceeded for webhook %s", webhook.Name) + } + // Service-specific enrichment switch webhook.Service { case "telegram":