Skip to content

feat(ingress): bounded webhook verifier pool and pre-authentication budgets - #79

Open
iperev wants to merge 1 commit into
feat/adaptation-recommendationsfrom
feat/bounded-ingress-pool
Open

iperev wants to merge 1 commit into
feat/adaptation-recommendationsfrom
feat/bounded-ingress-pool

Conversation

@iperev

@iperev iperev commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Stacked PR 24 — base: #78 (feat/adaptation-recommendations). Do not merge yet. Review only the top commit.

Context

A single process-wide lock let only one webhook be verified at a time. Concurrent deliveries got 503 and were lost unless REST recovery or an operator restored them (T10). Forged and valid back-channel logouts shared one bucket of 16 burst and 4 per second (F-06). This implements the reviewed design webhook-ingress-scheduling.md with no migration.

What changed

  • Verifier pool: a no-queue pool of 4 permits per admission instance replaces the REQ-CI-RUNTIME-008 single-worker clause.
    • Each permit is held from before verification until the commit result, and is released on cancellation or deadline.
    • Saturation and a broken worker keep the existing 503.
    • Composition rejects fewer than 2 or more than 8 permits.
  • Checks before the body is read: after the declared-length checks and before the path lease or body read, the webhook route checks:
    • content type and encoding (400);
    • exactly one canonical signature header (401);
    • a request bucket (200 burst, 50/s) and a declared-byte bucket (32 MiB, 8 MiB/s). A request with no declared length is charged 25 MiB.
    • Refusal returns the existing 503 "webhook ingestion overloaded". The token bucket is extracted from public_request_limits and reused.
  • Webhook deadline: 8 s, or the global deadline if that is shorter.
  • Back-channel logout budget: 64 burst, 32/s. Login start and callback keep 16/4. Key lookup still happens after the header-shape checks.
  • Recheck rotation: recent and repair recheck claims visit due repository scopes in key order after one in-memory cursor per lane. Order within a scope is unchanged.
  • Metrics: three new fixed-label series: ci_coordinator_webhook_admission_total{outcome}, ci_coordinator_public_request_limited_total{route_class}, and a permit-hold histogram (REQ-CI-RUNTIME-015).
  • Ingestion contract: names who recovers each refused delivery.
  • Records: requirements, Proofkit routes and bindings, owner docs and the ROADMAP CI-014 row are updated; the row stays open.

Expected result

  • Up to 4 concurrent valid deliveries are claimed; the 5th gets the existing 503.
  • Requests with invalid headers never read the body or use a token.
  • Forged logouts within the budget do not block valid ones.
  • One repository's backlog cannot hold up other scopes in its recheck lane.

Provisional owner decisions (2026-10-11):

  • Design decisions 1–5.
  • The startup connection-share check is not adopted. The background share is 22, so the required pool would be 30 against 16. Only the counted inventory (runtime/connection_shares.py) and its test ship; foreground contention stays a recorded residual for CI-069.

Non-claims: no capacity, cross-replica abuse control, valid-delivery capacity beyond the declared budgets, provider behaviour or deployment readiness. All budgets are declared and unmeasured (CI-069 measures them). Native unit, PostgreSQL and mutation tests run only in CI, at the exact head. CI-014 stays open.

Independent review: gpt-6-astra (xhigh); findings addressed or rejected with reasons in the single commit. Provisional owner decisions (2026-10-11) are labelled in the changed documents.

…udgets

One process-global lock admitted a single webhook verification at a time, so
every concurrent delivery received 503 and stayed lost unless REST recovery or
an operator restored it (T10). The back-channel logout bucket was shared by
forged and valid tokens at 16/4 (F-06). This delivers the reviewed CI-014 design
(webhook-ingress-scheduling.md) without a migration.

- Verifier pool: each ProcessWebhookAdmission owns NoQueueAdmission(4) and a
  matching CapacityLimiter; the permit is held from before verification until
  the commit result and released on cancellation or deadline. Saturation and a
  broken worker keep the existing 503 "webhook ingestion unavailable";
  composition rejects N < 2 or N > 8. REQ-CI-RUNTIME-008 successor of the
  single-worker clause.
- Webhook pre-body stage (generic BodyLimitPolicy.pre_body_admission hook): after
  the declared-length checks and before the path lease or body read it decides
  content type/encoding (400) and exactly one canonical X-Hub-Signature-256
  (401), then charges request (200, 50/s) and declared-byte (32 MiB, 8 MiB/s)
  buckets together; unknown length is charged 25 MiB; refusal is the existing
  503 "webhook ingestion overloaded". The token bucket is extracted from
  public_request_limits and reused. Precedence changes only for multi-defect
  requests, as admitted by the design.
- Webhook route deadline policy of 8 s (min with the global deadline).
- Back-channel logout envelope burst 64, refill 32/s; login start/callback keep
  16/4. Key lookup stays after the header-shape checks (pinned by witnesses).
- Recent/repair recheck claims rotate due scopes in key order after one
  process cursor per lane; in-scope row order is unchanged.
- Closed series ci_coordinator_webhook_admission_total{outcome},
  ci_coordinator_public_request_limited_total{route_class} and a permit hold
  histogram (REQ-CI-RUNTIME-015).
- Ingestion contract names the recovery owner of refused deliveries.

Provisional owner decision (2026-10-11): the source-counted background
connection share is B = 22, so N + B + F_min = 30 exceeds the declared pool of
16. Plan step 6's startup rejection is not adopted. No new pool or engine is
added. runtime/connection_shares.py records the inventory, a witness pins it,
and foreground contention stays a recorded residual for CI-069.

All budgets remain declared and unmeasured. Native unit and PostgreSQL
witnesses are unexecuted locally and run only in GitHub CI. No capacity,
provider or deployment readiness is claimed. CI-014 stays open.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant