mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 22:17:00 +00:00
fix(browser): preserve own profile entries (#101694)
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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<void> {
|
||||
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 = {
|
||||
|
||||
@@ -121,6 +121,14 @@ export type ResolvedBrowserProfile = {
|
||||
attachOnly: boolean;
|
||||
};
|
||||
|
||||
/** Read a named browser profile without falling through to inherited object keys. */
|
||||
export function getOwnBrowserProfile<T>(
|
||||
profiles: Record<string, T> | 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;
|
||||
}
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<string> {
|
||||
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")],
|
||||
});
|
||||
}
|
||||
|
||||
@@ -37,8 +37,8 @@ export function createMergePatch(base: unknown, target: unknown): unknown {
|
||||
const patch: Record<string, unknown> = {};
|
||||
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;
|
||||
|
||||
@@ -40,4 +40,38 @@ describe("applyMergePatch prototype pollution guard", () => {
|
||||
expect(result.nested.polluted).toBeUndefined();
|
||||
expect(({} as Record<string, unknown>).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<string, Record<string, unknown>> } };
|
||||
|
||||
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<string, unknown> } };
|
||||
expect(Object.keys(removed.browser?.profiles ?? {})).toEqual([]);
|
||||
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>).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);
|
||||
|
||||
Reference in New Issue
Block a user