🐛 bug: enforce paginate sort allowlist when AllowedSorts is unset

[gaby]

Motivation

• Restore safe pagination behavior after parseSortQuery became permissive when AllowedSorts was nil or empty, which allowed attacker-controlled sort text to reach downstream consumers (e.g., SQL ORDER BY) and created an injection risk.
• Ensure the middleware falls back to a safe DefaultSort when no whitelist is configured rather than implicitly permitting all client-supplied fields.

Description

• Reworked parseSortQuery in middleware/paginate/paginate.go to accept a sort field only when it is explicitly present in AllowedSorts (removed the len(allowedSorts) == 0 permit-all branch).
• Updated tests in middleware/paginate/paginate_test.go so nil/empty AllowedSorts falls back to DefaultSort and tests assert the safe behavior.
• Clarified AllowedSorts semantics in docs/middleware/paginate.md to state that nil/empty allowlist causes request sort fields to be ignored and the default sort to be used.
• Regenerated/updated interface artifacts (ctx_interface_gen.go, req_interface_gen.go) as part of repository checks.

[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: 2bd2e2aa-53a7-4c9a-8c91-086ebd810fbe

📥 Commits

Reviewing files that changed from the base of the PR and between 1ad31dd and 67b9a53.

📒 Files selected for processing (1)

• middleware/paginate/paginate_test.go

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

• middleware/paginate/paginate_test.go

---

Walkthrough

This PR tightens sort-field validation in the paginate middleware so that empty or nil allowedSorts disables request-provided sort fields and falls back to the default sort. It also clarifies documentation for request-source identification methods across generated interface files to explicitly reference loopback IP behavior.

Changes

Paginate Sort Query Validation

Layer / File(s)	Summary
Core Behavior Change middleware/paginate/paginate.go	Sort-field inclusion logic in parseSortQuery updated to only accept fields present in allowedSorts; empty allowedSorts now results in no sort fields collected, falling back to defaultSort.
Middleware Documentation docs/middleware/paginate.md	AllowedSorts configuration docs clarified: nil or empty now disables request sort fields and uses the default sort instead of the previous nil-only behavior.
Test Updates middleware/paginate/paginate_test.go	Test_ParseSortQuery expectations updated: "Nil AllowedSorts" test case now expects fallback to defaultSort, and "Dash in comma list" test case adjusted to match new nil-allowedSorts fallback behavior.

Request Interface Documentation

Layer / File(s)	Summary
Context Interface ctx_interface_gen.go	Ctx.IsFromLocal() documentation updated to explicitly state it returns true when the request originates from a loopback IP.
Request Interface req_interface_gen.go	IsProxyTrusted() doc expanded to describe behavior when Config.TrustProxy is false and proxy-range/IP-map checking; IsFromLocal() doc clarified to reference loopback IP instead of "local" phrasing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• gofiber/fiber#3206: Modifies Ctx/Req interface docs including IsFromLocal/IsProxyTrusted, related to doc changes here.
• gofiber/fiber#4270: Changes IsFromLocal semantics and adds IsFromUnixSocket, touching the same interface/docs.
• gofiber/fiber#4127: Adds/changes pagination middleware that this PR further adjusts (parseSortQuery/docs/tests).

Suggested labels

📒 Documentation

Suggested reviewers

• sixcolors
• ReneWerner87
• efectn

Poem

🐰 I hopped through docs and code one night,
Empty sorts now yield default right,
Loopback whispers, clear and small,
Docstrings sing — precise for all. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: enforcing the paginate sort allowlist when AllowedSorts is unset, which aligns with the core security fix in the changeset.
Description check	✅ Passed	The PR description is well-structured with clear motivation, implementation details, and scope. It addresses the security issue, documents code changes across multiple files, and notes test/documentation updates.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-nil-allowedsorts-vulnerability

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.1)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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.}

[gemini-code-assist]

Code Review

This pull request clarifies the documentation for the IsFromLocal method across the context and request interfaces and modifies the pagination middleware's sorting logic. The AllowedSorts configuration now ignores requested sort fields if the whitelist is nil or empty, defaulting to the specified default sort instead of allowing all fields. Corresponding updates were made to the documentation and test suite to reflect this change in behavior. I have no feedback to provide as there were no review comments.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.26%. Comparing base (c48bbc2) to head (67b9a53).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4276      +/-   ##
==========================================
+ Coverage   91.21%   91.26%   +0.04%     
==========================================
  Files         130      130              
  Lines       12760    12760              
==========================================
+ Hits        11639    11645       +6     
+ Misses        709      704       -5     
+ Partials      412      411       -1     

Flag	Coverage Δ
unittests	91.26% <100.00%> (+0.04%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

[Copilot]

Pull request overview

This PR restores safe pagination sorting defaults by requiring an explicit AllowedSorts allowlist for any client-supplied sort fields, preventing unvalidated sort text from propagating to downstream consumers (e.g., SQL ORDER BY).

Changes:

• Enforced allowlist-only behavior in parseSortQuery (no “permit all when allowlist is empty” fallback).
• Updated pagination tests to expect default sorting when AllowedSorts is nil.
• Clarified AllowedSorts semantics in paginate docs and regenerated interface artifacts with updated IsFromLocal wording.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
middleware/paginate/paginate.go	Enforces that only explicitly allowed sort fields are accepted; otherwise falls back to DefaultSort.
middleware/paginate/paginate_test.go	Updates expectations so nil AllowedSorts ignores request sorts and uses the default.
docs/middleware/paginate.md	Documents that nil/empty AllowedSorts ignores request sort and uses the default sort.
ctx_interface_gen.go	Updates generated interface comment for IsFromLocal to reflect loopback semantics.
req_interface_gen.go	Updates generated interface comment for IsFromLocal to reflect loopback semantics.