From 6c679e5f049a289233716f0ffe3ea314497ff671 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Tue, 31 Mar 2026 09:05:03 -0700 Subject: [PATCH] Gateway: reject mixed trusted-proxy token config (#58371) * Gateway: reject mixed trusted-proxy token config Co-authored-by: boy-hack * Gateway: fail closed for loopback trusted-proxy auth --------- Co-authored-by: boy-hack --- src/gateway/auth.test.ts | 121 ++++++++++++++++++++++++++++----------- src/gateway/auth.ts | 31 ++-------- 2 files changed, 92 insertions(+), 60 deletions(-) diff --git a/src/gateway/auth.test.ts b/src/gateway/auth.test.ts index e87ed80a2d0..71265e6aae1 100644 --- a/src/gateway/auth.test.ts +++ b/src/gateway/auth.test.ts @@ -685,6 +685,58 @@ describe("trusted-proxy auth", () => { expect(res.reason).toBe("trusted_proxy_config_missing"); }); + it.each([ + { + name: "config token", + authConfig: { + mode: "trusted-proxy" as const, + token: "shared-secret", + trustedProxy: { + userHeader: "x-forwarded-user", + }, + }, + env: undefined, + }, + { + name: "environment token", + authConfig: { + mode: "trusted-proxy" as const, + trustedProxy: { + userHeader: "x-forwarded-user", + }, + }, + env: { + OPENCLAW_GATEWAY_TOKEN: "shared-secret", + } as NodeJS.ProcessEnv, + }, + ])("rejects trusted-proxy mode when shared token comes from $name", ({ authConfig, env }) => { + const auth = resolveGatewayAuth({ + authConfig, + env, + }); + + expect(auth.mode).toBe("trusted-proxy"); + expect(auth.token).toBe("shared-secret"); + + expect(() => assertGatewayAuthConfigured(auth, authConfig)).toThrow(/mutually exclusive/); + }); + + it("still requires trustedProxy config before reporting a token conflict", () => { + const auth = resolveGatewayAuth({ + authConfig: { + mode: "trusted-proxy", + token: "shared-secret", + }, + }); + + expect(() => + assertGatewayAuthConfigured(auth, { + mode: "trusted-proxy", + token: "shared-secret", + }), + ).toThrow(/no trustedProxy config was provided/); + }); + it("supports Pomerium-style headers", async () => { const res = await authorizeTrustedProxy({ auth: { @@ -726,7 +778,7 @@ describe("trusted-proxy auth", () => { expect(res.user).toBe("nick@example.com"); }); - describe("local-direct token fallback", () => { + describe("local-direct trusted-proxy requests", () => { function authorizeLocalDirect(options?: { token?: string; connectToken?: string; @@ -751,38 +803,37 @@ describe("trusted-proxy auth", () => { }); } - it("allows local-direct request with a valid token", async () => { - const res = await authorizeLocalDirect({ - token: "secret", - connectToken: "secret", - }); - expect(res.ok).toBe(true); - expect(res.method).toBe("token"); - }); - - it("rejects local-direct request without credentials", async () => { - const res = await authorizeLocalDirect({ - token: "secret", - }); + it.each([ + { + name: "without credentials", + options: { + token: "secret", + }, + }, + { + name: "with a valid token", + options: { + token: "secret", + connectToken: "secret", + }, + }, + { + name: "with a wrong token", + options: { + token: "secret", + connectToken: "wrong", + }, + }, + { + name: "when no local token is configured", + options: { + connectToken: "secret", + }, + }, + ])("rejects local-direct request $name", async ({ options }) => { + const res = await authorizeLocalDirect(options); expect(res.ok).toBe(false); - expect(res.reason).toBe("token_missing"); - }); - - it("rejects local-direct request with a wrong token", async () => { - const res = await authorizeLocalDirect({ - token: "secret", - connectToken: "wrong", - }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_mismatch"); - }); - - it("rejects local-direct request when no local token is configured", async () => { - const res = await authorizeLocalDirect({ - connectToken: "secret", - }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_missing_config"); + expect(res.reason).toBe("trusted_proxy_loopback_source"); }); it("rejects trusted-proxy identity headers from loopback sources", async () => { @@ -831,7 +882,7 @@ describe("trusted-proxy auth", () => { expect(res.reason).toBe("trusted_proxy_loopback_source"); }); - it("uses token fallback for direct loopback even when Host is not localish", async () => { + it("rejects direct loopback even when Host is not localish", async () => { const res = await authorizeGatewayConnect({ auth: { mode: "trusted-proxy", @@ -849,8 +900,8 @@ describe("trusted-proxy auth", () => { } as never, }); - expect(res.ok).toBe(true); - expect(res.method).toBe("token"); + expect(res.ok).toBe(false); + expect(res.reason).toBe("trusted_proxy_loopback_source"); }); it("rejects same-host proxy request with missing required header", async () => { diff --git a/src/gateway/auth.ts b/src/gateway/auth.ts index 09cc50a7ff7..42802548d16 100644 --- a/src/gateway/auth.ts +++ b/src/gateway/auth.ts @@ -322,6 +322,11 @@ export function assertGatewayAuthConfigured( "gateway auth mode is trusted-proxy, but trustedProxy.userHeader is empty (set gateway.auth.trustedProxy.userHeader)", ); } + if (auth.token) { + throw new Error( + "gateway auth mode is trusted-proxy, but a shared token is also configured; remove gateway.auth.token / OPENCLAW_GATEWAY_TOKEN because trusted-proxy and token auth are mutually exclusive", + ); + } } } @@ -447,7 +452,7 @@ export async function authorizeGatewayConnect( if (auth.mode === "trusted-proxy") { // Same-host reverse proxies may forward identity headers without a full // forwarded chain; keep those on the trusted-proxy path so allowUsers and - // requiredHeaders still apply. Only raw local-direct traffic falls back. + // requiredHeaders still apply. if (!auth.trustedProxy) { return { ok: false, reason: "trusted_proxy_config_missing" }; } @@ -455,30 +460,6 @@ export async function authorizeGatewayConnect( return { ok: false, reason: "trusted_proxy_no_proxies_configured" }; } - const proxyUserHeader = auth.trustedProxy?.userHeader?.toLowerCase(); - const hasProxyIdentityHeader = - proxyUserHeader !== undefined && Boolean(req?.headers?.[proxyUserHeader]); - if (localDirect && !hasProxyIdentityHeader) { - if (limiter) { - const rlCheck: RateLimitCheckResult = limiter.check(ip, rateLimitScope); - if (!rlCheck.allowed) { - return { - ok: false, - reason: "rate_limited", - rateLimited: true, - retryAfterMs: rlCheck.retryAfterMs, - }; - } - } - return authorizeTokenAuth({ - authToken: auth.token, - connectToken: connectAuth?.token, - limiter, - ip, - rateLimitScope, - }); - } - const result = authorizeTrustedProxy({ req, trustedProxies,