diff --git a/extensions/migrate-hermes/config.test.ts b/extensions/migrate-hermes/config.test.ts index a17455038a5c..ab526304028f 100644 --- a/extensions/migrate-hermes/config.test.ts +++ b/extensions/migrate-hermes/config.test.ts @@ -187,6 +187,58 @@ describe("Hermes migration config mapping", () => { expect(config.skills?.entries?.["ship-it"]?.config?.mode).toBe("fast"); }); + it("drops prototype-bearing provider, MCP, and skill keys during apply", async () => { + const root = await makeTempRoot(); + const source = path.join(root, "hermes"); + const workspaceDir = path.join(root, "workspace"); + const stateDir = path.join(root, "state"); + const config = { + agents: { defaults: { workspace: workspaceDir } }, + } as OpenClawConfig; + await writeFile( + path.join(source, "config.yaml"), + [ + "providers:", + ' "__proto__":', + " base_url: https://untrusted.example/v1", + " models: [untrusted-model]", + "mcp_servers:", + " constructor:", + " command: untrusted-command", + " safe-server:", + " command: safe-command", + "skills:", + " config:", + " prototype:", + " mode: untrusted", + " safe-skill:", + " mode: safe", + "", + ].join("\n"), + ); + + const result = await buildHermesMigrationProvider().apply( + makeContext({ + source, + stateDir, + workspaceDir, + runtime: makeConfigRuntime(config), + }), + ); + + expect(result.summary.errors).toBe(0); + const providers = config.models?.providers as Record; + const servers = config.mcp?.servers as Record; + const skills = config.skills?.entries as Record; + expect(Object.hasOwn(providers, "__proto__")).toBe(false); + expect(Object.hasOwn(servers, "constructor")).toBe(false); + expect(Object.hasOwn(skills, "prototype")).toBe(false); + expect(Object.getPrototypeOf(providers)).toBe(Object.prototype); + expect((servers["safe-server"] as { command?: string }).command).toBe("safe-command"); + expect((skills["safe-skill"] as { config?: { mode?: string } }).config?.mode).toBe("safe"); + expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); + }); + it("uses the provider runtime for CLI-applied config items", async () => { const root = await makeTempRoot(); const source = path.join(root, "hermes"); diff --git a/src/plugin-sdk/migration-config.test.ts b/src/plugin-sdk/migration-config.test.ts new file mode 100644 index 000000000000..9e83e5371942 --- /dev/null +++ b/src/plugin-sdk/migration-config.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from "vitest"; +import type { MigrationProviderContext } from "../plugins/types.js"; +import { + applyMigrationConfigPatchItem, + createMigrationConfigPatchItem, + hasMigrationConfigPatchConflict, + mergeMigrationConfigValue, + writeMigrationConfigPath, +} from "./migration.js"; + +describe("migration config patches", () => { + it("drops blocked keys recursively from objects and arrays", () => { + const patch = JSON.parse( + '{"__proto__":{"polluted":true},"safe":{"prototype":{"polluted":true},"next":true},"list":[{"constructor":{"polluted":true},"kept":true}]}', + ); + + const merged = mergeMigrationConfigValue({ safe: { keep: true } }, patch) as Record< + string, + unknown + >; + + expect(merged).toEqual({ + safe: { keep: true, next: true }, + list: [{ kept: true }], + }); + expect(Object.getPrototypeOf(merged)).toBe(Object.prototype); + expect(Object.getPrototypeOf(merged.safe as object)).toBe(Object.prototype); + expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); + }); + + it("rejects blocked path segments before mutating the target", () => { + const config: Record = {}; + + expect(() => + writeMigrationConfigPath(config, ["models", "__proto__", "polluted"], true), + ).toThrow("unsafe config patch path"); + expect(config).toEqual({}); + expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); + }); + + it("reports unsafe migration items as errors without calling the config writer", async () => { + const mutateConfigFile = vi.fn(); + const config: MigrationProviderContext["config"] = {}; + const ctx = { + config, + stateDir: "/tmp/openclaw-migration-test", + logger: {}, + runtime: { + config: { + current: () => config, + mutateConfigFile, + }, + }, + } as unknown as MigrationProviderContext; + const item = createMigrationConfigPatchItem({ + id: "config:unsafe", + target: "models.__proto__.polluted", + path: ["models", "__proto__", "polluted"], + value: true, + message: "unsafe patch", + }); + + await expect(applyMigrationConfigPatchItem(ctx, item)).resolves.toEqual( + expect.objectContaining({ status: "error", reason: "unsafe config patch path" }), + ); + expect(mutateConfigFile).not.toHaveBeenCalled(); + }); + + it("ignores blocked and inherited keys during conflict checks", () => { + const patch = JSON.parse( + '{"__proto__":{"polluted":true},"toString":{"command":"safe-own-value"}}', + ); + + expect( + hasMigrationConfigPatchConflict({ mcp: { servers: {} } }, ["mcp", "servers"], patch), + ).toBe(false); + }); +}); diff --git a/src/plugin-sdk/migration.ts b/src/plugin-sdk/migration.ts index 3f97cd3ffbee..aba062c30efa 100644 --- a/src/plugin-sdk/migration.ts +++ b/src/plugin-sdk/migration.ts @@ -1,6 +1,7 @@ // Shared migration-provider helpers for plan/apply item bookkeeping. import { isRecord } from "../../packages/normalization-core/src/record-coerce.js"; +import { isBlockedObjectKey } from "../infra/prototype-keys.js"; import type { MigrationDetection, MigrationItem, @@ -115,11 +116,40 @@ class MigrationConfigPatchConflictError extends Error { } } +const MIGRATION_REASON_UNSAFE_CONFIG_PATCH_PATH = "unsafe config patch path"; + +function isSafeMigrationConfigPath(path: readonly string[]): boolean { + return ( + path.length > 0 && path.every((segment) => segment.length > 0 && !isBlockedObjectKey(segment)) + ); +} + +function cloneMigrationConfigValue(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map((entry) => cloneMigrationConfigValue(entry)); + } + if (!isRecord(value)) { + return structuredClone(value); + } + const next: Record = {}; + for (const [key, entry] of Object.entries(value)) { + // Migration patches come from external tools. Drop prototype-bearing keys + // recursively before any value reaches a live config object. + if (!isBlockedObjectKey(key)) { + next[key] = cloneMigrationConfigValue(entry); + } + } + return next; +} + /** Reads a nested config value, returning undefined when a parent is not an object. */ export function readMigrationConfigPath( root: Record, path: readonly string[], ): unknown { + if (!isSafeMigrationConfigPath(path)) { + return undefined; + } let current: unknown = root; for (const segment of path) { if (!isRecord(current)) { @@ -133,10 +163,13 @@ export function readMigrationConfigPath( /** Deep-merges object patches and replaces scalar/array values with a cloned target value. */ export function mergeMigrationConfigValue(left: unknown, right: unknown): unknown { if (!isRecord(left) || !isRecord(right)) { - return structuredClone(right); + return cloneMigrationConfigValue(right); } const next: Record = { ...left }; for (const [key, value] of Object.entries(right)) { + if (isBlockedObjectKey(key)) { + continue; + } next[key] = mergeMigrationConfigValue(next[key], value); } return next; @@ -148,6 +181,9 @@ export function writeMigrationConfigPath( path: readonly string[], value: unknown, ): void { + if (!isSafeMigrationConfigPath(path)) { + throw new Error(MIGRATION_REASON_UNSAFE_CONFIG_PATCH_PATH); + } let current = root; for (const segment of path.slice(0, -1)) { const existing = current[segment]; @@ -158,7 +194,7 @@ export function writeMigrationConfigPath( } const leaf = path.at(-1); if (!leaf) { - return; + throw new Error(MIGRATION_REASON_UNSAFE_CONFIG_PATCH_PATH); } current[leaf] = mergeMigrationConfigValue(current[leaf], value); } @@ -177,7 +213,10 @@ export function hasMigrationConfigPatchConflict( if (!isRecord(existing)) { return false; } - return Object.keys(value).some((key) => existing[key] !== undefined); + return Object.keys(value).some( + (key) => + !isBlockedObjectKey(key) && Object.hasOwn(existing, key) && existing[key] !== undefined, + ); } /** Builds a planned or conflicting config-merge migration item. */ @@ -251,6 +290,9 @@ export async function applyMigrationConfigPatchItem( if (!details) { return markMigrationItemError(item, "missing config patch"); } + if (!isSafeMigrationConfigPath(details.path)) { + return markMigrationItemError(item, MIGRATION_REASON_UNSAFE_CONFIG_PATCH_PATH); + } const configApi = ctx.runtime?.config; if (!configApi?.current || !configApi.mutateConfigFile) { return markMigrationItemError(item, "config runtime unavailable");