diff --git a/src/agents/skills-install.download-tarbz2.e2e.test.ts b/src/agents/skills-install.download-tarbz2.e2e.test.ts new file mode 100644 index 00000000000..1cadeb621e6 --- /dev/null +++ b/src/agents/skills-install.download-tarbz2.e2e.test.ts @@ -0,0 +1,317 @@ +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 { installSkill } from "./skills-install.js"; + +const runCommandWithTimeoutMock = vi.fn(); +const scanDirectoryWithSummaryMock = vi.fn(); +const fetchWithSsrFGuardMock = vi.fn(); + +const originalOpenClawStateDir = process.env.OPENCLAW_STATE_DIR; + +afterEach(() => { + if (originalOpenClawStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = originalOpenClawStateDir; + } +}); + +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), +})); + +vi.mock("../infra/net/fetch-guard.js", () => ({ + fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args), +})); + +vi.mock("../security/skill-scanner.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args), + }; +}); + +async function writeDownloadSkill(params: { + workspaceDir: string; + name: string; + installId: string; + url: string; + stripComponents?: number; + targetDir: string; +}): Promise { + const skillDir = path.join(params.workspaceDir, "skills", params.name); + await fs.mkdir(skillDir, { recursive: true }); + const meta = { + openclaw: { + install: [ + { + id: params.installId, + kind: "download", + url: params.url, + archive: "tar.bz2", + extract: true, + stripComponents: params.stripComponents, + targetDir: params.targetDir, + }, + ], + }, + }; + await fs.writeFile( + path.join(skillDir, "SKILL.md"), + `--- +name: ${params.name} +description: test skill +metadata: ${JSON.stringify(meta)} +--- + +# ${params.name} +`, + "utf-8", + ); + await fs.writeFile(path.join(skillDir, "runner.js"), "export {};\n", "utf-8"); + return skillDir; +} + +function setTempStateDir(workspaceDir: string): string { + const stateDir = path.join(workspaceDir, "state"); + process.env.OPENCLAW_STATE_DIR = stateDir; + return stateDir; +} + +describe("installSkill download extraction safety (tar.bz2)", () => { + beforeEach(() => { + runCommandWithTimeoutMock.mockReset(); + scanDirectoryWithSummaryMock.mockReset(); + fetchWithSsrFGuardMock.mockReset(); + scanDirectoryWithSummaryMock.mockResolvedValue({ + scannedFiles: 0, + critical: 0, + warn: 0, + info: 0, + findings: [], + }); + }); + + it("rejects tar.bz2 traversal before extraction", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + const targetDir = path.join(stateDir, "tools", "tbz2-slip", "target"); + const url = "https://example.invalid/evil.tbz2"; + + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), + release: async () => undefined, + }); + + runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { + const cmd = argv as string[]; + if (cmd[0] === "tar" && cmd[1] === "tf") { + return { code: 0, stdout: "../outside.txt\n", stderr: "", signal: null, killed: false }; + } + if (cmd[0] === "tar" && cmd[1] === "tvf") { + return { + code: 0, + stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 ../outside.txt\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "xf") { + throw new Error("should not extract"); + } + return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; + }); + + await writeDownloadSkill({ + workspaceDir, + name: "tbz2-slip", + installId: "dl", + url, + targetDir, + }); + + const result = await installSkill({ workspaceDir, skillName: "tbz2-slip", installId: "dl" }); + expect(result.ok).toBe(false); + expect( + runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), + ).toBe(false); + } finally { + await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); + } + }); + + it("rejects tar.bz2 archives containing symlinks", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + const targetDir = path.join(stateDir, "tools", "tbz2-symlink", "target"); + const url = "https://example.invalid/evil.tbz2"; + + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), + release: async () => undefined, + }); + + runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { + const cmd = argv as string[]; + if (cmd[0] === "tar" && cmd[1] === "tf") { + return { + code: 0, + stdout: "link\nlink/pwned.txt\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "tvf") { + return { + code: 0, + stdout: "lrwxr-xr-x 0 0 0 0 Jan 1 00:00 link -> ../outside\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "xf") { + throw new Error("should not extract"); + } + return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; + }); + + await writeDownloadSkill({ + workspaceDir, + name: "tbz2-symlink", + installId: "dl", + url, + targetDir, + }); + + const result = await installSkill({ + workspaceDir, + skillName: "tbz2-symlink", + installId: "dl", + }); + expect(result.ok).toBe(false); + expect(result.stderr.toLowerCase()).toContain("link"); + } finally { + await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); + } + }); + + it("extracts tar.bz2 with stripComponents safely (preflight only)", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + const targetDir = path.join(stateDir, "tools", "tbz2-ok", "target"); + const url = "https://example.invalid/good.tbz2"; + + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), + release: async () => undefined, + }); + + runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { + const cmd = argv as string[]; + if (cmd[0] === "tar" && cmd[1] === "tf") { + return { + code: 0, + stdout: "package/hello.txt\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "tvf") { + return { + code: 0, + stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 package/hello.txt\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "xf") { + return { code: 0, stdout: "ok", stderr: "", signal: null, killed: false }; + } + return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; + }); + + await writeDownloadSkill({ + workspaceDir, + name: "tbz2-ok", + installId: "dl", + url, + stripComponents: 1, + targetDir, + }); + + const result = await installSkill({ workspaceDir, skillName: "tbz2-ok", installId: "dl" }); + expect(result.ok).toBe(true); + expect( + runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), + ).toBe(true); + } finally { + await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); + } + }); + + it("rejects tar.bz2 stripComponents escape", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + const targetDir = path.join(stateDir, "tools", "tbz2-strip-escape", "target"); + const url = "https://example.invalid/evil.tbz2"; + + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), + release: async () => undefined, + }); + + runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { + const cmd = argv as string[]; + if (cmd[0] === "tar" && cmd[1] === "tf") { + return { code: 0, stdout: "a/../b.txt\n", stderr: "", signal: null, killed: false }; + } + if (cmd[0] === "tar" && cmd[1] === "tvf") { + return { + code: 0, + stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 a/../b.txt\n", + stderr: "", + signal: null, + killed: false, + }; + } + if (cmd[0] === "tar" && cmd[1] === "xf") { + throw new Error("should not extract"); + } + return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; + }); + + await writeDownloadSkill({ + workspaceDir, + name: "tbz2-strip-escape", + installId: "dl", + url, + stripComponents: 1, + targetDir, + }); + + const result = await installSkill({ + workspaceDir, + skillName: "tbz2-strip-escape", + installId: "dl", + }); + expect(result.ok).toBe(false); + expect( + runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), + ).toBe(false); + } finally { + await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); + } + }); +}); diff --git a/src/agents/skills-install.download.e2e.test.ts b/src/agents/skills-install.download.e2e.test.ts new file mode 100644 index 00000000000..540922ea6f1 --- /dev/null +++ b/src/agents/skills-install.download.e2e.test.ts @@ -0,0 +1,335 @@ +import JSZip from "jszip"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import * as tar from "tar"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { installSkill } from "./skills-install.js"; + +const runCommandWithTimeoutMock = vi.fn(); +const scanDirectoryWithSummaryMock = vi.fn(); +const fetchWithSsrFGuardMock = vi.fn(); + +const originalOpenClawStateDir = process.env.OPENCLAW_STATE_DIR; + +afterEach(() => { + if (originalOpenClawStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = originalOpenClawStateDir; + } +}); + +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), +})); + +vi.mock("../infra/net/fetch-guard.js", () => ({ + fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args), +})); + +vi.mock("../security/skill-scanner.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args), + }; +}); + +async function writeDownloadSkill(params: { + workspaceDir: string; + name: string; + installId: string; + url: string; + archive: "tar.gz" | "tar.bz2" | "zip"; + stripComponents?: number; + targetDir: string; +}): Promise { + const skillDir = path.join(params.workspaceDir, "skills", params.name); + await fs.mkdir(skillDir, { recursive: true }); + const meta = { + openclaw: { + install: [ + { + id: params.installId, + kind: "download", + url: params.url, + archive: params.archive, + extract: true, + stripComponents: params.stripComponents, + targetDir: params.targetDir, + }, + ], + }, + }; + await fs.writeFile( + path.join(skillDir, "SKILL.md"), + `--- +name: ${params.name} +description: test skill +metadata: ${JSON.stringify(meta)} +--- + +# ${params.name} +`, + "utf-8", + ); + await fs.writeFile(path.join(skillDir, "runner.js"), "export {};\n", "utf-8"); + return skillDir; +} + +async function fileExists(filePath: string): Promise { + try { + await fs.stat(filePath); + return true; + } catch { + return false; + } +} + +function setTempStateDir(workspaceDir: string): string { + const stateDir = path.join(workspaceDir, "state"); + process.env.OPENCLAW_STATE_DIR = stateDir; + return stateDir; +} + +describe("installSkill download extraction safety", () => { + beforeEach(() => { + runCommandWithTimeoutMock.mockReset(); + scanDirectoryWithSummaryMock.mockReset(); + fetchWithSsrFGuardMock.mockReset(); + scanDirectoryWithSummaryMock.mockResolvedValue({ + scannedFiles: 0, + critical: 0, + warn: 0, + info: 0, + findings: [], + }); + }); + + it("rejects zip slip traversal", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); + try { + const stateDir = setTempStateDir(workspaceDir); + const targetDir = path.join(stateDir, "tools", "zip-slip", "target"); + const outsideWriteDir = path.join(workspaceDir, "outside-write"); + const outsideWritePath = path.join(outsideWriteDir, "pwned.txt"); + const url = "https://example.invalid/evil.zip"; + + const zip = new JSZip(); + zip.file("../outside-write/pwned.txt", "pwnd"); + const buffer = await zip.generateAsync({ type: "nodebuffer" }); + + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "zip-slip", + installId: "dl", + url, + archive: "zip", + targetDir, + }); + + 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); + const targetDir = path.join(stateDir, "tools", "tar-slip", "target"); + const insideDir = path.join(workspaceDir, "inside"); + const outsideWriteDir = path.join(workspaceDir, "outside-write"); + const outsideWritePath = path.join(outsideWriteDir, "pwned.txt"); + const archivePath = path.join(workspaceDir, "evil.tgz"); + const url = "https://example.invalid/evil"; + + await fs.mkdir(insideDir, { recursive: true }); + await fs.mkdir(outsideWriteDir, { recursive: true }); + await fs.writeFile(outsideWritePath, "pwnd", "utf-8"); + + await tar.c({ cwd: insideDir, file: archivePath, gzip: true }, [ + "../outside-write/pwned.txt", + ]); + await fs.rm(outsideWriteDir, { recursive: true, force: true }); + + const buffer = await fs.readFile(archivePath); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "tar-slip", + installId: "dl", + url, + archive: "tar.gz", + targetDir, + }); + + 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); + const targetDir = path.join(stateDir, "tools", "zip-good", "target"); + const url = "https://example.invalid/good.zip"; + + const zip = new JSZip(); + zip.file("package/hello.txt", "hi"); + const buffer = await zip.generateAsync({ type: "nodebuffer" }); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "zip-good", + installId: "dl", + url, + archive: "zip", + stripComponents: 1, + targetDir, + }); + + 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); + const targetDir = path.join(workspaceDir, "outside"); + const url = "https://example.invalid/good.zip"; + + const zip = new JSZip(); + zip.file("hello.txt", "hi"); + const buffer = await zip.generateAsync({ type: "nodebuffer" }); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "targetdir-escape", + installId: "dl", + url, + archive: "zip", + targetDir, + }); + + const result = await installSkill({ + workspaceDir, + skillName: "targetdir-escape", + installId: "dl", + }); + expect(result.ok).toBe(false); + expect(result.stderr).toContain("Refusing to install outside the skill tools directory"); + 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); + const url = "https://example.invalid/good.zip"; + + const zip = new JSZip(); + zip.file("hello.txt", "hi"); + const buffer = await zip.generateAsync({ type: "nodebuffer" }); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "relative-targetdir", + installId: "dl", + url, + archive: "zip", + targetDir: "runtime", + }); + + const result = await installSkill({ + workspaceDir, + skillName: "relative-targetdir", + installId: "dl", + }); + expect(result.ok).toBe(true); + expect( + await fs.readFile( + path.join(stateDir, "tools", "relative-targetdir", "runtime", "hello.txt"), + "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); + const url = "https://example.invalid/good.zip"; + + const zip = new JSZip(); + zip.file("hello.txt", "hi"); + const buffer = await zip.generateAsync({ type: "nodebuffer" }); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: new Response(buffer, { status: 200 }), + release: async () => undefined, + }); + + await writeDownloadSkill({ + workspaceDir, + name: "relative-traversal", + installId: "dl", + url, + archive: "zip", + targetDir: "../outside", + }); + + const result = await installSkill({ + workspaceDir, + skillName: "relative-traversal", + installId: "dl", + }); + 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); + } + }); +}); diff --git a/src/agents/skills-install.e2e.test.ts b/src/agents/skills-install.e2e.test.ts index eeb64121b20..696b03e828b 100644 --- a/src/agents/skills-install.e2e.test.ts +++ b/src/agents/skills-install.e2e.test.ts @@ -1,23 +1,16 @@ -import JSZip from "jszip"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import * as tar from "tar"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { installSkill } from "./skills-install.js"; const runCommandWithTimeoutMock = vi.fn(); const scanDirectoryWithSummaryMock = vi.fn(); -const fetchWithSsrFGuardMock = vi.fn(); vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), })); -vi.mock("../infra/net/fetch-guard.js", () => ({ - fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args), -})); - vi.mock("../security/skill-scanner.js", async (importOriginal) => { const actual = await importOriginal(); return { @@ -45,62 +38,10 @@ metadata: {"openclaw":{"install":[{"id":"deps","kind":"node","package":"example- return skillDir; } -async function writeDownloadSkill(params: { - workspaceDir: string; - name: string; - installId: string; - url: string; - archive: "tar.gz" | "tar.bz2" | "zip"; - stripComponents?: number; - targetDir: string; -}): Promise { - const skillDir = path.join(params.workspaceDir, "skills", params.name); - await fs.mkdir(skillDir, { recursive: true }); - const meta = { - openclaw: { - install: [ - { - id: params.installId, - kind: "download", - url: params.url, - archive: params.archive, - extract: true, - stripComponents: params.stripComponents, - targetDir: params.targetDir, - }, - ], - }, - }; - await fs.writeFile( - path.join(skillDir, "SKILL.md"), - `--- -name: ${params.name} -description: test skill -metadata: ${JSON.stringify(meta)} ---- - -# ${params.name} -`, - "utf-8", - ); - await fs.writeFile(path.join(skillDir, "runner.js"), "export {};\n", "utf-8"); - return skillDir; -} - -async function fileExists(filePath: string): Promise { - try { - await fs.stat(filePath); - return true; - } catch { - return false; - } -} - describe("installSkill code safety scanning", () => { beforeEach(() => { runCommandWithTimeoutMock.mockReset(); scanDirectoryWithSummaryMock.mockReset(); - fetchWithSsrFGuardMock.mockReset(); runCommandWithTimeoutMock.mockResolvedValue({ code: 0, stdout: "ok", @@ -171,346 +112,3 @@ describe("installSkill code safety scanning", () => { } }); }); - -describe("installSkill download extraction safety", () => { - beforeEach(() => { - runCommandWithTimeoutMock.mockReset(); - scanDirectoryWithSummaryMock.mockReset(); - fetchWithSsrFGuardMock.mockReset(); - scanDirectoryWithSummaryMock.mockResolvedValue({ - scannedFiles: 0, - critical: 0, - warn: 0, - info: 0, - findings: [], - }); - }); - - it("rejects zip slip traversal", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const targetDir = path.join(workspaceDir, "target"); - const outsideWriteDir = path.join(workspaceDir, "outside-write"); - const outsideWritePath = path.join(outsideWriteDir, "pwned.txt"); - const url = "https://example.invalid/evil.zip"; - - const zip = new JSZip(); - zip.file("../outside-write/pwned.txt", "pwnd"); - const buffer = await zip.generateAsync({ type: "nodebuffer" }); - - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(buffer, { status: 200 }), - release: async () => undefined, - }); - - await writeDownloadSkill({ - workspaceDir, - name: "zip-slip", - installId: "dl", - url, - archive: "zip", - targetDir, - }); - - 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 targetDir = path.join(workspaceDir, "target"); - const insideDir = path.join(workspaceDir, "inside"); - const outsideWriteDir = path.join(workspaceDir, "outside-write"); - const outsideWritePath = path.join(outsideWriteDir, "pwned.txt"); - const archivePath = path.join(workspaceDir, "evil.tgz"); - const url = "https://example.invalid/evil"; - - await fs.mkdir(insideDir, { recursive: true }); - await fs.mkdir(outsideWriteDir, { recursive: true }); - await fs.writeFile(outsideWritePath, "pwnd", "utf-8"); - - await tar.c({ cwd: insideDir, file: archivePath, gzip: true }, [ - "../outside-write/pwned.txt", - ]); - await fs.rm(outsideWriteDir, { recursive: true, force: true }); - - const buffer = await fs.readFile(archivePath); - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(buffer, { status: 200 }), - release: async () => undefined, - }); - - await writeDownloadSkill({ - workspaceDir, - name: "tar-slip", - installId: "dl", - url, - archive: "tar.gz", - targetDir, - }); - - 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 targetDir = path.join(workspaceDir, "target"); - const url = "https://example.invalid/good.zip"; - - const zip = new JSZip(); - zip.file("package/hello.txt", "hi"); - const buffer = await zip.generateAsync({ type: "nodebuffer" }); - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(buffer, { status: 200 }), - release: async () => undefined, - }); - - await writeDownloadSkill({ - workspaceDir, - name: "zip-good", - installId: "dl", - url, - archive: "zip", - stripComponents: 1, - targetDir, - }); - - 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 tar.bz2 traversal before extraction", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const targetDir = path.join(workspaceDir, "target"); - const url = "https://example.invalid/evil.tbz2"; - - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), - release: async () => undefined, - }); - - runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { - const cmd = argv as string[]; - if (cmd[0] === "tar" && cmd[1] === "tf") { - return { code: 0, stdout: "../outside.txt\n", stderr: "", signal: null, killed: false }; - } - if (cmd[0] === "tar" && cmd[1] === "tvf") { - return { - code: 0, - stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 ../outside.txt\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "xf") { - throw new Error("should not extract"); - } - return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; - }); - - await writeDownloadSkill({ - workspaceDir, - name: "tbz2-slip", - installId: "dl", - url, - archive: "tar.bz2", - targetDir, - }); - - const result = await installSkill({ workspaceDir, skillName: "tbz2-slip", installId: "dl" }); - expect(result.ok).toBe(false); - expect( - runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), - ).toBe(false); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } - }); - - it("rejects tar.bz2 archives containing symlinks", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const targetDir = path.join(workspaceDir, "target"); - const url = "https://example.invalid/evil.tbz2"; - - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), - release: async () => undefined, - }); - - runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { - const cmd = argv as string[]; - if (cmd[0] === "tar" && cmd[1] === "tf") { - return { - code: 0, - stdout: "link\nlink/pwned.txt\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "tvf") { - return { - code: 0, - stdout: "lrwxr-xr-x 0 0 0 0 Jan 1 00:00 link -> ../outside\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "xf") { - throw new Error("should not extract"); - } - return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; - }); - - await writeDownloadSkill({ - workspaceDir, - name: "tbz2-symlink", - installId: "dl", - url, - archive: "tar.bz2", - targetDir, - }); - - const result = await installSkill({ - workspaceDir, - skillName: "tbz2-symlink", - installId: "dl", - }); - expect(result.ok).toBe(false); - expect(result.stderr.toLowerCase()).toContain("link"); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } - }); - - it("extracts tar.bz2 with stripComponents safely (preflight only)", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const targetDir = path.join(workspaceDir, "target"); - const url = "https://example.invalid/good.tbz2"; - - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), - release: async () => undefined, - }); - - runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { - const cmd = argv as string[]; - if (cmd[0] === "tar" && cmd[1] === "tf") { - return { - code: 0, - stdout: "package/hello.txt\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "tvf") { - return { - code: 0, - stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 package/hello.txt\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "xf") { - return { code: 0, stdout: "ok", stderr: "", signal: null, killed: false }; - } - return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; - }); - - await writeDownloadSkill({ - workspaceDir, - name: "tbz2-ok", - installId: "dl", - url, - archive: "tar.bz2", - stripComponents: 1, - targetDir, - }); - - const result = await installSkill({ workspaceDir, skillName: "tbz2-ok", installId: "dl" }); - expect(result.ok).toBe(true); - expect( - runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), - ).toBe(true); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } - }); - - it("rejects tar.bz2 stripComponents escape", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); - try { - const targetDir = path.join(workspaceDir, "target"); - const url = "https://example.invalid/evil.tbz2"; - - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), - release: async () => undefined, - }); - - runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { - const cmd = argv as string[]; - if (cmd[0] === "tar" && cmd[1] === "tf") { - return { code: 0, stdout: "a/../b.txt\n", stderr: "", signal: null, killed: false }; - } - if (cmd[0] === "tar" && cmd[1] === "tvf") { - return { - code: 0, - stdout: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 a/../b.txt\n", - stderr: "", - signal: null, - killed: false, - }; - } - if (cmd[0] === "tar" && cmd[1] === "xf") { - throw new Error("should not extract"); - } - return { code: 0, stdout: "", stderr: "", signal: null, killed: false }; - }); - - await writeDownloadSkill({ - workspaceDir, - name: "tbz2-strip-escape", - installId: "dl", - url, - archive: "tar.bz2", - stripComponents: 1, - targetDir, - }); - - const result = await installSkill({ - workspaceDir, - skillName: "tbz2-strip-escape", - installId: "dl", - }); - expect(result.ok).toBe(false); - expect( - runCommandWithTimeoutMock.mock.calls.some((call) => (call[0] as string[])[1] === "xf"), - ).toBe(false); - } finally { - await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); - } - }); -}); diff --git a/src/infra/install-safe-path.test.ts b/src/infra/install-safe-path.test.ts new file mode 100644 index 00000000000..1d6b9b6e4e5 --- /dev/null +++ b/src/infra/install-safe-path.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { safePathSegmentHashed } from "./install-safe-path.js"; + +describe("safePathSegmentHashed", () => { + it("keeps safe names unchanged", () => { + expect(safePathSegmentHashed("demo-skill")).toBe("demo-skill"); + }); + + it("normalizes separators and adds hash suffix", () => { + const result = safePathSegmentHashed("../../demo/skill"); + expect(result.includes("/")).toBe(false); + expect(result.includes("\\")).toBe(false); + expect(result).toMatch(/-[a-f0-9]{10}$/); + }); + + it("hashes long names while staying bounded", () => { + const long = "a".repeat(100); + const result = safePathSegmentHashed(long); + expect(result.length).toBeLessThanOrEqual(61); + expect(result).toMatch(/-[a-f0-9]{10}$/); + }); +}); diff --git a/src/infra/path-safety.test.ts b/src/infra/path-safety.test.ts new file mode 100644 index 00000000000..b05eeced172 --- /dev/null +++ b/src/infra/path-safety.test.ts @@ -0,0 +1,16 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { isWithinDir, resolveSafeBaseDir } from "./path-safety.js"; + +describe("path-safety", () => { + it("resolves safe base dir with trailing separator", () => { + const base = resolveSafeBaseDir("/tmp/demo"); + expect(base.endsWith(path.sep)).toBe(true); + }); + + it("checks directory containment", () => { + expect(isWithinDir("/tmp/demo", "/tmp/demo")).toBe(true); + expect(isWithinDir("/tmp/demo", "/tmp/demo/sub/file.txt")).toBe(true); + expect(isWithinDir("/tmp/demo", "/tmp/demo/../escape.txt")).toBe(false); + }); +});