diff --git a/CHANGELOG.md b/CHANGELOG.md index acf0e908331..aaf074a12c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage. - Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift. +- Security/Plugins: harden plugin discovery by blocking unsafe candidates (root escapes, world-writable paths, suspicious ownership), add startup warnings when `plugins.allow` is empty with discoverable non-bundled plugins, and warn on loaded plugins without install/load-path provenance. - Security/Gateway: rate-limit control-plane write RPCs (`config.apply`, `config.patch`, `update.run`) to 3 requests per minute per `deviceId+clientIp`, add restart single-flight coalescing plus a 30-second restart cooldown, and log actor/device/ip with changed-path audit details for config/update-triggered restarts. - Commands/Doctor: skip embedding-provider warnings when `memory.backend` is `qmd`, because QMD manages embeddings internally and does not require `memorySearch` providers. (#17263) Thanks @miloudbelarebia. - Security/Webhooks: harden Feishu and Zalo webhook ingress with webhook-mode token preconditions, loopback-default Feishu bind host, JSON content-type enforcement, per-path rate limiting, replay dedupe for Zalo events, constant-time Zalo secret comparison, and anomaly status counters. diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index ab031c389b7..9433e9ea387 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -116,6 +116,15 @@ Bundled plugins must be enabled explicitly via `plugins.entries..enabled` or `openclaw plugins enable `. Installed plugins are enabled by default, but can be disabled the same way. +Hardening notes: + +- If `plugins.allow` is empty and non-bundled plugins are discoverable, OpenClaw logs a startup warning with plugin ids and sources. +- Candidate paths are safety-checked before discovery admission. OpenClaw blocks candidates when: + - extension entry resolves outside plugin root (including symlink/path traversal escapes), + - plugin root/source path is world-writable, + - path ownership is suspicious for non-bundled plugins (POSIX owner is neither current uid nor root). +- Loaded non-bundled plugins without install/load-path provenance emit a warning so you can pin trust (`plugins.allow`) or install tracking (`plugins.installs`). + Each plugin must include a `openclaw.plugin.json` file in its root. If a path points at a file, the plugin root is the file's directory and must contain the manifest. diff --git a/src/plugins/discovery.test.ts b/src/plugins/discovery.test.ts index 3d19b02b17a..1a81a46024d 100644 --- a/src/plugins/discovery.test.ts +++ b/src/plugins/discovery.test.ts @@ -149,4 +149,78 @@ describe("discoverOpenClawPlugins", () => { const ids = candidates.map((c) => c.idHint); expect(ids).toContain("demo-plugin-dir"); }); + + it("blocks extension entries that escape plugin root", async () => { + const stateDir = makeTempDir(); + const globalExt = path.join(stateDir, "extensions", "escape-pack"); + const outside = path.join(stateDir, "outside.js"); + fs.mkdirSync(globalExt, { recursive: true }); + + fs.writeFileSync( + path.join(globalExt, "package.json"), + JSON.stringify({ + name: "@openclaw/escape-pack", + openclaw: { extensions: ["../../outside.js"] }, + }), + "utf-8", + ); + fs.writeFileSync(outside, "export default function () {}", "utf-8"); + + const result = await withStateDir(stateDir, async () => { + return discoverOpenClawPlugins({}); + }); + + expect(result.candidates).toHaveLength(0); + expect( + result.diagnostics.some((diag) => diag.message.includes("source escapes plugin root")), + ).toBe(true); + }); + + it.runIf(process.platform !== "win32")("blocks world-writable plugin paths", async () => { + const stateDir = makeTempDir(); + const globalExt = path.join(stateDir, "extensions"); + fs.mkdirSync(globalExt, { recursive: true }); + const pluginPath = path.join(globalExt, "world-open.ts"); + fs.writeFileSync(pluginPath, "export default function () {}", "utf-8"); + fs.chmodSync(pluginPath, 0o777); + + const result = await withStateDir(stateDir, async () => { + return discoverOpenClawPlugins({}); + }); + + expect(result.candidates).toHaveLength(0); + expect(result.diagnostics.some((diag) => diag.message.includes("world-writable path"))).toBe( + true, + ); + }); + + it.runIf(process.platform !== "win32" && typeof process.getuid === "function")( + "blocks suspicious ownership when uid mismatch is detected", + async () => { + const stateDir = makeTempDir(); + const globalExt = path.join(stateDir, "extensions"); + fs.mkdirSync(globalExt, { recursive: true }); + fs.writeFileSync( + path.join(globalExt, "owner-mismatch.ts"), + "export default function () {}", + "utf-8", + ); + + const proc = process as NodeJS.Process & { getuid: () => number }; + const originalGetUid = proc.getuid; + const actualUid = originalGetUid(); + try { + proc.getuid = () => actualUid + 1; + const result = await withStateDir(stateDir, async () => { + return discoverOpenClawPlugins({}); + }); + expect(result.candidates).toHaveLength(0); + expect( + result.diagnostics.some((diag) => diag.message.includes("suspicious ownership")), + ).toBe(true); + } finally { + proc.getuid = originalGetUid; + } + }, + ); }); diff --git a/src/plugins/discovery.ts b/src/plugins/discovery.ts index 02b10ade64e..3625ab9a1c8 100644 --- a/src/plugins/discovery.ts +++ b/src/plugins/discovery.ts @@ -29,6 +29,111 @@ export type PluginDiscoveryResult = { diagnostics: PluginDiagnostic[]; }; +function isPathInside(baseDir: string, targetPath: string): boolean { + const rel = path.relative(baseDir, targetPath); + if (!rel) { + return true; + } + return !rel.startsWith("..") && !path.isAbsolute(rel); +} + +function safeRealpathSync(targetPath: string): string | null { + try { + return fs.realpathSync(targetPath); + } catch { + return null; + } +} + +function safeStatSync(targetPath: string): fs.Stats | null { + try { + return fs.statSync(targetPath); + } catch { + return null; + } +} + +function formatMode(mode: number): string { + return (mode & 0o777).toString(8).padStart(3, "0"); +} + +function currentUid(): number | null { + if (process.platform === "win32") { + return null; + } + if (typeof process.getuid !== "function") { + return null; + } + return process.getuid(); +} + +function isUnsafePluginCandidate(params: { + source: string; + rootDir: string; + origin: PluginOrigin; + diagnostics: PluginDiagnostic[]; +}): boolean { + const sourceReal = safeRealpathSync(params.source); + const rootReal = safeRealpathSync(params.rootDir); + if (sourceReal && rootReal && !isPathInside(rootReal, sourceReal)) { + params.diagnostics.push({ + level: "warn", + source: params.source, + message: `blocked plugin candidate: source escapes plugin root (${params.source} -> ${sourceReal}; root=${rootReal})`, + }); + return true; + } + + if (process.platform === "win32") { + return false; + } + + const uid = currentUid(); + const pathsToCheck = [params.rootDir, params.source]; + const seen = new Set(); + for (const targetPath of pathsToCheck) { + const normalized = path.resolve(targetPath); + if (seen.has(normalized)) { + continue; + } + seen.add(normalized); + const stat = safeStatSync(targetPath); + if (!stat) { + params.diagnostics.push({ + level: "warn", + source: targetPath, + message: `blocked plugin candidate: cannot stat path (${targetPath})`, + }); + return true; + } + const modeBits = stat.mode & 0o777; + if ((modeBits & 0o002) !== 0) { + params.diagnostics.push({ + level: "warn", + source: targetPath, + message: `blocked plugin candidate: world-writable path (${targetPath}, mode=${formatMode(modeBits)})`, + }); + return true; + } + if ( + params.origin !== "bundled" && + uid !== null && + typeof stat.uid === "number" && + stat.uid !== uid && + stat.uid !== 0 + ) { + params.diagnostics.push({ + level: "warn", + source: targetPath, + message: `blocked plugin candidate: suspicious ownership (${targetPath}, uid=${stat.uid}, expected uid=${uid} or root)`, + }); + return true; + } + } + + return false; +} + function isExtensionFile(filePath: string): boolean { const ext = path.extname(filePath); if (!EXTENSION_EXTS.has(ext)) { @@ -83,6 +188,7 @@ function deriveIdHint(params: { function addCandidate(params: { candidates: PluginCandidate[]; + diagnostics: PluginDiagnostic[]; seen: Set; idHint: string; source: string; @@ -96,12 +202,23 @@ function addCandidate(params: { if (params.seen.has(resolved)) { return; } + const resolvedRoot = path.resolve(params.rootDir); + if ( + isUnsafePluginCandidate({ + source: resolved, + rootDir: resolvedRoot, + origin: params.origin, + diagnostics: params.diagnostics, + }) + ) { + return; + } params.seen.add(resolved); const manifest = params.manifest ?? null; params.candidates.push({ idHint: params.idHint, source: resolved, - rootDir: path.resolve(params.rootDir), + rootDir: resolvedRoot, origin: params.origin, workspaceDir: params.workspaceDir, packageName: manifest?.name?.trim() || undefined, @@ -143,6 +260,7 @@ function discoverInDirectory(params: { } addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: path.basename(entry.name, path.extname(entry.name)), source: fullPath, @@ -163,6 +281,7 @@ function discoverInDirectory(params: { const resolved = path.resolve(fullPath, extPath); addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: deriveIdHint({ filePath: resolved, @@ -187,6 +306,7 @@ function discoverInDirectory(params: { if (indexFile && isExtensionFile(indexFile)) { addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: entry.name, source: indexFile, @@ -230,6 +350,7 @@ function discoverFromPath(params: { } addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: path.basename(resolved, path.extname(resolved)), source: resolved, @@ -249,6 +370,7 @@ function discoverFromPath(params: { const source = path.resolve(resolved, extPath); addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: deriveIdHint({ filePath: source, @@ -274,6 +396,7 @@ function discoverFromPath(params: { if (indexFile && isExtensionFile(indexFile)) { addCandidate({ candidates: params.candidates, + diagnostics: params.diagnostics, seen: params.seen, idHint: path.basename(resolved), source: indexFile, diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 7db185c8e87..0af06b0df9e 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -489,4 +489,76 @@ describe("loadOpenClawPlugins", () => { expect(loaded?.origin).toBe("config"); expect(overridden?.origin).toBe("bundled"); }); + + it("warns when plugins.allow is empty and non-bundled plugins are discoverable", () => { + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; + const plugin = writePlugin({ + id: "warn-open-allow", + body: `export default { id: "warn-open-allow", register() {} };`, + }); + const warnings: string[] = []; + loadOpenClawPlugins({ + cache: false, + logger: { + info: () => {}, + warn: (msg) => warnings.push(msg), + error: () => {}, + }, + config: { + plugins: { + load: { paths: [plugin.file] }, + }, + }, + }); + expect( + warnings.some((msg) => msg.includes("plugins.allow is empty") && msg.includes(plugin.id)), + ).toBe(true); + }); + + it("warns when loaded non-bundled plugin has no install/load-path provenance", () => { + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; + const prevStateDir = process.env.OPENCLAW_STATE_DIR; + const stateDir = makeTempDir(); + process.env.OPENCLAW_STATE_DIR = stateDir; + try { + const globalDir = path.join(stateDir, "extensions", "rogue"); + fs.mkdirSync(globalDir, { recursive: true }); + writePlugin({ + id: "rogue", + body: `export default { id: "rogue", register() {} };`, + dir: globalDir, + filename: "index.js", + }); + + const warnings: string[] = []; + const registry = loadOpenClawPlugins({ + cache: false, + logger: { + info: () => {}, + warn: (msg) => warnings.push(msg), + error: () => {}, + }, + config: { + plugins: { + allow: ["rogue"], + }, + }, + }); + + const rogue = registry.plugins.find((entry) => entry.id === "rogue"); + expect(rogue?.status).toBe("loaded"); + expect( + warnings.some( + (msg) => + msg.includes("rogue") && msg.includes("loaded without install/load-path provenance"), + ), + ).toBe(true); + } finally { + if (prevStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = prevStateDir; + } + } + }); }); diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 3060b3daab8..ab688865f08 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -177,6 +177,128 @@ function pushDiagnostics(diagnostics: PluginDiagnostic[], append: PluginDiagnost diagnostics.push(...append); } +function isPathInside(baseDir: string, targetPath: string): boolean { + const rel = path.relative(baseDir, targetPath); + if (!rel) { + return true; + } + return !rel.startsWith("..") && !path.isAbsolute(rel); +} + +function pathMatchesBaseOrFile(params: { baseOrFile: string; targetFile: string }): boolean { + const baseResolved = resolveUserPath(params.baseOrFile); + const targetResolved = resolveUserPath(params.targetFile); + if (baseResolved === targetResolved) { + return true; + } + try { + const stat = fs.statSync(baseResolved); + if (!stat.isDirectory()) { + return false; + } + } catch { + return false; + } + return isPathInside(baseResolved, targetResolved); +} + +function isTrackedByInstallRecord(params: { + pluginId: string; + source: string; + config: OpenClawConfig; +}): boolean { + const install = params.config.plugins?.installs?.[params.pluginId]; + if (!install) { + return false; + } + const trackedPaths = [install.installPath, install.sourcePath] + .map((entry) => (typeof entry === "string" ? entry.trim() : "")) + .filter(Boolean); + if (trackedPaths.length === 0) { + return true; + } + return trackedPaths.some((trackedPath) => + pathMatchesBaseOrFile({ + baseOrFile: trackedPath, + targetFile: params.source, + }), + ); +} + +function isTrackedByLoadPath(params: { source: string; loadPaths: string[] }): boolean { + return params.loadPaths.some((loadPath) => + pathMatchesBaseOrFile({ + baseOrFile: loadPath, + targetFile: params.source, + }), + ); +} + +function warnWhenAllowlistIsOpen(params: { + logger: PluginLogger; + pluginsEnabled: boolean; + allow: string[]; + discoverablePlugins: Array<{ id: string; source: string; origin: PluginRecord["origin"] }>; +}) { + if (!params.pluginsEnabled) { + return; + } + if (params.allow.length > 0) { + return; + } + const nonBundled = params.discoverablePlugins.filter((entry) => entry.origin !== "bundled"); + if (nonBundled.length === 0) { + return; + } + const preview = nonBundled + .slice(0, 6) + .map((entry) => `${entry.id} (${entry.source})`) + .join(", "); + const extra = nonBundled.length > 6 ? ` (+${nonBundled.length - 6} more)` : ""; + params.logger.warn( + `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, + ); +} + +function warnAboutUntrackedLoadedPlugins(params: { + registry: PluginRegistry; + config: OpenClawConfig; + normalizedLoadPaths: string[]; + logger: PluginLogger; +}) { + for (const plugin of params.registry.plugins) { + if (plugin.status !== "loaded" || plugin.origin === "bundled") { + continue; + } + if ( + isTrackedByInstallRecord({ + pluginId: plugin.id, + source: plugin.source, + config: params.config, + }) + ) { + continue; + } + if ( + isTrackedByLoadPath({ + source: plugin.source, + loadPaths: params.normalizedLoadPaths, + }) + ) { + 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} (${plugin.source})`); + } +} + export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegistry { // Test env: default-disable plugins unless explicitly configured. // This keeps unit/gateway suites fast and avoids loading heavyweight plugin deps by accident. @@ -219,6 +341,16 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi diagnostics: discovery.diagnostics, }); pushDiagnostics(registry.diagnostics, manifestRegistry.diagnostics); + warnWhenAllowlistIsOpen({ + logger, + pluginsEnabled: normalized.enabled, + allow: normalized.allow, + discoverablePlugins: manifestRegistry.plugins.map((plugin) => ({ + id: plugin.id, + source: plugin.source, + origin: plugin.origin, + })), + }); // Lazy: avoid creating the Jiti loader when all plugins are disabled (common in unit tests). let jitiLoader: ReturnType | null = null; @@ -471,6 +603,13 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi }); } + warnAboutUntrackedLoadedPlugins({ + registry, + config: cfg, + normalizedLoadPaths: normalized.loadPaths, + logger, + }); + if (cacheEnabled) { registryCache.set(cacheKey, registry); }