Repository navigation
JITSU-258: SSRF guards (Slack webhooks, function fetch, bulker destinations) - #1601
Merged
Merged
Conversation
The notification test endpoint (and the periodic sender) POSTed from the server to any stored URL and returned the response body of failures. Only https://hooks.slack.com is accepted now, both when saving a channel and when sending; redirects are not followed and the body is not echoed. JITSU-258
With FETCH_FORBID_LOCAL the pre-check resolved the hostname on its own and cached the answer for five minutes, and the request then followed redirects freely, so DNS rebinding or a redirect to an internal address got past it. The check now also runs when a connection is opened, on the address that is actually used, including IP-literal redirect targets. JITSU-258
Bulker had no SSRF guard for the URLs customers put in webhook and OTLP destinations. With FETCH_FORBID_LOCAL set (same switch as rotor's function fetch) the dialer now refuses non-public addresses at connect time, which also covers IP-literal hosts, DNS rebinding and redirect hops. FETCH_LOCAL_WHITELIST lists hosts that may stay local. The webhook that points at our own ingest endpoint keeps the plain client. JITSU-258
sahiltyagi-jitsu
marked this pull request as ready for review
October 27, 2026 07:42
![jitsu-code-review[bot]](https://avatars.githubusercontent.com/in/3391383?s=60&v=4){kind=small}
A webhook endpoint is customer-controlled. Its response was read without limit and returned in full in the batch error and in the stored task state, so an answer from an address a request was forged to reach travelled back to whoever configured it. Reads are bounded to 64 KB and what is stored or echoed to 512 bytes on a character boundary. JITSU-258
…ntries are normalized, Deno fetch checks every redirect - bulker webhook: only the exact configured global ingest endpoint takes the unguarded client; a URL that merely ends with it no longer does - fetch guard: [::1] and ::1 are the same whitelist entry in the pre-check and in the guarded agent - under Deno, which cannot use the guarded agent, redirects are followed in code and the host check runs on every hop
… redirect policy under Deno The FETCH_FORBID_LOCAL pre-check now uses the same classifier as the connect-time guard, so IPv4-mapped loopback such as [::ffff:7f00:1] is refused. fetchWithCheckedRedirects returns a redirect for redirect: manual and rejects it for redirect: error instead of always following. JITSU-258
…cts under Deno JITSU-258
There was a problem hiding this comment.
Review of d371d3693c96db49a74411e95d39fdf506c589b8
Reviewed the full diff, relevant callers, and prior findings. No new high-confidence defects found. Targeted runtime probes passed; full suites were unavailable due to missing dependencies and an older Go toolchain.
Deno's native fetch takes no dispatcher, so the guarded agent did not apply there: the pre-check resolved the name on its own and cached the verdict, and a DNS answer that turned local before the connection got through (14 of 60 attempts on the function servers, which run on Deno). With FETCH_FORBID_LOCAL on, makeFetch now uses undici's fetch with the guarded agent under Deno too, which also checks every redirect hop, so the hand-written redirect following is removed. Without the guard the native fetch and its pool are still used. JITSU-258
absorbb
reviewed
Nov 3, 2026
…n the webhook ingest check Production sets the endpoint as a host-suffix pattern (.d.jitsu.com/batch), which the exact-match check could never match, so ingest destinations lost the internal rewrite and the ingest retries. The destination URL is now parsed and checked part by part: host boundary, path, scheme, no userinfo, port, query or fragment. The complete-URL form still needs an exact match. Only a destination that is rewritten to LOCAL_INGEST_ENDPOINT takes the plain client; every other one stays behind the outbound guard.
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 11, 2026 06:50
4654167
…thout brackets The whitelist map kept an entry such as [::1] as written, while the dialer gets the host from net.SplitHostPort without brackets, so a bracketed entry never matched and the guarded dialer refused the destination the entry was meant to allow. The function fetch in rotor accepts both spellings; Bulker now does too.
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 11, 2026 10:09
080e24e
…of a fixed sleep integrationTest slept a fixed number of seconds after posting and then counted rows once. The batch and stream consumers join their Kafka consumer groups and commit on their own schedule, which takes about 5 s of the 10 s sleep on a fast machine, so on a loaded CI runner TestGoodAndBadStreams/good_batch counted 20 rows where 30 were expected, and failed. After the minimum sleep the test now keeps counting rows, and dead letters when it expects some, for up to 60 s more until it has what it expects. A table that does not exist yet is not an error until the time is up. A test that expects no rows or no dead letters, such as invalid_token, checks once. Passing tests take as long as before. Shortening good_batch's sleep to 4 s: the previous test fails with 'unexpected error in step count_rows' (the table does not exist yet), this one passes after 11.6 s. The whole bulkerapp/app package passes.
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 11, 2026 10:56
e5e719c
…fixed one inside the ephemeral range The integration tests started the app on 33042, which is inside Linux's ephemeral port range (32768-60999). On CI the app twice failed to listen with 'bind: address already in use' while nothing in the repository uses that port, and the test then waited four minutes for readiness and failed with 'bulker is not ready'. initApp now picks a free port with utils.GetPort() each time it starts the app, and the readiness failure names the port and the last error. Checked on a machine where something else holds 33042: the previous test cannot start its app, this one passes (it listened on :53915); the whole bulkerapp/app package passes. Who held 33042 on CI was not found, so whether CI is fixed is only known from a CI run.
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 11, 2026 12:11
8639d97
…f-guard # Conflicts: # webapps/console/lib/schema/config-objects.ts
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 13, 2026 04:07
46df5ab
…sses JITSU-258. The "send test" button of the OTLP logs export fetched any URL a workspace editor typed in, followed redirects, and returned up to 1,000 characters of the answer, so it could be used to read internal services. - console: the request moves to lib/server/otlp-test-request.ts and uses a guarded fetch. A non-public address is refused when the connection is opened and on every redirect hop; FETCH_LOCAL_WHITELIST (now declared in serverEnv) lets a self-hosted install test an internal collector. The error says why instead of "fetch failed". - core-functions-lib: createGuardedFetch, isBlockedAddressError and parseLocalWhitelist next to the guarded agent. FETCH_FORBID_LOCAL accepts the same spellings as Bulker (any case, 1/true/yes/on), so one value set for every service turns the guard on everywhere. A guard refusal in makeFetch is logged with the host and carries EBLOCKED. - tests: the pre-check classifier is told apart from the connect-time one, the "any answer non-public" test no longer touches the internet, and the IPv6 tests are shown as skipped when there is no IPv6 loopback.
….com JITSU-258. The host check on Slack webhook URLs is there for URLs that workspace members type in. The admin channel's URL comes from the operator (SLACK_WEBHOOK_URL) and may be a Slack-compatible endpoint, so a self-hosted install that points it at one lost its admin notifications. The channel is recognised by the id and workspace id "admin", which a stored channel cannot have (the server takes the workspace id from the request path). Tests: the admin channel posts to another host, a workspace channel only named "admin" is still refused, a redirect from the webhook is not followed, and the save-time check refuses other hosts on create and on update.
…error JITSU-258. Follow-ups to the outbound guard for customer-supplied URLs. - A refused connection fails with ErrBlockedAddress. The webhook (also for ingest-looking URLs) and OTLP clients do not retry it in process: the address is not going to become public. - The error no longer shows the address a name resolved to: net/http wraps a dial error with it, which would let a customer map internal DNS from the batch error. The transport hands back the bare refusal. - The block list has the five anycast ranges rotor's list already had (192.31.196.0/24, 192.52.193.0/24, 192.88.99.0/24, 192.175.48.0/24, 2620:4f:8000::/48), and the flag spellings match rotor's. - The rest of a capped webhook response (up to 1 MB) is drained so that the connection is reused. - The dialer's check is a named function, so a test can show that it lets a public address through. Tests: the OTLP destination is guarded through NewOtlpBulker, a refusal is not retried by either client, the proxy environment is ignored, the stored response is cut inside the stream, the capped response keeps the connection, and every range in the table has an address in the test.
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 13, 2026 06:50
4c18996
JITSU-258. The guarded undici agent used undici's default keep-alive of 4 s. On the function servers it replaces Deno's pool (poolIdleTimeout: false), so a host called less often than every few seconds cost a new TCP and TLS handshake and a DNS lookup on every request after the gap. Measured under Deno 2.9.8 against example.com over HTTPS: with a 6 s gap between requests 41 ms and 3 DNS lookups in 4 requests, with a 15 s gap 42 ms; with this change 18 ms and 16 ms with no lookups, against 14 ms and 12 ms for the native pool. Throughput, connection count and memory did not change (90 s soak, 0 errors). A keep-alive hint from the server still overrides the value, up to 10 minutes. Test: a server that sends no hint, an idle gap of 5 s, and the same connection is reused (it opens a second one without this change).
sahiltyagi-jitsu
dismissed
jitsu-code-review[bot]’s stale review
via
November 13, 2026 08:52
e35f497
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JITSU-258(audit findings T7 and R8, High). Part ofJITSU-250.Problem
FETCH_FORBID_LOCAL) resolved the host separately from the connection (DNS rebinding) and followed redirects freely; Bulker had no guard for webhook and OTLP destinations.Change
https://hooks.slack.com/...when a channel is saved and when sending; redirects aren't followed; the error shows only the HTTP status. The operator's own admin channel (SLACK_WEBHOOK_URL) is not held to the host check: the operator sets that URL, and it may be a Slack-compatible endpoint.FETCH_FORBID_LOCAL,makeFetchuses an undici agent that refuses non-public addresses when the connection is opened. That covers DNS rebinding, IP-literal hosts in every spelling (IPv4-mapped IPv6 included) and every redirect hop. Under Deno it uses undici'sfetchwith the same agent, because Deno's nativefetchtakes no dispatcher.FETCH_LOCAL_WHITELISTstill works, for[::1]as well as::1(onenormalizeHostname). Without the guard nothing changes: the native fetch and the tuned pool infunctions-server.tsstay in use.FETCH_FORBID_LOCALaccepts the same spellings as Bulker (any case,1,true,yes,on), and a refusal is logged with the host. The guarded agent keeps idle connections for 60 s (undici's default is 4 s, which cost a reconnect and a DNS lookup per request after a short idle gap on the function servers; a server's own keep-alive hint still overrides it).FETCH_FORBID_LOCAL/FETCH_LOCAL_WHITELIST. A whitelist entry may be an IPv6 literal with or without brackets ([::1]and::1). Webhook responses are bounded: reads are capped at 64 KB, and what is stored in the task state or echoed in the batch error is cut to 512 bytes. A refusal is a typed error (ErrBlockedAddress): the webhook and OTLP clients do not retry it in process, and the text no longer shows the address a name resolved to. The block list has the five anycast ranges rotor's list already had. The rest of a capped response (up to 1 MB) is drained, so the connection is reused.isIngestEndpoint, which parses the URL and checks it part by part.GLOBAL_INGEST_ENDPOINTcomes in two forms: a complete URL (exact match), or a host-suffix pattern such as.d.jitsu.com/batch(the host must end with.d.jitsu.comwith at least one label in front of it, and the path must be/batch). Userinfo, a port, a query, a fragment and a scheme other than http(s) never qualify. A qualifying destination keeps the ingest retries. Only one that is actually rewritten toLOCAL_INGEST_ENDPOINTtakes the plain client; without the rewrite it stays behind the guard, redirects included.createGuardedFetch(new, in core-functions-lib): a non-public address is refused when the connection is opened and on every redirect hop. This one is on by default and does not needFETCH_FORBID_LOCAL. A self-hosted install that tests an internal collector lists its host inFETCH_LOCAL_WHITELIST(now declared inserverEnv).Verified
Each fix has tests that fail on the code before it, except the redirect-policy test, which passes on both on purpose (the behaviour is unchanged). The tests added in a second round (OTLP test request, admin channel, typed refusal, ranges, flag spellings, drained response and the earlier gaps) were each checked by reverting their change on its own: 21 reverts, and each made a test fail.
makeFetchtests: the native fetch is not used while the guard is on; a name whose answer turns local after the pre-check is refused at connect time; a redirect to an internal address is refused and never requested. Core lib 253 and destination-functions 792 tests pass, typecheck clean. Console: 672 unit tests and 8 Slack integration tests pass;tsc, eslint and prettier are clean.api_based): Go tests for address classification, off by default, local block, redirect block, whitelist and the response caps (a 5 MB response). The ingest check is tested with the production value.d.jitsu.com/batch: legitimate subdomains keep the rewrite and the retries; 23 forms with a fragment, query, userinfo, port, other path or lookalike host do not qualify; without a local endpoint the guard stays on; the complete-URL form still works. The two tests for the production form fail on the previous commit. A webhook delivery through the whole path (NewWebhookBulkertoUpload) to a real server on[::1]works with the whitelist[::1],::1orinternal.example, [::1], and is refused, without reaching the server, with no entry,[::2]orlocalhost; it fails on the guard from before the IPv6 fix. The wholeapi_basedpackage passes, the ingest tests also with-race;go vetis clean;go buildpasses for bulkerlib, bulkerapp and ingest.FETCH_FORBID_LOCAL=true,FETCH_LOCAL_WHITELISTset to the test host, UDFs run through the console's Functions Debugger, the same UDF on the images before and after): a whitelisted host that redirects to an internal service is followed before and refused after (fetch failed). The internal service directly is refused on both. The whitelisted host itself, a redirect back to itself,redirect: "manual"and a public site (https://example.com) work on both. One visible difference:redirect: "error"now rejects withfetch failed, where the native fetch gives its own message.poolMaxIdlePerHost: 100,poolIdleTimeout: false) against the guarded agent: 15 of 19 fetch-option cases are identical (redirect follow and manual, POST string and bytes, gzip, streaming, abort timeout, a 5 MB body, HEAD, 204, cookies, headers); the other 4 differ only in error wording (redirect: "error", DNS failure, refused port) or the random port inside a redirect URL. Againstexample.comover HTTPS with a gap between requests: with undici's default keep-alive, a 6 s gap cost 41 ms and 3 DNS lookups in 4 requests (native 14 ms), a 15 s gap 42 ms (native 12 ms); with the 60 s keep-alive 18 ms and 16 ms, 0 lookups. Burst of 3000 requests at concurrency 50 on loopback: about 18,000 requests/s against 28,000 native, 85 connections against 138. 90 s soak with idle gaps: 0 errors, 1 connection, memory flat, about 28 MB more resident than native (the limits are 1 to 2 GiB).http://console:3000/api/healthcheckreturned200with its body; the fake internal host was reached (1 request) and its secret body came back to the caller. Fixed route: both refused with the new message, 0 requests reached the fake server. WithFETCH_LOCAL_WHITELISTset: the whitelisted host works and receives the OTLP payload; a redirect from it to an internal service is refused after the first hop;localhost(not listed) and a literal internal IP are refused.Post "http://.../hook": connection to a non-public address is not allowed(nodial tcp, no resolved address), 0 requests reached the server, also for[::ffff:7f00:1]. Flag off: the request reached the server and its secret body came back in the batch error. The Bulker app testsTestGoodAndBadStreams,TestBatchSizeBytesandTestEventsRetry(Kafka and Postgres in Docker) pass.https://hooks.slack.com/..., so the Slack check refuses none of them.FETCH_FORBID_LOCAL=true, and none sets a proxy or a whitelist, so this takes effect on all of them when deployed.Not done / rollout
FETCH_FORBID_LOCALis set, and no Bulker pod has it in production (checked 13 Nov: not in the live deployments' env, the manifests, or the key names of thebulkersealed secret). Until infra sets it, the Bulker part protects nothing in cloud. Draft, not merged: jitsucom/jitsu-cloud-infra#140. Rotor and the function servers already have the flag, and the console checks (Slack, OTLP test) are on by default. Turning the Bulker guard on by default in code would change every self-hosted install: a decision for the owners.FETCH_LOCAL_WHITELIST), and a Slack channel with another host fails to send and cannot be edited until its URL is replaced.functions-chainsuite does not run here); long-running behaviour (socket counts over hours; undici is pinned at the version that fixed the leak described inindex.ts); the ingest check against a real destination through a running Bulker. The tuned pool infunctions-server.tsis bypassed for guarded fetches, so itspoolMaxIdlePerHostno longer applies to them; the agent's 60 s keep-alive stands in for it (see Verified). This changes how every outbound UDF fetch connects on the function servers, so it should be rolled out with the guard flag. The function servers have no per-class image setting (the operator reads oneFUNCTIONS_SERVER_IMAGE), so there is no canary: a rollout reaches all of them, and the rollback is the previous tag.example.com.