Skip to content

[HDX-5198] Accept both bearer token authorization header formats on ingest - #3027

Merged
kodiakhq[bot] merged 2 commits into
mainfrom
warren/HDX-5198-accept-bearer-prefixed-auth-header
Aug 31, 2026
Merged

[HDX-5198] Accept both bearer token authorization header formats on ingest#3027
kodiakhq[bot] merged 2 commits into
mainfrom
warren/HDX-5198-accept-bearer-prefixed-auth-header

Conversation

@wrn14897

@wrn14897 wrn14897 commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

Our OTel ingest endpoint only accepted the bare-token form of the Authorization header, so RFC 6750 clients that send Authorization: Bearer <token> were rejected.

Root cause: the contrib bearertokenauth extension compares the full header value exactly against scheme + " " + token (bare token when scheme: ''), with no prefix-stripping or case-insensitive scheme handling. Since tokens is a list, we enumerate the accepted forms explicitly:

  • OpAMP-managed mode (opampController.ts): new bearerTokenVariants() helper emits <key>, Bearer <key>, bearer <key>, BEARER <key> for each ingestion API key in bearertokenauth/hyperdx. The DD-API-KEY-based datadog authenticator is intentionally unchanged (bare key only).
  • Standalone mode (config.standalone.auth.yaml): same four forms of ${env:OTLP_AUTH_TOKEN}.

Known limitation: exotic mixed-case schemes (e.g. BeArEr) remain rejected — truly case-insensitive matching would require a custom Go authenticator. The enumerated variants cover the forms real clients send.

Testing

  • Unit tests on buildOtelCollectorConfig assert the emitted token variants (team keys + INGESTION_API_KEY) and that the datadog authenticator stays bare-only.
  • New end-to-end smoke suite (smoke-tests/otel-collector/bearer-auth.bats + otel-collector-bearer-auth service): bare and Bearer-prefixed tokens accepted with data landing in ClickHouse, bearer/BEARER variants accepted, wrong token and missing header rejected. All 7 tests pass locally against the real collector build.
  • make ci-lint and make ci-unit pass.

How to test on Vercel preview

N/A — non-UI change.

References

@wrn14897 wrn14897 added the ai-generated AI-generated content; review carefully before merging. label Aug 28, 2026
@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 96176b8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Patch
@hyperdx/otel-collector Patch
@hyperdx/app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 31, 2026 5:41am
hyperdx-storybook Ready Ready Preview Aug 31, 2026 5:41am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extends standalone and OpAMP-managed OTLP authentication to recognize common Bearer scheme casings while preserving bare-token support and leaving Datadog authentication unchanged.

  • Adds reusable bearer-token variant generation to managed collector configuration.
  • Updates standalone collector authentication with equivalent accepted header values.
  • Adds unit and collector smoke coverage for accepted and rejected authorization forms.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/api/src/opamp/controllers/opampController.ts Expands each managed ingestion key into the bare form and three common Bearer-prefixed forms before generating the collector authentication configuration.
docker/otel-collector/config.standalone.auth.yaml Adds equivalent Bearer-prefixed values to standalone OTLP authentication while retaining the existing bare token.
packages/api/src/opamp/controllers/tests/opampController.test.ts Verifies managed token expansion, all-in-one ingestion-key handling, receiver authentication wiring, and unchanged Datadog behavior.
smoke-tests/otel-collector/bearer-auth.bats Exercises accepted bare and common Bearer-prefixed forms plus rejection of missing and incorrect credentials.
smoke-tests/otel-collector/docker-compose.yaml Adds an isolated standalone authenticated collector service for end-to-end smoke coverage.
smoke-tests/otel-collector/setup_suite.bash Includes the new authenticated collector in the smoke suite readiness sequence.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Client[OTLP client] --> Header[Authorization header]
  Header --> Forms{Header value}
  Forms -->|Bare key| Auth[Bearer-token authenticator]
  Forms -->|Bearer / bearer / BEARER + key| Auth
  Forms -->|Missing or incorrect key| Reject[Reject request]
  Auth -->|Configured token matches| Receiver[OTLP receiver]
  Receiver --> ClickHouse[(ClickHouse)]
Loading

Reviews (4): Last reviewed commit: "Merge branch 'main' into warren/HDX-5198..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 325 passed • 1 skipped • 1282s

Status Count
✅ Passed 325
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@wrn14897
wrn14897 marked this pull request as ready for review August 28, 2026 20:38
@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Additional context: touches background tasks or the delivery pipeline lightly (6 lines, under the 30-line bar for Tier 4)

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 11
  • Production lines changed: 211 (+ 64 in test files, excluded from tier calculation)
  • Branch: warren/HDX-5198-accept-bearer-prefixed-auth-header
  • Author: wrn14897

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Deep Review

No critical issues found. This is a small, well-scoped auth change: bearerTokenVariants() enumerates the bare token plus three Bearer scheme casings for the bearertokenauth/hyperdx tokens list, mirrored in config.standalone.auth.yaml. Each accepted form still requires knowledge of the secret key, so authentication is not weakened, and the teams.filter(team => team.apiKey) / if (config.INGESTION_API_KEY) guards prevent an empty key from ever emitting a permissive bare Bearer token. Datadog DD-API-KEY auth is intentionally left bare-only. No P0/P1/P2 issues surfaced.

🔵 P3 nitpicks (3)
  • packages/api/src/opamp/controllers/opampController.ts:175 — the accepted Bearer scheme casings are enumerated independently in the bearerTokenVariants helper and in docker/otel-collector/config.standalone.auth.yaml, so a future change to the accepted forms in one place can silently diverge from the other.
    • Fix: cross-reference the two locations in a comment so the standalone YAML and the helper are kept in sync deliberately.
  • smoke-tests/otel-collector/bearer-auth.bats:70 — the rejection tests assert [ "$output" != "200" ], which passes on any non-200 status including a 5xx server error, so a collector misconfiguration returning 500 would read as a valid auth rejection.
    • Fix: assert the specific auth-failure status (e.g. [ "$output" = "401" ]) for the wrong-token and missing-header cases.
  • smoke-tests/otel-collector/bearer-auth.bats:52 — the bearer/BEARER accepted-form tests use post_empty_logs_with_auth and assert only HTTP 200, unlike the bare and Bearer cases which verify data lands in ClickHouse.
    • Fix: acceptable as a status-only smoke, but optionally verify row landing for at least one cased variant to confirm the full ingest path.

Reviewers: ce-security-reviewer completed (no findings); ce-correctness, ce-testing, ce-maintainability, ce-kieran-typescript, ce-adversarial, and ce-project-standards were dispatched but had not reported when synthesis was finalized. Findings above reflect the completed security report plus direct analysis of the full diff and opampController.ts context.

Testing gaps:

  • No unit test locks in the empty/falsy apiKey anti-bypass invariant (that no bare Bearer token is ever emitted) — the guard at opampController.ts:181 is only exercised indirectly.
  • No end-to-end coverage of the gRPC receiver auth path; smoke tests exercise only the HTTP receiver on :4318, though gRPC is configured with the same authenticator.
  • No smoke assertion that a token-less Bearer (scheme with empty token) is rejected.

…5198)

The bearertokenauth extension matches the full Authorization header
value exactly, and both ingest auth surfaces (OpAMP-managed config and
the standalone OTLP_AUTH_TOKEN config) listed only the bare token, so
RFC 6750 clients that send 'Authorization: Bearer <token>' were
rejected. Accept the Bearer, bearer, and BEARER prefixed forms
alongside the bare token, and cover both paths with unit tests plus an
end-to-end bearer-auth smoke suite.
@kodiakhq
kodiakhq Bot merged commit 808b345 into main Aug 31, 2026
27 checks passed
@kodiakhq
kodiakhq Bot deleted the warren/HDX-5198-accept-bearer-prefixed-auth-header branch August 31, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-generated AI-generated content; review carefully before merging. automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants