fix(net): strip external undici dispatcher from custom fetchImpl calls by oliviareid-svg · Pull Request #62240 · openclaw/openclaw

oliviareid-svg · 2026-04-07T02:09:59Z

Summary

Fixes Slack file attachment downloads silently failing on OpenClaw 2026.4.5 with InvalidArgumentError: invalid onRequestStart method
Root cause: fetchWithSsrFGuard passes an external undici dispatcher (from the bundled undici package) through init to custom fetchImpl callers. These custom fetchers (e.g. Slack/Telegram media) wrap Node's built-in fetch(), which bundles its own undici 8.x internally. The handler interface changed between undici 7→8, causing the version mismatch error.
Fix: strip the dispatcher property from init before forwarding to custom fetchImpl callers. The SSRF DNS-pinning validation has already been performed before the custom fetcher is invoked, so security guarantees are preserved.

Test plan

Existing fetch-guard.ssrf.test.ts tests pass (no behavior change for runtime-fetch path)
Slack file attachment download works on Node 22+ with undici 8.x
Verify SSRF protection still blocks private IPs when custom fetchImpl is used

🤖 Generated with Claude Code

Custom fetch implementations (e.g. Slack/Telegram media fetchers) wrap Node's built-in fetch(), which bundles its own undici version. When the SSRF guard passes an external undici dispatcher through the init object, native fetch() rejects it with `InvalidArgumentError: invalid onRequestStart method` because the handler interface changed in undici 8. Strip the dispatcher from init before forwarding to custom fetchImpl callers. DNS-pinning SSRF validation has already been performed before the custom fetcher is invoked, so security guarantees are preserved. Fixes openclaw#62218 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

openclaw-barnacle · 2026-04-07T02:10:35Z

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0edbebe15c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-07T02:12:37Z

+        const { dispatcher: _stripDispatcher, ...initWithoutDispatcher } = init;
+        response = await defaultFetch(
+          parsedUrl.toString(),
+          initWithoutDispatcher as RequestInit,


Preserve dispatcher for dispatcher-capable custom fetchers

This branch now removes dispatcher for every non-runtime fetch call, including the supportsDispatcherInit path where callers intentionally provide a dispatcher-capable fetchImpl. That regresses guarded transport behavior: pinned/proxy dispatchers are no longer applied, so requests can bypass DNS pinning (re-resolve hostname at send time) and ignore explicit proxy policy in custom-fetch integrations. The previous logic forwarded init unchanged in this path; stripping here weakens SSRF guarantees whenever a custom fetcher actually supports dispatcher.

Useful? React with 👍 / 👎.

greptile-apps · 2026-04-07T02:13:42Z

Greptile Summary

The core fix is correct: stripping dispatcher from init before forwarding to a caller-supplied fetchImpl resolves the undici 7→8 interface mismatch that was silently breaking Slack file downloads, and SSRF host validation has already completed before this point so the security posture is unchanged.

P1: The existing test "fails closed when the runtime rejects the pinned dispatcher shape" (fetch-guard.ssrf.test.ts:332–349) will now fail — its mock only throws the compat error when requestInit.dispatcher is present, which the fix strips. This test must be updated before merging.
P2: Missing trailing newline at end of fetch-guard.ts.

Confidence Score: 4/5

Not safe to merge as-is — the P1 test regression must be resolved first.

The fix logic is sound and well-targeted, but the existing 'fails closed' test will fail because it was written to exercise the exact compat error path the PR eliminates. Resolving that one test update brings this to merge-ready.

src/infra/net/fetch-guard.ssrf.test.ts (test at lines 332–349 needs updating to assert the new no-dispatcher behavior); src/infra/net/fetch-guard.ts (missing trailing newline)

Comments Outside Diff (1)

src/infra/net/fetch-guard.ssrf.test.ts, line 332-349 (link)

Existing "fails closed" test will break after this change

This test simulates the exact undici 7→8 compat error the PR is fixing — createPinnedDispatcherCompatibilityError() is thrown only when the mock observes requestInit.dispatcher in init (line 335). After the fix strips dispatcher from init before forwarding to defaultFetch, the if (requestInit.dispatcher) branch is never entered: the mock falls through to return okResponse(), the call resolves successfully, and expect(...).rejects.toThrow("fetch failed") fails.

This test must be updated to reflect the new behavior: assert that the call succeeds and that requestInit.dispatcher is absent inside the mock.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ssrf.test.ts
Line: 332-349

Comment:
**Existing "fails closed" test will break after this change**

This test simulates the exact undici 7→8 compat error the PR is fixing — `createPinnedDispatcherCompatibilityError()` is thrown only when the mock observes `requestInit.dispatcher` in `init` (line 335). After the fix strips `dispatcher` from `init` before forwarding to `defaultFetch`, the `if (requestInit.dispatcher)` branch is never entered: the mock falls through to `return okResponse()`, the call resolves successfully, and `expect(...).rejects.toThrow("fetch failed")` fails.

This test must be updated to reflect the new behavior: assert that the call succeeds and that `requestInit.dispatcher` is absent inside the mock.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ssrf.test.ts
Line: 332-349

Comment:
**Existing "fails closed" test will break after this change**

This test simulates the exact undici 7→8 compat error the PR is fixing — `createPinnedDispatcherCompatibilityError()` is thrown only when the mock observes `requestInit.dispatcher` in `init` (line 335). After the fix strips `dispatcher` from `init` before forwarding to `defaultFetch`, the `if (requestInit.dispatcher)` branch is never entered: the mock falls through to `return okResponse()`, the call resolves successfully, and `expect(...).rejects.toThrow("fetch failed")` fails.

This test must be updated to reflect the new behavior: assert that the call succeeds and that `requestInit.dispatcher` is absent inside the mock.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ts
Line: 376

Comment:
**Missing trailing newline**

The diff shows `\ No newline at end of file` on the closing `}`. Please add a trailing newline to keep the file consistent with the repo's formatting conventions.

```suggestion
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(net): strip external undici dispatch..." | Re-trigger Greptile}

greptile-apps · 2026-04-07T02:13:49Z

    }
  }
-}
+}


Missing trailing newline

The diff shows \ No newline at end of file on the closing }. Please add a trailing newline to keep the file consistent with the repo's formatting conventions.

Suggested change

}

}

Prompt To Fix With AI

This is a comment left during a code review. Path: src/infra/net/fetch-guard.ts Line: 376 Comment: **Missing trailing newline** The diff shows `\ No newline at end of file` on the closing `}`. Please add a trailing newline to keep the file consistent with the repo's formatting conventions. ```suggestion } ``` How can I resolve this? If you propose a fix, please make it concise.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

openclaw-barnacle Bot added size: XS r: too-many-prs Auto-close: author has more than twenty active PRs. labels Apr 7, 2026

openclaw-barnacle Bot closed this Apr 7, 2026

chatgpt-codex-connector Bot reviewed Apr 7, 2026

View reviewed changes

greptile-apps Bot reviewed Apr 7, 2026

View reviewed changes

oliviareid-svg mentioned this pull request Apr 7, 2026

fix(net): strip external undici dispatcher from custom fetchImpl calls #62241

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(net): strip external undici dispatcher from custom fetchImpl calls#62240

fix(net): strip external undici dispatcher from custom fetchImpl calls#62240
oliviareid-svg wants to merge 1 commit intoopenclaw:mainfrom
oliviareid-svg:fix/slack-media-undici-dispatcher

oliviareid-svg commented Apr 7, 2026

Uh oh!

openclaw-barnacle Bot commented Apr 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 7, 2026

Uh oh!

greptile-apps Bot commented Apr 7, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

oliviareid-svg commented Apr 7, 2026

Summary

Test plan

Uh oh!

openclaw-barnacle Bot commented Apr 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Apr 7, 2026 •

edited

Loading