From 18264d1669133be62c1910267f4195183189c80c Mon Sep 17 00:00:00 2001 From: Renn F Date: Thu, 15 Oct 2026 13:43:37 +0200 Subject: [PATCH] fix(middleware): apply the custom response modifier and refresh stale docs The adapter now holds the handler response when CustomResponseModifier is configured and runs Engine.ModifyResponse last, then re-applies CORS, matching the reference process_response. The examples and docs no longer tell users to mirror the default address-header exclusion or to avoid EnableAgent: the current engine covers the headers by default and EnableAgent validates. Closes #40 --- docs/configuration.md | 15 +- examples/advanced_app/README.md | 3 +- .../advanced_app/internal/config/config.go | 36 ++-- examples/simple_app/README.md | 4 +- examples/simple_app/main.go | 34 ++- middleware.go | 72 ++++++- parity_test.go | 194 ++++++++++++++++++ 7 files changed, 300 insertions(+), 58 deletions(-) create mode 100644 parity_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 5abc233..bdc9c06 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -25,18 +25,11 @@ cfg.CustomErrorResponses = map[int]string{ ## Address headers -The Python engine skips ssrf scanning for address headers (`host`, -`x-forwarded-for`, `x-real-ip`, ...) automatically. The Go engine does not -apply that built-in exclusion yet, so mirror it explicitly when clients can -send internal hostnames: +The engine skips ssrf scanning for address headers (`host`, `x-forwarded-for`, `x-real-ip`, ...) through its built-in default set (`guardcore.DefaultExcludedHeaders`, the reference's `_DEFAULT_EXCLUDED_HEADERS`), so no manual exclusion is needed. `ExcludedDetectionHeaders` only adds names on top of that default. -```go -cfg.ExcludedDetectionHeaders = map[string]bool{ - "host": true, "origin": true, "via": true, - "x-forwarded-for": true, "x-forwarded-host": true, - "x-real-ip": true, "x-client-ip": true, -} -``` +## Custom response modifier + +`cfg.CustomResponseModifier` runs on every pass-through response after the security and CORS headers, as the last step of the reference response pass, and CORS headers are applied again after it. Blocked responses already carry it from the engine. With a modifier configured the adapter holds the whole handler response in memory until the handler returns, so a streamed response is delivered at the end instead of incrementally; without one the response is untouched and streams as usual. ## Redis diff --git a/examples/advanced_app/README.md b/examples/advanced_app/README.md index eda39e4..67408cc 100644 --- a/examples/advanced_app/README.md +++ b/examples/advanced_app/README.md @@ -85,8 +85,7 @@ curl -s -X POST http://localhost/admin/unban -H 'X-Admin-Token: admin-token-chan `/admin/*`), attached with `nethttp.WithRouteID` - `OnBlock` hook: the telemetry seam for [guard-agent-go](https://github.com/Guard-Core/guard-agent-go) wiring - (comment-level guidance in `internal/config/config.go`; `EnableAgent` is - fail-closed in this port, so the hook is the integration point) + (comment-level guidance in `internal/config/config.go`; `EnableAgent` validates but streams only to a `guardcore.AgentHandler` set in `AgentHandler`, so the example keeps it off and only logs blocks) ## Intentional simplifications diff --git a/examples/advanced_app/internal/config/config.go b/examples/advanced_app/internal/config/config.go index 33e7989..0c7e0b8 100644 --- a/examples/advanced_app/internal/config/config.go +++ b/examples/advanced_app/internal/config/config.go @@ -45,18 +45,12 @@ func New() (*guardcore.SecurityConfig, error) { // Detection: every category, default detector tuning. c.EnablePenetrationDetection = true - // The Python engine automatically skips ssrf scanning for address - // headers (host, x-forwarded-for, x-real-ip, ...). The Go engine - // does not apply that built-in exclusion yet, so mirror it here; - // otherwise nginx's forwarded headers get flagged as ssrf. - c.ExcludedDetectionHeaders = map[string]bool{ - "host": true, "origin": true, "via": true, - "x-forwarded-for": true, "x-forwarded-host": true, - "x-real-ip": true, "x-client-ip": true, - "x-cluster-client-ip": true, "cf-connecting-ip": true, - "true-client-ip": true, "fly-client-ip": true, - "x-envoy-external-address": true, - } + // Address headers (host, x-forwarded-for, x-real-ip, ...) are already + // excluded from detection by the engine's built-in default set + // (guardcore.DefaultExcludedHeaders, the reference's + // _DEFAULT_EXCLUDED_HEADERS), so a plain "Host: localhost" request is + // not flagged and no ExcludedDetectionHeaders entry is needed here. + // ExcludedDetectionHeaders only adds names on top of that default. // Edge filters. c.BlockedUserAgents = []string{"badbot", "evil-crawler", "sqlmap"} @@ -78,14 +72,16 @@ func New() (*guardcore.SecurityConfig, error) { c.LogRequestLevel = envOr("LOG_REQUEST_LEVEL", "INFO") c.LogSuspiciousLevel = envOr("LOG_SUSPICIOUS_LEVEL", "WARNING") - // Agent wiring (comment-level): guard-core-go's only telemetry seam - // is OnBlock. The Go agent (guard-agent-go, - // https://github.com/Guard-Core/guard-agent-go) mirrors the Python - // guard-agent's API: once its engine-event pipeline accepts these - // payloads, replace the log line below with the agent client call - // and set AGENT_ENDPOINT/AGENT_PROJECT_ID here. EnableAgent stays - // false because the engine fails config validation on it (the - // feature is not ported yet); do not turn it on. + // OnBlock is the engine's block-event hook (check_name, reason, + // trigger_info, passive_mode, client_ip, path, method, status_code). + // Agent telemetry rides a different seam: EnableAgent validates, and + // the engine streams events and metrics to the guardcore.AgentHandler + // set in AgentHandler (SendEvent/SendMetric). Without a handler the + // engine logs that the agent cannot initialize and continues without + // it (AgentStrict makes NewEngine fail instead), so this example keeps + // EnableAgent off and only logs blocks. guard-agent-go takes a + // context and its own event types, so it needs a small bridge to + // satisfy guardcore.AgentHandler. c.OnBlock = func(req guardcore.Request, payload map[string]any) { log.Printf("guard blocked %s %s from %s via %s: %s", payload["method"], payload["path"], payload["client_ip"], diff --git a/examples/simple_app/README.md b/examples/simple_app/README.md index fa52e04..fb9d0d8 100644 --- a/examples/simple_app/README.md +++ b/examples/simple_app/README.md @@ -65,9 +65,7 @@ Inline comments in `main.go` walk through every knob used: - Redis via `REDIS_URL` / `REDIS_PREFIX` (compose wires Redis in; without it the managers fall back to in-process state) - The `OnBlock` hook: the telemetry seam for wiring - [guard-agent-go](https://github.com/Guard-Core/guard-agent-go) (comment-level - guidance in `main.go`; agent integration is not implemented in the engine - port yet, and `EnableAgent` fails config validation) + [guard-agent-go](https://github.com/Guard-Core/guard-agent-go) (comment-level guidance in `main.go`; `EnableAgent` validates but streams only to a `guardcore.AgentHandler` set in `AgentHandler`, so the example keeps it off) ## Environment variables diff --git a/examples/simple_app/main.go b/examples/simple_app/main.go index 81369ad..4a5d865 100644 --- a/examples/simple_app/main.go +++ b/examples/simple_app/main.go @@ -37,18 +37,12 @@ func main() { // Penetration detection: all categories, default thresholds. c.EnablePenetrationDetection = true - // The Python engine automatically skips ssrf scanning for address - // headers (host, x-forwarded-for, x-real-ip, ...). The Go engine - // does not apply that built-in exclusion yet, so mirror it here; - // otherwise a plain "Host: localhost" request is flagged as ssrf. - c.ExcludedDetectionHeaders = map[string]bool{ - "host": true, "origin": true, "via": true, - "x-forwarded-for": true, "x-forwarded-host": true, - "x-real-ip": true, "x-client-ip": true, - "x-cluster-client-ip": true, "cf-connecting-ip": true, - "true-client-ip": true, "fly-client-ip": true, - "x-envoy-external-address": true, - } + // Address headers (host, x-forwarded-for, x-real-ip, ...) are already + // excluded from detection by the engine's built-in default set + // (guardcore.DefaultExcludedHeaders, the reference's + // _DEFAULT_EXCLUDED_HEADERS), so a plain "Host: localhost" request is + // not flagged and no ExcludedDetectionHeaders entry is needed here. + // ExcludedDetectionHeaders only adds names on top of that default. // Blocked user agents (regex patterns). c.BlockedUserAgents = []string{"badbot", "evil-crawler", "sqlmap"} @@ -63,12 +57,16 @@ func main() { "/docs", "/redoc", "/openapi.json", "/favicon.ico", "/static", "/health", } - // OnBlock is the engine's telemetry seam. Guard Agent integration - // is not implemented in guard-core-go yet (setting EnableAgent - // fails config validation), so wire the agent from here: forward - // these payloads to guard-agent-go - // (https://github.com/Guard-Core/guard-agent-go) once its event - // pipeline accepts engine events. + // OnBlock is the engine's block-event hook (check_name, reason, + // trigger_info, passive_mode, client_ip, path, method, status_code). + // Agent telemetry rides a different seam: EnableAgent validates, and + // the engine streams events and metrics to the guardcore.AgentHandler + // set in AgentHandler (SendEvent/SendMetric). Without a handler the + // engine logs that the agent cannot initialize and continues without + // it (AgentStrict makes NewEngine fail instead), so this example keeps + // EnableAgent off and only logs blocks. guard-agent-go takes a + // context and its own event types, so it needs a small bridge to + // satisfy guardcore.AgentHandler. c.OnBlock = func(req guardcore.Request, payload map[string]any) { log.Printf("guard blocked %s %s from %s via %s: %s", payload["method"], payload["path"], payload["client_ip"], diff --git a/middleware.go b/middleware.go index 1b6a052..9ad9c48 100644 --- a/middleware.go +++ b/middleware.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "net/http" + "strconv" "github.com/rennf93/guard-core-go/v4/guardcore" ) @@ -65,8 +66,9 @@ func (m *middleware) wrap(next http.Handler) http.Handler { // merged on top (the reference _inject_cors_headers runs after the // security-header set, so CORS wins on a shared name) whenever the // request carries an Origin and CORS is enabled. + cors := m.engine.CORSResponseHeaders(req) headers := m.engine.ResponseHeaders() - for name, value := range m.engine.CORSResponseHeaders(req) { + for name, value := range cors { headers[name] = value } for name, value := range headers { @@ -77,25 +79,36 @@ func (m *middleware) wrap(next http.Handler) http.Handler { // response factory's behavioral phase. The adapter captures the // leading response body up to the configured inspect budget and // hands status code plus captured prefix to the engine after the - // handler ran: return rules never modify the response. + // handler ran: return rules never modify the response. With a + // CustomResponseModifier configured the whole response is held + // instead, so the modifier (the last step of the reference + // process_response) can rewrite it before it is sent. capture := m.newBodyCapture() - observed := &statusObserver{ResponseWriter: w, capture: capture} + observed := &statusObserver{ResponseWriter: w, capture: capture, + hold: m.engine.Config.CustomResponseModifier != nil} next.ServeHTTP(observed, r) m.engine.ProcessResponse(req, &guardcore.Response{ StatusCode: observed.status(), Headers: map[string]string{}, Body: observed.body(), }) + if observed.hold { + observed.release(m.engine, cors) + } }) } // statusObserver wraps the net/http ResponseWriter to record the status // code (defaulting to the implicit 200) and the leading response body bytes. +// In hold mode it also buffers the whole response (status, body) until +// release, so the engine's CustomResponseModifier can rewrite it. type statusObserver struct { http.ResponseWriter capture *bodyCapture wroteHeader bool statusCode int + hold bool + held []byte } func (o *statusObserver) WriteHeader(code int) { @@ -103,7 +116,9 @@ func (o *statusObserver) WriteHeader(code int) { o.statusCode = code o.wroteHeader = true } - o.ResponseWriter.WriteHeader(code) + if !o.hold { + o.ResponseWriter.WriteHeader(code) + } } func (o *statusObserver) Write(p []byte) (int, error) { @@ -114,6 +129,10 @@ func (o *statusObserver) Write(p []byte) (int, error) { if o.capture != nil { o.capture.record(p) } + if o.hold { + o.held = append(o.held, p...) + return len(p), nil + } return o.ResponseWriter.Write(p) } @@ -131,6 +150,51 @@ func (o *statusObserver) body() []byte { return o.capture.buf } +// release runs the modifier over the held response and writes the result to +// the real writer. Headers the modifier did not touch keep every value +// (Set-Cookie and friends); changed or added ones are set, removed ones are +// deleted. CORS headers are applied again after the modifier, as the +// reference dispatch injects them on the processed response. +func (o *statusObserver) release(engine *guardcore.Engine, cors map[string]string) { + header := o.ResponseWriter.Header() + resp := &guardcore.Response{StatusCode: o.status(), Headers: headerSnapshot(header), Body: o.held} + before := make(map[string]string, len(resp.Headers)) + for name, value := range resp.Headers { + before[name] = value + } + out := engine.ModifyResponse(resp) + for name := range before { + if _, kept := out.Headers[name]; !kept { + header.Del(name) + } + } + for name, value := range out.Headers { + if prev, seen := before[name]; !seen || prev != value { + header.Set(name, value) + } + } + if header.Get("Content-Length") != "" { + header.Set("Content-Length", strconv.Itoa(len(out.Body))) + } + for name, value := range cors { + header.Set(name, value) + } + o.ResponseWriter.WriteHeader(out.StatusCode) + if len(out.Body) > 0 { + _, _ = o.ResponseWriter.Write(out.Body) + } +} + +func headerSnapshot(h http.Header) map[string]string { + out := make(map[string]string, len(h)) + for name, values := range h { + if len(values) > 0 { + out[name] = values[0] + } + } + return out +} + // bodyCapture records the leading bytes of the pass-through response body, // bounded by the engine's behavior_max_response_body_inspect_bytes budget, // and only when behavior_scan_response_body is enabled (with the flag off diff --git a/parity_test.go b/parity_test.go new file mode 100644 index 0000000..d48f8ba --- /dev/null +++ b/parity_test.go @@ -0,0 +1,194 @@ +package nethttp + +// Parity tests against the reference adapter (fastapi-guard): the response +// pass runs the custom response modifier last, and the request metrics and +// return rules see the real response status. + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/rennf93/guard-core-go/v4/guardcore" +) + +func serveWith(t *testing.T, engine *guardcore.Engine, handler http.HandlerFunc, r *http.Request) *httptest.ResponseRecorder { + t.Helper() + wrap, err := New(engine) + if err != nil { + t.Fatalf("middleware: %v", err) + } + rec := httptest.NewRecorder() + wrap(handler).ServeHTTP(rec, r) + return rec +} + +func TestParityModifierRewritesPassThroughResponse(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { + resp.SetHeader("X-Modified", "yes") + delete(resp.Headers, "X-Frame-Options") + resp.Headers["X-Content-Type-Options"] = "changed" + resp.Body = append([]byte("modified:"), resp.Body...) + return resp + } + }) + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Set-Cookie", "a=1") + w.Header().Add("Set-Cookie", "b=2") + w.Header().Set("Content-Length", "5") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte("hello")) + }, httptest.NewRequest("GET", "/api", nil)) + if rec.Code != http.StatusCreated { + t.Fatalf("modifier must keep the handler status, got %d", rec.Code) + } + if rec.Body.String() != "modified:hello" { + t.Fatalf("modifier must rewrite the pass-through body, got %q", rec.Body.String()) + } + if rec.Header().Get("X-Modified") != "yes" { + t.Fatalf("modifier header missing: %v", rec.Header()) + } + if rec.Header().Get("X-Frame-Options") != "" { + t.Fatalf("modifier must be able to drop a security header: %v", rec.Header()) + } + if rec.Header().Get("X-Content-Type-Options") != "changed" { + t.Fatalf("modifier must be able to change a header: %v", rec.Header()) + } + if got := rec.Header().Values("Set-Cookie"); len(got) != 2 { + t.Fatalf("untouched multi-value headers must survive, got %v", got) + } + if rec.Header().Get("Content-Length") != "14" { + t.Fatalf("content length must follow the modified body, got %q", rec.Header().Get("Content-Length")) + } +} + +func TestParityModifierRunsAfterCORSAndCORSIsReapplied(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.EnableCORS = true + c.CORSAllowOrigins = []string{"https://app.example"} + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { + sawCORS := resp.Headers["Access-Control-Allow-Origin"] == "https://app.example" + delete(resp.Headers, "Access-Control-Allow-Origin") + if sawCORS { + resp.SetHeader("X-Saw-CORS", "yes") + } + return resp + } + }) + r := httptest.NewRequest("GET", "/api", nil) + r.Header.Set("Origin", "https://app.example") + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("ok")) }, r) + if rec.Header().Get("X-Saw-CORS") != "yes" { + t.Fatalf("the modifier must see the CORS headers: %v", rec.Header()) + } + if rec.Header().Get("Access-Control-Allow-Origin") != "https://app.example" { + t.Fatalf("CORS is injected after the modifier in the reference dispatch: %v", rec.Header()) + } +} + +func TestParityModifierAppliesToGuardBlockedResponse(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { + resp.SetHeader("X-Modified", "yes") + return resp + } + }) + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("ok")) }, + httptest.NewRequest("GET", "/api?"+xssVector, nil)) + if rec.Code != 400 || rec.Header().Get("X-Modified") != "yes" { + t.Fatalf("guard responses carry the modifier, got %d %v", rec.Code, rec.Header()) + } +} + +func TestParityModifierStatusAndBodyFeedReturnRules(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.BehaviorScanResponseBody = true + c.BehaviorMaxResponseBodyInspectBytes = 1024 + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { return resp } + c.GlobalBehaviorRules = []guardcore.BehaviorRuleConfig{ + {RuleType: "return_pattern", Pattern: "secret", Threshold: 1, Window: 60, Action: "ban"}, + } + }) + for i := 0; i < 2; i++ { + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("a secret leaked here")) + }, httptest.NewRequest("GET", "/report", nil)) + if rec.Code != 200 || rec.Body.String() != "a secret leaked here" { + t.Fatalf("held response must reach the client intact, got %d %q", rec.Code, rec.Body.String()) + } + } + if !engine.Ban.IsIPBanned("192.0.2.1") { + t.Fatal("the body prefix of a held response must reach the return rules") + } +} + +func TestParityHeldBodyBeyondBudgetIsNotScanned(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.BehaviorScanResponseBody = true + c.BehaviorMaxResponseBodyInspectBytes = 1024 + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { return resp } + c.GlobalBehaviorRules = []guardcore.BehaviorRuleConfig{ + {RuleType: "return_pattern", Pattern: "beyond-the-budget", Threshold: 1, Window: 60, Action: "ban"}, + } + }) + big := []byte(strings.Repeat("a", 2048)) + copy(big[1500:], "beyond-the-budget") + for i := 0; i < 3; i++ { + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(big) }, + httptest.NewRequest("GET", "/report", nil)) + if rec.Code != 200 || rec.Body.Len() != 2048 { + t.Fatalf("the client must get the whole body, got %d with %d bytes", rec.Code, rec.Body.Len()) + } + } + if engine.Ban.IsIPBanned("192.0.2.1") { + t.Fatal("a marker beyond the inspect budget must never trip a return rule") + } +} + +func TestParityHeldImplicitStatusAndNoBody(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.CustomResponseModifier = func(resp *guardcore.Response) *guardcore.Response { + resp.SetHeader("X-Status-Seen", http.StatusText(resp.StatusCode)) + return resp + } + }) + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) {}, httptest.NewRequest("GET", "/empty", nil)) + if rec.Code != 200 || rec.Header().Get("X-Status-Seen") != "OK" { + t.Fatalf("an empty handler answers the implicit 200, got %d %v", rec.Code, rec.Header()) + } +} + +func TestParityStatusWithoutBodyWrite(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { + c.GlobalBehaviorRules = []guardcore.BehaviorRuleConfig{ + {RuleType: "return_pattern", Pattern: "status:500", Threshold: 1, Window: 60, Action: "ban"}, + } + }) + for i := 0; i < 2; i++ { + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) }, + httptest.NewRequest("GET", "/boom", nil)) + if rec.Code != 500 { + t.Fatalf("handler status must reach the client, got %d", rec.Code) + } + } + if !engine.Ban.IsIPBanned("192.0.2.1") { + t.Fatal("a 500 without a body must be reported to the engine as 500") + } +} + +// The engine's built-in default header exclusions (DefaultExcludedHeaders) +// keep address headers out of ssrf detection, so the examples and docs need +// no manual ExcludedDetectionHeaders workaround. +func TestParityAddressHeadersNeedNoManualExclusion(t *testing.T) { + engine := newTestEngine(t, func(c *guardcore.SecurityConfig) { c.EnablePenetrationDetection = true }) + r := httptest.NewRequest("GET", "/api", nil) + r.Host = "169.254.169.254" + r.Header.Set("X-Forwarded-For", "10.0.0.1") + r.Header.Set("Origin", "http://localhost") + rec := serveWith(t, engine, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("ok")) }, r) + if rec.Code != 200 { + t.Fatalf("address headers must not be flagged by default, got %d %q", rec.Code, rec.Body.String()) + } +}