From 07fdb50b028b4a82d96dfbbdf1d812e054acbb61 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 4 Jun 2026 03:14:05 +0200 Subject: [PATCH] fix(plugin-sdk): guard facade registry rows --- .../facade-activation-check.runtime.ts | 84 ++++++-- src/plugin-sdk/facade-resolution-shared.ts | 89 +++++++-- src/plugin-sdk/facade-runtime.test.ts | 189 +++++++++++++----- 3 files changed, 286 insertions(+), 76 deletions(-) diff --git a/src/plugin-sdk/facade-activation-check.runtime.ts b/src/plugin-sdk/facade-activation-check.runtime.ts index 89fef908e2a6..ea412ab8c31d 100644 --- a/src/plugin-sdk/facade-activation-check.runtime.ts +++ b/src/plugin-sdk/facade-activation-check.runtime.ts @@ -25,7 +25,11 @@ import { type PluginManifestRecord, } from "../plugins/manifest-registry.js"; import { parseJsonWithJson5Fallback } from "../utils/parse-json-compat.js"; -import { resolveRegistryPluginModuleLocationFromRecords } from "./facade-resolution-shared.js"; +import { + readFacadeRegistryRecord, + type FacadeRegistryRecordLike, + resolveRegistryPluginModuleLocationFromRecords, +} from "./facade-resolution-shared.js"; const ALWAYS_ALLOWED_RUNTIME_DIR_NAMES = new Set([ "image-generation-core", @@ -224,24 +228,80 @@ function resolveBundledPluginManifestRecord(params: { } const registry = getFacadeManifestRegistry(params.env ? { env: params.env } : {}); + const records = registry.flatMap((record) => { + const safeRecord = readFacadePluginManifestRecord(record); + return safeRecord ? [safeRecord] : []; + }); const resolved = (params.location - ? registry.find((plugin) => { - const normalizedRootDir = path.resolve(plugin.rootDir); - const normalizedModulePath = path.resolve(params.location!.modulePath); - return ( - normalizedModulePath === normalizedRootDir || - normalizedModulePath.startsWith(`${normalizedRootDir}${path.sep}`) - ); - }) + ? records.find((plugin) => + isFacadeManifestRecordLocationMatch({ + modulePath: params.location!.modulePath, + record: plugin, + }), + ) : null) ?? - registry.find((plugin) => plugin.id === params.dirName) ?? - registry.find((plugin) => path.basename(plugin.rootDir) === params.dirName) ?? - registry.find((plugin) => plugin.channels.includes(params.dirName)) ?? + records.find((plugin) => plugin.id === params.dirName) ?? + records.find((plugin) => path.basename(plugin.rootDir) === params.dirName) ?? + records.find((plugin) => plugin.channels.includes(params.dirName)) ?? null; return resolved; } +function readFacadePluginManifestRecord(record: unknown): FacadePluginManifestLike | null { + const base = readFacadeRegistryRecord(record); + if (!base) { + return null; + } + + try { + const candidate = record as { + enabledByDefault?: unknown; + enabledByDefaultOnPlatforms?: unknown; + origin?: unknown; + }; + if (!isPluginOrigin(candidate.origin)) { + return null; + } + return { + ...base, + origin: candidate.origin, + ...(typeof candidate.enabledByDefault === "boolean" + ? { enabledByDefault: candidate.enabledByDefault } + : {}), + ...(Array.isArray(candidate.enabledByDefaultOnPlatforms) + ? { + enabledByDefaultOnPlatforms: candidate.enabledByDefaultOnPlatforms.filter( + (platform): platform is string => typeof platform === "string", + ), + } + : {}), + }; + } catch { + return null; + } +} + +function isPluginOrigin(value: unknown): value is FacadePluginManifestLike["origin"] { + return value === "bundled" || value === "global" || value === "workspace" || value === "config"; +} + +function isFacadeManifestRecordLocationMatch(params: { + modulePath: string; + record: FacadeRegistryRecordLike; +}): boolean { + try { + const normalizedRootDir = path.resolve(params.record.rootDir); + const normalizedModulePath = path.resolve(params.modulePath); + return ( + normalizedModulePath === normalizedRootDir || + normalizedModulePath.startsWith(`${normalizedRootDir}${path.sep}`) + ); + } catch { + return false; + } +} + /** Resolves the stable plugin id used for telemetry and error reporting. */ export function resolveTrackedFacadePluginId(params: { dirName: string; diff --git a/src/plugin-sdk/facade-resolution-shared.ts b/src/plugin-sdk/facade-resolution-shared.ts index 6118cc34e1af..24fb107bce54 100644 --- a/src/plugin-sdk/facade-resolution-shared.ts +++ b/src/plugin-sdk/facade-resolution-shared.ts @@ -17,10 +17,10 @@ export type FacadeModuleLocationLike = { boundaryRoot: string; }; -type FacadeRegistryRecordLike = { +export type FacadeRegistryRecordLike = { id: string; rootDir: string; - channels: readonly string[]; + channels: string[]; }; /** Builds the cache key for one facade lookup under the current bundled-plugin mode. */ @@ -106,6 +106,10 @@ export function resolveRegistryPluginModuleLocationFromRecords(params: { dirName: string; artifactBasename: string; }): FacadeModuleLocationLike | null { + const records = params.registry.flatMap((record) => { + const safeRecord = readFacadeRegistryRecord(record); + return safeRecord ? [safeRecord] : []; + }); const tiers: Array<(plugin: FacadeRegistryRecordLike) => boolean> = [ (plugin) => plugin.id === params.dirName, (plugin) => path.basename(plugin.rootDir) === params.dirName, @@ -114,23 +118,76 @@ export function resolveRegistryPluginModuleLocationFromRecords(params: { const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); const sourceBaseName = artifactBasename.replace(/\.js$/u, ""); for (const matchFn of tiers) { - for (const record of params.registry.filter(matchFn)) { - const rootDir = path.resolve(record.rootDir); - for (const builtCandidate of [ - path.join(rootDir, artifactBasename), - path.join(rootDir, "dist", artifactBasename), - ]) { - if (fs.existsSync(builtCandidate)) { - return { modulePath: builtCandidate, boundaryRoot: rootDir }; - } + for (const record of records) { + if (!matchFn(record)) { + continue; } - for (const ext of PUBLIC_SURFACE_SOURCE_EXTENSIONS) { - const sourceCandidate = path.join(rootDir, `${sourceBaseName}${ext}`); - if (fs.existsSync(sourceCandidate)) { - return { modulePath: sourceCandidate, boundaryRoot: rootDir }; - } + const location = resolveFacadeRegistryRecordLocation({ + record, + artifactBasename, + sourceBaseName, + }); + if (location) { + return location; } } } return null; } + +export function readFacadeRegistryRecord(record: unknown): FacadeRegistryRecordLike | null { + if (!record || typeof record !== "object") { + return null; + } + + try { + const candidate = record as { + channels?: unknown; + id?: unknown; + rootDir?: unknown; + }; + const { channels, id, rootDir } = candidate; + if (typeof id !== "string" || id.length === 0) { + return null; + } + if (typeof rootDir !== "string" || rootDir.length === 0) { + return null; + } + return { + id, + rootDir, + channels: Array.isArray(channels) + ? channels.filter((channel): channel is string => typeof channel === "string") + : [], + }; + } catch { + return null; + } +} + +function resolveFacadeRegistryRecordLocation(params: { + record: FacadeRegistryRecordLike; + artifactBasename: string; + sourceBaseName: string; +}): FacadeModuleLocationLike | null { + try { + const rootDir = path.resolve(params.record.rootDir); + for (const builtCandidate of [ + path.join(rootDir, params.artifactBasename), + path.join(rootDir, "dist", params.artifactBasename), + ]) { + if (fs.existsSync(builtCandidate)) { + return { modulePath: builtCandidate, boundaryRoot: rootDir }; + } + } + for (const ext of PUBLIC_SURFACE_SOURCE_EXTENSIONS) { + const sourceCandidate = path.join(rootDir, `${params.sourceBaseName}${ext}`); + if (fs.existsSync(sourceCandidate)) { + return { modulePath: sourceCandidate, boundaryRoot: rootDir }; + } + } + } catch { + return null; + } + return null; +} diff --git a/src/plugin-sdk/facade-runtime.test.ts b/src/plugin-sdk/facade-runtime.test.ts index c4ca8d2a7b35..501b1959616d 100644 --- a/src/plugin-sdk/facade-runtime.test.ts +++ b/src/plugin-sdk/facade-runtime.test.ts @@ -32,6 +32,53 @@ const trustedBundledFixturesRoot = path.resolve("dist-runtime", "extensions"); const trustedBundledFixtureDirs: string[] = []; type SnapshotPluginRecord = PluginMetadataSnapshot["manifestRegistry"]["plugins"][number]; +function createPluginMetadataSnapshotFixture( + params: { + config?: OpenClawConfig; + plugins?: SnapshotPluginRecord[]; + } = {}, +): PluginMetadataSnapshot { + const policyHash = resolveInstalledPluginIndexPolicyHash(params.config); + return { + policyHash, + index: { + version: 1, + hostContractVersion: "test", + compatRegistryVersion: "test", + migrationVersion: 1, + policyHash, + generatedAtMs: 1, + installRecords: {}, + plugins: [], + diagnostics: [], + }, + registryDiagnostics: [], + manifestRegistry: { plugins: params.plugins ?? [], diagnostics: [] }, + plugins: [], + diagnostics: [], + byPluginId: new Map(), + normalizePluginId: (pluginId) => pluginId, + owners: { + channels: new Map(), + channelConfigs: new Map(), + providers: new Map(), + modelCatalogProviders: new Map(), + cliBackends: new Map(), + setupProviders: new Map(), + commandAliases: new Map(), + contracts: new Map(), + }, + metrics: { + registrySnapshotMs: 0, + manifestRegistryMs: 0, + ownerMapsMs: 0, + totalMs: 0, + indexPluginCount: 0, + manifestPluginCount: 0, + }, + }; +} + function writeJsonFile(filePath: string, value: unknown): void { fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync(filePath, `${JSON.stringify(value, null, 2)}\n`, "utf8"); @@ -413,6 +460,39 @@ describe("plugin-sdk facade runtime", () => { }); }); + it("skips unreadable registry records while resolving plugin facade locations", () => { + const lineDir = createTempDirSync("openclaw-facade-poisoned-registry-"); + fs.mkdirSync(lineDir, { recursive: true }); + fs.writeFileSync( + path.join(lineDir, "runtime-api.js"), + 'export const marker = "poisoned-registry-ok";\n', + "utf8", + ); + const poisonedRecord = Object.defineProperty({}, "id", { + get() { + throw new Error("facade registry plugin id exploded"); + }, + }); + + expect( + testing.resolveRegistryPluginModuleLocationFromRegistry({ + registry: [ + poisonedRecord as never, + { + id: "line", + rootDir: lineDir, + channels: ["line"], + }, + ], + dirName: "line", + artifactBasename: "runtime-api.js", + }), + ).toEqual({ + modulePath: path.join(lineDir, "runtime-api.js"), + boundaryRoot: lineDir, + }); + }); + it("resolves a globally-installed plugin public surface from package dist", () => { const lineDir = createTempDirSync("openclaw-facade-global-line-dist-"); fs.mkdirSync(path.join(lineDir, "dist"), { recursive: true }); @@ -578,6 +658,66 @@ describe("plugin-sdk facade runtime", () => { }); }); + it("skips unreadable registry records while resolving facade activation metadata", () => { + const dir = createTempDirSync("openclaw-facade-activation-poisoned-"); + const pluginDir = path.join(dir, "demo"); + fs.mkdirSync(pluginDir, { recursive: true }); + const runtimeApiPath = path.join(pluginDir, "runtime-api.js"); + fs.writeFileSync(runtimeApiPath, 'export const marker = "activation-ok";\n', "utf8"); + const config = { + plugins: { + entries: { + demo: { + enabled: true, + }, + }, + }, + } satisfies OpenClawConfig; + const poisonedRecord = Object.defineProperty({}, "rootDir", { + get() { + throw new Error("facade activation manifest rootDir exploded"); + }, + }); + setRuntimeConfigSnapshot(config, config); + setCurrentPluginMetadataSnapshot( + createPluginMetadataSnapshotFixture({ + config, + plugins: [ + poisonedRecord as never, + { + id: "demo", + rootDir: pluginDir, + source: runtimeApiPath, + manifestPath: path.join(pluginDir, "openclaw.plugin.json"), + channels: ["demo"], + providers: [], + cliBackends: [], + skills: [], + hooks: [], + origin: "bundled" as const, + }, + ], + }), + { config }, + ); + + expect( + resolveActivationCheckBundledPluginPublicSurfaceAccess({ + dirName: "demo", + artifactBasename: "runtime-api.js", + location: { + modulePath: runtimeApiPath, + boundaryRoot: dir, + }, + sourceExtensionsRoot: path.join(dir, "source-root"), + resolutionKey: "activation-poisoned-demo", + }), + ).toEqual({ + allowed: true, + pluginId: "demo", + }); + }); + it("validates current snapshot against facade boundary config and ignores on mismatch", () => { const dir = createTempDirSync("openclaw-facade-snapshot-validate-"); fs.mkdirSync(path.join(dir, "demo"), { recursive: true }); @@ -589,53 +729,6 @@ describe("plugin-sdk facade runtime", () => { // Do NOT write openclaw.plugin.json on disk to force fallback to registry scan useBundledPluginDirOverrideForTest(dir); - function createTestSnapshot( - params: { - config?: OpenClawConfig; - plugins?: SnapshotPluginRecord[]; - } = {}, - ): PluginMetadataSnapshot { - const policyHash = resolveInstalledPluginIndexPolicyHash(params.config); - return { - policyHash, - index: { - version: 1, - hostContractVersion: "test", - compatRegistryVersion: "test", - migrationVersion: 1, - policyHash, - generatedAtMs: 1, - installRecords: {}, - plugins: [], - diagnostics: [], - }, - registryDiagnostics: [], - manifestRegistry: { plugins: params.plugins ?? [], diagnostics: [] }, - plugins: [], - diagnostics: [], - byPluginId: new Map(), - normalizePluginId: (pluginId) => pluginId, - owners: { - channels: new Map(), - channelConfigs: new Map(), - providers: new Map(), - modelCatalogProviders: new Map(), - cliBackends: new Map(), - setupProviders: new Map(), - commandAliases: new Map(), - contracts: new Map(), - }, - metrics: { - registrySnapshotMs: 0, - manifestRegistryMs: 0, - ownerMapsMs: 0, - totalMs: 0, - indexPluginCount: 0, - manifestPluginCount: 0, - }, - }; - } - const configWithPaths = { plugins: { load: { paths: ["/path/one"] }, @@ -645,7 +738,7 @@ describe("plugin-sdk facade runtime", () => { }, }, } satisfies OpenClawConfig; - const matchedSnapshot = createTestSnapshot({ + const matchedSnapshot = createPluginMetadataSnapshotFixture({ config: configWithPaths, plugins: [ {