fix(security): apply full non-routable IP-class filter on WebClient (GHSA-v49v-673j-g4vj, GHSA-m23h-pvf3-2m7p)#41849
Conversation
…GHSA-v49v-673j-g4vj, GHSA-m23h-pvf3-2m7p) Extends the WebClient/Netty filter check to the full non-routable set already enforced by resolveIfAllowed: full loopback range (127.0.0.0/8, ::1), any-local (0.0.0.0, ::), link-local (169.254/16, fe80::/10), multicast, and IPv6 Unique Local Addresses (fc00::/7). RFC 1918 site-local ranges remain allowed for legitimate REST API access to internal infrastructure. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughWebClientUtils extends its DNS resolver and request filtering guards to block non-routable IP address classes (loopback, any-local, link-local, multicast, IPv6 ULA ranges) when running inside Docker, alongside the existing denylist. New parameterized tests validate both blocked and allowed host patterns. ChangesDocker IP Address Class Blocking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java (1)
288-313: 💤 Low valueImplementation looks solid.
The bitmask check
(firstByte & 0xFE) == 0xFCcorrectly identifies the fc00::/7 range (ULA). Package-private visibility is appropriate for testing.Minor observation: the address classification logic (loopback, link-local, any-local, multicast, ULA checks) is duplicated between this method and
resolveIfAllowed(lines 236-247). Consider extracting a shared helper if these paths continue to evolve together.🤖 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/WebClientUtils.java` around lines 288 - 313, Duplicate address-classification logic exists in isBlockedIpAddressClass and resolveIfAllowed; extract the shared checks (loopback, any-local, link-local, multicast, IPv6 ULA fc00::/7) into a single helper (e.g., isBlockedAddressClass or isAddressInBlockedClass) and replace the duplicated blocks in both isBlockedIpAddressClass and resolveIfAllowed to call that helper; ensure the helper is static and package-private (or private static if only used in this class) and keep existing behavior for isBlockedAddressClassInDocker by calling the refactored isBlockedIpAddressClass which delegates to the new helper.
🤖 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.
Nitpick comments:
In
`@app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java`:
- Around line 288-313: Duplicate address-classification logic exists in
isBlockedIpAddressClass and resolveIfAllowed; extract the shared checks
(loopback, any-local, link-local, multicast, IPv6 ULA fc00::/7) into a single
helper (e.g., isBlockedAddressClass or isAddressInBlockedClass) and replace the
duplicated blocks in both isBlockedIpAddressClass and resolveIfAllowed to call
that helper; ensure the helper is static and package-private (or private static
if only used in this class) and keep existing behavior for
isBlockedAddressClassInDocker by calling the refactored isBlockedIpAddressClass
which delegates to the new helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85af20a9-5972-4816-afa8-824074abd0f2
📒 Files selected for processing (2)
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.javaapp/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java
Both resolveIfAllowed (SMTP path) and isBlockedIpAddressClass (REST path) had the same 4-way isXxxAddress check plus IPv6 ULA byte-mask inlined. Consolidate into one private helper so the two paths can't drift. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/26477545224. |
|
Deploy-Preview-URL: https://ce-41849.dp.appsmith.com |
Summary
Extend the
WebClientUtilshost filter (REST API / GraphQL / OAuth2 plugin paths) to reject the same set of non-routable IP address classes that the SMTP code path already rejects viaresolveIfAllowed: loopback (127.0.0.0/8,::1), any-local (0.0.0.0,::), link-local (169.254.0.0/16,fe80::/10), multicast, and IPv6 Unique Local Addresses (fc00::/7). RFC 1918 site-local ranges remain allowed (same intentional exception asresolveIfAllowed).Addresses:
Approach
requestFilterFnandisDisallowedAndFailnowORan additional check against the existing exact-matchDISALLOWED_HOSTSlookup: an IP-literal helper that returns true whenInetAddress.isLoopbackAddress(),isAnyLocalAddress(),isLinkLocalAddress(), orisMulticastAddress()is true, or when the address is IPv6 in thefc00::/7ULA range. The existing canonicalization (IPv4-mapped IPv6 literals normalized to IPv4) handles literal variants.Test plan
mvn -pl appsmith-interfaces test -Dtest=WebClientUtilsTest(48/48, +24 parameterized cases overisBlockedIpAddressClass).mvn -pl appsmith-interfaces spotless:checkclean.Host not allowedrather than attempting the outbound connection. RFC 1918 and external hosts continue to work.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/26515121230
Commit: 7c75fba
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 27 May 2026 15:22:15 UTC
Automation
/ok-to-test tags="@tag.All"