Skip to content

feat(api): add ENABLE_INGESTION_API_KEY to honor INGESTION_API_KEY on the app image - #3633

Open
andrewdunndev wants to merge 1 commit into
ClickHouse:mainfrom
andrewdunndev:ai/app-image-ingestion-key
Open

andrewdunndev wants to merge 1 commit into
ClickHouse:mainfrom
andrewdunndev:ai/app-image-ingestion-key

Conversation

@andrewdunndev

Copy link
Copy Markdown
Contributor

Summary

On the app image, the OpAMP config ignores INGESTION_API_KEY, so an external collector has to authenticate with the team's generated key. That key changes whenever the team is recreated, for example after the database is rebuilt, and the collector is locked out until someone copies the new key from Team Settings into its config.

This adds an opt-in ENABLE_INGESTION_API_KEY=true, in the same style as ENABLE_DATADOG_RECEIVER. With it set, INGESTION_API_KEY is accepted alongside the team key, so a deployment can pin a key that survives a rebuild. Nothing changes when it is unset. I kept the existing all-in-one and dev gate as it is, since its security reasoning isn't written down anywhere. Happy to rename the variable, or to drop the gate instead if you'd prefer that.

How to reproduce

Starting point: the app image (HYPERDX_IMAGE=hyperdx, NODE_ENV=production) with the standalone collector in OpAMP supervisor mode, and collector authentication enforced for the team (the default for a new team). yarn dev does not show the problem, because IS_DEV already honors the key. The new unit tests in opampController.test.ts cover the same three cases.

On main:

  1. Set INGESTION_API_KEY=preset-key on the API and restart it.
  2. Send OTLP to the collector with Authorization: preset-key: it is rejected, and preset-key is missing from the bearertokenauth/hyperdx tokens in the OpAMP config.

On this branch:

  1. Set INGESTION_API_KEY=preset-key and ENABLE_INGESTION_API_KEY=true and restart the API.
  2. The same request is accepted, and the team key still works. With the flag unset, the behaviour matches main.

How to test on Vercel preview

N/A, non-UI change.

References

@andrewdunndev
andrewdunndev requested a review from a team as a code owner November 12, 2026 17:46
@changeset-bot

changeset-bot Bot commented Nov 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6f63812

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/app Patch
@hyperdx/otel-collector 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 Nov 12, 2026

Copy link
Copy Markdown

@andrewdunndev is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

PR Review

✅ No issues found.

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The change is a clean, default-off opt-in (env.ENABLE_INGESTION_API_KEY === 'true') that mirrors the existing ENABLE_DATADOG_RECEIVER pattern, is type-safe, and leaves default app-image behavior byte-for-byte unchanged. One recommended test gap and one style nit below.

🟡 P2 — recommended

  • packages/api/src/opamp/controllers/opampController.ts:425 — when opted in, INGESTION_API_KEY is appended to the shared apiKeys array before the Datadog block, so it silently becomes a valid bearertokenauth/datadog token whenever ENABLE_DATADOG_RECEIVER is also enabled, yet no test exercises this cross-receiver effect.
    • Fix: add a test with ENABLE_INGESTION_API_KEY, ENABLE_DATADOG_RECEIVER, and collectorAuthenticationEnforced all set, asserting the ingestion key appears in extensions['bearertokenauth/datadog'].tokens.
    • testing, correctness, security
🔵 P3 nitpicks (1)
  • agent_docs/architecture.md:108 — the added prose is a single ~195-char line, violating the repo .prettierrc (proseWrap: "always", printWidth: 80) that the rest of the file follows; lint-staged does not run prettier on .md, so it is not auto-corrected on commit.
    • Fix: hard-wrap the line at 80 columns to match the surrounding prose.

Reviewers (6): correctness, security, testing, maintainability, kieran-typescript, project-standards.

Testing gaps:

  • No test for the no-team-keys-but-opted-in path where INGESTION_API_KEY alone makes apiKeys non-empty and flips the apiKeys.length > 0 gate (opampController.ts:348) to attach otlp/hyperdx.
  • No test confirming a non-'true' value (e.g. 'TRUE', '1') is treated as disabled by the strict === 'true' comparison.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow forcing ingestion key

1 participant