fix(security): restrict datasource access to the instance's own network address#41985
Open
wyattwalter wants to merge 3 commits into
Open
fix(security): restrict datasource access to the instance's own network address#41985wyattwalter wants to merge 3 commits into
wyattwalter wants to merge 3 commits into
Conversation
…address (SSRF defense-in-depth) The SSRF host filter (RestrictedHostFilter) intentionally allows RFC 1918 / site-local addresses so that legitimate customer datasources on private networks keep working. The Appsmith instance's own routable address is always in that range (Docker bridge 172.17.x, k8s pod 10.x, etc.), so a user-defined datasource could point at the instance itself. Add a defense-in-depth own-host block that mirrors the existing internal-Redis mechanism: register the instance's own hostname(s) at startup (from InetAddress.getLocalHost() and the HOSTNAME env var), resolve them live on every check, and block any datasource whose host is the instance's own hostname or resolves to its own IP. This blocks the instance's own address whether reached by the container hostname, the raw own IP, or the IPv4-mapped IPv6 form, while leaving the rest of the private network reachable. Honors the APPSMITH_DISABLE_SSRF_FILTER kill-switch and the test allowlist like the existing paths. Wired into RedisConfig at startup alongside the internal-Redis registration. Adds RestrictedHostFilterTest coverage for the own-IP block (raw IPv4, IPv4-mapped IPv6, hostname), the no-over-block guarantee for other RFC 1918 addresses, and the kill-switch. Co-Authored-By: Claude <noreply@anthropic.com>
…search resolver paths Follow-up hardening for the own-host/own-IP defense-in-depth block. The initial change enforced it only on the datasource-save (isHostBlocked) and Redis (firstAllowedRedisAddress) paths; the REST/GraphQL/SaaS (WebClient) and Elasticsearch datasource requests resolve through isLiteralBlocked and isDisallowedAndFail, which did not consult the own-host set, so those datasources could still reach the instance's own routable address. Changes: - isLiteralBlocked now blocks a URL that literally names the instance's own host (pre-resolver fast path, no DNS), matching how it already handles internal Redis. - isDisallowedAndFail — the shared address-level policy for both the WebClient (Netty) and Elasticsearch resolver hooks — now applies the own-host literal and own-IP overlap via a shared matchesOwnHost helper, working on the already-resolved address the hook hands it (no user-host re-resolution, so no DNS-rebinding TOCTOU). - Comments corrected to state coverage is best-effort for the address(es) the registered own hostname(s) resolve to, not full network-interface enumeration; a multi-homed container's secondary-interface IPs are out of scope by design. - Tests: added a test-only seam (setOwnResolvedIpsForTesting) so own-IP overlap is exercised deterministically without live DNS, removing the previous public-DNS dependency. Added coverage proving a resolved own IP is rejected via isLiteralBlocked, isDisallowedAndFail, and firstAllowedRedisAddress — not only isHostBlocked. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/29275340682. |
Contributor
Walkthrough
ChangesOwn-host SSRF protection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Datasource
participant RestrictedHostFilter
participant DNS
Datasource->>RestrictedHostFilter: submit target host
RestrictedHostFilter->>DNS: resolve target
DNS-->>RestrictedHostFilter: canonical host and addresses
RestrictedHostFilter->>RestrictedHostFilter: compare own hostname and IP overlaps
RestrictedHostFilter-->>Datasource: allow or block target
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![hacktron-app[bot]](https://avatars.githubusercontent.com/in/1250513?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java (1)
663-682: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winAvoid live DNS on the resolver path here.
matchesOwnHost(canonicalHost)callsresolveOwnIps(), which doesInetAddress.getAllByName(...)on every resolver check (pre-DNS and again for each resolved address indoResolveAll). Cache the own-IP set outside this path, or resolve it once per request and reuse it; a slow or unresolvableHOSTNAMEcan block outbound requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java` around lines 663 - 682, The resolver path in isDisallowedAndFail must not perform live DNS through matchesOwnHost on every check. Cache the own-IP set outside this method, or resolve it once per request and reuse it across pre-DNS and resolved-address checks, while preserving the existing disallowed-host and blocked-IP checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java`:
- Around line 663-682: The resolver path in isDisallowedAndFail must not perform
live DNS through matchesOwnHost on every check. Cache the own-IP set outside
this method, or resolve it once per request and reuse it across pre-DNS and
resolved-address checks, while preserving the existing disallowed-host and
blocked-IP checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f53cd626-2a38-4142-8d3e-3cde6b6c1300
📒 Files selected for processing (3)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.javaapp/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java
…er check does no DNS The own-IP overlap check runs inside isDisallowedAndFail, which the WebClient (Netty) and Elasticsearch resolver hooks invoke on their reactive EventLoop threads. The previous implementation resolved the instance's own hostname(s) live on every check (matchesOwnHost -> resolveOwnIps -> InetAddress.getAllByName), putting a synchronous DNS lookup on that hot path. Resolve the own hostname(s) to IPs once, off the hot path, and cache them in a volatile ownResolvedIps set: at static init and whenever registerOwnHost runs (the server calls that at startup, before any datasource is served). The enforcement checks — matchesOwnHost on the resolver EventLoop and isAnyResolvedAddressBlocked on the datasource-save / Redis paths — are now pure set-membership with no DNS. The own IP is effectively static for the process lifetime (a re-IP means a restart, which re-seeds the cache), so a startup resolve is sufficient and no periodic refresh is needed. Internal-Redis resolution is unchanged — it runs only on blocking-tolerant paths, never the EventLoop. The setOwnResolvedIpsForTesting seam now seeds the cache directly. Added tests proving the hot path reads the cache and performs no resolution, and that registerOwnHost recomputes the cache off the hot path. Co-Authored-By: Claude <noreply@anthropic.com>
|
Deploy-Preview-URL: https://ce-41985.dp.appsmith.com |
Contributor
Author
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/29277637331. |
|
Deploy-Preview-URL: https://ce-41985.dp.appsmith.com |
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
1 participant
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.
What
Adds a defense-in-depth SSRF protection so a user-defined datasource cannot connect
to the Appsmith instance's own network address.
Why
The SSRF host filter (
RestrictedHostFilter) intentionally allows private-network(RFC 1918 / site-local) addresses so that legitimate customer datasources on private
networks keep working. In a typical Docker/Kubernetes deployment the instance's own
routable address is itself a private/site-local address, so it falls inside that
allowed range. As a hardening measure, this change blocks that one target — the
instance's own address — without restricting the rest of the private network.
How
HOSTNAMEenvironment variable) and resolves them to their current address(es),mirroring the existing internal-Redis host registration. Resolution is live per check,
so an instance whose address changes stays covered with no cached-address bookkeeping.
the instance's own addresses. Enforced consistently across the datasource egress paths
— WebClient (REST/GraphQL/SaaS), Elasticsearch, Redis, and datasource validation —
evaluating the address already resolved by each path, so no additional lookup of the
user-supplied host is introduced.
continue to work unchanged.
(the primary, hostname-mapped address), not a full network-interface enumeration.
Testing
(raw IPv4 literal, IPv4-mapped IPv6, and hostname), confirm other private-network
addresses stay allowed, and confirm the existing SSRF kill-switch still applies.
Own-address resolution is injected in tests, so the suite has no external DNS
dependency.
appsmith-interfacesmodule tests pass.Summary by CodeRabbit
Warning
Tests have not run on the HEAD 1d3d64d yet
Mon, 13 Jul 2026 19:13:41 UTC