Skip to content

fix(ingest): allow user-agent in the CORS preflight - #1337

Open
JeremyFunk wants to merge 1 commit into
mainfrom
fix/ingest-cors-allow-user-agent
Open

JeremyFunk wants to merge 1 commit into
mainfrom
fix/ingest-cors-allow-user-agent

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adds user-agent to the request headers the ingest gateway allows in a CORS preflight.

  • apps/ingest/src/main.rs: USER_AGENT in the CorsLayer allow_headers list, plus a wording fix in the SDK_HINT_HEADER doc comment, which said no browser lets a page set user-agent.
  • apps/cli/src/server/serve.ts: the same header in the local-mode listener's allow-list, which mirrors the gateway's for the same SDKs.
  • apps/landing/src/content/docs/reference/ingest.md: the Browsers section lists the allowed headers, so it gains User-Agent.

Origins, methods, exposed headers, max-age and layer ordering are unchanged.

Why

@maple-dev/effect-sdk/client sets a user-agent request header on every OTLP request. Whether that reaches the network depends on the browser:

  • User-Agent is not a forbidden request-header in the Fetch standard, and it is not CORS-safelisted, so a browser that honours a script-set value lists it in Access-Control-Request-Headers. The preflight fails with a network error when a listed name is missing from Access-Control-Allow-Headers (CORS-preflight fetch, section 4.8, step 7).
  • tower-http 0.6.8 answers every OPTIONS with the configured list verbatim (cors/allow_headers.rs, AllowHeadersInner::Const); it does not mirror the request, and enforcement is the browser's.

Measured on 2026-10-08 in real browsers against a mock ingest whose CORS answers mirror this CorsLayer:

Browser Script-set user-agent Preflight Access-Control-Request-Headers /v1/traces, /v1/logs, /v1/metrics POSTs arriving cross-origin
Firefox 157, before sent authorization,content-type,user-agent,x-maple-sdk (keyless: content-type,user-agent,x-maple-sdk) 0, with and without an ingest key
Firefox 157, with user-agent allowed sent same all
Chrome / Chromium ignored no user-agent all, before and after

Session posts and @maple-dev/browser never set the header and are unaffected.

Other browsers

  • Chromium: drops a script-set User-Agent from fetch requests (MDN, forbidden request header note, Chromium issue 571722). Matches the measurement.
  • Firefox: sends it. Measured.
  • Safari / WebKit: not measured. From source, WebKit is expected to behave like Firefox: isForbiddenHeaderName in HTTPParsers.cpp does not list User-Agent, isCrossOriginSafeRequestHeader in the same file does not safelist it, and r224684 (2017) made the loader keep a User-Agent the request already carries. Treat this as a source reading until someone confirms it in Safari.

Coverage

  • Gateway (Rust): no unit test. The CorsLayer is built inline in main next to the full application state, and the crate has no preflight or router-level test; tests call the handlers' _inner functions directly. Covering it would mean extracting the layer and adding a served-router harness, which is more than this change. It is covered by the browser measurement above and by ingest-rust-tests compiling it.
  • Local mode (CLI): apps/cli/test/server-network.test.ts has the exact-header assertion updated and one new case next to the existing authorization and x-maple-sdk ones.
  • Local mode was not part of the browser measurement; it gets the header because its allow-list has the same role and the same SDKs point at it.

Deploy

  • The gateway change takes effect when ingest deploys, and it unblocks every already-published SDK version without an SDK upgrade.
  • The local-mode change ships with the next CLI release.
  • A companion SDK change (fix/sdk-browser-drop-user-agent) stops the browser client from setting the header; the two are independent and can merge in either order.

Review bots

CodeRabbit and the Maple review reported no findings on 187f3b3, so nothing was declined.

Some browsers send a user-agent header that a page set on fetch and list it in Access-Control-Request-Headers. The ingest allow-list did not include it, so the preflight was refused and the request never left the browser. Add it to the gateway allow-list, to the local-mode listener that mirrors it, and to the ingest reference page.
@maple-review-bot

maple-review-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
The gateway change is one constant in an existing allow-list; only the CLI mirror has a test, the Rust side rests on the author's browser measurement.
quality 100/100 · no findings · tests partial · risk low

Adds user-agent to the ingest gateway's and the CLI local-mode CORS allow-header lists, plus the matching docs and comment wording. Contained, no behavior beyond preflight acceptance; safe to merge.

  • CorsLayer.allow_headers in apps/ingest/src/main.rs now allows USER_AGENT
  • corsHeadersForAllowedOrigin in apps/cli/src/server/serve.ts lists user-agent
  • Docs and the SDK_HINT_HEADER comment stop claiming no browser sends it
What was checked
  • user-agent is only added to response Access-Control-Allow-Headers; no server-side gate reads it, and record_client_identity merely records it (apps/ingest/src/main.rs:3040)
  • The effect-sdk really does set the header on every OTLP request, so the Firefox preflight failure it describes is reachable (packages/effect-sdk/src/shared/flush-core.ts:90)
  • Origins, methods, exposed headers and layer ordering are unchanged; the CLI exact-header assertion is updated to match (apps/cli/test/server-network.test.ts:229)

187f3b3 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 793387f2-63c5-428a-ba1b-cdbf1060e3f3
📥 Commits

Reviewing files that changed from the base of the PR and between 4bf4b7a and 187f3b3.

📒 Files selected for processing (4)
  • apps/cli/src/server/serve.ts
  • apps/cli/test/server-network.test.ts
  • apps/ingest/src/main.rs
  • apps/landing/src/content/docs/reference/ingest.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The CLI server and ingest service now allow the user-agent request header in CORS preflight responses. The CLI test and ingest reference documentation reflect the updated allow-list.

Changes

CORS request headers

Layer / File(s) Summary
Allow user-agent in CORS preflight requests
apps/cli/src/server/serve.ts, apps/cli/test/server-network.test.ts, apps/ingest/src/main.rs, apps/landing/src/content/docs/reference/ingest.md
The CLI server and ingest service add user-agent to their CORS allow-lists. The CLI test checks the allowed headers for a browser origin, and the reference documentation lists User-Agent.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: makisuo

Merge Risk: ⚪ Minimal · up to 187f3

The change adds User-Agent to the CORS preflight allow-lists while retaining the CLI’s existing origin check. No actionable merge-blocking risk is evident, so it appears ready for normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 …
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: allowing the user-agent header in the ingest CORS preflight.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant