feat: resolve client IP from X-Forwarded-For/X-Real-IP headers and log error response body on 4xx/5xx

[BearTS]

Summary

Improves HTTP request logging by resolving the true client IP from reverse-proxy headers and including error response bodies in logs for failed requests.

Issues

Closes #3904

Changes

• Added a clientIP helper that extracts the originating client IP by checking X-Forwarded-For (taking the leftmost entry in comma-separated lists), then X-Real-IP, and finally falling back to the direct peer address. This ensures accurate client identification when Bifrost sits behind a reverse proxy.
• Replaced the direct ctx.RemoteAddr().String() call in the http.remote_addr log field with clientIP(ctx) so logs reflect the real client rather than the proxy.
• Added logging of the response body as http.error for any request that results in a 4xx or 5xx status code, making it easier to diagnose failures from logs alone.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

go test ./...

• Deploy Bifrost behind a reverse proxy that sets X-Forwarded-For or X-Real-IP headers and verify that http.remote_addr in logs reflects the originating client IP rather than the proxy address.
• Trigger a request that returns a 4xx or 5xx response and confirm the http.error field appears in the log output with the response body.

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

Related issues

N/A

Security considerations

The clientIP helper trusts X-Forwarded-For and X-Real-IP headers as provided by upstream proxies. If Bifrost is exposed directly to the internet without a trusted reverse proxy, these headers could be spoofed by clients, resulting in inaccurate IP logging. Ensure Bifrost is always deployed behind a trusted proxy when relying on these values.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicablecs

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an unexported helper that returns the leftmost X-Forwarded-For or X-Real-IP (or "") and updates the CORS middleware completion log to emit http.forwarded_for, include trace_id when present, and attach http.error from non-streaming response bodies for status >= 400.

Changes

Reverse-proxy-aware request logging

Layer / File(s)	Summary
Client IP helper and CORS completion logging transports/bifrost-http/handlers/middlewares.go	Adds clientForwardedIP(ctx) which picks leftmost X-Forwarded-For or X-Real-IP. CORS middleware completion logging now conditionally adds http.forwarded_for, trace_id (from request user values), and for status >= 400 non-streaming responses attaches http.error from the response body when non-empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniff the headers, leftmost scent I find,
X-Forwarded-For whispers the client's mind,
Trace threads tied and error bytes unfurled,
Logs bloom clearer across the HTTP world. 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR aims to enhance HTTP middleware logging and IP resolution, but linked issue #123 requests Files API support—completely unrelated objectives.	Unlink issue #123 or address its Files API requirements. If this PR is part of a broader task, ensure linked issues align with the stated objectives.
Out of Scope Changes check	⚠️ Warning	The code changes (IP resolution and error logging in middleware) are within scope for the PR title but diverge from the linked Files API issue #123.	Link appropriate issues that match the PR objectives, or update the PR to align with the linked Files API requirements.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main changes: resolving client IP from headers and logging error response bodies for 4xx/5xx responses, matching the code diff.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check	✅ Passed	The PR description follows the template structure with all major sections filled out comprehensively, including summary, changes, type of change, affected areas, testing instructions, security considerations, and breaking changes status.

_{✏️ 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 06-02-feat_log_request_body_in_case_of_4xx_

---

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

[BearTS]

• feat: resolve client IP from X-Forwarded-For/X-Real-IP headers and log error response body on 4xx/5xx #3985 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

The change is safe to merge; it adds logging-only fields with no effect on request routing, auth, or response semantics.

Both additions are isolated to the access-log defer block. The IsBodyStream() guard correctly mirrors the existing deferred-trace pattern, preventing any race with streaming responses. No control flow, auth, or data-path logic is altered.

No files require special attention.

Important Files Changed

Filename	Overview
transports/bifrost-http/handlers/middlewares.go	Adds clientForwardedIP helper to extract originating IP from X-Forwarded-For/X-Real-IP headers and logs the response body on 4xx/5xx non-streaming responses; logic is correct and the streaming-safety guard (!IsBodyStream()) follows existing patterns.

_{Reviews (2): Last reviewed commit: "feat: log request body in case of 4xx+" | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@transports/bifrost-http/handlers/middlewares.go`:
- Around line 52-63: The clientIP helper (clientIP(ctx *fasthttp.RequestCtx))
currently trusts X-Forwarded-For/X-Real-IP unconditionally; add a new config
field in transports/config.schema.json (e.g., trusted_proxies: array of CIDR/IP)
and change clientIP to only honor X-Forwarded-For / X-Real-IP when
ctx.RemoteAddr() is in that trusted_proxies list; otherwise return
ctx.RemoteAddr().String(). Ensure the code that loads transport config exposes
the trusted_proxies (parsed as net.IPNet/CIDRs) and reference that configuration
from where clientIP is called or make it accept the config/context so the
header-trusting behavior is gated by the trusted_proxies setting.
- Around line 92-95: The current error-body logging in middlewares.go
unconditionally attaches ctx.Response.Body() for statusCode >= 400; change it to
first consult the content-logging controls (the global disable_content_logging
flag and the per-request content-storage overrides) and only call
logBuilder.Str("http.error", string(body)) when content logging is permitted.
Locate the request/response metadata (e.g., the per-request overrides and
disable_content_logging flag available on the request context or request
metadata object used in this middleware), evaluate whether content logging is
allowed for this request, and gate the ctx.Response.Body() read and
logBuilder.Str call behind that check so error bodies are logged only when
allowed.
- Around line 92-95: The CorsMiddleware log path currently calls
ctx.Response.Body() for statusCode >= 400 which can drain/destructively consume
fasthttp response bodies; update the branch that sets
logBuilder.Str("http.error", ...) so it first reuses the same guards used in
fasthttpResponseToHTTPResponse (deferred trace completion,
large-payload/large-response mode checks and the large-response threshold) and
also respects the disable_content_logging flag / x-bf-disable-content-logging
header; only if all those guards allow materializing the body should you call
ctx.Response.Body() and set http.error, otherwise skip adding http.error to the
log to avoid corrupting chunked/SSE/deferred responses.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 707b1031-48ef-4770-9019-12696a056111

📥 Commits

Reviewing files that changed from the base of the PR and between 0d916ba and 23011cc.

📒 Files selected for processing (1)

• transports/bifrost-http/handlers/middlewares.go

[BearTS]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[akshaydeo]

Merge activity

• Jun 2, 4:43 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 2, 4:43 PM UTC: @akshaydeo merged this pull request with Graphite.