From b292a93376a0b39d3ced2f5dd87a8128df3080dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Sat, 18 Apr 2026 06:03:30 +0200 Subject: [PATCH] fix(server): honor X-Forwarded-* via configurable trust proxy (#19824) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The bug Pasting `https://.twenty.com/mcp` into an MCP client (Claude connector, etc.) fails discovery. Curl shows why: ```bash $ curl -si https://twentyfortwenty.twenty.com/.well-known/oauth-protected-resource HTTP/2 200 ... { \"resource\": \"http://{workspace}.twenty.com/mcp\", \"authorization_servers\": [\"http://twentyfortwenty.twenty.com\"], ... } ``` The response advertises `http://` even though the request came in on `https://`. RFC 9728 / RFC 8707 require the client to validate that the advertised `resource` matches the URL it connected to, so strict MCP clients reject the mismatch and OAuth never starts. ## Why request.protocol returns \"http\" Per [Express docs](https://expressjs.com/en/guide/behind-proxies.html), `request.protocol` returns the socket-level protocol unless `app.set('trust proxy', ...)` is configured. In our deployment: ``` client -- https --> Cloudflare -- https --> ingress-nginx -- http --> NestJS pod ``` TLS is terminated at the edge. The upstream TCP connection into the pod is plain HTTP, and nginx sets `X-Forwarded-Proto: https` for the pod to read. Without a `trust proxy` setting, Express ignores `X-Forwarded-Proto` and `request.protocol === 'http'`. `main.ts` currently has no `app.set('trust proxy', ...)` call anywhere. ## Why this only surfaced now `grep -rn request.protocol` finds three pre-existing call sites — `RestApiMetadataService`, `OpenApiService`, `RouteTriggerService`. All three wrap it in `getServerUrl({ serverUrlEnv: SERVER_URL, serverUrlFallback: \`${request.protocol}://${request.get('host')}\` })`, which returns `SERVER_URL` whenever it's non-empty. In production `SERVER_URL` is always set (e.g. \`api.twenty.com\`), so the \`request.protocol\` branch is effectively dead code there. #19755 introduced the first call site that uses `request.protocol` unconditionally — the OAuth discovery controller has to echo the request host, because the whole point is supporting multiple paste-able origins (workspace subdomains, custom domains, etc.). That's why this is the first \"wrong protocol\" bug anyone has seen in our app. ## The fix One line in `main.ts`: ```ts app.set('trust proxy', twentyConfigService.get('TRUST_PROXY')); ``` Backed by a new `TRUST_PROXY` env var with a default. `request.protocol` then honors `X-Forwarded-Proto`, `request.ip` honors `X-Forwarded-For`, etc. OAuth discovery URLs come out on the right scheme, and any future `request.protocol` callers Just Work. ## Why this needs to be configurable (not hardcoded) Twenty is open-source and deployed in at least three distinct topologies: 1. **Kubernetes with ingress** (us, enterprise self-hosters) — TLS terminated upstream, needs `trust proxy` **on**. 2. **Self-host behind a user-supplied reverse proxy** (Caddy, Traefik, nginx — our [recommended setup](https://twenty.com/developers/section/self-hosting)) — same as above, needs `trust proxy` **on**. 3. **Self-host with NestJS exposed directly to the internet** — no upstream proxy, needs `trust proxy` **off** (otherwise any curl with `X-Forwarded-For: 1.2.3.4` spoofs `request.ip`, poisoning rate-limiters and audit logs). There is no single static value that's correct for all three. Express makes this a setting for exactly this reason — we follow suit. ## Why the default is `'loopback, linklocal, uniquelocal'` Shorthand for loopback (127/8, ::1), link-local (169.254/16, fe80::/10), and unique-local (10/8, 172.16/12, 192.168/16, fc00::/7). In practical terms: **trust peers coming from private networks; don't trust the public internet**. This default is correct for shapes 1 and 2 (cloud, proxied self-host) because the ingress/proxy peer is always a private-network IP in every sane deployment. For shape 3 (directly exposed), the default is still safe because public clients have public IPs, which are not in any of those ranges — so `X-Forwarded-For` from an attacker on the internet is ignored. The only way to be bitten is the exotic case where a public client reaches NestJS through a private-network hop that isn't a proxy (e.g. a NAT appliance that forwards to the pod on a private IP and blindly appends headers). Narrow attack surface, and an operator running that kind of setup is expected to configure `TRUST_PROXY=false` explicitly. \"Safer than the naïve `true`, more useful than `false`\" — this matches what Rails, Django, and many other frameworks recommend for Kubernetes-style deployments. ## Why an env var instead of hardcoded - Rejecting hardcoded `true`: would expose shape-3 self-hosters to IP spoofing without a way to opt out. - Rejecting hardcoded `false`: would leave cloud + shape-2 self-hosters broken, same bug as today. - Accepting string-typed env (not boolean): Express's `trust proxy` accepts booleans, hop counts (`1`, `2`), IP ranges (`'10.0.0.0/8'`), and named CIDRs (`'loopback'`). A boolean would hide that flexibility; operators occasionally need the richer values. The string maps 1:1 onto what Express accepts. ## Deployment matrix | Deployment | Default works? | Override needed? | |---|---|---| | Cloud (us, K8s + nginx ingress + Cloudflare) | ✓ | — | | Self-host behind reverse proxy (recommended) | ✓ | — | | Self-host exposed directly on public IP | ✓ (public IPs not in private ranges) | Optional: `TRUST_PROXY=false` for strictness | | Local dev (direct, no proxy) | ✓ (no `X-Forwarded-*` headers arrive) | — | | Exotic: multi-hop through non-sanitizing private-network middlebox | Risky | `TRUST_PROXY=false` | ## Related - Blocks MCP connector OAuth on `.twenty.com` / custom domains. After deploy: `curl -s https://.twenty.com/.well-known/oauth-protected-resource | jq .resource` should return `https://...` (not `http://...`). - Fixes latent issue in `RestApiMetadataService`, `OpenApiService`, `RouteTriggerService` fallback paths (pre-existing but dead in production because `SERVER_URL` is always set — no behavior change there). ## Test plan - [x] `tsc --noEmit` clean - [ ] After deploy: `curl -s https://twentyfortwenty.twenty.com/.well-known/oauth-protected-resource` returns `https://` URLs - [ ] After deploy: MCP connector in Claude successfully completes OAuth against `https://.twenty.com/mcp` - [ ] No change in `request.ip` logging behavior on cloud (nginx-ingress peer is already private-network, was already being trusted implicitly by every framework layer that wasn't `request.protocol`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 --- .../twenty-config/config-variables.ts | 17 +++++++++++++++++ packages/twenty-server/src/main.ts | 8 ++++++++ 2 files changed, 25 insertions(+) diff --git a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts index 5c153b3eef..78c7e51053 100644 --- a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts @@ -1126,6 +1126,23 @@ export class ConfigVariables { @IsOptional() SERVER_URL = 'http://localhost:3000'; + @ConfigVariablesMetadata({ + group: ConfigVariablesGroup.SERVER_CONFIG, + description: + 'Express "trust proxy" setting. Controls whether X-Forwarded-* ' + + 'headers are honored — required for request.protocol to return ' + + '"https" when TLS is terminated upstream (reverse proxy, ingress, ' + + 'Cloudflare, etc.). Default trusts loopback + RFC1918/ULA peers, ' + + 'which is correct when NestJS runs behind a reverse proxy (our ' + + 'recommended self-host setup). Set to "false" when NestJS is ' + + 'exposed directly to the internet. Accepts any value Express ' + + 'supports — see https://expressjs.com/en/guide/behind-proxies.html.', + type: ConfigVariableType.STRING, + isEnvOnly: true, + }) + @IsOptional() + TRUST_PROXY: string = 'loopback, linklocal, uniquelocal'; + @ConfigVariablesMetadata({ group: ConfigVariablesGroup.SERVER_CONFIG, description: diff --git a/packages/twenty-server/src/main.ts b/packages/twenty-server/src/main.ts index 0265aca64d..13b2cd708e 100644 --- a/packages/twenty-server/src/main.ts +++ b/packages/twenty-server/src/main.ts @@ -14,6 +14,7 @@ import { setPgDateTypeParser } from 'src/database/pg/set-pg-date-type-parser'; import { LoggerService } from 'src/engine/core-modules/logger/logger.service'; import { getSessionStorageOptions } from 'src/engine/core-modules/session-storage/session-storage.module-factory'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; +import { configTransformers } from 'src/engine/core-modules/twenty-config/utils/config-transformers.util'; import { UnhandledExceptionFilter } from 'src/filters/unhandled-exception.filter'; import { AppModule } from './app.module'; @@ -43,6 +44,13 @@ const bootstrap = async () => { const logger = app.get(LoggerService); const twentyConfigService = app.get(TwentyConfigService); + const trustProxyRaw = twentyConfigService.get('TRUST_PROXY'); + const trustProxy = /^\d+$/.test(trustProxyRaw) + ? Number(trustProxyRaw) + : (configTransformers.boolean(trustProxyRaw) ?? trustProxyRaw); + + app.set('trust proxy', trustProxy); + app.use(session(getSessionStorageOptions(twentyConfigService))); // Apply class-validator container so that we can use injection in validators