From 9905533ba70ec51997d30cb003cbcbf727c5fdc8 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 01:53:51 -0500 Subject: [PATCH 1/8] =?UTF-8?q?=F0=9F=A7=B9=20chore:=20centralize=20Sec-Fe?= =?UTF-8?q?tch-Site=20header=20constant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- constants.go | 1 + middleware/csrf/csrf.go | 39 ++++++++++++---- middleware/csrf/csrf_test.go | 88 ++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 9 deletions(-) diff --git a/constants.go b/constants.go index a93b2413180..5f93fe1cd4b 100644 --- a/constants.go +++ b/constants.go @@ -256,6 +256,7 @@ const ( HeaderTE = "TE" HeaderTrailer = "Trailer" HeaderTransferEncoding = "Transfer-Encoding" + HeaderSecFetchSite = "Sec-Fetch-Site" HeaderSecWebSocketAccept = "Sec-WebSocket-Accept" HeaderSecWebSocketExtensions = "Sec-WebSocket-Extensions" HeaderSecWebSocketKey = "Sec-WebSocket-Key" diff --git a/middleware/csrf/csrf.go b/middleware/csrf/csrf.go index 28a287bbcb2..3d1e5f05e2b 100644 --- a/middleware/csrf/csrf.go +++ b/middleware/csrf/csrf.go @@ -14,15 +14,16 @@ import ( ) var ( - ErrTokenNotFound = errors.New("csrf: token not found") - ErrTokenInvalid = errors.New("csrf: token invalid") - ErrRefererNotFound = errors.New("csrf: referer header missing") - ErrRefererInvalid = errors.New("csrf: referer header invalid") - ErrRefererNoMatch = errors.New("csrf: referer does not match host or trusted origins") - ErrOriginInvalid = errors.New("csrf: origin header invalid") - ErrOriginNoMatch = errors.New("csrf: origin does not match host or trusted origins") - errOriginNotFound = errors.New("origin not supplied or is null") // internal error, will not be returned to the user - dummyValue = []byte{'+'} // dummyValue is a placeholder value stored in token storage. The actual token validation relies on the key, not this value. + ErrTokenNotFound = errors.New("csrf: token not found") + ErrTokenInvalid = errors.New("csrf: token invalid") + ErrFetchSiteInvalid = errors.New("csrf: sec-fetch-site header invalid") + ErrRefererNotFound = errors.New("csrf: referer header missing") + ErrRefererInvalid = errors.New("csrf: referer header invalid") + ErrRefererNoMatch = errors.New("csrf: referer does not match host or trusted origins") + ErrOriginInvalid = errors.New("csrf: origin header invalid") + ErrOriginNoMatch = errors.New("csrf: origin does not match host or trusted origins") + errOriginNotFound = errors.New("origin not supplied or is null") // internal error, will not be returned to the user + dummyValue = []byte{'+'} // dummyValue is a placeholder value stored in token storage. The actual token validation relies on the key, not this value. ) @@ -127,6 +128,11 @@ func New(config ...Config) fiber.Handler { default: // Assume that anything not defined as 'safe' by RFC7231 needs protection + // Evaluate Sec-Fetch-Site to reject cross-site requests earlier when available. + if err := validateSecFetchSite(c); err != nil { + return cfg.ErrorHandler(c, err) + } + // Enforce an origin check for unsafe requests. err := originMatchesHost(c, trustedOrigins, trustedSubOrigins) @@ -313,6 +319,21 @@ func (handler *Handler) DeleteToken(c fiber.Ctx) error { return nil } +func validateSecFetchSite(c fiber.Ctx) error { + secFetchSite := utils.Trim(c.Get(fiber.HeaderSecFetchSite), ' ') + + if secFetchSite == "" { + return nil + } + + switch utils.ToLower(secFetchSite) { + case "same-origin", "none": + return nil + default: + return ErrFetchSiteInvalid + } +} + // originMatchesHost checks that the origin header matches the host header // returns an error if the origin header is not present or is invalid // returns nil if the origin header is valid diff --git a/middleware/csrf/csrf_test.go b/middleware/csrf/csrf_test.go index 646ffc695a3..0a3211da373 100644 --- a/middleware/csrf/csrf_test.go +++ b/middleware/csrf/csrf_test.go @@ -823,6 +823,94 @@ func Test_CSRF_Extractor_EmptyString(t *testing.T) { require.Equal(t, ErrTokenNotFound.Error(), string(ctx.Response.Body())) } +func Test_CSRF_SecFetchSite(t *testing.T) { + t.Parallel() + + errorHandler := func(c fiber.Ctx, err error) error { + return c.Status(fiber.StatusForbidden).SendString(err.Error()) + } + + app := fiber.New() + + app.Use(New(Config{ErrorHandler: errorHandler})) + + app.All("/", func(c fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + h := app.Handler() + ctx := &fasthttp.RequestCtx{} + ctx.Request.Header.SetMethod(fiber.MethodGet) + ctx.Request.URI().SetScheme("http") + ctx.Request.URI().SetHost("example.com") + ctx.Request.Header.SetHost("example.com") + h(ctx) + token := string(ctx.Response.Header.Peek(fiber.HeaderSetCookie)) + token = strings.Split(strings.Split(token, ";")[0], "=")[1] + + tests := []struct { + name string + method string + secFetchSite string + origin string + scheme string + expectedStatus int + }{ + {"same-origin allowed", fiber.MethodPost, "same-origin", "http://example.com", "http", http.StatusOK}, + {"none allowed", fiber.MethodPost, "none", "http://example.com", "http", http.StatusOK}, + {"cross-site blocked", fiber.MethodPost, "cross-site", "http://example.com", "http", http.StatusForbidden}, + {"same-site blocked", fiber.MethodPost, "same-site", "http://example.com", "http", http.StatusForbidden}, + + {"no header with no origin", fiber.MethodPost, "", "", "http", http.StatusOK}, + {"no header with matching origin", fiber.MethodPost, "", "http://example.com", "http", http.StatusOK}, + {"no header with mismatched origin", fiber.MethodPost, "", "https://attacker.example", "http", http.StatusForbidden}, + {"no header with null origin", fiber.MethodPost, "", "null", "https", http.StatusForbidden}, + + {"GET allowed", fiber.MethodGet, "cross-site", "", "http", http.StatusOK}, + {"HEAD allowed", fiber.MethodHead, "cross-site", "", "http", http.StatusOK}, + {"OPTIONS allowed", fiber.MethodOptions, "cross-site", "", "http", http.StatusOK}, + {"PUT blocked", fiber.MethodPut, "cross-site", "http://example.com", "http", http.StatusForbidden}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + c := &fasthttp.RequestCtx{} + scheme := tt.scheme + if scheme == "" { + scheme = "http" + } + c.Request.Header.SetMethod(tt.method) + c.Request.URI().SetScheme(scheme) + c.Request.URI().SetHost("example.com") + c.Request.Header.SetHost("example.com") + c.Request.Header.SetProtocol(scheme) + if scheme == "https" { + c.Request.Header.Set(fiber.HeaderXForwardedProto, "https") + } + if tt.origin != "" { + c.Request.Header.Set(fiber.HeaderOrigin, tt.origin) + } + if tt.secFetchSite != "" { + c.Request.Header.Set(fiber.HeaderSecFetchSite, tt.secFetchSite) + } + + safe := tt.method == fiber.MethodGet || tt.method == fiber.MethodHead || tt.method == fiber.MethodOptions || tt.method == fiber.MethodTrace + + if !safe { + c.Request.Header.Set(HeaderName, token) + c.Request.Header.SetCookie(ConfigDefault.CookieName, token) + } + + h(c) + require.Equal(t, tt.expectedStatus, c.Response.StatusCode()) + if tt.expectedStatus == http.StatusForbidden && tt.secFetchSite != "" && !safe { + require.Equal(t, ErrFetchSiteInvalid.Error(), string(c.Response.Body())) + } + }) + } +} + func Test_CSRF_Origin(t *testing.T) { t.Parallel() app := fiber.New() From d539f5d069f1f500d2034f91a883a36c20f70790 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 07:50:21 -0500 Subject: [PATCH 2/8] =?UTF-8?q?=F0=9F=90=9B=20bug:=20relax=20Sec-Fetch-Sit?= =?UTF-8?q?e=20validation=20flow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- middleware/csrf/csrf.go | 2 +- middleware/csrf/csrf_test.go | 138 ++++++++++++++++++++++++++++------- 2 files changed, 114 insertions(+), 26 deletions(-) diff --git a/middleware/csrf/csrf.go b/middleware/csrf/csrf.go index 3d1e5f05e2b..98a4da7b8ae 100644 --- a/middleware/csrf/csrf.go +++ b/middleware/csrf/csrf.go @@ -327,7 +327,7 @@ func validateSecFetchSite(c fiber.Ctx) error { } switch utils.ToLower(secFetchSite) { - case "same-origin", "none": + case "same-origin", "none", "cross-site", "same-site": return nil default: return ErrFetchSiteInvalid diff --git a/middleware/csrf/csrf_test.go b/middleware/csrf/csrf_test.go index 0a3211da373..812dc8959e3 100644 --- a/middleware/csrf/csrf_test.go +++ b/middleware/csrf/csrf_test.go @@ -849,36 +849,124 @@ func Test_CSRF_SecFetchSite(t *testing.T) { token = strings.Split(strings.Split(token, ";")[0], "=")[1] tests := []struct { - name string - method string - secFetchSite string - origin string - scheme string - expectedStatus int + name string + method string + secFetchSite string + origin string + expectedStatus int16 + https bool + expectFetchSiteInvalid bool }{ - {"same-origin allowed", fiber.MethodPost, "same-origin", "http://example.com", "http", http.StatusOK}, - {"none allowed", fiber.MethodPost, "none", "http://example.com", "http", http.StatusOK}, - {"cross-site blocked", fiber.MethodPost, "cross-site", "http://example.com", "http", http.StatusForbidden}, - {"same-site blocked", fiber.MethodPost, "same-site", "http://example.com", "http", http.StatusForbidden}, - - {"no header with no origin", fiber.MethodPost, "", "", "http", http.StatusOK}, - {"no header with matching origin", fiber.MethodPost, "", "http://example.com", "http", http.StatusOK}, - {"no header with mismatched origin", fiber.MethodPost, "", "https://attacker.example", "http", http.StatusForbidden}, - {"no header with null origin", fiber.MethodPost, "", "null", "https", http.StatusForbidden}, - - {"GET allowed", fiber.MethodGet, "cross-site", "", "http", http.StatusOK}, - {"HEAD allowed", fiber.MethodHead, "cross-site", "", "http", http.StatusOK}, - {"OPTIONS allowed", fiber.MethodOptions, "cross-site", "", "http", http.StatusOK}, - {"PUT blocked", fiber.MethodPut, "cross-site", "http://example.com", "http", http.StatusForbidden}, + { + name: "same-origin allowed", + method: fiber.MethodPost, + secFetchSite: "same-origin", + origin: "http://example.com", + expectedStatus: http.StatusOK, + }, + { + name: "none allowed", + method: fiber.MethodPost, + secFetchSite: "none", + origin: "http://example.com", + expectedStatus: http.StatusOK, + }, + { + name: "cross-site with origin allowed", + method: fiber.MethodPost, + secFetchSite: "cross-site", + origin: "http://example.com", + expectedStatus: http.StatusOK, + }, + { + name: "same-site with origin allowed", + method: fiber.MethodPost, + secFetchSite: "same-site", + origin: "http://example.com", + expectedStatus: http.StatusOK, + }, + { + name: "cross-site with mismatched origin blocked", + method: fiber.MethodPost, + secFetchSite: "cross-site", + origin: "https://attacker.example", + expectedStatus: http.StatusForbidden, + }, + { + name: "same-site with null origin blocked", + method: fiber.MethodPost, + secFetchSite: "same-site", + origin: "null", + expectedStatus: http.StatusForbidden, + https: true, + }, + { + name: "invalid header blocked", + method: fiber.MethodPost, + secFetchSite: "weird", + origin: "http://example.com", + expectedStatus: http.StatusForbidden, + expectFetchSiteInvalid: true, + }, + { + name: "no header with no origin", + method: fiber.MethodPost, + origin: "", + expectedStatus: http.StatusOK, + }, + { + name: "no header with matching origin", + method: fiber.MethodPost, + origin: "http://example.com", + expectedStatus: http.StatusOK, + }, + { + name: "no header with mismatched origin", + method: fiber.MethodPost, + origin: "https://attacker.example", + expectedStatus: http.StatusForbidden, + }, + { + name: "no header with null origin", + method: fiber.MethodPost, + origin: "null", + expectedStatus: http.StatusForbidden, + https: true, + }, + { + name: "GET allowed", + method: fiber.MethodGet, + secFetchSite: "cross-site", + expectedStatus: http.StatusOK, + }, + { + name: "HEAD allowed", + method: fiber.MethodHead, + secFetchSite: "cross-site", + expectedStatus: http.StatusOK, + }, + { + name: "OPTIONS allowed", + method: fiber.MethodOptions, + secFetchSite: "cross-site", + expectedStatus: http.StatusOK, + }, + { + name: "PUT with mismatched origin blocked", + method: fiber.MethodPut, + secFetchSite: "cross-site", + origin: "https://attacker.example", + expectedStatus: http.StatusForbidden, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() c := &fasthttp.RequestCtx{} - scheme := tt.scheme - if scheme == "" { - scheme = "http" + scheme := "http" + if tt.https { + scheme = "https" } c.Request.Header.SetMethod(tt.method) c.Request.URI().SetScheme(scheme) @@ -903,8 +991,8 @@ func Test_CSRF_SecFetchSite(t *testing.T) { } h(c) - require.Equal(t, tt.expectedStatus, c.Response.StatusCode()) - if tt.expectedStatus == http.StatusForbidden && tt.secFetchSite != "" && !safe { + require.Equal(t, int(tt.expectedStatus), c.Response.StatusCode()) + if tt.expectFetchSiteInvalid { require.Equal(t, ErrFetchSiteInvalid.Error(), string(c.Response.Body())) } }) From b7cfc54deb05fb7a611dcd5bd178ed2a8f3b571d Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 08:29:31 -0500 Subject: [PATCH 3/8] docs: clarify PR summary scope --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index ed237756805..87fbb2d070d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,7 +56,7 @@ These targets can be invoked via `make ` as needed during development an ## Pull request guidelines - PR titles must start with a category prefix describing the change: `๐Ÿ› bug:`, `๐Ÿ”ฅ feat:`, `๐Ÿ“’ docs:`, or `๐Ÿงน chore:`. -- Generated PR bodies should contain a **Summary** section that captures all changes included in the PR, not just the latest commit. +- Generated PR titles and bodies must summarize the *entire* set of changes on the branch (for example, based on `git log --oneline ..HEAD` or the full diff), **not** just the latest commit. The Summary section should reflect all modifications that will be merged. ## Programmatic checks From a718cbe926df5dd987184484835a01b47008486c Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 08:58:26 -0500 Subject: [PATCH 4/8] docs: verify docs after checks --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 87fbb2d070d..cf81d32820d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,3 +75,7 @@ make test ``` All checks must pass before the generated code can be merged. + +After completing the programmatic checks above, confirm that any relevant +documentation has been updated to reflect the changes made, including PR +instructions when applicable. From f817f2bd9b515dc567961e2a081846126bf71229 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 08:58:34 -0500 Subject: [PATCH 5/8] docs: update CSRF Sec-Fetch-Site guidance --- docs/middleware/csrf.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/middleware/csrf.md b/docs/middleware/csrf.md index c5d7cb39881..62deebb3a28 100644 --- a/docs/middleware/csrf.md +++ b/docs/middleware/csrf.md @@ -171,6 +171,10 @@ async function makeRequest(url, data) { The middleware employs a robust, defense-in-depth strategy to protect against CSRF attacks. The primary defense is token-based validation, which operates in one of two modes depending on your configuration. This is supplemented by a mandatory secondary check on the request's origin. +### Fetch Metadata Guardrails + +- **Sec-Fetch-Site**: For unsafe methods, the middleware inspects the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header when present. Requests with `same-origin` or `none` proceed directly to origin/token validation. Requests with `same-site` or `cross-site` are allowed to continue to the standard origin/trusted-origin checks (and token verification), enabling trusted cross-site clients to function. Any other header value is rejected with `ErrFetchSiteInvalid`. Browsers that omit the header are treated as if it were absent and continue through origin validation normally. + ### 1. Token Validation Patterns #### Double Submit Cookie (Default Mode) From 38324a86fff1f226c3312161bd7544e64c935646 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 3 Dec 2025 09:10:36 -0500 Subject: [PATCH 6/8] Update docs/middleware/csrf.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- docs/middleware/csrf.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/middleware/csrf.md b/docs/middleware/csrf.md index 62deebb3a28..162f8a84eeb 100644 --- a/docs/middleware/csrf.md +++ b/docs/middleware/csrf.md @@ -173,7 +173,7 @@ The middleware employs a robust, defense-in-depth strategy to protect against CS ### Fetch Metadata Guardrails -- **Sec-Fetch-Site**: For unsafe methods, the middleware inspects the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header when present. Requests with `same-origin` or `none` proceed directly to origin/token validation. Requests with `same-site` or `cross-site` are allowed to continue to the standard origin/trusted-origin checks (and token verification), enabling trusted cross-site clients to function. Any other header value is rejected with `ErrFetchSiteInvalid`. Browsers that omit the header are treated as if it were absent and continue through origin validation normally. +- **Sec-Fetch-Site**: For unsafe methods, the middleware inspects the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header when present. If the header value is not one of "same-origin", "none", "same-site", or "cross-site", the request is rejected with `ErrFetchSiteInvalid`. If the header is valid or absent, the request proceeds to the standard origin and token validation checks. This provides an early check to block requests with invalid `Sec-Fetch-Site` values, while allowing legitimate same-site and cross-site requests to be validated by the existing mechanisms. ### 1. Token Validation Patterns From 2e486a77332e2a223457b3727a95192e2169b24d Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Thu, 4 Dec 2025 07:35:10 -0500 Subject: [PATCH 7/8] Enhance CSRF middleware with Sec-Fetch-Site validation CSRF middleware now inspects the Sec-Fetch-Site header for unsafe methods, rejecting requests with invalid values. Also, added redaction options for CSRF and Idempotency middleware. --- docs/whats_new.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/whats_new.md b/docs/whats_new.md index 1ffaa70f82b..8a1cd4a74bd 100644 --- a/docs/whats_new.md +++ b/docs/whats_new.md @@ -1286,6 +1286,8 @@ The `Expiration` field in the CSRF middleware configuration has been renamed to CSRF now redacts tokens and storage keys by default and exposes a `DisableValueRedaction` toggle (default `false`) if you must surface those values in diagnostics. +The CSRF now handles **Sec-Fetch-Site** header for unsafe methods, the middleware inspects the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header when present. If the header value is not one of "same-origin", "none", "same-site", or "cross-site", the request is rejected with `ErrFetchSiteInvalid`. If the header is valid or absent, the request proceeds to the standard origin and token validation checks. This provides an early check to block requests with invalid `Sec-Fetch-Site` values, while allowing legitimate same-site and cross-site requests to be validated by the existing mechanisms. + ### Idempotency Idempotency middleware now redacts keys by default and offers a `DisableValueRedaction` configuration flag (default `false`) to expose them when debugging. From c0b6297ac7220031d00d1b4f5c1e2c609178649e Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Thu, 4 Dec 2025 07:41:46 -0500 Subject: [PATCH 8/8] Update CSRF middleware validation and configuration The CSRF middleware now validates the Sec-Fetch-Site header for unsafe HTTP methods, rejecting requests with invalid values. Additionally, the Expiration field has been renamed to IdleTimeout, and the default value has been changed from 1 hour to 30 minutes. --- docs/whats_new.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/whats_new.md b/docs/whats_new.md index 8a1cd4a74bd..2a09e7f5278 100644 --- a/docs/whats_new.md +++ b/docs/whats_new.md @@ -1286,7 +1286,7 @@ The `Expiration` field in the CSRF middleware configuration has been renamed to CSRF now redacts tokens and storage keys by default and exposes a `DisableValueRedaction` toggle (default `false`) if you must surface those values in diagnostics. -The CSRF now handles **Sec-Fetch-Site** header for unsafe methods, the middleware inspects the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header when present. If the header value is not one of "same-origin", "none", "same-site", or "cross-site", the request is rejected with `ErrFetchSiteInvalid`. If the header is valid or absent, the request proceeds to the standard origin and token validation checks. This provides an early check to block requests with invalid `Sec-Fetch-Site` values, while allowing legitimate same-site and cross-site requests to be validated by the existing mechanisms. +The CSRF middleware now validates the [`Sec-Fetch-Site`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site) header for unsafe HTTP methods. When present, requests with invalid `Sec-Fetch-Site` values (not one of "same-origin", "none", "same-site", or "cross-site") are rejected with `ErrFetchSiteInvalid`. Valid or absent headers proceed to standard origin and token validation checks, providing an early gate to catch malformed requests while maintaining compatibility with legitimate cross-site traffic. ### Idempotency