From 6cd12ca1cefe396a9a73967979e0026000447fd5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 15:21:46 +0000 Subject: [PATCH] test: merge download archive safety suites --- .../skills-install.download-tarbz2.test.ts | 215 ---------------- src/agents/skills-install.download.test.ts | 243 +++++++++++++++--- 2 files changed, 213 insertions(+), 245 deletions(-) delete mode 100644 src/agents/skills-install.download-tarbz2.test.ts diff --git a/src/agents/skills-install.download-tarbz2.test.ts b/src/agents/skills-install.download-tarbz2.test.ts deleted file mode 100644 index 5795d786fd9..00000000000 --- a/src/agents/skills-install.download-tarbz2.test.ts +++ /dev/null @@ -1,215 +0,0 @@ -import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { withTempWorkspace, writeDownloadSkill } from "./skills-install.download-test-utils.js"; -import { installSkill } from "./skills-install.js"; - -const mocks = { - runCommand: vi.fn(), - scanSummary: vi.fn(), - fetchGuard: vi.fn(), -}; - -function mockDownloadResponse() { - mocks.fetchGuard.mockResolvedValue({ - response: new Response(new Uint8Array([1, 2, 3]), { status: 200 }), - release: async () => undefined, - }); -} - -function runCommandResult(params?: Partial>) { - return { - code: 0, - stdout: "", - stderr: "", - signal: null, - killed: false, - ...params, - }; -} - -function mockTarExtractionFlow(params: { - listOutput: string; - verboseListOutput: string; - extract: "ok" | "reject"; -}) { - mocks.runCommand.mockImplementation(async (argv: unknown[]) => { - const cmd = argv as string[]; - if (cmd[0] === "tar" && cmd[1] === "tf") { - return runCommandResult({ stdout: params.listOutput }); - } - if (cmd[0] === "tar" && cmd[1] === "tvf") { - return runCommandResult({ stdout: params.verboseListOutput }); - } - if (cmd[0] === "tar" && cmd[1] === "xf") { - if (params.extract === "reject") { - throw new Error("should not extract"); - } - return runCommandResult({ stdout: "ok" }); - } - return runCommandResult(); - }); -} - -async function writeTarBz2Skill(params: { - workspaceDir: string; - stateDir: string; - name: string; - url: string; - stripComponents?: number; -}) { - const targetDir = path.join(params.stateDir, "tools", params.name, "target"); - await writeDownloadSkill({ - workspaceDir: params.workspaceDir, - name: params.name, - installId: "dl", - url: params.url, - archive: "tar.bz2", - ...(typeof params.stripComponents === "number" - ? { stripComponents: params.stripComponents } - : {}), - targetDir, - }); -} - -vi.mock("../process/exec.js", () => ({ - runCommandWithTimeout: (...args: unknown[]) => mocks.runCommand(...args), -})); - -vi.mock("../infra/net/fetch-guard.js", () => ({ - fetchWithSsrFGuard: (...args: unknown[]) => mocks.fetchGuard(...args), -})); - -vi.mock("../security/skill-scanner.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - scanDirectoryWithSummary: (...args: unknown[]) => mocks.scanSummary(...args), - }; -}); - -describe("installSkill download extraction safety (tar.bz2)", () => { - beforeEach(() => { - mocks.runCommand.mockClear(); - mocks.scanSummary.mockClear(); - mocks.fetchGuard.mockClear(); - mocks.scanSummary.mockResolvedValue({ - scannedFiles: 0, - critical: 0, - warn: 0, - info: 0, - findings: [], - }); - }); - - it("rejects tar.bz2 traversal before extraction", async () => { - await withTempWorkspace(async ({ workspaceDir, stateDir }) => { - const url = "https://example.invalid/evil.tbz2"; - - mockDownloadResponse(); - mockTarExtractionFlow({ - listOutput: "../outside.txt\n", - verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 ../outside.txt\n", - extract: "reject", - }); - - await writeTarBz2Skill({ - workspaceDir, - stateDir, - name: "tbz2-slip", - url, - }); - - const result = await installSkill({ workspaceDir, skillName: "tbz2-slip", installId: "dl" }); - expect(result.ok).toBe(false); - expect(mocks.runCommand.mock.calls.some((call) => (call[0] as string[])[1] === "xf")).toBe( - false, - ); - }); - }); - - it("rejects tar.bz2 archives containing symlinks", async () => { - await withTempWorkspace(async ({ workspaceDir, stateDir }) => { - const url = "https://example.invalid/evil.tbz2"; - - mockDownloadResponse(); - mockTarExtractionFlow({ - listOutput: "link\nlink/pwned.txt\n", - verboseListOutput: "lrwxr-xr-x 0 0 0 0 Jan 1 00:00 link -> ../outside\n", - extract: "reject", - }); - - await writeTarBz2Skill({ - workspaceDir, - stateDir, - name: "tbz2-symlink", - url, - }); - - const result = await installSkill({ - workspaceDir, - skillName: "tbz2-symlink", - installId: "dl", - }); - expect(result.ok).toBe(false); - expect(result.stderr.toLowerCase()).toContain("link"); - }); - }); - - it("extracts tar.bz2 with stripComponents safely (preflight only)", async () => { - await withTempWorkspace(async ({ workspaceDir, stateDir }) => { - const url = "https://example.invalid/good.tbz2"; - - mockDownloadResponse(); - mockTarExtractionFlow({ - listOutput: "package/hello.txt\n", - verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 package/hello.txt\n", - extract: "ok", - }); - - await writeTarBz2Skill({ - workspaceDir, - stateDir, - name: "tbz2-ok", - url, - stripComponents: 1, - }); - - const result = await installSkill({ workspaceDir, skillName: "tbz2-ok", installId: "dl" }); - expect(result.ok).toBe(true); - expect(mocks.runCommand.mock.calls.some((call) => (call[0] as string[])[1] === "xf")).toBe( - true, - ); - }); - }); - - it("rejects tar.bz2 stripComponents escape", async () => { - await withTempWorkspace(async ({ workspaceDir, stateDir }) => { - const url = "https://example.invalid/evil.tbz2"; - - mockDownloadResponse(); - mockTarExtractionFlow({ - listOutput: "a/../b.txt\n", - verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 a/../b.txt\n", - extract: "reject", - }); - - await writeTarBz2Skill({ - workspaceDir, - stateDir, - name: "tbz2-strip-escape", - url, - stripComponents: 1, - }); - - const result = await installSkill({ - workspaceDir, - skillName: "tbz2-strip-escape", - installId: "dl", - }); - expect(result.ok).toBe(false); - expect(mocks.runCommand.mock.calls.some((call) => (call[0] as string[])[1] === "xf")).toBe( - false, - ); - }); - }); -}); diff --git a/src/agents/skills-install.download.test.ts b/src/agents/skills-install.download.test.ts index b566b53c78c..0281bc555a1 100644 --- a/src/agents/skills-install.download.test.ts +++ b/src/agents/skills-install.download.test.ts @@ -35,16 +35,70 @@ async function fileExists(filePath: string): Promise { } } -async function seedZipDownloadResponse() { +async function createZipBuffer( + entries: Array<{ name: string; contents: string }>, +): Promise { const zip = new JSZip(); - zip.file("hello.txt", "hi"); - const buffer = await zip.generateAsync({ type: "nodebuffer" }); + for (const entry of entries) { + zip.file(entry.name, entry.contents); + } + return zip.generateAsync({ type: "nodebuffer" }); +} + +const SAFE_ZIP_BUFFER_PROMISE = createZipBuffer([{ name: "hello.txt", contents: "hi" }]); +const STRIP_COMPONENTS_ZIP_BUFFER_PROMISE = createZipBuffer([ + { name: "package/hello.txt", contents: "hi" }, +]); +const ZIP_SLIP_BUFFER_PROMISE = createZipBuffer([ + { name: "../outside-write/pwned.txt", contents: "pwnd" }, +]); + +function mockArchiveResponse(buffer: Uint8Array): void { fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array(buffer), { status: 200 }), + response: new Response(buffer, { status: 200 }), release: async () => undefined, }); } +function runCommandResult(params?: Partial>) { + return { + code: 0, + stdout: "", + stderr: "", + signal: null, + killed: false, + ...params, + }; +} + +function mockTarExtractionFlow(params: { + listOutput: string; + verboseListOutput: string; + extract: "ok" | "reject"; +}) { + runCommandWithTimeoutMock.mockImplementation(async (argv: unknown[]) => { + const cmd = argv as string[]; + if (cmd[0] === "tar" && cmd[1] === "tf") { + return runCommandResult({ stdout: params.listOutput }); + } + if (cmd[0] === "tar" && cmd[1] === "tvf") { + return runCommandResult({ stdout: params.verboseListOutput }); + } + if (cmd[0] === "tar" && cmd[1] === "xf") { + if (params.extract === "reject") { + throw new Error("should not extract"); + } + return runCommandResult({ stdout: "ok" }); + } + return runCommandResult(); + }); +} + +async function seedZipDownloadResponse() { + const buffer = await SAFE_ZIP_BUFFER_PROMISE; + mockArchiveResponse(new Uint8Array(buffer)); +} + async function installZipDownloadSkill(params: { workspaceDir: string; name: string; @@ -68,6 +122,27 @@ async function installZipDownloadSkill(params: { }); } +async function writeTarBz2Skill(params: { + workspaceDir: string; + stateDir: string; + name: string; + url: string; + stripComponents?: number; +}) { + const targetDir = path.join(params.stateDir, "tools", params.name, "target"); + await writeDownloadSkill({ + workspaceDir: params.workspaceDir, + name: params.name, + installId: "dl", + url: params.url, + archive: "tar.bz2", + ...(typeof params.stripComponents === "number" + ? { stripComponents: params.stripComponents } + : {}), + targetDir, + }); +} + describe("installSkill download extraction safety", () => { beforeEach(() => { runCommandWithTimeoutMock.mockClear(); @@ -89,14 +164,8 @@ describe("installSkill download extraction safety", () => { 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(new Uint8Array(buffer), { status: 200 }), - release: async () => undefined, - }); + const buffer = await ZIP_SLIP_BUFFER_PROMISE; + mockArchiveResponse(new Uint8Array(buffer)); await writeDownloadSkill({ workspaceDir, @@ -132,10 +201,7 @@ describe("installSkill download extraction safety", () => { await fs.rm(outsideWriteDir, { recursive: true, force: true }); const buffer = await fs.readFile(archivePath); - fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(new Uint8Array(buffer), { status: 200 }), - release: async () => undefined, - }); + mockArchiveResponse(new Uint8Array(buffer)); await writeDownloadSkill({ workspaceDir, @@ -157,13 +223,8 @@ describe("installSkill download extraction safety", () => { 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(new Uint8Array(buffer), { status: 200 }), - release: async () => undefined, - }); + const buffer = await STRIP_COMPONENTS_ZIP_BUFFER_PROMISE; + mockArchiveResponse(new Uint8Array(buffer)); await writeDownloadSkill({ workspaceDir, @@ -186,13 +247,8 @@ describe("installSkill download extraction safety", () => { 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(new Uint8Array(buffer), { status: 200 }), - release: async () => undefined, - }); + const buffer = await SAFE_ZIP_BUFFER_PROMISE; + mockArchiveResponse(new Uint8Array(buffer)); await writeDownloadSkill({ workspaceDir, @@ -246,3 +302,130 @@ describe("installSkill download extraction safety", () => { }); }); }); + +describe("installSkill download extraction safety (tar.bz2)", () => { + beforeEach(() => { + runCommandWithTimeoutMock.mockClear(); + scanDirectoryWithSummaryMock.mockClear(); + fetchWithSsrFGuardMock.mockClear(); + scanDirectoryWithSummaryMock.mockResolvedValue({ + scannedFiles: 0, + critical: 0, + warn: 0, + info: 0, + findings: [], + }); + }); + + it("rejects tar.bz2 traversal before extraction", async () => { + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { + const url = "https://example.invalid/evil.tbz2"; + + mockArchiveResponse(new Uint8Array([1, 2, 3])); + mockTarExtractionFlow({ + listOutput: "../outside.txt\n", + verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 ../outside.txt\n", + extract: "reject", + }); + + await writeTarBz2Skill({ + workspaceDir, + stateDir, + name: "tbz2-slip", + url, + }); + + 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); + }); + }); + + it("rejects tar.bz2 archives containing symlinks", async () => { + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { + const url = "https://example.invalid/evil.tbz2"; + + mockArchiveResponse(new Uint8Array([1, 2, 3])); + mockTarExtractionFlow({ + listOutput: "link\nlink/pwned.txt\n", + verboseListOutput: "lrwxr-xr-x 0 0 0 0 Jan 1 00:00 link -> ../outside\n", + extract: "reject", + }); + + await writeTarBz2Skill({ + workspaceDir, + stateDir, + name: "tbz2-symlink", + url, + }); + + const result = await installSkill({ + workspaceDir, + skillName: "tbz2-symlink", + installId: "dl", + }); + expect(result.ok).toBe(false); + expect(result.stderr.toLowerCase()).toContain("link"); + }); + }); + + it("extracts tar.bz2 with stripComponents safely (preflight only)", async () => { + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { + const url = "https://example.invalid/good.tbz2"; + + mockArchiveResponse(new Uint8Array([1, 2, 3])); + mockTarExtractionFlow({ + listOutput: "package/hello.txt\n", + verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 package/hello.txt\n", + extract: "ok", + }); + + await writeTarBz2Skill({ + workspaceDir, + stateDir, + name: "tbz2-ok", + url, + stripComponents: 1, + }); + + 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); + }); + }); + + it("rejects tar.bz2 stripComponents escape", async () => { + await withTempWorkspace(async ({ workspaceDir, stateDir }) => { + const url = "https://example.invalid/evil.tbz2"; + + mockArchiveResponse(new Uint8Array([1, 2, 3])); + mockTarExtractionFlow({ + listOutput: "a/../b.txt\n", + verboseListOutput: "-rw-r--r-- 0 0 0 0 Jan 1 00:00 a/../b.txt\n", + extract: "reject", + }); + + await writeTarBz2Skill({ + workspaceDir, + stateDir, + name: "tbz2-strip-escape", + url, + stripComponents: 1, + }); + + 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); + }); + }); +});