From 79f21a44426773237d8d8c429ea561e72b58c8a1 Mon Sep 17 00:00:00 2001 From: Shakker Date: Wed, 6 May 2026 15:16:20 +0100 Subject: [PATCH] fix: normalize symbolic fetch headers --- CHANGELOG.md | 1 + src/infra/fetch-headers.test.ts | 82 ++++++++++++++++++++++++++ src/infra/fetch-headers.ts | 48 +++++++++++++++ src/infra/fetch.test.ts | 31 ++++++++++ src/infra/fetch.ts | 3 +- src/infra/net/fetch-guard.ssrf.test.ts | 33 +++++++++++ src/infra/net/fetch-guard.ts | 10 +++- src/infra/net/proxy-fetch.test.ts | 25 ++++++++ src/infra/net/proxy-fetch.ts | 10 +++- src/infra/net/redirect-headers.ts | 3 +- src/infra/net/runtime-fetch.test.ts | 35 +++++++++++ src/infra/net/runtime-fetch.ts | 15 +++-- src/plugin-sdk/fetch-auth.test.ts | 33 +++++++++++ src/plugin-sdk/fetch-auth.ts | 10 +++- 14 files changed, 326 insertions(+), 13 deletions(-) create mode 100644 src/infra/fetch-headers.test.ts create mode 100644 src/infra/fetch-headers.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e08ca2f9efa..39469d6d722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Telegram/Codex: generate DM topic labels with Codex-compatible simple-completion requests so auto-created private topics can be renamed instead of staying `New Chat`. +- Plugins/runtime fetch: drop third-party symbol metadata from plain request header dictionaries before passing them into native `fetch` or `Headers`, so SDK and guarded/proxy fetch paths do not reject otherwise valid plugin requests. Fixes #77846. Thanks @shakkernerd. - Web fetch: bound guarded dispatcher cleanup after request timeouts so timed-out fetches return tool errors instead of leaving Gateway tool lanes active. (#78439) Thanks @obviyus. - Mattermost/setup: prompt for and persist the server base URL after the bot token in `openclaw setup --wizard`, instead of failing validation before `--http-url` is collected. Fixes #76670. Thanks @jacobtomlinson. - Gate Slack startup user allowlist resolution [AI]. (#77898) Thanks @pgondhi987. diff --git a/src/infra/fetch-headers.test.ts b/src/infra/fetch-headers.test.ts new file mode 100644 index 00000000000..34a798abda8 --- /dev/null +++ b/src/infra/fetch-headers.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import { + normalizeHeadersInitForFetch, + normalizeRequestInitHeadersForFetch, +} from "./fetch-headers.js"; + +function createHeadersWithSymbol(enumerable: boolean): HeadersInit { + const headers = { "Content-Type": "application/json" } as Record & { + [key: symbol]: unknown; + }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["content-type"]), + enumerable, + }); + return headers; +} + +describe("normalizeHeadersInitForFetch", () => { + it("keeps Headers instances unchanged", () => { + const headers = new Headers({ Accept: "application/json" }); + + expect(normalizeHeadersInitForFetch(headers)).toBe(headers); + }); + + it("keeps tuple headers unchanged", () => { + const headers: HeadersInit = [["Accept", "application/json"]]; + + expect(normalizeHeadersInitForFetch(headers)).toBe(headers); + }); + + it("keeps plain string-key dictionaries unchanged when they have no symbol keys", () => { + const headers = { Accept: "application/json" }; + + expect(normalizeHeadersInitForFetch(headers)).toBe(headers); + }); + + it.each([ + { enumerable: true, name: "enumerable" }, + { enumerable: false, name: "non-enumerable" }, + ])("drops $name own symbol keys from plain header dictionaries", ({ enumerable }) => { + const headers = createHeadersWithSymbol(enumerable); + + const normalized = normalizeHeadersInitForFetch(headers); + + expect(normalized).not.toBe(headers); + expect(Object.getOwnPropertySymbols(normalized as object)).toEqual([]); + expect(new Headers(normalized).get("content-type")).toBe("application/json"); + expect(Object.getOwnPropertySymbols(headers as object)).toHaveLength(1); + }); + + it("preserves non-enumerable string header keys when dropping symbol keys", () => { + const headers = createHeadersWithSymbol(false); + Object.defineProperty(headers, "X-Hidden", { + value: "yes", + enumerable: false, + }); + + const normalized = normalizeHeadersInitForFetch(headers); + + expect(Object.getOwnPropertySymbols(normalized as object)).toEqual([]); + expect(new Headers(normalized).get("x-hidden")).toBe("yes"); + expect(new Headers(normalized).get("content-type")).toBe("application/json"); + }); +}); + +describe("normalizeRequestInitHeadersForFetch", () => { + it("returns the original init when headers do not need normalization", () => { + const init: RequestInit = { headers: { Accept: "application/json" } }; + + expect(normalizeRequestInitHeadersForFetch(init)).toBe(init); + }); + + it("returns a cloned init with symbol-free headers when needed", () => { + const init: RequestInit = { headers: createHeadersWithSymbol(false) }; + + const normalized = normalizeRequestInitHeadersForFetch(init); + + expect(normalized).not.toBe(init); + expect(Object.getOwnPropertySymbols(normalized?.headers as object)).toEqual([]); + expect(Object.getOwnPropertySymbols(init.headers as object)).toHaveLength(1); + }); +}); diff --git a/src/infra/fetch-headers.ts b/src/infra/fetch-headers.ts new file mode 100644 index 00000000000..52635fbb47a --- /dev/null +++ b/src/infra/fetch-headers.ts @@ -0,0 +1,48 @@ +type HeadersLike = { + entries: () => IterableIterator<[string, string]>; + get: (name: string) => string | null; + [Symbol.iterator]: () => IterableIterator<[string, string]>; +}; + +function isHeadersLike(value: object): value is HeadersLike { + if (typeof Headers !== "undefined" && value instanceof Headers) { + return true; + } + const candidate = value as Partial; + return ( + typeof candidate.entries === "function" && + typeof candidate.get === "function" && + typeof candidate[Symbol.iterator] === "function" + ); +} + +export function normalizeHeadersInitForFetch( + headers: HeadersInit | undefined, +): HeadersInit | undefined { + if (!headers || typeof headers !== "object" || Array.isArray(headers) || isHeadersLike(headers)) { + return headers; + } + if (Object.getOwnPropertySymbols(headers).length === 0) { + return headers; + } + + const normalized = Object.create(null) as Record; + const headerRecord = headers as Record; + for (const key of Object.getOwnPropertyNames(headerRecord)) { + normalized[key] = String(headerRecord[key]); + } + return normalized; +} + +export function normalizeRequestInitHeadersForFetch( + init: T | undefined, +): T | undefined { + if (!init?.headers) { + return init; + } + const headers = normalizeHeadersInitForFetch(init.headers); + if (headers === init.headers) { + return init; + } + return { ...init, headers } as T; +} diff --git a/src/infra/fetch.test.ts b/src/infra/fetch.test.ts index c73843e28c2..0024422f327 100644 --- a/src/infra/fetch.test.ts +++ b/src/infra/fetch.test.ts @@ -65,6 +65,17 @@ function createSeenSignalFetch() { return { fetchImpl, getSeenSignal: () => seenSignal }; } +function createSymbolHeaderInit(enumerable: boolean): RequestInit { + const headers = { "Content-Type": "application/json" } as Record & { + [key: symbol]: unknown; + }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["content-type"]), + enumerable, + }); + return { headers }; +} + describe("wrapFetchWithAbortSignal", () => { it("adds duplex for requests with a body", async () => { const { fetchImpl, getSeenInit } = createSeenInitFetch(); @@ -277,6 +288,26 @@ describe("wrapFetchWithAbortSignal", () => { expect(() => wrapped.preconnect("https://example.com")).not.toThrow(); }); + + it.each([ + { enumerable: true, name: "enumerable" }, + { enumerable: false, name: "non-enumerable" }, + ])( + "drops $name header symbol metadata before calling the wrapped fetch", + async ({ enumerable }) => { + const { fetchImpl, getSeenInit } = createSeenInitFetch(); + const wrapped = wrapFetchWithAbortSignal(fetchImpl); + const init = createSymbolHeaderInit(enumerable); + + await wrapped("https://example.com", init); + + const seenHeaders = getSeenInit()?.headers; + expect(seenHeaders).not.toBe(init.headers); + expect(Object.getOwnPropertySymbols(seenHeaders as object)).toEqual([]); + expect(new Headers(seenHeaders).get("content-type")).toBe("application/json"); + expect(Object.getOwnPropertySymbols(init.headers as object)).toHaveLength(1); + }, + ); }); describe("resolveFetch", () => { diff --git a/src/infra/fetch.ts b/src/infra/fetch.ts index d4612780438..2b9aa8dd9c7 100644 --- a/src/infra/fetch.ts +++ b/src/infra/fetch.ts @@ -1,4 +1,5 @@ import { bindAbortRelay } from "../utils/fetch-timeout.js"; +import { normalizeRequestInitHeadersForFetch } from "./fetch-headers.js"; type FetchWithPreconnect = typeof fetch & { preconnect: (url: string, init?: { credentials?: RequestCredentials }) => void; @@ -39,7 +40,7 @@ export function wrapFetchWithAbortSignal(fetchImpl: typeof fetch): typeof fetch } const wrapped = ((input: RequestInfo | URL, init?: RequestInit) => { - const patchedInit = withDuplex(init, input); + const patchedInit = normalizeRequestInitHeadersForFetch(withDuplex(init, input)); const signal = patchedInit?.signal; if (!signal) { return fetchImpl(input, patchedInit); diff --git a/src/infra/net/fetch-guard.ssrf.test.ts b/src/infra/net/fetch-guard.ssrf.test.ts index 1b848e63241..94b9c0768e6 100644 --- a/src/infra/net/fetch-guard.ssrf.test.ts +++ b/src/infra/net/fetch-guard.ssrf.test.ts @@ -644,6 +644,39 @@ describe("fetchWithSsrFGuard hardening", () => { await result.release(); }); + it("handles symbol-bearing header dictionaries while rewriting cross-origin redirects", async () => { + const lookupFn = createPublicLookup(); + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(redirectResponse("https://cdn.example.com/asset")) + .mockResolvedValueOnce(okResponse()); + const headers = { + Authorization: "Bearer secret", + Accept: "application/json", + } as Record & { [key: symbol]: unknown }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["authorization"]), + enumerable: false, + }); + + const result = await fetchWithSsrFGuard({ + url: "https://api.example.com/start", + fetchImpl, + lookupFn, + init: { headers }, + }); + + expect(result.response.status).toBe(200); + const firstHeaders = fetchImpl.mock.calls[0]?.[1]?.headers; + expect(firstHeaders).not.toBe(headers); + expect(Object.getOwnPropertySymbols(firstHeaders as object)).toEqual([]); + const secondHeaders = getSecondRequestHeaders(fetchImpl); + expect(secondHeaders.get("authorization")).toBeNull(); + expect(secondHeaders.get("accept")).toBe("application/json"); + expect(Object.getOwnPropertySymbols(headers)).toHaveLength(1); + await result.release(); + }); + it("rewrites POST redirects to GET and clears the body for cross-origin 302 responses", async () => { const lookupFn = createPublicLookup(); const fetchImpl = vi diff --git a/src/infra/net/fetch-guard.ts b/src/infra/net/fetch-guard.ts index 1be073cbc42..0f0527aa253 100644 --- a/src/infra/net/fetch-guard.ts +++ b/src/infra/net/fetch-guard.ts @@ -1,6 +1,10 @@ import type { Dispatcher } from "undici"; import { logWarn } from "../../logger.js"; import { buildTimeoutAbortSignal } from "../../utils/fetch-timeout.js"; +import { + normalizeHeadersInitForFetch, + normalizeRequestInitHeadersForFetch, +} from "../fetch-headers.js"; import { hasProxyEnvConfigured, shouldUseEnvHttpProxyForUrl } from "./proxy-env.js"; import { retainSafeHeadersForCrossOriginRedirect as retainSafeRedirectHeaders } from "./redirect-headers.js"; import { @@ -277,7 +281,7 @@ function dropBodyHeaders(headers?: HeadersInit): HeadersInit | undefined { if (!headers) { return headers; } - const nextHeaders = new Headers(headers); + const nextHeaders = new Headers(normalizeHeadersInitForFetch(headers)); nextHeaders.delete("content-encoding"); nextHeaders.delete("content-language"); nextHeaders.delete("content-length"); @@ -368,7 +372,9 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise([params.url]); let currentUrl = params.url; - let currentInit = params.init ? { ...params.init } : undefined; + let currentInit = normalizeRequestInitHeadersForFetch( + params.init ? { ...params.init } : undefined, + ); let redirectCount = 0; while (true) { diff --git a/src/infra/net/proxy-fetch.test.ts b/src/infra/net/proxy-fetch.test.ts index 2601dd5dd17..8d64922346f 100644 --- a/src/infra/net/proxy-fetch.test.ts +++ b/src/infra/net/proxy-fetch.test.ts @@ -180,6 +180,31 @@ describe("makeProxyFetch", () => { expect(undiciFetch.mock.calls[0]?.[1]?.body).toBe(body); }); + it("drops symbol metadata from plain header dictionaries before undici fetch", async () => { + undiciFetch.mockResolvedValue({ ok: true }); + + const proxyFetch = makeProxyFetch("http://proxy.test:8080"); + const headers = { "Content-Type": "application/json" } as Record & { + [key: symbol]: unknown; + }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["content-type"]), + enumerable: false, + }); + + await proxyFetch("https://api.example.com/json", { + method: "POST", + headers, + body: "{}", + }); + + const passedHeaders = undiciFetch.mock.calls[0]?.[1]?.headers; + expect(passedHeaders).not.toBe(headers); + expect(Object.getOwnPropertySymbols(passedHeaders as object)).toEqual([]); + expect(new Headers(passedHeaders).get("content-type")).toBe("application/json"); + expect(Object.getOwnPropertySymbols(headers)).toHaveLength(1); + }); + it("keeps undici FormData instances unchanged", async () => { undiciFetch.mockResolvedValue({ ok: true }); diff --git a/src/infra/net/proxy-fetch.ts b/src/infra/net/proxy-fetch.ts index 5200e3a392f..4483225a15e 100644 --- a/src/infra/net/proxy-fetch.ts +++ b/src/infra/net/proxy-fetch.ts @@ -1,5 +1,6 @@ import { logWarn } from "../../logger.js"; import { formatErrorMessage } from "../errors.js"; +import { normalizeHeadersInitForFetch } from "../fetch-headers.js"; import { resolveEnvHttpProxyAgentOptions } from "./proxy-env.js"; import { loadUndiciRuntimeDeps, type UndiciRuntimeDeps } from "./undici-runtime.js"; @@ -44,18 +45,21 @@ function normalizeInitForUndici( if (!init) { return init; } + const normalizedHeaders = normalizeHeadersInitForFetch(init.headers); + const initWithNormalizedHeaders = + normalizedHeaders === init.headers ? init : { ...init, headers: normalizedHeaders }; const body = init.body; if (!isFormDataLike(body) || body instanceof UndiciFormData) { - return init; + return initWithNormalizedHeaders; } const form = new UndiciFormData(); for (const [key, value] of body.entries()) { appendFormDataEntry(form, key, value); } - const headers = new Headers(init.headers); + const headers = new Headers(normalizedHeaders); headers.delete("content-length"); headers.delete("content-type"); - return { ...init, headers, body: form as unknown as BodyInit }; + return { ...initWithNormalizedHeaders, headers, body: form as unknown as BodyInit }; } /** diff --git a/src/infra/net/redirect-headers.ts b/src/infra/net/redirect-headers.ts index c638692b894..d708c4e5975 100644 --- a/src/infra/net/redirect-headers.ts +++ b/src/infra/net/redirect-headers.ts @@ -1,4 +1,5 @@ import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js"; +import { normalizeHeadersInitForFetch } from "../fetch-headers.js"; const CROSS_ORIGIN_REDIRECT_SAFE_HEADERS = new Set([ "accept", @@ -22,7 +23,7 @@ export function retainSafeHeadersForCrossOriginRedirect( if (!headers) { return headers; } - const incoming = new Headers(headers); + const incoming = new Headers(normalizeHeadersInitForFetch(headers)); const safeHeaders: Record = {}; for (const [key, value] of incoming.entries()) { if (CROSS_ORIGIN_REDIRECT_SAFE_HEADERS.has(normalizeLowercaseStringOrEmpty(key))) { diff --git a/src/infra/net/runtime-fetch.test.ts b/src/infra/net/runtime-fetch.test.ts index 70c118696e1..562ddb9f16b 100644 --- a/src/infra/net/runtime-fetch.test.ts +++ b/src/infra/net/runtime-fetch.test.ts @@ -45,6 +45,41 @@ afterEach(() => { }); describe("fetchWithRuntimeDispatcher", () => { + it("drops symbol metadata from plain header dictionaries before runtime fetch", async () => { + const runtimeFetch = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + expect(new Headers(init?.headers).get("content-type")).toBe("application/json"); + return new Response("ok", { status: 200 }); + }); + + (globalThis as Record)[TEST_UNDICI_RUNTIME_DEPS_KEY] = { + Agent: MockAgent, + EnvHttpProxyAgent: MockEnvHttpProxyAgent, + FormData: RuntimeFormData, + ProxyAgent: MockProxyAgent, + fetch: runtimeFetch, + }; + + const headers = { "Content-Type": "application/json" } as Record & { + [key: symbol]: unknown; + }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["content-type"]), + enumerable: false, + }); + + const response = await fetchWithRuntimeDispatcher("https://example.com/json", { + method: "POST", + headers, + body: "{}", + }); + + expect(response.status).toBe(200); + const sentHeaders = runtimeFetch.mock.calls[0]?.[1]?.headers; + expect(sentHeaders).not.toBe(headers); + expect(Object.getOwnPropertySymbols(sentHeaders as object)).toEqual([]); + expect(Object.getOwnPropertySymbols(headers)).toHaveLength(1); + }); + it("normalizes global FormData bodies into the runtime FormData implementation", async () => { const runtimeFetch = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { // init.body was rebuilt as RuntimeFormData by normalizeRuntimeFormData; diff --git a/src/infra/net/runtime-fetch.ts b/src/infra/net/runtime-fetch.ts index 9696c6a6ee8..13f88eae00c 100644 --- a/src/infra/net/runtime-fetch.ts +++ b/src/infra/net/runtime-fetch.ts @@ -1,4 +1,5 @@ import type { Dispatcher } from "undici"; +import { normalizeHeadersInitForFetch } from "../fetch-headers.js"; import { loadUndiciRuntimeDeps, type UndiciRuntimeDeps } from "./undici-runtime.js"; export type DispatcherAwareRequestInit = RequestInit & { dispatcher?: Dispatcher }; @@ -50,20 +51,26 @@ function normalizeRuntimeRequestInit( init: DispatcherAwareRequestInit | undefined, RuntimeFormData: RuntimeFormDataCtor | undefined, ): DispatcherAwareRequestInit | undefined { - if (!init?.body) { + if (!init) { return init; } + const normalizedHeaders = normalizeHeadersInitForFetch(init.headers); + const initWithNormalizedHeaders = + normalizedHeaders === init.headers ? init : { ...init, headers: normalizedHeaders }; + if (!init.body) { + return initWithNormalizedHeaders; + } const body = normalizeRuntimeFormData(init.body, RuntimeFormData); if (body === init.body) { - return init; + return initWithNormalizedHeaders; } - const headers = new Headers(init.headers); + const headers = new Headers(normalizedHeaders); headers.delete("content-length"); headers.delete("content-type"); return { - ...init, + ...initWithNormalizedHeaders, headers, body, }; diff --git a/src/plugin-sdk/fetch-auth.test.ts b/src/plugin-sdk/fetch-auth.test.ts index e2bdbd36e9c..4f2dbdc20ce 100644 --- a/src/plugin-sdk/fetch-auth.test.ts +++ b/src/plugin-sdk/fetch-auth.test.ts @@ -115,6 +115,39 @@ describe("fetchWithBearerAuthScopeFallback", () => { expect(tokenProvider.getAccessToken).toHaveBeenNthCalledWith(1, "https://first.example"); expect(tokenProvider.getAccessToken).toHaveBeenNthCalledWith(2, "https://second.example"); }); + + it("normalizes symbol-bearing request headers across unauthenticated and retry attempts", async () => { + const headers = { Accept: "application/json" } as Record & { + [key: symbol]: unknown; + }; + Object.defineProperty(headers, Symbol("sensitiveHeaders"), { + value: new Set(["accept"]), + enumerable: false, + }); + const fetchFn = vi.fn(async (_url: string, init?: RequestInit) => { + new Headers(init?.headers); + return fetchFn.mock.calls.length === 1 + ? new Response("unauthorized", { status: 401 }) + : new Response("ok", { status: 200 }); + }); + const tokenProvider = { getAccessToken: vi.fn(async () => "token-1") }; + + const response = await fetchWithBearerAuthScopeFallback({ + url: "https://graph.microsoft.com/v1.0/me", + scopes: ["https://graph.microsoft.com"], + fetchFn: asFetch(fetchFn), + tokenProvider, + requestInit: { headers }, + }); + + expect(response.status).toBe(200); + expect(fetchFn).toHaveBeenCalledTimes(2); + expect(Object.getOwnPropertySymbols(fetchFn.mock.calls[0]?.[1]?.headers as object)).toEqual([]); + expect(new Headers(fetchFn.mock.calls[1]?.[1]?.headers).get("authorization")).toBe( + "Bearer token-1", + ); + expect(Object.getOwnPropertySymbols(headers)).toHaveLength(1); + }); }); describe("resolveRequestUrl", () => { diff --git a/src/plugin-sdk/fetch-auth.ts b/src/plugin-sdk/fetch-auth.ts index 10945bb2a44..b29df19953d 100644 --- a/src/plugin-sdk/fetch-auth.ts +++ b/src/plugin-sdk/fetch-auth.ts @@ -1,3 +1,8 @@ +import { + normalizeHeadersInitForFetch, + normalizeRequestInitHeadersForFetch, +} from "../infra/fetch-headers.js"; + export type ScopeTokenProvider = { getAccessToken: (scope: string) => Promise; }; @@ -28,9 +33,10 @@ export async function fetchWithBearerAuthScopeFallback(params: { throw new Error(`URL must use HTTPS: ${params.url}`); } + const requestInit = normalizeRequestInitHeadersForFetch(params.requestInit); const fetchOnce = (headers?: Headers): Promise => fetchFn(params.url, { - ...params.requestInit, + ...requestInit, ...(headers ? { headers } : {}), }); @@ -54,7 +60,7 @@ export async function fetchWithBearerAuthScopeFallback(params: { for (const scope of params.scopes) { try { const token = await params.tokenProvider.getAccessToken(scope); - const authHeaders = new Headers(params.requestInit?.headers); + const authHeaders = new Headers(normalizeHeadersInitForFetch(requestInit?.headers)); authHeaders.set("Authorization", `Bearer ${token}`); const authAttempt = await fetchOnce(authHeaders); if (authAttempt.ok) {