From 97e2af7f97d4d1ff89260807b0ca4977c7101ba6 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sun, 15 Mar 2026 14:55:53 +0000 Subject: [PATCH] Plugins: add loader discovery policy --- src/extension-host/cutover-inventory.md | 7 +- .../loader-discovery-policy.test.ts | 39 ++++++++ src/extension-host/loader-discovery-policy.ts | 33 +++++++ src/extension-host/loader-orchestrator.ts | 8 +- src/extension-host/loader-policy.test.ts | 85 ------------------ src/extension-host/loader-policy.ts | 89 ------------------- 6 files changed, 81 insertions(+), 180 deletions(-) create mode 100644 src/extension-host/loader-discovery-policy.test.ts create mode 100644 src/extension-host/loader-discovery-policy.ts diff --git a/src/extension-host/cutover-inventory.md b/src/extension-host/cutover-inventory.md index 4d0e64c4ad1..0c10492389e 100644 --- a/src/extension-host/cutover-inventory.md +++ b/src/extension-host/cutover-inventory.md @@ -33,7 +33,8 @@ This is an implementation checklist, not a future-design spec. | Manifest/package metadata loading | `src/plugins/manifest.ts`, `src/plugins/discovery.ts`, `src/plugins/install.ts` | `src/extension-host/schema.ts` and `src/extension-host/manifest-registry.ts` | `partial` | Package metadata parsing is routed through host schema helpers; legacy loader flow still supplies the source manifests. | | Loader SDK alias compatibility | `src/plugins/loader.ts` | `src/extension-host/loader-compat.ts` | `partial` | Plugin-SDK alias candidate ordering, alias-file resolution, and scoped alias-map construction now live in host-owned loader compatibility helpers. | | Loader cache key and registry cache control | `src/plugins/loader.ts` | `src/extension-host/loader-cache.ts` | `partial` | Cache-key construction, LRU registry cache reads and writes, and cache clearing now delegate through host-owned loader-cache helpers while preserving the current cache shape and cap. | -| Loader provenance and duplicate-order policy | `src/plugins/loader.ts` | `src/extension-host/loader-policy.ts` | `partial` | Plugin-record creation, duplicate precedence, provenance indexing, and allowlist/untracked warnings now live in host-owned loader-policy helpers. | +| Loader provenance and duplicate-order policy | `src/plugins/loader.ts` | `src/extension-host/loader-policy.ts` | `partial` | Plugin-record creation, duplicate precedence, and provenance indexing now live in host-owned loader-policy helpers. | +| Loader discovery policy results | mixed inside `src/plugins/loader.ts`, `src/extension-host/loader-policy.ts`, and `src/extension-host/loader-orchestrator.ts` | `src/extension-host/loader-discovery-policy.ts` | `partial` | Open-allowlist discovery warnings now resolve through explicit host-owned discovery-policy results before the orchestrator logs them. | | Loader initial candidate planning and record creation | `src/plugins/loader.ts` | `src/extension-host/loader-records.ts` | `partial` | Duplicate detection, initial record creation, manifest metadata attachment, and first-pass enable-state planning now delegate through host-owned loader-records helpers. | | Loader entry-path opening and module import | `src/plugins/loader.ts` | `src/extension-host/loader-import.ts` | `partial` | Boundary-checked entry opening and module import now delegate through host-owned loader-import helpers while preserving the current trusted in-process loading model. | | Loader module-export, config-validation, and memory-slot decisions | `src/plugins/loader.ts` | `src/extension-host/loader-runtime.ts` | `partial` | Module export resolution, export-metadata application, config validation, and early or final memory-slot decisions now delegate through host-owned loader-runtime helpers. | @@ -90,14 +91,14 @@ That pattern has been used for: - active registry ownership - normalized extension schema and resolved-extension records - static consumers such as skills, validation, auto-enable, and config baseline generation -- loader compatibility, cache control, initial candidate planning, entry-path import, explicit activation-policy outcomes, runtime decisions, post-import register flow, per-candidate orchestration, top-level load orchestration, session-owned activation state, explicit loader lifecycle transitions, explicit finalization-policy results, and final cache plus activation finalization +- loader compatibility, cache control, initial candidate planning, entry-path import, explicit discovery-policy outcomes, explicit activation-policy outcomes, runtime decisions, post-import register flow, per-candidate orchestration, top-level load orchestration, session-owned activation state, explicit loader lifecycle transitions, explicit finalization-policy results, and final cache plus activation finalization ## Immediate Next Targets These are the next lowest-risk cutover steps: 1. Replace remaining static-only manifest-registry injections with resolved-extension registry inputs where practical. -2. Extend the new loader lifecycle state machine, session-owned activation state, activation-policy outcomes, and finalization-policy results into broader activation-state and policy ownership in `src/extension-host/*`. +2. Extend the new loader lifecycle state machine, session-owned activation state, discovery-policy outcomes, activation-policy outcomes, and finalization-policy results into broader activation-state and policy ownership in `src/extension-host/*`. 3. Introduce explicit host-owned registration surfaces for runtime writes, starting with the least-coupled registries. 4. Move minimal SDK compatibility and loader normalization into `src/extension-host/*` without breaking current `openclaw/plugin-sdk/*` loading. 5. Start the first pilot on `extensions/thread-ownership` only after the host-side registry and lifecycle seams are explicit. diff --git a/src/extension-host/loader-discovery-policy.test.ts b/src/extension-host/loader-discovery-policy.test.ts new file mode 100644 index 00000000000..1cdc8cb793b --- /dev/null +++ b/src/extension-host/loader-discovery-policy.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { resolveExtensionHostDiscoveryPolicy } from "./loader-discovery-policy.js"; + +describe("extension host loader discovery policy", () => { + it("warns when allowlist is open for non-bundled discoverable plugins", () => { + const warningCache = new Set(); + + const result = resolveExtensionHostDiscoveryPolicy({ + pluginsEnabled: true, + allow: [], + warningCacheKey: "warn-key", + warningCache, + discoverablePlugins: [ + { id: "bundled", source: "/bundled/index.js", origin: "bundled" }, + { id: "workspace-demo", source: "/workspace/demo.js", origin: "workspace" }, + ], + }); + + expect(result.warningMessages).toHaveLength(1); + expect(result.warningMessages[0]).toContain("plugins.allow is empty"); + expect(warningCache.has("warn-key")).toBe(true); + }); + + it("does not warn twice for the same cache key", () => { + const warningCache = new Set(["warn-key"]); + + const result = resolveExtensionHostDiscoveryPolicy({ + pluginsEnabled: true, + allow: [], + warningCacheKey: "warn-key", + warningCache, + discoverablePlugins: [ + { id: "workspace-demo", source: "/workspace/demo.js", origin: "workspace" }, + ], + }); + + expect(result.warningMessages).toHaveLength(0); + }); +}); diff --git a/src/extension-host/loader-discovery-policy.ts b/src/extension-host/loader-discovery-policy.ts new file mode 100644 index 00000000000..fb245d27f13 --- /dev/null +++ b/src/extension-host/loader-discovery-policy.ts @@ -0,0 +1,33 @@ +import type { PluginRecord } from "../plugins/registry.js"; + +export function resolveExtensionHostDiscoveryPolicy(params: { + pluginsEnabled: boolean; + allow: string[]; + warningCacheKey: string; + warningCache: Set; + discoverablePlugins: Array<{ id: string; source: string; origin: PluginRecord["origin"] }>; +}): { + warningMessages: string[]; +} { + if (!params.pluginsEnabled || params.allow.length > 0) { + return { warningMessages: [] }; + } + + const nonBundled = params.discoverablePlugins.filter((entry) => entry.origin !== "bundled"); + if (nonBundled.length === 0 || params.warningCache.has(params.warningCacheKey)) { + return { warningMessages: [] }; + } + + const preview = nonBundled + .slice(0, 6) + .map((entry) => `${entry.id} (${entry.source})`) + .join(", "); + const extra = nonBundled.length > 6 ? ` (+${nonBundled.length - 6} more)` : ""; + params.warningCache.add(params.warningCacheKey); + + return { + warningMessages: [ + `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, + ], + }; +} diff --git a/src/extension-host/loader-orchestrator.ts b/src/extension-host/loader-orchestrator.ts index 2016ad69e48..f66a67de9f6 100644 --- a/src/extension-host/loader-orchestrator.ts +++ b/src/extension-host/loader-orchestrator.ts @@ -7,11 +7,11 @@ import { getCachedExtensionHostRegistry, setCachedExtensionHostRegistry, } from "../extension-host/loader-cache.js"; +import { resolveExtensionHostDiscoveryPolicy } from "../extension-host/loader-discovery-policy.js"; import { buildExtensionHostProvenanceIndex, compareExtensionHostDuplicateCandidateOrder, pushExtensionHostDiagnostics, - warnWhenExtensionAllowlistIsOpen, } from "../extension-host/loader-policy.js"; import type { GatewayRequestHandler } from "../gateway/server-methods/types.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; @@ -132,8 +132,7 @@ export function loadExtensionHostPluginRegistry( diagnostics: discovery.diagnostics, }); pushExtensionHostDiagnostics(registry.diagnostics, manifestRegistry.diagnostics); - warnWhenExtensionAllowlistIsOpen({ - logger, + const discoveryPolicy = resolveExtensionHostDiscoveryPolicy({ pluginsEnabled: normalized.enabled, allow: normalized.allow, warningCacheKey: cacheKey, @@ -144,6 +143,9 @@ export function loadExtensionHostPluginRegistry( origin: plugin.origin, })), }); + for (const warning of discoveryPolicy.warningMessages) { + logger.warn(warning); + } const provenance = buildExtensionHostProvenanceIndex({ config: cfg, normalizedLoadPaths: normalized.loadPaths, diff --git a/src/extension-host/loader-policy.test.ts b/src/extension-host/loader-policy.test.ts index e508ef7c725..e894d8cd559 100644 --- a/src/extension-host/loader-policy.test.ts +++ b/src/extension-host/loader-policy.test.ts @@ -3,13 +3,10 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import type { PluginCandidate } from "../plugins/discovery.js"; -import { createEmptyPluginRegistry } from "../plugins/registry.js"; import { buildExtensionHostProvenanceIndex, compareExtensionHostDuplicateCandidateOrder, createExtensionHostPluginRecord, - warnAboutUntrackedLoadedExtensions, - warnWhenExtensionAllowlistIsOpen, } from "./loader-policy.js"; const tempDirs: string[] = []; @@ -93,86 +90,4 @@ describe("extension host loader policy", () => { }), ).toBeLessThan(0); }); - - it("warns when allowlist is open for non-bundled discoverable plugins", () => { - const warnings: string[] = []; - const warningCache = new Set(); - - warnWhenExtensionAllowlistIsOpen({ - logger: { - info: () => {}, - warn: (message) => warnings.push(message), - error: () => {}, - }, - pluginsEnabled: true, - allow: [], - warningCacheKey: "warn-key", - warningCache, - discoverablePlugins: [ - { id: "bundled", source: "/bundled/index.js", origin: "bundled" }, - { id: "workspace-demo", source: "/workspace/demo.js", origin: "workspace" }, - ], - }); - - expect(warnings).toHaveLength(1); - expect(warnings[0]).toContain("plugins.allow is empty"); - expect(warningCache.has("warn-key")).toBe(true); - }); - - it("warns about loaded untracked non-bundled plugins", () => { - const trackedDir = makeTempDir(); - const untrackedDir = makeTempDir(); - const trackedFile = path.join(trackedDir, "tracked.js"); - const untrackedFile = path.join(untrackedDir, "untracked.js"); - fs.writeFileSync(trackedFile, "export {};\n", "utf8"); - fs.writeFileSync(untrackedFile, "export {};\n", "utf8"); - - const registry = createEmptyPluginRegistry(); - registry.plugins.push( - { - ...createExtensionHostPluginRecord({ - id: "tracked", - source: trackedFile, - origin: "workspace", - enabled: true, - configSchema: false, - }), - status: "loaded", - }, - { - ...createExtensionHostPluginRecord({ - id: "untracked", - source: untrackedFile, - origin: "workspace", - enabled: true, - configSchema: false, - }), - status: "loaded", - }, - ); - - const warnings: string[] = []; - const env = { ...process.env, HOME: makeTempDir() }; - const provenance = buildExtensionHostProvenanceIndex({ - config: {}, - normalizedLoadPaths: [trackedDir], - env, - }); - - warnAboutUntrackedLoadedExtensions({ - registry, - provenance, - logger: { - info: () => {}, - warn: (message) => warnings.push(message), - error: () => {}, - }, - env, - }); - - expect(registry.diagnostics).toHaveLength(1); - expect(registry.diagnostics[0]?.pluginId).toBe("untracked"); - expect(warnings).toHaveLength(1); - expect(warnings[0]).toContain("untracked"); - }); }); diff --git a/src/extension-host/loader-policy.ts b/src/extension-host/loader-policy.ts index 495bb0d55c9..1fcd957a77a 100644 --- a/src/extension-host/loader-policy.ts +++ b/src/extension-host/loader-policy.ts @@ -1,5 +1,3 @@ -import fs from "node:fs"; -import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; import type { PluginCandidate } from "../plugins/discovery.js"; import { isPathInside, safeStatSync } from "../plugins/path-safety.js"; @@ -11,14 +9,6 @@ import { setExtensionHostPluginRecordLifecycleState, } from "./loader-state.js"; -function safeRealpathOrResolve(value: string): string { - try { - return fs.realpathSync(value); - } catch { - return path.resolve(value); - } -} - type PathMatcher = { exact: Set; dirs: string[]; @@ -184,25 +174,6 @@ export function buildExtensionHostProvenanceIndex(params: { return { loadPathMatcher, installRules }; } -function isTrackedByProvenance(params: { - pluginId: string; - source: string; - index: ExtensionHostProvenanceIndex; - env: NodeJS.ProcessEnv; -}): boolean { - const sourcePath = resolveUserPath(params.source, params.env); - const installRule = params.index.installRules.get(params.pluginId); - if (installRule) { - if (installRule.trackedWithoutPaths) { - return true; - } - if (matchesPathMatcher(installRule.matcher, sourcePath)) { - return true; - } - } - return matchesPathMatcher(params.index.loadPathMatcher, sourcePath); -} - function matchesExplicitInstallRule(params: { pluginId: string; source: string; @@ -274,63 +245,3 @@ export function compareExtensionHostDuplicateCandidateOrder(params: { }) ); } - -export function warnWhenExtensionAllowlistIsOpen(params: { - logger: PluginLogger; - pluginsEnabled: boolean; - allow: string[]; - warningCacheKey: string; - warningCache: Set; - discoverablePlugins: Array<{ id: string; source: string; origin: PluginRecord["origin"] }>; -}): void { - if (!params.pluginsEnabled || params.allow.length > 0) { - return; - } - const nonBundled = params.discoverablePlugins.filter((entry) => entry.origin !== "bundled"); - if (nonBundled.length === 0 || params.warningCache.has(params.warningCacheKey)) { - return; - } - const preview = nonBundled - .slice(0, 6) - .map((entry) => `${entry.id} (${entry.source})`) - .join(", "); - const extra = nonBundled.length > 6 ? ` (+${nonBundled.length - 6} more)` : ""; - params.warningCache.add(params.warningCacheKey); - params.logger.warn( - `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, - ); -} - -export function warnAboutUntrackedLoadedExtensions(params: { - registry: PluginRegistry; - provenance: ExtensionHostProvenanceIndex; - logger: PluginLogger; - env: NodeJS.ProcessEnv; -}): void { - for (const plugin of params.registry.plugins) { - if (plugin.status !== "loaded" || plugin.origin === "bundled") { - continue; - } - if ( - isTrackedByProvenance({ - pluginId: plugin.id, - source: plugin.source, - index: params.provenance, - env: params.env, - }) - ) { - continue; - } - const message = - "loaded without install/load-path provenance; treat as untracked local code and pin trust via plugins.allow or install records"; - params.registry.diagnostics.push({ - level: "warn", - pluginId: plugin.id, - source: plugin.source, - message, - }); - params.logger.warn( - `[plugins] ${plugin.id}: ${message} (${safeRealpathOrResolve(plugin.source)})`, - ); - } -}