🐛 bug: validate nested CSRF extractor chains

[gaby]

Motivation

• Prevent a security bypass where a nested extractors.Chain(...) can hide a fallback FromCookie extractor that reads the same cookie used for CSRF token storage, causing the double-submit check to compare a cookie to itself.
• The CSRF refactor introduced acceptance of the shared extractors.Extractor type (which includes FromCookie) while the existing validation only inspected one chain level, leaving nested cookie fallbacks unchecked.

Description

• Add a recursive inspector isInsecureCookieExtractorRecursive and use it when validating chained extractors in middleware/csrf/config.go so nested Chain entries are checked transitively.
• Replace the shallow chained check in validateExtractorSecurity with the recursive check so any nested cookie extractor matching CookieName triggers a panic.
• Add a regression unit test InsecureNestedChainedExtractor to middleware/csrf/config_test.go that constructs a nested Chain(..., Chain(..., FromCookie("csrf_"))) and asserts the configuration normalization panics.
• Preserve existing behavior for primary extractor checks and case-insensitive cookie-name warnings.

Testing

• Ran go test ./middleware/csrf -run 'Test_CSRF_ExtractorSecurity_Validation' and it passed (ok github.com/gofiber/fiber/v3/middleware/csrf).
• Ran the full test suite via make test and all tests passed (3645 tests run, 2 skipped).
• Ran make generate, make betteralign, make format, and make lint and they completed successfully.
• Ran make audit which failed due to govulncheck findings in the local Go 1.25.1 standard library (the scan reported 25 standard-library vulnerabilities); this is an environment/toolchain issue and not a regression introduced by these changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 103fd563-e4cb-4cf3-b29e-2143ee2afd02

📥 Commits

Reviewing files that changed from the base of the PR and between e831c14 and 4858cf8.

📒 Files selected for processing (2)

• docs/guide/extractors.md
• extractors/README.md

✅ Files skipped from review due to trivial changes (2)

• docs/guide/extractors.md
• extractors/README.md

---

Walkthrough

The PR adds Extractor.Contains introspection and runtime cycle detection for chained extractors, enabling CSRF middleware to recursively validate nested chains for insecure cookie reads. Test helper functions are refactored to simplify request construction, and comprehensive tests verify cycle prevention, safe reuse across middleware, and nested insecure composition detection. Documentation is updated to explain the new introspection and safety mechanisms.

Changes

Extractor Introspection, Cycle Detection, and CSRF Validation

Layer / File(s)	Summary
Extractor Contains method and cycle detection extractors/extractors.go	ErrChainCycle sentinel and Extractor.Contains(pred) method enable introspection via explicit-stack iteration. Chain factory adds per-request cycle guarding using c.Locals, detecting re-entrancy and returning ErrChainCycle on cyclic re-entry.
CSRF recursive nested chain validation middleware/csrf/config.go, middleware/csrf/config_test.go	validateExtractorSecurity now uses cfg.Extractor.Contains to traverse the full extractor tree and detect unsafe nested cookie reads. Configuration panics if any nested extractor unsafely reads the CSRF cookie. The fmt import is removed since index-specific panic messages are no longer needed. New tests verify panic on nested insecure extractors and termination on deep secure chains.
Extractor Contains and cycle prevention tests extractors/extractors_test.go	Test_Extractor_Contains validates predicate matching across nested chains. Test_Extractor_Chain_Cycle_Prevention asserts cycles return ErrChainCycle while non-cyclic nesting succeeds. Test_Extractor_Chain_ReusedAcrossMiddleware verifies safe reuse of Chain instances across handlers.
Test helper function refactoring extractors/extractors_test.go	newRequest helper simplified to accept only path target and always construct GET requests. All existing test invocations updated to the new signature.
Documentation updates for Contains and chain introspection docs/guide/extractors.md, extractors/README.md	Both docs now document Extractor.Contains, cycle prevention via ErrChainCycle, metadata preservation from first extractor, and defensive copying. Code examples demonstrate detecting specific extractors within chained configurations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• gofiber/fiber#3630: Both PRs modify CSRF extractor security validation in middleware/csrf/config.go, with the main PR extending the prior "check insecure cookie extractors (including chained ones)" approach by traversing the full extractor tree via the new Extractor.Contains(...) logic (plus cycle-safe handling).
• gofiber/fiber#3746: Both PRs modify CSRF's extractor-security validation logic in middleware/csrf/config.go, including how isInsecureCookieExtractor and the extractor tree/chain are inspected (main PR switches to a full-tree cfg.Extractor.Contains(...) traversal).

Suggested labels

🧹 Updates

Suggested reviewers

• efectn
• ReneWerner87

Poem

🐇 With stacks so tall and chains so deep,
We traverse each link, no secrets to keep!
Cycles are caught before they spiral 'round,
Each extractor checked—safety abound!
The warren's introspection tools now complete, hooray! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	Title is vague and uses emoji; lacks specificity about fixing nested CSRF extractor chain validation.	Replace emoji and generic 'bug' label with descriptive title like 'Add recursive validation for nested CSRF extractor chains'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	Description provides clear motivation, implementation details, testing results, but most checklist items are unchecked without explanation of which apply.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-csrf-validation-for-nested-extractors

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.60%. Comparing base (099802b) to head (e831c14).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4439      +/-   ##
==========================================
+ Coverage   91.59%   91.60%   +0.01%     
==========================================
  Files         134      134              
  Lines       13518    13537      +19     
==========================================
+ Hits        12382    12401      +19     
  Misses        722      722              
  Partials      414      414              

Flag	Coverage Δ
unittests	91.60% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gaby]

This should be a package const