Skip to content

fix: stop stripping credential headers from rewrite proxy requests #277

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.

Sign up for GitHub

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

Merged
southpolesteve merged 2 commits into from
Mar 7, 2026
+13 −24
Merged

fix: stop stripping credential headers from rewrite proxy requests #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f01de62
fix: stop stripping credential headers from rewrite proxy requests
17hz Mar 6, 2026
ae52c07
test: update proxy tests to assert credential headers are forwarded
17hz Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions packages/vinext/src/config/config-matchers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,12 +658,6 @@ export async function proxyExternalRequest(
headers.set("host", targetUrl.host);
// Remove headers that should not be forwarded to external services
headers.delete("connection");
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The credential-stripping removal here is correct — Next.js forwards all request headers (including cookie, authorization, etc.) to rewrite destinations via http-proxy with no filtering.

However, the existing test at tests/shims.test.ts:3707 ("strips credentials and x-middleware-* headers from proxied requests") asserts the old behavior and is now failing in CI. Please update it to verify that credential headers are preserved while x-middleware-* headers remain stripped.

// Strip credentials and internal headers to prevent leaking auth tokens,
// session cookies, and middleware internals to third-party origins.
headers.delete("cookie");
headers.delete("authorization");
headers.delete("x-api-key");
headers.delete("proxy-authorization");
const keysToDelete: string[] = [];
for (const key of headers.keys()) {
if (key.startsWith("x-middleware-")) {
Expand Down
6 changes: 0 additions & 6 deletions packages/vinext/src/server/app-dev-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1238,12 +1238,6 @@ async function __proxyExternalRequest(request, externalUrl) {
const headers = new Headers(request.headers);
headers.set("host", targetUrl.host);
headers.delete("connection");
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — correct change, but the integration test at tests/app-router.test.ts:2748 ("strips credential headers from proxied requests to external rewrite targets") is now failing. It needs to be updated to match the new forwarding behavior.

// Strip credentials and internal headers to prevent leaking auth tokens,
// session cookies, and middleware internals to third-party origins.
headers.delete("cookie");
headers.delete("authorization");
headers.delete("x-api-key");
headers.delete("proxy-authorization");
for (const key of [...headers.keys()]) {
if (key.startsWith("x-middleware-")) headers.delete(key);
}
Expand Down
12 changes: 6 additions & 6 deletions tests/app-router.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2910,7 +2910,7 @@ describe("App Router external rewrite proxy credential stripping", () => {
await new Promise<void>((resolve) => mockServer?.close(() => resolve()));
});

it("strips credential headers from proxied requests to external rewrite targets", async () => {
it("forwards credential headers through proxied requests to external rewrite targets", async () => {
mockResponseMode = "plain";
capturedHeaders = null;

Expand All @@ -2926,11 +2926,11 @@ describe("App Router external rewrite proxy credential stripping", () => {
});

expect(capturedHeaders).not.toBeNull();
// Credential headers must be stripped
expect(capturedHeaders!["cookie"]).toBeUndefined();
expect(capturedHeaders!["authorization"]).toBeUndefined();
expect(capturedHeaders!["x-api-key"]).toBeUndefined();
expect(capturedHeaders!["proxy-authorization"]).toBeUndefined();
// Credential headers must be forwarded (matching Next.js behavior)
expect(capturedHeaders!["cookie"]).toBe("session=secret123");
expect(capturedHeaders!["authorization"]).toBe("Bearer tok_secret");
expect(capturedHeaders!["x-api-key"]).toBe("sk_live_secret");
expect(capturedHeaders!["proxy-authorization"]).toBe("Basic cHJveHk=");
// Internal middleware headers must be stripped
expect(capturedHeaders!["x-middleware-next"]).toBeUndefined();
// Non-sensitive headers must be preserved
Expand Down
13 changes: 7 additions & 6 deletions tests/shims.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3813,7 +3813,7 @@ describe("proxyExternalRequest", () => {
}
});

it("strips credentials and x-middleware-* headers from proxied requests", async () => {
it("forwards credential headers and strips x-middleware-* headers from proxied requests", async () => {
const { proxyExternalRequest } = await import(
"../packages/vinext/src/config/config-matchers.js"
);
Expand Down Expand Up @@ -3842,11 +3842,12 @@ describe("proxyExternalRequest", () => {
try {
await proxyExternalRequest(request, "https://api.example.com/data");
expect(capturedHeaders).toBeDefined();
// Sensitive headers must be stripped
expect(capturedHeaders!.get("cookie")).toBeNull();
expect(capturedHeaders!.get("authorization")).toBeNull();
expect(capturedHeaders!.get("x-api-key")).toBeNull();
expect(capturedHeaders!.get("proxy-authorization")).toBeNull();
// Credential headers must be forwarded (matching Next.js behavior)
expect(capturedHeaders!.get("cookie")).toBe("session=secret123");
expect(capturedHeaders!.get("authorization")).toBe("Bearer tok_secret");
expect(capturedHeaders!.get("x-api-key")).toBe("sk_live_secret");
expect(capturedHeaders!.get("proxy-authorization")).toBe("Basic cHJveHk=");
// Internal middleware headers must be stripped
expect(capturedHeaders!.get("x-middleware-rewrite")).toBeNull();
expect(capturedHeaders!.get("x-middleware-next")).toBeNull();
// Non-sensitive headers must be preserved
Expand Down
Loading