diff --git a/docs/configuration.md b/docs/configuration.md index 408018c7f..4638477bd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -89,6 +89,13 @@ or `[::1]`, with optional ports. Restart DevSpace after changing `oauth.allowedResourceUrls`: the provider reads this policy at server creation. After restarting, refresh tokens for removed aliases can no longer mint tokens. +`server.trustProxy` controls how DevSpace derives the client IP used for +request logs and OAuth rate limiting. `false` ignores forwarding headers. +`"loopback"` trusts `X-Forwarded-For` only from a proxy on the same machine, +such as `cloudflared` or a local reverse proxy; this is the recommended value +for tunnel setups. `true` trusts every hop, so any client can choose its own IP +by sending `X-Forwarded-For`. + ## Tool modes and UI `tools.mode` accepts two values: diff --git a/schema/v1/devspace.schema.json b/schema/v1/devspace.schema.json index cf9ce8f84..5791288a3 100644 --- a/schema/v1/devspace.schema.json +++ b/schema/v1/devspace.schema.json @@ -51,7 +51,15 @@ }, "trustProxy": { "default": false, - "type": "boolean" + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string", + "const": "loopback" + } + ] } }, "additionalProperties": false diff --git a/src/config-schema.ts b/src/config-schema.ts index 95141fbd9..df758fc18 100644 --- a/src/config-schema.ts +++ b/src/config-schema.ts @@ -10,7 +10,7 @@ const serverConfigSchema = z.object({ port: z.number().int().min(1).max(65_535).default(7676), publicBaseUrl: z.string().url().nullable().default(null), allowedHosts: z.array(z.string().trim().min(1)).default([]), - trustProxy: z.boolean().default(false), + trustProxy: z.union([z.boolean(), z.literal("loopback")]).default(false), }).strict().prefault({}); const workspacesConfigSchema = z.object({ diff --git a/src/config.test.ts b/src/config.test.ts index ddb2fc1c3..2792a1c4d 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -121,6 +121,13 @@ try { }); assert.equal(loadConfig(env).oauth.ownerToken, env.DEVSPACE_OAUTH_OWNER_TOKEN); + + writeDevspaceConfig({ configVersion: 1, server: { trustProxy: "loopback" } }, env); + assert.equal(loadConfig(env).logging.trustProxy, "loopback"); + assert.throws( + () => writeDevspaceConfig({ configVersion: 1, server: { trustProxy: "uniquelocal" as never } }, env), + /trustProxy/, + ); } finally { rmSync(configDir, { recursive: true, force: true }); } diff --git a/src/logger.test.ts b/src/logger.test.ts new file mode 100644 index 000000000..123b26b38 --- /dev/null +++ b/src/logger.test.ts @@ -0,0 +1,41 @@ +import assert from "node:assert/strict"; +import type { AddressInfo } from "node:net"; +import test from "node:test"; +import express from "express"; +import { requestIp, type TrustProxyMode } from "./logger.js"; + +async function observedIps(trustProxy: TrustProxyMode): Promise<{ reqIp: string; logged: string }> { + const app = express(); + if (trustProxy) app.set("trust proxy", trustProxy); + app.get("/", (req, res) => { + res.json({ reqIp: req.ip, logged: requestIp(req, trustProxy) }); + }); + + const server = app.listen(0, "127.0.0.1"); + await new Promise((resolve) => server.once("listening", resolve)); + try { + const { port } = server.address() as AddressInfo; + // The client supplies the leftmost hop and a CF header; the local proxy appends the real peer. + const response = await fetch(`http://127.0.0.1:${port}/`, { + headers: { + "cf-connecting-ip": "192.0.2.99", + "x-forwarded-for": "198.51.100.66, 203.0.113.7", + }, + }); + return await response.json() as { reqIp: string; logged: string }; + } finally { + server.close(); + } +} + +test("disabled trust proxy ignores forwarding headers", async () => { + assert.deepEqual(await observedIps(false), { reqIp: "127.0.0.1", logged: "127.0.0.1" }); +}); + +test("loopback trust proxy uses the hop appended by the local proxy", async () => { + assert.deepEqual(await observedIps("loopback"), { reqIp: "203.0.113.7", logged: "203.0.113.7" }); +}); + +test("full trust proxy accepts client-supplied forwarding headers", async () => { + assert.deepEqual(await observedIps(true), { reqIp: "198.51.100.66", logged: "192.0.2.99" }); +}); diff --git a/src/logger.ts b/src/logger.ts index 2bcd2d261..7c7d160db 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -2,6 +2,7 @@ import type { Request } from "express"; export type LogLevel = "silent" | "error" | "warn" | "info" | "debug"; export type LogFormat = "json" | "pretty"; +export type TrustProxyMode = boolean | "loopback"; export interface LoggingConfig { level: LogLevel; @@ -10,7 +11,7 @@ export interface LoggingConfig { assets: boolean; toolCalls: boolean; shellCommands: boolean; - trustProxy: boolean; + trustProxy: TrustProxyMode; } type LogFields = Record; @@ -52,8 +53,10 @@ export function logEvent( } } -export function requestIp(req: Request, trustProxy: boolean): string | undefined { - if (trustProxy) { +export function requestIp(req: Request, trustProxy: TrustProxyMode): string | undefined { + // "loopback" lets Express resolve req.ip from hops appended by a local proxy; + // the raw headers here would return client-supplied values. + if (trustProxy === true) { const cfConnectingIp = firstHeaderValue(req.header("cf-connecting-ip")); if (cfConnectingIp) return cfConnectingIp; diff --git a/src/server.ts b/src/server.ts index 0a2792f3f..5a618e635 100644 --- a/src/server.ts +++ b/src/server.ts @@ -862,7 +862,7 @@ export function createServer( }); if (config.logging.trustProxy) { - app.set("trust proxy", true); + app.set("trust proxy", config.logging.trustProxy); } app.use((req, res, next) => { @@ -1002,7 +1002,7 @@ if (await isMainModule()) { console.log(`logging: ${config.logging.level} ${config.logging.format}`); console.log(`request logging: ${config.logging.requests ? "enabled" : "disabled"}`); console.log(`asset logging: ${config.logging.assets ? "enabled" : "disabled"}`); - console.log(`trust proxy: ${config.logging.trustProxy ? "enabled" : "disabled"}`); + console.log(`trust proxy: ${config.logging.trustProxy === true ? "enabled" : config.logging.trustProxy || "disabled"}`); const artifactDownloadStatus = !config.artifactsEnabled ? "disabled" : isArtifactDownloadSupportedPlatform()