fix(ai): disable redirects on worker AI provider client (GHSA-5q4v)

[rubenfiszel]

Summary

Fixes the SSRF advisory GHSA-5q4v-c4v3-v7wr. A low-privileged AI-provider resource writer could bypass private-endpoint SSRF protection because the worker AI request path followed HTTP redirects into private hosts.

SSRF validation on a provider base_url is single-shot (AIProvider::get_base_url → validate_url_for_ssrf), but the worker then issued the request with the shared windmill_common::utils::HTTP_CLIENT, which sets no redirect policy. reqwest's default follows up to 10 redirects without revalidating each hop, so a public-looking base_url that 3xx-redirects to 127.0.0.1 / 169.254.169.254 (cloud metadata) got followed into the internal host, forwarding attacker-chosen headers. ALLOW_PRIVATE_AI_BASE_URLS being off did not help.

The API proxy was already hardened in #9370 (windmill-api/src/ai.rs uses reqwest::redirect::Policy::none()); the worker path was missed. This PR mirrors that fix.

Changes

• Add a dedicated AI_HTTP_CLIENT lazy_static in backend/windmill-ai/src/utils.rs, built via configure_client(...).redirect(reqwest::redirect::Policy::none()) (also preserves FORCE_IPV4). Redirects are disabled unconditionally, which holds even for ALLOW_PRIVATE_AI_BASE_URLS deployments since AI APIs respond directly.
• backend/windmill-worker/src/ai_executor.rs build_http_request — the only worker call targeting the user-controlled provider endpoint — now uses AI_HTTP_CLIENT instead of the redirect-following HTTP_CLIENT.
• Add a Tokio regression test that stands up a loopback listener returning a 302 to the link-local metadata address and asserts the client surfaces the 302 rather than following it.

Left unchanged (verified internal-only)

• ai_executor.rs:397 — Hub-script fetch against Windmill's own hub, not the user base_url.
• ai/utils.rs OAuth token refresh — targets windmill_common::BASE_URL (internal).
• handle_google_ai_chat_proxy / handle_google_ai_models_proxy take a client param and are only called from windmill-api/src/ai.rs with the already-fixed API client. The worker's Google path goes through build_http_request, so it is covered. Bedrock uses the AWS SDK (region-scoped endpoints), a separate surface.

Test plan

• cargo check -p windmill-ai -p windmill-worker --tests --features quickjs — clean
• cargo test -p windmill-ai ai_http_client_does_not_follow_redirects — passes
• (optional e2e) Configure an AI provider whose base_url returns a 3xx to an internal host; confirm the worker AI step surfaces the redirect response instead of connecting onward.

No SQL / API-surface changes (no update_sqlx / openapi regeneration needed). No *_ee.rs files touched, so no EE companion PR.

🤖 Generated with Claude Code

---

Summary by cubic

Disables HTTP redirects for worker requests to user‑configured AI providers to close the SSRF bypass (GHSA-5q4v-c4v3-v7wr). Redirects now return 3xx to the caller instead of being followed to private hosts.

• Bug Fixes
  • Added AI_HTTP_CLIENT with redirects disabled (reqwest::redirect::Policy::none()) and switched worker provider endpoint requests to use it, mirroring the API proxy policy.
  • Added ALLOW_AI_BASE_URL_REDIRECTS escape hatch to re-enable redirects for debugging; default off with a startup warning. Condensed comments and anchored the SSRF rationale to this override.
  • Added a regression test ensuring a 302 to link‑local metadata is surfaced, not followed.

^{Written for commit a7f5693. Summary will update on new commits.}

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	a7f5693
Status:	⚡️  Build in progress...

View logs

[claude]

Claude finished @rubenfiszel's task in 1m 17s —— View job

---

Review — GHSA-5q4v worker AI SSRF redirect fix

Good to merge.

I verified the fix against the codebase. It's a correct, well-scoped mirror of the already-hardened API proxy path.

What I checked

• AI_HTTP_CLIENT mirrors the API proxy client (windmill-api/src/ai.rs:107-120): both use configure_client(...) (preserving FORCE_IPV4) plus .redirect(reqwest::redirect::Policy::none()) and user_agent("windmill/beta"). Redirects are disabled unconditionally, which is the right call — the single-shot SSRF validation in AIProvider::get_base_url can't re-validate 3xx hops, so following any redirect reopens the bypass regardless of ALLOW_PRIVATE_AI_BASE_URLS. ✅
• build_http_request is the only worker outbound to the user-controlled endpoint. Grepping ai_executor.rs, the remaining HTTP_CLIENT use is line 397 (get_full_hub_script_by_path for hub/ paths — Windmill's own hub, internal), and there are no other reqwest client / .post(/.get( calls hitting the provider base_url. No Google/Gemini path constructs its own client in the worker, so nothing is missed. ✅
• Per-request timeout preserved. AI_HTTP_CLIENT sets only connect_timeout(10s) (no overall .timeout), but build_http_request applies .timeout(timeout) on each request, so request-level timeout behavior is unchanged. ✅
• Regression test is sound. The loopback listener returns a 302 to 169.254.169.254 and the test asserts the client surfaces the 302 rather than following it — directly exercising the vulnerable behavior.

AGENTS.md compliance

• Comments record the constraint (why redirects must stay disabled) rather than narrating the change — matches the "comments record constraints, not narration" rule.
• No SQL / API-surface / *_ee.rs changes, so no update_sqlx, openapi regen, or EE companion PR needed — consistent with the PR's own assessment.
• No attribution to any customer.

Test coverage

Backend-only change. A Rust regression test (ai_http_client_does_not_follow_redirects) covers the new client's redirect policy directly — appropriate for this surface. The internal-only call sites left on HTTP_CLIENT (hub fetch, OAuth refresh against BASE_URL, Bedrock AWS SDK) are correctly excluded per the PR notes.

No further manual verification strictly required; the optional e2e in the PR test plan (provider base_url returning a 3xx to an internal host, confirming the worker surfaces the 3xx instead of connecting onward) would be a nice belt-and-suspenders check but the unit test already exercises the core guarantee.
· fix/ghsa-5q4v-worker-ai-ssrf-redirect

[github-actions]

Pi Review

Good to merge.

The fix correctly mirrors the API proxy hardening from #9370 by creating a redirect-disabled AI_HTTP_CLIENT and replacing the one worker call-site that targets the user-controlled provider endpoint. The other HTTP_CLIENT usage at ai_executor.rs:397 fetches hub scripts against Windmill's own hub (not the provider base_url), so it's correctly left unchanged. Bedrock goes through the AWS SDK (separate surface), and the Google proxy path only runs from windmill-api where the redirect-disabled client was already applied.

No issues found. Checked for bugs, security, and AGENTS.md compliance.

Test coverage

• Backend: Includes a well-structured regression test (ai_http_client_does_not_follow_redirects) that stands up a loopback listener returning a 302 to link-local metadata and asserts the client surfaces the 302 rather than following it. No additional integration tests expected — this is a pure client-configuration change with no DB, API, or queue surface touched.
• Further manual verification is not strictly required since the test directly exercises the SSRF bypass scenario, but the optional e2e scenario described in the PR body (configure a provider whose base_url 3xx-redirects to an internal host, run a worker AI step, confirm the redirect response surfaces rather than reaching the internal host) would provide additional confidence if desired.

[github-actions]

Codex Review

cc @rubenfiszel

Mergeable, but should ideally address nits: duplicated overlong redirect invariant comments

• [P2] duplicated overlong redirect invariant comments — backend/windmill-ai/src/utils.rs:8

  The seven-line client comment repeats the same “user-controlled endpoint must use the redirect-disabled client” rationale added at backend/windmill-worker/src/ai_executor.rs:989. Root AGENTS.md requires: “State each invariant once, at the place where someone would break it, in ≤4 lines” and “don't repeat the same rationale at multiple sites.” Please condense the client comment and keep each distinct constraint at its relevant break point.

Test coverage

The new Tokio unit test directly verifies that AI_HTTP_CLIENT returns the 302 response without following it. There is no worker-level integration test confirming run_agent uses this client. Local execution was blocked by the environment’s unavailable mold linker, rather than a PR compilation failure.

Manual verification remains: run an AI agent job against a valid public provider URL that redirects to a controlled private listener, confirming the redirected target receives no request or forwarded headers, then verify a direct provider endpoint still streams normally.

[github-actions]

Now I have the full picture. Here's my review.

Pi Review

Good to merge.

The fix correctly mirrors the API proxy hardening from #9370 by creating a redirect-disabled AI_HTTP_CLIENT and replacing the one worker call-site (build_http_request) that targets the user-controlled provider endpoint. configure_client preserves FORCE_IPV4, per-request timeout is applied separately in build_http_request, and the remaining HTTP_CLIENT uses (hub fetch, OAuth refresh, Bedrock AWS SDK) are correctly left untouched.

Minor observation (not blocking):

• backend/windmill-ai/src/utils.rs:16 — The AI_HTTP_CLIENT doc comment runs ~8 lines, exceeding the AGENTS.md guideline of "≤4 lines" for invariant comments. The content is useful and security-justified, so this is at the author's discretion to condense; no need to hold the merge.

Test coverage

• Backend: The new regression test ai_http_client_does_not_follow_redirects stands up a loopback listener returning a 302 to link-local metadata and asserts the client surfaces the redirect rather than following it — this directly exercises the SSRF bypass scenario. No additional integration tests expected since this is a pure client-configuration change with no DB, API, or queue surface.
• Manual verification is not strictly required (the unit test covers the core guarantee), but the optional e2e in the PR body (configure a provider whose base_url 3xx-redirects to an internal host, confirm the worker surfaces the redirect) would add end-to-end confidence.

[github-actions]

Codex Review

cc @rubenfiszel

Mergeable, but should ideally address nits: escape hatch leaves the worker call-site invariant false

• [P2] escape hatch leaves the worker call-site invariant false — backend/windmill-worker/src/ai_executor.rs:990

  The comment requires the “redirect-disabled AI_HTTP_CLIENT,” but ALLOW_AI_BASE_URL_REDIRECTS=true now selects Policy::default() and follows redirects. This is a concrete doc/code mismatch and conflicts with the root AGENTS.md rule: “Describe the code as it is, never its drafting history.” Describe redirects as disabled by default and account for the override.

Test coverage

The Tokio unit test covers the secure default. The new redirect-enabled branch lacks unit coverage, and the existing test unconditionally expects a 302, so it is incompatible with test environments setting the new override. No worker-level integration test is included; no DB or API integration coverage is needed.

Manually run an AI agent against a provider redirecting to a controlled listener. By default, the listener should receive nothing. With the override enabled, it should receive the redirected request and the worker should emit the warning.

[github-actions]

Pi Review

cc @rubenfiszel

Mergeable, but should ideally address nits: duplicated overlong redirect invariant comments

What I checked

• AI_HTTP_CLIENT correctly mirrors the API proxy hardening (windmill-api/src/ai.rs): both use configure_client + redirect(Policy::none()) by default, preserving FORCE_IPV4. The ALLOW_AI_BASE_URL_REDIRECTS escape hatch is off by default and logs a warning when activated — a safe debug valve that doesn't regress the fix.
• build_http_request is the only worker outbound to the user-controlled endpoint. The remaining HTTP_CLIENT uses (hub fetch at line 397, OAuth token refresh against BASE_URL, Bedrock AWS SDK) are correctly left untouched.
• Per-request timeout preserved. AI_HTTP_CLIENT sets only connect_timeout(10s); build_http_request applies .timeout(timeout) on each request, so request-level timeout behavior is unchanged.
• Regression test is sound. The loopback listener returns a 302 to 169.254.169.254 (link-local metadata) and the test asserts the client surfaces the 302 instead of following it — directly exercising the vulnerable behavior.

Findings

[P2] duplicated overlong redirect invariant comments — backend/windmill-ai/src/utils.rs:15-23 and backend/windmill-worker/src/ai_executor.rs:989-992

The AI_HTTP_CLIENT doc comment runs 8 lines, exceeding the AGENTS.md guideline:

State each invariant once, at the place where someone would break it, in ≤4 lines. Don't repeat the same rationale at multiple sites.

Both this comment and the worker call-site comment explain the single-shot SSRF validation rationale — that a validated public base_url can 3xx into a private host. The worker comment already ends with "See AI_HTTP_CLIENT," so the detailed rationale belongs at the AI_HTTP_CLIENT definition only. Condense the client comment to ≤4 lines and drop the redundant SSRF explanation at the call site, keeping just the direction to use AI_HTTP_CLIENT instead of HTTP_CLIENT.

Test coverage

• Backend: The new regression test ai_http_client_does_not_follow_redirects directly exercises the SSRF bypass scenario — stands up a loopback listener, serves a 302 to link-local metadata, and asserts the client surfaces the redirect rather than following it. This is appropriate for a client-configuration change with no DB, API, or queue surface touched. No additional integration tests expected.
• Manual verification is not strictly required (the unit test covers the core guarantee), but the optional e2e from the PR test plan — configure a provider whose base_url 3xx-redirects to an internal host, confirm the worker surfaces the redirect — would add end-to-end confidence.

[github-actions]

Codex Review

cc @rubenfiszel

Mergeable, but should ideally address nits: duplicated overlong redirect invariant comments

• [P2] duplicated overlong redirect invariant comments — backend/windmill-ai/src/utils.rs:17

  The eight-line client comment repeats the worker call-site’s requirement to use AI_HTTP_CLIENT. Root AGENTS.md requires: “State each invariant once, at the place where someone would break it, in ≤4 lines” and “don't repeat the same rationale at multiple sites.” Please condense the client comment and keep each distinct constraint at its relevant break point.

Test coverage

The added Tokio test directly covers the secure default redirect policy. The escape-hatch branch and worker call site lack integration coverage; tests were not executed during this read-only review.

Manually run an AI agent against a provider that redirects to a controlled private listener. By default, the listener should receive no request and the worker should surface the redirect. With the override enabled, the redirect should be followed and the warning emitted.