From 23bbc6dd49d44205a635fb77ba578a955699e8c6 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 13:33:29 -0500 Subject: [PATCH 01/12] fix(security): block Redis datasource from internal Appsmith Redis (GHSA-qhfj-g87x-m39w) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Redis datasource plugin opened a Jedis connection to a user-controlled host/port with no IP-class filtering, so a user with datasource-create permission could point a Redis datasource at the internal Appsmith Redis. The existing HTTP-side filter doesn't cover Redis — Jedis uses the JVM's default resolver and bypasses the Reactor Netty hook. Three changes: 1. Block Redis datasources from reaching the internal Appsmith Redis or non-routable addresses. RedisPlugin.datasourceCreate now consults a shared RestrictedHostFilter — blocks hostname / IP overlap with APPSMITH_REDIS_URL and APPSMITH_REDIS_GIT_URL, plus loopback / link-local / cloud-metadata. Enforcement is connection-time only; PluginExecutor.validateDatasource is documented format-only, so host-policy belongs on the connection path. UX: Save succeeds, Test Datasource rejects with "Host not allowed.". 2. Make SSRF protections always-on; rename IN_DOCKER to APPSMITH_DISABLE_SSRF_FILTER. The previous filter gated loopback / link-local on IN_DOCKER=1, which the official Docker image always sets, so users on documented deployment paths were already getting this protection. Closes the gap for unofficial distributions (self- built images, bare-metal, etc.) and replaces a poorly-named variable. The most likely place to hit this is a dev machine running the server directly against http://localhost:* — set APPSMITH_DISABLE_SSRF_FILTER=true to allow it. 3. Extract RestrictedHostFilter from WebClientUtils. Pure refactor — the host-filter logic had already grown several non-HTTP call sites. --- .../appsmith/util/RestrictedHostFilter.java | 495 ++++++++++++++++++ .../com/appsmith/util/WebClientUtils.java | 232 +------- .../OAuth2ClientCredentialsTest.java | 3 + .../util/RestrictedHostFilterTest.java | 367 +++++++++++++ .../com/appsmith/util/WebClientUtilsTest.java | 131 +---- .../nio/client/HttpAsyncClientBuilder.java | 6 +- .../plugins/ElasticSearchPluginTest.java | 391 ++++++++------ .../external/plugins/GraphQLPluginTest.java | 112 ++-- .../com/external/plugins/RedisPlugin.java | 27 + .../com/external/plugins/RedisPluginTest.java | 40 ++ .../external/plugins/RestApiPluginTest.java | 200 ++++--- .../server/solutions/ce/EnvManagerCEImpl.java | 4 +- .../server/solutions/EnvManagerTest.java | 25 +- app/server/pom.xml | 22 + 14 files changed, 1437 insertions(+), 618 deletions(-) create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java create mode 100644 app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java new file mode 100644 index 000000000000..bf034818de8a --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -0,0 +1,495 @@ +package com.appsmith.util; + +import io.netty.util.concurrent.Promise; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.validator.routines.InetAddressValidator; +import org.springframework.util.StringUtils; + +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; + +/** + * Host-allowed/blocked filter for Appsmith's SSRF protections. Today used by: + * + * + *

Not applied to the JDBC plugins (Postgres, MySQL, MSSQL, Oracle, Redshift, Snowflake, + * Databricks), Mongo, ArangoDB, or the SMTP runtime send path. Several of those support SSH + * tunneling, where the user-entered host is resolved on the SSH server rather than in the + * Java process — a literal/IP check here would false-positive any datasource tunneling to + * loopback or an RFC1918 address on the far side of the tunnel. Closing those gaps needs + * per-plugin design, not a drop-in of {@link #isHostBlocked(String)}. + * + *

Two entry points for the two access patterns: {@link #isHostBlocked(String)} is + * DNS-aware and intended for plugin connection paths that run on a scheduler tolerating + * blocking I/O (e.g. Redis's {@code datasourceCreate}, wrapped in + * {@code Mono.fromCallable(...).subscribeOn(boundedElastic)}); {@link #isLiteralBlocked(String)} + * is literal-only, for pre-resolver fast paths where the Netty/driver resolver runs DNS + * separately, or for sync paths that can't afford a DNS round-trip. + */ +@Slf4j +public final class RestrictedHostFilter { + + public static final String HOST_NOT_ALLOWED = "Host not allowed."; + + /** + * Opt-out env var for the SSRF filter. Default is on (secure by default). Set + * {@code APPSMITH_DISABLE_SSRF_FILTER=true} in production only when the deployment + * intentionally needs to allow datasources / WebClient targets that point at loopback, + * link-local, or cloud-metadata addresses — e.g. local development against a service on + * {@code localhost}, or a single-tenant install where the operator accepts the risk. + * + *

The surefire test JVM also sets the {@code appsmith.test.bypass.ssrf} system property + * (see root pom) so that integration tests using MockWebServer / Testcontainers on loopback + * don't fight the filter. The two filter-specific test classes + * ({@code RestrictedHostFilterTest}, {@code WebClientUtilsTest}) flip the filter back on in + * {@code @BeforeAll} and use {@link #resetSsrfFilterDisabledForTesting()} in + * {@code @AfterAll} to restore the test-JVM-wide default. + * + *

When the filter is disabled, all of {@link #isHostBlocked(String)}, + * {@link #isLiteralBlocked(String)}, {@link #isDisallowedAndFail(String, Promise)}, and + * {@link #resolveIfAllowed(String)} skip their checks. The address-class predicates + * ({@link #isBlockedIpAddressClass(String)}, {@link #matchesBlockedAddressClass(InetAddress)}) + * still return facts about the address itself; callers that want to honor the kill-switch + * should consult the high-level "is this allowed" methods instead. + */ + private static final boolean INITIAL_SSRF_FILTER_DISABLED = computeSsrfFilterDisabled(); + + private static volatile boolean ssrfFilterDisabled = INITIAL_SSRF_FILTER_DISABLED; + + private static final InetAddressValidator inetAddressValidator = InetAddressValidator.getInstance(); + + private static final Set DISALLOWED_HOSTS = computeDisallowedHosts(); + + /** + * Hostnames of the internal Redis instances Appsmith runs for its own state — the session + * store ({@code APPSMITH_REDIS_URL}) and the Git Redis used by the workspace git + * import/sync flow ({@code APPSMITH_REDIS_GIT_URL}). Both are parsed at JVM start; the set + * is empty if neither env var is set or both are unparseable. The Git Redis URL falls back + * to the main URL via Spring property resolution, so in practice this set is usually a + * single entry — but we read both env vars directly here in case they're pointed at + * different hosts. + * + *

Used by {@link #isHostBlocked(String)} and {@link #isLiteralBlocked(String)} to + * prevent datasource plugins from being pointed at either internal Redis via direct + * hostname or via an IP that one of those hostnames currently resolves to. + * + *

Volatile + public setter (for tests only) instead of {@code final} so unit tests can + * exercise the overlap-detection logic without relying on the JVM's launch environment. + * Production code never mutates this. + */ + private static volatile Set internalRedisHosts = computeInternalRedisHosts(); + + /** + * Test-only override that lets specific hosts bypass {@link #isHostBlocked(String)}. Used by + * integration tests that spin up real services on loopback (e.g. Testcontainers-exposed + * Redis on localhost) where the production block on loopback would otherwise prevent the + * test from connecting. Empty in production runs; not mutated by production code. + */ + private static volatile Set alwaysAllowedHostsForTesting = Collections.emptySet(); + + private RestrictedHostFilter() {} + + private static Set computeDisallowedHosts() { + final Set hosts = new HashSet<>(); + addDisallowedHosts( + hosts, + "169.254.169.254", + "168.63.129.16", + "fd00:ec2::254", + "fd20:ce::254", + "100.100.100.200", + "169.254.10.10", + "169.254.170.2", + "metadata.google.internal", + "metadata.tencentyun.com"); + + // Loopback literals — always denied. matchesBlockedAddressClass catches them via + // isLoopbackAddress() on a resolved InetAddress, but isBlockedIpAddressClass operates + // on the canonical *string* form, and the IPv4-compat normalization in normalizeIpAddress + // turns "::1" into "0.0.0.1" (the embedded IPv4 bytes) which is no longer recognized as + // loopback. Seeding both literals here closes that gap: they get stored under their + // canonical forms, so any input that normalizes to the same form is blocked via this set. + // (Previously gated behind IN_DOCKER=1; now unconditional — secure for every deployment + // shape, with APPSMITH_DISABLE_SSRF_FILTER as the opt-out.) + addDisallowedHosts(hosts, "127.0.0.1", "::1"); + + return Collections.unmodifiableSet(hosts); + } + + private static boolean computeSsrfFilterDisabled() { + return "true".equalsIgnoreCase(System.getenv("APPSMITH_DISABLE_SSRF_FILTER")) + || "true".equalsIgnoreCase(System.getProperty("appsmith.test.bypass.ssrf")); + } + + /** Visible for testing only. Pairs with the {@code APPSMITH_DISABLE_SSRF_FILTER} env var. */ + public static void setSsrfFilterDisabledForTesting(boolean disabled) { + ssrfFilterDisabled = disabled; + } + + /** + * Restores the SSRF-filter state to whatever the JVM env / system properties produced at + * startup. Used by filter-testing classes in {@code @AfterAll} to undo a temporary enable + * without hardcoding the expected default. + */ + public static void resetSsrfFilterDisabledForTesting() { + ssrfFilterDisabled = INITIAL_SSRF_FILTER_DISABLED; + } + + private static void addDisallowedHosts(Set hosts, String... hostCandidates) { + for (String hostCandidate : hostCandidates) { + try { + hosts.add(normalizeHostForComparison(hostCandidate)); + } catch (UnknownHostException e) { + throw new IllegalStateException("Invalid disallowed host configured: " + hostCandidate, e); + } + } + } + + private static Set computeInternalRedisHosts() { + final Set hosts = new HashSet<>(); + addInternalRedisHostFromEnv(hosts, "APPSMITH_REDIS_URL"); + addInternalRedisHostFromEnv(hosts, "APPSMITH_REDIS_GIT_URL"); + return hosts.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(hosts); + } + + private static void addInternalRedisHostFromEnv(Set hosts, String envVar) { + final String url = System.getenv(envVar); + if (!StringUtils.hasText(url)) { + return; + } + try { + final URI uri = URI.create(url.trim()); + final String host = uri.getHost(); + if (StringUtils.hasText(host)) { + hosts.add(host.trim().toLowerCase(Locale.ROOT)); + } + } catch (IllegalArgumentException e) { + log.warn("Could not parse {} as a URI; that internal Redis host won't be filtered.", envVar); + } + } + + /** Visible for testing only. Production code never mutates the internal Redis hosts set. */ + public static void setInternalRedisHostsForTesting(String... hosts) { + if (hosts == null || hosts.length == 0) { + internalRedisHosts = Collections.emptySet(); + return; + } + final Set normalized = new HashSet<>(hosts.length); + for (String host : hosts) { + if (StringUtils.hasText(host)) { + normalized.add(host.trim().toLowerCase(Locale.ROOT)); + } + } + internalRedisHosts = normalized.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(normalized); + } + + /** + * Visible for testing only. Adds hosts that should be exempt from {@link #isHostBlocked(String)} + * for the duration of a test run. Tests that drive Testcontainers-exposed services on loopback + * should call this in {@code @BeforeAll} with the container's host string, then clear it with + * {@link #clearAlwaysAllowedHostsForTesting()} in {@code @AfterAll}. + */ + public static void setAlwaysAllowedHostsForTesting(String... hosts) { + if (hosts == null || hosts.length == 0) { + alwaysAllowedHostsForTesting = Collections.emptySet(); + return; + } + final Set normalized = new HashSet<>(hosts.length); + for (String host : hosts) { + if (StringUtils.hasText(host)) { + normalized.add(normalizeHostForComparisonQuietly(host)); + } + } + alwaysAllowedHostsForTesting = Collections.unmodifiableSet(normalized); + } + + /** Visible for testing only. Pairs with {@link #setAlwaysAllowedHostsForTesting(String...)}. */ + public static void clearAlwaysAllowedHostsForTesting() { + alwaysAllowedHostsForTesting = Collections.emptySet(); + } + + /** + * Resolves a hostname and validates that none of its addresses are disallowed for + * outbound connections from non-HTTP paths (e.g. SMTP via JavaMail). Checks against + * the cloud-metadata denylist, loopback, link-local, any-local, multicast, and IPv6 + * Unique Local Addresses (fc00::/7). Returns the first validated resolved address so + * callers can connect to it directly, preventing DNS-rebinding TOCTOU bypasses. + * + *

RFC 1918 site-local ranges (10/8, 172.16/12, 192.168/16) are intentionally + * allowed because legitimate SMTP servers frequently reside on private networks. + * + * @return the resolved {@link InetAddress} if the host is allowed, or empty if blocked + */ + public static Optional resolveIfAllowed(String host) { + if (!StringUtils.hasText(host)) { + return Optional.empty(); + } + + final InetAddress[] resolved; + try { + resolved = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + return Optional.empty(); + } + + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + // Operator has opted out (or this is a test-allowlisted host) — return the resolved + // address but skip the deny-set / address-class checks. The empty-on-unresolvable + // contract above still holds. + return Optional.of(resolved[0]); + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (DISALLOWED_HOSTS.contains(canonicalHost)) { + return Optional.empty(); + } + + for (InetAddress addr : resolved) { + if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(addr.getHostAddress())) + || matchesBlockedAddressClass(addr)) { + return Optional.empty(); + } + } + + return Optional.of(resolved[0]); + } + + private static boolean isExplicitlyAllowedForTesting(String host) { + if (alwaysAllowedHostsForTesting.isEmpty() || !StringUtils.hasText(host)) { + return false; + } + return alwaysAllowedHostsForTesting.contains(normalizeHostForComparisonQuietly(host)); + } + + /** + * Returns {@code true} if {@code host} is definitively in the disallowed set: either the + * literal/canonical host string is on the static denylist, the literal is a non-routable + * address class (loopback, any-local, link-local, multicast, IPv6 ULA), the host matches + * any configured internal Redis hostname (session store + git Redis), or DNS resolves it + * to at least one address that intersects with the denylist, a blocked address class, or + * the IPs that any configured internal Redis hostname currently resolves to. + * + *

Returns {@code false} for an unresolvable hostname so a transient DNS failure + * doesn't reject a legitimate but temporarily unreachable host. The driver's own + * connection-time error will surface naturally in that case. + * + *

Currently called from the Redis plugin's {@code datasourceCreate}, which is wrapped + * in {@code Mono.fromCallable(...).subscribeOn(boundedElastic)} so the blocking DNS call + * is safe. The other deny-list-aware paths use sibling methods: + * {@link #isLiteralBlocked(String)} for the WebClient request-filter fast path, + * {@link #isDisallowedAndFail(String, Promise)} for the Netty / Apache HttpAsyncClient + * resolver hooks, and {@link #resolveIfAllowed(String)} for the SMTP test-email path. + */ + public static boolean isHostBlocked(String host) { + if (!StringUtils.hasText(host) || ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + return true; + } + + final Set redisHosts = internalRedisHosts; + if (redisHosts.contains(canonicalHost)) { + return true; + } + + final InetAddress[] userAddresses; + try { + userAddresses = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + // Host can't be resolved right now — don't block at save time. A truly bad host will + // fail later at connection time with a clearer driver-specific error. + return false; + } + + final Set internalRedisIps = resolveInternalRedisIps(redisHosts); + for (InetAddress addr : userAddresses) { + final String addrString = normalizeHostForComparisonQuietly(addr.getHostAddress()); + if (DISALLOWED_HOSTS.contains(addrString) + || matchesBlockedAddressClass(addr) + || internalRedisIps.contains(addrString)) { + return true; + } + } + + return false; + } + + private static Set resolveInternalRedisIps(Set redisHosts) { + if (redisHosts.isEmpty()) { + return Collections.emptySet(); + } + final Set ips = new HashSet<>(); + for (String redisHost : redisHosts) { + try { + for (InetAddress addr : InetAddress.getAllByName(redisHost)) { + ips.add(normalizeHostForComparisonQuietly(addr.getHostAddress())); + } + } catch (UnknownHostException e) { + // Hostname doesn't currently resolve — skip it in the overlap check; literal + // match still applies for the same hostname string. + log.debug("Internal Redis hostname {} could not be resolved; skipping in IP overlap check.", redisHost); + } + } + return ips; + } + + /** + * Literal/canonical-only block check — no DNS, no network I/O. Catches the static denylist + * (cloud-metadata IPs), literal non-routable IP-class addresses, and a literal match + * against any configured internal Redis hostname (session store + git Redis). + * + *

Returns {@code false} for unparseable input and for anything that requires DNS to + * decide. Callers that need the DNS-resolved check (IP overlap with the internal Redis, + * hostname-resolves-to-loopback, etc.) should use {@link #isHostBlocked(String)} from an + * async path that tolerates blocking I/O. + * + *

Used by {@link WebClientUtils} request filter as the pre-resolver fast path — the + * Netty resolver runs the DNS-aware check separately via + * {@link #isDisallowedAndFail(String, Promise)}. + */ + public static boolean isLiteralBlocked(String host) { + if (!StringUtils.hasText(host) || ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + final String canonicalHost; + try { + canonicalHost = normalizeHostForComparison(host); + } catch (UnknownHostException e) { + return false; + } + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + return true; + } + return internalRedisHosts.contains(canonicalHost); + } + + public static boolean isDisallowedAndFail(String host, Promise promise) { + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + log.warn("Host {} is disallowed. Failing the request.", host); + if (promise != null) { + promise.setFailure(new UnknownHostException(HOST_NOT_ALLOWED)); + } + return true; + } + return false; + } + + public static boolean isBlockedIpAddressClass(String canonicalHost) { + if (!isValidIpAddress(canonicalHost)) { + return false; + } + try { + return matchesBlockedAddressClass(InetAddress.getByName(canonicalHost)); + } catch (UnknownHostException e) { + return false; + } + } + + public static boolean matchesBlockedAddressClass(InetAddress address) { + if (address.isLoopbackAddress() + || address.isAnyLocalAddress() + || address.isLinkLocalAddress() + || address.isMulticastAddress()) { + return true; + } + if (address instanceof Inet6Address) { + // fc00::/7 — IPv6 Unique Local Addresses + byte firstByte = address.getAddress()[0]; + return (firstByte & (byte) 0xFE) == (byte) 0xFC; + } + return false; + } + + private static boolean isValidIpAddress(String host) { + if (!StringUtils.hasText(host)) { + return false; + } + host = stripHostDecorators(host); + return inetAddressValidator.isValid(host); + } + + static String normalizeHostForComparison(String host) throws UnknownHostException { + if (!StringUtils.hasText(host)) { + return host; + } + + final String normalizedHost = stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)); + return isValidIpAddress(normalizedHost) ? normalizeIpAddress(normalizedHost) : normalizedHost; + } + + private static String normalizeHostForComparisonQuietly(String host) { + try { + return normalizeHostForComparison(host); + } catch (UnknownHostException e) { + return StringUtils.hasText(host) ? stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)) : host; + } + } + + private static String stripHostDecorators(String host) { + String sanitizedHost = host; + while (sanitizedHost.endsWith(".")) { + sanitizedHost = sanitizedHost.substring(0, sanitizedHost.length() - 1); + } + if (sanitizedHost.startsWith("[") && sanitizedHost.endsWith("]")) { + sanitizedHost = sanitizedHost.substring(1, sanitizedHost.length() - 1); + } + return sanitizedHost; + } + + private static String normalizeIpAddress(String host) throws UnknownHostException { + final InetAddress address = InetAddress.getByName(host); + + if (address instanceof Inet6Address) { + final byte[] addressBytes = address.getAddress(); + // Normalize IPv4-compatible and IPv4-mapped IPv6 literals back to the embedded IPv4 address so a single + // denylist entry blocks equivalent literal representations such as `100.100.100.200` and + // `[::100.100.100.200]`. + if (isIpv4CompatibleOrMapped(addressBytes)) { + return InetAddress.getByAddress(Arrays.copyOfRange(addressBytes, 12, 16)) + .getHostAddress(); + } + } + + return address.getHostAddress(); + } + + private static boolean isIpv4CompatibleOrMapped(byte[] addressBytes) { + if (addressBytes.length != 16) { + return false; + } + + for (int i = 0; i < 10; i++) { + if (addressBytes[i] != 0) { + return false; + } + } + + return (addressBytes[10] == 0 && addressBytes[11] == 0) + || (addressBytes[10] == (byte) 0xff && addressBytes[11] == (byte) 0xff); + } +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java index 196f5dac5b5b..d529392ee90b 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java @@ -10,7 +10,6 @@ import io.netty.util.concurrent.Promise; import io.netty.util.internal.SocketUtils; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.validator.routines.InetAddressValidator; import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.ClientRequest; @@ -21,28 +20,32 @@ import reactor.netty.http.client.HttpClient; import reactor.netty.resources.ConnectionProvider; -import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.time.Duration; import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Locale; -import java.util.Optional; -import java.util.Set; +/** + * Factory for {@link WebClient} instances pre-wired with the SSRF host filter + * ({@link RestrictedHostFilter}). The filter is enforced at two layers: + * + *

    + *
  1. A request-stage {@link ExchangeFilterFunction} ({@link #IP_CHECK_FILTER}) that + * rejects literal/canonical hosts on the deny set before DNS even runs.
    2. + *
  2. A custom Netty {@link AddressResolver} ({@link ResolverGroup}) that runs DNS + * itself and rejects when any resolved address lands on the deny set or matches a + * non-routable address class.
    3. + *
+ * + *

Host-filter logic lives in {@link RestrictedHostFilter} so non-HTTP plugins (Redis, + * SMTP, the Elasticsearch HttpAsyncClient hook, etc.) can call into the same denylist + * without depending on Spring/Netty. + */ @Slf4j public class WebClientUtils { - private static final InetAddressValidator inetAddressValidator = InetAddressValidator.getInstance(); - - private static final Set DISALLOWED_HOSTS = computeDisallowedHosts(); - - public static final String HOST_NOT_ALLOWED = "Host not allowed."; - private static final int MAX_IN_MEMORY_SIZE_IN_BYTES = 16 * 1024 * 1024; public static final ExchangeFilterFunction IP_CHECK_FILTER = @@ -66,37 +69,6 @@ public class WebClientUtils { private WebClientUtils() {} - private static Set computeDisallowedHosts() { - final Set hosts = new HashSet<>(); - addDisallowedHosts( - hosts, - "169.254.169.254", - "168.63.129.16", - "fd00:ec2::254", - "fd20:ce::254", - "100.100.100.200", - "169.254.10.10", - "169.254.170.2", - "metadata.google.internal", - "metadata.tencentyun.com"); - - if ("1".equals(System.getenv("IN_DOCKER"))) { - addDisallowedHosts(hosts, "127.0.0.1", "::1"); - } - - return Collections.unmodifiableSet(hosts); - } - - private static void addDisallowedHosts(Set hosts, String... hostCandidates) { - for (String hostCandidate : hostCandidates) { - try { - hosts.add(normalizeHostForComparison(hostCandidate)); - } catch (UnknownHostException e) { - throw new IllegalStateException("Invalid disallowed host configured: " + hostCandidate, e); - } - } - } - public static WebClient create() { return builder().build(); } @@ -199,58 +171,6 @@ protected AddressResolver newResolver(EventExecutor executor) } } - /** - * Resolves a hostname and validates that none of its addresses are disallowed for - * outbound connections from non-HTTP paths (e.g. SMTP via JavaMail). Checks against - * the cloud-metadata denylist, loopback, link-local, any-local, multicast, and IPv6 - * Unique Local Addresses (fc00::/7). Returns the first validated resolved address so - * callers can connect to it directly, preventing DNS-rebinding TOCTOU bypasses. - * - *

RFC 1918 site-local ranges (10/8, 172.16/12, 192.168/16) are intentionally - * allowed because legitimate SMTP servers frequently reside on private networks. - * - * @return the resolved {@link InetAddress} if the host is allowed, or empty if blocked - */ - public static Optional resolveIfAllowed(String host) { - if (!StringUtils.hasText(host)) { - return Optional.empty(); - } - - final String canonicalHost = normalizeHostForComparisonQuietly(host); - - if (DISALLOWED_HOSTS.contains(canonicalHost)) { - return Optional.empty(); - } - - final InetAddress[] resolved; - try { - resolved = InetAddress.getAllByName(host); - } catch (UnknownHostException e) { - return Optional.empty(); - } - - for (InetAddress addr : resolved) { - if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(addr.getHostAddress())) - || matchesBlockedAddressClass(addr)) { - return Optional.empty(); - } - } - - return Optional.of(resolved[0]); - } - - public static boolean isDisallowedAndFail(String host, Promise promise) { - final String canonicalHost = normalizeHostForComparisonQuietly(host); - if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedAddressClassInDocker(canonicalHost)) { - log.warn("Host {} is disallowed. Failing the request.", host); - if (promise != null) { - promise.setFailure(new UnknownHostException(HOST_NOT_ALLOWED)); - } - return true; - } - return false; - } - private static Mono requestFilterFn(ClientRequest request) { final String host = request.url().getHost(); @@ -259,119 +179,11 @@ private static Mono requestFilterFn(ClientRequest request) { AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, "Requested url host is null or empty")); } - final String canonicalHost; - try { - canonicalHost = normalizeHostForComparison(host); - } catch (UnknownHostException e) { - // This exception is thrown, if the given host couldn't be resolved to an IP address. But, since we only - // canonicalize after ensuring that `host` is a valid IP address, this exception should never occur. - return Mono.error(new AppsmithPluginException( - AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, "IP Address resolution is invalid")); - } - - return (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedAddressClassInDocker(canonicalHost)) - ? Mono.error(new UnknownHostException(HOST_NOT_ALLOWED)) + return RestrictedHostFilter.isLiteralBlocked(host) + ? Mono.error(new UnknownHostException(RestrictedHostFilter.HOST_NOT_ALLOWED)) : Mono.just(request); } - static boolean isBlockedIpAddressClass(String canonicalHost) { - if (!isValidIpAddress(canonicalHost)) { - return false; - } - try { - return matchesBlockedAddressClass(InetAddress.getByName(canonicalHost)); - } catch (UnknownHostException e) { - return false; - } - } - - private static boolean matchesBlockedAddressClass(InetAddress address) { - if (address.isLoopbackAddress() - || address.isAnyLocalAddress() - || address.isLinkLocalAddress() - || address.isMulticastAddress()) { - return true; - } - if (address instanceof Inet6Address) { - // fc00::/7 — IPv6 Unique Local Addresses - byte firstByte = address.getAddress()[0]; - return (firstByte & (byte) 0xFE) == (byte) 0xFC; - } - return false; - } - - private static boolean isBlockedAddressClassInDocker(String canonicalHost) { - return "1".equals(System.getenv("IN_DOCKER")) && isBlockedIpAddressClass(canonicalHost); - } - - private static boolean isValidIpAddress(String host) { - if (!StringUtils.hasText(host)) { - return false; - } - host = stripHostDecorators(host); - return inetAddressValidator.isValid(host); - } - - private static String normalizeHostForComparison(String host) throws UnknownHostException { - if (!StringUtils.hasText(host)) { - return host; - } - - final String normalizedHost = stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)); - return isValidIpAddress(normalizedHost) ? normalizeIpAddress(normalizedHost) : normalizedHost; - } - - private static String normalizeHostForComparisonQuietly(String host) { - try { - return normalizeHostForComparison(host); - } catch (UnknownHostException e) { - return StringUtils.hasText(host) ? stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)) : host; - } - } - - private static String stripHostDecorators(String host) { - String sanitizedHost = host; - while (sanitizedHost.endsWith(".")) { - sanitizedHost = sanitizedHost.substring(0, sanitizedHost.length() - 1); - } - if (sanitizedHost.startsWith("[") && sanitizedHost.endsWith("]")) { - sanitizedHost = sanitizedHost.substring(1, sanitizedHost.length() - 1); - } - return sanitizedHost; - } - - private static String normalizeIpAddress(String host) throws UnknownHostException { - final InetAddress address = InetAddress.getByName(host); - - if (address instanceof Inet6Address) { - final byte[] addressBytes = address.getAddress(); - // Normalize IPv4-compatible and IPv4-mapped IPv6 literals back to the embedded IPv4 address so a single - // denylist entry blocks equivalent literal representations such as `100.100.100.200` and - // `[::100.100.100.200]`. - if (isIpv4CompatibleOrMapped(addressBytes)) { - return InetAddress.getByAddress(Arrays.copyOfRange(addressBytes, 12, 16)) - .getHostAddress(); - } - } - - return address.getHostAddress(); - } - - private static boolean isIpv4CompatibleOrMapped(byte[] addressBytes) { - if (addressBytes.length != 16) { - return false; - } - - for (int i = 0; i < 10; i++) { - if (addressBytes[i] != 0) { - return false; - } - } - - return (addressBytes[10] == 0 && addressBytes[11] == 0) - || (addressBytes[10] == (byte) 0xff && addressBytes[11] == (byte) 0xff); - } - private static class NameResolver extends InetNameResolver { public NameResolver(EventExecutor executor) { @@ -380,7 +192,7 @@ public NameResolver(EventExecutor executor) { @Override protected void doResolve(String inetHost, Promise promise) { - if (isDisallowedAndFail(inetHost, promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(inetHost, promise)) { return; } @@ -392,7 +204,7 @@ protected void doResolve(String inetHost, Promise promise) { return; } - if (isDisallowedAndFail(address.getHostAddress(), promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), promise)) { return; } @@ -401,7 +213,7 @@ protected void doResolve(String inetHost, Promise promise) { @Override protected void doResolveAll(String inetHost, Promise> promise) { - if (isDisallowedAndFail(inetHost, promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(inetHost, promise)) { return; } @@ -415,7 +227,7 @@ protected void doResolveAll(String inetHost, Promise> promise) // Even if _one_ of the addresses is disallowed, we fail the request. for (InetAddress address : addresses) { - if (isDisallowedAndFail(address.getHostAddress(), promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), promise)) { return; } } diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java index d9793b99da56..422a8c581216 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java @@ -39,6 +39,9 @@ public class OAuth2ClientCredentialsTest { public static void setUp() throws IOException { mockEndpoint = new MockWebServer(); mockEndpoint.start(); + // The SSRF filter is JVM-wide disabled for all surefire tests (see root pom), so this + // MockWebServer-on-loopback test works without further setup. The filter is exercised + // separately in RestrictedHostFilterTest / WebClientUtilsTest. } @AfterAll diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java new file mode 100644 index 000000000000..cf3bc8bb1706 --- /dev/null +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java @@ -0,0 +1,367 @@ +package com.appsmith.util; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.net.InetAddress; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class RestrictedHostFilterTest { + + @BeforeAll + public static void enableFilterForThisClass() { + // Surefire defaults the test JVM to bypass=true (see root pom). The filter-behavior + // tests in this class need the filter actually ON to verify their assertions, so flip + // the kill-switch back. resetSsrfFilterDisabledForTesting() in @AfterAll restores + // whatever the surefire-set default was. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @AfterAll + public static void restoreSurefireDefault() { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + + @AfterEach + public void clearTestOverrides() { + // Restore the internal Redis host filter and the always-allowed override to whatever + // the JVM env produced at startup, so tests don't leak state into each other. The + // disable-knob is also reset to the per-class state set in @BeforeAll above (filter on) + // — individual tests that toggle it (e.g. the kill-switch tests) still need to flip it + // back themselves between cases. + RestrictedHostFilter.setInternalRedisHostsForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @ParameterizedTest + @ValueSource(strings = {"metadata.tencentyun.com", "Metadata.TencentYun.Com.", "METAdata.google.internal."}) + public void testIsDisallowedAndFailNormalizesMetadataHostnames(String host) { + assertTrue(RestrictedHostFilter.isDisallowedAndFail(host, null)); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "::1", "169.254.0.1", "fe80::1", "0.0.0.0", "fc00::1"}) + public void isDisallowedAndFail_blocksNonRoutableLiteralsUnconditionally(String host) { + // Previously gated behind IN_DOCKER=1; now blocked everywhere (secure-by-default). + assertTrue( + RestrictedHostFilter.isDisallowedAndFail(host, null), + "Expected " + host + " to be blocked by isDisallowedAndFail without IN_DOCKER"); + } + + // ---------- resolveIfAllowed: used by the SMTP test-email path (EnvManagerCEImpl) ---------- + + @ParameterizedTest + @ValueSource( + strings = { + "127.0.0.1", + "169.254.169.254", + "169.254.10.10", + "100.100.100.200", + "168.63.129.16", + "0.0.0.0", + }) + public void resolveIfAllowed_blocksLoopbackMetadataAndSpecialHosts(String host) { + Optional result = RestrictedHostFilter.resolveIfAllowed(host); + assertTrue(result.isEmpty(), "Expected host " + host + " to be blocked"); + } + + @Test + public void resolveIfAllowed_blocksNullAndEmpty() { + assertTrue(RestrictedHostFilter.resolveIfAllowed(null).isEmpty()); + assertTrue(RestrictedHostFilter.resolveIfAllowed("").isEmpty()); + assertTrue(RestrictedHostFilter.resolveIfAllowed(" ").isEmpty()); + } + + @Test + public void resolveIfAllowed_blocksLocalhostHostname() { + Optional result = RestrictedHostFilter.resolveIfAllowed("localhost"); + assertTrue(result.isEmpty(), "Expected 'localhost' to be blocked"); + } + + @ParameterizedTest + @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "smtp.sendgrid.net"}) + public void resolveIfAllowed_allowsLegitimateSmtpHosts(String host) { + Optional result = RestrictedHostFilter.resolveIfAllowed(host); + assertTrue(result.isPresent(), "Expected host " + host + " to be allowed"); + } + + @Test + public void resolveIfAllowed_blocksUnresolvableHost() { + Optional result = + RestrictedHostFilter.resolveIfAllowed("definitely-not-a-real-host-xyz123.invalid"); + assertTrue(result.isEmpty(), "Expected unresolvable host to be blocked"); + } + + @Test + public void resolveIfAllowed_returnsResolvedAddress() { + Optional result = RestrictedHostFilter.resolveIfAllowed("smtp.gmail.com"); + assertTrue(result.isPresent()); + assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+")); + } + + // ---------- isBlockedIpAddressClass: literal-only IP-class check ---------- + + @ParameterizedTest + @ValueSource( + strings = { + // Loopback + "127.0.0.1", + "127.0.0.2", + "127.0.0.254", + "127.1.2.3", + "127.255.255.255", + "::1", + // Any-local + "0.0.0.0", + "::", + // Link-local + "169.254.0.1", + "169.254.169.254", + "fe80::1", + // Multicast + "224.0.0.1", + "239.255.255.250", + "ff02::1", + // IPv6 ULA (fc00::/7) + "fc00::1", + "fd00::1", + "fdff::ffff", + }) + public void isBlockedIpAddressClass_recognizesNonRoutableClasses(String host) { + assertTrue( + RestrictedHostFilter.isBlockedIpAddressClass(host), + "Expected " + host + " to be recognized as a blocked address class"); + } + + @ParameterizedTest + @ValueSource( + strings = { + "1.1.1.1", + "8.8.8.8", + // RFC 1918 site-local — intentionally allowed for internal REST API targets + "192.168.1.1", + "10.0.0.1", + "172.16.0.1", + // Non-literals + "smtp.gmail.com", + "localhost", + }) + public void isBlockedIpAddressClass_doesNotMatchOtherHosts(String host) { + assertFalse( + RestrictedHostFilter.isBlockedIpAddressClass(host), + "Did not expect " + host + " to be recognized as a blocked address class"); + } + + // ---------- isHostBlocked: used by the Redis plugin (GHSA-qhfj-g87x-m39w) ---------- + + @Test + public void isHostBlocked_returnsFalseForNullOrEmpty() { + assertFalse(RestrictedHostFilter.isHostBlocked(null)); + assertFalse(RestrictedHostFilter.isHostBlocked("")); + assertFalse(RestrictedHostFilter.isHostBlocked(" ")); + } + + @ParameterizedTest + @ValueSource( + strings = { + // Static metadata-endpoint denylist literals + "169.254.169.254", + "100.100.100.200", + "168.63.129.16", + "metadata.google.internal", + "metadata.tencentyun.com", + "METAdata.google.internal.", + // Non-routable address classes — blocked unconditionally for Redis (unlike the + // IN_DOCKER-gated HTTP filter), since the GHSA targets loopback in any + // deployment that doesn't sit behind Docker. + "127.0.0.1", + "127.0.0.42", + "0.0.0.0", + "169.254.10.10", + "::1", + "fe80::1", + "fc00::1", + "fdff::ffff", + "224.0.0.1", + }) + public void isHostBlocked_blocksDenylistAndNonRoutableLiterals(String host) { + assertTrue(RestrictedHostFilter.isHostBlocked(host), "Expected " + host + " to be blocked"); + } + + @Test + public void isHostBlocked_blocksLocalhostHostname() { + // "localhost" resolves to 127.0.0.1 / ::1 — caught via the resolved-address loopback check. + assertTrue(RestrictedHostFilter.isHostBlocked("localhost")); + } + + @Test + public void isHostBlocked_returnsFalseForUnresolvableHost() { + // Key difference vs. resolveIfAllowed(): an unresolvable host is NOT blocked so that a + // transient DNS failure at config-save time doesn't reject an otherwise legitimate + // datasource. The driver will surface the real connection error later. + assertFalse(RestrictedHostFilter.isHostBlocked("definitely-not-a-real-host-xyz123.invalid")); + } + + @ParameterizedTest + @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "1.1.1.1"}) + public void isHostBlocked_allowsLegitimateHosts(String host) { + assertFalse(RestrictedHostFilter.isHostBlocked(host), "Did not expect " + host + " to be blocked"); + } + + @Test + public void isHostBlocked_matchesConfiguredInternalRedisHostnameLiterally() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isHostBlocked("internal-redis.svc.cluster.local")); + // Case-insensitive — datasource configs are user-entered. + assertTrue(RestrictedHostFilter.isHostBlocked("INTERNAL-Redis.svc.cluster.local")); + } + + @Test + public void isHostBlocked_blocksWhenUserHostResolvesToSameIpsAsInternalRedis() { + // Same hostname for both sides — guarantees IP overlap on whatever the test environment + // resolves it to. Validates the dynamic-resolution overlap path. Uses a stable public + // domain rather than depending on the env-resolved internal Redis hostname. + RestrictedHostFilter.setInternalRedisHostsForTesting("one.one.one.one"); + assertTrue(RestrictedHostFilter.isHostBlocked("one.one.one.one")); + } + + @Test + public void isHostBlocked_doesNotBlockUnrelatedHostWhenInternalRedisConfigured() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertFalse(RestrictedHostFilter.isHostBlocked("smtp.gmail.com")); + } + + @Test + public void isHostBlocked_blocksAllConfiguredInternalRedisHosts() { + // Production reads APPSMITH_REDIS_URL (session store) and APPSMITH_REDIS_GIT_URL (git + // Redis used by the workspace git import/sync flow). When they're set to different + // hosts, both must be in the deny set. + RestrictedHostFilter.setInternalRedisHostsForTesting( + "session-redis.svc.cluster.local", "git-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isHostBlocked("session-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("git-redis.svc.cluster.local")); + // Unrelated host stays allowed. + assertFalse(RestrictedHostFilter.isHostBlocked("redis.example.com")); + } + + // ---------- alwaysAllowedHostsForTesting: opt-in test escape hatch ---------- + + @Test + public void alwaysAllowedHostsForTesting_lets_an_otherwise_blocked_host_through() { + // Sanity: 127.0.0.1 is normally blocked. + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + // Allow it explicitly. + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1"); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + // Other blocked hosts are still blocked. + assertTrue(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + } + + // ---------- isLiteralBlocked: synchronous fast path, no DNS ---------- + + @Test + public void isLiteralBlocked_returnsFalseForNullEmptyAndUnresolvable() { + assertFalse(RestrictedHostFilter.isLiteralBlocked(null)); + assertFalse(RestrictedHostFilter.isLiteralBlocked("")); + assertFalse(RestrictedHostFilter.isLiteralBlocked(" ")); + // Unresolvable hostname must not trigger a DNS lookup; the literal isn't on the deny set + // so it falls through to "not blocked". + assertFalse(RestrictedHostFilter.isLiteralBlocked("definitely-not-a-real-host-xyz123.invalid")); + } + + @ParameterizedTest + @ValueSource( + strings = { + "169.254.169.254", + "metadata.google.internal", + "127.0.0.1", + "::1", + "169.254.10.10", + "fc00::1", + }) + public void isLiteralBlocked_blocksDenylistAndNonRoutableLiterals(String host) { + assertTrue(RestrictedHostFilter.isLiteralBlocked(host), "Expected " + host + " to be blocked"); + } + + @Test + public void isLiteralBlocked_blocksConfiguredInternalRedisHostnameLiterally() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isLiteralBlocked("internal-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isLiteralBlocked("INTERNAL-Redis.svc.cluster.local")); + } + + @Test + public void isLiteralBlocked_doesNotResolveHostnamesToCheckBlockedClass() { + // "localhost" resolves to 127.0.0.1, but isLiteralBlocked must not do DNS — and "localhost" + // itself isn't on the static denylist, so it passes. (isHostBlocked would catch it via DNS; + // that's the correct method to call from the async connection path.) + assertFalse(RestrictedHostFilter.isLiteralBlocked("localhost")); + } + + @Test + public void alwaysAllowedHostsForTesting_clears_cleanly() { + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1"); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + } + + // ---------- APPSMITH_DISABLE_SSRF_FILTER kill-switch ---------- + + @Test + public void ssrfFilterDisabled_bypassesIsHostBlocked() { + // Sanity: blocked by default. + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + assertTrue(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + // Flip the kill-switch — everything passes. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + assertFalse(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + assertFalse(RestrictedHostFilter.isHostBlocked("localhost")); + } + + @Test + public void ssrfFilterDisabled_bypassesIsLiteralBlocked() { + assertTrue(RestrictedHostFilter.isLiteralBlocked("127.0.0.1")); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isLiteralBlocked("127.0.0.1")); + assertFalse(RestrictedHostFilter.isLiteralBlocked("169.254.169.254")); + } + + @Test + public void ssrfFilterDisabled_bypassesIsDisallowedAndFail() { + assertTrue(RestrictedHostFilter.isDisallowedAndFail("127.0.0.1", null)); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isDisallowedAndFail("127.0.0.1", null)); + assertFalse(RestrictedHostFilter.isDisallowedAndFail("169.254.169.254", null)); + } + + @Test + public void ssrfFilterDisabled_bypassesResolveIfAllowed() { + // Loopback normally returns empty. + assertTrue(RestrictedHostFilter.resolveIfAllowed("127.0.0.1").isEmpty()); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + // With the kill-switch on, the resolved address comes back. + Optional result = RestrictedHostFilter.resolveIfAllowed("127.0.0.1"); + assertTrue(result.isPresent()); + assertEquals("127.0.0.1", result.get().getHostAddress()); + } + + @Test + public void ssrfFilterDisabled_doesNotAffectAddressClassPredicates() { + // The kill-switch turns off enforcement, not facts about the address itself. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("127.0.0.1")); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("169.254.0.1")); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("fc00::1")); + } +} diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java index d80a84351e70..652faa24365c 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java @@ -1,6 +1,7 @@ package com.appsmith.util; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.springframework.http.HttpMethod; @@ -9,17 +10,32 @@ import reactor.test.StepVerifier; import java.lang.reflect.Method; -import java.net.InetAddress; import java.net.URI; import java.net.UnknownHostException; -import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +/** + * Integration test for the WebClient request filter wiring. The filter logic itself + * lives in {@link RestrictedHostFilter} and is tested in {@link RestrictedHostFilterTest}; + * this class just verifies that {@link WebClientUtils#IP_CHECK_FILTER} actually rejects + * disallowed hosts at the WebClient layer. + */ public class WebClientUtilsTest { + @BeforeAll + public static void enableFilterForThisClass() { + // Surefire defaults to bypass=true (see root pom); re-enable the filter so the + // request-filter integration test below can verify blocking behavior. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @AfterAll + public static void restoreSurefireDefault() { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + @ParameterizedTest @ValueSource( strings = { @@ -36,116 +52,11 @@ public void testRequestFilterFnRejectsExpandedMetadataEndpoints(String url) thro StepVerifier.create(invokeRequestFilterFn(url)) .expectErrorSatisfies(throwable -> { assertTrue(throwable instanceof UnknownHostException); - assertEquals(WebClientUtils.HOST_NOT_ALLOWED, throwable.getMessage()); + assertEquals(RestrictedHostFilter.HOST_NOT_ALLOWED, throwable.getMessage()); }) .verify(); } - @ParameterizedTest - @ValueSource(strings = {"metadata.tencentyun.com", "Metadata.TencentYun.Com.", "METAdata.google.internal."}) - public void testIsDisallowedAndFailNormalizesMetadataHostnames(String host) { - assertTrue(WebClientUtils.isDisallowedAndFail(host, null)); - } - - @ParameterizedTest - @ValueSource( - strings = { - "127.0.0.1", - "169.254.169.254", - "169.254.10.10", - "100.100.100.200", - "168.63.129.16", - "0.0.0.0", - }) - public void resolveIfAllowed_blocksLoopbackMetadataAndSpecialHosts(String host) { - Optional result = WebClientUtils.resolveIfAllowed(host); - assertTrue(result.isEmpty(), "Expected host " + host + " to be blocked"); - } - - @Test - public void resolveIfAllowed_blocksNullAndEmpty() { - assertTrue(WebClientUtils.resolveIfAllowed(null).isEmpty()); - assertTrue(WebClientUtils.resolveIfAllowed("").isEmpty()); - assertTrue(WebClientUtils.resolveIfAllowed(" ").isEmpty()); - } - - @Test - public void resolveIfAllowed_blocksLocalhostHostname() { - Optional result = WebClientUtils.resolveIfAllowed("localhost"); - assertTrue(result.isEmpty(), "Expected 'localhost' to be blocked"); - } - - @ParameterizedTest - @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "smtp.sendgrid.net"}) - public void resolveIfAllowed_allowsLegitimateSmtpHosts(String host) { - Optional result = WebClientUtils.resolveIfAllowed(host); - assertTrue(result.isPresent(), "Expected host " + host + " to be allowed"); - } - - @Test - public void resolveIfAllowed_blocksUnresolvableHost() { - Optional result = WebClientUtils.resolveIfAllowed("definitely-not-a-real-host-xyz123.invalid"); - assertTrue(result.isEmpty(), "Expected unresolvable host to be blocked"); - } - - @Test - public void resolveIfAllowed_returnsResolvedAddress() { - Optional result = WebClientUtils.resolveIfAllowed("smtp.gmail.com"); - assertTrue(result.isPresent()); - assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+")); - } - - @ParameterizedTest - @ValueSource( - strings = { - // Loopback - "127.0.0.1", - "127.0.0.2", - "127.0.0.254", - "127.1.2.3", - "127.255.255.255", - "::1", - // Any-local - "0.0.0.0", - "::", - // Link-local - "169.254.0.1", - "169.254.169.254", - "fe80::1", - // Multicast - "224.0.0.1", - "239.255.255.250", - "ff02::1", - // IPv6 ULA (fc00::/7) - "fc00::1", - "fd00::1", - "fdff::ffff", - }) - public void isBlockedIpAddressClass_recognizesNonRoutableClasses(String host) { - assertTrue( - WebClientUtils.isBlockedIpAddressClass(host), - "Expected " + host + " to be recognized as a blocked address class"); - } - - @ParameterizedTest - @ValueSource( - strings = { - "1.1.1.1", - "8.8.8.8", - // RFC 1918 site-local — intentionally allowed for internal REST API targets - "192.168.1.1", - "10.0.0.1", - "172.16.0.1", - // Non-literals - "smtp.gmail.com", - "localhost", - }) - public void isBlockedIpAddressClass_doesNotMatchOtherHosts(String host) { - assertFalse( - WebClientUtils.isBlockedIpAddressClass(host), - "Did not expect " + host + " to be recognized as a blocked address class"); - } - @SuppressWarnings("unchecked") private Mono invokeRequestFilterFn(String url) throws Exception { final Method method = WebClientUtils.class.getDeclaredMethod("requestFilterFn", ClientRequest.class); diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java index 13c8a348fe37..68ef0afa488f 100644 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java @@ -29,7 +29,7 @@ package org.apache.http.impl.nio.client; -import com.appsmith.util.WebClientUtils; +import com.appsmith.util.RestrictedHostFilter; import org.apache.http.ConnectionReuseStrategy; import org.apache.http.Header; import org.apache.http.HttpHost; @@ -654,12 +654,12 @@ public CloseableHttpAsyncClient build() { // This `dnsResolver` is the only thing different from the original class. // In the original class, it is set to SystemDefaultDnsResolver.INSTANCE, inlined. final DnsResolver dnsResolver = host -> { - if (WebClientUtils.isDisallowedAndFail(host, null)) { + if (RestrictedHostFilter.isDisallowedAndFail(host, null)) { throw new UnknownHostException("Host " + host + " is not allowed"); } final InetAddress[] addresses = InetAddress.getAllByName(host); for (InetAddress address : addresses) { - if (WebClientUtils.isDisallowedAndFail(address.getHostAddress(), null)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), null)) { throw new UnknownHostException("Host " + host + " is not allowed"); } } diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java index c410833a411f..d990d56ce1d2 100755 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java @@ -7,6 +7,7 @@ import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.RequestParamDTO; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.ElasticSearchPluginError; import lombok.extern.slf4j.Slf4j; import mockwebserver3.MockResponse; @@ -393,206 +394,256 @@ public void shouldVerifyNotFound() { .verifyComplete(); } + // The itShouldDeny* / itShouldRejectGetToMetadata* tests below all explicitly verify the + // SSRF filter's blocking behavior. Surefire bypasses the filter JVM-wide (see root pom) for + // the benefit of the rest of this class (which talks to the Testcontainers ES on loopback), + // so each test method flips it back on for its body. The redirect variants additionally + // allowlist loopback so the MockWebServer that serves the 301 is reachable. + @Test public void itShouldDenyTestDatasourceWithInstanceMetadataAws() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host 169.254.169.254 is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host 169.254.169.254 is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldDenyTestDatasourceWithInstanceMetadataAwsWithDnsResolution() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254.nip.io"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host 169.254.169.254.nip.io is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254.nip.io"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host 169.254.169.254.nip.io is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldDenyTestDatasourceWithInstanceMetadataGcp() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://metadata.google.internal"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host metadata.google.internal is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://metadata.google.internal"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host metadata.google.internal is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAws() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254 is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254 is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAwsWithDnsResolution() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254.nip.io"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254.nip.io is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254.nip.io"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254.nip.io is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAwsWithDnsResolutionAndRedirect() throws IOException { - MockWebServer mockWebServer = new MockWebServer(); - MockResponse mockRedirectResponse = new MockResponse() - .setResponseCode(301) - .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); - mockWebServer.enqueue(mockRedirectResponse); - mockWebServer.start(); - - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://" + mockWebServer.getHostName()); - endpoint.setPort((long) mockWebServer.getPort()); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254.nip.io is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + try { + MockWebServer mockWebServer = new MockWebServer(); + MockResponse mockRedirectResponse = new MockResponse() + .setResponseCode(301) + .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); + mockWebServer.enqueue(mockRedirectResponse); + mockWebServer.start(); + + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://" + mockWebServer.getHostName()); + endpoint.setPort((long) mockWebServer.getPort()); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254.nip.io is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + } } @Test public void itShouldRejectGetToMetadataGcp() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://metadata.google.internal"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host metadata.google.internal is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://metadata.google.internal"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host metadata.google.internal is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataGcpAndRedirect() throws IOException { - MockWebServer mockWebServer = new MockWebServer(); - MockResponse mockRedirectResponse = - new MockResponse().setResponseCode(301).addHeader("Location", "http://metadata.google.internal"); - mockWebServer.enqueue(mockRedirectResponse); - mockWebServer.start(); - - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://" + mockWebServer.getHostName()); - endpoint.setPort((long) mockWebServer.getPort()); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host metadata.google.internal is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + try { + MockWebServer mockWebServer = new MockWebServer(); + MockResponse mockRedirectResponse = + new MockResponse().setResponseCode(301).addHeader("Location", "http://metadata.google.internal"); + mockWebServer.enqueue(mockRedirectResponse); + mockWebServer.start(); + + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://" + mockWebServer.getHostName()); + endpoint.setPort((long) mockWebServer.getPort()); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host metadata.google.internal is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + } } @Test diff --git a/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java b/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java index 534a15845a6e..6707e57f96db 100644 --- a/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java +++ b/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java @@ -16,6 +16,7 @@ import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; import com.appsmith.external.services.SharedConfig; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.GraphQLPluginError; import com.external.utils.GraphQLHintMessageUtils; import com.fasterxml.jackson.databind.JsonNode; @@ -1160,49 +1161,94 @@ public void testQueryParamsInDatasource() { .verifyComplete(); } + // These deny tests previously only checked that the request did not succeed, which would + // pass on any ordinary network failure as well — making them useless as SSRF regression + // coverage. They now (a) re-enable the filter (surefire bypasses it JVM-wide; see root pom) + // and (b) assert the downstream error message specifically contains "Host not allowed.", + // so a network outage to the target host can no longer mask a filter regression. + @Test public void testDenyInstanceMetadataAws() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCname() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataGcp() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } /** diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 0ba19679f656..9d181284c915 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -13,6 +13,7 @@ import com.appsmith.external.models.SSLDetails; import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RedisErrorMessages; import com.external.plugins.exceptions.RedisPluginError; import com.external.utils.RedisURIUtils; @@ -258,6 +259,12 @@ private JedisPoolConfig buildPoolConfig() { public Mono datasourceCreate(DatasourceConfiguration datasourceConfiguration) { log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for Redis plugin."); return Mono.fromCallable(() -> { + // Single SSRF enforcement point — see assertHostAllowed below. validateDatasource + // intentionally does not duplicate this check (its contract is format-only; + // host-policy belongs on the connection path). testDatasource builds its pool + // via this same path, so the user sees "Host not allowed." immediately on + // "Test Datasource". See GHSA-qhfj-g87x-m39w. + assertHostAllowed(datasourceConfiguration); final JedisPoolConfig poolConfig = buildPoolConfig(); boolean isTlsEnabled = isTlsEnabled(datasourceConfiguration); int timeout = @@ -270,6 +277,18 @@ public Mono datasourceCreate(DatasourceConfiguration datasourceConfig .subscribeOn(scheduler); } + private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) throws AppsmithPluginException { + if (isEndpointMissing(datasourceConfiguration.getEndpoints())) { + // Let the existing missing-host validation surface the error. + return; + } + final String host = datasourceConfiguration.getEndpoints().get(0).getHost(); + if (RestrictedHostFilter.isHostBlocked(host)) { + throw new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); + } + } + private boolean isTlsEnabled(DatasourceConfiguration datasourceConfiguration) throws AppsmithPluginException { if (datasourceConfiguration.getConnection() == null || datasourceConfiguration.getConnection().getSsl() == null @@ -318,6 +337,14 @@ public Set validateDatasource(DatasourceConfiguration datasourceConfigur if (isEndpointMissing(datasourceConfiguration.getEndpoints())) { invalids.add(RedisErrorMessages.DS_MISSING_HOST_ADDRESS_ERROR_MSG); } + // SSRF host-allowed enforcement intentionally lives in datasourceCreate (which goes + // through assertHostAllowed → RestrictedHostFilter.isHostBlocked on the + // bounded-elastic scheduler) rather than here. PluginExecutor#validateDatasource is + // documented as "mandatory fields and format only — does NOT check validity of + // those fields; use testDatasource for that". Host-on-denylist is a policy/validity + // check, not a format check, so it belongs on the connection path. testDatasource + // calls datasourceCreate to build the pool and surfaces "Host not allowed." + // immediately when the user clicks Test. See GHSA-qhfj-g87x-m39w. DBAuth auth = (DBAuth) datasourceConfiguration.getAuthentication(); if (isAuthenticationMissing(auth)) { diff --git a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java index c8e4964fa681..30addaa53584 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java +++ b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java @@ -11,11 +11,13 @@ import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.models.SSLDetails; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RedisErrorMessages; import com.external.plugins.exceptions.RedisPluginError; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -56,6 +58,15 @@ public class RedisPluginTest { public static void setup() { host = redis.getContainerIpAddress(); port = redis.getFirstMappedPort(); + // Testcontainers exposes Redis on loopback, which the production isHostBlocked filter + // rejects (GHSA-qhfj-g87x-m39w). Allow this specific host for the duration of the test + // class so the integration tests can drive the real Redis. Cleared in @AfterAll. + RestrictedHostFilter.setAlwaysAllowedHostsForTesting(host); + } + + @AfterAll + public static void teardown() { + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); } private DatasourceConfiguration createDatasourceConfiguration() { @@ -125,6 +136,35 @@ public void itShouldThrowErrorForUnsupportedTlsAuthType() { .verify(); } + @Test + public void datasourceCreate_blockedHost_failsWithHostNotAllowed() { + // End-to-end check that the Redis plugin actually wires through to the SSRF filter + // (GHSA-qhfj-g87x-m39w): a config pointing at a denylist host fails datasourceCreate + // with HOST_NOT_ALLOWED, never opening a Jedis pool. Surefire bypasses the filter + // JVM-wide (see root pom) so the rest of this class can talk to Testcontainers Redis + // on loopback via the @BeforeAll allowlist; this test flips the filter back on for + // its body so the deny path is exercised. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration blockedConfig = new DatasourceConfiguration(); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("169.254.169.254"); // AWS instance metadata + endpoint.setPort(6379L); + blockedConfig.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.datasourceCreate(blockedConfig)) + .expectErrorSatisfies(error -> { + assertTrue(error instanceof AppsmithPluginException); + assertTrue( + error.getMessage().contains(RestrictedHostFilter.HOST_NOT_ALLOWED), + "Expected '" + RestrictedHostFilter.HOST_NOT_ALLOWED + "', got: " + error.getMessage()); + }) + .verify(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + } + @Test public void itShouldValidateDatasourceWithNoEndpoints() { DatasourceConfiguration invalidDatasourceConfiguration = new DatasourceConfiguration(); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 3723c9b8f730..172dbf79f2e9 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -19,6 +19,7 @@ import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; import com.appsmith.external.services.SharedConfig; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RestApiPluginError; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -1879,85 +1880,109 @@ public void testQueryParamsInDatasource() { .verifyComplete(); } + // The testDenyInstanceMetadata* tests below all explicitly verify the SSRF filter's blocking + // behavior. Surefire bypasses the filter JVM-wide (see root pom) for the benefit of tests + // that use MockWebServer on loopback, so each of these methods flips it back on for its body. + @Test public void testDenyInstanceMetadataAws() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue( - result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed."), - "Unexpected error message. Did this fail for a different reason?"); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Unexpected error message. Did this fail for a different reason?"); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCname() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertThat(result.getPluginErrorDetails().getDownstreamErrorMessage()) - .endsWith("Host not allowed."); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertThat(result.getPluginErrorDetails().getDownstreamErrorMessage()) + .endsWith("Host not allowed."); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCnameIpv6() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://0--a9fe-a9fe.sslip.io/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://0--a9fe-a9fe.sslip.io/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCompatibleIpv6Address() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://[::169.254.169.254]/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://[::169.254.169.254]/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test @@ -1977,31 +2002,42 @@ public void testDenyInstanceMetadataGcp() { @Test public void testDenyInstanceMetadataAwsWithRedirect() throws IOException { - // Generate a mock response which redirects to the invalid host - mockEndpoint = new MockWebServer(); - MockResponse mockRedirectResponse = new MockResponse() - .setResponseCode(301) - .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); - mockEndpoint.enqueue(mockRedirectResponse); - mockEndpoint.start(); - - HttpUrl mockHttpUrl = mockEndpoint.url("/mock/redirect"); - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl(mockHttpUrl.toString()); - - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + // Combined fix: re-enable the filter so the redirect target (169.254.169.254) is blocked, + // but allowlist loopback so the initial MockWebServer call (which serves the 301) goes + // through. "::1" is in the allowlist because the IPv4-compat normalizer canonicalizes it + // to "0.0.0.1", matching whatever form the Netty resolver hands the filter on IPv6 hosts. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + try { + // Generate a mock response which redirects to the invalid host + mockEndpoint = new MockWebServer(); + MockResponse mockRedirectResponse = new MockResponse() + .setResponseCode(301) + .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); + mockEndpoint.enqueue(mockRedirectResponse); + mockEndpoint.start(); + + HttpUrl mockHttpUrl = mockEndpoint.url("/mock/redirect"); + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl(mockHttpUrl.toString()); + + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + } } @Test diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java index b1954e63140a..d806df46869f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java @@ -28,7 +28,7 @@ import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserService; -import com.appsmith.util.WebClientUtils; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.mail.MessagingException; @@ -816,7 +816,7 @@ public Mono sendTestEmail(TestEmailConfigRequestDTO requestDTO) { new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration.")); } - var resolvedAddress = WebClientUtils.resolveIfAllowed(requestDTO.getSmtpHost()); + var resolvedAddress = RestrictedHostFilter.resolveIfAllowed(requestDTO.getSmtpHost()); if (resolvedAddress.isEmpty()) { return Mono.error( new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration.")); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java index a514fdefd141..a113ce0d542e 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java @@ -20,6 +20,7 @@ import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserService; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.BeforeEach; @@ -410,14 +411,22 @@ private TestEmailConfigRequestDTO buildDto(String smtpHost, int port) { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "169.254.169.254", "localhost"}) public void sendTestEmail_WhenBlockedHost_ThrowsException(String host) { - mockSuperUser(); - - StepVerifier.create(envManager.sendTestEmail(buildDto(host))) - .expectErrorSatisfies(e -> { - assertThat(e).isInstanceOf(AppsmithException.class); - assertThat(e.getMessage()).contains("Invalid SMTP configuration"); - }) - .verify(); + // Surefire bypasses the SSRF filter JVM-wide (see root pom) for the benefit of tests that + // use MockWebServer / Testcontainers on loopback. This test explicitly verifies the + // filter, so re-enable it for the test body. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + mockSuperUser(); + + StepVerifier.create(envManager.sendTestEmail(buildDto(host))) + .expectErrorSatisfies(e -> { + assertThat(e).isInstanceOf(AppsmithException.class); + assertThat(e.getMessage()).contains("Invalid SMTP configuration"); + }) + .verify(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @ParameterizedTest diff --git a/app/server/pom.xml b/app/server/pom.xml index 7204205fa307..9630729ce1ac 100644 --- a/app/server/pom.xml +++ b/app/server/pom.xml @@ -125,6 +125,28 @@ --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED + + + true + src/test/java ${skipUTs} From bb197cc49b3cac93fe472cf2ff59d43d669c413b Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 14:36:17 -0500 Subject: [PATCH 02/12] fixup: CodeRabbit review fixes (CE PR) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six follow-ups from the CodeRabbit review on the CE PR — none change behavior beyond closing existing edge-case gaps and tightening tests. Will collapse into the squash-merged commit. - RestrictedHostFilter: route stored internal-Redis hosts through normalizeHostForComparisonQuietly so the storage form matches the compare-side canonicalization. Closes an IPv6-bracketed-URL bypass (e.g. redis://[::1]:6379 was stored as "[::1]" but lookups compared to "0.0.0.1"). - RedisPlugin.assertHostAllowed: document the deliberate TOCTOU acceptance between isHostBlocked and Jedis's own re-resolution. - RestApiPluginTest.testDenyInstanceMetadataGcp: wrap in setSsrfFilterDisabledForTesting(false) and assert "Host not allowed." so a network failure to metadata.google.internal can't mask a filter regression. Was the one outlier left unwrapped when the AWS variants were strengthened. - RestApiPluginTest.testDenyInstanceMetadataAwsWithRedirect: use a local MockWebServer rather than reassigning the @BeforeEach `mockEndpoint` field; the reassignment leaked the original (tearDown only shuts down whatever's in the field at AfterEach time). - ElasticSearchPluginTest.itShouldRejectGetTo*AndRedirect: add mockWebServer.shutdown() in finally so the per-test MockWebServer instances don't leak threads / sockets across the suite. - RestrictedHostFilterTest.resolveIfAllowed_returnsResolvedAddress: use the literal IP 1.1.1.1 and assert exact equality, instead of smtp.gmail.com + an IPv4-only regex that would fail on IPv6-preferred CI environments. --- .../appsmith/util/RestrictedHostFilter.java | 10 ++++- .../util/RestrictedHostFilterTest.java | 7 ++- .../plugins/ElasticSearchPluginTest.java | 6 ++- .../com/external/plugins/RedisPlugin.java | 8 ++++ .../external/plugins/RestApiPluginTest.java | 43 +++++++++++++------ 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java index bf034818de8a..691d6ebc2a5b 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -178,7 +178,12 @@ private static void addInternalRedisHostFromEnv(Set hosts, String envVar final URI uri = URI.create(url.trim()); final String host = uri.getHost(); if (StringUtils.hasText(host)) { - hosts.add(host.trim().toLowerCase(Locale.ROOT)); + // Normalize through the same canonicalization that isHostBlocked / + // isLiteralBlocked use for the compare side — strips IPv6 brackets, lowercases, + // collapses IPv4-compat / IPv4-mapped IPv6 to the embedded IPv4. Without this, + // a redis://[::1] env var stores "[::1]" but lookups canonicalize to "0.0.0.1" + // and silently miss. + hosts.add(normalizeHostForComparisonQuietly(host)); } } catch (IllegalArgumentException e) { log.warn("Could not parse {} as a URI; that internal Redis host won't be filtered.", envVar); @@ -194,7 +199,8 @@ public static void setInternalRedisHostsForTesting(String... hosts) { final Set normalized = new HashSet<>(hosts.length); for (String host : hosts) { if (StringUtils.hasText(host)) { - normalized.add(host.trim().toLowerCase(Locale.ROOT)); + // Same canonicalization as the compare side — see addInternalRedisHostFromEnv. + normalized.add(normalizeHostForComparisonQuietly(host)); } } internalRedisHosts = normalized.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(normalized); diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java index cf3bc8bb1706..f0483c639db2 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java @@ -103,9 +103,12 @@ public void resolveIfAllowed_blocksUnresolvableHost() { @Test public void resolveIfAllowed_returnsResolvedAddress() { - Optional result = RestrictedHostFilter.resolveIfAllowed("smtp.gmail.com"); + // Use a literal IP rather than a public hostname — avoids a DNS lookup in CI and + // dodges IPv6-preferred resolvers that would have failed the old IPv4-only regex + // assertion on the resolved address. + Optional result = RestrictedHostFilter.resolveIfAllowed("1.1.1.1"); assertTrue(result.isPresent()); - assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+")); + assertEquals("1.1.1.1", result.get().getHostAddress()); } // ---------- isBlockedIpAddressClass: literal-only IP-class check ---------- diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java index d990d56ce1d2..c3abb3e737f2 100755 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java @@ -537,8 +537,8 @@ public void itShouldRejectGetToMetadataAwsWithDnsResolution() { public void itShouldRejectGetToMetadataAwsWithDnsResolutionAndRedirect() throws IOException { RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + MockWebServer mockWebServer = new MockWebServer(); try { - MockWebServer mockWebServer = new MockWebServer(); MockResponse mockRedirectResponse = new MockResponse() .setResponseCode(301) .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); @@ -571,6 +571,7 @@ public void itShouldRejectGetToMetadataAwsWithDnsResolutionAndRedirect() throws } finally { RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + mockWebServer.shutdown(); } } @@ -610,8 +611,8 @@ public void itShouldRejectGetToMetadataGcp() { public void itShouldRejectGetToMetadataGcpAndRedirect() throws IOException { RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + MockWebServer mockWebServer = new MockWebServer(); try { - MockWebServer mockWebServer = new MockWebServer(); MockResponse mockRedirectResponse = new MockResponse().setResponseCode(301).addHeader("Location", "http://metadata.google.internal"); mockWebServer.enqueue(mockRedirectResponse); @@ -643,6 +644,7 @@ public void itShouldRejectGetToMetadataGcpAndRedirect() throws IOException { } finally { RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + mockWebServer.shutdown(); } } diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 9d181284c915..251d47a89b2b 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -283,6 +283,14 @@ private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) return; } final String host = datasourceConfiguration.getEndpoints().get(0).getHost(); + // Deliberate TOCTOU: isHostBlocked resolves the hostname here, but Jedis will + // re-resolve it independently when the pool opens its first socket. A hostile + // DNS server could in theory flip the answer between the two resolutions and + // bypass the filter. We accept this gap because the actual threat this PR + // closes — a user typing the internal Redis hostname / IP literally — doesn't + // rely on rebinding, and closing the gap would mean swapping Jedis's connection + // factory for one that takes a pre-resolved IP (a meaningful maintenance burden + // for a hypothetical attack). See GHSA-qhfj-g87x-m39w discussion. if (RestrictedHostFilter.isHostBlocked(host)) { throw new AppsmithPluginException( AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 172dbf79f2e9..1c0301672b23 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -1987,17 +1987,30 @@ public void testDenyInstanceMetadataAwsViaCompatibleIpv6Address() { @Test public void testDenyInstanceMetadataGcp() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test @@ -2008,16 +2021,19 @@ public void testDenyInstanceMetadataAwsWithRedirect() throws IOException { // to "0.0.0.1", matching whatever form the Netty resolver hands the filter on IPv6 hosts. RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + // Use a local server rather than reassigning the @BeforeEach `mockEndpoint` field — + // overwriting that field would orphan the original (tearDown only shuts down the + // current value of the field) and leak server threads/sockets across the suite. + MockWebServer redirectMockServer = new MockWebServer(); try { // Generate a mock response which redirects to the invalid host - mockEndpoint = new MockWebServer(); MockResponse mockRedirectResponse = new MockResponse() .setResponseCode(301) .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); - mockEndpoint.enqueue(mockRedirectResponse); - mockEndpoint.start(); + redirectMockServer.enqueue(mockRedirectResponse); + redirectMockServer.start(); - HttpUrl mockHttpUrl = mockEndpoint.url("/mock/redirect"); + HttpUrl mockHttpUrl = redirectMockServer.url("/mock/redirect"); DatasourceConfiguration dsConfig = new DatasourceConfiguration(); dsConfig.setUrl(mockHttpUrl.toString()); @@ -2037,6 +2053,7 @@ public void testDenyInstanceMetadataAwsWithRedirect() throws IOException { } finally { RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + redirectMockServer.shutdown(); } } From 8c5ffa0742196c9893625ad798781c1692ec6d00 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 15:43:47 -0500 Subject: [PATCH 03/12] chore(redis): upgrade Jedis 3.3.0 to 5.2.0 Pure dependency bump, no code changes. Jedis 3.3.0 (2020) predates the JedisSocketFactory injection API needed to close the DNS-rebinding TOCTOU in the Redis SSRF host filter (next commit). 5.2.0 keeps the Java 8 baseline and every API the plugin uses is unchanged: JedisPool, getResource, sendCommand, ping, JedisPoolConfig, Protocol.Command, SafeEncoder, JedisException all compile as-is (verified locally). Jedis is used only by redisPlugin; the server's own Redis state runs on Lettuce, so this bump is plugin-isolated (PF4J classloader + shaded jar). Co-Authored-By: Claude Opus 4.8 --- app/server/appsmith-plugins/redisPlugin/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/appsmith-plugins/redisPlugin/pom.xml b/app/server/appsmith-plugins/redisPlugin/pom.xml index 13396cde0f45..caadf59dd011 100644 --- a/app/server/appsmith-plugins/redisPlugin/pom.xml +++ b/app/server/appsmith-plugins/redisPlugin/pom.xml @@ -19,7 +19,7 @@ redis.clients jedis - 3.3.0 + 5.2.0 org.slf4j From ffc9e616f1af6537c357e94ba3e27e8c686d89d6 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 16:07:05 -0500 Subject: [PATCH 04/12] fix(test): de-pin RedisPluginTest invalid-host assertion for Jedis 5.2.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Jedis 5.x client reports an unresolvable host with a different exception message than 3.x did, so the assertion pinned to "Failed connecting to :" no longer matches. Assert the behavioral contract instead — an invalid host yields an unsuccessful DatasourceTestResult carrying a non-empty error — which is robust across driver versions. Necessitated by the Jedis 3.3.0 to 5.2.0 upgrade. Co-Authored-By: Claude Opus 4.8 --- .../test/java/com/external/plugins/RedisPluginTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java index 30addaa53584..905862e59303 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java +++ b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java @@ -250,7 +250,6 @@ public void itShouldTestDatasource() { public void itShouldThrowErrorIfHostnameIsInvalid() { String invalidHost = "invalidHost"; - String errorMessage = "Failed connecting to " + invalidHost + ":" + port; DatasourceConfiguration datasourceConfiguration = createDatasourceConfiguration(); Endpoint endpoint = new Endpoint(); @@ -263,8 +262,12 @@ public void itShouldThrowErrorIfHostnameIsInvalid() { StepVerifier.create(datasourceTestResultMono) .assertNext(datasourceTestResult -> { assertNotNull(datasourceTestResult); + // An invalid/unreachable host must fail the test with an error reported back to + // the user. We deliberately don't assert the exact message: Jedis 5.x reports an + // unresolvable host with a different string than 3.x did, so pinning to a + // driver-internal message makes the test brittle across upgrades. assertFalse(datasourceTestResult.isSuccess()); - assertTrue(datasourceTestResult.getInvalids().contains(errorMessage)); + assertFalse(datasourceTestResult.getInvalids().isEmpty()); }) .verifyComplete(); } From c046f6b2a674e7c98d130037e7c0121cf58c3e1f Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 16:08:18 -0500 Subject: [PATCH 05/12] fix(security): close Redis SSRF DNS-rebinding TOCTOU with single-resolution socket factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create-time pre-check (RestrictedHostFilter.isHostBlocked) and Jedis's own connect-time resolution were two independent DNS lookups — a hostile resolver could return an allowed IP to the pre-check and the internal Redis IP to the driver (DNS rebinding). Now requires Jedis 5.2.0's socket-factory injection (prior commits). - Add RestrictedHostFilter.firstAllowedRedisAddress(host, resolved): pure policy check over an already-resolved address set, returning the validated address to connect to (or empty if blocked). Caller owns the single DNS resolution. - Add RestrictedHostJedisSocketFactory: resolves once at connect time, validates via the above, and connects the socket directly to the pinned IP so the driver never re-resolves. For rediss:// the plaintext socket connects to the pinned IP while the original hostname is used for SNI / cert verification; connect + TLS logic mirrors DefaultJedisSocketFactory. - Wire it into RedisPlugin.datasourceCreate via JedisPool(poolConfig, socketFactory, clientConfig), deriving the client config from the URI exactly as Jedis's own URI constructor does. The isHostBlocked pre-check stays as create-time UX/defense-in-depth. - Test that the factory rejects a denylisted host at connect time. See GHSA-qhfj-g87x-m39w. Co-Authored-By: Claude Opus 4.8 --- .../appsmith/util/RestrictedHostFilter.java | 53 +++++++ .../com/external/plugins/RedisPlugin.java | 51 +++++-- .../RestrictedHostJedisSocketFactory.java | 131 ++++++++++++++++++ .../com/external/plugins/RedisPluginTest.java | 27 ++++ 4 files changed, 248 insertions(+), 14 deletions(-) create mode 100644 app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java index 691d6ebc2a5b..5e138c61e205 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -360,6 +360,59 @@ private static Set resolveInternalRedisIps(Set redisHosts) { return ips; } + /** + * Connection-time policy check for the Redis plugin's socket factory. Given a host and the + * addresses it has ALREADY been resolved to (by the caller, in a single DNS lookup), returns + * the first address that is safe to connect to, or empty if any resolved address is blocked — + * static denylist (cloud-metadata literals), non-routable address class (loopback, any-local, + * link-local, multicast, IPv6 ULA), a literal match against an internal Appsmith Redis + * hostname, or overlap with the IPs an internal Redis hostname currently resolves to. + * + *

This is the half of the fix that closes the DNS-rebinding TOCTOU. {@link + * #isHostBlocked(String)} resolves at datasource-create time, but Jedis would resolve the + * hostname again when it opens the socket. {@code RestrictedHostJedisSocketFactory} (in the + * redisPlugin module) resolves exactly once at connect time and passes those addresses here, + * then connects directly to the returned {@link InetAddress} — so the check and the connect + * share one resolution and the driver never re-resolves the hostname. + * + *

The caller owns DNS resolution deliberately: passing addresses in (rather than resolving + * here) lets the caller distinguish an unresolvable host (a genuine connection error) from a + * policy block (this method returning empty), and guarantees the validated address is the one + * connected to. Honors the {@code APPSMITH_DISABLE_SSRF_FILTER} kill-switch and the test + * allowlist, in which case the first resolved address is returned without policy checks. + */ + public static Optional firstAllowedRedisAddress(String host, InetAddress[] resolvedAddresses) { + if (resolvedAddresses == null || resolvedAddresses.length == 0) { + return Optional.empty(); + } + + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return Optional.of(resolvedAddresses[0]); + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + return Optional.empty(); + } + + final Set redisHosts = internalRedisHosts; + if (redisHosts.contains(canonicalHost)) { + return Optional.empty(); + } + + final Set internalRedisIps = resolveInternalRedisIps(redisHosts); + for (InetAddress addr : resolvedAddresses) { + final String addrString = normalizeHostForComparisonQuietly(addr.getHostAddress()); + if (DISALLOWED_HOSTS.contains(addrString) + || matchesBlockedAddressClass(addr) + || internalRedisIps.contains(addrString)) { + return Optional.empty(); + } + } + + return Optional.of(resolvedAddresses[0]); + } + /** * Literal/canonical-only block check — no DNS, no network I/O. Catches the static denylist * (cloud-metadata IPs), literal non-routable IP-class addresses, and a literal match diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 251d47a89b2b..00005af2fc3f 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -26,11 +26,16 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.HostAndPort; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; +import redis.clients.jedis.JedisSocketFactory; import redis.clients.jedis.Protocol; import redis.clients.jedis.exceptions.JedisException; +import redis.clients.jedis.util.JedisURIHelper; import redis.clients.jedis.util.SafeEncoder; import java.net.URI; @@ -259,18 +264,39 @@ private JedisPoolConfig buildPoolConfig() { public Mono datasourceCreate(DatasourceConfiguration datasourceConfiguration) { log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for Redis plugin."); return Mono.fromCallable(() -> { - // Single SSRF enforcement point — see assertHostAllowed below. validateDatasource - // intentionally does not duplicate this check (its contract is format-only; - // host-policy belongs on the connection path). testDatasource builds its pool - // via this same path, so the user sees "Host not allowed." immediately on - // "Test Datasource". See GHSA-qhfj-g87x-m39w. + // SSRF enforcement is layered (GHSA-qhfj-g87x-m39w): + // 1. assertHostAllowed below is an early, create-time pre-check so the user + // sees "Host not allowed." immediately on Save/Test for an obviously + // denylisted host. validateDatasource intentionally does not duplicate it + // (its contract is format-only; host-policy belongs on the connection path). + // 2. The real, TOCTOU-proof guarantee is RestrictedHostJedisSocketFactory + // (wired below), which re-validates with a SINGLE DNS resolution at the + // moment the pool opens a socket and connects to the validated IP — so a + // DNS-rebinding resolver cannot slip a different IP past the pre-check. assertHostAllowed(datasourceConfiguration); final JedisPoolConfig poolConfig = buildPoolConfig(); boolean isTlsEnabled = isTlsEnabled(datasourceConfiguration); int timeout = (int) Duration.ofSeconds(CONNECTION_TIMEOUT).toMillis(); URI uri = RedisURIUtils.getURI(datasourceConfiguration, isTlsEnabled); - JedisPool jedisPool = new JedisPool(poolConfig, uri, timeout); + + // Derive the client config exactly as Jedis's own URI constructor does (user, + // password, db index, ssl scheme — see JedisFactory(URI, ...)), then route + // socket creation through RestrictedHostJedisSocketFactory. This preserves all + // existing URI parsing semantics while pinning the connection to a single + // SSRF-validated DNS resolution. + HostAndPort hostAndPort = new HostAndPort(uri.getHost(), uri.getPort()); + JedisClientConfig clientConfig = DefaultJedisClientConfig.builder() + .connectionTimeoutMillis(timeout) + .socketTimeoutMillis(timeout) + .user(JedisURIHelper.getUser(uri)) + .password(JedisURIHelper.getPassword(uri)) + .database(JedisURIHelper.getDBIndex(uri)) + .ssl(JedisURIHelper.isRedisSSLScheme(uri)) + .build(); + JedisSocketFactory socketFactory = + new RestrictedHostJedisSocketFactory(hostAndPort, clientConfig); + JedisPool jedisPool = new JedisPool(poolConfig, socketFactory, clientConfig); log.debug(Thread.currentThread().getName() + ": Created Jedis pool."); return jedisPool; }) @@ -283,14 +309,11 @@ private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) return; } final String host = datasourceConfiguration.getEndpoints().get(0).getHost(); - // Deliberate TOCTOU: isHostBlocked resolves the hostname here, but Jedis will - // re-resolve it independently when the pool opens its first socket. A hostile - // DNS server could in theory flip the answer between the two resolutions and - // bypass the filter. We accept this gap because the actual threat this PR - // closes — a user typing the internal Redis hostname / IP literally — doesn't - // rely on rebinding, and closing the gap would mean swapping Jedis's connection - // factory for one that takes a pre-resolved IP (a meaningful maintenance burden - // for a hypothetical attack). See GHSA-qhfj-g87x-m39w discussion. + // Create-time pre-check for fast, clear UX on Save/Test. This resolution and the one + // Jedis performs at connect time are technically distinct, so this check alone is + // subject to DNS rebinding — but it is no longer the security boundary: + // RestrictedHostJedisSocketFactory re-validates with a single resolution at connect + // time and is what actually closes the rebinding TOCTOU. See GHSA-qhfj-g87x-m39w. if (RestrictedHostFilter.isHostBlocked(host)) { throw new AppsmithPluginException( AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java new file mode 100644 index 000000000000..ec20f837713d --- /dev/null +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -0,0 +1,131 @@ +package com.external.plugins; + +import com.appsmith.util.RestrictedHostFilter; +import redis.clients.jedis.HostAndPort; +import redis.clients.jedis.JedisClientConfig; +import redis.clients.jedis.JedisSocketFactory; +import redis.clients.jedis.SSLSocketWrapper; +import redis.clients.jedis.exceptions.JedisConnectionException; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.UnknownHostException; +import java.util.Optional; + +/** + * A Jedis {@link JedisSocketFactory} that enforces Appsmith's SSRF host policy at the exact + * moment the socket is opened, using a SINGLE DNS resolution shared between the policy check and + * the TCP connect. + * + *

Background (GHSA-qhfj-g87x-m39w): the Redis plugin pre-checks the host in + * {@code datasourceCreate} via {@link RestrictedHostFilter#isHostBlocked(String)}, but the stock + * {@link redis.clients.jedis.DefaultJedisSocketFactory} resolves the hostname again when it opens + * the pool's first socket. Those two independent resolutions are a DNS-rebinding TOCTOU window: a + * hostile resolver can return an allowed IP to the pre-check and the internal Redis IP to the + * driver. This factory closes the window by resolving once, validating the resolved addresses via + * {@link RestrictedHostFilter#firstAllowedRedisAddress(String, InetAddress[])}, and connecting the + * socket directly to the returned {@link InetAddress} — the driver never re-resolves the hostname. + * + *

For TLS ({@code rediss://}) the plaintext socket connects to the pinned IP, but the original + * hostname is handed to the {@link SSLSocketFactory} and hostname verifier, so SNI and certificate + * verification run against the hostname the user configured, not the IP. The connect and TLS + * handshake logic mirrors {@code DefaultJedisSocketFactory} (socket options, {@link + * SSLSocketWrapper}) to avoid behavioral drift; the only deviation is the single-resolution + * SSRF check in place of the stock {@code InetAddress.getAllByName(...)} + connect. + */ +public class RestrictedHostJedisSocketFactory implements JedisSocketFactory { + + private final String host; + private final int port; + private final int connectionTimeout; + private final int socketTimeout; + private final boolean ssl; + private final SSLSocketFactory sslSocketFactory; + private final SSLParameters sslParameters; + private final HostnameVerifier hostnameVerifier; + + public RestrictedHostJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig config) { + this.host = hostAndPort.getHost(); + this.port = hostAndPort.getPort(); + this.connectionTimeout = config.getConnectionTimeoutMillis(); + this.socketTimeout = config.getSocketTimeoutMillis(); + this.ssl = config.isSsl(); + this.sslSocketFactory = config.getSslSocketFactory(); + this.sslParameters = config.getSslParameters(); + this.hostnameVerifier = config.getHostnameVerifier(); + } + + @Override + public Socket createSocket() throws JedisConnectionException { + // SINGLE resolution. A genuine resolution failure surfaces as a connection error (clearer + // than "Host not allowed."); only a resolved-but-blocked host is a policy rejection. + final InetAddress[] resolved; + try { + resolved = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + throw new JedisConnectionException("Failed to resolve Redis host '" + host + "'.", e); + } + + // Validate the addresses we just resolved and connect to one of them — the driver never + // re-resolves, so a rebinding resolver cannot swap in a different (internal) IP. + final Optional pinned = RestrictedHostFilter.firstAllowedRedisAddress(host, resolved); + if (!pinned.isPresent()) { + throw new JedisConnectionException(RestrictedHostFilter.HOST_NOT_ALLOWED); + } + final InetAddress pinnedAddress = pinned.get(); + + Socket socket = null; + try { + socket = new Socket(); + // Socket options mirror DefaultJedisSocketFactory. + socket.setReuseAddress(true); + socket.setKeepAlive(true); + socket.setTcpNoDelay(true); + socket.setSoLinger(true, 0); + + // Connect to the validated IP literal — no second DNS lookup happens here. + socket.connect(new InetSocketAddress(pinnedAddress, port), connectionTimeout); + socket.setSoTimeout(socketTimeout); + + if (ssl) { + SSLSocketFactory factory = + sslSocketFactory != null ? sslSocketFactory : (SSLSocketFactory) SSLSocketFactory.getDefault(); + final Socket plainSocket = socket; + // Hand the original hostname (not the pinned IP) to the TLS layer so SNI and + // certificate hostname verification run against the configured hostname. + socket = factory.createSocket(plainSocket, host, port, true); + if (sslParameters != null) { + ((SSLSocket) socket).setSSLParameters(sslParameters); + } + socket = new SSLSocketWrapper((SSLSocket) socket, plainSocket); + if (hostnameVerifier != null && !hostnameVerifier.verify(host, ((SSLSocket) socket).getSession())) { + throw new JedisConnectionException( + String.format("The connection to '%s' failed ssl/tls hostname verification.", host)); + } + } + + return socket; + } catch (JedisConnectionException e) { + closeQuietly(socket); + throw e; + } catch (Exception e) { + closeQuietly(socket); + throw new JedisConnectionException("Failed to create socket.", e); + } + } + + private static void closeQuietly(Socket socket) { + if (socket != null) { + try { + socket.close(); + } catch (Exception ignored) { + // Best-effort cleanup on a failed connect; nothing actionable here. + } + } + } +} diff --git a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java index 905862e59303..80af94771665 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java +++ b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java @@ -27,7 +27,11 @@ import org.testcontainers.utility.DockerImageName; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.HostAndPort; +import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisPool; +import redis.clients.jedis.exceptions.JedisConnectionException; import java.util.ArrayList; import java.util.Arrays; @@ -165,6 +169,29 @@ public void datasourceCreate_blockedHost_failsWithHostNotAllowed() { } } + @Test + public void socketFactory_blockedHost_failsAtConnectTime() { + // The connect-time guarantee that closes the DNS-rebinding TOCTOU (GHSA-qhfj-g87x-m39w): + // RestrictedHostJedisSocketFactory re-validates with a single DNS resolution at the moment + // it opens the socket, so even if the datasourceCreate pre-check were bypassed by a flipping + // resolver, the factory still refuses to connect to a denylisted address. Surefire bypasses + // the filter JVM-wide (see root pom); flip it back on for this body. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + JedisClientConfig clientConfig = DefaultJedisClientConfig.builder().build(); + RestrictedHostJedisSocketFactory factory = + new RestrictedHostJedisSocketFactory(new HostAndPort("169.254.169.254", 6379), clientConfig); + + JedisConnectionException ex = + Assertions.assertThrows(JedisConnectionException.class, factory::createSocket); + assertTrue( + ex.getMessage().contains(RestrictedHostFilter.HOST_NOT_ALLOWED), + "Expected '" + RestrictedHostFilter.HOST_NOT_ALLOWED + "', got: " + ex.getMessage()); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + } + @Test public void itShouldValidateDatasourceWithNoEndpoints() { DatasourceConfiguration invalidDatasourceConfiguration = new DatasourceConfiguration(); From 58eadee9df45660fdb638fab854100e910fd0629 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 16:59:14 -0500 Subject: [PATCH 06/12] refactor: share SSRF deny-logic between isHostBlocked and firstAllowedRedisAddress Both methods evaluated the deny list independently (canonical denylist + IP-class check + internal-Redis literal, then a per-resolved-address loop). Extract isCanonicalHostBlocked and isAnyResolvedAddressBlocked so the two share one evaluation and cannot drift into an SSRF gap. Behavior-preserving: isHostBlocked keeps its canonical short-circuit before DNS and its unresolvable-host-returns-false contract. RestrictedHostFilterTest (87) green. Co-Authored-By: Claude Opus 4.8 --- .../appsmith/util/RestrictedHostFilter.java | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java index 5e138c61e205..d5ef2126213a 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -310,12 +310,7 @@ public static boolean isHostBlocked(String host) { final String canonicalHost = normalizeHostForComparisonQuietly(host); - if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { - return true; - } - - final Set redisHosts = internalRedisHosts; - if (redisHosts.contains(canonicalHost)) { + if (isCanonicalHostBlocked(canonicalHost)) { return true; } @@ -328,8 +323,31 @@ public static boolean isHostBlocked(String host) { return false; } - final Set internalRedisIps = resolveInternalRedisIps(redisHosts); - for (InetAddress addr : userAddresses) { + return isAnyResolvedAddressBlocked(userAddresses); + } + + /** + * Shared deny-logic (1 of 2): is the canonical host string itself blocked, without any DNS? + * Covers the static denylist (cloud-metadata literals, loopback), a non-routable IP-class + * literal, and a literal match against an internal Appsmith Redis hostname. Extracted so + * {@link #isHostBlocked(String)} and {@link #firstAllowedRedisAddress(String, InetAddress[])} + * evaluate the deny list identically and cannot drift into an SSRF gap. + */ + private static boolean isCanonicalHostBlocked(String canonicalHost) { + return DISALLOWED_HOSTS.contains(canonicalHost) + || isBlockedIpAddressClass(canonicalHost) + || internalRedisHosts.contains(canonicalHost); + } + + /** + * Shared deny-logic (2 of 2): does any already-resolved address fall in the denylist, a + * blocked address class, or overlap with the IPs an internal Appsmith Redis hostname currently + * resolves to? Extracted alongside {@link #isCanonicalHostBlocked(String)} so both callers + * share one address-level deny evaluation. + */ + private static boolean isAnyResolvedAddressBlocked(InetAddress[] resolvedAddresses) { + final Set internalRedisIps = resolveInternalRedisIps(internalRedisHosts); + for (InetAddress addr : resolvedAddresses) { final String addrString = normalizeHostForComparisonQuietly(addr.getHostAddress()); if (DISALLOWED_HOSTS.contains(addrString) || matchesBlockedAddressClass(addr) @@ -337,7 +355,6 @@ public static boolean isHostBlocked(String host) { return true; } } - return false; } @@ -391,25 +408,10 @@ public static Optional firstAllowedRedisAddress(String host, InetAd } final String canonicalHost = normalizeHostForComparisonQuietly(host); - if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { - return Optional.empty(); - } - - final Set redisHosts = internalRedisHosts; - if (redisHosts.contains(canonicalHost)) { + if (isCanonicalHostBlocked(canonicalHost) || isAnyResolvedAddressBlocked(resolvedAddresses)) { return Optional.empty(); } - final Set internalRedisIps = resolveInternalRedisIps(redisHosts); - for (InetAddress addr : resolvedAddresses) { - final String addrString = normalizeHostForComparisonQuietly(addr.getHostAddress()); - if (DISALLOWED_HOSTS.contains(addrString) - || matchesBlockedAddressClass(addr) - || internalRedisIps.contains(addrString)) { - return Optional.empty(); - } - } - return Optional.of(resolvedAddresses[0]); } From 36d9a65a405c0eddfa0bde0ab05dce3ce99789a8 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Tue, 23 Jun 2026 22:07:48 -0500 Subject: [PATCH 07/12] fix(security): enforce TLS hostname verification for rediss:// Redis datasources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Jedis 5.2.0 does not verify the certificate hostname by default (only the trust chain), so a MITM presenting any CA-trusted-but-mismatched cert on the pinned IP would be accepted. Set the endpoint identification algorithm to HTTPS in the socket factory so the cert SAN/CN is checked against the configured hostname (we connect to the pinned IP but pass the hostname to the TLS layer, so verification runs against the hostname). Defaults to HTTPS only when a caller hasn't set its own endpoint-identification policy. Low blast radius: rediss:// support was added 2026-03-03 (#41587) and is barely adopted. Chain validation was already on, so self-signed certs already failed; this newly rejects only CA-trusted-but-mismatched certs and IP-addressed rediss:// (certs rarely carry IP SANs). No opt-out env var yet — add one if a real datasource needs a mismatched/IP cert. Addresses the CodeRabbit "TLS hostname verification effectively off" finding. Co-Authored-By: Claude Opus 4.8 --- .../RestrictedHostJedisSocketFactory.java | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java index ec20f837713d..07606b4ac618 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -32,11 +32,14 @@ * socket directly to the returned {@link InetAddress} — the driver never re-resolves the hostname. * *

For TLS ({@code rediss://}) the plaintext socket connects to the pinned IP, but the original - * hostname is handed to the {@link SSLSocketFactory} and hostname verifier, so SNI and certificate - * verification run against the hostname the user configured, not the IP. The connect and TLS - * handshake logic mirrors {@code DefaultJedisSocketFactory} (socket options, {@link - * SSLSocketWrapper}) to avoid behavioral drift; the only deviation is the single-resolution - * SSRF check in place of the stock {@code InetAddress.getAllByName(...)} + connect. + * hostname is handed to the {@link SSLSocketFactory}, so SNI and certificate verification run + * against the hostname the user configured, not the IP. Unlike stock Jedis 5.2.0 — which leaves + * hostname verification off by default — this factory enforces it by setting the endpoint + * identification algorithm to {@code HTTPS}, so a CA-trusted-but-mismatched cert on the pinned IP + * is rejected. The connect and TLS handshake logic otherwise mirrors {@code DefaultJedisSocketFactory} + * (socket options, {@link SSLSocketWrapper}); the deviations are the single-resolution SSRF check + * in place of the stock {@code InetAddress.getAllByName(...)} + connect, and the enforced + * endpoint identification. */ public class RestrictedHostJedisSocketFactory implements JedisSocketFactory { @@ -99,10 +102,22 @@ public Socket createSocket() throws JedisConnectionException { // Hand the original hostname (not the pinned IP) to the TLS layer so SNI and // certificate hostname verification run against the configured hostname. socket = factory.createSocket(plainSocket, host, port, true); - if (sslParameters != null) { - ((SSLSocket) socket).setSSLParameters(sslParameters); + + final SSLSocket sslSocket = (SSLSocket) socket; + // Enforce TLS certificate hostname verification (RFC 2818): the cert's SAN/CN must + // match the configured hostname. Jedis 5.2.0 leaves this off by default, which would + // let a MITM present any CA-trusted-but-mismatched cert on the pinned IP. Verification + // runs against `host` (passed to createSocket above), not the IP we connected to. + // Only default to HTTPS when a caller hasn't deliberately set its own endpoint- + // identification policy. + final SSLParameters params = sslParameters != null ? sslParameters : sslSocket.getSSLParameters(); + final String endpointIdAlgorithm = params.getEndpointIdentificationAlgorithm(); + if (endpointIdAlgorithm == null || endpointIdAlgorithm.isEmpty()) { + params.setEndpointIdentificationAlgorithm("HTTPS"); } - socket = new SSLSocketWrapper((SSLSocket) socket, plainSocket); + sslSocket.setSSLParameters(params); + + socket = new SSLSocketWrapper(sslSocket, plainSocket); if (hostnameVerifier != null && !hostnameVerifier.verify(host, ((SSLSocket) socket).getSession())) { throw new JedisConnectionException( String.format("The connection to '%s' failed ssl/tls hostname verification.", host)); From a7966b6c44545c7ba2ec6f3a628feb07a5b07e8c Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Wed, 24 Jun 2026 10:06:15 -0500 Subject: [PATCH 08/12] fix(redis): surface "Host not allowed." on connect-time SSRF rejection instead of a generic error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit testDatasource read error.getCause().getMessage(), but the socket factory's connection-time rejection throws a causeless JedisConnectionException (message "Host not allowed."), so getCause() was null and the call NPE'd — the user saw a generic "Couldn't establish a connection" instead of the intended message. This is the DNS-rebinding path: the host passes the create-time pre-check (resolves to an allowed IP) but the socket factory's own resolution lands on a blocked IP and refuses. Fall back to the error's own message when there's no cause (also fixes the latent NPE on the no-PONG path). Add a log line in the factory so the rejection is visible server-side. Verified on the deploy preview with rbndr.us flipping 172.31.4.163 <-> 127.0.0.1. Co-Authored-By: Claude Opus 4.8 --- .../main/java/com/external/plugins/RedisPlugin.java | 11 +++++++++-- .../plugins/RestrictedHostJedisSocketFactory.java | 6 ++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 00005af2fc3f..60406d94205f 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -459,8 +459,15 @@ public Mono testDatasource(JedisPool connectionPool) { return Mono.just(connectionPool) .flatMap(c -> verifyPing(connectionPool)) .then(Mono.just(new DatasourceTestResult())) - .onErrorResume(error -> - Mono.just(new DatasourceTestResult(error.getCause().getMessage()))); + .onErrorResume(error -> { + // Prefer the cause's message (driver wraps connection failures with an + // IOException cause), but fall back to the error itself so a causeless + // exception doesn't NPE here. The connection-time SSRF rejection throws a + // causeless JedisConnectionException whose message is "Host not allowed."; + // without this fallback it surfaced as a generic connection error. + final Throwable reported = error.getCause() != null ? error.getCause() : error; + return Mono.just(new DatasourceTestResult(reported.getMessage())); + }); } } } diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java index 07606b4ac618..e3121fa15473 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -1,6 +1,7 @@ package com.external.plugins; import com.appsmith.util.RestrictedHostFilter; +import lombok.extern.slf4j.Slf4j; import redis.clients.jedis.HostAndPort; import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisSocketFactory; @@ -41,6 +42,7 @@ * in place of the stock {@code InetAddress.getAllByName(...)} + connect, and the enforced * endpoint identification. */ +@Slf4j public class RestrictedHostJedisSocketFactory implements JedisSocketFactory { private final String host; @@ -78,6 +80,10 @@ public Socket createSocket() throws JedisConnectionException { // re-resolves, so a rebinding resolver cannot swap in a different (internal) IP. final Optional pinned = RestrictedHostFilter.firstAllowedRedisAddress(host, resolved); if (!pinned.isPresent()) { + // Connection-time SSRF rejection (incl. the DNS-rebinding case where the resolved + // address differs from the create-time pre-check). Log it — this is the only place the + // block is observable server-side, since the pre-check rejection happens elsewhere. + log.warn("Refusing Redis connection: host '{}' resolved to a disallowed address.", host); throw new JedisConnectionException(RestrictedHostFilter.HOST_NOT_ALLOWED); } final InetAddress pinnedAddress = pinned.get(); From f027b9f90ba2fa08be40d499ab0d78eac9dfdb72 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Wed, 24 Jun 2026 11:21:17 -0500 Subject: [PATCH 09/12] feat(redis): log SSRF block at the create-time pre-check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connect-time rejection logs (RestrictedHostJedisSocketFactory), but the create-time pre-check (assertHostAllowed) — which catches the common case of a datasource pointed at a disallowed host — threw silently. Log it at WARN; the log framework tags the line with the requesting userEmail / orgId / traceId, so a cluster of these is a useful reconnaissance signal. No server-side trace otherwise, since the block is returned as a graceful HTTP 200 DatasourceTestResult. Co-Authored-By: Claude Opus 4.8 --- .../src/main/java/com/external/plugins/RedisPlugin.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 60406d94205f..6171719bb75a 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -315,6 +315,12 @@ private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) // RestrictedHostJedisSocketFactory re-validates with a single resolution at connect // time and is what actually closes the rebinding TOCTOU. See GHSA-qhfj-g87x-m39w. if (RestrictedHostFilter.isHostBlocked(host)) { + // Most blocks land here (a user pointing a datasource at a disallowed host), so log + // it as the primary observability signal — the log framework tags this with the + // requesting userEmail / orgId / traceId, which makes a spike a useful recon flag. + // The connect-time sibling (RestrictedHostJedisSocketFactory) logs the rarer + // rebinding case. + log.warn("Blocked a Redis datasource pointed at disallowed host '{}' (SSRF filter).", host); throw new AppsmithPluginException( AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); } From ad9359d24e6abaa7deff68b061ace36fd29b159c Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Thu, 25 Jun 2026 13:58:12 -0500 Subject: [PATCH 10/12] fix(security): seed the internal-Redis denylist from the Spring-resolved URL, not just the env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RestrictedHostFilter built its internal-Redis set only from the APPSMITH_REDIS_URL / APPSMITH_REDIS_GIT_URL environment variables at static init. But the app binds appsmith.redis.url via Spring, which can also resolve from application.properties, a -D system property, or a profile. An operator configuring Redis that way (no env var) left the internal-Redis set empty, so a datasource could be pointed at the in-cluster Redis — fail-open. (Loopback and cloud-metadata literals were still blocked, but an RFC1918 address or a service hostname was not.) RedisConfig now registers the resolved appsmith.redis.url / appsmith.redis.git.url with the filter at startup (@PostConstruct, before any datasource test), unioning with the env-derived seed so configuring through either channel blocks both. The filter is shared with the redisPlugin (appsmith-interfaces is provided-scope to plugins, so it's parent-loaded), so the pushed state is visible at the connection path. RestrictedHostFilterTest covers the new registration and union semantics. See GHSA-qhfj-g87x-m39w. Co-Authored-By: Claude Opus 4.8 --- .../appsmith/util/RestrictedHostFilter.java | 33 +++++++++++++++++-- .../util/RestrictedHostFilterTest.java | 26 +++++++++++++++ .../server/configurations/RedisConfig.java | 18 ++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java index d5ef2126213a..253144d9e3a5 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -170,7 +170,10 @@ private static Set computeInternalRedisHosts() { } private static void addInternalRedisHostFromEnv(Set hosts, String envVar) { - final String url = System.getenv(envVar); + addInternalRedisHostFromUrl(hosts, System.getenv(envVar), envVar); + } + + private static void addInternalRedisHostFromUrl(Set hosts, String url, String sourceLabel) { if (!StringUtils.hasText(url)) { return; } @@ -186,8 +189,34 @@ private static void addInternalRedisHostFromEnv(Set hosts, String envVar hosts.add(normalizeHostForComparisonQuietly(host)); } } catch (IllegalArgumentException e) { - log.warn("Could not parse {} as a URI; that internal Redis host won't be filtered.", envVar); + log.warn("Could not parse {} as a Redis URI; that internal Redis host won't be filtered.", sourceLabel); + } + } + + /** + * Registers internal Appsmith Redis hosts from their connection URLs as resolved by the + * application (Spring), not the process environment. The static initializer seeds + * {@link #internalRedisHosts} from the {@code APPSMITH_REDIS_URL} / {@code APPSMITH_REDIS_GIT_URL} + * environment variables, but the app actually binds {@code appsmith.redis.url} via Spring — which + * can also come from {@code application.properties}, a {@code -D} system property, or a profile. + * An operator who configures Redis through one of those (without the env var) would otherwise + * leave this set empty, and the internal-Redis block fails open (loopback / cloud-metadata + * literals are still blocked, but an in-cluster Redis on an RFC1918 address or service hostname + * would be reachable). The server calls this at startup with the resolved property values so the + * filter reflects the Redis the app truly uses, regardless of how it was configured. + * + *

Unions with — does not replace — the env-seeded set, so configuring Redis through both + * channels blocks both. Safe to call before/after the static seed; null/blank URLs are ignored. + */ + public static void registerInternalRedisHosts(String... redisUrls) { + if (redisUrls == null || redisUrls.length == 0) { + return; + } + final Set merged = new HashSet<>(internalRedisHosts); + for (String url : redisUrls) { + addInternalRedisHostFromUrl(merged, url, "configured Redis URL"); } + internalRedisHosts = merged.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(merged); } /** Visible for testing only. Production code never mutates the internal Redis hosts set. */ diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java index f0483c639db2..768d7a5a7fbd 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java @@ -256,6 +256,32 @@ public void isHostBlocked_blocksAllConfiguredInternalRedisHosts() { assertFalse(RestrictedHostFilter.isHostBlocked("redis.example.com")); } + @Test + public void registerInternalRedisHosts_blocksHostFromSpringResolvedUrl() { + // Mirrors the server's startup path (RedisConfig#registerInternalRedisHostsWithSsrfFilter): + // the app binds appsmith.redis.url via Spring, which the env-only static seed misses when + // Redis is configured via application.properties / -D. Hosts registered from the resolved + // URLs must be blocked, closing the fail-open gap. See GHSA-qhfj-g87x-m39w. + RestrictedHostFilter.setInternalRedisHostsForTesting(); // start empty (no env var set) + RestrictedHostFilter.registerInternalRedisHosts( + "redis://session-redis.svc.cluster.local:6379", "rediss://git-redis.svc.cluster.local:6380"); + assertTrue(RestrictedHostFilter.isHostBlocked("session-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("git-redis.svc.cluster.local")); + // Null / blank URLs are ignored and unrelated hosts stay allowed. + RestrictedHostFilter.registerInternalRedisHosts((String) null, "", " "); + assertFalse(RestrictedHostFilter.isHostBlocked("redis.example.com")); + } + + @Test + public void registerInternalRedisHosts_unionsWithExistingHosts() { + // Registration must not clobber the env-seeded set: configuring Redis through both the env + // var and a property must block both. + RestrictedHostFilter.setInternalRedisHostsForTesting("env-seeded-redis.svc.cluster.local"); + RestrictedHostFilter.registerInternalRedisHosts("redis://property-redis.svc.cluster.local:6379"); + assertTrue(RestrictedHostFilter.isHostBlocked("env-seeded-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("property-redis.svc.cluster.local")); + } + // ---------- alwaysAllowedHostsForTesting: opt-in test escape hatch ---------- @Test diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java index 6a8a6cc6c7b2..26f06496ab12 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java @@ -3,6 +3,7 @@ import com.appsmith.server.domains.LoginSource; import com.appsmith.server.dtos.OAuth2AuthorizedClientDTO; import com.appsmith.server.dtos.UserSessionDTO; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import io.lettuce.core.AbstractRedisClient; @@ -13,6 +14,7 @@ import io.lettuce.core.cluster.RedisClusterClient; import io.lettuce.core.resource.ClientResources; import io.micrometer.observation.ObservationRegistry; +import jakarta.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Value; @@ -59,6 +61,22 @@ public class RedisConfig { @Value("${appsmith.redis.url:}") private String redisURL; + @Value("${appsmith.redis.git.url:}") + private String redisGitURL; + + /** + * Teach the SSRF host filter which Redis the app is actually configured against, using the + * Spring-resolved property rather than only the {@code APPSMITH_REDIS_URL} env var the filter + * reads at static init. This closes a fail-open gap: an operator who sets {@code appsmith.redis.url} + * via application.properties or a {@code -D} system property (no env var) would otherwise leave + * the internal-Redis denylist empty, letting a datasource reach an in-cluster Redis. Runs once + * at startup, before any datasource can be tested. See GHSA-qhfj-g87x-m39w. + */ + @PostConstruct + public void registerInternalRedisHostsWithSsrfFilter() { + RestrictedHostFilter.registerInternalRedisHosts(redisURL, redisGitURL); + } + /** * This is the topic to which we will publish & subscribe to. We can have multiple topics based on the messages * that we wish to broadcast. Starting with a single one for now. From 9b056f3be8d7e13ad92d606cba86265d6367cf24 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Thu, 25 Jun 2026 14:10:34 -0500 Subject: [PATCH 11/12] fix(redis): fail over across all validated resolved addresses, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RestrictedHostJedisSocketFactory connected to only the first resolved address. For a dual-stack host whose leading record is an unreachable IPv6 (AAAA first, but Redis listens on IPv4 / the client has no IPv6 route), that dropped the connection even though a reachable IPv4 was in the resolved set — a resilience regression versus stock DefaultJedisSocketFactory, which tries each address in turn. Now iterate the resolved addresses and connect to the first reachable one. Every address was already validated by the single firstAllowedRedisAddress() call (present only if none is blocked), so failover does not widen the SSRF surface and the single-resolution guarantee is unchanged. Raised in PR review. Co-Authored-By: Claude Opus 4.8 --- .../RestrictedHostJedisSocketFactory.java | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java index e3121fa15473..bf743f8b493e 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -16,7 +16,6 @@ import java.net.InetSocketAddress; import java.net.Socket; import java.net.UnknownHostException; -import java.util.Optional; /** * A Jedis {@link JedisSocketFactory} that enforces Appsmith's SSRF host policy at the exact @@ -76,29 +75,24 @@ public Socket createSocket() throws JedisConnectionException { throw new JedisConnectionException("Failed to resolve Redis host '" + host + "'.", e); } - // Validate the addresses we just resolved and connect to one of them — the driver never - // re-resolves, so a rebinding resolver cannot swap in a different (internal) IP. - final Optional pinned = RestrictedHostFilter.firstAllowedRedisAddress(host, resolved); - if (!pinned.isPresent()) { + // Gate: firstAllowedRedisAddress returns present only when EVERY resolved address passed the + // filter, so a present result means all of `resolved` are safe to connect to. The driver + // never re-resolves, so a rebinding resolver cannot swap in a different (internal) IP. + if (RestrictedHostFilter.firstAllowedRedisAddress(host, resolved).isEmpty()) { // Connection-time SSRF rejection (incl. the DNS-rebinding case where the resolved // address differs from the create-time pre-check). Log it — this is the only place the // block is observable server-side, since the pre-check rejection happens elsewhere. log.warn("Refusing Redis connection: host '{}' resolved to a disallowed address.", host); throw new JedisConnectionException(RestrictedHostFilter.HOST_NOT_ALLOWED); } - final InetAddress pinnedAddress = pinned.get(); Socket socket = null; try { - socket = new Socket(); - // Socket options mirror DefaultJedisSocketFactory. - socket.setReuseAddress(true); - socket.setKeepAlive(true); - socket.setTcpNoDelay(true); - socket.setSoLinger(true, 0); - - // Connect to the validated IP literal — no second DNS lookup happens here. - socket.connect(new InetSocketAddress(pinnedAddress, port), connectionTimeout); + // Connect to the first reachable validated address. Trying all of them (not just the + // first) means a dual-stack host whose leading record is an unreachable IPv6 still falls + // back to its IPv4, matching DefaultJedisSocketFactory. Every candidate was validated + // above, so the failover does not widen the SSRF surface. No second DNS lookup happens. + socket = connectToFirstReachable(resolved); socket.setSoTimeout(socketTimeout); if (ssl) { @@ -140,6 +134,39 @@ public Socket createSocket() throws JedisConnectionException { } } + /** + * Opens a plain TCP socket to the first reachable address, trying each in order (mirrors + * {@code DefaultJedisSocketFactory#connectToFirstSuccessfulHost}). All addresses passed here + * have already cleared the SSRF filter, so iterating only adds connection resilience — e.g. a + * dual-stack host whose first record is an unreachable IPv6 still connects via its IPv4. Throws + * once every candidate has failed, with each failure attached as a suppressed exception. + */ + private Socket connectToFirstReachable(InetAddress[] addresses) throws JedisConnectionException { + JedisConnectionException failure = null; + for (InetAddress address : addresses) { + Socket candidate = null; + try { + candidate = new Socket(); + // Socket options mirror DefaultJedisSocketFactory. + candidate.setReuseAddress(true); + candidate.setKeepAlive(true); + candidate.setTcpNoDelay(true); + candidate.setSoLinger(true, 0); + candidate.connect(new InetSocketAddress(address, port), connectionTimeout); + return candidate; + } catch (Exception e) { + closeQuietly(candidate); + if (failure == null) { + failure = new JedisConnectionException("Failed to connect to Redis host '" + host + "'."); + } + failure.addSuppressed(e); + } + } + throw failure != null + ? failure + : new JedisConnectionException("Failed to connect to Redis host '" + host + "'."); + } + private static void closeQuietly(Socket socket) { if (socket != null) { try { From bfef59b71e8c987fc20dac0822fcc30982b989d5 Mon Sep 17 00:00:00 2001 From: Wyatt Walter Date: Thu, 25 Jun 2026 15:38:43 -0500 Subject: [PATCH 12/12] feat(redis): include the resolved IP(s) in the SSRF block logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both block log lines recorded only the host, so a hostname that resolved to loopback / cloud-metadata looked identical to one typed as an internal IP literally. Add the resolved address(es) for forensics: - Connection-time (RestrictedHostJedisSocketFactory) logs its own already- validated resolution — exact, no extra lookup. - Create-time pre-check (RedisPlugin) uses a new log-only helper, RestrictedHostFilter.describeResolvedAddresses, which does a best-effort lookup ("unresolved" when it can't resolve). It is point-in-time and not a security decision, since isHostBlocked only returns a boolean. Co-Authored-By: Claude Opus 4.8 --- .../appsmith/util/RestrictedHostFilter.java | 21 +++++++++++++++++++ .../util/RestrictedHostFilterTest.java | 8 +++++++ .../com/external/plugins/RedisPlugin.java | 5 ++++- .../RestrictedHostJedisSocketFactory.java | 10 ++++++++- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java index 253144d9e3a5..911d689537a7 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -444,6 +444,27 @@ public static Optional firstAllowedRedisAddress(String host, InetAd return Optional.of(resolvedAddresses[0]); } + /** + * Best-effort, log-only description of the addresses {@code host} currently resolves to, for + * annotating SSRF block log lines. Does its own lookup and is NOT a security decision — it only + * explains a block, e.g. distinguishing a host typed as an internal IP literally from a hostname + * that resolved to loopback / cloud-metadata. Returns {@code "unresolved"} when it can't be + * resolved (the block may have fired on a literal/denylist hostname). Because it re-resolves, a + * hostile flipping resolver could in theory report a different address than the one that + * triggered the block; the annotation is point-in-time and diagnostic only. + */ + public static String describeResolvedAddresses(String host) { + try { + return String.join( + ", ", + Arrays.stream(InetAddress.getAllByName(host)) + .map(InetAddress::getHostAddress) + .toList()); + } catch (UnknownHostException e) { + return "unresolved"; + } + } + /** * Literal/canonical-only block check — no DNS, no network I/O. Catches the static denylist * (cloud-metadata IPs), literal non-routable IP-class addresses, and a literal match diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java index 768d7a5a7fbd..5aed92284d25 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java @@ -282,6 +282,14 @@ public void registerInternalRedisHosts_unionsWithExistingHosts() { assertTrue(RestrictedHostFilter.isHostBlocked("property-redis.svc.cluster.local")); } + @Test + public void describeResolvedAddresses_reportsResolvedIpOrUnresolved() { + // Log-only annotation for the SSRF block lines. An IP literal resolves to itself; the + // reserved .invalid TLD never resolves (RFC 2606) so it reports "unresolved". + assertEquals("127.0.0.1", RestrictedHostFilter.describeResolvedAddresses("127.0.0.1")); + assertEquals("unresolved", RestrictedHostFilter.describeResolvedAddresses("definitely-not-a-host.invalid")); + } + // ---------- alwaysAllowedHostsForTesting: opt-in test escape hatch ---------- @Test diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 6171719bb75a..a383276da654 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -320,7 +320,10 @@ private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) // requesting userEmail / orgId / traceId, which makes a spike a useful recon flag. // The connect-time sibling (RestrictedHostJedisSocketFactory) logs the rarer // rebinding case. - log.warn("Blocked a Redis datasource pointed at disallowed host '{}' (SSRF filter).", host); + log.warn( + "Blocked a Redis datasource pointed at disallowed host '{}' (resolves to [{}]) — SSRF filter.", + host, + RestrictedHostFilter.describeResolvedAddresses(host)); throw new AppsmithPluginException( AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); } diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java index bf743f8b493e..63ed27d2c6c3 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -16,6 +16,7 @@ import java.net.InetSocketAddress; import java.net.Socket; import java.net.UnknownHostException; +import java.util.Arrays; /** * A Jedis {@link JedisSocketFactory} that enforces Appsmith's SSRF host policy at the exact @@ -82,7 +83,14 @@ public Socket createSocket() throws JedisConnectionException { // Connection-time SSRF rejection (incl. the DNS-rebinding case where the resolved // address differs from the create-time pre-check). Log it — this is the only place the // block is observable server-side, since the pre-check rejection happens elsewhere. - log.warn("Refusing Redis connection: host '{}' resolved to a disallowed address.", host); + log.warn( + "Refusing Redis connection: host '{}' resolved to a disallowed address [{}].", + host, + String.join( + ", ", + Arrays.stream(resolved) + .map(InetAddress::getHostAddress) + .toList())); throw new JedisConnectionException(RestrictedHostFilter.HOST_NOT_ALLOWED); }