diff --git a/CHANGELOG.md b/CHANGELOG.md index e3159266f1e..c329cfdb6a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(browser): auto-generate browser control auth token for none/trusted-proxy modes [AI]. (#63280) Thanks @pgondhi987. - fix(exec): replace TOCTOU check-then-read with atomic pinned-fd open in script preflight [AI]. (#62333) Thanks @pgondhi987. - WhatsApp/auto-reply: keep inbound reply, media, and composing sends on the current socket across reconnects, wait through reconnect gaps, and retry timeout-only send failures without dropping the active socket ref. (#62892) Thanks @mcaxtr. - Config/plugins: let config writes keep disabled plugin entries without forcing required plugin config schemas or crashing raw plugin validation, so slot switches and similar plugin-state updates persist cleanly. (#63296) Thanks @fuller-stack-dev. 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 2cea42344ea..118683dc8a6 100644 --- a/extensions/browser/src/browser/control-auth.auto-token.test.ts +++ b/extensions/browser/src/browser/control-auth.auto-token.test.ts @@ -4,6 +4,7 @@ import type { OpenClawConfig } from "../config/config.js"; const mocks = vi.hoisted(() => ({ loadConfig: vi.fn<() => OpenClawConfig>(), + writeConfigFile: vi.fn<(cfg: OpenClawConfig) => Promise>(async (_cfg) => {}), resolveGatewayAuth: vi.fn( ({ authConfig, @@ -46,6 +47,7 @@ const mocks = vi.hoisted(() => ({ vi.mock("../config/config.js", () => ({ loadConfig: mocks.loadConfig, + writeConfigFile: mocks.writeConfigFile, })); vi.mock("../gateway/startup-auth.js", () => ({ @@ -59,7 +61,7 @@ vi.mock("../gateway/auth.js", () => ({ let ensureBrowserControlAuth: typeof import("./control-auth.js").ensureBrowserControlAuth; describe("ensureBrowserControlAuth", () => { - const expectExplicitModeSkipsAutoAuth = async (mode: "password" | "none") => { + const expectExplicitModeSkipsAutoAuth = async (mode: "password") => { const cfg: OpenClawConfig = { gateway: { auth: { mode }, @@ -72,6 +74,7 @@ describe("ensureBrowserControlAuth", () => { const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); expect(result).toEqual({ auth: {} }); expect(mocks.loadConfig).not.toHaveBeenCalled(); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }; @@ -95,6 +98,7 @@ describe("ensureBrowserControlAuth", () => { beforeEach(() => { vi.restoreAllMocks(); mocks.loadConfig.mockClear(); + mocks.writeConfigFile.mockClear(); mocks.resolveGatewayAuth.mockClear(); mocks.ensureGatewayStartupAuth.mockClear(); }); @@ -112,6 +116,7 @@ describe("ensureBrowserControlAuth", () => { expect(result).toEqual({ auth: { token: "already-set" } }); expect(mocks.loadConfig).not.toHaveBeenCalled(); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }); @@ -129,6 +134,7 @@ describe("ensureBrowserControlAuth", () => { const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); await expectGeneratedTokenPersisted(result); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); }); it("skips auto-generation in test env", async () => { @@ -145,6 +151,7 @@ describe("ensureBrowserControlAuth", () => { expect(result).toEqual({ auth: {} }); expect(mocks.loadConfig).not.toHaveBeenCalled(); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }); @@ -152,8 +159,146 @@ describe("ensureBrowserControlAuth", () => { await expectExplicitModeSkipsAutoAuth("password"); }); - it("respects explicit none mode", async () => { - await expectExplicitModeSkipsAutoAuth("none"); + it("auto-generates and persists browser auth token in none mode", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { mode: "none" }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result.generatedToken).toMatch(/^[a-f0-9]{48}$/); + expect(result.auth.token).toBe(result.generatedToken); + expect(result.auth.password).toBeUndefined(); + expect(mocks.writeConfigFile).toHaveBeenCalledTimes(1); + const persistedCfg = mocks.writeConfigFile.mock.calls[0]?.[0] as OpenClawConfig | undefined; + expect(persistedCfg?.gateway?.auth?.mode).toBe("none"); + expect(persistedCfg?.gateway?.auth?.token).toBe(result.generatedToken); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); + }); + + it("does not persist over unresolved token SecretRef in none mode", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "none", + token: { source: "env", provider: "default", id: "BROWSER_TOKEN" }, + }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result).toEqual({ auth: {} }); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); + }); + + it("still auto-generates in none mode when only password SecretRef is set", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "none", + password: { source: "env", provider: "default", id: "INACTIVE_PASSWORD" }, + }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result.generatedToken).toMatch(/^[a-f0-9]{48}$/); + expect(result.auth.token).toBe(result.generatedToken); + expect(result.auth.password).toBeUndefined(); + expect(mocks.writeConfigFile).toHaveBeenCalledTimes(1); + const persistedCfg = mocks.writeConfigFile.mock.calls[0]?.[0] as OpenClawConfig | undefined; + expect(persistedCfg?.gateway?.auth?.mode).toBe("none"); + expect(persistedCfg?.gateway?.auth?.token).toBe(result.generatedToken); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); + }); + + it("auto-generates in trusted-proxy mode and persists browser auth password", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { mode: "trusted-proxy", trustedProxy: { userHeader: "x-forwarded-user" } }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result.generatedToken).toMatch(/^[a-f0-9]{48}$/); + expect(result.auth.password).toBe(result.generatedToken); + expect(result.auth.token).toBeUndefined(); + expect(mocks.writeConfigFile).toHaveBeenCalledTimes(1); + const persistedCfg = mocks.writeConfigFile.mock.calls[0]?.[0] as OpenClawConfig | undefined; + expect(persistedCfg?.gateway?.auth?.mode).toBe("trusted-proxy"); + expect(persistedCfg?.gateway?.auth?.password).toBe(result.generatedToken); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); + }); + + it("still auto-generates in trusted-proxy mode when only token SecretRef is set", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + token: { source: "env", provider: "default", id: "INACTIVE_TOKEN" }, + trustedProxy: { userHeader: "x-forwarded-user" }, + }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result.generatedToken).toMatch(/^[a-f0-9]{48}$/); + expect(result.auth.password).toBe(result.generatedToken); + expect(result.auth.token).toBeUndefined(); + expect(mocks.writeConfigFile).toHaveBeenCalledTimes(1); + const persistedCfg = mocks.writeConfigFile.mock.calls[0]?.[0] as OpenClawConfig | undefined; + expect(persistedCfg?.gateway?.auth?.mode).toBe("trusted-proxy"); + expect(persistedCfg?.gateway?.auth?.password).toBe(result.generatedToken); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); + }); + + it("does not persist over unresolved password SecretRef in trusted-proxy mode", async () => { + const cfg: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + password: { source: "env", provider: "default", id: "BROWSER_PASSWORD" }, + trustedProxy: { userHeader: "x-forwarded-user" }, + }, + }, + browser: { + enabled: true, + }, + }; + mocks.loadConfig.mockReturnValue(cfg); + + const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); + + expect(result).toEqual({ auth: {} }); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }); it("reuses auth from latest config snapshot", async () => { @@ -176,6 +321,7 @@ describe("ensureBrowserControlAuth", () => { const result = await ensureBrowserControlAuth({ cfg, env: {} as NodeJS.ProcessEnv }); expect(result).toEqual({ auth: { token: "latest-token" } }); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled(); }); diff --git a/extensions/browser/src/browser/control-auth.test.ts b/extensions/browser/src/browser/control-auth.test.ts index 34978a4b31a..51c19d95052 100644 --- a/extensions/browser/src/browser/control-auth.test.ts +++ b/extensions/browser/src/browser/control-auth.test.ts @@ -6,7 +6,7 @@ describe("ensureBrowserControlAuth", () => { async function expectNoAutoGeneratedAuth(cfg: OpenClawConfig): Promise { const result = await ensureBrowserControlAuth({ cfg, - env: { OPENCLAW_BROWSER_AUTO_AUTH: "1" }, + env: { NODE_ENV: "test" }, }); expect(result.generatedToken).toBeUndefined(); expect(result.auth.token).toBeUndefined(); @@ -14,7 +14,7 @@ describe("ensureBrowserControlAuth", () => { } describe("trusted-proxy mode", () => { - it("should not auto-generate token when auth mode is trusted-proxy", async () => { + it("should skip auto-generation in test mode", async () => { const cfg: OpenClawConfig = { gateway: { auth: { @@ -31,7 +31,7 @@ describe("ensureBrowserControlAuth", () => { }); describe("password mode", () => { - it("should not auto-generate token when auth mode is password (even if password not set)", async () => { + it("should skip auto-generation in test mode", async () => { const cfg: OpenClawConfig = { gateway: { auth: { @@ -44,7 +44,7 @@ describe("ensureBrowserControlAuth", () => { }); describe("none mode", () => { - it("should not auto-generate token when auth mode is none", async () => { + it("should skip auto-generation in test mode", async () => { const cfg: OpenClawConfig = { gateway: { auth: { @@ -69,7 +69,7 @@ describe("ensureBrowserControlAuth", () => { const result = await ensureBrowserControlAuth({ cfg, - env: { OPENCLAW_BROWSER_AUTO_AUTH: "1" }, + env: {} as NodeJS.ProcessEnv, }); expect(result.generatedToken).toBeUndefined(); diff --git a/extensions/browser/src/browser/control-auth.ts b/extensions/browser/src/browser/control-auth.ts index 64cfa77affc..77dc072705b 100644 --- a/extensions/browser/src/browser/control-auth.ts +++ b/extensions/browser/src/browser/control-auth.ts @@ -1,9 +1,10 @@ +import crypto from "node:crypto"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, } from "openclaw/plugin-sdk/text-runtime"; import type { OpenClawConfig } from "../config/config.js"; -import { loadConfig } from "../config/config.js"; +import { loadConfig, writeConfigFile } from "../config/config.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; import { ensureGatewayStartupAuth } from "../gateway/startup-auth.js"; @@ -13,7 +14,7 @@ export type BrowserControlAuth = { }; export function resolveBrowserControlAuth( - cfg: OpenClawConfig | undefined, + cfg?: OpenClawConfig, env: NodeJS.ProcessEnv = process.env, ): BrowserControlAuth { const auth = resolveGatewayAuth({ @@ -29,7 +30,7 @@ export function resolveBrowserControlAuth( }; } -function shouldAutoGenerateBrowserAuth(env: NodeJS.ProcessEnv): boolean { +export function shouldAutoGenerateBrowserAuth(env: NodeJS.ProcessEnv): boolean { const nodeEnv = normalizeLowercaseStringOrEmpty(env.NODE_ENV); if (nodeEnv === "test") { return false; @@ -41,6 +42,89 @@ function shouldAutoGenerateBrowserAuth(env: NodeJS.ProcessEnv): boolean { return true; } +function hasExplicitNonStringGatewayCredentialForMode(params: { + cfg?: OpenClawConfig; + mode: "none" | "trusted-proxy"; +}): boolean { + const { cfg, mode } = params; + const auth = cfg?.gateway?.auth; + if (!auth) { + return false; + } + if (mode === "none") { + return auth.token != null && typeof auth.token !== "string"; + } + return auth.password != null && typeof auth.password !== "string"; +} + +function generateBrowserControlToken(): string { + return crypto.randomBytes(24).toString("hex"); +} + +async function generateAndPersistBrowserControlToken(params: { + cfg: OpenClawConfig; + env: NodeJS.ProcessEnv; +}): Promise<{ + auth: BrowserControlAuth; + generatedToken?: string; +}> { + const token = generateBrowserControlToken(); + const nextCfg: OpenClawConfig = { + ...params.cfg, + gateway: { + ...params.cfg.gateway, + auth: { + ...params.cfg.gateway?.auth, + token, + }, + }, + }; + await writeConfigFile(nextCfg); + + // Re-read to stay consistent with any concurrent config writer. + const persistedAuth = resolveBrowserControlAuth(loadConfig(), params.env); + if (persistedAuth.token || persistedAuth.password) { + return { + auth: persistedAuth, + generatedToken: persistedAuth.token === token ? token : undefined, + }; + } + + return { auth: { token }, generatedToken: token }; +} + +async function generateAndPersistBrowserControlPassword(params: { + cfg: OpenClawConfig; + env: NodeJS.ProcessEnv; +}): Promise<{ + auth: BrowserControlAuth; + generatedToken?: string; +}> { + const password = generateBrowserControlToken(); + const nextCfg: OpenClawConfig = { + ...params.cfg, + gateway: { + ...params.cfg.gateway, + auth: { + ...params.cfg.gateway?.auth, + password, + }, + }, + }; + await writeConfigFile(nextCfg); + + // Re-read to stay consistent with any concurrent config writer. + const persistedAuth = resolveBrowserControlAuth(loadConfig(), params.env); + if (persistedAuth.token || persistedAuth.password) { + return { + auth: persistedAuth, + generatedToken: persistedAuth.password === password ? password : undefined, + }; + } + + return { auth: { password }, generatedToken: password }; +} + export async function ensureBrowserControlAuth(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; @@ -62,14 +146,6 @@ export async function ensureBrowserControlAuth(params: { return { auth }; } - if (params.cfg.gateway?.auth?.mode === "none") { - return { auth }; - } - - if (params.cfg.gateway?.auth?.mode === "trusted-proxy") { - return { auth }; - } - // Re-read latest config to avoid racing with concurrent config writers. const latestCfg = loadConfig(); const latestAuth = resolveBrowserControlAuth(latestCfg, env); @@ -79,11 +155,25 @@ export async function ensureBrowserControlAuth(params: { if (latestCfg.gateway?.auth?.mode === "password") { return { auth: latestAuth }; } - if (latestCfg.gateway?.auth?.mode === "none") { - return { auth: latestAuth }; - } - if (latestCfg.gateway?.auth?.mode === "trusted-proxy") { - return { auth: latestAuth }; + const latestMode = latestCfg.gateway?.auth?.mode; + if (latestMode === "none" || latestMode === "trusted-proxy") { + if ( + hasExplicitNonStringGatewayCredentialForMode({ + cfg: latestCfg, + mode: latestMode, + }) + ) { + // Avoid silently overwriting SecretRef-style gateway auth inputs with generated plaintext. + // Startup will fail closed if no resolved browser auth is available. + return { auth: latestAuth }; + } + if (latestMode === "trusted-proxy") { + // gateway.auth.mode=trusted-proxy must never be persisted with gateway.auth.token. + // Persist a browser-only shared secret through gateway.auth.password instead so + // out-of-process loopback clients can resolve it from config/env. + return await generateAndPersistBrowserControlPassword({ cfg: latestCfg, env }); + } + return await generateAndPersistBrowserControlToken({ cfg: latestCfg, env }); } const ensured = await ensureGatewayStartupAuth({ 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 0c883236aec..558c7a1ef09 100644 --- a/extensions/browser/src/browser/server.auth-fail-closed.test.ts +++ b/extensions/browser/src/browser/server.auth-fail-closed.test.ts @@ -2,12 +2,22 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { startBrowserControlServerFromConfig, stopBrowserControlServer } from "../server.js"; import { getFreePort } from "./test-port.js"; +type EnsureBrowserControlAuthResult = { + auth: { + token?: string; + password?: string; + }; + generatedToken?: string; +}; + const mocks = vi.hoisted(() => ({ controlPort: 0, - ensureBrowserControlAuth: vi.fn(async () => { + gatewayAuthMode: undefined as "password" | undefined, + ensureBrowserControlAuth: vi.fn<() => Promise>(async () => { throw new Error("read-only config"); }), resolveBrowserControlAuth: vi.fn(() => ({})), + shouldAutoGenerateBrowserAuth: vi.fn(() => true), ensureExtensionRelayForProfiles: vi.fn(async () => {}), })); @@ -18,9 +28,12 @@ vi.mock("../config/config.js", async () => { }; return { ...actual, - loadConfig: () => ({ - browser: browserConfig, - }), + loadConfig: () => { + return { + browser: browserConfig, + ...(mocks.gatewayAuthMode ? { gateway: { auth: { mode: mocks.gatewayAuthMode } } } : {}), + }; + }, }; }); @@ -38,6 +51,7 @@ vi.mock("./config.js", async () => { vi.mock("./control-auth.js", () => ({ ensureBrowserControlAuth: mocks.ensureBrowserControlAuth, resolveBrowserControlAuth: mocks.resolveBrowserControlAuth, + shouldAutoGenerateBrowserAuth: mocks.shouldAutoGenerateBrowserAuth, })); vi.mock("./routes/index.js", () => ({ @@ -60,8 +74,10 @@ vi.mock("./pw-ai-state.js", () => ({ describe("browser control auth bootstrap failures", () => { beforeEach(async () => { mocks.controlPort = await getFreePort(); + mocks.gatewayAuthMode = undefined; mocks.ensureBrowserControlAuth.mockClear(); mocks.resolveBrowserControlAuth.mockClear(); + mocks.shouldAutoGenerateBrowserAuth.mockClear(); mocks.ensureExtensionRelayForProfiles.mockClear(); }); @@ -77,4 +93,28 @@ describe("browser control auth bootstrap failures", () => { expect(mocks.resolveBrowserControlAuth).toHaveBeenCalledTimes(1); expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled(); }); + + it("fails closed when auth bootstrap resolves empty auth in production-like mode", async () => { + mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} }); + mocks.resolveBrowserControlAuth.mockReturnValueOnce({}); + mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(true); + + const started = await startBrowserControlServerFromConfig(); + + expect(started).toBeNull(); + expect(mocks.ensureBrowserControlAuth).toHaveBeenCalledTimes(1); + expect(mocks.resolveBrowserControlAuth).toHaveBeenCalledTimes(1); + expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled(); + }); + + it("keeps legacy password-mode startup when password is not configured", async () => { + mocks.gatewayAuthMode = "password"; + mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} }); + mocks.resolveBrowserControlAuth.mockReturnValueOnce({}); + mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(true); + + const started = await startBrowserControlServerFromConfig(); + + expect(started).not.toBeNull(); + }); }); diff --git a/extensions/browser/src/server.ts b/extensions/browser/src/server.ts index cd233bc6f94..176da61584c 100644 --- a/extensions/browser/src/server.ts +++ b/extensions/browser/src/server.ts @@ -1,7 +1,12 @@ import type { Server } from "node:http"; import express from "express"; +import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./browser/bridge-auth-registry.js"; import { resolveBrowserConfig } from "./browser/config.js"; -import { ensureBrowserControlAuth, resolveBrowserControlAuth } from "./browser/control-auth.js"; +import { + ensureBrowserControlAuth, + resolveBrowserControlAuth, + shouldAutoGenerateBrowserAuth, +} from "./browser/control-auth.js"; import { registerBrowserRoutes } from "./browser/routes/index.js"; import type { BrowserRouteRegistrar } from "./browser/routes/types.js"; import { createBrowserRuntimeState, stopBrowserRuntime } from "./browser/runtime-lifecycle.js"; @@ -38,19 +43,36 @@ export async function startBrowserControlServerFromConfig(): Promise logServer.warn(message), }); + setBridgeAuthForPort(port, browserAuth); const authMode = browserAuth.token ? "token" : browserAuth.password ? "password" : "off"; logServer.info(`Browser control listening on http://127.0.0.1:${port}/ (auth=${authMode})`); @@ -91,6 +114,9 @@ export async function startBrowserControlServerFromConfig(): Promise { const current = state; + if (current?.port) { + deleteBridgeAuthForPort(current.port); + } await stopBrowserRuntime({ current, getState: () => state,