From 6ccac3d2089f5225d66c047415fbeb82e7856279 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 18 Apr 2026 19:50:30 +0100 Subject: [PATCH] test: optimize skills workspace fixtures --- src/agents/skills-install-fallback.test.ts | 41 +++++++ ...install-uv-python-env-override.poc.test.ts | 82 -------------- src/agents/skills-install.test.ts | 16 +-- .../skills.agents-skills-directory.test.ts | 2 +- ...without-affecting-workspace-skills.test.ts | 57 ++++++---- ...erged-skills-into-target-workspace.test.ts | 105 ++++++++---------- ...skills.buildworkspaceskillsnapshot.test.ts | 2 +- src/agents/skills.compact-skill-paths.test.ts | 2 +- .../skills.loadworkspaceskillentries.test.ts | 2 +- .../skills.resolveskillspromptforrun.test.ts | 2 +- src/agents/skills.test.ts | 11 +- 11 files changed, 142 insertions(+), 180 deletions(-) delete mode 100644 src/agents/skills-install-uv-python-env-override.poc.test.ts diff --git a/src/agents/skills-install-fallback.test.ts b/src/agents/skills-install-fallback.test.ts index 63f9218a4c1..c05d74cb568 100644 --- a/src/agents/skills-install-fallback.test.ts +++ b/src/agents/skills-install-fallback.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { loadWorkspaceDotEnvFile } from "../infra/dotenv.js"; import { captureEnv } from "../test-utils/env.js"; import { hasBinaryMock, @@ -216,4 +217,44 @@ describe("skills-install fallback edge cases", () => { envSnapshot.restore(); } }); + + it("blocks workspace dotenv UV_PYTHON from uv install execution", async () => { + mockAvailableBinaries(["uv"]); + runCommandWithTimeoutMock.mockResolvedValueOnce({ + code: 0, + stdout: "ok", + stderr: "", + signal: null, + killed: false, + }); + + const workspaceEnvPath = path.join(workspaceDir, ".env"); + const envSnapshot = captureEnv(["UV_PYTHON"]); + try { + delete process.env.UV_PYTHON; + await fs.writeFile(workspaceEnvPath, "UV_PYTHON=/tmp/attacker-python\n", "utf-8"); + + loadWorkspaceDotEnvFile(workspaceEnvPath, { quiet: true }); + expect(process.env.UV_PYTHON).toBeUndefined(); + + const result = await installSkill({ + workspaceDir, + skillName: "py-tool", + installId: "deps", + timeoutMs: 10_000, + }); + + expect(result.ok).toBe(true); + expect(runCommandWithTimeoutMock).toHaveBeenCalledWith( + ["uv", "tool", "install", "example-package"], + expect.objectContaining({ + timeoutMs: 10_000, + env: undefined, + }), + ); + } finally { + envSnapshot.restore(); + await fs.rm(workspaceEnvPath, { force: true }); + } + }); }); diff --git a/src/agents/skills-install-uv-python-env-override.poc.test.ts b/src/agents/skills-install-uv-python-env-override.poc.test.ts deleted file mode 100644 index 74be895a669..00000000000 --- a/src/agents/skills-install-uv-python-env-override.poc.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { loadWorkspaceDotEnvFile } from "../infra/dotenv.js"; -import { captureEnv } from "../test-utils/env.js"; -import { installSkill } from "./skills-install.js"; - -describe("workspace .env UV_PYTHON handling for uv skill installs", () => { - let envSnapshot: ReturnType | undefined; - - afterEach(async () => { - envSnapshot?.restore(); - envSnapshot = undefined; - }); - - it.runIf(process.platform !== "win32")( - "does not propagate UV_PYTHON from workspace dotenv into uv tool install execution", - async () => { - const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-poc-uv-python-")); - const cwdDir = path.join(base, "cwd"); - const binDir = path.join(base, "bin"); - const markerPath = path.join(base, "uv-python-marker.txt"); - const fakeUvPath = path.join(binDir, "uv"); - try { - await fs.mkdir(cwdDir, { recursive: true }); - await fs.mkdir(binDir, { recursive: true }); - await fs.mkdir(path.join(cwdDir, "skills", "uv-skill"), { recursive: true }); - - await fs.writeFile( - path.join(cwdDir, "skills", "uv-skill", "SKILL.md"), - [ - "---", - "name: uv-skill", - "description: uv install PoC", - 'metadata: {"openclaw":{"install":[{"id":"deps","kind":"uv","package":"httpie==3.2.2"}]}}', - "---", - "", - "# uv-skill", - "", - ].join("\n"), - "utf8", - ); - - await fs.writeFile( - fakeUvPath, - [ - "#!/bin/sh", - 'printf "%s\\n" "$UV_PYTHON" > "$OPENCLAW_POC_MARKER_PATH"', - "exit 0", - "", - ].join("\n"), - "utf8", - ); - await fs.chmod(fakeUvPath, 0o755); - - const attackerPython = path.join(base, "attacker-python"); - await fs.writeFile(path.join(cwdDir, ".env"), `UV_PYTHON=${attackerPython}\n`, "utf8"); - - envSnapshot = captureEnv(["PATH", "UV_PYTHON", "OPENCLAW_POC_MARKER_PATH"]); - delete process.env.UV_PYTHON; - process.env.OPENCLAW_POC_MARKER_PATH = markerPath; - process.env.PATH = `${binDir}${path.delimiter}${process.env.PATH ?? ""}`; - - loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true }); - expect(process.env.UV_PYTHON).toBeUndefined(); - - const result = await installSkill({ - workspaceDir: cwdDir, - skillName: "uv-skill", - installId: "deps", - timeoutMs: 10_000, - }); - - expect(result.ok).toBe(true); - await expect(fs.readFile(markerPath, "utf8")).resolves.toBe("\n"); - } finally { - await fs.rm(base, { recursive: true, force: true }); - } - }, - ); -}); diff --git a/src/agents/skills-install.test.ts b/src/agents/skills-install.test.ts index ab72b00aa19..8555f51acb7 100644 --- a/src/agents/skills-install.test.ts +++ b/src/agents/skills-install.test.ts @@ -6,9 +6,8 @@ import { resetGlobalHookRunner, } from "../plugins/hook-runner-global.js"; import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js"; +import { captureEnv } from "../test-utils/env.js"; import { createFixtureSuite } from "../test-utils/fixture-suite.js"; -import { createTempHomeEnv, type TempHomeEnv } from "../test-utils/temp-home.js"; -import { setTempStateDir } from "./skills-install.download-test-utils.js"; import { installSkill } from "./skills-install.js"; import { runCommandWithTimeoutMock, @@ -47,25 +46,28 @@ metadata: {"openclaw":{"install":[{"id":"deps","kind":"node","package":"example- } const workspaceSuite = createFixtureSuite("openclaw-skills-install-"); -let tempHome: TempHomeEnv; beforeAll(async () => { - tempHome = await createTempHomeEnv("openclaw-skills-install-home-"); await workspaceSuite.setup(); }); afterAll(async () => { resetGlobalHookRunner(); await workspaceSuite.cleanup(); - await tempHome.restore(); }); async function withWorkspaceCase( run: (params: { workspaceDir: string; stateDir: string }) => Promise, ): Promise { const workspaceDir = await workspaceSuite.createCaseDir("case"); - const stateDir = setTempStateDir(workspaceDir); - await run({ workspaceDir, stateDir }); + const stateDir = path.join(workspaceDir, "state"); + const envSnapshot = captureEnv(["OPENCLAW_STATE_DIR"]); + try { + process.env.OPENCLAW_STATE_DIR = stateDir; + await run({ workspaceDir, stateDir }); + } finally { + envSnapshot.restore(); + } } describe("installSkill code safety scanning", () => { diff --git a/src/agents/skills.agents-skills-directory.test.ts b/src/agents/skills.agents-skills-directory.test.ts index bf4b5995260..febbad8045a 100644 --- a/src/agents/skills.agents-skills-directory.test.ts +++ b/src/agents/skills.agents-skills-directory.test.ts @@ -2,13 +2,13 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { buildWorkspaceSkillsPrompt } from "./skills.js"; import { writeSkill } from "./skills.test-helpers.js"; import { restoreMockSkillsHomeEnv, setMockSkillsHomeEnv, type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +import { buildWorkspaceSkillsPrompt } from "./skills/workspace.js"; vi.mock("./skills/plugin-skills.js", () => ({ resolvePluginSkillDirs: () => [], diff --git a/src/agents/skills.build-workspace-skills-prompt.applies-bundled-allowlist-without-affecting-workspace-skills.test.ts b/src/agents/skills.build-workspace-skills-prompt.applies-bundled-allowlist-without-affecting-workspace-skills.test.ts index dad26e0fb74..f50260ff778 100644 --- a/src/agents/skills.build-workspace-skills-prompt.applies-bundled-allowlist-without-affecting-workspace-skills.test.ts +++ b/src/agents/skills.build-workspace-skills-prompt.applies-bundled-allowlist-without-affecting-workspace-skills.test.ts @@ -2,36 +2,47 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; import { writeSkill } from "./skills.e2e-test-helpers.js"; -import { buildWorkspaceSkillsPrompt } from "./skills.js"; +import { buildWorkspaceSkillsPrompt } from "./skills/workspace.js"; describe("buildWorkspaceSkillsPrompt", () => { it("applies bundled allowlist without affecting workspace skills", async () => { + const env = captureEnv(["HOME", "USERPROFILE", "OPENCLAW_HOME", "OPENCLAW_STATE_DIR"]); const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-")); - const bundledDir = path.join(workspaceDir, ".bundled"); - const bundledSkillDir = path.join(bundledDir, "peekaboo"); - const workspaceSkillDir = path.join(workspaceDir, "skills", "demo-skill"); + try { + process.env.HOME = workspaceDir; + process.env.USERPROFILE = workspaceDir; + delete process.env.OPENCLAW_HOME; + delete process.env.OPENCLAW_STATE_DIR; + const bundledDir = path.join(workspaceDir, ".bundled"); + const bundledSkillDir = path.join(bundledDir, "peekaboo"); + const workspaceSkillDir = path.join(workspaceDir, "skills", "demo-skill"); - await writeSkill({ - dir: bundledSkillDir, - name: "peekaboo", - description: "Capture UI", - body: "# Peekaboo\n", - }); - await writeSkill({ - dir: workspaceSkillDir, - name: "demo-skill", - description: "Workspace version", - body: "# Workspace\n", - }); + await writeSkill({ + dir: bundledSkillDir, + name: "peekaboo", + description: "Capture UI", + body: "# Peekaboo\n", + }); + await writeSkill({ + dir: workspaceSkillDir, + name: "demo-skill", + description: "Workspace version", + body: "# Workspace\n", + }); - const prompt = buildWorkspaceSkillsPrompt(workspaceDir, { - bundledSkillsDir: bundledDir, - managedSkillsDir: path.join(workspaceDir, ".managed"), - config: { skills: { allowBundled: ["missing-skill"] } }, - }); + const prompt = buildWorkspaceSkillsPrompt(workspaceDir, { + bundledSkillsDir: bundledDir, + managedSkillsDir: path.join(workspaceDir, ".managed"), + config: { skills: { allowBundled: ["missing-skill"] } }, + }); - expect(prompt).toContain("Workspace version"); - expect(prompt).not.toContain("peekaboo"); + expect(prompt).toContain("Workspace version"); + expect(prompt).not.toContain("peekaboo"); + } finally { + env.restore(); + await fs.rm(workspaceDir, { recursive: true, force: true }); + } }); }); 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 368e9db47bb..dde5cd28104 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 @@ -4,7 +4,7 @@ import path from "node:path"; 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"; +import { buildWorkspaceSkillsPrompt, syncSkillsToWorkspace } from "./skills/workspace.js"; vi.mock("./skills/plugin-skills.js", () => ({ resolvePluginSkillDirs: () => [], @@ -30,14 +30,12 @@ async function createCaseDir(prefix: string): Promise { } async function syncSourceSkillsToTarget(sourceWorkspace: string, targetWorkspace: string) { - await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => - syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), - managedSkillsDir: path.join(sourceWorkspace, ".managed"), - }), - ); + await syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), + managedSkillsDir: path.join(sourceWorkspace, ".managed"), + }); } async function expectSyncedSkillConfinement(params: { @@ -89,8 +87,7 @@ describe("buildWorkspaceSkillsPrompt", () => { const buildPrompt = ( workspaceDir: string, opts?: Parameters[1], - ) => - withEnv({ HOME: workspaceDir, PATH: "" }, () => buildWorkspaceSkillsPrompt(workspaceDir, opts)); + ) => withEnv({ HOME: workspaceDir }, () => buildWorkspaceSkillsPrompt(workspaceDir, opts)); const cloneSourceTemplate = async () => { const sourceWorkspace = await createCaseDir("source"); @@ -114,15 +111,13 @@ describe("buildWorkspaceSkillsPrompt", () => { "export {}", ); - await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => - syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - config: { skills: { load: { extraDirs: [extraDir] } } }, - bundledSkillsDir: bundledDir, - managedSkillsDir: managedDir, - }), - ); + await syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + config: { skills: { load: { extraDirs: [extraDir] } } }, + bundledSkillsDir: bundledDir, + managedSkillsDir: managedDir, + }); const prompt = buildPrompt(targetWorkspace, { bundledSkillsDir: path.join(targetWorkspace, ".bundled"), @@ -156,23 +151,21 @@ describe("buildWorkspaceSkillsPrompt", () => { description: "Dot variant", }); - await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => - syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - agentId: "alpha", - config: { - agents: { - defaults: { - skills: ["foo_bar", "foo.dot"], - }, - list: [{ id: "alpha", skills: ["foo_bar"] }], + await syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + agentId: "alpha", + config: { + agents: { + defaults: { + skills: ["foo_bar", "foo.dot"], }, + list: [{ id: "alpha", skills: ["foo_bar"] }], }, - bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), - managedSkillsDir: path.join(sourceWorkspace, ".managed"), - }), - ); + }, + bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), + managedSkillsDir: path.join(sourceWorkspace, ".managed"), + }); const prompt = buildPrompt(targetWorkspace, { bundledSkillsDir: path.join(targetWorkspace, ".bundled"), @@ -329,31 +322,29 @@ describe("buildWorkspaceSkillsPrompt", () => { metadata: '{"openclaw":{"requires":{"anyBins":["missingbin","sandboxbin"]}}}', }); - await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => - syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - agentId: "alpha", - config: { - agents: { - defaults: { - skills: ["remote-only"], - }, - list: [{ id: "alpha" }], + await syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + agentId: "alpha", + config: { + agents: { + defaults: { + skills: ["remote-only"], }, + list: [{ id: "alpha" }], }, - eligibility: { - remote: { - platforms: ["linux"], - hasBin: () => false, - hasAnyBin: (bins: string[]) => bins.includes("sandboxbin"), - note: "sandbox", - }, + }, + eligibility: { + remote: { + platforms: ["linux"], + hasBin: () => false, + hasAnyBin: (bins: string[]) => bins.includes("sandboxbin"), + note: "sandbox", }, - bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), - managedSkillsDir: path.join(sourceWorkspace, ".managed"), - }), - ); + }, + bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), + managedSkillsDir: path.join(sourceWorkspace, ".managed"), + }); expect(await pathExists(path.join(targetWorkspace, "skills", "remote-only", "SKILL.md"))).toBe( true, diff --git a/src/agents/skills.buildworkspaceskillsnapshot.test.ts b/src/agents/skills.buildworkspaceskillsnapshot.test.ts index 4e7c04ec91e..293cad66834 100644 --- a/src/agents/skills.buildworkspaceskillsnapshot.test.ts +++ b/src/agents/skills.buildworkspaceskillsnapshot.test.ts @@ -5,12 +5,12 @@ 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"; import { writeSkill } from "./skills.e2e-test-helpers.js"; -import { buildWorkspaceSkillSnapshot, buildWorkspaceSkillsPrompt } from "./skills.js"; import { restoreMockSkillsHomeEnv, setMockSkillsHomeEnv, type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +import { buildWorkspaceSkillSnapshot, buildWorkspaceSkillsPrompt } from "./skills/workspace.js"; vi.mock("./skills/plugin-skills.js", () => ({ resolvePluginSkillDirs: () => [], diff --git a/src/agents/skills.compact-skill-paths.test.ts b/src/agents/skills.compact-skill-paths.test.ts index 764af374594..fb671812f89 100644 --- a/src/agents/skills.compact-skill-paths.test.ts +++ b/src/agents/skills.compact-skill-paths.test.ts @@ -1,8 +1,8 @@ import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { buildWorkspaceSkillsPrompt } from "./skills.js"; import { createCanonicalFixtureSkill } from "./skills.test-helpers.js"; +import { buildWorkspaceSkillsPrompt } from "./skills/workspace.js"; describe("compactSkillPaths", () => { it("replaces home directory prefix with ~ in skill locations", () => { diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index 4462bd99d0e..f44c83e4b9e 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -6,13 +6,13 @@ import { resetLogger, setLoggerOverride } from "../logging/logger.js"; import { loggingState } from "../logging/state.js"; import { withPathResolutionEnv } from "../test-utils/env.js"; import { writeSkill } from "./skills.e2e-test-helpers.js"; -import { loadWorkspaceSkillEntries } from "./skills.js"; import { restoreMockSkillsHomeEnv, setMockSkillsHomeEnv, type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; import { readSkillFrontmatterSafe } from "./skills/local-loader.js"; +import { loadWorkspaceSkillEntries } from "./skills/workspace.js"; import { writePluginWithSkill } from "./test-helpers/skill-plugin-fixtures.js"; const tempDirs: string[] = []; diff --git a/src/agents/skills.resolveskillspromptforrun.test.ts b/src/agents/skills.resolveskillspromptforrun.test.ts index 410aa1fbc9f..e7a5c70e839 100644 --- a/src/agents/skills.resolveskillspromptforrun.test.ts +++ b/src/agents/skills.resolveskillspromptforrun.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; -import { resolveSkillsPromptForRun } from "./skills.js"; import { createCanonicalFixtureSkill } from "./skills.test-helpers.js"; import type { SkillEntry } from "./skills/types.js"; +import { resolveSkillsPromptForRun } from "./skills/workspace.js"; describe("resolveSkillsPromptForRun", () => { it("prefers snapshot prompt when available", () => { diff --git a/src/agents/skills.test.ts b/src/agents/skills.test.ts index 1e4e28427cf..01f9ff57401 100644 --- a/src/agents/skills.test.ts +++ b/src/agents/skills.test.ts @@ -12,20 +12,19 @@ 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"; import { writeSkill } from "./skills.e2e-test-helpers.js"; +import { buildWorkspaceSkillCommandSpecs } from "./skills/command-specs.js"; import { applySkillEnvOverrides, applySkillEnvOverridesFromSnapshot, - buildWorkspaceSkillCommandSpecs, - buildWorkspaceSkillsPrompt, - type SkillEntry, - type SkillSnapshot, -} from "./skills.js"; -import { getActiveSkillEnvKeys } from "./skills/env-overrides.js"; + getActiveSkillEnvKeys, +} from "./skills/env-overrides.js"; import { restoreMockSkillsHomeEnv, setMockSkillsHomeEnv, type SkillsHomeEnvSnapshot, } from "./skills/home-env.test-support.js"; +import type { SkillEntry, SkillSnapshot } from "./skills/types.js"; +import { buildWorkspaceSkillsPrompt } from "./skills/workspace.js"; vi.mock("./skills/plugin-skills.js", () => ({ resolvePluginSkillDirs: () => [],