You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
POST /api/v1/webhooks/:id/test (src/routes/webhooks.js) lets the owner of a registered webhook trigger an on-demand delivery attempt and returns delivery metadata directly in the response:
router.post('/webhooks/:id/test',testLimit,async(req,res,next)=>{constdelivery=awaitdispatcher.sendTest(req.params.id);if(!delivery)returnnext(newAppError('NOT_FOUND','Webhook not found',404));returnres.status(202).json({delivery_id: delivery.id,status: delivery.status,attempts: delivery.attempts,response_status: delivery.response_status,last_error: delivery.last_error,});});
Combined with the companion SSRF issue (webhook URL registration accepts private/internal network targets with no validation), this endpoint becomes a direct, rate-limited-but-repeatable oracle: an attacker registers a webhook pointing at an internal address/port (http://10.0.0.5:5432/, http://redis:6379/, http://169.254.169.254/latest/meta-data/iam/security-credentials/, or any other internal Docker/VPC-network host), then calls /webhooks/:id/test repeatedly. The response directly leaks:
response_status — distinguishes "port closed/connection refused" (postOnce's axios.post rejects with a network error, surfaced via last_error) from "port open, got an HTTP response" (even a non-2xx one, since validateStatus: () => true in postOnce accepts any status) from "port open, service responded with 2xx" (status: 'success') — enough to fingerprint which internal hosts/ports exist and are reachable.
last_error — the raw underlying network error message (e.g. ECONNREFUSED, ETIMEDOUT, ECONNRESET), which differs meaningfully by failure mode and further aids network reconnaissance (ECONNREFUSED = host up, port closed; ETIMEDOUT = host firewalled/unreachable; a real HTTP response = port open and speaking HTTP).
Timing: even without the response body being returned to the attacker, dispatcher.attempt()'s use of axios with a config.webhooks.timeoutMs bound means a distinguishable time-to-respond is implicitly observable via how quickly the attacker's own subsequent poll of GET /webhooks/:id/deliveries reflects a resolved status.
WEBHOOK_TEST_RATELIMIT_MAX (default 5/min) slows this down but does not prevent it — 5 requests/minute is still enough to slowly but reliably fingerprint an internal network's topology over time, and nothing distinguishes or specifically restricts targets of the test endpoint beyond the general per-IP rate limit that applies to all webhook management actions equally.
Requirements
This issue should be resolved primarily by the companion SSRF-prevention work (rejecting private/internal targets at registration and delivery time) — but track it separately because the test endpoint specifically deserves additional hardening given it is a purpose-built, on-demand, attacker-triggerable oracle in a way that a random passive pool.* event dispatch is not.
Once basic SSRF protection lands, verify explicitly that /webhooks/:id/test is also covered (it shares dispatcher.deliverToWebhook under the hood via sendTest, so it should be, but add a dedicated test proving the test endpoint specifically cannot be pointed at an internal target even if some future refactor changes its code path).
Consider reducing the information returned by the test endpoint response — e.g. do not return the raw last_error network error string to the caller (log it server-side, but return a generic "delivery failed" externally) since the granular error message itself is part of what makes this a useful reconnaissance oracle even after SSRF filtering closes the direct-request vector (defense in depth against any SSRF-filter bypass).
Ensure the per-IP WEBHOOK_TEST_RATELIMIT_MAX limiter cannot be trivially bypassed by registering many different webhooks and testing each once (i.e. consider whether rate limiting should also account for per-webhook or per-account test frequency, not just per-IP).
Acceptance Criteria
/webhooks/:id/test is verified (via a dedicated test, independent of the general webhook-delivery SSRF test) to refuse testing a webhook whose URL targets a private/internal/link-local address.
The externally-visible last_error field on the test endpoint's response no longer leaks raw low-level network error codes (ECONNREFUSED/ETIMEDOUT/etc.) verbatim; a generic category is returned instead, with the raw detail preserved only in server-side logs.
A test demonstrates that registering N different webhooks and testing each once does not allow exceeding the intended aggregate test-request budget in a way that defeats the purpose of WEBHOOK_TEST_RATELIMIT_MAX.
Existing test/webhooks.routes.test.js continues to pass.
README's webhook section documents the reduced error detail exposed by the test endpoint and why.
Additional Notes
More precise references
src/routes/webhooks.js:142-156 (POST /webhooks/:id/test): confirmed exact response shape — delivery_id, status, attempts, response_status, last_error all returned directly to the caller, no filtering.
src/services/webhookDispatcher.js:37-44 (postOnce): confirmed validateStatus: () => true, meaning axios never rejects on a non-2xx HTTP response (only on genuine network-level failures) — this is what makes response_status a reliable "port open, got any HTTP response" signal distinct from a network-level rejection.
src/services/webhookDispatcher.js:180-191 (sendTest): confirmed sendTest calls deliverToWebhook(webhook, eventType, payload.event_id, payload) directly — the exact same function dispatch() uses for real pool.* events (line 145-153 area, deliverToWebhook defined at 145-153) — confirming the issue's claim that /webhooks/:id/test "shares dispatcher.deliverToWebhook under the hood."
src/routes/webhooks.js:21-25 (testLimit): confirmed built via buildRateLimit({ windowSeconds: config.webhooks.testRateLimit.windowSeconds, max: config.webhooks.testRateLimit.max, keyPrefix: 'webhooks_test' }) — a single shared keyPrefix across all test-endpoint calls from a given identifier, meaning the current fixed-window limiter (per Fixed-window rate limiter allows boundary bursts and trusts req.ip without configuring Express trust proxy #90) is keyed purely on identifier (IP), not on webhook id — confirming the issue's own concern that registering N webhooks and testing each once doesn't multiply the per-IP budget, but also confirming there's no separate per-webhook throttle layered on top today (the rate limit is already IP-scoped, not per-webhook-id-scoped, so "testing many different webhooks" doesn't currently bypass anything additional — it's already correctly bounded by the IP-level limiter; the issue's phrasing implies this should be double-checked/tested explicitly rather than assumed).
src/routes/webhooks.js:38-46 (validateCreate)/104-130 (PATCH /webhooks/:id): confirmed isValidUrl (lines 29-36) only checks protocol === 'http:' || 'https:' — no check against private/internal/link-local address ranges at all, at either creation or update time, confirming there is currently zero SSRF filtering anywhere in this router.
Additional edge cases
DNS rebinding: even if a companion SSRF fix validates the URL's hostname against private-IP ranges at registration/test time, axios.post(url, ...) in postOnce resolves DNS again at actual request time — a hostname that resolves to a public IP at validation time but is rebound to 127.0.0.1/an internal address by request time (attacker controls their own DNS) bypasses a naive one-time-validation approach entirely. The SSRF fix (tracked elsewhere, out of this issue's direct scope per its own text) needs either to validate the resolved IP at request time (not just the hostname at registration time) or use an HTTP client configured to reject redirects/re-resolutions to private ranges on every actual connection — worth flagging here since the test endpoint is the fastest, cheapest, most repeatable way an attacker would iterate on such a bypass, making this issue's hardening (reduced error detail, defense in depth) particularly relevant even after the primary SSRF fix lands.
IPv6 and unusual representations of loopback/private addresses (http://0x7f000001/, http://[::ffff:127.0.0.1]/, decimal/octal IP encodings) are common SSRF-filter-bypass techniques that a naive regex-based hostname check would miss — worth an explicit test matrix for these once the underlying SSRF-prevention issue is implemented, even though the actual validation logic itself is out of this issue's direct scope.
The suggestion to "return a generic 'delivery failed' externally" needs to be careful not to also break the legitimate debugging use case this endpoint exists for (a real subscriber testing their own real, external, non-internal webhook endpoint deserves to know their server returned a 500 vs. timed out, to fix their own integration) — the generic-error requirement should probably apply specifically when SSRF filtering itself is what rejected the request (a clear, distinct "target not allowed" error is fine and even helpful there), while genuine delivery failures against a legitimately-external URL could arguably keep somewhat more detail than "delivery failed" with nothing else — this nuance isn't addressed by the issue's current acceptance criteria and is worth raising explicitly in the PR/discussion rather than silently deciding one way.
Implementation sketch
Once the companion SSRF-prevention validation function exists (e.g. isPublicAddress(hostname) or similar, resolving DNS and checking against RFC1918/loopback/link-local/multicast ranges), call it both at webhook create/update time (validateCreate, PATCH handler) AND at actual delivery time inside postOnce or immediately before it (to close the DNS-rebinding gap above) — sendTest's shared code path through deliverToWebhook means adding the check inside attempt()/postOnce automatically covers both real dispatches and test deliveries with one change.
In src/routes/webhooks.js's test-endpoint handler, replace the direct passthrough of delivery.last_error with a generic mapping: last_error: delivery.last_error ? 'Delivery failed' : null (or a small enum of coarse categories — 'unreachable'/'error_response'/'timeout' — if slightly more granularity is desired without leaking raw ECONNREFUSED/ETIMEDOUT strings); keep the raw detail in the logger.warn/error calls already present in webhookDispatcher.js's attempt(), unchanged.
Add a dedicated test (independent of any general SSRF test suite) specifically hitting POST /webhooks/:id/test with a webhook pre-registered against a private-range URL (bypassing create-time validation via direct repository seeding if needed, to simulate "what if create-time validation is ever bypassed/refactored") to prove the delivery-time check is what actually blocks it, not just the registration-time gate — this is exactly the "defense in depth against any SSRF-filter bypass" the issue asks for.
Test/reproduction plan
POST /webhooks/:id/test against a webhook whose URL resolves to 127.0.0.1, 169.254.169.254, and a private RFC1918 address (10.0.0.5) — each rejected with a clear, distinct "target not allowed" error, not attempted at all.
Same test but with the webhook's URL seeded directly into the repository bypassing route-level validation (simulating a bypass) — assert the delivery-time check in attempt()/postOnce still catches it.
response_status/last_error fields in the test endpoint's response, for a delivery against a real (mocked) unreachable/erroring target, no longer contain raw ECONNREFUSED/ETIMEDOUT-style strings — assert against a generic category instead.
Register 10 distinct webhooks pointing at 10 distinct (mocked, non-private) URLs and call /webhooks/:id/test once each from the same IP within one rate-limit window; assert the aggregate count against that IP's webhooks_test bucket reflects all 10 calls (i.e., confirm the existing IP-scoped limiter already correctly aggregates across different webhook ids, closing out the acceptance criterion about "testing each once does not allow exceeding the intended aggregate budget").
Overview
POST /api/v1/webhooks/:id/test(src/routes/webhooks.js) lets the owner of a registered webhook trigger an on-demand delivery attempt and returns delivery metadata directly in the response:Combined with the companion SSRF issue (webhook URL registration accepts private/internal network targets with no validation), this endpoint becomes a direct, rate-limited-but-repeatable oracle: an attacker registers a webhook pointing at an internal address/port (
http://10.0.0.5:5432/,http://redis:6379/,http://169.254.169.254/latest/meta-data/iam/security-credentials/, or any other internal Docker/VPC-network host), then calls/webhooks/:id/testrepeatedly. The response directly leaks:response_status— distinguishes "port closed/connection refused" (postOnce'saxios.postrejects with a network error, surfaced vialast_error) from "port open, got an HTTP response" (even a non-2xx one, sincevalidateStatus: () => trueinpostOnceaccepts any status) from "port open, service responded with 2xx" (status: 'success') — enough to fingerprint which internal hosts/ports exist and are reachable.last_error— the raw underlying network error message (e.g.ECONNREFUSED,ETIMEDOUT,ECONNRESET), which differs meaningfully by failure mode and further aids network reconnaissance (ECONNREFUSED= host up, port closed;ETIMEDOUT= host firewalled/unreachable; a real HTTP response = port open and speaking HTTP).dispatcher.attempt()'s use ofaxioswith aconfig.webhooks.timeoutMsbound means a distinguishable time-to-respond is implicitly observable via how quickly the attacker's own subsequent poll ofGET /webhooks/:id/deliveriesreflects a resolved status.WEBHOOK_TEST_RATELIMIT_MAX(default 5/min) slows this down but does not prevent it — 5 requests/minute is still enough to slowly but reliably fingerprint an internal network's topology over time, and nothing distinguishes or specifically restricts targets of the test endpoint beyond the general per-IP rate limit that applies to all webhook management actions equally.Requirements
pool.*event dispatch is not./webhooks/:id/testis also covered (it sharesdispatcher.deliverToWebhookunder the hood viasendTest, so it should be, but add a dedicated test proving the test endpoint specifically cannot be pointed at an internal target even if some future refactor changes its code path).last_errornetwork error string to the caller (log it server-side, but return a generic"delivery failed"externally) since the granular error message itself is part of what makes this a useful reconnaissance oracle even after SSRF filtering closes the direct-request vector (defense in depth against any SSRF-filter bypass).WEBHOOK_TEST_RATELIMIT_MAXlimiter cannot be trivially bypassed by registering many different webhooks and testing each once (i.e. consider whether rate limiting should also account for per-webhook or per-account test frequency, not just per-IP).Acceptance Criteria
/webhooks/:id/testis verified (via a dedicated test, independent of the general webhook-delivery SSRF test) to refuse testing a webhook whose URL targets a private/internal/link-local address.last_errorfield on the test endpoint's response no longer leaks raw low-level network error codes (ECONNREFUSED/ETIMEDOUT/etc.) verbatim; a generic category is returned instead, with the raw detail preserved only in server-side logs.WEBHOOK_TEST_RATELIMIT_MAX.test/webhooks.routes.test.jscontinues to pass.Additional Notes
More precise references
src/routes/webhooks.js:142-156(POST /webhooks/:id/test): confirmed exact response shape —delivery_id,status,attempts,response_status,last_errorall returned directly to the caller, no filtering.src/services/webhookDispatcher.js:37-44(postOnce): confirmedvalidateStatus: () => true, meaning axios never rejects on a non-2xx HTTP response (only on genuine network-level failures) — this is what makesresponse_statusa reliable "port open, got any HTTP response" signal distinct from a network-level rejection.src/services/webhookDispatcher.js:180-191(sendTest): confirmedsendTestcallsdeliverToWebhook(webhook, eventType, payload.event_id, payload)directly — the exact same functiondispatch()uses for realpool.*events (line 145-153 area,deliverToWebhookdefined at 145-153) — confirming the issue's claim that/webhooks/:id/test"sharesdispatcher.deliverToWebhookunder the hood."src/routes/webhooks.js:21-25(testLimit): confirmed built viabuildRateLimit({ windowSeconds: config.webhooks.testRateLimit.windowSeconds, max: config.webhooks.testRateLimit.max, keyPrefix: 'webhooks_test' })— a single sharedkeyPrefixacross all test-endpoint calls from a given identifier, meaning the current fixed-window limiter (per Fixed-window rate limiter allows boundary bursts and trusts req.ip without configuring Express trust proxy #90) is keyed purely onidentifier(IP), not on webhook id — confirming the issue's own concern that registering N webhooks and testing each once doesn't multiply the per-IP budget, but also confirming there's no separate per-webhook throttle layered on top today (the rate limit is already IP-scoped, not per-webhook-id-scoped, so "testing many different webhooks" doesn't currently bypass anything additional — it's already correctly bounded by the IP-level limiter; the issue's phrasing implies this should be double-checked/tested explicitly rather than assumed).src/routes/webhooks.js:38-46(validateCreate)/104-130(PATCH /webhooks/:id): confirmedisValidUrl(lines 29-36) only checksprotocol === 'http:' || 'https:'— no check against private/internal/link-local address ranges at all, at either creation or update time, confirming there is currently zero SSRF filtering anywhere in this router.Additional edge cases
axios.post(url, ...)inpostOnceresolves DNS again at actual request time — a hostname that resolves to a public IP at validation time but is rebound to127.0.0.1/an internal address by request time (attacker controls their own DNS) bypasses a naive one-time-validation approach entirely. The SSRF fix (tracked elsewhere, out of this issue's direct scope per its own text) needs either to validate the resolved IP at request time (not just the hostname at registration time) or use an HTTP client configured to reject redirects/re-resolutions to private ranges on every actual connection — worth flagging here since the test endpoint is the fastest, cheapest, most repeatable way an attacker would iterate on such a bypass, making this issue's hardening (reduced error detail, defense in depth) particularly relevant even after the primary SSRF fix lands.http://0x7f000001/,http://[::ffff:127.0.0.1]/, decimal/octal IP encodings) are common SSRF-filter-bypass techniques that a naive regex-based hostname check would miss — worth an explicit test matrix for these once the underlying SSRF-prevention issue is implemented, even though the actual validation logic itself is out of this issue's direct scope.'delivery failed'externally" needs to be careful not to also break the legitimate debugging use case this endpoint exists for (a real subscriber testing their own real, external, non-internal webhook endpoint deserves to know their server returned a 500 vs. timed out, to fix their own integration) — the generic-error requirement should probably apply specifically when SSRF filtering itself is what rejected the request (a clear, distinct "target not allowed" error is fine and even helpful there), while genuine delivery failures against a legitimately-external URL could arguably keep somewhat more detail than "delivery failed" with nothing else — this nuance isn't addressed by the issue's current acceptance criteria and is worth raising explicitly in the PR/discussion rather than silently deciding one way.Implementation sketch
isPublicAddress(hostname)or similar, resolving DNS and checking against RFC1918/loopback/link-local/multicast ranges), call it both at webhook create/update time (validateCreate,PATCHhandler) AND at actual delivery time insidepostOnceor immediately before it (to close the DNS-rebinding gap above) —sendTest's shared code path throughdeliverToWebhookmeans adding the check insideattempt()/postOnceautomatically covers both real dispatches and test deliveries with one change.src/routes/webhooks.js's test-endpoint handler, replace the direct passthrough ofdelivery.last_errorwith a generic mapping:last_error: delivery.last_error ? 'Delivery failed' : null(or a small enum of coarse categories —'unreachable'/'error_response'/'timeout'— if slightly more granularity is desired without leaking rawECONNREFUSED/ETIMEDOUTstrings); keep the raw detail in thelogger.warn/errorcalls already present inwebhookDispatcher.js'sattempt(), unchanged.POST /webhooks/:id/testwith a webhook pre-registered against a private-range URL (bypassing create-time validation via direct repository seeding if needed, to simulate "what if create-time validation is ever bypassed/refactored") to prove the delivery-time check is what actually blocks it, not just the registration-time gate — this is exactly the "defense in depth against any SSRF-filter bypass" the issue asks for.Test/reproduction plan
POST /webhooks/:id/testagainst a webhook whose URL resolves to127.0.0.1,169.254.169.254, and a private RFC1918 address (10.0.0.5) — each rejected with a clear, distinct "target not allowed" error, not attempted at all.attempt()/postOncestill catches it.response_status/last_errorfields in the test endpoint's response, for a delivery against a real (mocked) unreachable/erroring target, no longer contain rawECONNREFUSED/ETIMEDOUT-style strings — assert against a generic category instead./webhooks/:id/testonce each from the same IP within one rate-limit window; assert the aggregate count against that IP'swebhooks_testbucket reflects all 10 calls (i.e., confirm the existing IP-scoped limiter already correctly aggregates across different webhook ids, closing out the acceptance criterion about "testing each once does not allow exceeding the intended aggregate budget").Cross-references
testLimitthis issue relies on for its "aggregate budget" acceptance criterion is exactly the fixed-window limiter Fixed-window rate limiter allows boundary bursts and trusts req.ip without configuring Express trust proxy #90 is replacing; once Fixed-window rate limiter allows boundary bursts and trusts req.ip without configuring Express trust proxy #90 lands, re-verify this issue's rate-limit-aggregation test still holds under the new sliding-window implementation.webhookDispatcher.js/webhook-delivery code path; a reviewer should look at both together since Dispatcher webhook signatures have no timestamp or nonce — signed payloads are permanently replayable #97's timestamp/nonce work and this issue's error-detail reduction both touchbuildHeaders/attempt()'s response-handling logic in the same file.