fix(media): disable pinned DNS dispatcher for FormData transcription requests#64766
Conversation
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟠 SSRF guard bypass via DNS rebinding/TOCTOU when `pinDns === false`
DescriptionThe SSRF protection in
This creates a classic time-of-check/time-of-use window enabling DNS rebinding: an attacker-controlled hostname can resolve to a public IP during the check, then rebind to a private/internal IP when the connection is made, bypassing private-network blocks. Vulnerable code: } else if (params.pinDns === false) {
await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
lookupFn: params.lookupFn,
policy: params.policy,
});
dispatcher = createPolicyDispatcherWithoutPinnedDns(params.dispatcherPolicy);
}RecommendationAvoid check-then-connect without pinning. Options:
Example (pin the lookup even when you don't want to “freeze” a single address, but still want policy enforcement): // Use a lookup wrapper that calls dns.lookup(all:true) and rejects private/special-use
// results before returning them to undici.
const policyLookup = async (hostname: string, opts: any, cb: any) => {
const results = await dnsLookup(hostname, { all: true });
assertAllowedResolvedAddressesOrThrow(results, params.policy);
// then pass through to undici in expected callback form
};
return createHttp1Agent({ connect: { lookup: policyLookup } });Also reconsider call sites newly setting 2. 🟠 DNS rebinding SSRF risk by disabling pinned DNS in OpenAI-compatible multipart transcription request
Description
While the SSRF guard still performs an initial DNS resolution check (via
This can allow SSRF to internal services if an attacker controls a hostname used in Vulnerable code: const { response: res, release } = await postTranscriptionRequest({
url,
headers,
body: form,
timeoutMs: params.timeoutMs,
fetchFn,
pinDns: false,
allowPrivateNetwork,
dispatcherPolicy,
});RecommendationDo not disable DNS pinning for user-influenceable targets. Options:
const { response: res, release } = await postTranscriptionRequest({
url,
headers,
body: form,
timeoutMs: params.timeoutMs,
fetchFn,
// omit pinDns or set to true
allowPrivateNetwork,
dispatcherPolicy,
});
Analyzed PR: #64766 at commit Last updated on: 2026-04-11T12:40:08Z |
Greptile SummaryThis PR fixes audio transcription failures for all OpenAI-compatible providers by ensuring Confidence Score: 5/5This PR is safe to merge; the fix is narrowly scoped, well-tested, and the acknowledged TOCTOU tradeoff is acceptable given operator-controlled transcription endpoints. All three files make coherent, complementary changes. The root cause (undici dispatcher corrupting FormData when pinDns defaults to undefined) is correctly addressed. The new hostname validation in the pinDns===false path is consistent with the existing pinned-DNS path. Tests cover the default case, the with-options case, and the negative case for JSON requests. No P0 or P1 findings. No files require special attention. Reviews (3): Last reviewed commit: "fix(ssrf): validate hostname even when p..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d5695b5d3
ℹ️ 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".
|
@aisle-research-bot Valid finding — addressed in e49b536. The The fix is at the guard level so all |
|
@greptile-apps P2 addressed in e49b536 — renamed the test to |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟠 SSRF policy bypass via DNS rebinding when `pinDns === false` (TOCTOU between DNS check and fetch)
DescriptionIn
Because the checked DNS answer is not bound to the used connection, an attacker controlling DNS for a hostname can exploit DNS rebinding / round-robin to pass the policy check with a public IP and then have the actual fetch resolve to a private IP (e.g., Vulnerable code (new behavior): } else if (params.pinDns === false) {
await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
lookupFn: params.lookupFn,
policy: params.policy,
});
dispatcher = createPolicyDispatcherWithoutPinnedDns(params.dispatcherPolicy);
}This creates a classic time-of-check/time-of-use gap in SSRF protection. RecommendationAvoid performing DNS-based IP allow/deny checks unless you can ensure the same resolved address is used for the connection. Options:
Example secure approach (keep pinning and use the pinned dispatcher): const pinned = await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
lookupFn: params.lookupFn,
policy: params.policy,
});
dispatcher = createPinnedDispatcher(pinned, params.dispatcherPolicy, params.policy);If Analyzed PR: #64766 at commit Last updated on: 2026-04-11T11:03:29Z |
|
@aisle-research-bot Acknowledged — the TOCTOU/DNS rebinding gap between the hostname validation and the actual fetch is an inherent trade-off of the Why we accept this trade-off here:
The |
|
@greptile-apps Both concerns addressed:
|
|
@greptile review |
…requests The SSRF guard's pinned DNS dispatcher (undici) corrupts FormData multipart bodies, causing audio transcription to fail with HTTP 400 on OpenAI-compatible providers. Always set pinDns: false in postTranscriptionRequest so native fetch handles FormData correctly. SSRF hostname validation is preserved via resolvePinnedHostnameWithPolicy.
When pinDns=false was set to avoid undici dispatcher corruption of FormData bodies, resolvePinnedHostnameWithPolicy was skipped entirely, removing SSRF hostname/private-IP validation. Now the pinDns=false path runs hostname validation as a preflight before creating the non-pinned dispatcher, preserving defense-in-depth. Also renames a stale test description per Greptile review feedback.
2a80917 to
a350430
Compare
obviyus
left a comment
There was a problem hiding this comment.
Verified the multipart transcription failure path and confirmed the fix is scoped to the OpenAI-compatible FormData caller while the generic shared transcription helper stays on the normal pinned-DNS path.
Maintainer follow-up: moved pinDns: false out of postTranscriptionRequest, added a caller-specific regression test, and appended the Unreleased changelog entry for #64766.
Local gate: pnpm test src/media-understanding/shared.test.ts src/media-understanding/openai-compatible-audio.test.ts src/media-understanding/openai-compatible-audio.pin-dns.test.ts extensions/deepgram/audio.test.ts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a350430d8a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, { | ||
| lookupFn: params.lookupFn, | ||
| policy: params.policy, | ||
| }); |
There was a problem hiding this comment.
Skip local DNS preflight for pinDns=false proxy flows
Calling resolvePinnedHostnameWithPolicy in the pinDns === false branch makes every request depend on local DNS resolution before the proxy dispatcher is used. For dispatcherPolicy.mode === "explicit-proxy" (and similarly proxy-only environments), the target hostname may only be resolvable by the proxy, so this now throws before fetchImpl runs even though the proxy route itself is valid. This regresses existing pinDns: false callers (for example provider requests with explicit proxy overrides) in corporate/proxy-only networks.
Useful? React with 👍 / 👎.
Summary
FormDatamultipart bodies, causing audio transcription to fail with HTTP 400 on OpenAI-compatible providers (OpenAI, Groq, Mistral). The API returns "you must provide a model parameter" even though all fields are correctly appended.postTranscriptionRequestnow always passespinDns: falsetofetchWithTimeoutGuarded, ensuring nativefetchhandlesFormDataencoding correctly instead of the undici dispatcher.fetchWithSsrFGuardnow runsresolvePinnedHostnameWithPolicyin thepinDns === falsepath, closing a pre-existing SSRF bypass where hostname validation was skipped entirely when DNS pinning was disabled. This is a defense-in-depth improvement that affects allpinDns: falsecallers.postJsonRequestbehavior is unchanged. ThepinDns === falsedispatcher selection logic is unchanged — only hostname validation was added as a preflight.pinDns: falsecallers verified:extensions/google/image-generation-provider.tsandextensions/minimax/music-generation-provider.tsboth usepinDns: falsetargeting public API endpoints (Google Generative AI, MiniMax API) withallowPrivateNetworkanddispatcherPolicyfrom provider config. These are unaffected by the new hostname validation since they target public endpoints.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
postTranscriptionRequestsends aFormDatabody withoutallowPrivateNetworkordispatcherPolicy, the options argument tofetchWithTimeoutGuardedisundefined. This causespinDnsto default toundefined, whichfetchWithSsrFGuardtreats as truthy — creating an undici pinned DNS dispatcher. Undici's fetch corruptsFormDatamultipart encoding.postTranscriptionRequestavoids the pinned DNS dispatcher by default. Additionally, thepinDns === falsepath infetchWithSsrFGuardskipped hostname validation entirely — a pre-existing SSRF bypass.resolveGuardedPostRequestOptionshelper returnsundefinedwhen no options are set, sopinDnswas never explicitly set tofalsefor the common case.Regression Test Plan (if applicable)
src/media-understanding/shared.test.tspostTranscriptionRequestalways passespinDns: falseto the SSRF guard, even when the caller does not specify it.fetchWithSsrFGuardand asserts thepinDnsfield is alwaysfalsefor transcription requests, catching any regression that re-enables the pinned dispatcher.pinDns: falsepassthrough — updated to verify it's always set.User-visible / Behavior Changes
Audio transcription via OpenAI-compatible providers (OpenAI, Groq, Mistral) now works correctly. Voice notes and audio files are transcribed successfully.
Diagram (if applicable)
Security Impact (required)
pinDns === falsepath infetchWithSsrFGuardnow runsresolvePinnedHostnameWithPolicyas a preflight, closing a pre-existing bypass where hostname/private-IP validation was skipped entirely. Note: a TOCTOU gap remains between the DNS validation and the actual fetch (DNS rebinding), which is an inherent trade-off of not using the pinned dispatcher. Transcription URLs come from operator-controlled provider config, mitigating this risk.Repro + Verification
Environment
Steps
gpt-4o-transcribe.Expected
Actual
Evidence
Runtime patch on compiled output confirmed the fix before this source change. See #64762 for detailed evidence.
Human Verification (required)
pinDns: falseis always set for transcription requests and NOT forced for JSON requests.allowPrivateNetwork: true+auditContextstill pass through correctly alongsidepinDns: false. Verified otherpinDns: falsecallers (Google image gen, MiniMax music gen) target public endpoints and are unaffected.Review Conversations
Compatibility / Migration
Risks and Mitigations
resolvePinnedHostnameWithPolicynow runs as a preflight even in thepinDns === falsepath, validating hostnames and blocking private IPs. The remaining TOCTOU gap requires an attacker to control DNS for the target hostname (operator-configured, not user-input). Transcription endpoints are always public HTTPS APIs.fetchWithSsrFGuard'spinDns === falsepath could block other callers.pinDns: falsecallers (Google image gen, MiniMax music gen) target public API endpoints and passallowPrivateNetwork/dispatcherPolicyfrom provider config. No breakage expected.Post-Deploy Monitoring & Validation
Audio transcription failed (HTTP 400)— should drop to zero after deploy.