fix(orb): authenticate fleet telemetry ingest - #1285
Conversation
d764889 to
696cdf7
Compare
| // collector REQUIRES an exact bearer match, so the write path can be locked down after the matching | ||
| // ORB_COLLECTOR_TOKEN is rolled out to exporters. | ||
| function isAuthorizedOrbIngest(env: Env, token: string | undefined): boolean { | ||
| if (!env.ORB_INGEST_TOKEN) return true; |
There was a problem hiding this comment.
P1: Authentication gate fails open when ORB_INGEST_TOKEN is unset
When ORB_INGEST_TOKEN is unset, the ingest endpoint remains unauthenticated, so the vulnerability is not fixed by default.
Make the auth gate fail-closed or log a loud startup warning when the token is missing.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="src/api/routes.ts">
<violation number="1" location="src/api/routes.ts:4955">
<priority>P1</priority>
<title>Authentication gate fails open when ORB_INGEST_TOKEN is unset</title>
<evidence>The isAuthorizedOrbIngest function returns true when ORB_INGEST_TOKEN is not configured: `if (!env.ORB_INGEST_TOKEN) return true;`. This means the /v1/orb/ingest endpoint remains unauthenticated by default, contradicting the PR's stated goal of preventing unauthenticated callers from poisoning fleet analytics.</evidence>
<recommendation>Either make the auth gate fail-closed (require a token to be set before accepting requests) or add a loud, unavoidable startup warning when ORB_INGEST_TOKEN is missing so operators are aware the endpoint is still open.</recommendation>
</violation>
</file>
| // ORB_COLLECTOR_TOKEN is rolled out to exporters. | ||
| function isAuthorizedOrbIngest(env: Env, token: string | undefined): boolean { | ||
| if (!env.ORB_INGEST_TOKEN) return true; | ||
| return token === env.ORB_INGEST_TOKEN; |
There was a problem hiding this comment.
P2: Bearer token comparison uses simple equality vulnerable to timing attacks
The === comparison in isAuthorizedOrbIngest short-circuits on the first mismatched character, leaking timing information.
Use crypto.subtle.timingSafeEqual or a constant-time comparison to prevent timing attacks.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="src/api/routes.ts">
<violation number="1" location="src/api/routes.ts:4956">
<priority>P2</priority>
<title>Bearer token comparison uses simple equality vulnerable to timing attacks</title>
<evidence>The isAuthorizedOrbIngest function compares the bearer token with `return token === env.ORB_INGEST_TOKEN;`. Standard string equality in JavaScript short-circuits on the first mismatched character, making the comparison vulnerable to timing attacks that could allow an attacker to recover the token byte-by-byte.</evidence>
<recommendation>Replace the simple equality comparison with a timing-safe comparison such as `crypto.subtle.timingSafeEqual` (available in Cloudflare Workers) or a constant-time comparison function. Ensure the comparison handles strings of different lengths safely without leaking timing information.</recommendation>
</violation>
</file>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1285 +/- ##
=======================================
Coverage 95.37% 95.37%
=======================================
Files 192 192
Lines 20866 20874 +8
Branches 7546 7549 +3
=======================================
+ Hits 19901 19909 +8
Misses 383 383
Partials 582 582
🚀 New features to boost your workflow:
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Motivation
/v1/orb/ingest. Add an optional shared-token gate so an operator can lock the write path down, plus per-field size caps so oversized identifiers can't bloat analytics.Description (reworked for the LIVE fleet)
The Orb ingest endpoint is intentionally open by design on
main(the fleet topology has no per-instance key) and already has a hard body-size ceiling. The original codex version madeORB_INGEST_TOKENmandatory — which would have 401'd the entire live fleet on deploy. This revision makes the token optional / fail-open:isAuthorizedOrbIngest: unset → OPEN (matches today; deploying is non-breaking, the fleet keeps working). Set → require an exact bearer match, so an operator locks the write path down after rolling the matchingORB_COLLECTOR_TOKENout to exporters. (src/api/routes.ts,src/env.d.ts)instance_id/repo_hash/pr_hash/gate_reasoncode_bucketreject oversized values. (src/orb/ingest.ts)Authorization: Bearer ${ORB_COLLECTOR_TOKEN}only when that token is configured. (src/selfhost/orb-collector.ts)main's body-size ceiling (413) and reverted the default test-env token (enforcement is opt-in); added a focused test for the unset-open / set-enforced matrix.Rollout: deploy (non-breaking) → distribute
ORB_COLLECTOR_TOKENto exporters → setORB_INGEST_TOKENon the collector to switch enforcement on.Testing
npm run test:ci— green;npm audit --audit-level=moderate— 0 vulnerabilities.npm run test:coverage— every changed line and branch covered (the fail-open / enforced matrix, the size caps, and the conditional exporter header).Codex Task — reworked by maintainer for live-fleet safety (opt-in instead of mandatory).