From d0764f35660381eb35f1bcfe564617a39ad9cd5f Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Sun, 12 Jul 2026 11:10:59 +0800 Subject: [PATCH] fix(browser): preserve own profile entries (#101694) Co-authored-by: Peter Steinberger --- .../browser/src/browser/config-mutations.ts | 15 ++-- extensions/browser/src/browser/config.ts | 12 +++- .../src/browser/profiles-service.test.ts | 39 ++++++++++ .../browser/src/browser/profiles-service.ts | 21 +++--- ...server-context.hot-reload-profiles.test.ts | 44 ++++++++++++ .../server-context.list-profiles.test.ts | 25 ++++++- .../browser/src/browser/server-context.ts | 6 +- extensions/browser/src/browser/trash.test.ts | 72 ++++++++++++++++--- extensions/browser/src/browser/trash.ts | 12 ++-- src/config/io.write-prepare.ts | 4 +- .../merge-patch.proto-pollution.test.ts | 34 +++++++++ src/config/merge-patch.ts | 17 ++++- src/gateway/server-methods/config.ts | 12 ++-- src/gateway/server.config-patch.test.ts | 55 ++++++++++++++ 14 files changed, 324 insertions(+), 44 deletions(-) diff --git a/extensions/browser/src/browser/config-mutations.ts b/extensions/browser/src/browser/config-mutations.ts index abaaba77c759..c56bc37b2835 100644 --- a/extensions/browser/src/browser/config-mutations.ts +++ b/extensions/browser/src/browser/config-mutations.ts @@ -10,7 +10,11 @@ import type { BrowserProfileConfig } from "../config/config.js"; import { deriveDefaultBrowserCdpPortRange } from "../config/port-defaults.js"; import { formatErrorMessage } from "../infra/errors.js"; import { assertCdpEndpointAllowed } from "./cdp.helpers.js"; -import { resolveBrowserConfig, type ResolvedBrowserConfig } from "./config.js"; +import { + getOwnBrowserProfile, + resolveBrowserConfig, + type ResolvedBrowserConfig, +} from "./config.js"; import { BrowserConflictError, BrowserResourceExhaustedError, @@ -104,7 +108,10 @@ export async function createBrowserProfileConfig(params: { const latestRootResolved = resolveBrowserConfig(draft.browser, draft); const latestProfileSource = useRebasedPortRange ? latestRootResolved : latestResolved; const latestProfiles = draft.browser?.profiles ?? {}; - if (params.name in latestProfiles || params.name in latestProfileSource.profiles) { + if ( + getOwnBrowserProfile(latestProfiles, params.name) || + getOwnBrowserProfile(latestProfileSource.profiles, params.name) + ) { throw new BrowserConflictError(`profile "${params.name}" already exists`); } @@ -176,7 +183,7 @@ export async function deleteBrowserProfileConfig(params: { `cannot delete the default profile "${params.name}"; change browser.defaultProfile first`, ); } - const currentProfile = draft.browser?.profiles?.[params.name]; + const currentProfile = getOwnBrowserProfile(draft.browser?.profiles, params.name); if (!isDeepStrictEqual(currentProfile, params.expected)) { throw new BrowserConflictError( `profile "${params.name}" changed while deletion was pending; retry the delete request`, @@ -196,7 +203,7 @@ export async function setDefaultBrowserProfile(name: string): Promise { await mutateConfigFile({ afterWrite: { mode: "auto" }, mutate: (draft) => { - if (!(name in (draft.browser?.profiles ?? {}))) { + if (!getOwnBrowserProfile(draft.browser?.profiles, name)) { throw new BrowserValidationError(`profile "${name}" does not exist`); } draft.browser = { diff --git a/extensions/browser/src/browser/config.ts b/extensions/browser/src/browser/config.ts index 22585f1ced0f..66a69417de56 100644 --- a/extensions/browser/src/browser/config.ts +++ b/extensions/browser/src/browser/config.ts @@ -121,6 +121,14 @@ export type ResolvedBrowserProfile = { attachOnly: boolean; }; +/** Read a named browser profile without falling through to inherited object keys. */ +export function getOwnBrowserProfile( + profiles: Record | undefined, + name: string, +): T | undefined { + return profiles && Object.hasOwn(profiles, name) ? profiles[name] : undefined; +} + const DEFAULT_BROWSER_CDP_PORT_RANGE_START = 18800; /** * Default extension relay port offset from the browser control port. Sits just @@ -410,7 +418,7 @@ function applyLegacyCdpUrlToExistingSessionDefaultProfile( if (!legacyCdpUrl) { return profiles; } - const profile = profiles[defaultProfile]; + const profile = getOwnBrowserProfile(profiles, defaultProfile); if ( !profile || profile.driver !== "existing-session" || @@ -573,7 +581,7 @@ export function resolveProfile( resolved: ResolvedBrowserConfig, profileName: string, ): ResolvedBrowserProfile | null { - const profile = resolved.profiles[profileName]; + const profile = getOwnBrowserProfile(resolved.profiles, profileName); if (!profile) { return null; } diff --git a/extensions/browser/src/browser/profiles-service.test.ts b/extensions/browser/src/browser/profiles-service.test.ts index 32322d1718a7..2cc0a30d39c2 100644 --- a/extensions/browser/src/browser/profiles-service.test.ts +++ b/extensions/browser/src/browser/profiles-service.test.ts @@ -179,6 +179,45 @@ describe("BrowserProfilesService", () => { expect(writeConfigFile).toHaveBeenCalled(); }); + it("round-trips prototype-like profile names as own entries", async () => { + for (const profileName of ["constructor", "prototype"] as const) { + writeConfigFile.mockClear(); + const resolved = resolveBrowserConfig({}); + const { ctx, state } = createCtx(resolved); + vi.mocked(getRuntimeConfig).mockReturnValue({ browser: { profiles: {} } }); + + const service = createBrowserProfilesService(ctx); + const result = await service.createProfile({ name: profileName }); + + expect(result.profile).toBe(profileName); + expect(Object.hasOwn(state.resolved.profiles, profileName)).toBe(true); + const createdProfiles = writtenBrowserConfig().profiles as Record< + string, + { cdpPort?: number; color: string } + >; + expect(Object.hasOwn(createdProfiles, profileName)).toBe(true); + + writeConfigFile.mockClear(); + vi.mocked(getRuntimeConfig).mockReturnValue({ + browser: { + defaultProfile: "openclaw", + profiles: { [profileName]: createdProfiles[profileName] }, + }, + }); + await service.deleteProfile(profileName); + + const deletedProfiles = writtenBrowserConfig().profiles as Record< + string, + { cdpPort?: number; color: string } + >; + expect(Object.hasOwn(deletedProfiles, profileName)).toBe(false); + expect(Object.hasOwn(state.resolved.profiles, profileName)).toBe(false); + expect(resolveProfile(resolveBrowserConfig({ profiles: deletedProfiles }), profileName)).toBe( + null, + ); + } + }); + it("persists an existing managed profile as the browser default", async () => { vi.mocked(getRuntimeConfig).mockReturnValue({ browser: { diff --git a/extensions/browser/src/browser/profiles-service.ts b/extensions/browser/src/browser/profiles-service.ts index e735b6d37ae3..5a0199777325 100644 --- a/extensions/browser/src/browser/profiles-service.ts +++ b/extensions/browser/src/browser/profiles-service.ts @@ -17,7 +17,12 @@ import { deleteBrowserProfileConfig, setDefaultBrowserProfile, } from "./config-mutations.js"; -import { parseHttpUrl, resolveBrowserConfig, resolveProfile } from "./config.js"; +import { + getOwnBrowserProfile, + parseHttpUrl, + resolveBrowserConfig, + resolveProfile, +} from "./config.js"; import { BrowserConflictError, BrowserProfileNotFoundError, @@ -94,13 +99,13 @@ export function createBrowserProfilesService(ctx: BrowserRouteContext) { const state = ctx.state(); const resolvedProfiles = state.resolved.profiles; - if (name in resolvedProfiles) { + if (getOwnBrowserProfile(resolvedProfiles, name)) { throw new BrowserConflictError(`profile "${name}" already exists`); } const cfg = getRuntimeConfig(); const rawProfiles = cfg.browser?.profiles ?? {}; - if (name in rawProfiles) { + if (getOwnBrowserProfile(rawProfiles, name)) { throw new BrowserConflictError(`profile "${name}" already exists`); } @@ -236,14 +241,14 @@ export function createBrowserProfilesService(ctx: BrowserRouteContext) { `cannot delete the default profile "${name}"; change browser.defaultProfile first`, ); } - if (!(name in profiles)) { - throw new BrowserProfileNotFoundError(`profile "${name}" not found`); - } - const runtimeProfile = profiles[name]; + const runtimeProfile = getOwnBrowserProfile(profiles, name); if (!runtimeProfile) { throw new BrowserProfileNotFoundError(`profile "${name}" not found`); } - const sourceProfile = getRuntimeConfigSourceSnapshot()?.browser?.profiles?.[name]; + const sourceProfile = getOwnBrowserProfile( + getRuntimeConfigSourceSnapshot()?.browser?.profiles, + name, + ); const expected = structuredClone(sourceProfile ?? runtimeProfile); let deleted = false; diff --git a/extensions/browser/src/browser/server-context.hot-reload-profiles.test.ts b/extensions/browser/src/browser/server-context.hot-reload-profiles.test.ts index 030a730bacdb..443c263c7e47 100644 --- a/extensions/browser/src/browser/server-context.hot-reload-profiles.test.ts +++ b/extensions/browser/src/browser/server-context.hot-reload-profiles.test.ts @@ -210,6 +210,50 @@ describe("server-context hot-reload profiles", () => { ).toBeNull(); }); + it.each(["constructor", "prototype"] as const)( + "treats removed %s profiles as absent during hot reload", + (profileName) => { + mockState.cfgProfiles = { + [profileName]: { cdpPort: 18801, color: "#0066CC" }, + }; + const cfg = getRuntimeConfig(); + const resolved = resolveBrowserConfig(cfg.browser, cfg); + const profile = requireValue( + resolveProfile(resolved, profileName), + `${profileName} profile missing`, + ); + const state: BrowserServerState = { + server: null, + port: 18791, + resolved, + profiles: new Map([ + [ + profileName, + { + profile, + running: { pid: 123 } as never, + lastTargetId: "tab-1", + reconcile: null, + }, + ], + ]), + }; + + mockState.cfgProfiles = {}; + mockState.cachedConfig = null; + refreshResolvedBrowserConfigFromDisk({ + current: state, + refreshConfigFromDisk: true, + }); + + expect(resolveProfile(state.resolved, profileName)).toBeNull(); + const runtime = requireValue(state.profiles.get(profileName), "runtime missing"); + const actor = getProfileLifecycle(runtime); + expect(actor.terminal).toBe("config-removed"); + expect(actor.transitionReason).toBe("profile removed from config"); + }, + ); + it("forProfile refreshes existing profile config after getRuntimeConfig cache updates", () => { const cfg = getRuntimeConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); diff --git a/extensions/browser/src/browser/server-context.list-profiles.test.ts b/extensions/browser/src/browser/server-context.list-profiles.test.ts index 4b0b296689f7..0dcccc1387a1 100644 --- a/extensions/browser/src/browser/server-context.list-profiles.test.ts +++ b/extensions/browser/src/browser/server-context.list-profiles.test.ts @@ -4,7 +4,7 @@ import "./server-context.chrome-test-harness.js"; import * as chromeModule from "./chrome.js"; import { createBrowserRouteContext } from "./server-context.js"; import { beginProfileTransition } from "./server-context.lifecycle.js"; -import { makeBrowserServerState } from "./server-context.test-harness.js"; +import { makeBrowserProfile, makeBrowserServerState } from "./server-context.test-harness.js"; afterEach(() => { vi.clearAllMocks(); @@ -136,4 +136,27 @@ describe("browser server-context listProfiles", () => { ); expect(profiles[0]?.cdpUrl).toBe("http://127.0.0.1:9222"); }); + + it.each(["constructor", "prototype"] as const)( + "marks runtime-only %s profiles as missing from config", + async (profileName) => { + const profile = makeBrowserProfile({ name: profileName }); + const state = makeBrowserServerState({ + profile, + resolvedOverrides: { profiles: {} }, + }); + state.profiles.set(profileName, { + profile, + running: { pid: 123 } as never, + lastTargetId: null, + reconcile: null, + }); + + const ctx = createBrowserRouteContext({ getState: () => state }); + const profiles = await ctx.listProfiles(); + + expect(profiles).toHaveLength(1); + expect(profiles[0]).toMatchObject({ name: profileName, missingFromConfig: true }); + }, + ); }); diff --git a/extensions/browser/src/browser/server-context.ts b/extensions/browser/src/browser/server-context.ts index 70c386c6e035..298d9b97862d 100644 --- a/extensions/browser/src/browser/server-context.ts +++ b/extensions/browser/src/browser/server-context.ts @@ -10,8 +10,7 @@ import { usesFastLoopbackCdpProbeClass } from "./cdp-timeouts.js"; import { redactCdpUrl } from "./cdp.helpers.js"; import { countChromeMcpTabs } from "./chrome-mcp.js"; import { isChromeReachable, resolveOpenClawUserDataDir } from "./chrome.js"; -import type { ResolvedBrowserProfile } from "./config.js"; -import { resolveProfile } from "./config.js"; +import { getOwnBrowserProfile, resolveProfile, type ResolvedBrowserProfile } from "./config.js"; import { BrowserProfileNotFoundError, BrowserProfileUnavailableError, @@ -366,7 +365,8 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon tabCount, isDefault: name === current.resolved.defaultProfile, isRemote: !statusProfile.cdpIsLoopback, - missingFromConfig: !(name in current.resolved.profiles) || undefined, + missingFromConfig: + getOwnBrowserProfile(current.resolved.profiles, name) === undefined || undefined, reconcileReason: unavailableReason, }); } diff --git a/extensions/browser/src/browser/trash.test.ts b/extensions/browser/src/browser/trash.test.ts index 0258dbc31ae3..98c0782825b9 100644 --- a/extensions/browser/src/browser/trash.test.ts +++ b/extensions/browser/src/browser/trash.test.ts @@ -4,15 +4,17 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const resolvePreferredOpenClawTmpDirMock = vi.hoisted(() => vi.fn(() => "/tmp/openclaw")); +const browserUtilsMock = vi.hoisted(() => ({ configDir: "/tmp/openclaw-state" })); const realMkdirSync = fs.mkdirSync.bind(fs); const realMkdtempSync = fs.mkdtempSync.bind(fs); const realRmSync = fs.rmSync.bind(fs); const realWriteFileSync = fs.writeFileSync.bind(fs); const realRealpathSyncNative = fs.realpathSync.native.bind(fs.realpathSync); -vi.mock("openclaw/plugin-sdk/temp-path", () => ({ - resolvePreferredOpenClawTmpDir: resolvePreferredOpenClawTmpDirMock, +vi.mock("../utils.js", () => ({ + get CONFIG_DIR() { + return browserUtilsMock.configDir; + }, })); function mockTrashContainer(...suffixes: string[]) { @@ -28,21 +30,20 @@ function mockTrashContainer(...suffixes: string[]) { describe("browser trash", () => { let testRoot = ""; + let configDir = ""; let homeDir = ""; - let tmpDir = ""; beforeEach(() => { vi.restoreAllMocks(); + vi.resetModules(); testRoot = realRealpathSyncNative(realMkdtempSync(path.join(os.tmpdir(), "openclaw-browser-"))); + configDir = path.join(testRoot, "state"); homeDir = path.join(testRoot, "home", "test"); - tmpDir = path.join(testRoot, "tmp"); + browserUtilsMock.configDir = configDir; + realMkdirSync(configDir, { recursive: true, mode: 0o700 }); realMkdirSync(path.join(homeDir, ".Trash"), { recursive: true, mode: 0o700 }); - realMkdirSync(tmpDir, { recursive: true, mode: 0o700 }); - resolvePreferredOpenClawTmpDirMock.mockReset(); - resolvePreferredOpenClawTmpDirMock.mockReturnValue(tmpDir); vi.spyOn(Date, "now").mockReturnValue(123); vi.spyOn(os, "homedir").mockReturnValue(homeDir); - vi.spyOn(os, "tmpdir").mockReturnValue(tmpDir); vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => realRealpathSyncNative(candidate), ); @@ -56,7 +57,9 @@ describe("browser trash", () => { }); function writeTrashTarget(name = "demo"): string { - const target = path.join(tmpDir, name); + const browserDir = path.join(configDir, "browser"); + realMkdirSync(browserDir, { recursive: true }); + const target = path.join(browserDir, name); realWriteFileSync(target, "demo"); return target; } @@ -82,6 +85,55 @@ describe("browser trash", () => { expect(rmSync).not.toHaveBeenCalled(); }); + it("allows managed browser data under a configured state directory outside home and temp", async () => { + const { movePathToTrash } = await import("./trash.js"); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("secure"); + const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined); + const target = path.join(configDir, "browser", "constructor"); + realMkdirSync(target, { recursive: true }); + const expected = path.join(homeDir, ".Trash", "constructor-123-secure", "constructor"); + + await expect(movePathToTrash(target)).resolves.toBe(expected); + expect(renameSync).toHaveBeenCalledWith(target, expected); + }); + + it("does not authorize other configured-state paths", async () => { + const { movePathToTrash } = await import("./trash.js"); + const target = path.join(configDir, "credentials", "token.json"); + realMkdirSync(path.dirname(target), { recursive: true }); + realWriteFileSync(target, "secret"); + + await expect(movePathToTrash(target)).rejects.toThrow( + "Refusing to trash path outside allowed roots", + ); + }); + + it("does not grant arbitrary filesystem authority for a root config directory", async () => { + browserUtilsMock.configDir = path.parse(testRoot).root; + const { movePathToTrash } = await import("./trash.js"); + const target = path.join(testRoot, "outside-root-browser"); + realWriteFileSync(target, "outside"); + + await expect(movePathToTrash(target)).rejects.toThrow( + "Refusing to trash path outside allowed roots", + ); + }); + + it("rejects browser-subtree symlinks that escape the configured state directory", async () => { + const { movePathToTrash } = await import("./trash.js"); + const browserDir = path.join(configDir, "browser"); + const outsideDir = path.join(testRoot, "outside-profile"); + realMkdirSync(browserDir, { recursive: true }); + realMkdirSync(outsideDir, { recursive: true }); + const target = path.join(browserDir, "constructor"); + fs.symlinkSync(outsideDir, target, "dir"); + + await expect(movePathToTrash(target)).rejects.toThrow( + "Refusing to trash path outside allowed roots", + ); + }); + it("uses the resolved trash directory for reserved destinations", async () => { const { movePathToTrash } = await import("./trash.js"); vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); diff --git a/extensions/browser/src/browser/trash.ts b/extensions/browser/src/browser/trash.ts index 9e6edc1230b9..e1e40d410bc9 100644 --- a/extensions/browser/src/browser/trash.ts +++ b/extensions/browser/src/browser/trash.ts @@ -1,14 +1,16 @@ /** - * Trash helpers for Browser-owned files constrained to user and OpenClaw temp - * roots. + * Trash helpers for data under the Browser-owned config subtree. */ -import os from "node:os"; +import path from "node:path"; import { movePathToTrash as movePathToTrashWithAllowedRoots } from "openclaw/plugin-sdk/browser-config"; -import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +import { CONFIG_DIR } from "../utils.js"; /** Moves a path to trash only when it lives under allowed Browser roots. */ export async function movePathToTrash(targetPath: string): Promise { return await movePathToTrashWithAllowedRoots(targetPath, { - allowedRoots: [os.homedir(), resolvePreferredOpenClawTmpDir()], + // Managed browser data follows OPENCLAW_STATE_DIR/OPENCLAW_CONFIG_PATH, which + // may intentionally live outside the OS home. Limit authority to Browser's + // owned subtree; fs-safe also checks target identity, realpaths, and symlinks. + allowedRoots: [path.join(CONFIG_DIR, "browser")], }); } diff --git a/src/config/io.write-prepare.ts b/src/config/io.write-prepare.ts index 7d80512593de..39cfbae35ac4 100644 --- a/src/config/io.write-prepare.ts +++ b/src/config/io.write-prepare.ts @@ -37,8 +37,8 @@ export function createMergePatch(base: unknown, target: unknown): unknown { const patch: Record = {}; const keys = new Set([...Object.keys(base), ...Object.keys(target)]); for (const key of keys) { - const hasBase = key in base; - const hasTarget = key in target; + const hasBase = Object.hasOwn(base, key); + const hasTarget = Object.hasOwn(target, key); if (!hasTarget) { patch[key] = null; continue; diff --git a/src/config/merge-patch.proto-pollution.test.ts b/src/config/merge-patch.proto-pollution.test.ts index ffafe8b10f16..9922cee79129 100644 --- a/src/config/merge-patch.proto-pollution.test.ts +++ b/src/config/merge-patch.proto-pollution.test.ts @@ -40,4 +40,38 @@ describe("applyMergePatch prototype pollution guard", () => { expect(result.nested.polluted).toBeUndefined(); expect(({} as Record).polluted).toBeUndefined(); }); + + it("allows prototype-like names only as direct browser profile keys", () => { + const names = ["constructor", "prototype"] as const; + const profile = { + cdpPort: 18801, + color: "#0066CC", + constructor: { polluted: true }, + prototype: { polluted: true }, + }; + const result = applyMergePatch( + { browser: { profiles: {} } }, + { + constructor: { polluted: true }, + browser: { + prototype: { polluted: true }, + profiles: Object.fromEntries(names.map((name) => [name, profile])), + }, + }, + ) as { browser?: { profiles?: Record> } }; + + expect(Object.hasOwn(result, "constructor")).toBe(false); + expect(Object.hasOwn(result.browser ?? {}, "prototype")).toBe(false); + const profiles = result.browser?.profiles ?? {}; + for (const name of names) { + expect(profiles[name]?.cdpPort).toBe(18801); + expect(Object.hasOwn(profiles[name] ?? {}, "constructor")).toBe(false); + expect(Object.hasOwn(profiles[name] ?? {}, "prototype")).toBe(false); + } + const removed = applyMergePatch(result, { + browser: { profiles: { constructor: null, prototype: null } }, + }) as { browser?: { profiles?: Record } }; + expect(Object.keys(removed.browser?.profiles ?? {})).toEqual([]); + expect(({} as Record).polluted).toBeUndefined(); + }); }); diff --git a/src/config/merge-patch.ts b/src/config/merge-patch.ts index 613977303b6e..fb4dc19c8201 100644 --- a/src/config/merge-patch.ts +++ b/src/config/merge-patch.ts @@ -25,6 +25,16 @@ function formatMergePatchArrayEntryPath(arrayPath: string): string { return `${arrayPath}[]`; } +/** Whether a merge-patch key is safe at its exact config path. */ +export function isMergePatchObjectKeyAllowed(key: string, parentPath?: string): boolean { + if (!isBlockedObjectKey(key)) { + return true; + } + // Browser profile names are schema-validated map ids. Their values still + // recurse through this guard, so nested prototype-related keys stay blocked. + return parentPath === "browser.profiles" && (key === "constructor" || key === "prototype"); +} + /** * Merge arrays of object-like entries keyed by `id`. * @@ -78,7 +88,8 @@ function mergeObjectArraysById( * Applies an RFC 7396-style object merge patch with OpenClaw config safeguards. * * Non-object patches replace the base, `null` deletes keys, blocked prototype - * keys are ignored, and id-keyed arrays may merge when the caller opts in. + * keys are ignored outside schema-owned record-key paths, and id-keyed arrays + * may merge when the caller opts in. */ export function applyMergePatch( base: unknown, @@ -92,10 +103,10 @@ export function applyMergePatch( const result: PlainObject = isPlainObject(base) ? { ...base } : {}; for (const [key, value] of Object.entries(patch)) { - if (isBlockedObjectKey(key)) { + const path = formatMergePatchPath(options.path, key); + if (!isMergePatchObjectKeyAllowed(key, options.path)) { continue; } - const path = formatMergePatchPath(options.path, key); if (value === null) { delete result[key]; continue; diff --git a/src/gateway/server-methods/config.ts b/src/gateway/server-methods/config.ts index 9f56c3ab09ac..d970235376ca 100644 --- a/src/gateway/server-methods/config.ts +++ b/src/gateway/server-methods/config.ts @@ -30,7 +30,7 @@ import { } from "../../config/io.js"; import { createMergePatch, projectSourceOntoRuntimeShape } from "../../config/io.write-prepare.js"; import { formatConfigIssueLines } from "../../config/issue-format.js"; -import { applyMergePatch } from "../../config/merge-patch.js"; +import { applyMergePatch, isMergePatchObjectKeyAllowed } from "../../config/merge-patch.js"; import { normalizeConfigPatchReplacePaths } from "../../config/patch-replace-paths.js"; import { redactConfigObject, @@ -47,7 +47,6 @@ import { import { isBuiltInModelProviderOverlayId } from "../../config/zod-schema.core.js"; import { formatErrorMessage, toErrorObject } from "../../infra/errors.js"; import { isPlainObject } from "../../infra/plain-object.js"; -import { isBlockedObjectKey } from "../../infra/prototype-keys.js"; import { prepareSecretsRuntimeSnapshot, type PreparedSecretsRuntimeSnapshot, @@ -155,10 +154,10 @@ function collectDestructiveArrayPatchPaths(params: { const merged = isPlainObject(params.merged) ? params.merged : {}; const paths: string[] = []; for (const [key, patchValue] of Object.entries(params.patch)) { - if (isBlockedObjectKey(key)) { + const path = formatConfigPatchPath(params.path ?? "", key); + if (!isMergePatchObjectKeyAllowed(key, params.path)) { continue; } - const path = formatConfigPatchPath(params.path ?? "", key); const baseValue = params.base[key]; const mergedValue = merged[key]; @@ -214,10 +213,11 @@ function collectBaseArrayPaths(base: unknown, path: string): string[] { } const paths: string[] = []; for (const [key, value] of Object.entries(base)) { - if (isBlockedObjectKey(key)) { + const childPath = formatConfigPatchPath(path, key); + if (!isMergePatchObjectKeyAllowed(key, path)) { continue; } - paths.push(...collectBaseArrayPaths(value, formatConfigPatchPath(path, key))); + paths.push(...collectBaseArrayPaths(value, childPath)); } return paths; } diff --git a/src/gateway/server.config-patch.test.ts b/src/gateway/server.config-patch.test.ts index 21298ee6aed3..e19b3463269e 100644 --- a/src/gateway/server.config-patch.test.ts +++ b/src/gateway/server.config-patch.test.ts @@ -404,6 +404,61 @@ describe("gateway config methods", () => { } }); + it("round-trips prototype-like browser profile names through config.patch", async () => { + const original = await getCurrentConfigObject(); + const profileNames = ["constructor", "prototype"] as const; + + try { + const create = await rpcReq<{ ok?: boolean }>(requireWs(), "config.patch", { + raw: JSON.stringify({ + browser: { + profiles: Object.fromEntries( + profileNames.map((name, index) => [ + name, + { + cdpPort: 18991 + index, + color: "#0066CC", + constructor: { polluted: true }, + prototype: { polluted: true }, + }, + ]), + ), + }, + }), + baseHash: original.hash, + }); + expect(create.ok).toBe(true); + + const afterCreate = await getCurrentConfigObject(); + const browser = requireConfigObject(afterCreate.config.browser, "browser"); + const profiles = requireConfigObject(browser.profiles, "browser.profiles"); + for (const [index, name] of profileNames.entries()) { + const profile = requireConfigObject(profiles[name], `browser.profiles.${name}`); + expect(profile.cdpPort).toBe(18991 + index); + expect(Object.hasOwn(profile, "constructor")).toBe(false); + expect(Object.hasOwn(profile, "prototype")).toBe(false); + } + expect(({} as Record).polluted).toBeUndefined(); + + const remove = await rpcReq<{ ok?: boolean }>(requireWs(), "config.patch", { + raw: JSON.stringify({ + browser: { profiles: { constructor: null, prototype: null } }, + }), + baseHash: afterCreate.hash, + }); + expect(remove.ok).toBe(true); + + const afterRemove = await getCurrentConfigObject(); + const afterBrowser = requireConfigObject(afterRemove.config.browser, "browser"); + const afterProfiles = requireConfigObject(afterBrowser.profiles, "browser.profiles"); + for (const name of profileNames) { + expect(Object.hasOwn(afterProfiles, name)).toBe(false); + } + } finally { + await restoreConfigFileForTest(original); + } + }); + it("does not reject config.set for unresolved auth-profile refs outside submitted config", async () => { const missingEnvVar = `OPENCLAW_MISSING_AUTH_PROFILE_REF_${Date.now()}`; await writeUnresolvedAuthProfileTokenRef(missingEnvVar);