Skip to content

feat(ingest): enforce custom ingest keys in the gateway - #1519

Closed
Makisuo wants to merge 20 commits into
feat/ingest-keys-custom-apifrom
feat/ingest-keys-custom-gateway
Closed

Makisuo wants to merge 20 commits into
feat/ingest-keys-custom-apifrom
feat/ingest-keys-custom-gateway

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Part 2 of 3: the ingest gateway enforces custom ingest keys (maple_ik_). Base: #1518.

Behavior

  • maple_ik_ keys resolve only from org_ingest_custom_keys (same 60s cache as default keys). Default keys are unchanged.
  • Checks run in this order on HTTP OTLP, gRPC and the replay/session/events endpoints:
    1. An expired key gets 401.
    2. A signal outside the allow-list gets 403, or gRPC PermissionDenied.
    3. A paused key gets 200 (gRPC OK) and its data is dropped: not forwarded, not stored, not billed. A timed pause ends at paused_until without waiting for a cache refresh.
    4. Over the rate limit gets 429 with Retry-After, or gRPC ResourceExhausted. The per-key token bucket is ceil(rps / INGEST_RATE_LIMIT_FLEET_SIZE) per task.
  • Once a payload is enqueued or forwarded, the key's last_used_at is written on a spawned task, at most once a minute per key and instance. A key whose requests all fail never reads as live.
  • Resource attribute overrides run before the maple_* stamps and can never write org_id/maple_*. Telemetry gets maple_ingest_key_id = ingk_…, and a sender cannot spoof it.

Verified

  • cargo test: 245 + 123 tests pass. New tests cover:
    • pause boundaries;
    • a paused key answered with success and nothing recorded;
    • an expired key refused on the OTLP and replay paths;
    • use recorded once per interval;
    • the end-to-end overrides, 403 and 429 tests from before.
  • Local end-to-end: the gateway against dev Postgres, forwarding to a capture sink, with a key from the UI:
traces (active):   200   forwarded, last_used_at written
metrics:           403   (not allowed)
paused, after the 60s cache: traces 200, nothing forwarded
resumed, after the cache:    traces 200, forwarded, last_used_at advanced

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

maple_ik_ keys resolve from org_ingest_custom_keys. After auth and before
decode the gateway rejects expired keys (401), signals outside the key's
allow-list (403) and requests over the key's rate limit (429, Retry-After).
The limit is split across INGEST_RATE_LIMIT_FLEET_SIZE tasks. Resource
attribute overrides apply before the maple_* stamps, and telemetry is
stamped with maple_ingest_key_id.
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8bf14249-170a-439d-8a86-4e4f489f80e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 7/10 · likely safe to merge
Auth, tenancy stamping and the gRPC/expiry units are pinned by tests and code I read; the untested part is which signal a session-events batch is charged to.
quality 90/100 · 1 warning · tests covered · risk high · 4/4 new units observable

The gateway now resolves maple_ik_ custom keys from org_ingest_custom_keys and enforces expiry, signal allow-list and a per-key token bucket on the HTTP, gRPC and replay paths, plus protected resource-attribute overrides. The mechanics are sound and well tested; the allow-list split leaves one gap.

  • resolve_ingest_key reads maple_ik_ keys from org_ingest_custom_keys with the 60s positive cache
  • authorize_custom_key rejects expired (401), disallowed signal (403) and over-limit (429) requests before decode
  • apply_resource_attribute_overrides runs before the maple_* stamps and refuses org_id/maple_*
  • Per-key token bucket sized ceil(rps / INGEST_RATE_LIMIT_FLEET_SIZE) per task

Before merge

Findings

🟠 Warning · F1 · events allow-list does not gate browser product events

correctness · apps/ingest/src/main.rs:3769

/v1/sessionEvents is authorized as KeySignal::Sessions, but a browser track() call on that endpoint is a type == "custom" row that the gateway meters as product_events (main.rs:1119), i.e. the same feature /v1/events gates. A key created with allowed_signals: ["sessions"] therefore keeps sending product events with no 403, while a key scoped to ["events"] cannot send them at all — the customer's product-event restriction is unenforced in the direction it matters.

Either classify `type == "custom"` rows on `/v1/sessionEvents` as `KeySignal::Events` and reject/drop just those rows, or state in the API's `allowed_signals` description that browser product events ride the `sessions` signal.
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit b6b278f759f90042c6464d04c50891d5d45b9232. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Warning · correctness · apps/ingest/src/main.rs:3769
`events` allow-list does not gate browser product events
`/v1/sessionEvents` is authorized as `KeySignal::Sessions`, but a browser `track()` call on that endpoint is a `type == "custom"` row that the gateway meters as `product_events` (main.rs:1119), i.e. the same feature `/v1/events` gates. A key created with `allowed_signals: ["sessions"]` therefore keeps sending product events with no 403, while a key scoped to `["events"]` cannot send them at all — the customer's product-event restriction is unenforced in the direction it matters.
Suggested fix: Either classify `type == "custom"` rows on `/v1/sessionEvents` as `KeySignal::Events` and reject/drop just those rows, or state in the API's `allowed_signals` description that browser product events ride the `sessions` signal.

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 228k POST /v1/logs 130k/day, 0.0% err
POST /v1/logs 66k/day, 0.0% err
POST /v1/sessionEvents 32k/day, 0.0% err
apps/ingest/src/custom_keys.rs 32k POST /v1/sessionEvents 32k/day, 0.0% err
POST /v1/events 14/day, 0.0% err
POST /v1/events 10/day, 0.0% err
What was checked
  • Expiry units: the store selects EXTRACT(EPOCH FROM expires_at) * 1000 (main.rs:7046), matching is_expired's ms comparison
  • Tenancy: overrides drop org_id/maple_* and enrich_resource_attributes retains those keys out before stamping (main.rs:5869)
  • gRPC admission rejects before the service runs, mapping 401/403/429 to Unauthenticated/PermissionDenied/ResourceExhausted (main.rs:2947)
Observability coverage: 4 of 4 changes observable
Change Kind Observable Evidence
Custom-key policy check (expiry, allow-list, rate limit) on HTTP OTLP, replay/session/events endpoints request handler yes existing server spans record error.type/maple.ingest.reject_reason; maple.ingest.custom_key_id recorded at main.rs:4644 and declared at main.rs:4432
Custom-key policy check in the gRPC admission middleware rpc middleware yes grpc_server_span + record_grpc_identity/record_grpc_outcome (main.rs:2910-2924, 2003-2005)
Postgres read of org_ingest_custom_keys db query yes postgres_client_span "fetch_custom_ingest_key" (main.rs:7056)
Resource-attribute overrides applied to the payload enrichment yes resulting attributes ride the exported resource (main.rs:5875-5888), not a separate span

b6b278f · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot maple-review-bot 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.

1 inline note from Maple's review. The score and summary are in the review comment above.

Comment thread apps/ingest/src/main.rs Outdated
A /v1/sessionEvents batch is admitted when the custom key allows sessions
or events; custom (track) rows are kept only with events, the rest only
with sessions, and dropped rows are counted on the span.
@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
The delta since the last pass is the row-level gating fix plus an any-of signal refactor, both pinned by a new end-to-end test.
quality 100/100 · no findings · tests covered · risk medium · 5/5 new units observable

Fixes the previously flagged hole: /v1/sessionEvents now admits a key that allows sessions or events and gates each type == "custom" row on the events signal; the rest is an any-of refactor of the check. Safe to merge.

  • authorize_custom_key now takes an any-of signal slice instead of one signal
  • /v1/sessionEvents admits sessions or events and gates each row by kind
  • Denied session-event rows are counted in maple.session_events.denied

Before merge

Fixed since the last review

  • ✅ F1 · events allow-list does not gate browser product events

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 229k POST /v1/logs 131k/day, 0.0% err
POST /v1/logs 66k/day, 0.0% err
POST /v1/sessionEvents 32k/day, 0.0% err
apps/ingest/src/custom_keys.rs 32k POST /v1/sessionEvents 32k/day, 0.0% err
POST /v1/events 14/day, 0.0% err
POST /v1/events 10/day, 0.0% err
What was checked
  • F1 is fixed: custom rows require KeySignal::Events and other rows KeySignal::Sessions (main.rs:3855), with session_event_rows_follow_the_custom_key_signals asserting 1/2/403 for sessions-, event…
  • Default maple_pk_/maple_sk_ keys are unaffected: allows is true whenever custom is None (main.rs:3783)
  • A maple_ik_ key never falls through to the default key table (infer_ingest_key_type returns Custom before the resolver match, main.rs:6314)
Observability coverage: 5 of 5 changes observable
Change Kind Observable Evidence
Custom-key authorization on HTTP OTLP signals (handle_signal_inner) inbound HTTP yes Existing server span records maple.ingest.custom_key_id (main.rs:4669) and the rejection maps to a tagged FailureKind.
Custom-key authorization on gRPC OTLP export (admit_grpc_export) inbound gRPC yes grpc server span plus record_grpc_identity records the custom key id (main.rs:1988, 2935).
Custom-key authorization on replay/session/events endpoints inbound HTTP yes resolve_replay_key authorizes inside each handler's server span; per-row denials recorded as maple.session_events.denied (main.rs:3874).
Postgres read of org_ingest_custom_keys database query yes postgres_client_span("fetch_custom_ingest_key", "SELECT", "org_ingest_custom_keys") at main.rs:7083.
Per-key rate limit throttle inbound HTTP yes metrics::org_throttled(&org_id, "key_rate_limit") after a refused token (main.rs:3492).

d0c24b1 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Auth enforcement on the hot OTLP routes is pinned by end-to-end tests; the conditionally applied maple_ingest_key_id stamp is the one gap.
quality 98/100 · 1 note · tests covered · risk high · 2/2 new units observable

The gateway now enforces custom maple_ik_ keys — expiry (401), signal allow-lists (403) and a per-key token bucket (429) on HTTP OTLP, gRPC and the session/event/replay routes — and applies their resource-attribute overrides. Safe to merge; one hardening gap in the new key-id stamp.

  • maple_ik_ keys resolve only from org_ingest_custom_keys, never the default key table
  • authorize_custom_key checks expiry, allow-list and rate limit before the body is decoded, on every transport
  • Overrides apply before the maple_* stamps and can never write org_id/maple_*
  • INGEST_RATE_LIMIT_FLEET_SIZE splits each key's rps across the fleet

Before merge

Findings

🔵 Note · F2 · A default key can spoof maple_ingest_key_id on forwarded telemetry

correctness · apps/ingest/src/main.rs:5913-5915

This stamp is written only for custom keys, and the retain above strips only org_id and maple_org_id, so a maple_pk_/maple_sk_ sender's own maple_ingest_key_id resource attribute reaches the warehouse unchanged — the attribute the API documents as identifying the key that sent the telemetry. Strip it with the other gateway-owned keys.

Add `&& key != "maple_ingest_key_id"` to the `retain` closure in `enrich_resource_attributes` (line 5898), so a default key's sender-supplied value is dropped like `maple_org_id` is.
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit 9227fe5902e2357b8f24c91e71f10d45572b5aaa. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F2 · Note · correctness · apps/ingest/src/main.rs:5913-5915
A default key can spoof `maple_ingest_key_id` on forwarded telemetry
This stamp is written only for custom keys, and the `retain` above strips only `org_id` and `maple_org_id`, so a `maple_pk_`/`maple_sk_` sender's own `maple_ingest_key_id` resource attribute reaches the warehouse unchanged — the attribute the API documents as identifying the key that sent the telemetry. Strip it with the other gateway-owned keys.
Suggested fix: Add `&& key != "maple_ingest_key_id"` to the `retain` closure in `enrich_resource_attributes` (line 5898), so a default key's sender-supplied value is dropped like `maple_org_id` is.

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 229k POST /v1/logs 131k/day, 0.0% err
POST /v1/logs 66k/day, 0.0% err
POST /v1/sessionEvents 32k/day, 0.0% err
apps/ingest/src/custom_keys.rs 32k POST /v1/sessionEvents 32k/day, 0.0% err
POST /v1/events 14/day, 0.0% err
POST /v1/events 10/day, 0.0% err
What was checked
  • The new org_ingest_custom_keys lookup is a Client span with db.system/peer.service (apps/ingest/src/main.rs:7083)
  • Fleet split uses the scaling floor (prd min 2 vs ASG max 12, apps/ingest/alchemy.run.ts:621), so a scaled-out fleet admits above the configured rps — deliberate per the code comment
  • Reserved keys are guarded twice: the API rejects maple_*/org_id case-insensitively, and custom_keys::is_protected_attribute re-checks at the gateway
Observability coverage: 2 of 2 changes observable
Change Kind Observable Evidence
Postgres lookup of org_ingest_custom_keys per uncached custom key client DB query yes postgres_client_span("fetch_custom_ingest_key", "SELECT", "org_ingest_custom_keys") at apps/ingest/src/main.rs:7083
Expiry / allow-list / rate-limit enforcement for custom keys on HTTP, gRPC, replay and session/event routes inbound enforcement on existing Server spans yes custom key id recorded on ingest.authenticate (main.rs:4670) and gRPC spans (main.rs:1989); metrics::org_throttled(org, "key_rate_limit") at main.rs:3492

9227fe5 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

The key id is gateway-owned: drop it with org_id and maple_org_id so a
default-key sender cannot spoof which custom key sent its telemetry.
@Makisuo

Makisuo commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Re F2: fixed in e9dca51. enrich_resource_attributes now drops a sender-supplied maple_ingest_key_id along with org_id/maple_org_id, so only the gateway's own stamp (custom keys) survives. default_keys_get_no_key_id_stamp_or_overrides now sends a spoofed maple_ingest_key_id and asserts it is gone. Before-merge items: INGEST_RATE_LIMIT_FLEET_SIZE is set by alchemy for every stage (scaling.min ?? desiredCount, all ≥1) and defaults to 1; no other infra changes; merge order is #1518 → #1519 → #1520.

@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
The only delta since 9227fe5 is the six-line retain fix in enrich_resource_attributes, which I read and traced end to end.
quality 100/100 · no findings · tests covered · risk medium

This head only changes enrich_resource_attributes to drop a sender-supplied maple_ingest_key_id before the gateway stamps its own; safe to merge.

  • enrich_resource_attributes strips an incoming maple_ingest_key_id for every key type
  • default_keys_get_no_key_id_stamp_or_overrides now sends a spoofed key id and asserts it is gone

Before merge

Fixed since the last review

  • ✅ F2 · A default key can spoof maple_ingest_key_id on forwarded telemetry

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 229k POST /v1/logs 131k/day, 0.0% err
POST /v1/logs 66k/day, 0.0% err
POST /v1/sessionEvents 32k/day, 0.0% err
apps/ingest/src/custom_keys.rs 32k POST /v1/sessionEvents 32k/day, 0.0% err
POST /v1/events 14/day, 0.0% err
POST /v1/events 10/day, 0.0% err
What was checked
  • maple_ingest_key_id is emitted only by the gateway's own stamp after the retain (main.rs:5914)
  • The retain runs before overrides and stamps, so a custom key's stamp still wins (main.rs:5896-5915)

e9dca51 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo

Makisuo commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Preview check of F2 on 3f696ba (pr-1520): a span sent with the org's default public key and a spoofed maple_ingest_key_id=ingk_spoofed lands with resource attributes service.name=qa-spoof, maple_ingest_key_type=public, maple_ingest_source, maple_org_id and no maple_ingest_key_id. The spoofed value is dropped.

… use

A paused custom key is admitted after the expiry and signal checks but its
data is dropped: OTLP gets 200, gRPC gets OK and the replay and session
endpoints return early, so senders do not retry and nothing is billed.
Timed pauses end at paused_until without a cache refresh.

Admitted keys stamp last_used_at at most once a minute per key and
instance, the same throttle the connector health writes use.
…path

last_used_at is now stamped after a payload is enqueued or forwarded, on a
spawned task, so a key whose requests all fail (402, 413, 429) no longer
reads as live and the write never holds up a request. gRPC carries the
admission decision to the export instead of re-reading the clock, and a
paused key's 0-byte requests no longer reach billing.
@maple-review-bot

maple-review-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Rate-limit math, override protection and the base58 id vector are pinned by tests I re-derived; the paused-key drop being unobservable in production is the one gap.
quality 90/100 · 1 warning · tests covered · risk medium

The ingest gateway now enforces custom maple_ik_ keys: expiry, signal allow-lists, pause, a per-key token bucket and resource attribute overrides applied before the Maple stamps. The logic and its tests hold up; the one gap is that a paused key's drop is invisible in production.

  • maple_ik_ keys resolve from org_ingest_custom_keys and run expiry, allow-list, pause and rate-limit checks after auth.
  • A paused key's payload is dropped with a 200 and is neither stored nor billed.
  • Resource attribute overrides apply before the maple_* stamps and cannot write org_id/maple_*.
  • Custom keys stamp maple_ingest_key_id = ingk_… and the key type on the ingest spans.

Before merge

Findings

🟠 Warning · F3 · A paused key's dropped payload leaves no trace in production

observability · MET-02 · apps/ingest/src/main.rs:3529-3531

A paused custom key answers 200 and discards the payload, and nothing in telemetry records it: the only line is this debug!, which the default production filter drops (maple_ingest=warn, main.rs:2105), the span stays Ok with no drop attribute (handle_signal_inner returns at main.rs:4739 with item_count unset), and no counter moves. An operator cannot tell a paused key's traffic apart from a healthy empty export, on paths carrying ~67k OTLP log calls/day. Record the drop where the decision is made.

Add a counter next to the existing ones (e.g. `metrics::org_data_loss`-style or a new `maple.ingest.key_paused.dropped`) and record `Span::current().record("maple.ingest.reject_reason", "paused")` before returning `KeyAdmission::Drop`.
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit 4eae5a51f33ec3e664c1c5c8900c7d0ec3959808. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F3 · Warning · observability · MET-02 · apps/ingest/src/main.rs:3529-3531
A paused key's dropped payload leaves no trace in production
A paused custom key answers 200 and discards the payload, and nothing in telemetry records it: the only line is this `debug!`, which the default production filter drops (`maple_ingest=warn`, main.rs:2105), the span stays `Ok` with no drop attribute (`handle_signal_inner` returns at main.rs:4739 with `item_count` unset), and no counter moves. An operator cannot tell a paused key's traffic apart from a healthy empty export, on paths carrying ~67k OTLP log calls/day. Record the drop where the decision is made.
Suggested fix: Add a counter next to the existing ones (e.g. `metrics::org_data_loss`-style or a new `maple.ingest.key_paused.dropped`) and record `Span::current().record("maple.ingest.reject_reason", "paused")` before returning `KeyAdmission::Drop`.

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 644k ingest.authenticate 411k/day, 0.0% err
POST /v1/logs 134k/day, 0.0% err
POST /v1/logs 67k/day, 0.0% err
apps/ingest/src/custom_keys.rs 31k POST /v1/sessionEvents 31k/day, 0.0% err
POST /v1/events 15/day, 0.0% err
POST /v1/events 9/day, 0.0% err
What was checked
  • per_instance_limit is div_ceil(fleet) floored at 1 and the bucket's refill and limit-change paths are pinned by the two bucket tests (custom_keys.rs:239, 453-493).
  • Overrides drop org_id/maple_* and the insertion happens before the gateway stamps, so a spoofed maple_org_id is replaced (custom_keys.rs:198, main.rs:5982).
  • public_key_id matches the API test vector ingk_aXunrkWtzvXSkw9Z2smcft, and the maple_ik_ + base64url secret alphabet fits the WAF pattern in alchemy.run.ts.

4eae5a5 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one. Check ids refer to Maple's instrumentation audit.

…ches

A throttled key no longer spawns a task, the write runs under a root span
linked to the request so request traces do not wait on it, and a collector
4xx no longer counts as use.

@maple-review-bot maple-review-bot 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.

1 inline note from Maple's review. The score and summary are in the review comment above.

Comment thread apps/ingest/src/main.rs
@maple-review-bot

maple-review-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Maple review

🔴 Confidence 4/10 · risky as written
Ingest-key authorization is enforcement-heavy, the paused-drop path still records nothing, and the new success gate has no test.
quality 88/100 · 1 warning · 1 note · tests partial · risk high · 3/4 new units observable

Enforces maple_ik_ custom keys at the gateway: expiry, signal allow-list, pause-drop, per-key rate limit, plus last_used_at bookkeeping and a schema-scoped pause migration. The new gate and limiter hold up; the paused-drop observability gap flagged earlier is still open.

  • note_custom_key_use records last_used_at only for a 2xx response, in its own root span
  • An early recent_key_uses.contains_key check skips the spawn inside the one-minute window
  • The pause migration scopes its enabled probe to current_schema()

Before merge

Findings

🔵 Note · F4 · The is_success gate on key-use recording is pinned by no test

tests · apps/ingest/src/main.rs:4874-4876

note_custom_key_use now runs only for a 2xx forward, but no test exercises a non-2xx one: the end-to-end test's capturing collector always answers OK and never reads store.custom_key_uses, and the paused-key test asserts use is empty on a path that returns before the forward (main.rs:4770). Dropping the if response.status().is_success() guard would leave the suite green while last_used_at is written for batches the collector refused. Assert custom_key_uses is empty after handle_signal_inner against a collector that answers 500.

Extend `custom_key_end_to_end_overrides_denies_and_rate_limits` (or add one) with a collector route that answers 500 and assert `store.custom_key_uses` stays empty for that request while the accepted request records the key id.
🤖 Prompt to fix this finding with an AI agent
Findings from an automated review of commit 799a2ec63837653d4cd31cd1892b94da77305987. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F4 · Note · tests · apps/ingest/src/main.rs:4874-4876
The `is_success` gate on key-use recording is pinned by no test
`note_custom_key_use` now runs only for a 2xx forward, but no test exercises a non-2xx one: the end-to-end test's capturing collector always answers `OK` and never reads `store.custom_key_uses`, and the paused-key test asserts use is empty on a path that returns before the forward (`main.rs:4770`). Dropping the `if response.status().is_success()` guard would leave the suite green while last_used_at is written for batches the collector refused. Assert `custom_key_uses` is empty after `handle_signal_inner` against a collector that answers 500.
Suggested fix: Extend `custom_key_end_to_end_overrides_denies_and_rate_limits` (or add one) with a collector route that answers 500 and assert `store.custom_key_uses` stays empty for that request while the accepted request records the key id.

Still open from earlier reviews

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 641k ingest.authenticate 409k/day, 0.0% err
POST /v1/logs 133k/day, 0.0% err
POST /v1/logs 67k/day, 0.0% err
apps/ingest/src/custom_keys.rs 31k POST /v1/sessionEvents 31k/day, 0.0% err
POST /v1/events 15/day, 0.0% err
POST /v1/events 9/day, 0.0% err
What was checked
  • A failed forward returns non-2xx (forward_to_collector, main.rs:6108), so the new gate drops exactly the unaccepted batches
  • Per-instance throttling reuses the entry().or_insert().is_fresh() idiom of record_health (main.rs:6722)
  • information_schema scoping matches the public schema every other drizzle statement and the parity harness use
Observability coverage: 3 of 4 changes observable
Change Kind Observable Evidence
HTTP OTLP and gRPC export admission (authorize_custom_key) inbound entrypoint yes existing handle_signal/grpc_server_span record maple.ingest.custom_key_id (main.rs:4523, otel.rs:310)
Custom key lookup fetch_custom_ingest_key (Postgres SELECT) outbound call yes postgres_client_span("fetch_custom_ingest_key", "SELECT", "org_ingest_custom_keys", …)
record_custom_key_used (Postgres UPDATE on a spawned task) background work yes root span record_custom_key_use linking the request, wrapping postgres_client_span(… "UPDATE" …) (main.rs:6368)
Paused key's dropped payload outcome of an inbound entrypoint no only a debug! (main.rs:3530) under the production maple_ingest=warn filter; span stays Ok with no drop attribute

799a2ec · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

ingest_custom_key_paused_drops_total (org_id, signal) moves on each drop
and the request span records maple.ingest.reject_reason=custom_key_paused,
so a paused key's traffic is visible instead of looking like an empty export.
Pins the 2xx gate on last_used_at: a collector 400 is relayed and records
nothing, and the end-to-end test now asserts an accepted batch records the
key id.
@Makisuo

Makisuo commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

Re the review at 799a2ec:

  • F3 (paused drop leaves no trace): fixed in 4edf2fd, pushed after that commit. A drop increments ingest_custom_key_paused_drops_total (org_id, signal) and records maple.ingest.reject_reason=custom_key_paused on the request span.
  • F4 (2xx gate untested): a_batch_the_collector_refuses_does_not_count_as_key_use sends a batch to a collector answering 400 and asserts no use is recorded. The end-to-end test now asserts an accepted batch records the key id. I removed the gate to check the new test fails, then restored it.
  • INGEST_RATE_LIMIT_FLEET_SIZE: defaults to 1 when unset. The deploy sets it from scaling.min ?? desiredCount in apps/ingest/alchemy.run.ts, so every alchemy environment gets it.

@maple-review-bot

maple-review-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Since 799a2ec only the paused-drop counter, one span attribute and two tests changed; git diff --stat confirms nothing else moved.
quality 100/100 · no findings · tests covered · risk medium · 1/1 new units observable

The delta makes a paused custom key's discarded payload visible: a per-org/signal counter and a reason on the request span, plus tests pinning the pause drop and the 2xx gate on last_used_at. Safe to merge — no accept, deny or drop decision changed.

  • authorize_custom_key counts ingest_custom_key_paused_drops_total by org and signal
  • The request span records maple.ingest.reject_reason=custom_key_paused
  • A refusing-collector test pins the is_success gate on key-use writes

Before merge

Fixed since the last review

  • ✅ F3 · A paused key's dropped payload leaves no trace in production
  • ✅ F4 · The is_success gate on key-use recording is pinned by no test

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 642k ingest.authenticate 410k/day, 0.0% err
POST /v1/logs 134k/day, 0.0% err
POST /v1/logs 67k/day, 0.0% err
apps/ingest/src/custom_keys.rs 31k POST /v1/sessionEvents 31k/day, 0.0% err
POST /v1/events 15/day, 0.0% err
POST /v1/events 9/day, 0.0% err
What was checked
  • maple.ingest.reject_reason is declared on the HTTP ingest span (main.rs:4508) and the gRPC span (otel.rs:301)
  • A paused key still reads as success on every endpoint: sessionEvents and /v1/events return Ok(0) (main.rs:3831, main.rs:4038)
  • The new counter follows the file's ingest_*_total pattern with bounded labels (metrics.rs:142)
Observability coverage: 1 of 1 changes observable
Change Kind Observable Evidence
paused custom-key request drop metric + span attribute yes metrics.rs:142 ingest_custom_key_paused_drops_total (org_id, signal) and main.rs:3532 maple.ingest.reject_reason=custom_key_paused

46a564a · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@maple-review-bot

maple-review-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Nothing in the gateway source changed since the prior review; the one file in the delta, the pause migration, is byte-identical to the base branch.
quality 100/100 · no findings · tests covered · risk high · 2/2 new units observable

No source changed since the last review: git diff 46a564a..head touches only the pause migration, and that file is byte-identical to the base branch's copy, so this PR adds nothing new to review. The custom-key enforcement itself is safe to merge.

Before merge

Production impact

Production traffic of the changed files (last 7 days)
File Calls/day Busiest operations
apps/ingest/src/main.rs 643k ingest.authenticate 410k/day, 0.0% err
POST /v1/logs 134k/day, 0.0% err
POST /v1/logs 67k/day, 0.0% err
apps/ingest/src/custom_keys.rs 31k POST /v1/sessionEvents 31k/day, 0.0% err
POST /v1/events 15/day, 0.0% err
POST /v1/events 9/day, 0.0% err
What was checked
  • git diff 46a564a..b860bc9 --stat shows only the pause migration, and git diff 3f85d04 b860bc9 for that path is empty
  • StaticKeyStore::fetch_custom_ingest_key returns a config-less row, so local mode keeps its existing accept-anything behavior (main.rs:7574)
  • Every replay, session and events path routes through authorize_custom_key, and overrides cannot write org_id/maple_* (main.rs:3495, custom_keys.rs:198)
Observability coverage: 2 of 2 changes observable
Change Kind Observable Evidence
HTTP and gRPC custom-key authorization (expiry, allow-list, pause, rate limit) request gate yes maple.ingest.reject_reason and maple.ingest.custom_key_id span attributes plus ingest_custom_key_paused_drops_total (main.rs:3531, metrics.rs:587)
custom key last_used_at write background write yes record_custom_key_use root span and the record_custom_key_used postgres client span (main.rs:6360, main.rs:7446)

b860bc9 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo

Makisuo commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by the Sources stack (#1535, #1536, #1537, #1538): custom ingest keys became push sources with the same rules, alongside integration sources and one health model.

@Makisuo Makisuo closed this Oct 17, 2026
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