fix(server): keep the client port in X-Forwarded-Host (#2288)

* fix(server): keep the client port in X-Forwarded-Host

The service proxy stripped the port when setting X-Forwarded-Host, so a
workspace service that derives its public origin from forwarded headers
emitted absolute URLs pointing at port 80. Opening a service at
http://api--branch--repo.localhost:6767/ and letting it redirect gave
back a Location without the port, which the browser followed to nothing.

Host was already forwarded intact, so services reading Host were fine.
Nothing rewrites Location on the way back, so the wrong URL reached the
browser unchanged.

The strip was not localhost-only: buildPublicServiceProxyUrl preserves
an explicit port in publicBaseUrl, so a public alias on :8443 hit the
same path. It was a no-op only on the default-port public case.

Forward the authority verbatim instead, and add X-Forwarded-Port under a
never-invent, never-clobber rule: report only a port observed in the
Host header, and leave any value an upstream proxy already set alone.
Deriving it from the scheme would overwrite nginx's port on a
non-default listener, and the upgrade path's hardcoded "http" would turn
a correct 443 into 80. An empty inbound value carries no port, so it
does not count as a value to preserve; out-of-range ports are dropped
rather than passed on, since the authority is client-supplied.

The header block existed in four copies. Collapse them: the subsystem
now delegates to createScriptProxyMiddleware and
createScriptProxyUpgradeHandler (passing passthroughUnknown explicitly,
since the factory defaults it to true), and both remaining call sites
share one buildForwardedHeaders helper.

Adds the first tests to cover any x-forwarded-* header on this path,
driven over a real proxy with an upstream that echoes what it received.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(server): let the observed authority win over a forged port

X-Forwarded-Port deferred to an inbound value, so a client connecting
directly could send Host: svc.localhost:63735 with a forged
X-Forwarded-Port: 443 and have services build URLs for 443. Frameworks
apply X-Forwarded-Port after X-Forwarded-Host, so the forged value won.

This was inconsistent with the line above it: X-Forwarded-Host is
overwritten with the real authority unconditionally, and there is a test
asserting that. Both headers carry the same trust, so both now follow
the same rule.

First-hand observation wins; an inbound value is a fallback, never an
override. A port parsed from Host replaces any inbound X-Forwarded-Port.
When Host carries no port there is nothing to observe, so a proxy's
value survives untouched: the nginx-on-:8443 case, where $host drops
the port and X-Forwarded-Port is the only source.

The empty-inbound-value special case is gone; always overwriting when we
have an observation covers it with one branch less.

Reported by Greptile on #2288. Not a regression: before this branch an
inbound X-Forwarded-Port passed through untouched, so the forged value
already reached services.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(server): correct the forwarded-port comment and note the limit

The comment claimed the parsed port was "the port the client really
connected on". It is not: parseHostHeaderPort reads the Host header,
which the client writes, and route lookup normalizes the port away
before matching, so any value reaches the proxy. The only observed
port would be req.socket.localPort, which this code does not use.

Replaced it with the actual reason the Host port wins, which is keeping
x-forwarded-host and x-forwarded-port from disagreeing: the former is
set from Host, and frameworks apply the latter afterwards, so a
mismatched inbound value would silently override the forwarded
authority.

Added a LIMITATION note recording that the forwarded authority is not
authenticated, that inbound x-forwarded-port is not checked against
trustedProxies, and that closing this needs that config threaded into
the subsystem and the upgrade path, which has no Express trust context.
Both predate this branch: Host has always carried a client-chosen port
and an inbound x-forwarded-port has always passed straight through.

Docs gain a matching section telling service authors to pin their public
origin in configuration rather than derive it from request headers.

No behavior change. Raised by Greptile on #2288.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Christoph Leiter
2026-07-24 15:17:16 +02:00
committed by GitHub
parent cf36b2cc40
commit e22c85373c
3 changed files with 357 additions and 114 deletions

View File

@@ -107,6 +107,29 @@ server {
}
```
Nginx's `$host` drops the port. If you terminate on a non-default port, use `$http_host` instead so the port survives — that is what "forwards the `Host` header unchanged" means here.
## Forwarded headers
Paseo sets these when it forwards a request to a workspace service:
| Header | Value |
| ------------------- | --------------------------------------------------------------------------------------------------------------------------------------- |
| `X-Forwarded-Host` | The `Host` header verbatim, including the port when the client used one |
| `X-Forwarded-Proto` | The request scheme (`http` on the WebSocket upgrade path) |
| `X-Forwarded-For` | The immediate peer address. Replaces any existing chain, so behind your own reverse proxy this is the proxy's address, not the client's |
| `X-Forwarded-Port` | The port from the `Host` header when it has one, otherwise whatever your proxy already set |
`X-Forwarded-Port` follows the same trust rule as `X-Forwarded-Host`: the authority Paseo observed wins. When the `Host` header carries a port, that port is reported and replaces any inbound `X-Forwarded-Port`, so a client cannot forge one. When `Host` carries no port there is nothing to observe, so a value your reverse proxy set survives untouched — that is the case where nginx's `$host` drops the port and `X-Forwarded-Port` is the only source. Paseo never derives the port from the scheme. Any other `X-Forwarded-*` header your proxy sends is passed through untouched.
Services that build absolute URLs should prefer `Host` or `X-Forwarded-Host`.
### The forwarded authority is not authenticated
Route lookup normalizes the port away before matching a service hostname, so a client can address the daemon with any port in `Host` and still reach the service. That port is what lands in `X-Forwarded-Host` and `X-Forwarded-Port`. Paseo also does not check whether an inbound `X-Forwarded-Port` came from a proxy in `trustedProxies` — when `Host` carries no port, a client-supplied value is passed through.
Treat the forwarded authority as client-influenced input. A service that builds password reset links, absolute redirects, or cached URLs from it should pin its own public origin in configuration rather than deriving one from request headers. This is not specific to `X-Forwarded-Port`: the `Host` header has always carried a client-chosen port.
## Environment variables
The listen address and public base URL can also be set via environment variables, which take precedence over `config.json`:

View File

@@ -1,6 +1,7 @@
import { readdirSync, readFileSync, statSync } from "node:fs";
import path from "node:path";
import http from "node:http";
import net from "node:net";
import express from "express";
import { describe, expect, it } from "vitest";
import pino from "pino";
@@ -30,10 +31,16 @@ function readServerSourceFiles(dir = path.resolve(import.meta.dirname)): string[
return entries;
}
function httpGet(port: number, host: string, requestPath = "/api/health") {
interface HttpGetOptions {
path?: string;
headers?: Record<string, string>;
}
function httpGet(port: number, host: string, options: HttpGetOptions = {}) {
const { path: requestPath = "/api/health", headers: extraHeaders = {} } = options;
return new Promise<{ status: number; body: string }>((resolve, reject) => {
const req = http.get(
{ hostname: "127.0.0.1", port, path: requestPath, headers: { host } },
{ hostname: "127.0.0.1", port, path: requestPath, headers: { host, ...extraHeaders } },
(res) => {
let body = "";
res.on("data", (chunk: Buffer) => {
@@ -294,3 +301,259 @@ describe("service proxy subsystem shape", () => {
});
});
});
interface ForwardedFixture {
daemonPort: number;
hostname: string;
close(): Promise<void>;
}
/**
* Runs a real workspace service behind a real daemon listener. The upstream
* echoes the headers it received so tests can assert what actually crossed the
* proxy, not what a helper returned.
*/
async function startForwardedHeadersFixture(): Promise<ForwardedFixture> {
const upstreamPort = await findFreePort();
const upstream = http.createServer((req, res) => {
res.writeHead(200, { "content-type": "application/json" });
res.end(JSON.stringify(req.headers));
});
const upgradeSockets: net.Socket[] = [];
upstream.on("upgrade", (req, socket) => {
upgradeSockets.push(socket);
// The client resets this connection once it has the echo, so the reset
// reaches us as an 'error'. A raw upgrade socket has no default handler —
// without this the event goes unhandled and takes down the test process.
socket.on("error", () => socket.destroy());
const payload = JSON.stringify(req.headers);
socket.write(
`HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nX-Echo-Length: ${payload.length}\r\n\r\n${payload}`,
);
});
await new Promise<void>((resolve) => upstream.listen(upstreamPort, "127.0.0.1", resolve));
const serviceProxy = createServiceProxySubsystem({ logger });
const route = serviceProxy.registerWorkspaceService({
workspaceId: "workspace-a",
projectSlug: "repo",
branchName: "feature",
scriptName: "api",
port: upstreamPort,
});
const daemonPort = await findFreePort();
const app = express();
app.set("trust proxy", true);
app.use(serviceProxy.middleware());
app.use((_req, res) => {
res.status(404).send("404 Not Found");
});
const daemon = http.createServer(app);
daemon.on("upgrade", serviceProxy.upgradeHandler({ passthroughUnknown: false }));
await new Promise<void>((resolve) => daemon.listen(daemonPort, "127.0.0.1", resolve));
return {
daemonPort,
hostname: route.hostname,
async close() {
// Upgraded sockets keep the server alive; close() alone would hang.
for (const socket of upgradeSockets) socket.destroy();
daemon.closeAllConnections();
await new Promise<void>((resolve) => daemon.close(() => resolve()));
upstream.closeAllConnections();
await new Promise<void>((resolve) => upstream.close(() => resolve()));
},
};
}
function upgradeThroughProxy(
port: number,
host: string,
extraHeaders: Record<string, string> = {},
): Promise<Record<string, string | undefined>> {
return new Promise((resolve, reject) => {
const socket = net.connect({ host: "127.0.0.1", port }, () => {
const lines = [
"GET /ws HTTP/1.1",
`Host: ${host}`,
"Upgrade: websocket",
"Connection: Upgrade",
...Object.entries(extraHeaders).map(([key, value]) => `${key}: ${value}`),
"",
"",
];
socket.write(lines.join("\r\n"));
});
let raw = "";
let settled = false;
function fail(reason: string) {
if (settled) return;
settled = true;
socket.destroy();
reject(new Error(`${reason} (received ${raw.length} bytes)`));
}
socket.on("data", (chunk: Buffer) => {
raw += chunk.toString();
const separator = raw.indexOf("\r\n\r\n");
if (separator === -1) return;
const body = raw.slice(separator + 4);
const lengthMatch = /x-echo-length: (\d+)/i.exec(raw.slice(0, separator));
if (!lengthMatch || body.length < Number(lengthMatch[1])) return;
settled = true;
socket.destroy();
resolve(JSON.parse(body) as Record<string, string | undefined>);
});
// Without these the promise stays pending forever when the proxy drops the
// upgrade, and the test reports a timeout instead of the real failure.
socket.on("close", () => fail("upgrade closed before the echo arrived"));
socket.on("error", (err) => {
if (settled) return;
settled = true;
socket.destroy();
reject(err);
});
});
}
describe("service proxy forwarded headers", () => {
it("forwards the client authority with its port so services build reachable URLs", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(
fixture.daemonPort,
`${fixture.hostname}:${fixture.daemonPort}`,
{ path: "/" },
);
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers.host).toBe(`${fixture.hostname}:${fixture.daemonPort}`);
expect(headers["x-forwarded-host"]).toBe(`${fixture.hostname}:${fixture.daemonPort}`);
expect(headers["x-forwarded-port"]).toBe(String(fixture.daemonPort));
expect(headers["x-forwarded-proto"]).toBe("http");
} finally {
await fixture.close();
}
});
it("does not invent a port when the client authority has none", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(fixture.daemonPort, fixture.hostname, {
path: "/",
headers: { "x-forwarded-proto": "https" },
});
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-host"]).toBe(fixture.hostname);
expect(headers["x-forwarded-port"]).toBeUndefined();
expect(headers["x-forwarded-proto"]).toBe("https");
} finally {
await fixture.close();
}
});
it("keeps a port an upstream proxy already reported", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(fixture.daemonPort, fixture.hostname, {
path: "/",
headers: { "x-forwarded-proto": "https", "x-forwarded-port": "8443" },
});
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-port"]).toBe("8443");
} finally {
await fixture.close();
}
});
it("overrides a client-supplied forwarded port with the observed authority port", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(
fixture.daemonPort,
`${fixture.hostname}:${fixture.daemonPort}`,
{ path: "/", headers: { "x-forwarded-port": "443" } },
);
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-port"]).toBe(String(fixture.daemonPort));
} finally {
await fixture.close();
}
});
it("reports the real port when a client sends an empty forwarded port", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(
fixture.daemonPort,
`${fixture.hostname}:${fixture.daemonPort}`,
{ path: "/", headers: { "x-forwarded-port": "" } },
);
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-port"]).toBe(String(fixture.daemonPort));
} finally {
await fixture.close();
}
});
it("ignores an out-of-range port in the client authority", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(fixture.daemonPort, `${fixture.hostname}:99999999`, {
path: "/",
});
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-host"]).toBe(`${fixture.hostname}:99999999`);
expect(headers["x-forwarded-port"]).toBeUndefined();
} finally {
await fixture.close();
}
});
it("overwrites a client-supplied forwarded host with the real authority", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const response = await httpGet(
fixture.daemonPort,
`${fixture.hostname}:${fixture.daemonPort}`,
{ path: "/", headers: { "x-forwarded-host": "attacker.example.com" } },
);
const headers = JSON.parse(response.body) as Record<string, string | undefined>;
expect(headers["x-forwarded-host"]).toBe(`${fixture.hostname}:${fixture.daemonPort}`);
} finally {
await fixture.close();
}
});
it("applies the same forwarded-header rules to WebSocket upgrades", async () => {
const fixture = await startForwardedHeadersFixture();
try {
const withPort = await upgradeThroughProxy(
fixture.daemonPort,
`${fixture.hostname}:${fixture.daemonPort}`,
);
expect(withPort["x-forwarded-host"]).toBe(`${fixture.hostname}:${fixture.daemonPort}`);
expect(withPort["x-forwarded-port"]).toBe(String(fixture.daemonPort));
const behindTls = await upgradeThroughProxy(fixture.daemonPort, fixture.hostname, {
"X-Forwarded-Proto": "https",
"X-Forwarded-Port": "443",
});
expect(behindTls["x-forwarded-host"]).toBe(fixture.hostname);
expect(behindTls["x-forwarded-port"]).toBe("443");
// Known limitation, tracked separately: the upgrade path hardcodes the
// scheme, so an HTTPS proxy's "https" is replaced with "http" even though
// its port survives. Asserted rather than hidden so the day it is fixed
// this test fails loudly instead of silently passing.
expect(behindTls["x-forwarded-proto"]).toBe("http");
} finally {
await fixture.close();
}
});
});

View File

@@ -256,6 +256,65 @@ function stripHopByHopHeaders(
return out;
}
const MAX_TCP_PORT = 65535;
function parseHostHeaderPort(hostHeader: string): string | null {
const match = /:(\d+)$/.exec(hostHeader);
if (!match) {
return null;
}
// The authority is client-supplied — route lookup strips the port before
// matching, so any digits reach us. Only pass on something that could be a
// real port rather than handing services a number they will build URLs from.
const port = Number(match[1]);
return port >= 1 && port <= MAX_TCP_PORT ? match[1] : null;
}
function buildForwardedHeaders({
req,
route,
protocol,
}: {
req: IncomingMessage;
route: ServiceProxyRoute;
protocol: string;
}): Record<string, string | string[]> {
const hostHeader = String(req.headers.host ?? route.hostname);
const forwardedHeaders = stripHopByHopHeaders(req.headers);
forwardedHeaders["x-forwarded-for"] = req.socket.remoteAddress ?? "127.0.0.1";
// Forward the authority verbatim, port included. Services derive their public
// origin from this header, so a stripped port makes them emit absolute URLs
// (redirects, links) pointing at port 80. Route lookup already normalized the
// port away, which also means the port here is client-chosen: upstream apps
// must treat the forwarded authority as untrusted input.
forwardedHeaders["x-forwarded-host"] = hostHeader;
forwardedHeaders["x-forwarded-proto"] = protocol;
// Keep the two headers consistent: x-forwarded-host is set from Host just
// above, and frameworks apply x-forwarded-port afterwards, so an inbound port
// that disagrees would silently win over the authority we forwarded. The port
// in Host therefore takes precedence. When Host carries no port there is
// nothing to derive from and an upstream proxy's value survives — the
// nginx-on-:8443 case, where $host drops the port and x-forwarded-port is the
// only source. Never derive the port from the scheme: the upgrade path
// hardcodes "http", so defaulting would turn a correct 443 into 80.
//
// LIMITATION: none of this is authenticated, and the port in Host is not an
// observation of the connection — it is whatever the client wrote, since
// route lookup normalizes the port away before matching. Paseo also does not
// check whether an inbound x-forwarded-port arrived from a configured trusted
// proxy. Services must treat the forwarded authority as client-influenced
// input, not as a trusted origin. This predates the header handling here:
// Host has always carried a client-chosen port, and an inbound
// x-forwarded-port has always passed straight through. Closing it means
// threading the daemon's trustedProxies into this subsystem and into the
// upgrade path, which has no Express trust context. Tracked separately.
const port = parseHostHeaderPort(hostHeader);
if (port !== null) {
forwardedHeaders["x-forwarded-port"] = port;
}
return forwardedHeaders;
}
function proxyHttpRequest({
req,
res,
@@ -267,11 +326,7 @@ function proxyHttpRequest({
route: ServiceProxyRoute;
logger: Logger;
}): void {
const hostHeader = req.headers.host ?? route.hostname;
const forwardedHeaders = stripHopByHopHeaders(req.headers);
forwardedHeaders["x-forwarded-for"] = req.socket.remoteAddress ?? "127.0.0.1";
forwardedHeaders["x-forwarded-host"] = String(hostHeader).replace(/:\d+$/, "");
forwardedHeaders["x-forwarded-proto"] = req.protocol;
const forwardedHeaders = buildForwardedHeaders({ req, route, protocol: req.protocol });
const proxyReq = http.request(
{
@@ -313,12 +368,8 @@ function proxyUpgradeRequest({
route: ServiceProxyRoute;
logger: Logger;
}): void {
const hostHeader = req.headers.host ?? route.hostname;
const targetSocket = net.connect({ host: "127.0.0.1", port: route.port }, () => {
const forwardedHeaders = stripHopByHopHeaders(req.headers);
forwardedHeaders["x-forwarded-for"] = req.socket.remoteAddress ?? "127.0.0.1";
forwardedHeaders["x-forwarded-host"] = String(hostHeader).replace(/:\d+$/, "");
forwardedHeaders["x-forwarded-proto"] = "http";
const forwardedHeaders = buildForwardedHeaders({ req, route, protocol: "http" });
forwardedHeaders.connection = "Upgrade";
forwardedHeaders.upgrade = req.headers.upgrade ?? "websocket";
@@ -867,33 +918,19 @@ class NodeServiceProxySubsystem implements ServiceProxySubsystem {
}
middleware(): RequestHandler {
return (req, res, next) => {
const classification = this.routes.classifyHost(req.headers.host);
if (classification.type === "daemon") {
next();
return;
}
if (classification.type === "known-service-miss") {
res.status(404).send("404 Not Found");
return;
}
this.proxyHttpRequest(req, res, classification.route);
};
return createScriptProxyMiddleware({ routeStore: this.routes, logger: this.logger });
}
upgradeHandler(options: {
passthroughUnknown: boolean;
}): (req: IncomingMessage, socket: net.Socket, head: Buffer) => void {
return (req, socket, head) => {
const classification = this.routes.classifyHost(req.headers.host);
if (classification.type !== "registered-service") {
if (!options.passthroughUnknown) {
socket.destroy();
}
return;
}
this.proxyUpgradeRequest(req, socket, head, classification.route);
};
// Pass passthroughUnknown explicitly: the factory defaults it to true, the
// subsystem requires callers to choose.
return createScriptProxyUpgradeHandler({
routeStore: this.routes,
logger: this.logger,
passthroughUnknown: options.passthroughUnknown,
});
}
async startStandalone(options: {
@@ -938,86 +975,6 @@ class NodeServiceProxySubsystem implements ServiceProxySubsystem {
unlinkSync(listenTarget.path);
}
}
private proxyHttpRequest(
req: Parameters<RequestHandler>[0],
res: Parameters<RequestHandler>[1],
route: ServiceProxyRoute,
): void {
const hostHeader = req.headers.host ?? route.hostname;
const forwardedHeaders = stripHopByHopHeaders(req.headers);
forwardedHeaders["x-forwarded-for"] = req.socket.remoteAddress ?? "127.0.0.1";
forwardedHeaders["x-forwarded-host"] = String(hostHeader).replace(/:\d+$/, "");
forwardedHeaders["x-forwarded-proto"] = req.protocol;
const proxyReq = http.request(
{
hostname: "127.0.0.1",
port: route.port,
path: req.originalUrl,
method: req.method,
headers: forwardedHeaders,
},
(proxyRes) => {
const responseHeaders = stripHopByHopHeaders(proxyRes.headers);
res.writeHead(proxyRes.statusCode ?? 502, responseHeaders);
proxyRes.pipe(res, { end: true });
},
);
proxyReq.on("error", (err) => {
this.logger.warn(
{ err, hostname: route.hostname, port: route.port },
"Service proxy: upstream unreachable",
);
if (!res.headersSent) {
res.writeHead(502, { "content-type": "text/plain" });
res.end("502 Bad Gateway");
}
});
req.pipe(proxyReq, { end: true });
}
private proxyUpgradeRequest(
req: IncomingMessage,
socket: net.Socket,
head: Buffer,
route: ServiceProxyRoute,
): void {
const hostHeader = req.headers.host ?? route.hostname;
const targetSocket = net.connect({ host: "127.0.0.1", port: route.port }, () => {
const forwardedHeaders = stripHopByHopHeaders(req.headers);
forwardedHeaders["x-forwarded-for"] = req.socket.remoteAddress ?? "127.0.0.1";
forwardedHeaders["x-forwarded-host"] = String(hostHeader).replace(/:\d+$/, "");
forwardedHeaders["x-forwarded-proto"] = "http";
forwardedHeaders.connection = "Upgrade";
forwardedHeaders.upgrade = req.headers.upgrade ?? "websocket";
const headerLines: string[] = [];
headerLines.push(`${req.method ?? "GET"} ${req.url ?? "/"} HTTP/${req.httpVersion}`);
for (const [key, value] of Object.entries(forwardedHeaders)) {
if (Array.isArray(value)) {
for (const v of value) headerLines.push(`${key}: ${v}`);
} else {
headerLines.push(`${key}: ${value}`);
}
}
headerLines.push("\r\n");
targetSocket.write(headerLines.join("\r\n"));
if (head.length > 0) targetSocket.write(head);
targetSocket.pipe(socket);
socket.pipe(targetSocket);
});
targetSocket.on("error", (err) => {
this.logger.warn(
{ err, hostname: route.hostname, port: route.port },
"Service proxy: WebSocket upstream unreachable",
);
socket.end();
});
socket.on("error", () => {
targetSocket.destroy();
});
}
}
function listen(