From f7a7a28c5604e8afc871874d0b053b4c805ed24a Mon Sep 17 00:00:00 2001 From: Coy Geek <65363919+coygeek@users.noreply.github.com> Date: Thu, 19 Feb 2026 02:48:08 -0800 Subject: [PATCH] fix: enforce hooks token separation from gateway auth (#20813) * fix(an-03): apply security fix Generated by staged fix workflow. * fix(an-03): apply security fix Generated by staged fix workflow. * fix(an-03): remove stale test-link artifact from patch Remove accidental a2ui test-link artifact from the tracked diff and keep startup auth enforcement centralized in startup-auth.ts. --- src/gateway/startup-auth.test.ts | 80 +++++++++++++++++++++++++++++++- src/gateway/startup-auth.ts | 29 ++++++++++++ src/security/audit-extra.sync.ts | 2 +- src/security/audit.test.ts | 7 ++- 4 files changed, 114 insertions(+), 4 deletions(-) diff --git a/src/gateway/startup-auth.test.ts b/src/gateway/startup-auth.test.ts index cbde1431ecb..78a389ef848 100644 --- a/src/gateway/startup-auth.test.ts +++ b/src/gateway/startup-auth.test.ts @@ -13,7 +13,10 @@ vi.mock("../config/config.js", async (importOriginal) => { }; }); -import { ensureGatewayStartupAuth } from "./startup-auth.js"; +import { + assertHooksTokenSeparateFromGatewayAuth, + ensureGatewayStartupAuth, +} from "./startup-auth.js"; describe("ensureGatewayStartupAuth", () => { async function expectEphemeralGeneratedTokenWhenOverridden(cfg: OpenClawConfig) { @@ -188,4 +191,79 @@ describe("ensureGatewayStartupAuth", () => { }, }); }); + + it("throws when hooks token reuses gateway token resolved from env", async () => { + await expect( + ensureGatewayStartupAuth({ + cfg: { + hooks: { + enabled: true, + token: "shared-gateway-token-1234567890", + }, + }, + env: { + OPENCLAW_GATEWAY_TOKEN: "shared-gateway-token-1234567890", + } as NodeJS.ProcessEnv, + }), + ).rejects.toThrow(/hooks\.token must not match gateway auth token/i); + }); +}); + +describe("assertHooksTokenSeparateFromGatewayAuth", () => { + it("throws when hooks token reuses gateway token auth", () => { + expect(() => + assertHooksTokenSeparateFromGatewayAuth({ + cfg: { + hooks: { + enabled: true, + token: "shared-gateway-token-1234567890", + }, + }, + auth: { + mode: "token", + modeSource: "config", + token: "shared-gateway-token-1234567890", + allowTailscale: false, + }, + }), + ).toThrow(/hooks\.token must not match gateway auth token/i); + }); + + it("allows hooks token when gateway auth is not token mode", () => { + expect(() => + assertHooksTokenSeparateFromGatewayAuth({ + cfg: { + hooks: { + enabled: true, + token: "shared-gateway-token-1234567890", + }, + }, + auth: { + mode: "password", + modeSource: "config", + password: "pw", + allowTailscale: false, + }, + }), + ).not.toThrow(); + }); + + it("allows matching values when hooks are disabled", () => { + expect(() => + assertHooksTokenSeparateFromGatewayAuth({ + cfg: { + hooks: { + enabled: false, + token: "shared-gateway-token-1234567890", + }, + }, + auth: { + mode: "token", + modeSource: "config", + token: "shared-gateway-token-1234567890", + allowTailscale: false, + }, + }), + ).not.toThrow(); + }); }); diff --git a/src/gateway/startup-auth.ts b/src/gateway/startup-auth.ts index ec1ef7dd56e..9bb6e886746 100644 --- a/src/gateway/startup-auth.ts +++ b/src/gateway/startup-auth.ts @@ -109,6 +109,7 @@ export async function ensureGatewayStartupAuth(params: { tailscaleOverride: params.tailscaleOverride, }); if (resolved.mode !== "token" || (resolved.token?.trim().length ?? 0) > 0) { + assertHooksTokenSeparateFromGatewayAuth({ cfg: params.cfg, auth: resolved }); return { cfg: params.cfg, auth: resolved, persistedGeneratedToken: false }; } @@ -138,6 +139,7 @@ export async function ensureGatewayStartupAuth(params: { authOverride: params.authOverride, tailscaleOverride: params.tailscaleOverride, }); + assertHooksTokenSeparateFromGatewayAuth({ cfg: nextCfg, auth: nextAuth }); return { cfg: nextCfg, auth: nextAuth, @@ -145,3 +147,30 @@ export async function ensureGatewayStartupAuth(params: { persistedGeneratedToken: persist, }; } + +export function assertHooksTokenSeparateFromGatewayAuth(params: { + cfg: OpenClawConfig; + auth: ResolvedGatewayAuth; +}): void { + if (params.cfg.hooks?.enabled !== true) { + return; + } + const hooksToken = + typeof params.cfg.hooks.token === "string" ? params.cfg.hooks.token.trim() : ""; + if (!hooksToken) { + return; + } + const gatewayToken = + params.auth.mode === "token" && typeof params.auth.token === "string" + ? params.auth.token.trim() + : ""; + if (!gatewayToken) { + return; + } + if (hooksToken !== gatewayToken) { + return; + } + throw new Error( + "Invalid config: hooks.token must not match gateway auth token. Set a distinct hooks.token for hook ingress.", + ); +} diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index 50b18c37992..9741865d2fe 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -441,7 +441,7 @@ export function collectHooksHardeningFindings( if (token && gatewayToken && token === gatewayToken) { findings.push({ checkId: "hooks.token_reuse_gateway_token", - severity: "warn", + severity: "critical", title: "Hooks token reuses the Gateway token", detail: "hooks.token matches gateway.auth token; compromise of hooks expands blast radius to the Gateway API.", diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 3da11d825ca..c9e8143d39a 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -1356,7 +1356,7 @@ describe("security audit", () => { ); }); - it("warns when hooks token reuses the gateway env token", async () => { + it("flags hooks token reuse of the gateway env token as critical", async () => { const prevToken = process.env.OPENCLAW_GATEWAY_TOKEN; process.env.OPENCLAW_GATEWAY_TOKEN = "shared-gateway-token-1234567890"; const cfg: OpenClawConfig = { @@ -1372,7 +1372,10 @@ describe("security audit", () => { expect(res.findings).toEqual( expect.arrayContaining([ - expect.objectContaining({ checkId: "hooks.token_reuse_gateway_token", severity: "warn" }), + expect.objectContaining({ + checkId: "hooks.token_reuse_gateway_token", + severity: "critical", + }), ]), ); } finally {