feat(otlp): added CORS support - #2336
Conversation
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
d6c5f02 to
bc6fdcd
Compare
Binary Size Analysis (Agent Data Plane)Baseline: 0f47357 · Comparison: bc6fdcd · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc6fdcd225
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// characters (for example, `http://*.domain.com` matches `http://foo.domain.com`). Only the | ||
| /// first `*` is treated as a wildcard; additional `*` characters become literal suffix. | ||
| fn build_cors_layer(cors: &Cors) -> CorsLayer { | ||
| let mut layer = CorsLayer::new(); |
There was a problem hiding this comment.
Allow POST in CORS preflight responses
When a browser exporter sends OTLP using application/json or application/x-protobuf, it preflights the POST request. CorsLayer::new() starts with no allowed methods, and this builder never calls allow_methods, so the preflight response omits permission for POST and the browser blocks the upload despite the configured origin; add POST to the layer's allowed methods.
Useful? React with 👍 / 👎.
| let has_wildcard = cors.allowed_origins.iter().any(|o| o.contains('*')); | ||
|
|
||
| if has_wildcard { | ||
| layer = layer.allow_origin(Any); |
There was a problem hiding this comment.
Match partial origin wildcards instead of allowing every origin
For a partial-wildcard configuration such as http://*.example.com, this contains('*') check selects Any, so an unrelated origin such as https://evil.example receives CORS permission and can submit telemetry. Only a bare * should enable allow-all; partial wildcards need the documented prefix/suffix matching that the test-only helper currently implements.
Useful? React with 👍 / 👎.
| /// Origins allowed to make cross-origin requests. Allows for wildcard character when describing | ||
| /// domains (for example: "http://*.domains.com") | ||
| /// | ||
| /// Defaults to an empty list (disabling CORS). | ||
| pub allowed_origins: Vec<String>, |
There was a problem hiding this comment.
Document operator guidance for every CORS field
The new public CORS settings describe their mechanics and mostly their defaults, but none explains which operators or workloads should change them; allowed_origins also fails to document the security-relevant bare-* boundary. Add actionable operator guidance and complete edge-case documentation for all four fields as required for configuration fields.
AGENTS.md reference: AGENTS.md:L149-L154
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
A configured partial wildcard such as http://*.example.com currently activates allow_origin(Any), allowing browser OTLP submissions from unrelated origins. The new matcher tests do not protect this path because the matcher is test-only and never used by the CORS layer.
🤖 Datadog Autotest · Commit bc6fdcd · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| let mut layer = CorsLayer::new(); | ||
|
|
||
| // Origins. `rs/cors` treats any `*` in the list as allow-all. | ||
| let has_wildcard = cors.allowed_origins.iter().any(|o| o.contains('*')); |
There was a problem hiding this comment.
Preserve partial-wildcard origin restrictions
Operators attempting to restrict browser OTLP ingestion to a domain family instead expose the receiver to requests from every web origin.
Assertion details
- Input: Configure
otlp_config.receiver.protocols.http.cors.allowed_originswith a documented partial wildcard such ashttp://*.example.com. - Expected: Only a bare
*should permit every origin. Partial wildcards must replace zero or more characters while retaining their configured prefix and suffix, as required by the checked-in CORS schema. - Actual: Every configured origin containing
*selectsallow_origin(Any). Consequently,http://*.example.comalso permits an unrelated origin such ashttp://attacker.example. The adjacentwildcard_matchhelper is compiled only for tests and is never called by the CORS layer. A complete fix must reserveAnyfor a bare*, compile the matcher for production, wire partial patterns into an origin predicate, and make the existing tests exercise the resulting layer; those non-contiguous changes are not suitable for one inline suggestion.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Summary
Added support for the
otlp_config.http.corssub-config.Change Type
How did you test this PR?
Unit tests
References