From 61817c90e72fd3bf7d50c8d5078f1c7969fbde20 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 18:51:40 +0000 Subject: [PATCH] refactor(test): share temp workspace helper for skill download suites --- ...skills-install.download-tarbz2.e2e.test.ts | 16 +----- .../skills-install.download-test-utils.ts | 13 +++++ .../skills-install.download.e2e.test.ts | 51 +++++-------------- 3 files changed, 27 insertions(+), 53 deletions(-) diff --git a/src/agents/skills-install.download-tarbz2.e2e.test.ts b/src/agents/skills-install.download-tarbz2.e2e.test.ts index 73bb3c57e..0f486a28c 100644 --- a/src/agents/skills-install.download-tarbz2.e2e.test.ts +++ b/src/agents/skills-install.download-tarbz2.e2e.test.ts @@ -1,9 +1,7 @@ -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 { captureEnv } from "../test-utils/env.js"; -import { setTempStateDir, writeDownloadSkill } from "./skills-install.download-test-utils.js"; +import { withTempWorkspace, writeDownloadSkill } from "./skills-install.download-test-utils.js"; import { installSkill } from "./skills-install.js"; const mocks = { @@ -54,18 +52,6 @@ function mockTarExtractionFlow(params: { }); } -async function withTempWorkspace( - run: (params: { workspaceDir: string; stateDir: string }) => Promise, -) { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); - await run({ workspaceDir, stateDir }); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } -} - async function writeTarBz2Skill(params: { workspaceDir: string; stateDir: string; diff --git a/src/agents/skills-install.download-test-utils.ts b/src/agents/skills-install.download-test-utils.ts index 951bd5562..a3ea85d95 100644 --- a/src/agents/skills-install.download-test-utils.ts +++ b/src/agents/skills-install.download-test-utils.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; export function setTempStateDir(workspaceDir: string): string { @@ -7,6 +8,18 @@ export function setTempStateDir(workspaceDir: string): string { return stateDir; } +export async function withTempWorkspace( + run: (params: { workspaceDir: string; stateDir: string }) => Promise, +) { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + await run({ workspaceDir, stateDir }); + } finally { + await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); + } +} + export async function writeDownloadSkill(params: { workspaceDir: string; name: string; diff --git a/src/agents/skills-install.download.e2e.test.ts b/src/agents/skills-install.download.e2e.test.ts index 0cbf7648e..8ffe02249 100644 --- a/src/agents/skills-install.download.e2e.test.ts +++ b/src/agents/skills-install.download.e2e.test.ts @@ -1,11 +1,10 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import JSZip from "jszip"; import * as tar from "tar"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { captureEnv } from "../test-utils/env.js"; -import { setTempStateDir, writeDownloadSkill } from "./skills-install.download-test-utils.js"; +import { withTempWorkspace, writeDownloadSkill } from "./skills-install.download-test-utils.js"; import { installSkill } from "./skills-install.js"; const runCommandWithTimeoutMock = vi.fn(); @@ -92,9 +91,7 @@ describe("installSkill download extraction safety", () => { }); it("rejects zip slip traversal", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { const targetDir = path.join(stateDir, "tools", "zip-slip", "target"); const outsideWriteDir = path.join(workspaceDir, "outside-write"); const outsideWritePath = path.join(outsideWriteDir, "pwned.txt"); @@ -121,15 +118,11 @@ describe("installSkill download extraction safety", () => { const result = await installSkill({ workspaceDir, skillName: "zip-slip", installId: "dl" }); expect(result.ok).toBe(false); expect(await fileExists(outsideWritePath)).toBe(false); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); it("rejects tar.gz traversal", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { const targetDir = path.join(stateDir, "tools", "tar-slip", "target"); const insideDir = path.join(workspaceDir, "inside"); const outsideWriteDir = path.join(workspaceDir, "outside-write"); @@ -164,15 +157,11 @@ describe("installSkill download extraction safety", () => { const result = await installSkill({ workspaceDir, skillName: "tar-slip", installId: "dl" }); expect(result.ok).toBe(false); expect(await fileExists(outsideWritePath)).toBe(false); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); it("extracts zip with stripComponents safely", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { const targetDir = path.join(stateDir, "tools", "zip-good", "target"); const url = "https://example.invalid/good.zip"; @@ -197,15 +186,11 @@ describe("installSkill download extraction safety", () => { const result = await installSkill({ workspaceDir, skillName: "zip-good", installId: "dl" }); expect(result.ok).toBe(true); expect(await fs.readFile(path.join(targetDir, "hello.txt"), "utf-8")).toBe("hi"); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); it("rejects targetDir outside the per-skill tools root", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { const targetDir = path.join(workspaceDir, "outside"); const url = "https://example.invalid/good.zip"; @@ -236,15 +221,11 @@ describe("installSkill download extraction safety", () => { expect(fetchWithSsrFGuardMock.mock.calls.length).toBe(0); expect(stateDir.length).toBeGreaterThan(0); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); it("allows relative targetDir inside the per-skill tools root", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const stateDir = setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { const result = await installZipDownloadSkill({ workspaceDir, name: "relative-targetdir", @@ -257,15 +238,11 @@ describe("installSkill download extraction safety", () => { "utf-8", ), ).toBe("hi"); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); it("rejects relative targetDir traversal", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - setTempStateDir(workspaceDir); + await withTempWorkspace(async ({ workspaceDir }) => { const result = await installZipDownloadSkill({ workspaceDir, name: "relative-traversal", @@ -274,8 +251,6 @@ describe("installSkill download extraction safety", () => { expect(result.ok).toBe(false); expect(result.stderr).toContain("Refusing to install outside the skill tools directory"); expect(fetchWithSsrFGuardMock.mock.calls.length).toBe(0); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } + }); }); });