🛡️ fix: SSRF-Guard Speech (STT/TTS) and OCR Outbound Requests at Connect Time - #14560
🛡️ fix: SSRF-Guard Speech (STT/TTS) and OCR Outbound Requests at Connect Time#14560dustinhealy wants to merge 3 commits into
Conversation
… time Speech (STT/TTS) and OCR issued outbound HTTP to operator-provided target URLs with only proxy config attached, so a target that resolves to a private, loopback, link-local, or cloud-metadata address was reachable by the server. This is the same class already guarded for the custom models fetch, Actions, avatar, MCP, and the OpenAI/Anthropic endpoint clients. Add one helper applySSRFSafeAgentIfDirect(config, url, allowedAddresses) that rejects non-http(s) or unparseable target URLs, sets maxRedirects to 0 so a redirect cannot bypass the connect-time check, and attaches createSSRFSafeAgents when no proxy or agent is already set (proxy precedence preserved). Wire it into the six speech/OCR call sites and thread each section's allowedAddresses exemption from pr-01. Scope is private/internal SSRF only. It does not restrict forwarding a credential to a public host, which is a separate egress-allowlist concern. Absent an allowedAddresses entry, private targets now fail closed, matching endpoints, actions, and MCP. Document the new fields in librechat.example.yaml with the operator warning that allowedAddresses hostnames are trusted before the private-IP check.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68a293bc71
ℹ️ 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".
… generic uploads applySSRFSafeAgentIfDirect only attached the DNS-lookup agents, but Node skips the custom lookup for IP-literal hosts, so a literal private IP such as http://127.0.0.1 connected unchecked. Reject literal private IPs synchronously in the helper, reusing the same allowedAddresses exemption logic as the lookup path. The generic audio-upload path did not forward the section-level allowedAddresses to sttRequest, so a private STT endpoint permitted via speech.stt.allowedAddresses failed with ESSRF outside the speech route. Thread the exemption through files/audio.ts and widen the STTService type.
|
@codex re-review |
There was a problem hiding this comment.
Pull request overview
This PR extends LibreChat’s existing SSRF hardening to cover speech (STT/TTS) and Mistral OCR outbound HTTP requests by attaching SSRF-safe connect-time agents (DNS-rebinding resistant) and disabling redirects, while supporting operator opt-ins via allowedAddresses exemptions.
Changes:
- Add
applySSRFSafeAgentIfDirect()and apply it to OCR and speech outbound axios calls (plusmaxRedirects: 0). - Thread section-level
allowedAddressesthrough STT/TTS and OCR request construction to allow explicit private host:port exemptions. - Add/adjust Jest coverage for SSRF blocking, exemptions, and proxy precedence; document the new config in
librechat.example.yaml.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/types/files.ts | Extends STT service typing to thread allowedAddresses through schema selection and request execution. |
| packages/api/src/files/mistral/crud.ts | Applies SSRF-safe axios agent attachment (and redirect disabling) to Mistral OCR upload/sign URL/OCR/delete calls; threads ocr.allowedAddresses. |
| packages/api/src/files/mistral/crud.spec.ts | Updates existing assertions for new axios config fields and adds SSRF guard behavioral tests. |
| packages/api/src/files/audio.ts | Threads section-level allowedAddresses from getProviderSchema() into sttRequest(). |
| packages/api/src/files/audio.spec.ts | Adds a unit test ensuring allowedAddresses is passed through to the STT request. |
| packages/api/src/auth/agent.ts | Introduces applySSRFSafeAgentIfDirect() for axios SSRF guard wiring at connect time (+ redirect disabling). |
| packages/api/src/auth/agent.spec.ts | Adds focused tests for applySSRFSafeAgentIfDirect() behavior, including scheme rejection, proxy/agent precedence, and ESSRF blocking. |
| librechat.example.yaml | Documents SSRF-guard behavior for speech and OCR and the allowedAddresses exemption format. |
| api/server/services/Files/Audio/TTSService.js | Applies SSRF-safe agent wiring to TTS axios calls and threads speech.tts.allowedAddresses. |
| api/server/services/Files/Audio/STTService.js | Applies SSRF-safe agent wiring to STT axios calls and threads speech.stt.allowedAddresses. |
| api/server/services/Files/Audio/speechSSRF.spec.js | Adds integration-style SSRF tests for STT/TTS using the “real” agent lookup path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 501bb03218
ℹ️ 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".
…et before opening its stream The literal-IP precheck normalized an empty URL.port to '', so allowedAddresses exemptions on a default port (127.0.0.1:80, [::1]:443) never matched. Derive 80/443 from the scheme when the port is omitted. uploadDocumentToMistral opened the upload file stream before the SSRF check, so a blocked or malformed target threw with the descriptor still open. Run the proxy/SSRF setup before fs.createReadStream. Reword the librechat.example.yaml allowedAddresses guidance to prefer a private IP literal over a hostname, and reconcile the speech/OCR SSRF specs to assert the synchronous literal-IP block instead of driving the connect-time lookup that Node skips for IP literals.
|
@codex re-review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
danny-avila
left a comment
danny-avila
left a comment
There was a problem hiding this comment.
Additional review findings.
| } | ||
|
|
||
| config.maxRedirects = 0; | ||
| if (config.httpsAgent || config.httpAgent || config.proxy) { |
There was a problem hiding this comment.
[P1] The proxy or agent early return precedes the literal private-IP check. Because each call site invokes applyAxiosProxyConfig first, applySSRFSafeAgentIfDirect({ proxy: ... }, 'http://169.254.169.254') returns without ESSRF; Axios asks the proxy to fetch the metadata target. Move literal validation above this return. For hostnames, require an SSRF-enforcing trusted proxy or add a preflight guard.
| applyAxiosProxyConfig(config, `${baseURL}/files`); | ||
| applySSRFSafeAgentIfDirect(config, `${baseURL}/files`, allowedAddresses); | ||
|
|
||
| const fileStream = fs.createReadStream(filePath); |
There was a problem hiding this comment.
[P2] Moving validation before createReadStream fixes only synchronous URL and literal-IP failures. A hostname that resolves privately is rejected later by the agent during axios.post; Axios destroys the request but not this FormData source, leaving the paused ReadStream and fd open. Destroy fileStream in finally and add an async DNS ESSRF cleanup test.
| if (isIP(literalHost)) { | ||
| const exemptSet = normalizeAllowedAddressesSet(allowedAddresses); | ||
| const normalizedPort = normalizePort(port || (protocol === 'https:' ? '443' : '80')); | ||
| const hostnameAllowed = isAddressInAllowedSet(literalHost, exemptSet, normalizedPort); |
There was a problem hiding this comment.
[P2] IPv6 exemptions are compared textually after URL canonicalization. The schema accepts [::ffff:127.0.0.1]:8080, but new URL(...).hostname yields [::ffff:7f00:1]; expanded ULA forms compress similarly, so equivalent exemptions throw ESSRF. Canonicalize IP literals in both allowlist normalization and target comparison, then test mapped and expanded IPv6 forms.
Summary
The speech (STT/TTS) and OCR integrations issue outbound HTTP to operator-provided target URLs (
STTService.js,TTSService.js,packages/api/src/files/mistral/crud.ts) with onlyapplyAxiosProxyConfigattached, so they have no SSRF protection: a target URL, or a hostname that DNS-rebinds after a write-time check, that resolves to private, loopback, link-local, or cloud-metadata address space (169.254.169.254,metadata.google.internal, RFC1918, CGNAT) is reached by the server. This is the same CWE-918 class already fixed for the custom/modelsfetch (#13919), Actions, avatar, the OpenAI and Anthropic endpoint clients, and MCP; speech and OCR are the remaining unguarded clients of this kind in this repo.The fix reuses the established framework: it attaches
createSSRFSafeAgents(allowedAddresses)(from #11722, extended by #12933) to the axios config for each of these calls, validating the resolved IP at TCP-connect time so it holds against DNS rebinding that a write-time URL check cannot catch. Thespeech.stt.allowedAddresses,speech.tts.allowedAddresses, andocr.allowedAddressesfields (added in the stacked base PR) are threaded in as the per-address exemption, so operators pointing speech or OCR atlocalhostor a LAN host (LocalAI, a self-hosted Whisper server, a private Mistral-compatible OCR endpoint) can opt those exacthost:porttargets back in. Proxy precedence is preserved: if a proxy or agent is already set, the SSRF agents are not attached. Redirects are disabled on the guarded calls (maxRedirects: 0) so the connect-time check cannot be bypassed by a redirect to an internal host, and non-http(s) or unparseable target URLs are rejected before the request sofile:orgopher:cannot reach the client.Scope, stated plainly: this addresses private/internal SSRF only. It does not address forwarding an operator credential to a public host, since a private-IP guard never fires on a public address; restricting which public hosts are reachable is a separate control (a positive-host egress allowlist) and is not part of this PR.
Accepted tradeoff: with no
allowedAddressesconfigured, requests to private or link-local targets now fail closed, matching howendpoints,actions, and MCP already behave. Self-hosters using alocalhostspeech or OCR endpoint add a one-line exemption, documented inlibrechat.example.yaml.allowedAddressesentries are trusted before the private-IP check, so operators must list only hosts they fully control and that cannot be repointed or DNS-rebound by an attacker.Change Type
Testing
Tests are behavioral rather than mock-only: the ESSRF cases exercise the real
createSSRFSafeAgentslookup against a private target, since a fully-mocked axios would not catch a missing exemption, a missingmaxRedirects: 0, or an agent that does not actually block a private connect.packages/api/src/auth/agent.spec.ts: with an http(s) URL and no proxy/agent, both agents are attached andconfig.maxRedirects === 0; a target resolving to a private IP rejects witherr.code === 'ESSRF'through the real lookup; ahost:portinallowedAddressessucceeds; a non-http(s) URL (file:,gopher:) or malformed URL is rejected by the helper's scheme/parse guard; when a proxy or pre-set agent is present, the agents are not attached andmaxRedirectsis still 0.packages/api/src/files/mistral/crud.spec.ts: the axios config now carrieshttpAgent/httpsAgentandmaxRedirects: 0(updated expectations); a private-IP baseURL rejects withESSRFthrough the real lookup; anocr.allowedAddresseshost:portis exempted; a pre-set proxy short-circuits withmaxRedirectsstill 0.ESSRF, anallowedAddresseshost:portsucceeds,maxRedirects === 0on the built config, and provider selection still resolves exactly one provider whenallowedAddressesis set.All added specs pass and
tsc --noEmitis clean forpackages/api.Checklist