fix(import): apply SSRF request-filtering agent to import fetch#3565
fix(import): apply SSRF request-filtering agent to import fetch#3565ChristianTeroerde wants to merge 2 commits into
Conversation
Importer.getFile() fetched a user-supplied attachmentUrl (via GET /api/import/analyze) with raw node-fetch and no SSRF protection, while the attachments path already routes through getSsrfSafeAgents() (request-filtering-agent). Add getSsrfSafeFetchAgent() for node-fetch's (parsedUrl) => Agent factory and use it in the import fetch so attacker-supplied import URLs cannot reach private/link-local hosts (including via redirects). Honors TEABLE_SSRF_PROTECTION_DISABLED. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
![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: 8be690375b
ℹ️ 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".
| const { body: stream, headers } = await fetch(url, { | ||
| agent: getSsrfSafeFetchAgent(), | ||
| }); |
There was a problem hiding this comment.
When the UI uploads an import file with the default local storage provider, notify() returns a relative /api/attachments/read/... presigned URL for cookie-authenticated requests; this method then rewrites it to http://localhost:${process.env.PORT}... before fetching it. Adding the request-filtering agent here makes those normal uploaded-file imports resolve to loopback and get rejected as a private address, so /api/import/analyze and the subsequent import flow fail for the default local-storage path unless SSRF protection is disabled. The SSRF guard needs to exempt trusted first-party attachment URLs or read local attachments without going through a blocked localhost fetch.
Useful? React with 👍 / 👎.
… SSRF guard Review feedback: the request-filtering agent rejected the loopback address, so default local-storage imports (relative /api/attachments/read/... URLs that getFile() rewrites to http://localhost:PORT) would fail unless SSRF protection is disabled. Only apply the SSRF filter to externally-supplied absolute URLs. A relative reference is trusted as first-party only when its rewrite resolves to a genuine localhost host, so the loopback fetch bypasses the filter and default imports keep working. A crafted value such as "@169.254.169.254" resolves to a non-localhost host and stays filtered, so the guard is not bypassable through the rewrite branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
What
Importer.getFile()(the import "analyze" flow behindGET /api/import/analyze) fetches a user-suppliedattachmentUrlwith rawnode-fetchand no SSRF protection. The attachments path already routes its outbound fetches throughgetSsrfSafeAgents()(request-filtering-agent), but this import path does not — so an authenticated caller can make the server request arbitrary internal / link-local hosts, andnode-fetchwill follow redirects to them.Fix
getSsrfSafeFetchAgent()toutils/ssrf-guard.ts.node-fetch'sagentoption takes a(parsedUrl) => Agentfactory (unlike axios'httpAgent/httpsAgent), so it returns the http or https request-filtering agent per URL.fetch(url, { agent: getSsrfSafeFetchAgent() }).TEABLE_SSRF_PROTECTION_DISABLEDenv toggle; no behavior change for URLs that resolve to public hosts.This mirrors the protection the attachments path already applies, including redirect-based bypasses (e.g.
http://evil.com→http://169.254.169.254).Scope
Two files, +26/−1, no public API change:
apps/nestjs-backend/src/utils/ssrf-guard.ts(newgetSsrfSafeFetchAgent()helper)apps/nestjs-backend/src/features/import/open-api/import.class.ts(use it ingetFile())