Skip to content

fix(proxy): reject declared-empty OTLP bodies before auth#129

Merged
arseniycodes merged 2 commits into
mainfrom
ash/proxy-reject-empty-body
Jul 1, 2026
Merged

fix(proxy): reject declared-empty OTLP bodies before auth#129
arseniycodes merged 2 commits into
mainfrom
ash/proxy-reject-empty-body

Conversation

@arseniycodes

@arseniycodes arseniycodes commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

An OTLP HTTP export with a zero-length body carries no records. The proxy
already returns a permanent 400 for it — but only after the auth middleware
hashes the API key, reads the api_keys row, and writes last_used_at. That is
a DB read and write on every request, before the body is even inspected.

A client looping empty exports therefore drives real per-request work at high
volume. Enough of it saturates the shared ingest fleet's CPU until health
checks start failing; with no healthy targets the load balancer returns 5xx
to every tenant, not just the misbehaving one. We saw exactly this shape:
one project sent millions of empty-body requests in a tight loop and tipped the
whole ingest path over.

Change

  • New empty-body-guard.ts: isDeclaredEmptyBody(contentLength) — true only
    when Content-Length is present and provably zero. Absent / non-numeric
    values fall through to the normal path (we only fast-reject a body we can
    cheaply prove is empty).
  • A /v1/* middleware registered before auth returns the same 400
    (shared EMPTY_BODY_ERROR_MESSAGE) for a declared-empty POST, before any
    key hashing or DB access. Registered after cors() so the 400 keeps its
    CORS headers, and cors() still answers OPTIONS preflights itself.
  • New superlog.proxy.ingest.empty_body_rejected counter (by signal) so the
    rejected rate is visible.

Only Content-Length: 0 is fast-rejected. A chunked request with no declared
length that turns out empty still gets a 400 via the existing body-capture
EmptyBodyError path — just not cheaply. Real OTLP exporters set
Content-Length, so that gap does not matter in practice.

Test

TDD: empty-body-guard.test.ts covers zero / non-zero / absent / whitespace /
malformed. Full proxy suite green (84 tests), typecheck clean.


Note

Medium Risk
Touches the shared ingest hot path and middleware order on /v1/*; behavior change is narrow (only declared-empty POSTs) but incorrect header parsing could wrongly pass or reject requests.

Overview
Adds a pre-auth fast path on /v1/* so POST requests with a provably zero Content-Length get the same permanent 400 as before, but without API key hashing, api_keys lookup, or last_used_at updates—reducing CPU/DB load when clients loop empty OTLP exports.

New empty-body-guard.ts exposes isDeclaredEmptyBody() (regex ^\s*0+\s*$ so values like 0.5/0x0 are not misclassified) and shared EMPTY_BODY_ERROR_MESSAGE; the later EmptyBodyError handler now uses that constant. Chunked or unknown-length empty bodies still hit the existing body-capture path.

Instrumentation: superlog.proxy.ingest.empty_body_rejected counter by OTLP signal (no tenant—rejection happens before key resolution). Middleware runs after CORS so 400s keep CORS headers. Unit tests cover zero, non-zero, absent, whitespace, and malformed Content-Length.

Reviewed by Cursor Bugbot for commit fd1b305. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Reject OTLP requests that declare an empty body (Content-Length: 0) before auth to avoid DB work and protect ingest CPU from empty-export loops. Keeps the same 400 and now skips test keys so smoke probes still return 200.

  • Bug Fixes
    • Pre-auth middleware on /v1/* 400s POSTs with Content-Length: 0, runs after cors, skipping key hash and DB; chunked or unknown-length bodies still error later.
    • Exempt test keys (SUPERLOG_TEST/superlog_test_*) via shared isTestKey() so smoke probes still 200; auth uses the same helper.
    • Shared EMPTY_BODY_ERROR_MESSAGE across all empty-body paths and new isDeclaredEmptyBody(); added superlog.proxy.ingest.empty_body_rejected counter and tests for header parsing.

Written for commit 4beb69c. Summary will update on new commits.

Review in cubic

A Content-Length: 0 OTLP export carries no records and is already a permanent
400, but only after the auth middleware hashes the key, reads the api_keys row,
and writes last_used_at — a DB read and write on every request. A client looping
empty exports can therefore drive real per-request work at high volume, saturate
the shared ingest fleet's CPU until health checks fail, and cause the load
balancer to return 5xx to every tenant.

Add a pre-auth guard on /v1/* that returns the same 400 the body-capture path
does, before any key hashing or DB access, for requests that declare a
zero-length body. Only Content-Length: 0 is fast-rejected; a chunked empty body
still falls through to the existing EmptyBodyError path. Registered after cors()
so the 400 keeps its CORS headers. New superlog.proxy.ingest.empty_body_rejected
counter makes the rejected rate visible per signal.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fd1b305. Configure here.

Comment thread apps/proxy/src/index.ts
Comment thread apps/proxy/src/index.ts
…0 message

Address review feedback on the pre-auth empty-body guard:

- Test-key smoke probes regressed: the guard 400'd a declared-empty POST before
  the auth middleware's SUPERLOG_TEST / superlog_test_* short-circuit could
  answer it with a 200, and the outcome depended on whether the probe sent a
  Content-Length. Extract a shared isTestKey() helper (used by both the guard
  and auth so they can't drift) and skip the fast-reject for test keys — the
  check is a cheap header/prefix read, no DB, so it doesn't reopen the flood
  cost. Empty test probes now behave the same regardless of framing.
- The no-body path in forward() still hardcoded the empty-body 400 string;
  point it at the shared EMPTY_BODY_ERROR_MESSAGE so all three empty-body
  responses stay identical.
@arseniycodes

Copy link
Copy Markdown
Contributor Author

Both Bugbot findings addressed in 4beb69c:

  1. Test-key short-circuit — extracted a shared isTestKey() (used by the guard and the auth check so they can't drift) and exempted test keys from the pre-auth fast-reject. Empty SUPERLOG_TEST / superlog_test_* probes reach the 200 short-circuit again, and behave the same whether or not they declare a Content-Length. The check is a header/prefix read (no DB), so it doesn't reopen the flood cost.
  2. Shared message — the no-body path in forward() now uses EMPTY_BODY_ERROR_MESSAGE too; all three empty-body 400s return an identical body.

@arseniycodes arseniycodes merged commit bc11ffe into main Jul 1, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant