akanealw/Charon
1
0
Fork 0
Code Issues Pull Requests Actions 16 Packages Projects Releases Wiki Activity

fix: implement outbound request URL validation and redirect guard in HTTPWrapper

Browse Source
This commit is contained in:
GitHub Actions
2026-02-24 06:45:14 +00:00
parent e8a513541f
commit fdbf1a66cd
2 changed files with 195 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
backend/internal/notifications/http_wrapper.go
View File

@@ -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

142
backend/internal/notifications/http_wrapper_test.go
View File

@@ -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)
}
}