Skip to content

fix: strip cf-connecting-ip header in forward_export_request#1824

Merged
alexmojaki merged 1 commit intopydantic:mainfrom
BreytMN:main
Apr 1, 2026
Merged

fix: strip cf-connecting-ip header in forward_export_request#1824
alexmojaki merged 1 commit intopydantic:mainfrom
BreytMN:main

Conversation

@BreytMN
Copy link
Copy Markdown
Contributor

@BreytMN BreytMN commented Apr 1, 2026

Summary

logfire_proxy forwards cf-connecting-ip to Logfire's API when the app runs behind Cloudflare, causing a 403 response. By adding cf-connecting-ip to headers_to_remove in forward_export_request the issue disappears.

Problem

When using logfire_proxy to forward browser telemetry from an app deployed behind Cloudflare (e.g. Railway + Cloudflare as in my use case), Logfire's API returns 403 when trying to send telemetry from the browser like it is done like in the docs. Locally it works but in a server behind Cloudflare it adds a cf-connecting-ip header to the requests passed to the backend, and logfire_proxy forwards it in the server-to-server request to Logfire's API which results in the 403. Claude says it raises a 403 because Logfire's server also sits behinds Cloudflare.

Testing

Tested by injecting a bunch of production headers individually into the request locally before calling logfire_proxy:

for k, v in test_headers.items():
    request._headers._list.append((k.lower().encode(), v.encode()))

return await logfire_proxy(request)
Header(s) Result
All infrastructure headers (cf-* + x-forwarded-* + x-railway-*) 403
Cloudflare only 403
Railway only OK
cf-connecting-ip alone 403
cf-connecting-ipv6 alone OK
cf-ipcountry alone OK
cf-ray alone OK
cf-visitor alone OK
No injected headers (local, no Cloudflare) OK

Then I just removed the cf-connecting-ip from the request object directly before calling the logfire_proxy (see below) and it also worked when deployed to Railway + Cloudflare

@app.post("/logfire-proxy/{path:path}")
async def proxy_browser_telemetry(request: Request) -> Any:
    origin = request.headers.get("origin", "")
    if not ALLOWED_ORIGIN or origin != ALLOWED_ORIGIN:
        return Response(status_code=403)

    request._headers._list = [
        (k, v) for k, v in request._headers._list if k != b"cf-connecting-ip"
    ]

    return await logfire_proxy(request)

Fix

It is a small issue but since (according to Claude inference) Logfire uses Cloudflare it is just a one-liner that can help all other users that also use Cloudflare, as it is not clear from the docs that it could fail thanks to this specific reason.

Environment

  • logfire 4.31.0
  • Deployed on Railway behind Cloudflare
  • @pydantic/logfire-browser via esm.sh

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines 82 to 86
'trailer',
'upgrade',
'cookie',
'cf-connecting-ip',
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Other spoofable IP/proxy headers are not stripped

The PR adds cf-connecting-ip to the removal set, which is a good security fix. However, other headers commonly used for IP identification — such as x-forwarded-for, x-real-ip, true-client-ip, and cf-ipcountry — are also not stripped. A client could set these to spoof identity information forwarded to the upstream Logfire API. Whether this matters depends on how the Logfire backend uses these headers, but it may be worth considering stripping additional headers for defense in depth.

(Refers to lines 72-86)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@alexmojaki
Copy link
Copy Markdown
Collaborator

Thanks!

@alexmojaki alexmojaki merged commit d78810c into pydantic:main Apr 1, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants