Greptile Summary

This PR fixes a TypeError in createSlackMediaFetch on Node.js v25 by stripping dispatcher (and redirect) from the init options before passing them to globalThis.fetch. The globalThis.fetch fallback path is correctly fixed, but the undici path (fetchWithRuntimeDispatcher) also receives { ...rest, redirect: "manual" } without the original dispatcher, silently dropping the pinned SSRF dispatcher that fetchWithSsrFGuard constructed from SLACK_MEDIA_SSRF_POLICY. This voids the DNS-pinning guarantee on the undici path and needs to be addressed before merge.

Confidence Score: 4/5

Not safe to merge as-is: the undici path drops the pinned SSRF dispatcher, voiding DNS-rebinding protection.

The fix is directionally correct and solves the reported Node v25 TypeError for the globalThis.fetch fallback path. However, _dispatcher is also stripped from the undici (fetchWithRuntimeDispatcher) path, silently bypassing the SSRF protection that fetchWithSsrFGuard set up via SLACK_MEDIA_SSRF_POLICY. This is a present security-boundary defect introduced by the PR, warranting a 4 rather than 5.

extensions/slack/src/monitor/media.ts — specifically createSlackMediaFetch lines 80–85

This is a comment left during a code review.
Path: extensions/slack/src/monitor/media.ts
Line: 80-85

Comment:
**Pinned SSRF dispatcher is silently dropped on the undici path**

`_dispatcher` is destructured out of `rest` but never forwarded to `fetchWithRuntimeDispatcher`. When `fetchWithSsrFGuard` calls this fetch impl, it already resolved DNS and built a pinned dispatcher (`createPinnedDispatcher`) specifically to prevent DNS-rebinding/SSRF. Stripping it here causes undici to fall back to its global dispatcher, re-resolving DNS at connection time and voiding the SSRF guarantee. The `globalThis.fetch` path is correctly fixed — the undici path needs the same value forwarded:

```suggestion
    const { dispatcher: _dispatcher, redirect: _redirect, ...rest } = init ?? {};
    const usesUndici = _dispatcher !== undefined && !isMockedFetch(globalThis.fetch);
    const fetchImpl = usesUndici ? fetchWithRuntimeDispatcher : globalThis.fetch;
    return fetchImpl(parsed.href, {
      ...rest,
      ...(usesUndici && _dispatcher !== undefined ? { dispatcher: _dispatcher } : {}),
      redirect: "manual",
    });
```

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

_{Reviews (1): Last reviewed commit: "fix(slack): strip dispatcher option befo..." | Re-trigger Greptile}

+1 — this is a direct hit for us. We run 4 OpenClaw 2026.4.5 agents in NVIDIA OpenShell sandboxes behind an HTTP CONNECT proxy (10.200.0.1:3128). Slack file downloads (images, PDFs, audio) flow through createSlackMediaFetch → globalThis.fetch.

We've been carrying a local workaround in our openclaw-fetch-guard-patch.py that sets dispatcher = null to prevent exactly this class of failure — Node 22's built-in globalThis.fetch rejects npm undici v8 dispatchers with invalid onRequestStart method, and the error is silently swallowed by the bare catch block in the Slack download path.

This PR is the proper upstream fix. Happy to provide production confirmation across all 4 agents once this lands in a release.

Environment: 4 sandboxes, OpenClaw 2026.4.5, Node 22, Slack Socket Mode, all traffic via CONNECT proxy.

Closing this as duplicate or superseded after Codex automated review.

This PR is superseded by the newer Slack media dispatcher work in #64022/#63905. Current main still has the underlying dispatcher behavior, but this PR's one-file diff has unresolved review concerns and overlaps a later PR that carries the follow-up fix/test discussion and production confirmation.

Best possible solution:

Close this PR as superseded and continue the Slack media dispatcher fix in #64022/#63905, or a maintainer-owned replacement, with tests that explicitly settle the runtime-fetch versus pinned-dispatcher contract.

What I checked:

• Current main behavior: createSlackMediaFetch still selects fetchWithRuntimeDispatcher when init contains dispatcher and then forwards { ...init, redirect: "manual" }, so current main has not landed this PR's stripping change. (extensions/slack/src/monitor/media.ts:96, 2dba9e6a765a)
• Existing regression contract: The Slack media test currently asserts dispatcher-backed media requests use runtime fetch and that the runtime fetch init still contains dispatcher, which conflicts with this PR's proposed one-file change. (extensions/slack/src/monitor/media.test.ts:605, 2dba9e6a765a)
• Guarded fetch contract: fetchWithSsrFGuard attaches a dispatcher to the request init and calls caller-provided fetch implementations with that init; Slack media reaches that path through fetchRemoteMedia with createSlackMediaFetch as fetchImpl. (src/infra/net/fetch-guard.ts:387, 2dba9e6a765a)
• PR review discussion: Both review comments on the PR flagged that the proposed destructuring strips the pinned dispatcher from the runtime/undici branch, while a user comment confirmed the Slack attachment/download failure in proxy/container deployments. The discussion shows useful signal, but also a blocking contract/security concern before merge. (7016fcf0fdeb)
• Superseding related work: The provided timeline shows newer open PR fix(slack): strip SSRF dispatcher from media fetch to prevent undici version conflict #64022 and issue bug(slack): inbound attachments in container sandbox fail with placeholder-only turn and fetch error 'invalid onRequestStart method' #63905 track the same Slack media dispatcher failure with tests and production verification, making them the better canonical place to resolve the remaining work. (b6cdc58c7338)

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 2dba9e6a765a.