🐛 bug: keep IsFromLocal loopback-only and add unix-socket helper#4270
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds IsFromUnixSocket() to Req and Ctx interfaces, implements DefaultReq.IsFromUnixSocket(), changes DefaultReq.IsFromLocal() to consider only loopback IP, updates tests to reflect the separation, and adds API documentation for Ctx.IsFromUnixSocket(). ChangesUnix Socket Detection Separation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4270 +/- ##
=======================================
Coverage 91.25% 91.25%
=======================================
Files 130 130
Lines 12753 12753
=======================================
Hits 11638 11638
Misses 702 702
Partials 413 413
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api/ctx.md`:
- Around line 1267-1281: Run the Markdown linter by executing "make markdown" to
validate and fix any lint issues introduced when updating the IsFromUnixSocket
documentation; after running the linter, review and commit the
generated/modified Markdown fixes (ensure the docs/api/ctx.md entry for
IsFromUnixSocket remains consistent with project style and examples) so
CI/markdown checks pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6c6689e-baad-4738-a127-d7d7c63e077b
📒 Files selected for processing (5)
ctx_interface_gen.goctx_test.godocs/api/ctx.mdreq.goreq_interface_gen.go
There was a problem hiding this comment.
Pull request overview
This PR tightens Fiber’s “local request” detection to avoid treating Unix-domain-socket traffic as inherently trusted, closing an authz bypass risk when public reverse proxies forward requests over a Unix socket. It also adds an explicit helper to let applications intentionally detect Unix-socket transport.
Changes:
- Makes
Req.IsFromLocal()loopback-IP only (removes the previous unconditional Unix-socket “local” behavior). - Adds
IsFromUnixSocket()onReq/Ctxto explicitly detect Unix-domain-socket remote addresses. - Updates generated interfaces, API docs, and unit tests to cover the new/changed behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| req.go | Removes Unix-socket special-casing from IsFromLocal() and introduces IsFromUnixSocket(). |
| req_interface_gen.go | Exposes IsFromUnixSocket() on the generated Req interface. |
| ctx_interface_gen.go | Exposes IsFromUnixSocket() on the generated Ctx interface (via promoted methods). |
| docs/api/ctx.md | Documents the new Ctx.IsFromUnixSocket() API. |
| ctx_test.go | Updates IsFromLocal Unix-socket expectations and adds coverage for IsFromUnixSocket() on both Ctx and Req. |
There was a problem hiding this comment.
Code Review
This pull request introduces the IsFromUnixSocket method to the Ctx and Req interfaces and updates the IsFromLocal method to exclusively check for loopback IP addresses. Feedback highlights that IsFromUnixSocket needs to be implemented in DefaultCtx to satisfy the interface and prevent compilation errors. Additionally, suggestions were made to update the documentation for IsFromLocal to reflect its changed behavior and to provide a more illustrative example for the new method in the API documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Motivation
IsFromLocal(), enabling external requests forwarded by a public reverse proxy to be mistaken for localhost.Description
*net.UnixAddr=>truebranch fromIsFromLocal()so it only returnstruefor loopback IPs (req.go).IsFromUnixSocket()onReqthat returnstruewhen the remote address is a Unix domain socket (req.go).req_interface_gen.go,ctx_interface_gen.go) and added documentation forIsFromUnixSocket()indocs/api/ctx.md.ctx_test.goto assert Unix-socket addresses are no longer treated as local and addedTest_Ctx_IsFromUnixSocket_RemoteAddrto validate the new helper on bothCtxandReq.