Skip to content

webhooks: Bound delivery admission and per-repository hook count #436

Description

@euxaristia

Summary

Webhook delivery lacks bounded admission and a per-repository hook quota. The production HTTP client has a 10-second request timeout, but that does not bound the number of queued or concurrently executing deliveries or the payload memory retained by those tasks.

Source reviewed at ebafeae909364359466a8438a7cc9acec9970a2c:

  • crates/gitlawb-node/src/api/webhooks.rs:30: create_webhook() correctly checks repository ownership and validates the target hostname, but does not enforce a hook-count quota.
  • crates/gitlawb-node/src/db/mod.rs:2319: webhook creation inserts the row without a quota check; listing loads all repository hooks before event filtering.
  • crates/gitlawb-node/src/webhooks.rs:36: fire_event() starts a detached task per event.
  • crates/gitlawb-node/src/webhooks.rs:53: fire_event_async() clones the serialized payload and starts a detached task for each matching hook, with no bounded queue or concurrency admission.
  • crates/gitlawb-node/src/main.rs:1269: build_http_client() sets the shared 10-second timeout and disables redirects.

Impact and limits

Accumulated hooks and overlapping repository events can retain increasing payload copies, delivery tasks, and outbound requests, creating availability pressure on a shared node. Hook creation requires an authenticated repository owner. The timeout limits individual request duration, not total admitted work.

The missing bounds are source-confirmed. No load test, exhaustion threshold, node crash, or production impact was measured. Provisional severity: Medium, subject to operational limits and maintainer assessment.

Relationship to existing reports

#81 and #340 address outbound destination validation and SSRF. This report concerns delivery resource admission, independently of destination validation. A timeout alone does not resolve this gap.

Remediation and acceptance criteria

  • Enforce an atomic per-repository hook quota so concurrent creations cannot exceed it.
  • Use a bounded delivery queue with global and per-repository concurrency limits and an explicit overload policy.
  • Apply admission before creating per-delivery tasks or cloning payloads; a semaphore inside an unlimited number of spawned tasks would leave task retention unbounded.
  • Retain the existing request timeout and destination protections, and expose dropped/deferred delivery metrics.
  • Add deterministic tests using a controlled delivery implementation to assert bounded active work and queue size, quota enforcement under concurrent creation, and recovery after failed deliveries.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:nodegitlawb-node — the serving node and REST APIkind:securityVulnerability fix or hardeningsev:mediumDegraded but workaround existssubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions