Conversation
The sendTestEmail endpoint accepted attacker-controlled smtpHost/smtpPort and passed them directly to JavaMailSenderImpl, bypassing the WebClient IP_CHECK_FILTER. This allowed admin users to reach internal services, cloud metadata endpoints, and private networks via raw SMTP connections. - Add WebClientUtils.validateHostNotDisallowed() that checks hostnames against the existing denylist and, after DNS resolution, rejects loopback, link-local, site-local, and multicast addresses - Apply host validation in sendTestEmail before connecting - Sanitize error messages to prevent information leakage via error-based port scanning (CWE-209) - Add tests for blocked and allowed hosts Fixes https://linear.app/appsmith/issue/APP-15034 Advisory: GHSA-vvxf-f8q9-86gh
Contributor
WalkthroughAdds WebClientUtils.resolveIfAllowed(host) to DNS-resolve and allow/deny-check outbound hosts; EnvManagerCEImpl.sendTestEmail enforces allowed SMTP ports, uses the resolver to fail fast for disallowed SMTP hosts, and standardizes SMTP error handling. New unit tests cover resolver and EnvManager behaviors. Changes
Sequence DiagramsequenceDiagram
participant Client
participant EnvManager as EnvManager\n(sendTestEmail)
participant PortCheck as PortValidation
participant Resolver as WebClientUtils\n(resolveIfAllowed)
participant DNS as DNS/InetAddress
participant SMTP as SMTP Server
Client->>EnvManager: sendTestEmail(smtpHost, smtpPort)
EnvManager->>PortCheck: validate port ∈ ALLOWED_SMTP_PORTS
alt port invalid
PortCheck-->>EnvManager: invalid
EnvManager-->>Client: AppsmithError.GENERIC_BAD_REQUEST\n("Invalid SMTP configuration.")
else port valid
EnvManager->>Resolver: resolveIfAllowed(smtpHost)
Resolver->>Resolver: canonicalize & check DISALLOWED_HOSTS
alt host pre-disallowed
Resolver-->>EnvManager: Optional.empty()
else not pre-disallowed
Resolver->>DNS: InetAddress.getAllByName(host)
DNS-->>Resolver: resolved addresses[]
Resolver->>Resolver: check addresses (ULA, loopback, link-local, any-local, multicast)
alt any address rejected
Resolver-->>EnvManager: Optional.empty()
else all addresses allowed
Resolver-->>EnvManager: Optional.of(resolved[0])
end
end
alt resolution failed
EnvManager-->>Client: AppsmithError.GENERIC_BAD_REQUEST\n("Invalid SMTP configuration.")
else resolution passed
EnvManager->>SMTP: connect/send using resolved host
SMTP-->>EnvManager: success or error
alt error
EnvManager-->>Client: AppsmithError.GENERIC_BAD_REQUEST\n(SMTP_GENERIC_ERROR)
else
EnvManager-->>Client: success
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
subrata71
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java (1)
79-84: Avoid live DNS dependency in this unit test.Line 82 performs real DNS resolution for external domains, which can make the test flaky in restricted CI environments. Prefer a deterministic resolver abstraction (mocked in tests) or move live-resolution checks to an integration-test suite.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java` around lines 79 - 84, The test validateHostNotDisallowed_allowsLegitimateSmtpHosts calls WebClientUtils.validateHostNotDisallowed which triggers real DNS resolution making the unit test flaky; modify the code under test to accept a DNS/resolver abstraction (or provide an overloaded method) and update this test to inject a deterministic mocked resolver that returns expected answers, or alternatively replace external hosts with deterministic local test hosts (e.g., localhost or a controlled test domain) and assert against those; locate the WebClientUtils.validateHostNotDisallowed method and the test validateHostNotDisallowed_allowsLegitimateSmtpHosts to implement the resolver injection and update the test to mock/stub resolution so no live DNS lookup occurs.app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java (1)
404-450: Consider parameterizing the blocked-host tests.Line 404-Line 450 repeats the same pattern 4 times. A single
@ParameterizedTestwith host values would keep this easier to extend.♻️ Refactor sketch
+ `@ParameterizedTest` + `@ValueSource`(strings = {"127.0.0.1", "169.254.169.254", "10.0.0.1", "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 host"); + }) + .verify(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java` around lines 404 - 450, Replace the four near-duplicate tests in EnvManagerTest with a single parameterized test: create a new `@ParameterizedTest` method (e.g., sendTestEmail_WhenBlockedHost_ThrowsException) that calls mockSuperUser(), then uses StepVerifier.create(envManager.sendTestEmail(buildDto(host))) and the same expectErrorSatisfies assertions; supply the blocked host values ("127.0.0.1", "169.254.169.254", "10.0.0.1", "localhost") via `@ValueSource` or `@CsvSource` so the logic in sendTestEmail and buildDto is exercised once for each host. Ensure imports for the JUnit Jupiter parameterized annotations are added and remove the four original test methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java`:
- Around line 231-236: The validation in WebClientUtils currently relies on
InetAddress methods which miss IPv6 Unique Local Addresses (fc00::/7); update
the block in the method that checks addr (the addr variable and HOST_NOT_ALLOWED
return) to explicitly detect IPv6 ULA by: if addr is an instance of
java.net.Inet6Address, call addr.getAddress() and test the first byte with
((bytes[0] & 0xFE) == 0xFC) (this matches fc00::/7), and if true return
Optional.of(HOST_NOT_ALLOWED); keep the existing
loopback/link-local/site-local/any-local/multicast checks intact.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java`:
- Around line 793-801: The current validation via
WebClientUtils.validateHostNotDisallowed(requestDTO.getSmtpHost()) can be
bypassed by DNS rebinding because JavaMailSenderImpl (mailSender) reconnects
using the original hostname; fix by performing a single trusted DNS resolution
during validation and using that resolved InetAddress for the actual socket
connect (or supply a custom SocketFactory/SSLSocketFactory to JavaMailSenderImpl
that enforces/connects to the validated InetAddress) so the mail connection
cannot re-resolve to a different IP at connect time; update EnvManagerCEImpl to
use the resolved address returned from validateHostNotDisallowed (or a new
resolver helper) when configuring mailSender and ensure the SMTP EHLO/Host
header is set appropriately if SMTP requires the original hostname.
---
Nitpick comments:
In
`@app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java`:
- Around line 79-84: The test
validateHostNotDisallowed_allowsLegitimateSmtpHosts calls
WebClientUtils.validateHostNotDisallowed which triggers real DNS resolution
making the unit test flaky; modify the code under test to accept a DNS/resolver
abstraction (or provide an overloaded method) and update this test to inject a
deterministic mocked resolver that returns expected answers, or alternatively
replace external hosts with deterministic local test hosts (e.g., localhost or a
controlled test domain) and assert against those; locate the
WebClientUtils.validateHostNotDisallowed method and the test
validateHostNotDisallowed_allowsLegitimateSmtpHosts to implement the resolver
injection and update the test to mock/stub resolution so no live DNS lookup
occurs.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java`:
- Around line 404-450: Replace the four near-duplicate tests in EnvManagerTest
with a single parameterized test: create a new `@ParameterizedTest` method (e.g.,
sendTestEmail_WhenBlockedHost_ThrowsException) that calls mockSuperUser(), then
uses StepVerifier.create(envManager.sendTestEmail(buildDto(host))) and the same
expectErrorSatisfies assertions; supply the blocked host values ("127.0.0.1",
"169.254.169.254", "10.0.0.1", "localhost") via `@ValueSource` or `@CsvSource` so
the logic in sendTestEmail and buildDto is exercised once for each host. Ensure
imports for the JUnit Jupiter parameterized annotations are added and remove the
four original test methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3abcb242-ae73-41d6-9588-ab5160e4e72a
📒 Files selected for processing (4)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.javaapp/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java
Address CodeRabbitAI review findings and Cypress test regression: 1. DNS rebinding TOCTOU (Critical): Rename validateHostNotDisallowed to resolveIfAllowed, returning Optional<InetAddress>. The caller now uses the resolved IP directly for mailSender.setHost(), eliminating the window between validation and connection where DNS could rebind. 2. IPv6 ULA gap (Major): Java's isSiteLocalAddress() only checks the deprecated fec0::/10 prefix, not fc00::/7 (Unique Local Addresses). Added explicit byte-level check: (firstByte & 0xFE) == 0xFC. 3. Cypress test failure: Removed isSiteLocalAddress() from the blocking criteria. The existing WebClient denylist intentionally allows RFC 1918 (10/8, 172.16/12, 192.168/16) because legitimate SMTP servers reside on private networks. host.docker.internal (used in CI) resolves to a site-local address and was incorrectly blocked. 4. Test cleanup: Parameterized duplicate EnvManager blocked-host tests into a single @ParameterizedTest. Updated WebClientUtilsTest for new API signature and added resolved-address assertion.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java (1)
406-417: Good SSRF regression coverage; consider expanding test vectors.The parameterized test covers the critical SSRF vectors (loopback, AWS metadata IP, localhost). The assertion correctly uses
contains()which handles the trailing period in the production message.Consider adding
0.0.0.0(any-local),168.63.129.16(Azure metadata), and100.100.100.200(Alibaba metadata) to align with the broader coverage inWebClientUtilsTest. This strengthens the integration-level guarantee.♻️ Suggested expanded test vectors
`@ParameterizedTest` -@ValueSource(strings = {"127.0.0.1", "169.254.169.254", "localhost"}) +@ValueSource(strings = {"127.0.0.1", "169.254.169.254", "localhost", "0.0.0.0", "168.63.129.16", "100.100.100.200"}) public void sendTestEmail_WhenBlockedHost_ThrowsException(String host) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java` around lines 406 - 417, The parameterized test sendTestEmail_WhenBlockedHost_ThrowsException currently only checks {"127.0.0.1", "169.254.169.254", "localhost"} and should be expanded to include additional metadata/any-local hosts to improve SSRF coverage; update the `@ValueSource` on the test (and keep assertions on envManager.sendTestEmail and buildDto unchanged) to add "0.0.0.0", "168.63.129.16", and "100.100.100.200" so the test covers Azure, Alibaba, and any-local vectors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java`:
- Around line 90-95: The test resolveIfAllowed_returnsResolvedAddress assumes
IPv4; update it so WebClientUtils.resolveIfAllowed("smtp.gmail.com") is asserted
present and the returned InetAddress (result.get().getHostAddress()) is
validated for either IPv4 or IPv6—for example keep the existing IPv4 regex OR
accept addresses containing ':' (IPv6) or use a standard InetAddress validation
method—so the assertion passes regardless of whether the resolver returns IPv4
or IPv6.
---
Nitpick comments:
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java`:
- Around line 406-417: The parameterized test
sendTestEmail_WhenBlockedHost_ThrowsException currently only checks
{"127.0.0.1", "169.254.169.254", "localhost"} and should be expanded to include
additional metadata/any-local hosts to improve SSRF coverage; update the
`@ValueSource` on the test (and keep assertions on envManager.sendTestEmail and
buildDto unchanged) to add "0.0.0.0", "168.63.129.16", and "100.100.100.200" so
the test covers Azure, Alibaba, and any-local vectors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8a5e2d63-b5c2-42eb-9345-11184c43d151
📒 Files selected for processing (4)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.javaapp/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java
- app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java
Comment on lines
+90
to
+95
| @Test | ||
| public void resolveIfAllowed_returnsResolvedAddress() { | ||
| Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com"); | ||
| assertTrue(result.isPresent()); | ||
| assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+")); | ||
| } |
Contributor
There was a problem hiding this comment.
IPv4-only regex may fail for IPv6-capable resolvers.
The regex \\d+\\.\\d+\\.\\d+\\.\\d+ assumes IPv4. If smtp.gmail.com resolves to an IPv6 address (Google supports both), this assertion fails.
🛡️ Proposed fix to handle both IPv4 and IPv6
`@Test`
public void resolveIfAllowed_returnsResolvedAddress() {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com");
assertTrue(result.isPresent());
- assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+"));
+ // Just verify we got a valid address string - could be IPv4 or IPv6
+ String hostAddress = result.get().getHostAddress();
+ assertTrue(hostAddress != null && !hostAddress.isEmpty(), "Expected non-empty host address");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java`
around lines 90 - 95, The test resolveIfAllowed_returnsResolvedAddress assumes
IPv4; update it so WebClientUtils.resolveIfAllowed("smtp.gmail.com") is asserted
present and the returned InetAddress (result.get().getHostAddress()) is
validated for either IPv4 or IPv6—for example keep the existing IPv4 regex OR
accept addresses containing ':' (IPv6) or use a standard InetAddress validation
method—so the assertion passes regardless of whether the resolver returns IPv4
or IPv6.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai Should this be addressed with a code change, or does it only require updating the assertion?
Contributor
There was a problem hiding this comment.
@subrata71 This is only an assertion update — the production code in resolveIfAllowed() is already correct. The fix is just relaxing the test assertion so it passes whether the resolver returns an IPv4 or IPv6 address:
`@Test`
public void resolveIfAllowed_returnsResolvedAddress() {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com");
assertTrue(result.isPresent());
String hostAddress = result.get().getHostAddress();
assertTrue(hostAddress != null && !hostAddress.isEmpty(), "Expected non-empty host address");
}No production code changes needed.
Collaborator
Author
There was a problem hiding this comment.
Sounds good. In that case it's safe to ignore this concern. @coderabbitai
Contributor
There was a problem hiding this comment.
@subrata71, understood — acknowledged and moving on. The production logic is sound either way, and the test will pass in the vast majority of CI environments. 👍
…-email-javamail-bypass
Restrict the send-test-email endpoint to standard SMTP ports (25, 465, 587, 2525) by default. This prevents abuse of the endpoint for probing non-SMTP services on otherwise-allowed hosts. Operators can extend the allowlist via the APPSMITH_MAIL_ALLOWED_PORTS environment variable (comma-separated port numbers) for deployments that use non-standard SMTP ports. Port validation runs before host resolution (fail fast). Error messages use the same generic text as host validation to avoid leaking which ports are permitted.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java`:
- Around line 787-789: DEFAULT_SMTP_PORTS includes 465 which requires implicit
TLS (SMTPS) but the SMTP config in EnvManagerCEImpl currently only sets
STARTTLS; update the SMTP configuration logic (the code that builds SMTP
properties around ALLOWED_SMTP_PORTS / DEFAULT_SMTP_PORTS) to detect when port
465 is selected and set mail.smtp.ssl.enable=true and
mail.smtp.starttls.enable=false for that case, otherwise keep the existing
STARTTLS behavior; alternatively remove 465 from DEFAULT_SMTP_PORTS if you
prefer not to support implicit TLS—implement the conditional so
ALLOWED_SMTP_PORTS/DEFAULT_SMTP_PORTS usage is consistent with the chosen
behavior.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java`:
- Around line 423-434: The test currently uses hard-coded positive ports which
may be whitelisted via APPSMITH_MAIL_ALLOWED_PORTS and make the test flaky;
update sendTestEmail_WhenDisallowedPort_ThrowsException to avoid
environment-dependent ports by either (a) restrict the `@ValueSource` to only
values that can never be whitelisted (e.g., 0 and -1) so the validation branch
is always exercised, or (b) compute a disallowed port at runtime by reading the
configured allowlist (APPSMITH_MAIL_ALLOWED_PORTS) and selecting a port not in
that set, then call envManager.sendTestEmail(buildDto("smtp.gmail.com",
chosenPort)) within the test; adjust assertions around sendTestEmail
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5849f8f1-35bf-4c45-89be-1ed0c2880dfc
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java
Comment on lines
+423
to
+434
| @ParameterizedTest | ||
| @ValueSource(ints = {80, 443, 6379, 8080, 27017, 0, -1}) | ||
| public void sendTestEmail_WhenDisallowedPort_ThrowsException(int port) { | ||
| mockSuperUser(); | ||
|
|
||
| StepVerifier.create(envManager.sendTestEmail(buildDto("smtp.gmail.com", port))) | ||
| .expectErrorSatisfies(e -> { | ||
| assertThat(e).isInstanceOf(AppsmithException.class); | ||
| assertThat(e.getMessage()).contains("Invalid SMTP configuration"); | ||
| }) | ||
| .verify(); | ||
| } |
Contributor
There was a problem hiding this comment.
This port matrix is environment-dependent.
APPSMITH_MAIL_ALLOWED_PORTS can whitelist any positive port, so hard-coding 80/443/6379/8080/27017 here makes the test depend on the runner’s environment. If one of those ports is allowed, this no longer exercises the validation branch and can fall through into DNS/network behavior instead. Please derive the negative cases from the runtime allowlist, or restrict this test to values that can never be whitelisted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java`
around lines 423 - 434, The test currently uses hard-coded positive ports which
may be whitelisted via APPSMITH_MAIL_ALLOWED_PORTS and make the test flaky;
update sendTestEmail_WhenDisallowedPort_ThrowsException to avoid
environment-dependent ports by either (a) restrict the `@ValueSource` to only
values that can never be whitelisted (e.g., 0 and -1) so the validation branch
is always exercised, or (b) compute a disallowed port at runtime by reading the
configured allowlist (APPSMITH_MAIL_ALLOWED_PORTS) and selecting a port not in
that set, then call envManager.sendTestEmail(buildDto("smtp.gmail.com",
chosenPort)) within the test; adjust assertions around sendTestEmail
accordingly.
Port 465 uses SMTPS (implicit TLS per RFC 8314), not STARTTLS. When the configured port is 465, set mail.smtp.ssl.enable=true and disable STARTTLS so JavaMail connects with immediate TLS as the protocol requires.
wyattwalter
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
TL;DR: The
POST /api/v1/admin/send-test-emailendpoint was vulnerable to SSRF (CWE-918) and error-based information disclosure (CWE-209). Attacker-controlledsmtpHost/smtpPortwere passed directly to JavaMail, bypassing theWebClientUtils.IP_CHECK_FILTERthat only protects HTTP requests. This fix adds host validation and sanitizes error messages.Root Cause
EnvManagerCEImpl.sendTestEmail()accepted user-controlled SMTP host/port and established raw TCP connections viaJavaMailSenderImplwithout any IP validation. The existingWebClientUtils.IP_CHECK_FILTERonly applies to Spring WebClient HTTP requests — a completely separate code path from JavaMail SMTP.Changes
WebClientUtils.validateHostNotDisallowed()— new reusable method that checks a hostname against the existing cloud-metadata denylist, resolves it via DNS, and rejects loopback, link-local, site-local, any-local, and multicast addresses.EnvManagerCEImpl.sendTestEmail()— calls the new validation before connecting. Error messages fromMessagingException/MailExceptionare no longer returned verbatim to the caller, preventing error-based port scanning.WebClientUtilsTestfor blocked hosts (private ranges, metadata IPs, localhost, unresolvable) and allowed hosts (legitimate SMTP servers). Added SSRF regression tests inEnvManagerTest.Fixes https://linear.app/appsmith/issue/APP-15034/ssrf-via-post-apiv1adminsend-test-email-javamail-bypasses-webclient-ip
Advisory: GHSA-vvxf-f8q9-86gh
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/23764710161
Commit: ead9025
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 30 Mar 2026 21:09:27 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Tests