diff --git a/src/browser/client-fetch.bridge-auth-registry.test.ts b/src/browser/client-fetch.bridge-auth-registry.test.ts index dc4db32eed3..8e8ef5848b6 100644 --- a/src/browser/client-fetch.bridge-auth-registry.test.ts +++ b/src/browser/client-fetch.bridge-auth-registry.test.ts @@ -1,53 +1,19 @@ -import type { AddressInfo } from "node:net"; -import { createServer } from "node:http"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./bridge-auth-registry.js"; +import { describe, expect, it, vi } from "vitest"; +import { __test } from "./client-fetch.js"; describe("fetchBrowserJson loopback auth (bridge auth registry)", () => { - afterEach(() => { - vi.resetModules(); - vi.clearAllMocks(); - }); - it("falls back to per-port bridge auth when config auth is not available", async () => { - vi.doMock("../config/config.js", async (importOriginal) => { - const original = await importOriginal(); - return { - ...original, - loadConfig: () => ({}), - }; + const port = 18765; + const getBridgeAuthForPort = vi.fn((candidate: number) => + candidate === port ? { token: "registry-token" } : undefined, + ); + const init = __test.withLoopbackBrowserAuth(`http://127.0.0.1:${port}/`, undefined, { + loadConfig: () => ({}), + resolveBrowserControlAuth: () => ({}), + getBridgeAuthForPort, }); - - const server = createServer((req, res) => { - const auth = String(req.headers.authorization ?? "").trim(); - if (auth !== "Bearer registry-token") { - res.statusCode = 401; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end("Unauthorized"); - return; - } - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true })); - }); - - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(0, "127.0.0.1", () => resolve()); - }); - - const port = (server.address() as AddressInfo).port; - setBridgeAuthForPort(port, { token: "registry-token" }); - - try { - const { fetchBrowserJson } = await import("./client-fetch.js"); - const result = await fetchBrowserJson<{ ok: boolean }>(`http://127.0.0.1:${port}/`, { - timeoutMs: 2000, - }); - expect(result.ok).toBe(true); - } finally { - deleteBridgeAuthForPort(port); - await new Promise((resolve) => server.close(() => resolve())); - } + const headers = new Headers(init.headers ?? {}); + expect(headers.get("authorization")).toBe("Bearer registry-token"); + expect(getBridgeAuthForPort).toHaveBeenCalledWith(port); }); }); diff --git a/src/browser/client-fetch.ts b/src/browser/client-fetch.ts index bcc8f9faddf..3fe71934b3e 100644 --- a/src/browser/client-fetch.ts +++ b/src/browser/client-fetch.ts @@ -8,6 +8,12 @@ import { } from "./control-service.js"; import { createBrowserRouteDispatcher } from "./routes/dispatcher.js"; +type LoopbackBrowserAuthDeps = { + loadConfig: typeof loadConfig; + resolveBrowserControlAuth: typeof resolveBrowserControlAuth; + getBridgeAuthForPort: typeof getBridgeAuthForPort; +}; + function isAbsoluteHttp(url: string): boolean { return /^https?:\/\//i.test(url.trim()); } @@ -21,9 +27,10 @@ function isLoopbackHttpUrl(url: string): boolean { } } -function withLoopbackBrowserAuth( +function withLoopbackBrowserAuthImpl( url: string, init: (RequestInit & { timeoutMs?: number }) | undefined, + deps: LoopbackBrowserAuthDeps, ): RequestInit & { timeoutMs?: number } { const headers = new Headers(init?.headers ?? {}); if (headers.has("authorization") || headers.has("x-openclaw-password")) { @@ -34,8 +41,8 @@ function withLoopbackBrowserAuth( } try { - const cfg = loadConfig(); - const auth = resolveBrowserControlAuth(cfg); + const cfg = deps.loadConfig(); + const auth = deps.resolveBrowserControlAuth(cfg); if (auth.token) { headers.set("Authorization", `Bearer ${auth.token}`); return { ...init, headers }; @@ -58,7 +65,7 @@ function withLoopbackBrowserAuth( : parsed.protocol === "https:" ? 443 : 80; - const bridgeAuth = getBridgeAuthForPort(port); + const bridgeAuth = deps.getBridgeAuthForPort(port); if (bridgeAuth?.token) { headers.set("Authorization", `Bearer ${bridgeAuth.token}`); } else if (bridgeAuth?.password) { @@ -71,6 +78,17 @@ function withLoopbackBrowserAuth( return { ...init, headers }; } +function withLoopbackBrowserAuth( + url: string, + init: (RequestInit & { timeoutMs?: number }) | undefined, +): RequestInit & { timeoutMs?: number } { + return withLoopbackBrowserAuthImpl(url, init, { + loadConfig, + resolveBrowserControlAuth, + getBridgeAuthForPort, + }); +} + function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number): Error { const hint = isAbsoluteHttp(url) ? "If this is a sandboxed session, ensure the sandbox browser is running and try again." @@ -215,3 +233,7 @@ export async function fetchBrowserJson( throw enhanceBrowserFetchError(url, err, timeoutMs); } } + +export const __test = { + withLoopbackBrowserAuth: withLoopbackBrowserAuthImpl, +}; diff --git a/src/browser/server-context.hot-reload-profiles.test.ts b/src/browser/server-context.hot-reload-profiles.test.ts index 0ff64c23449..c6af1a80ed5 100644 --- a/src/browser/server-context.hot-reload-profiles.test.ts +++ b/src/browser/server-context.hot-reload-profiles.test.ts @@ -17,30 +17,26 @@ function buildConfig() { }; } -vi.mock("../config/config.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - createConfigIO: () => ({ - loadConfig: () => { - // Always return fresh config for createConfigIO to simulate fresh disk read - return buildConfig(); - }, - }), +vi.mock("../config/config.js", () => ({ + createConfigIO: () => ({ loadConfig: () => { - // simulate stale loadConfig that doesn't see updates unless cache cleared - if (!cachedConfig) { - cachedConfig = buildConfig(); - } - return cachedConfig; + // Always return fresh config for createConfigIO to simulate fresh disk read + return buildConfig(); }, - clearConfigCache: vi.fn(() => { - // Clear the simulated cache - cachedConfig = null; - }), - writeConfigFile: vi.fn(async () => {}), - }; -}); + }), + loadConfig: () => { + // simulate stale loadConfig that doesn't see updates unless cache cleared + if (!cachedConfig) { + cachedConfig = buildConfig(); + } + return cachedConfig; + }, + clearConfigCache: vi.fn(() => { + // Clear the simulated cache + cachedConfig = null; + }), + writeConfigFile: vi.fn(async () => {}), +})); vi.mock("./chrome.js", () => ({ isChromeCdpReady: vi.fn(async () => false), @@ -72,8 +68,34 @@ vi.mock("../media/store.js", () => ({ })); describe("server-context hot-reload profiles", () => { + let modulesPromise: Promise<{ + createBrowserRouteContext: typeof import("./server-context.js").createBrowserRouteContext; + resolveBrowserConfig: typeof import("./config.js").resolveBrowserConfig; + loadConfig: typeof import("../config/config.js").loadConfig; + clearConfigCache: typeof import("../config/config.js").clearConfigCache; + }> | null = null; + + const getModules = async () => { + if (!modulesPromise) { + modulesPromise = (async () => { + // Avoid parallel imports here; Vitest mock factories use async importOriginal + // and parallel loading can observe partially-initialized modules. + const configMod = await import("../config/config.js"); + const config = await import("./config.js"); + const serverContext = await import("./server-context.js"); + return { + createBrowserRouteContext: serverContext.createBrowserRouteContext, + resolveBrowserConfig: config.resolveBrowserConfig, + loadConfig: configMod.loadConfig, + clearConfigCache: configMod.clearConfigCache, + }; + })(); + } + return await modulesPromise; + }; + beforeEach(() => { - vi.resetModules(); + vi.clearAllMocks(); cfgProfiles = { openclaw: { cdpPort: 18800, color: "#FF4500" }, }; @@ -81,11 +103,10 @@ describe("server-context hot-reload profiles", () => { }); it("forProfile hot-reloads newly added profiles from config", async () => { - // Start with only openclaw profile - const { createBrowserRouteContext } = await import("./server-context.js"); - const { resolveBrowserConfig } = await import("./config.js"); - const { loadConfig } = await import("../config/config.js"); + const { createBrowserRouteContext, resolveBrowserConfig, loadConfig, clearConfigCache } = + await getModules(); + // Start with only openclaw profile // 1. Prime the cache by calling loadConfig() first const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -129,14 +150,11 @@ describe("server-context hot-reload profiles", () => { expect(stillStaleCfg.browser.profiles.desktop).toBeUndefined(); // Verify clearConfigCache was not called - const { clearConfigCache } = await import("../config/config.js"); expect(clearConfigCache).not.toHaveBeenCalled(); }); it("forProfile still throws for profiles that don't exist in fresh config", async () => { - const { createBrowserRouteContext } = await import("./server-context.js"); - const { resolveBrowserConfig } = await import("./config.js"); - const { loadConfig } = await import("../config/config.js"); + const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -157,9 +175,7 @@ describe("server-context hot-reload profiles", () => { }); it("forProfile refreshes existing profile config after loadConfig cache updates", async () => { - const { createBrowserRouteContext } = await import("./server-context.js"); - const { resolveBrowserConfig } = await import("./config.js"); - const { loadConfig } = await import("../config/config.js"); + const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -187,9 +203,7 @@ describe("server-context hot-reload profiles", () => { }); it("listProfiles refreshes config before enumerating profiles", async () => { - const { createBrowserRouteContext } = await import("./server-context.js"); - const { resolveBrowserConfig } = await import("./config.js"); - const { loadConfig } = await import("../config/config.js"); + const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); diff --git a/src/browser/server.auth-token-gates-http.test.ts b/src/browser/server.auth-token-gates-http.test.ts index 8ba2498d5dd..9ca60dcd32f 100644 --- a/src/browser/server.auth-token-gates-http.test.ts +++ b/src/browser/server.auth-token-gates-http.test.ts @@ -1,91 +1,46 @@ -import { createServer, type AddressInfo } from "node:net"; +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; import { fetch as realFetch } from "undici"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { isAuthorizedBrowserRequest } from "./http-auth.js"; -let testPort = 0; -let prevGatewayPort: string | undefined; - -vi.mock("../config/config.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - loadConfig: () => ({ - gateway: { - auth: { - token: "browser-control-secret", - }, - }, - browser: { - enabled: true, - defaultProfile: "openclaw", - profiles: { - openclaw: { cdpPort: testPort + 1, color: "#FF4500" }, - }, - }, - }), - }; -}); - -vi.mock("./routes/index.js", () => ({ - registerBrowserRoutes(app: { - get: ( - path: string, - handler: (req: unknown, res: { json: (body: unknown) => void }) => void, - ) => void; - }) { - app.get("/", (_req, res) => { - res.json({ ok: true }); - }); - }, -})); - -vi.mock("./server-context.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - createBrowserRouteContext: vi.fn(() => ({ - forProfile: vi.fn(() => ({ - stopRunningBrowser: vi.fn(async () => {}), - })), - })), - }; -}); +let server: ReturnType | null = null; +let port = 0; describe("browser control HTTP auth", () => { beforeEach(async () => { - prevGatewayPort = process.env.OPENCLAW_GATEWAY_PORT; - - const probe = createServer(); - await new Promise((resolve, reject) => { - probe.once("error", reject); - probe.listen(0, "127.0.0.1", () => resolve()); + server = createServer((req: IncomingMessage, res: ServerResponse) => { + if (!isAuthorizedBrowserRequest(req, { token: "browser-control-secret" })) { + res.statusCode = 401; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Unauthorized"); + return; + } + res.statusCode = 200; + res.setHeader("Content-Type", "application/json; charset=utf-8"); + res.end(JSON.stringify({ ok: true })); }); - const addr = probe.address() as AddressInfo; - testPort = addr.port; - await new Promise((resolve) => probe.close(() => resolve())); - - process.env.OPENCLAW_GATEWAY_PORT = String(testPort - 2); + await new Promise((resolve, reject) => { + server?.once("error", reject); + server?.listen(0, "127.0.0.1", () => resolve()); + }); + const addr = server.address(); + if (!addr || typeof addr === "string") { + throw new Error("server address missing"); + } + port = addr.port; }); afterEach(async () => { - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - if (prevGatewayPort === undefined) { - delete process.env.OPENCLAW_GATEWAY_PORT; - } else { - process.env.OPENCLAW_GATEWAY_PORT = prevGatewayPort; + const current = server; + server = null; + if (!current) { + return; } - - const { stopBrowserControlServer } = await import("./server.js"); - await stopBrowserControlServer(); + await new Promise((resolve) => current.close(() => resolve())); }); it("requires bearer auth for standalone browser HTTP routes", async () => { - const { startBrowserControlServerFromConfig } = await import("./server.js"); - const started = await startBrowserControlServerFromConfig(); - expect(started?.port).toBe(testPort); - - const base = `http://127.0.0.1:${testPort}`; + const base = `http://127.0.0.1:${port}`; const missingAuth = await realFetch(`${base}/`); expect(missingAuth.status).toBe(401); diff --git a/src/config/validation.ts b/src/config/validation.ts index 9f01ad1b24d..217a5af2713 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -195,31 +195,132 @@ function validateConfigObjectWithPluginsBase( const config = base.config; const issues: ConfigValidationIssue[] = []; const warnings: ConfigValidationIssue[] = []; - const pluginsConfig = config.plugins; - const normalizedPlugins = normalizePluginsConfig(pluginsConfig); + const hasExplicitPluginsConfig = + isRecord(raw) && Object.prototype.hasOwnProperty.call(raw, "plugins"); - const workspaceDir = resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)); - const registry = loadPluginManifestRegistry({ - config, - workspaceDir: workspaceDir ?? undefined, - }); + type RegistryInfo = { + registry: ReturnType; + knownIds: Set; + normalizedPlugins: ReturnType; + }; - const knownIds = new Set(registry.plugins.map((record) => record.id)); + let registryInfo: RegistryInfo | null = null; - for (const diag of registry.diagnostics) { - let path = diag.pluginId ? `plugins.entries.${diag.pluginId}` : "plugins"; - if (!diag.pluginId && diag.message.includes("plugin path not found")) { - path = "plugins.load.paths"; + const ensureRegistry = (): RegistryInfo => { + if (registryInfo) { + return registryInfo; } - const pluginLabel = diag.pluginId ? `plugin ${diag.pluginId}` : "plugin"; - const message = `${pluginLabel}: ${diag.message}`; - if (diag.level === "error") { - issues.push({ path, message }); - } else { - warnings.push({ path, message }); + + const workspaceDir = resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)); + const registry = loadPluginManifestRegistry({ + config, + workspaceDir: workspaceDir ?? undefined, + }); + const knownIds = new Set(registry.plugins.map((record) => record.id)); + const normalizedPlugins = normalizePluginsConfig(config.plugins); + + for (const diag of registry.diagnostics) { + let path = diag.pluginId ? `plugins.entries.${diag.pluginId}` : "plugins"; + if (!diag.pluginId && diag.message.includes("plugin path not found")) { + path = "plugins.load.paths"; + } + const pluginLabel = diag.pluginId ? `plugin ${diag.pluginId}` : "plugin"; + const message = `${pluginLabel}: ${diag.message}`; + if (diag.level === "error") { + issues.push({ path, message }); + } else { + warnings.push({ path, message }); + } + } + + registryInfo = { registry, knownIds, normalizedPlugins }; + return registryInfo; + }; + + const allowedChannels = new Set(["defaults", ...CHANNEL_IDS]); + + if (config.channels && isRecord(config.channels)) { + for (const key of Object.keys(config.channels)) { + const trimmed = key.trim(); + if (!trimmed) { + continue; + } + if (!allowedChannels.has(trimmed)) { + const { registry } = ensureRegistry(); + for (const record of registry.plugins) { + for (const channelId of record.channels) { + allowedChannels.add(channelId); + } + } + } + if (!allowedChannels.has(trimmed)) { + issues.push({ + path: `channels.${trimmed}`, + message: `unknown channel id: ${trimmed}`, + }); + } } } + const heartbeatChannelIds = new Set(); + for (const channelId of CHANNEL_IDS) { + heartbeatChannelIds.add(channelId.toLowerCase()); + } + + const validateHeartbeatTarget = (target: string | undefined, path: string) => { + if (typeof target !== "string") { + return; + } + const trimmed = target.trim(); + if (!trimmed) { + issues.push({ path, message: "heartbeat target must not be empty" }); + return; + } + const normalized = trimmed.toLowerCase(); + if (normalized === "last" || normalized === "none") { + return; + } + if (normalizeChatChannelId(trimmed)) { + return; + } + if (!heartbeatChannelIds.has(normalized)) { + const { registry } = ensureRegistry(); + for (const record of registry.plugins) { + for (const channelId of record.channels) { + const pluginChannel = channelId.trim(); + if (pluginChannel) { + heartbeatChannelIds.add(pluginChannel.toLowerCase()); + } + } + } + } + if (heartbeatChannelIds.has(normalized)) { + return; + } + issues.push({ path, message: `unknown heartbeat target: ${target}` }); + }; + + validateHeartbeatTarget( + config.agents?.defaults?.heartbeat?.target, + "agents.defaults.heartbeat.target", + ); + if (Array.isArray(config.agents?.list)) { + for (const [index, entry] of config.agents.list.entries()) { + validateHeartbeatTarget(entry?.heartbeat?.target, `agents.list.${index}.heartbeat.target`); + } + } + + if (!hasExplicitPluginsConfig) { + if (issues.length > 0) { + return { ok: false, issues, warnings }; + } + return { ok: true, config, warnings }; + } + + const { registry, knownIds, normalizedPlugins } = ensureRegistry(); + + const pluginsConfig = config.plugins; + const entries = pluginsConfig?.entries; if (entries && isRecord(entries)) { for (const pluginId of Object.keys(entries)) { @@ -266,73 +367,6 @@ function validateConfigObjectWithPluginsBase( }); } - const allowedChannels = new Set(["defaults", ...CHANNEL_IDS]); - for (const record of registry.plugins) { - for (const channelId of record.channels) { - allowedChannels.add(channelId); - } - } - - if (config.channels && isRecord(config.channels)) { - for (const key of Object.keys(config.channels)) { - const trimmed = key.trim(); - if (!trimmed) { - continue; - } - if (!allowedChannels.has(trimmed)) { - issues.push({ - path: `channels.${trimmed}`, - message: `unknown channel id: ${trimmed}`, - }); - } - } - } - - const heartbeatChannelIds = new Set(); - for (const channelId of CHANNEL_IDS) { - heartbeatChannelIds.add(channelId.toLowerCase()); - } - for (const record of registry.plugins) { - for (const channelId of record.channels) { - const trimmed = channelId.trim(); - if (trimmed) { - heartbeatChannelIds.add(trimmed.toLowerCase()); - } - } - } - - const validateHeartbeatTarget = (target: string | undefined, path: string) => { - if (typeof target !== "string") { - return; - } - const trimmed = target.trim(); - if (!trimmed) { - issues.push({ path, message: "heartbeat target must not be empty" }); - return; - } - const normalized = trimmed.toLowerCase(); - if (normalized === "last" || normalized === "none") { - return; - } - if (normalizeChatChannelId(trimmed)) { - return; - } - if (heartbeatChannelIds.has(normalized)) { - return; - } - issues.push({ path, message: `unknown heartbeat target: ${target}` }); - }; - - validateHeartbeatTarget( - config.agents?.defaults?.heartbeat?.target, - "agents.defaults.heartbeat.target", - ); - if (Array.isArray(config.agents?.list)) { - for (const [index, entry] of config.agents.list.entries()) { - validateHeartbeatTarget(entry?.heartbeat?.target, `agents.list.${index}.heartbeat.target`); - } - } - let selectedMemoryPluginId: string | null = null; const seenPlugins = new Set(); for (const record of registry.plugins) { diff --git a/test/setup.ts b/test/setup.ts index bb400a31e85..4e008ff1881 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -2,6 +2,9 @@ import { afterAll, afterEach, beforeEach, vi } from "vitest"; // Ensure Vitest environment is properly set process.env.VITEST = "true"; +// Config validation walks plugin manifests; keep an aggressive cache in tests to avoid +// repeated filesystem discovery across suites/workers. +process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS ??= "60000"; // Vitest vm forks can load transitive lockfile helpers many times per worker. // Raise listener budget to avoid noisy MaxListeners warnings and warning-stack overhead. const TEST_PROCESS_MAX_LISTENERS = 128;