Skip to content

fix: use hardened WebClientUtils builder in AI and Google Sheets plugins - #42177

Merged
wyattwalter merged 1 commit into
releasefrom
feature/app-15866
Sep 11, 2026
Merged

fix: use hardened WebClientUtils builder in AI and Google Sheets plugins#42177
wyattwalter merged 1 commit into
releasefrom
feature/app-15866

Conversation

@wyattwalter

@wyattwalter wyattwalter commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Description

The Anthropic, OpenAI, and Google AI plugins, and the Google Sheets trigger path, built their outbound HTTP clients with raw WebClient.builder(), skipping the SSRF host filter that WebClientUtils.builder() wires in for every other outbound client (the Google Sheets execute path in the same file already used the hardened builder).

All request targets in these plugins are fixed provider endpoints, so this is consistency hardening rather than a fix for reachable behavior: it ensures a future configurable endpoint (custom base URL, proxy, OpenAI-compatible gateway) cannot ship without the filter.

Each affected RequestUtils gains a regression test asserting that a request to a disallowed host is rejected by the filter with UnknownHostException: Host not allowed.. The tests were verified to fail against the previous builder wiring (the unfiltered client attempts a real connection instead).

Call sites checked (all raw WebClient.builder() usages in main code)

Changed:

  • anthropicPlugin RequestUtils.createWebClient()
  • openAiPlugin RequestUtils.createWebClient()
  • googleAiPlugin RequestUtils.createWebClient()
  • googleSheetsPlugin triggerWithFlags()

Deliberately excluded:

  • RTSCallerCEImpl — its only target is the local RTS process, which the filter blocks by design (documented in the source).
  • appsmithAiPlugin RequestUtils — shares one client between external requests and, in some deployments, the local RTS process; filtering it requires a per-target client split and is out of scope here.

Impact on existing instances

  • Fresh install: no change; all affected request targets are fixed external provider hosts, which the filter allows.
  • Upgrade from default: no change, same reasoning.
  • Upgrade from customized: none of the affected endpoints are configurable, so there is no customized state to affect. Operators who set APPSMITH_DISABLE_SSRF_FILTER=true get the previous unfiltered behavior everywhere, including these clients.
  • Rollback: reverts to the unfiltered clients; no persisted state involved.

Linear: https://linear.app/appsmith/issue/APP-15866

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/33426610112
Commit: 57410c6
Cypress dashboard.
Tags: @tag.All
Spec:


Mon, 31 Aug 2026 19:43:23 UTC

Summary by CodeRabbit

  • Security

    • Improved protection against requests to restricted cloud metadata endpoints across supported AI integrations.
    • Added validation before creating outbound connections for Google Sheets actions.
  • Reliability

    • Standardized outbound HTTP client setup across Anthropic, Google AI, OpenAI, and Google Sheets integrations.
  • Tests

    • Added coverage confirming that requests to disallowed metadata hosts are rejected.

The Anthropic, OpenAI, and Google AI plugins, and the Google Sheets
trigger path, built their outbound HTTP clients with raw
WebClient.builder(), skipping the SSRF host filter that
WebClientUtils.builder() wires in for every other outbound client.
All request targets in these plugins are fixed provider endpoints, so
this is consistency hardening rather than a fix for reachable
behavior; it ensures a future configurable endpoint cannot ship
without the filter.

Each affected RequestUtils gains a regression test asserting that a
request to a disallowed host is rejected by the filter. The tests were
verified to fail against the previous builder wiring.

Deliberately excluded:
- RTSCallerCEImpl: its only target is the local RTS process, which the
  filter blocks by design (documented in the source).
- appsmithAiPlugin: shares one client between external requests and,
  in some deployments, the local RTS process; filtering it requires a
  per-target client split and is out of scope here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 31, 2026

Copy link
Copy Markdown

APP-15866

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b2082a8a-cae4-4973-a771-b24a04e06ace

📥 Commits

Reviewing files that changed from the base of the PR and between 75847cf and 57410c6.

📒 Files selected for processing (7)
  • app/server/appsmith-plugins/anthropicPlugin/src/main/java/com/external/plugins/utils/RequestUtils.java
  • app/server/appsmith-plugins/anthropicPlugin/src/test/java/com/external/plugins/utils/RequestUtilsSsrfFilterTest.java
  • app/server/appsmith-plugins/googleAiPlugin/src/main/java/com/external/plugins/utils/RequestUtils.java
  • app/server/appsmith-plugins/googleAiPlugin/src/test/java/com/external/plugins/utils/RequestUtilsSsrfFilterTest.java
  • app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java
  • app/server/appsmith-plugins/openAiPlugin/src/main/java/com/external/plugins/utils/RequestUtils.java
  • app/server/appsmith-plugins/openAiPlugin/src/test/java/com/external/plugins/utils/RequestUtilsSsrfFilterTest.java

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

Three AI plugin clients now use WebClientUtils for HTTP client creation. New tests verify SSRF rejection for metadata-host requests. Google Sheets validates trigger requests before creating its client.

Changes

Plugin HTTP client security

Layer / File(s) Summary
Shared WebClient construction and SSRF tests
app/server/appsmith-plugins/{anthropicPlugin,googleAiPlugin,openAiPlugin}/src/...
The three RequestUtils implementations use WebClientUtils.builder(connectionProvider()). Each plugin adds a test that rejects requests to 169.254.169.254 with UnknownHostException and RestrictedHostFilter.HOST_NOT_ALLOWED.
Google Sheets trigger validation
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java
triggerWithFlags validates the request before client initialization and creates the client through WebClientUtils.builder().

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 57410

This PR hardens selected outbound clients with the existing host-filtered builder and adds targeted regression coverage without changing configured provider behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: amelia-c0n, sebastianiv21, sondermanish, subrata71, tomjose92

Poem

Shared builders guide the stream,
Metadata hosts are stopped mid-dream,
Tests guard each plugin’s door,
Sheets checks first, then asks for more,
Safe requests now flow downstream.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: replacing raw WebClient construction with the hardened WebClientUtils builder in the affected plugins.
Description check ✅ Passed The description explains the motivation, affected call sites, exclusions, impact, issue reference, and test results. It does not include the template's separate Testing and Communication sections or e…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the motivation, affected call sites, exclusions, impact, issue reference, and test results. It does not include the template's separate Testing and Communication sections or explicit validation checkbox selections, but the required information is mostly present.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/app-15866

Comment @coderabbitai help to get the list of available commands.

@wyattwalter wyattwalter added the ok-to-test Required label for CI label Aug 31, 2026
@github-actions github-actions Bot added the Bug Something isn't working label Aug 31, 2026
@wyattwalter
wyattwalter marked this pull request as ready for review August 31, 2026 19:44
@wyattwalter
wyattwalter requested a review from a team as a code owner August 31, 2026 19:44

@subrata71 subrata71 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Are there any other plugins in the EE repo that might need similar adjustments?

@wyattwalter

wyattwalter commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

EE-side sweep for the same pattern: one real site, fixed in the EE repo (see linked PR appsmithorg/appsmith-ee#9666)

Copy link
Copy Markdown
Collaborator

@wyatt What's remaining here?

@wyattwalter
wyattwalter merged commit 179453b into release Sep 11, 2026
100 checks passed
@wyattwalter
wyattwalter deleted the feature/app-15866 branch September 11, 2026 13:16
@wyattwalter

Copy link
Copy Markdown
Contributor Author

The EE side change was not approved yet. It was and now both are merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants