Disable OTLP export when authorization secrets are empty - #58312
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate issues can overwrite explicit exporter overrides and discard valid fallbacks after malformed header parsing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Filters OTLP endpoints with empty authorization credentials and propagates usable fallback exporter settings.
Changes:
- Rejects empty Authorization and Sentry credentials.
- Promotes fallback endpoints or disables exporters.
- Adds filtering and propagation tests.
File summaries
| File | Review |
|---|---|
actions/setup/js/send_otlp_span.test.cjs |
Adds authorization-filtering coverage. |
actions/setup/js/send_otlp_span.cjs |
Filters unusable endpoints. Moderate: malformed header escapes can discard all valid fallback endpoints. |
actions/setup/js/action_setup_otlp.test.cjs |
Adds fallback and exporter-disabling coverage. |
actions/setup/js/action_setup_otlp.cjs |
Propagates fallback exporter values. Moderate: explicit exporter overrides can be replaced unconditionally. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| const [primaryEndpoint] = parsedEndpoints; | ||
| const endpoint = primaryEndpoint?.url || ""; | ||
| const headers = primaryEndpoint?.headers || ""; | ||
| if (endpoint !== (process.env.OTEL_EXPORTER_OTLP_ENDPOINT || "")) { |
There was a problem hiding this comment.
Addressed in c527057: exporter values are promoted or cleared only when both still match the unfiltered primary endpoint, preserving explicit overrides.
| function hasEmptyOTLPAuthorizationHeader(raw) { | ||
| const headers = parseOTLPHeaders(raw); | ||
| return Object.entries(headers).some(([key, value]) => { | ||
| const normalizedKey = key.toLowerCase(); | ||
| return (normalizedKey === "authorization" || normalizedKey === "x-sentry-auth") && value === ""; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Addressed in c527057: malformed percent-encoded header pairs are skipped individually, so later valid endpoints remain available.
|
@copilot This PR still has unresolved review feedback. Please address these review threads and then run the
Also refresh the branch if needed after applying the fixes.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot This PR still has unresolved review feedback. Please address these review threads and then run the
Also refresh the branch if needed after applying the fixes.
|
Both requested fixes are present in c527057 and covered by the focused OTLP tests. The branch was refreshed to 211c1bd; the CJS workflow completed successfully on that head. |
|
🎉 This pull request is included in a new release. Release: |
Reusable workflows may receive empty OTLP authorization secrets, causing telemetry exporters to target protected endpoints without credentials.
Endpoint filtering
Authorizationorx-sentry-authheaders.Runtime propagation
Coverage