diff --git a/backend/internal/notifications/http_wrapper.go b/backend/internal/notifications/http_wrapper.go index e37f4883..aa1da80b 100644 --- a/backend/internal/notifications/http_wrapper.go +++ b/backend/internal/notifications/http_wrapper.go @@ -84,6 +84,7 @@ func (w *HTTPWrapper) Send(ctx context.Context, request HTTPWrapperRequest) (*HT headers := sanitizeOutboundHeaders(request.Headers) client := w.httpClientFactory(w.allowHTTP, w.maxRedirects) + w.applyRedirectGuard(client) var lastErr error for attempt := 1; attempt <= w.retryPolicy.MaxAttempts; attempt++ { @@ -100,6 +101,10 @@ func (w *HTTPWrapper) Send(ctx context.Context, request HTTPWrapperRequest) (*HT httpReq.Header.Set("Content-Type", "application/json") } + if guardErr := w.guardOutboundRequestURL(httpReq); guardErr != nil { + return nil, guardErr + } + resp, doErr := client.Do(httpReq) if doErr != nil { lastErr = doErr @@ -142,14 +147,30 @@ func (w *HTTPWrapper) Send(ctx context.Context, request HTTPWrapperRequest) (*HT return nil, fmt.Errorf("provider request failed") } +func (w *HTTPWrapper) applyRedirectGuard(client *http.Client) { + if client == nil { + return + } + + originalCheckRedirect := client.CheckRedirect + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if originalCheckRedirect != nil { + if err := originalCheckRedirect(req, via); err != nil { + return err + } + } + + return w.guardOutboundRequestURL(req) + } +} + func (w *HTTPWrapper) validateURL(rawURL string) (string, error) { parsedURL, err := neturl.Parse(rawURL) if err != nil { return "", fmt.Errorf("invalid destination URL") } - query := parsedURL.Query() - if query.Has("token") || query.Has("auth") || query.Has("apikey") || query.Has("api_key") { + if hasDisallowedQueryAuthKey(parsedURL.Query()) { return "", fmt.Errorf("destination URL query authentication is not allowed") } @@ -166,6 +187,36 @@ func (w *HTTPWrapper) validateURL(rawURL string) (string, error) { return validatedURL, nil } +func hasDisallowedQueryAuthKey(query neturl.Values) bool { + for key := range query { + normalizedKey := strings.ToLower(strings.TrimSpace(key)) + switch normalizedKey { + case "token", "auth", "apikey", "api_key": + return true + } + } + + return false +} + +func (w *HTTPWrapper) guardOutboundRequestURL(httpReq *http.Request) error { + if httpReq == nil || httpReq.URL == nil { + return fmt.Errorf("destination URL validation failed") + } + + reqURL := httpReq.URL.String() + validatedURL, err := w.validateURL(reqURL) + if err != nil { + return err + } + + if validatedURL != reqURL { + return fmt.Errorf("destination URL validation failed") + } + + return nil +} + func shouldRetry(resp *http.Response, err error) bool { if err != nil { var netErr net.Error diff --git a/backend/internal/notifications/http_wrapper_test.go b/backend/internal/notifications/http_wrapper_test.go index 846d78e3..085f2b79 100644 --- a/backend/internal/notifications/http_wrapper_test.go +++ b/backend/internal/notifications/http_wrapper_test.go @@ -2,9 +2,12 @@ package notifications import ( "context" + "errors" + "fmt" "io" "net/http" "net/http/httptest" + neturl "net/url" "strings" "sync/atomic" "testing" @@ -38,6 +41,79 @@ func TestHTTPWrapperRejectsTokenizedQueryURL(t *testing.T) { } } +func TestHTTPWrapperRejectsQueryAuthCaseVariants(t *testing.T) { + testCases := []string{ + "http://example.com/hook?Token=secret", + "http://example.com/hook?AUTH=secret", + "http://example.com/hook?apiKey=secret", + } + + for _, testURL := range testCases { + t.Run(testURL, func(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: testURL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "query authentication is not allowed") { + t.Fatalf("expected query auth rejection for %q, got: %v", testURL, err) + } + }) + } +} + +func TestHTTPWrapperSendRejectsRedirectTargetWithDisallowedScheme(t *testing.T) { + var attempts int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&attempts, 1) + http.Redirect(w, r, "ftp://example.com/redirected", http.StatusFound) + })) + defer server.Close() + + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + wrapper.maxRedirects = 3 + wrapper.retryPolicy.MaxAttempts = 1 + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "outbound request failed") { + t.Fatalf("expected outbound failure due to redirect target validation, got: %v", err) + } + if got := atomic.LoadInt32(&attempts); got != 1 { + t.Fatalf("expected only initial request due to blocked redirect, got %d attempts", got) + } +} + +func TestHTTPWrapperSendRejectsRedirectTargetWithMixedCaseQueryAuth(t *testing.T) { + var attempts int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&attempts, 1) + http.Redirect(w, r, "https://example.com/redirected?Token=secret", http.StatusFound) + })) + defer server.Close() + + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + wrapper.maxRedirects = 3 + wrapper.retryPolicy.MaxAttempts = 1 + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "outbound request failed") { + t.Fatalf("expected outbound failure due to redirect query auth validation, got: %v", err) + } + if got := atomic.LoadInt32(&attempts); got != 1 { + t.Fatalf("expected only initial request due to blocked redirect, got %d attempts", got) + } +} + func TestHTTPWrapperRetriesOn429ThenSucceeds(t *testing.T) { var calls int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -132,3 +208,69 @@ func TestSanitizeOutboundHeadersAllowlist(t *testing.T) { t.Fatalf("cookie header must be stripped") } } + +func TestHTTPWrapperGuardOutboundRequestURLRejectsNilRequest(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + + err := wrapper.guardOutboundRequestURL(nil) + if err == nil || !strings.Contains(err.Error(), "destination URL validation failed") { + t.Fatalf("expected validation failure for nil request, got: %v", err) + } +} + +func TestHTTPWrapperGuardOutboundRequestURLRejectsQueryAuth(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + httpReq := &http.Request{URL: &neturl.URL{Scheme: "http", Host: "example.com", Path: "/hook", RawQuery: "token=secret"}} + err := wrapper.guardOutboundRequestURL(httpReq) + if err == nil || !strings.Contains(err.Error(), "query authentication is not allowed") { + t.Fatalf("expected query auth rejection, got: %v", err) + } +} + +func TestHTTPWrapperGuardOutboundRequestURLRejectsMixedCaseQueryAuth(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + httpReq := &http.Request{URL: &neturl.URL{Scheme: "http", Host: "example.com", Path: "/hook", RawQuery: "apiKey=secret"}} + err := wrapper.guardOutboundRequestURL(httpReq) + if err == nil || !strings.Contains(err.Error(), "query authentication is not allowed") { + t.Fatalf("expected query auth rejection, got: %v", err) + } +} + +func TestHTTPWrapperApplyRedirectGuardPreservesOriginalBehavior(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + baseErr := fmt.Errorf("base redirect policy") + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return baseErr + }} + + wrapper.applyRedirectGuard(client) + err := client.CheckRedirect(&http.Request{URL: &neturl.URL{Scheme: "https", Host: "example.com"}}, nil) + if !errors.Is(err, baseErr) { + t.Fatalf("expected original redirect policy error, got: %v", err) + } +} + +func TestHTTPWrapperGuardOutboundRequestURLRejectsUnsafeDestination(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = false + + httpReq := &http.Request{URL: &neturl.URL{Scheme: "http", Host: "example.com", Path: "/hook"}} + err := wrapper.guardOutboundRequestURL(httpReq) + if err == nil || !strings.Contains(err.Error(), "destination URL validation failed") { + t.Fatalf("expected destination validation failure, got: %v", err) + } +} + +func TestHTTPWrapperGuardOutboundRequestURLAllowsValidatedDestination(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + + httpReq := &http.Request{URL: &neturl.URL{Scheme: "https", Host: "example.com", Path: "/hook"}} + err := wrapper.guardOutboundRequestURL(httpReq) + if err != nil { + t.Fatalf("expected validated destination to pass guard, got: %v", err) + } +}