Repository navigation
fix(security): trusted internal origin for provider auto-sync self-fetch (CodeQL #323 SSRF) #3336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| /** | ||
| * Regression guard — CodeQL js/request-forgery alert #323 (v3.8.13). | ||
| * | ||
| * POST /api/providers fires a non-blocking self-fetch to the connection's | ||
| * /sync-models route, forwarding the management cookie + internal sync auth | ||
| * headers. #3267 built that self-fetch origin from `new URL(request.url).origin` | ||
| * — i.e. the client-controlled Host header — so a caller could redirect the | ||
| * credential-bearing internal request to an arbitrary host (SSRF + internal | ||
| * auth-header exfiltration). | ||
| * | ||
| * The origin must come from the trusted loopback/env-pinned base URL | ||
| * (`getModelSyncInternalBaseUrl()`), never from the incoming request. | ||
| */ | ||
|
|
||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { readFileSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
|
|
||
| const routeSrc = readFileSync( | ||
| join(import.meta.dirname, "../../src/app/api/providers/route.ts"), | ||
| "utf8" | ||
| ); | ||
|
Comment on lines
+17
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
import { readFileSync } from "node:fs";
import { join, dirname } from "node:path";
import { fileURLToPath } from "node:url";
const __dirname = dirname(fileURLToPath(import.meta.url));
const routeSrc = readFileSync(
join(__dirname, "../../src/app/api/providers/route.ts"),
"utf8"
); |
||
|
|
||
| test("POST /api/providers auto-sync uses the trusted internal origin (not request.url) — #323", () => { | ||
| assert.ok( | ||
| routeSrc.includes("getModelSyncInternalBaseUrl()"), | ||
| "auto-sync self-fetch must derive its origin from getModelSyncInternalBaseUrl()" | ||
| ); | ||
| assert.doesNotMatch( | ||
| routeSrc, | ||
| /const\s+internalOrigin\s*=\s*new URL\(request\.url\)\.origin/, | ||
| "auto-sync origin must NOT be derived from the client-controlled request.url/Host (SSRF, CodeQL js/request-forgery #323)" | ||
| ); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
INTERNAL_BASE_URL(configured via environment variables likeBASE_URLorNEXT_PUBLIC_APP_URL) contains a trailing slash, appending/api/providers/...will result in a double slash (e.g.,https://example.com//api/providers/...). This can cause routing or redirection issues in Next.js, potentially dropping authentication headers. Normalizing the URL by stripping any trailing slash ensures robust request routing.