From 8a2207f8a179db57c31d3d5d8b43853bdcc50c92 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 2 May 2026 13:12:28 +0100 Subject: [PATCH] fix(security): ignore plugin install debris in audits --- CHANGELOG.md | 1 + src/security/audit-extra.async.test.ts | 48 ++++++++++++++++++++++++ src/security/audit-extra.async.ts | 2 + src/security/audit-plugins-trust.test.ts | 31 +++++++++++++++ src/security/audit-plugins-trust.ts | 2 + src/security/installed-plugin-dirs.ts | 26 +++++++++++++ 6 files changed, 110 insertions(+) create mode 100644 src/security/installed-plugin-dirs.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d3a822783d3..6cf16d159b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Telegram/native commands: pass persisted session files into plugin commands for topic-bound sessions, so `/codex bind` works from Telegram forum topics. Refs #75845 and #76049. Thanks @MatthewSchleder. +- Security audit/plugins: ignore plugin install backup, disabled, and dependency debris directories when enumerating installed plugin roots, avoiding false-positive findings for `.openclaw-install-backups` after plugin updates. Fixes #75456. - Telegram: honor runtime conversation bindings for native slash commands in bound top-level groups, so commands like `/status@bot` route to the active non-`main` session instead of falling back to the default route. Fixes #75405; supersedes #75558. Thanks @ziptbm and @yfge. - Gateway/tasks: make task registry maintenance use pass-local backing-session lookups and fresh active child-session indexes, avoiding repeated full task snapshots and session-store clones on large stale registries. Fixes #73517 and #75708; supersedes #74406 and #75709. Thanks @Lightningxxl, @glfruit, and @jared-rebel. - Models CLI: restore `openclaw models list --provider ` catalog and registry fallback rows for unconfigured providers, so provider-specific verification commands no longer report "No models found." Fixes #75517; supersedes #75615. Thanks @lotsoftick and @koshaji. diff --git a/src/security/audit-extra.async.test.ts b/src/security/audit-extra.async.test.ts index 83ae690a0eb..689feedae6d 100644 --- a/src/security/audit-extra.async.test.ts +++ b/src/security/audit-extra.async.test.ts @@ -153,6 +153,54 @@ description: test skill expect(findings.some((f) => f.checkId === "plugins.code_safety.entry_escape")).toBe(true); }); + it("ignores install backup and debris dirs when scanning installed plugin roots", async () => { + const scanSpy = vi + .spyOn(skillScanner, "scanDirectoryWithSummary") + .mockImplementation(async (dirPath) => ({ + scannedFiles: 1, + critical: dirPath.includes(`${path.sep}demo`) ? 1 : 0, + warn: 0, + info: 0, + findings: dirPath.includes(`${path.sep}demo`) + ? [ + { + ruleId: "dangerous-exec", + severity: "critical", + file: path.join(dirPath, "index.js"), + line: 1, + message: "dangerous exec", + evidence: "exec(...)", + }, + ] + : [], + })); + + try { + const tmpDir = await makeTmpDir("audit-scanner-install-debris"); + for (const name of [ + "demo", + ".openclaw-install-backups", + "node_modules", + "old-plugin.backup-20260502", + "old-plugin.disabled.20260502", + "old-plugin.bak", + ]) { + const pluginDir = path.join(tmpDir, "extensions", name); + await fs.mkdir(pluginDir, { recursive: true }); + await fs.writeFile(path.join(pluginDir, "index.js"), "eval('1+1');"); + } + + const findings = await collectPluginsCodeSafetyFindings({ stateDir: tmpDir }); + + expect(scanSpy.mock.calls.map(([dirPath]) => path.basename(dirPath))).toEqual(["demo"]); + const codeSafetyFinding = findings.find((f) => f.checkId === "plugins.code_safety"); + expect(codeSafetyFinding?.title).toContain('Plugin "demo"'); + expect(findings.map((f) => f.title).join("\n")).not.toContain(".openclaw-install-backups"); + } finally { + scanSpy.mockRestore(); + } + }); + it("surfaces manifest_parse_error finding when plugin package.json is malformed JSON", async () => { const tmpDir = await makeTmpDir("audit-manifest-parse-error"); const pluginDir = path.join(tmpDir, "extensions", "broken-plugin"); diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts index 64818306748..b47ee845669 100644 --- a/src/security/audit-extra.async.ts +++ b/src/security/audit-extra.async.ts @@ -15,6 +15,7 @@ import { normalizeOptionalLowercaseString, normalizeOptionalString, } from "../shared/string-coerce.js"; +import { shouldIgnoreInstalledPluginDirName } from "./installed-plugin-dirs.js"; import { extensionUsesSkippedScannerPath, isPathInside } from "./scan-paths.js"; import type { SkillScanFinding } from "./skill-scanner.js"; import type { ExecFn } from "./windows-acl.js"; @@ -219,6 +220,7 @@ async function listInstalledPluginDirs(params: { const pluginDirs = entries .filter((entry) => entry.isDirectory()) .map((entry) => entry.name) + .filter((name) => !shouldIgnoreInstalledPluginDirName(name)) .filter(Boolean); return { extensionsDir, pluginDirs }; } diff --git a/src/security/audit-plugins-trust.test.ts b/src/security/audit-plugins-trust.test.ts index 217114204f2..0f9f38e36ff 100644 --- a/src/security/audit-plugins-trust.test.ts +++ b/src/security/audit-plugins-trust.test.ts @@ -360,6 +360,37 @@ describe("security audit install metadata findings", () => { expect(phantomFinding?.detail).not.toContain("installed-plugin"); }); + it("ignores install backup and debris dirs when auditing installed plugin roots", async () => { + const stateDir = await makeTmpDir("installed-plugin-debris"); + for (const name of [ + "live-plugin", + ".openclaw-install-backups", + "node_modules", + "old-plugin.backup-20260502", + "old-plugin.disabled.20260502", + "old-plugin.bak", + ]) { + await fs.mkdir(path.join(stateDir, "extensions", name), { + recursive: true, + }); + } + + const findings = await runInstallMetadataAudit({}, stateDir); + + const noAllowlist = findings.find( + (finding) => finding.checkId === "plugins.extensions_no_allowlist", + ); + expect(noAllowlist?.detail).toContain("Found 1 extension(s)"); + + const toolsReachable = findings.find( + (finding) => finding.checkId === "plugins.tools_reachable_permissive_policy", + ); + expect(toolsReachable?.detail).toContain("Enabled extension plugins: live-plugin."); + expect(findings.map((finding) => finding.detail).join("\n")).not.toContain( + ".openclaw-install-backups", + ); + }); + it("does not report bundled provider and utility plugins as phantom allowlist entries", async () => { const stateDir = await makeTmpDir("phantom-bundled-providers"); await fs.mkdir(path.join(stateDir, "extensions", "installed-plugin"), { diff --git a/src/security/audit-plugins-trust.ts b/src/security/audit-plugins-trust.ts index c52d88aa175..fd54abe5188 100644 --- a/src/security/audit-plugins-trust.ts +++ b/src/security/audit-plugins-trust.ts @@ -15,6 +15,7 @@ import { } from "../plugins/plugin-registry.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; import type { SecurityAuditFinding } from "./audit.types.js"; +import { shouldIgnoreInstalledPluginDirName } from "./installed-plugin-dirs.js"; type SandboxToolPolicy = import("../agents/sandbox/types.js").SandboxToolPolicy; @@ -143,6 +144,7 @@ async function listInstalledPluginDirs(params: { const pluginDirs = entries .filter((entry) => entry.isDirectory()) .map((entry) => entry.name) + .filter((name) => !shouldIgnoreInstalledPluginDirName(name)) .filter(Boolean); return { extensionsDir, pluginDirs }; } diff --git a/src/security/installed-plugin-dirs.ts b/src/security/installed-plugin-dirs.ts new file mode 100644 index 00000000000..e80aabc14a4 --- /dev/null +++ b/src/security/installed-plugin-dirs.ts @@ -0,0 +1,26 @@ +import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; + +const IGNORED_INSTALLED_PLUGIN_DIR_NAMES = new Set(["node_modules", ".openclaw-install-backups"]); + +export function shouldIgnoreInstalledPluginDirName(name: string): boolean { + const normalized = normalizeOptionalLowercaseString(name); + if (!normalized) { + return true; + } + if (IGNORED_INSTALLED_PLUGIN_DIR_NAMES.has(normalized)) { + return true; + } + if (normalized.startsWith(".")) { + return true; + } + if (normalized.endsWith(".bak")) { + return true; + } + if (normalized.includes(".backup-")) { + return true; + } + if (normalized.includes(".disabled")) { + return true; + } + return false; +}