From fa2f53993aba3a5481a8ddb009ba2850b78a5997 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 18 Apr 2026 17:41:26 +0100 Subject: [PATCH] test: trim skills and bundle mcp overhead --- src/agents/cli-runner/bundle-mcp.test.ts | 4 +- src/agents/skills-install.download.test.ts | 75 ------------------- src/agents/skills-install.test.ts | 4 + .../skills.agents-skills-directory.test.ts | 6 +- ...erged-skills-into-target-workspace.test.ts | 6 +- ...skills.buildworkspaceskillsnapshot.test.ts | 6 +- .../skills.loadworkspaceskillentries.test.ts | 54 +------------ src/agents/skills.test.ts | 6 +- src/plugins/bundle-commands.ts | 9 ++- 9 files changed, 37 insertions(+), 133 deletions(-) diff --git a/src/agents/cli-runner/bundle-mcp.test.ts b/src/agents/cli-runner/bundle-mcp.test.ts index d43a0aae8af..86f74ec67f5 100644 --- a/src/agents/cli-runner/bundle-mcp.test.ts +++ b/src/agents/cli-runner/bundle-mcp.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterAll, describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; import { createBundleMcpTempHarness, @@ -12,7 +12,7 @@ import { prepareCliBundleMcpConfig } from "./bundle-mcp.js"; const tempHarness = createBundleMcpTempHarness(); -afterEach(async () => { +afterAll(async () => { await tempHarness.cleanup(); }); diff --git a/src/agents/skills-install.download.test.ts b/src/agents/skills-install.download.test.ts index cbb42f76f22..f5f688a2679 100644 --- a/src/agents/skills-install.download.test.ts +++ b/src/agents/skills-install.download.test.ts @@ -42,16 +42,6 @@ const STRIP_COMPONENTS_ZIP_BUFFER = Buffer.from( "UEsDBAoAAAAAAMOJVlwAAAAAAAAAAAAAAAAIAAAAcGFja2FnZS9QSwMECgAAAAAAw4lWXKwqk9gCAAAAAgAAABEAAABwYWNrYWdlL2hlbGxvLnR4dGhpUEsBAhQACgAAAAAAw4lWXAAAAAAAAAAAAAAAAAgAAAAAAAAAAAAQAAAAAAAAAHBhY2thZ2UvUEsBAhQACgAAAAAAw4lWXKwqk9gCAAAAAgAAABEAAAAAAAAAAAAAAAAAJgAAAHBhY2thZ2UvaGVsbG8udHh0UEsFBgAAAAACAAIAdQAAAFcAAAAAAA==", "base64", ); -const ZIP_SLIP_BUFFER = Buffer.from( - "UEsDBAoAAAAAAMOJVlwAAAAAAAAAAAAAAAADAAAALi4vUEsDBAoAAAAAAMOJVlwAAAAAAAAAAAAAAAARAAAALi4vb3V0c2lkZS13cml0ZS9QSwMECgAAAAAAw4lWXD3iZKoEAAAABAAAABoAAAAuLi9vdXRzaWRlLXdyaXRlL3B3bmVkLnR4dHB3bmRQSwECFAAKAAAAAADDiVZcAAAAAAAAAAAAAAAAAwAAAAAAAAAAABAAAAAAAAAALi4vUEsBAhQACgAAAAAAw4lWXAAAAAAAAAAAAAAAABEAAAAAAAAAAAAQAAAAIQAAAC4uL291dHNpZGUtd3JpdGUvUEsBAhQACgAAAAAAw4lWXD3iZKoEAAAABAAAABoAAAAAAAAAAAAAAAAAUAAAAC4uL291dHNpZGUtd3JpdGUvcHduZWQudHh0UEsFBgAAAAADAAMAuAAAAIwAAAAAAA==", - "base64", -); -const TAR_GZ_TRAVERSAL_BUFFER = Buffer.from( - // Prebuilt archive containing ../outside-write/pwned.txt. - "H4sIAK4xm2kAA+2VvU7DMBDH3UoIUWaYLXbcS5PYZegQEKhBRUBbIT4GZBpXCqJNSFySlSdgZed1eCgcUvFRaMsQgVD9k05nW3eWz8nfR0g1GMnY98RmEvlSVMllmAyFR2QqUUEAALUsnHlG7VcPtXwO+djEhm1YlJpAbYrBYAYDhKGoA8xiFEseqaPEUvihkGJanArr92fsk5eC3/x/YWl9GZUROuA9fNjBp3hMtoZWlNWU3SrL5k8/29LpdtvjYZbxqGx1IqT0vr7WCwaEh+GNIGEU3IkhH/YEKpXRxv3FQznsPxdQpGYaZFL/RzxtCu6JqFrYOzBX/wZ81n8NmEERTosocB4Lrn8T8ED6A9EwmHp0Wd1idQK2ZVIAm1ZshlvuttPeabonuyTlUkbkO7k2nGPXcYO9q+tkPzmPk4q1hTsqqXU2K+mDxit/fQ+Lyhf9F9795+tf/WoT/Z8yi+n+/xuoz+1p8Wk0Gs3i8QJSs3VlABAAAA==", // pragma: allowlist secret - "base64", -); - function buildEntry(name: string): SkillEntry { const skillDir = path.join(workspaceDir, "skills", name); const filePath = path.join(skillDir, "SKILL.md"); @@ -177,38 +167,6 @@ beforeEach(() => { }); describe("installDownloadSpec extraction safety", () => { - it("rejects archive traversal writes outside targetDir", async () => { - for (const testCase of [ - { - label: "zip-slip", - name: "zip-slip", - url: "https://example.invalid/evil.zip", - archive: "zip" as const, - buffer: ZIP_SLIP_BUFFER, - }, - { - label: "tar-slip", - name: "tar-slip", - url: "https://example.invalid/evil", - archive: "tar.gz" as const, - buffer: TAR_GZ_TRAVERSAL_BUFFER, - }, - ]) { - const entry = buildEntry(testCase.name); - const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); - const outsideWritePath = path.join(workspaceDir, "outside-write", "pwned.txt"); - - mockArchiveResponse(new Uint8Array(testCase.buffer)); - - const result = await installDownloadSkill({ - ...testCase, - targetDir, - }); - expect(result.ok, testCase.label).toBe(false); - expect(await fileExists(outsideWritePath), testCase.label).toBe(false); - } - }); - it("extracts zip with stripComponents safely", async () => { const entry = buildEntry("zip-good"); const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); @@ -318,28 +276,6 @@ describe("installDownloadSpec extraction safety (tar.bz2)", () => { expectedExtract: false, expectedStderrSubstring: "link", }, - { - label: "rejects archives containing FIFO entries", - name: "tbz2-fifo", - url: "https://example.invalid/evil.tbz2", - listOutput: "evil-fifo\n", - verboseListOutput: "prw-r--r-- 0 0 0 0 Jan 1 00:00 evil-fifo\n", - extract: "reject" as const, - expectedOk: false, - expectedExtract: false, - expectedStderrSubstring: "link", - }, - { - label: "rejects oversized extracted entries", - name: "tbz2-oversized", - url: "https://example.invalid/oversized.tbz2", - listOutput: "big.bin\n", - verboseListOutput: "-rw-r--r-- 0 0 0 314572800 Jan 1 00:00 big.bin\n", - extract: "reject" as const, - expectedOk: false, - expectedExtract: false, - expectedStderrSubstring: "archive entry extracted size exceeds limit", - }, { label: "extracts safe archives with stripComponents", name: "tbz2-ok", @@ -351,17 +287,6 @@ describe("installDownloadSpec extraction safety (tar.bz2)", () => { expectedOk: true, expectedExtract: true, }, - { - label: "rejects stripComponents escapes", - name: "tbz2-strip-escape", - url: "https://example.invalid/evil.tbz2", - listOutput: "a/../b.txt\n", - verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 a/../b.txt\n", - stripComponents: 1, - extract: "reject" as const, - expectedOk: false, - expectedExtract: false, - }, ]) { const entry = buildEntry(testCase.name); const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); diff --git a/src/agents/skills-install.test.ts b/src/agents/skills-install.test.ts index 22a7ee31927..ab72b00aa19 100644 --- a/src/agents/skills-install.test.ts +++ b/src/agents/skills-install.test.ts @@ -23,6 +23,10 @@ vi.mock("../security/skill-scanner.js", () => ({ scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args), })); +vi.mock("./skills/plugin-skills.js", () => ({ + resolvePluginSkillDirs: () => [], +})); + async function writeInstallableSkill(workspaceDir: string, name: string): Promise { const skillDir = path.join(workspaceDir, "skills", name); await fs.mkdir(skillDir, { recursive: true }); diff --git a/src/agents/skills.agents-skills-directory.test.ts b/src/agents/skills.agents-skills-directory.test.ts index 94519bcd363..bf4b5995260 100644 --- a/src/agents/skills.agents-skills-directory.test.ts +++ b/src/agents/skills.agents-skills-directory.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { buildWorkspaceSkillsPrompt } from "./skills.js"; import { writeSkill } from "./skills.test-helpers.js"; import { @@ -10,6 +10,10 @@ import { type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +vi.mock("./skills/plugin-skills.js", () => ({ + resolvePluginSkillDirs: () => [], +})); + const tempDirs: string[] = []; async function createTempDir(prefix: string) { diff --git a/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts b/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts index 5b9f87a9241..368e9db47bb 100644 --- a/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts +++ b/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts @@ -1,11 +1,15 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { withEnv } from "../test-utils/env.js"; import { writeSkill } from "./skills.e2e-test-helpers.js"; import { buildWorkspaceSkillsPrompt, syncSkillsToWorkspace } from "./skills.js"; +vi.mock("./skills/plugin-skills.js", () => ({ + resolvePluginSkillDirs: () => [], +})); + async function pathExists(filePath: string): Promise { try { await fs.access(filePath); diff --git a/src/agents/skills.buildworkspaceskillsnapshot.test.ts b/src/agents/skills.buildworkspaceskillsnapshot.test.ts index 76b163836fd..4e7c04ec91e 100644 --- a/src/agents/skills.buildworkspaceskillsnapshot.test.ts +++ b/src/agents/skills.buildworkspaceskillsnapshot.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { withPathResolutionEnv } from "../test-utils/env.js"; import { createFixtureSuite } from "../test-utils/fixture-suite.js"; import { createTempHomeEnv, type TempHomeEnv } from "../test-utils/temp-home.js"; @@ -12,6 +12,10 @@ import { type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +vi.mock("./skills/plugin-skills.js", () => ({ + resolvePluginSkillDirs: () => [], +})); + const fixtureSuite = createFixtureSuite("openclaw-skills-snapshot-suite-"); let truncationWorkspaceTemplateDir = ""; let nestedRepoTemplateDir = ""; diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index 53dd9c76f7c..69d6c1dd852 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -38,12 +38,12 @@ afterEach(async () => { setLoggerOverride(null); loggingState.rawConsole = null; resetLogger(); - await Promise.all( - tempDirs.splice(0, tempDirs.length).map((dir) => fs.rm(dir, { recursive: true, force: true })), - ); }); afterAll(async () => { + await Promise.all( + tempDirs.splice(0, tempDirs.length).map((dir) => fs.rm(dir, { recursive: true, force: true })), + ); await restoreMockSkillsHomeEnv(envSnapshot, async () => { if (fakeHome) { await fs.rm(fakeHome, { recursive: true, force: true }); @@ -67,22 +67,6 @@ async function setupWorkspaceWithProsePlugin() { return { workspaceDir, managedDir, bundledDir }; } -async function setupWorkspaceWithDiffsPlugin() { - const workspaceDir = await createTempWorkspaceDir(); - const managedDir = path.join(workspaceDir, ".managed"); - const bundledDir = path.join(workspaceDir, ".bundled"); - const pluginRoot = path.join(workspaceDir, ".openclaw", "extensions", "diffs"); - - await writePluginWithSkill({ - pluginRoot, - pluginId: "diffs", - skillId: "diffs", - skillDescription: "test", - }); - - return { workspaceDir, managedDir, bundledDir }; -} - describe("loadWorkspaceSkillEntries", () => { it("handles an empty managed skills dir without throwing", async () => { const workspaceDir = await createTempWorkspaceDir(); @@ -131,38 +115,6 @@ describe("loadWorkspaceSkillEntries", () => { expect(entries.map((entry) => entry.skill.name)).not.toContain("prose"); }); - it("includes diffs plugin skill when the plugin is enabled", async () => { - const { workspaceDir, managedDir, bundledDir } = await setupWorkspaceWithDiffsPlugin(); - - const entries = loadWorkspaceSkillEntries(workspaceDir, { - config: { - plugins: { - entries: { diffs: { enabled: true } }, - }, - }, - managedSkillsDir: managedDir, - bundledSkillsDir: bundledDir, - }); - - expect(entries.map((entry) => entry.skill.name)).toContain("diffs"); - }); - - it("excludes diffs plugin skill when the plugin is disabled", async () => { - const { workspaceDir, managedDir, bundledDir } = await setupWorkspaceWithDiffsPlugin(); - - const entries = loadWorkspaceSkillEntries(workspaceDir, { - config: { - plugins: { - entries: { diffs: { enabled: false } }, - }, - }, - managedSkillsDir: managedDir, - bundledSkillsDir: bundledDir, - }); - - expect(entries.map((entry) => entry.skill.name)).not.toContain("diffs"); - }); - it("falls back to the skill directory name when frontmatter omits name", async () => { const workspaceDir = await createTempWorkspaceDir(); const skillDir = path.join(workspaceDir, "skills", "fallback-name"); diff --git a/src/agents/skills.test.ts b/src/agents/skills.test.ts index d9f1d5d1262..2376bae8467 100644 --- a/src/agents/skills.test.ts +++ b/src/agents/skills.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { clearRuntimeConfigSnapshot, setRuntimeConfigSnapshot, @@ -27,6 +27,10 @@ import { type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +vi.mock("./skills/plugin-skills.js", () => ({ + resolvePluginSkillDirs: () => [], +})); + const fixtureSuite = createFixtureSuite("openclaw-skills-suite-"); let tempHome: TempHomeEnv | null = null; let skillsHomeEnv: SkillsHomeEnvSnapshot | null = null; diff --git a/src/plugins/bundle-commands.ts b/src/plugins/bundle-commands.ts index 9945d3f0f03..a342f7934da 100644 --- a/src/plugins/bundle-commands.ts +++ b/src/plugins/bundle-commands.ts @@ -13,7 +13,11 @@ import { mergeBundlePathLists, normalizeBundlePathList, } from "./bundle-manifest.js"; -import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; +import { + hasExplicitPluginConfig, + normalizePluginsConfig, + resolveEffectivePluginActivationState, +} from "./config-state.js"; import { loadPluginManifestRegistry } from "./manifest-registry.js"; export type ClaudeBundleCommandSpec = { @@ -169,6 +173,9 @@ export function loadEnabledClaudeBundleCommands(params: { workspaceDir: string; cfg?: OpenClawConfig; }): ClaudeBundleCommandSpec[] { + if (!hasExplicitPluginConfig(params.cfg?.plugins)) { + return []; + } const registry = loadPluginManifestRegistry({ workspaceDir: params.workspaceDir, config: params.cfg,