From 8ca66cad6881345bd264df7cb72e17b62cd819c6 Mon Sep 17 00:00:00 2001 From: Coy Geek <65363919+coygeek@users.noreply.github.com> Date: Fri, 24 Apr 2026 17:03:39 -0700 Subject: [PATCH] fix(browser): scope control auth to active gateway mode (#65639) Browser control now authorizes only the resolved active gateway credential and fails closed when password mode lacks a resolved password. Also removes the duplicate Slack test-helper middleware stub that kept current CI red after the base rebase. Fixes #65626. Co-authored-by: Coy Geek <65363919+coygeek@users.noreply.github.com> --- CHANGELOG.md | 1 + .../browser/control-auth.auto-token.test.ts | 99 ++++++++++++++++++- .../browser/src/browser/control-auth.ts | 16 ++- .../browser/server.auth-fail-closed.test.ts | 24 ++++- .../server.auth-token-gates-http.test.ts | 40 ++++++++ extensions/browser/src/server.ts | 12 +-- extensions/slack/src/monitor.test-helpers.ts | 3 - 7 files changed, 173 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a62821d84f5..1fa847853d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai - Slack/exec approvals: resolve native approval button clicks over the Gateway instead of delivering `/approve ...` as plain agent text, preserving retry buttons if Gateway resolution fails. Fixes #71023. (#71025) Thanks @marusan03. - Slack/files: return non-image `download-file` results as local file paths instead of image payloads, and include Slack file IDs in inbound file placeholders so agents can call `download-file`. Fixes #71212. Thanks @teamrazo. +- Browser control: scope standalone loopback auth to the resolved active gateway credential and fail closed when password mode lacks a resolved password, so inactive tokens or passwords no longer authorize browser routes. Fixes #65626. (#65639) Thanks @coygeek. - Discord/replies: run `message_sending` plugin hooks for Discord reply delivery, including DM targets, so plugins can transform or cancel outbound Discord replies consistently with other channels. Fixes #59350. (#71094) Thanks @wei840222. - Control UI/commands: carry provider-owned thinking option ids/labels in session rows and defaults so fresh sessions show and accept dynamic modes such as `adaptive`, `xhigh`, and `max`. Fixes #71269. Thanks @Young-Khalil. - Image generation: make explicit `model=` overrides exact-only so failed `openai/gpt-image-2` requests no longer fall through to Gemini or other configured providers, and update `image_generate list` to mention OpenAI Codex OAuth as valid auth for `openai/gpt-image-2`. Fixes #71290 and #71231. Thanks @Young-Khalil and @steipete. diff --git a/extensions/browser/src/browser/control-auth.auto-token.test.ts b/extensions/browser/src/browser/control-auth.auto-token.test.ts index 371b9e0fc37..9b1bf621969 100644 --- a/extensions/browser/src/browser/control-auth.auto-token.test.ts +++ b/extensions/browser/src/browser/control-auth.auto-token.test.ts @@ -18,7 +18,9 @@ const mocks = vi.hoisted(() => ({ ? undefined : undefined; const password = typeof authConfig?.password === "string" ? authConfig.password : undefined; + const mode = authConfig?.mode ?? (password ? "password" : token ? "token" : "token"); return { + mode, token, password, }; @@ -96,6 +98,7 @@ async function expectUnresolvedBrowserSecretRefSkipsPersistence(cfg: OpenClawCon } let ensureBrowserControlAuth: typeof import("./control-auth.js").ensureBrowserControlAuth; +let resolveBrowserControlAuth: typeof import("./control-auth.js").resolveBrowserControlAuth; describe("ensureBrowserControlAuth", () => { const expectExplicitModeSkipsAutoAuth = async (mode: "password") => { @@ -129,7 +132,7 @@ describe("ensureBrowserControlAuth", () => { }; beforeAll(async () => { - ({ ensureBrowserControlAuth } = await import("./control-auth.js")); + ({ ensureBrowserControlAuth, resolveBrowserControlAuth } = await import("./control-auth.js")); }); beforeEach(() => { @@ -157,6 +160,100 @@ describe("ensureBrowserControlAuth", () => { expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }); + it("returns only the active credential in password mode", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "password", + token: "inactive-token", + password: "active-password", + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({ + password: "active-password", + }); + }); + + it("returns only the resolved active credential when mode is inferred", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + token: "inactive-token", + password: "active-password", + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({ + password: "active-password", + }); + }); + + it("returns only the browser token in none mode", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "none", + token: "browser-token", + password: "inactive-password", + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({ + token: "browser-token", + }); + }); + + it("returns only the active token in token mode", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "token", + token: "active-token", + password: "inactive-password", + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({ + token: "active-token", + }); + }); + + it("returns only the browser password in trusted-proxy mode", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + token: "inactive-token", + password: "browser-password", + trustedProxy: { userHeader: "x-forwarded-user" }, + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({ + password: "browser-password", + }); + }); + + it("does not accept an inactive token in trusted-proxy mode", () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + token: "inactive-token", + trustedProxy: { userHeader: "x-forwarded-user" }, + }, + }, + }; + + expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({}); + }); + it("auto-generates and persists a token when auth is missing", async () => { const cfg: OpenClawConfig = { browser: { diff --git a/extensions/browser/src/browser/control-auth.ts b/extensions/browser/src/browser/control-auth.ts index dfaf962d9ee..1c76b8aa75f 100644 --- a/extensions/browser/src/browser/control-auth.ts +++ b/extensions/browser/src/browser/control-auth.ts @@ -24,10 +24,18 @@ export function resolveBrowserControlAuth( }); const token = normalizeOptionalString(auth.token) ?? ""; const password = normalizeOptionalString(auth.password) ?? ""; - return { - token: token || undefined, - password: password || undefined, - }; + const mode = auth.mode; + + switch (mode) { + case "password": + case "trusted-proxy": + return { password: password || undefined }; + case "token": + case "none": + return { token: token || undefined }; + default: + return {}; + } } export function shouldAutoGenerateBrowserAuth(env: NodeJS.ProcessEnv): boolean { diff --git a/extensions/browser/src/browser/server.auth-fail-closed.test.ts b/extensions/browser/src/browser/server.auth-fail-closed.test.ts index 558c7a1ef09..4b471b65cae 100644 --- a/extensions/browser/src/browser/server.auth-fail-closed.test.ts +++ b/extensions/browser/src/browser/server.auth-fail-closed.test.ts @@ -13,6 +13,7 @@ type EnsureBrowserControlAuthResult = { const mocks = vi.hoisted(() => ({ controlPort: 0, gatewayAuthMode: undefined as "password" | undefined, + gatewayAuthToken: undefined as string | undefined, ensureBrowserControlAuth: vi.fn<() => Promise>(async () => { throw new Error("read-only config"); }), @@ -31,7 +32,9 @@ vi.mock("../config/config.js", async () => { loadConfig: () => { return { browser: browserConfig, - ...(mocks.gatewayAuthMode ? { gateway: { auth: { mode: mocks.gatewayAuthMode } } } : {}), + ...(mocks.gatewayAuthMode || mocks.gatewayAuthToken + ? { gateway: { auth: { mode: mocks.gatewayAuthMode, token: mocks.gatewayAuthToken } } } + : {}), }; }, }; @@ -75,6 +78,7 @@ describe("browser control auth bootstrap failures", () => { beforeEach(async () => { mocks.controlPort = await getFreePort(); mocks.gatewayAuthMode = undefined; + mocks.gatewayAuthToken = undefined; mocks.ensureBrowserControlAuth.mockClear(); mocks.resolveBrowserControlAuth.mockClear(); mocks.shouldAutoGenerateBrowserAuth.mockClear(); @@ -107,7 +111,7 @@ describe("browser control auth bootstrap failures", () => { expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled(); }); - it("keeps legacy password-mode startup when password is not configured", async () => { + it("fails closed when password mode has no resolved password", async () => { mocks.gatewayAuthMode = "password"; mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} }); mocks.resolveBrowserControlAuth.mockReturnValueOnce({}); @@ -115,6 +119,20 @@ describe("browser control auth bootstrap failures", () => { const started = await startBrowserControlServerFromConfig(); - expect(started).not.toBeNull(); + expect(started).toBeNull(); + expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled(); + }); + + it("fails closed when password mode drops an inactive token but has no password", async () => { + mocks.gatewayAuthMode = "password"; + mocks.gatewayAuthToken = "inactive-token"; + mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} }); + mocks.resolveBrowserControlAuth.mockReturnValueOnce({}); + mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(true); + + const started = await startBrowserControlServerFromConfig(); + + expect(started).toBeNull(); + expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled(); }); }); diff --git a/extensions/browser/src/browser/server.auth-token-gates-http.test.ts b/extensions/browser/src/browser/server.auth-token-gates-http.test.ts index 800bf744e25..e7f61904aab 100644 --- a/extensions/browser/src/browser/server.auth-token-gates-http.test.ts +++ b/extensions/browser/src/browser/server.auth-token-gates-http.test.ts @@ -66,4 +66,44 @@ describe("browser control HTTP auth", () => { expect(ok.status).toBe(200); expect((await ok.json()) as { ok: boolean }).toEqual({ ok: true }); }); + + it("rejects bearer auth when password mode is active", async () => { + const base = `http://127.0.0.1:${port}`; + + server?.removeAllListeners("request"); + server?.on("request", (req: IncomingMessage, res: ServerResponse) => { + if (!isAuthorizedBrowserRequest(req, { password: "browser-password" })) { + res.statusCode = 401; + res.end("Unauthorized"); + return; + } + res.statusCode = 200; + res.end("ok"); + }); + + const bearer = await realFetch(`${base}/`, { + headers: { + Authorization: "Bearer browser-control-secret", + }, + }); + expect(bearer.status).toBe(401); + + const password = await realFetch(`${base}/`, { + headers: { + "x-openclaw-password": "browser-password", + }, + }); + expect(password.status).toBe(200); + }); + + it("rejects password auth when token mode is active", async () => { + const base = `http://127.0.0.1:${port}`; + + const password = await realFetch(`${base}/`, { + headers: { + "x-openclaw-password": "browser-control-secret", + }, + }); + expect(password.status).toBe(401); + }); }); diff --git a/extensions/browser/src/server.ts b/extensions/browser/src/server.ts index 176da61584c..66761d75689 100644 --- a/extensions/browser/src/server.ts +++ b/extensions/browser/src/server.ts @@ -54,17 +54,7 @@ export async function startBrowserControlServerFromConfig(): Promise { command() { /* no-op */ } - use() { - /* no-op */ - } start = vi.fn().mockResolvedValue(undefined); stop = vi.fn().mockResolvedValue(undefined); }