Skip to content

fix(gateway/webhook): prevent nil-channel send DoS in RequestHandler#7069

Open
kuntal1461 wants to merge 1 commit into
rudderlabs:masterfrom
kuntal1461:fix/nil-channel-webhook-request-handler
Open

fix(gateway/webhook): prevent nil-channel send DoS in RequestHandler#7069
kuntal1461 wants to merge 1 commit into
rudderlabs:masterfrom
kuntal1461:fix/nil-channel-webhook-request-handler

Conversation

@kuntal1461

Copy link
Copy Markdown

Summary

Fixes #6999 — sending on a nil channel in RequestHandler blocks the HTTP worker goroutine forever, enabling a denial-of-service condition.

Root cause: webhook.requestQ[sourceDefName] returns nil when the source has not been registered (e.g. webhookV2HandlerEnabled=false and no prior Register call). Go's nil channel send blocks indefinitely.

Changes:

  • Nil-channel guard — check requestQ == nil after the map lookup and return HTTP 503 service unavailable immediately instead of blocking.
  • Shutdown-aware enqueue — wrap the channel send in a select with r.Context().Done() (client disconnect / deadline) and webhook.ctx.Done() (server shutdown). The RLock is held across the select to prevent a concurrent Shutdown from closing the channel mid-send.
  • Buffered done channel (size 1) — if RequestHandler exits early due to context cancellation, the batcher goroutine can still complete its send without leaking.
  • Context guard on resp := <-done — also wraps the response-wait in a select with r.Context().Done() so a slow transformer doesn't permanently hold a worker after the client has gone.
  • Store shutdown contextHandleT.ctx added and populated in Setup so RequestHandler can reference it.

Test plan

  • TestWebhookRequestHandlerSourceNotRegistered — V1 mode, no Register call → asserts HTTP 503 returned immediately, no block
  • TestWebhookRequestHandlerRequestContextCancellation — blocking adapter + 100 ms request deadline → asserts handler returns promptly
  • All existing webhook unit tests pass (go test ./gateway/webhook/... -race -count=1)
  • Manual: run with webhookV2HandlerEnabled=false and send a request for an unregistered source — expect immediate 503

When webhookV2HandlerEnabled is false and a source has not been
registered, requestQ lookup returns nil. Sending on a nil channel
blocks forever, permanently consuming the HTTP worker goroutine.

- Guard the channel send with a nil check; return HTTP 503 immediately
  for unregistered sources instead of blocking
- Wrap the channel send in a select with r.Context().Done() and the
  shutdown context (webhook.ctx) so the handler exits promptly when
  the client disconnects or the server shuts down
- Buffer the done channel (size 1) so the batcher goroutine can always
  complete its send even if RequestHandler returned early, preventing
  goroutine leaks
- Also guard resp := <-done with r.Context().Done() for the same reason
- Store the background context in HandleT.ctx (set in Setup) to enable
  the shutdown-aware select in RequestHandler

Fixes rudderlabs#6999

Authored-By: Kuntal Maity <kuntal.1461@gmail.com>
@contributor-support

Copy link
Copy Markdown

Thank you @kuntal1461 for contributing this PR.
Please sign the Contributor License Agreement (CLA) before merging.

@github-actions

Copy link
Copy Markdown
Contributor

This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR.

@github-actions github-actions Bot added the Stale label Jun 29, 2026
@kuntal1461

Copy link
Copy Markdown
Author

hi
any update

@github-actions github-actions Bot removed the Stale label Jul 5, 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.

bug: nil channel send in webhook RequestHandler blocks HTTP handler forever (DoS)

1 participant