🐛 fail an AppSec challenge without a body closed, and replace its headers (#398)

* 🐛 fail an AppSec challenge without a body closed, and replace its headers

Fixes #397.

A challenge envelope without user_body_content was relayed as its
http_status with an empty body, so the visitor got a blank 200 instead of
being blocked. The challenge protocol requires failing that case closed.
appsecQuery now rejects it, and it ends in the ban response like any
other malformed AppSec answer.

user_headers were added next to any header already on the response, so a
policy set earlier left two Content-Security-Policy values and the
browser enforced both. They now replace headers of the same name, as the
protocol asks, while each user_cookies entry stays its own Set-Cookie.

Traefik's headers middleware sets its CSP as the response is written, so
when it is listed before the bouncer it still overrides the challenge
policy whatever the plugin does. Checked on Traefik 3.7.9. The README now
says to list the bouncer first.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* 📝 explain the challenge Content-Security-Policy and the middleware order

The previous paragraph read as if the plugin's header replacement
should win against Traefik's headers middleware, and it did not say
what breaks or how to keep security headers on the challenge page.

It now states what the challenge needs, the symptom when it is
blocked, and why a headers middleware listed before the bouncer
overrides the policy: it applies its headers when the response is
written, after the plugin. The recommended setup keeps the CSP in a
headers middleware after the bouncer and other security headers
before it. Checked on Traefik 3.7.13, where the challenge keeps its
policy and still gets X-Frame-Options. Links CrowdSec's own CSP
guidance and the protocol requirement. The "See examples" line goes
back under the /crowdsec-internal section, where it was on main.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* 🍱 reduce readme modif

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: maxlerebourg <maxlerebourg@gmail.com>

README.md

@@ -781,6 +781,35 @@ CrowdSec documents the same requirement, that the bouncer must forward
 
 See [examples/bot-detection/README.md](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/blob/main/examples/bot-detection/README.md).
 
+#### AppSec bot detection: Content-Security-Policy
+
+The challenge runs an inline script and a blob: worker, so AppSec sends every challenge with a fixed Content-Security-Policy allowing both:
+
+```
+default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; worker-src 'self' blob:;
+```
+
+If your own policy forbids inline scripts or blob: workers, the challenge can never be solved: blank page, and Refused to
+execute inline script / Refused to create a worker in the console.
+
+The plugin replaces any policy already set on the response, as the challenge protocol
+(https://docs.crowdsec.net/docs/appsec/bot_detection/challenge_protocol#requirements-for-a-remediation-component)
+requires. Traefik's headers middleware does not: it applies its headers when the response is written, after the plugin.
+Listed before this one, it therefore overwrites the challenge policy with yours.
+
+Put your Content-Security-Policy in a headers middleware listed after this one. Your other security headers can stay
+before it.
+The captcha and ban pages get none of your default CSP so rewrite it on your template.
+
+```yaml
+  - "traefik.http.routers.my-router.middlewares=security-headers@docker,crowdsec@docker,csp@docker"
+  - "traefik.http.middlewares.security-headers.headers.frameDeny=true"
+  - "traefik.http.middlewares.csp.headers.contentSecurityPolicy=default-src 'self'"
+```
+
+/!\ CrowdSec describes the same conflict, and how to solve it in nginx, in
+[enabling bot detection: Content-Security-Policy](https://docs.crowdsec.net/docs/appsec/bot_detection/enable#content-security-policy).
+
 #### Manually add an IP to the blocklist (for testing purposes)
 
 ```bash

bouncer.go

@@ -524,6 +524,7 @@ func (bouncer *Bouncer) handleAppsecResponseServeHTTP(rw http.ResponseWriter, re
 	atomic.AddInt64(&blockedRequests, 1)
 
 	for name, values := range decision.UserHeaders {
+		rw.Header().Del(name)
 		for _, value := range values {
 			rw.Header().Add(name, value)
 		}
@@ -536,7 +537,7 @@ func (bouncer *Bouncer) handleAppsecResponseServeHTTP(rw http.ResponseWriter, re
 	}
 	rw.WriteHeader(decision.HTTPStatus)
 
-	if req.Method == http.MethodHead || decision.UserBodyContent == "" {
+	if req.Method == http.MethodHead {
 		return
 	}
 	if _, err := rw.Write([]byte(decision.UserBodyContent)); err != nil {
@@ -890,6 +891,9 @@ func appsecQuery(bouncer *Bouncer, ip string, httpReq *http.Request) (*AppSecRes
 		if decision.Action == "" || decision.HTTPStatus == 0 {
 			return nil, errors.New("appsecQuery:responseAppsecKeysMissing")
 		}
+		if decision.Action == appsecChallengeAction && decision.UserBodyContent == "" {
+			return nil, errors.New("appsecQuery:challengeBodyMissing")
+		}
 		return &decision, nil
 	default:
 		return nil, fmt.Errorf("appsecQuery: statusCode:%d", res.StatusCode)

bouncer_test.go

@@ -688,6 +688,90 @@ func TestHandleNextServeHTTPRelaysStructuredAppsecChallenge(t *testing.T) {
 	}
 }
 
+func serveAppsecEnvelope(t *testing.T, envelope string, recorder *httptest.ResponseRecorder) bool {
+	t.Helper()
+	appsecServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		_, _ = w.Write([]byte(envelope))
+	}))
+	defer appsecServer.Close()
+
+	appsecURL, err := url.Parse(appsecServer.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	nextCalled := false
+	bouncer := &Bouncer{
+		next: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+			nextCalled = true
+		}),
+		appsecEnabled:           true,
+		appsecScheme:            appsecURL.Scheme,
+		appsecHost:              appsecURL.Host,
+		appsecPath:              "/",
+		httpAppsecClient:        appsecServer.Client(),
+		remediationStatusCode:   http.StatusForbidden,
+		remediationCustomHeader: "X-Remediation",
+		log:                     logger.New("DEBUG", ""),
+	}
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/protected", nil)
+	bouncer.handleNextServeHTTP(recorder, req, "192.0.2.10")
+	return nextCalled
+}
+
+func TestHandleNextServeHTTPAppsecChallengeWithoutBodyFallsBackToBan(t *testing.T) {
+	tests := []struct {
+		name     string
+		envelope string
+	}{
+		{name: "missing body", envelope: `{"action":"challenge","http_status":200}`},
+		{name: "empty body", envelope: `{"action":"challenge","http_status":200,"user_body_content":""}`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			recorder := httptest.NewRecorder()
+			if serveAppsecEnvelope(t, tt.envelope, recorder) {
+				t.Fatal("next handler should not be called for an appsec challenge without body")
+			}
+			if recorder.Code != http.StatusForbidden {
+				t.Fatalf("expected fallback ban status 403, got %d", recorder.Code)
+			}
+			if got := recorder.Header().Get("X-Remediation"); got != "ban" {
+				t.Fatalf("expected fallback remediation header ban, got %q", got)
+			}
+		})
+	}
+}
+
+func TestHandleNextServeHTTPAppsecChallengeReplacesHeaders(t *testing.T) {
+	recorder := httptest.NewRecorder()
+	recorder.Header().Set("Content-Security-Policy", "default-src 'none'")
+	recorder.Header().Set("Cache-Control", "public, max-age=3600")
+	envelope := `{
+		"action":"challenge",
+		"http_status":200,
+		"user_body_content":"<html>challenge</html>",
+		"user_cookies":["first=1; Path=/","second=2; Path=/"],
+		"user_headers":{
+			"Content-Security-Policy":["script-src 'self' 'unsafe-inline'"],
+			"Cache-Control":["no-store"]
+		}
+	}`
+	if serveAppsecEnvelope(t, envelope, recorder) {
+		t.Fatal("next handler should not be called for appsec challenge")
+	}
+	if got := recorder.Header().Values("Content-Security-Policy"); len(got) != 1 || got[0] != "script-src 'self' 'unsafe-inline'" {
+		t.Fatalf("expected only the challenge Content-Security-Policy, got %q", got)
+	}
+	if got := recorder.Header().Values("Cache-Control"); len(got) != 1 || got[0] != "no-store" {
+		t.Fatalf("expected only the challenge Cache-Control, got %q", got)
+	}
+	if got := recorder.Header().Values("Set-Cookie"); len(got) != 2 {
+		t.Fatalf("expected two separate Set-Cookie headers, got %q", got)
+	}
+}
+
 func TestHandleNextServeHTTPLegacyAppsecForbiddenFallsBackToBan(t *testing.T) {
 	appsecServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusForbidden)