diff --git a/src/plugins/marketplace.test.ts b/src/plugins/marketplace.test.ts index f0f805a4ec6..ee98ca0f5c1 100644 --- a/src/plugins/marketplace.test.ts +++ b/src/plugins/marketplace.test.ts @@ -55,6 +55,16 @@ async function withTempDir(fn: (dir: string) => Promise): Promise { } } +async function listMarketplaceDownloadTempDirs(): Promise { + const entries = await fs.readdir(os.tmpdir(), { withFileTypes: true }); + return entries + .filter( + (entry) => entry.isDirectory() && entry.name.startsWith("openclaw-marketplace-download-"), + ) + .map((entry) => entry.name) + .toSorted(); +} + async function writeMarketplaceManifest(rootDir: string, manifest: unknown): Promise { const manifestPath = path.join(rootDir, ".claude-plugin", "marketplace.json"); await fs.mkdir(path.dirname(manifestPath), { recursive: true }); @@ -297,10 +307,12 @@ describe("marketplace plugins", () => { it("returns a structured error for archive downloads with an empty response body", async () => { await withTempDir(async (rootDir) => { - vi.stubGlobal( - "fetch", - vi.fn(async () => new Response(null, { status: 200 })), - ); + const release = vi.fn(async () => undefined); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: new Response(null, { status: 200 }), + finalUrl: "https://example.com/frontend-design.tgz", + release, + }); const manifestPath = await writeMarketplaceManifest(rootDir, { plugins: [ { @@ -319,9 +331,373 @@ describe("marketplace plugins", () => { ok: false, error: "failed to download https://example.com/frontend-design.tgz: empty response body", }); - expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith({ - url: "https://example.com/frontend-design.tgz", - auditContext: "marketplace-plugin-download", + expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://example.com/frontend-design.tgz", + timeoutMs: 120_000, + auditContext: "marketplace-plugin-download", + }), + ); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + expect(release).toHaveBeenCalledTimes(1); + }); + }); + + it("returns a structured error for invalid archive URLs", async () => { + await withTempDir(async (rootDir) => { + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://%/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: "failed to download https://%/frontend-design.tgz: Invalid URL", + }); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled(); + }); + }); + + it("rejects Windows drive-relative archive filenames from redirects", async () => { + await withTempDir(async (rootDir) => { + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: new Response(new Blob([Buffer.from("tgz-bytes")]), { + status: 200, + }), + finalUrl: "https://cdn.example.com/C:plugin.tgz", + release: vi.fn(async () => undefined), + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: + "failed to download https://example.com/frontend-design.tgz: invalid download filename", + }); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + }); + }); + + it("falls back to the default archive timeout when the caller passes NaN", async () => { + await withTempDir(async (rootDir) => { + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: new Response(new Blob([Buffer.from("tgz-bytes")]), { + status: 200, + }), + finalUrl: "https://cdn.example.com/releases/12345", + release: vi.fn(async () => undefined), + }); + installPluginFromPathMock.mockResolvedValue({ + ok: true, + pluginId: "frontend-design", + targetDir: "/tmp/frontend-design", + version: "0.1.0", + extensions: ["index.ts"], + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + timeoutMs: Number.NaN, + }); + + expect(result).toMatchObject({ + ok: true, + pluginId: "frontend-design", + }); + expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://example.com/frontend-design.tgz", + timeoutMs: 120_000, + auditContext: "marketplace-plugin-download", + }), + ); + }); + }); + + it("downloads archive plugin sources through the SSRF guard", async () => { + await withTempDir(async (rootDir) => { + const release = vi.fn(async () => { + throw new Error("dispatcher close failed"); + }); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: new Response(new Blob([Buffer.from("tgz-bytes")]), { + status: 200, + }), + finalUrl: "https://cdn.example.com/releases/12345", + release, + }); + installPluginFromPathMock.mockResolvedValue({ + ok: true, + pluginId: "frontend-design", + targetDir: "/tmp/frontend-design", + version: "0.1.0", + extensions: ["index.ts"], + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toMatchObject({ + ok: true, + pluginId: "frontend-design", + marketplacePlugin: "frontend-design", + marketplaceSource: manifestPath, + }); + expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://example.com/frontend-design.tgz", + timeoutMs: 120_000, + auditContext: "marketplace-plugin-download", + }), + ); + expect(installPluginFromPathMock).toHaveBeenCalledWith( + expect.objectContaining({ + path: expect.stringMatching(/[\\/]frontend-design\.tgz$/), + }), + ); + expect(release).toHaveBeenCalledTimes(1); + }); + }); + + it("rejects non-streaming archive responses before buffering them", async () => { + await withTempDir(async (rootDir) => { + const arrayBuffer = vi.fn(async () => new Uint8Array([1, 2, 3]).buffer); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: { + ok: true, + status: 200, + body: {} as Response["body"], + headers: new Headers(), + arrayBuffer, + } as unknown as Response, + finalUrl: "https://cdn.example.com/releases/frontend-design.tgz", + release: vi.fn(async () => undefined), + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: + "failed to download https://example.com/frontend-design.tgz: " + + "streaming response body unavailable", + }); + expect(arrayBuffer).not.toHaveBeenCalled(); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + }); + }); + + it("rejects oversized streamed archive responses without falling back to arrayBuffer", async () => { + await withTempDir(async (rootDir) => { + const arrayBuffer = vi.fn(async () => new Uint8Array([1, 2, 3]).buffer); + const reader = { + read: vi + .fn() + .mockResolvedValueOnce({ + done: false, + value: { + length: 256 * 1024 * 1024 + 1, + } as Uint8Array, + }) + .mockResolvedValueOnce({ done: true, value: undefined }), + cancel: vi.fn(async () => undefined), + releaseLock: vi.fn(), + }; + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: { + ok: true, + status: 200, + body: { + getReader: () => reader, + } as unknown as Response["body"], + headers: new Headers(), + arrayBuffer, + } as unknown as Response, + finalUrl: "https://cdn.example.com/releases/frontend-design.tgz", + release: vi.fn(async () => undefined), + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: + "failed to download https://example.com/frontend-design.tgz: " + + "download too large: 268435457 bytes (limit: 268435456 bytes)", + }); + expect(arrayBuffer).not.toHaveBeenCalled(); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + }); + }); + + it("cleans up a partial download temp dir when streaming the archive fails", async () => { + await withTempDir(async (rootDir) => { + const beforeTempDirs = await listMarketplaceDownloadTempDirs(); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: new Response("x".repeat(1024), { + status: 200, + headers: { + "content-length": String(300 * 1024 * 1024), + }, + }), + finalUrl: "https://cdn.example.com/releases/frontend-design.tgz", + release: vi.fn(async () => undefined), + }); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: + "failed to download https://example.com/frontend-design.tgz: " + + "download too large: 314572800 bytes (limit: 268435456 bytes)", + }); + expect(await listMarketplaceDownloadTempDirs()).toEqual(beforeTempDirs); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + }); + }); + + it("sanitizes archive download errors before returning them", async () => { + await withTempDir(async (rootDir) => { + fetchWithSsrFGuardMock.mockRejectedValueOnce( + new Error( + "blocked\n\u001b[31mAuthorization: Bearer sk-1234567890abcdefghijklmnop\u001b[0m", + ), + ); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://user:pass@example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain( + "failed to download https://***:***@example.com/frontend-design.tgz:", + ); + expect(result.error).toContain("Authorization: Bearer sk-123…mnop"); + expect(result.error).not.toContain("user:pass@"); + let hasControlChars = false; + for (const char of result.error) { + const codePoint = char.codePointAt(0); + if (codePoint != null && (codePoint < 0x20 || codePoint === 0x7f)) { + hasControlChars = true; + break; + } + } + expect(hasControlChars).toBe(false); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + }); + }); + + it("returns a structured error when the SSRF guard rejects an archive URL", async () => { + await withTempDir(async (rootDir) => { + fetchWithSsrFGuardMock.mockRejectedValueOnce( + new Error("Blocked hostname (not in allowlist): 169.254.169.254"), + ); + const manifestPath = await writeMarketplaceManifest(rootDir, { + plugins: [ + { + name: "frontend-design", + source: "https://example.com/frontend-design.tgz", + }, + ], + }); + + const result = await installPluginFromMarketplace({ + marketplace: manifestPath, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: + "failed to download https://example.com/frontend-design.tgz: " + + "Blocked hostname (not in allowlist): 169.254.169.254", }); expect(installPluginFromPathMock).not.toHaveBeenCalled(); }); diff --git a/src/plugins/marketplace.ts b/src/plugins/marketplace.ts index acb9c8c7013..5e8556e35b4 100644 --- a/src/plugins/marketplace.ts +++ b/src/plugins/marketplace.ts @@ -1,16 +1,19 @@ -import { createWriteStream } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { Writable } from "node:stream"; import { resolveArchiveKind } from "../infra/archive.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { resolveOsHomeRelativePath } from "../infra/home-dir.js"; import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; import { runCommandWithTimeout } from "../process/exec.js"; +import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js"; +import { sanitizeForLog } from "../terminal/ansi.js"; import { resolveUserPath } from "../utils.js"; import { installPluginFromPath, type InstallPluginResult } from "./install.js"; const DEFAULT_GIT_TIMEOUT_MS = 120_000; +const DEFAULT_MARKETPLACE_DOWNLOAD_TIMEOUT_MS = 120_000; +const MAX_MARKETPLACE_ARCHIVE_BYTES = 256 * 1024 * 1024; const MARKETPLACE_MANIFEST_CANDIDATES = [ path.join(".claude-plugin", "marketplace.json"), "marketplace.json", @@ -579,7 +582,138 @@ async function loadMarketplace(params: { }; } -async function downloadUrlToTempFile(url: string): Promise< +function resolveSafeMarketplaceDownloadFileName(url: string, fallback: string): string { + const pathname = new URL(url).pathname; + const fileName = path.basename(pathname).trim() || fallback; + if ( + fileName === "." || + fileName === ".." || + /^[a-zA-Z]:/.test(fileName) || + path.isAbsolute(fileName) || + fileName.includes("/") || + fileName.includes("\\") + ) { + throw new Error("invalid download filename"); + } + return fileName; +} + +function resolveMarketplaceDownloadTimeoutMs(timeoutMs?: number): number { + const resolvedTimeoutMs = + typeof timeoutMs === "number" && Number.isFinite(timeoutMs) + ? timeoutMs + : DEFAULT_MARKETPLACE_DOWNLOAD_TIMEOUT_MS; + return Math.max(1_000, Math.floor(resolvedTimeoutMs)); +} + +function formatMarketplaceDownloadError(url: string, detail: string): string { + return ( + `failed to download ${sanitizeForLog(redactSensitiveUrlLikeString(url))}: ` + + sanitizeForLog(detail) + ); +} + +function hasStreamingResponseBody( + response: Response, +): response is Response & { body: ReadableStream } { + return Boolean( + response.body && typeof (response.body as { getReader?: unknown }).getReader === "function", + ); +} + +async function readMarketplaceChunkWithTimeout( + reader: ReadableStreamDefaultReader, + chunkTimeoutMs: number, +): Promise>> { + let timeoutId: ReturnType | undefined; + let timedOut = false; + + return await new Promise((resolve, reject) => { + const clear = () => { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + }; + + timeoutId = setTimeout(() => { + timedOut = true; + clear(); + void reader.cancel().catch(() => undefined); + reject(new Error(`download timed out after ${chunkTimeoutMs}ms`)); + }, chunkTimeoutMs); + + void reader.read().then( + (result) => { + clear(); + if (!timedOut) { + resolve(result); + } + }, + (err) => { + clear(); + if (!timedOut) { + reject(err); + } + }, + ); + }); +} + +async function writeMarketplaceChunk( + fileHandle: Awaited>, + chunk: Uint8Array, +): Promise { + let offset = 0; + while (offset < chunk.length) { + const { bytesWritten } = await fileHandle.write(chunk, offset, chunk.length - offset); + if (bytesWritten <= 0) { + throw new Error("failed to write download chunk"); + } + offset += bytesWritten; + } +} + +async function streamMarketplaceResponseToFile(params: { + response: Response & { body: ReadableStream }; + targetPath: string; + maxBytes: number; + chunkTimeoutMs: number; +}): Promise { + const reader = params.response.body.getReader(); + const fileHandle = await fs.open(params.targetPath, "wx"); + let total = 0; + + try { + while (true) { + const { done, value } = await readMarketplaceChunkWithTimeout(reader, params.chunkTimeoutMs); + if (done) { + return; + } + if (!value?.length) { + continue; + } + + const nextTotal = total + value.length; + if (nextTotal > params.maxBytes) { + throw new Error(`download too large: ${nextTotal} bytes (limit: ${params.maxBytes} bytes)`); + } + + await writeMarketplaceChunk(fileHandle, value); + total = nextTotal; + } + } finally { + await fileHandle.close().catch(() => undefined); + try { + reader.releaseLock(); + } catch {} + } +} + +async function downloadUrlToTempFile( + url: string, + timeoutMs?: number, +): Promise< | { ok: true; path: string; @@ -590,33 +724,80 @@ async function downloadUrlToTempFile(url: string): Promise< error: string; } > { - const { response, release } = await fetchWithSsrFGuard({ - url, - auditContext: "marketplace-plugin-download", - }); + let sourceFileName = "plugin.tgz"; + let tmpDir: string | undefined; try { - if (!response.ok) { - return { ok: false, error: `failed to download ${url}: HTTP ${response.status}` }; - } - if (!response.body) { - return { ok: false, error: `failed to download ${url}: empty response body` }; - } + sourceFileName = resolveSafeMarketplaceDownloadFileName(url, sourceFileName); + const downloadTimeoutMs = resolveMarketplaceDownloadTimeoutMs(timeoutMs); + const { response, finalUrl, release } = await fetchWithSsrFGuard({ + url, + timeoutMs: downloadTimeoutMs, + auditContext: "marketplace-plugin-download", + }); + try { + if (!response.ok) { + return { + ok: false, + error: formatMarketplaceDownloadError(url, `HTTP ${response.status}`), + }; + } + if (!response.body) { + return { + ok: false, + error: formatMarketplaceDownloadError(url, "empty response body"), + }; + } + // Fail closed unless we can stream and enforce the archive size bound incrementally. + if (!hasStreamingResponseBody(response)) { + return { + ok: false, + error: formatMarketplaceDownloadError(url, "streaming response body unavailable"), + }; + } - const pathname = new URL(url).pathname; - const fileName = path.basename(pathname) || "plugin.tgz"; - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-marketplace-download-")); - const targetPath = path.join(tmpDir, fileName); - const fileStream = createWriteStream(targetPath); - await response.body.pipeTo(Writable.toWeb(fileStream)); + const contentLength = response.headers.get("content-length"); + if (contentLength) { + const size = Number(contentLength); + if (Number.isFinite(size) && size > MAX_MARKETPLACE_ARCHIVE_BYTES) { + throw new Error( + `download too large: ${size} bytes (limit: ${MAX_MARKETPLACE_ARCHIVE_BYTES} bytes)`, + ); + } + } + + const finalFileName = resolveSafeMarketplaceDownloadFileName(finalUrl, sourceFileName); + const fileName = resolveArchiveKind(finalFileName) ? finalFileName : sourceFileName; + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-marketplace-download-")); + const createdTmpDir = tmpDir; + const targetPath = path.resolve(createdTmpDir, fileName); + const relativeTargetPath = path.relative(createdTmpDir, targetPath); + if (relativeTargetPath === ".." || relativeTargetPath.startsWith(`..${path.sep}`)) { + throw new Error("invalid download filename"); + } + await streamMarketplaceResponseToFile({ + response, + targetPath, + maxBytes: MAX_MARKETPLACE_ARCHIVE_BYTES, + chunkTimeoutMs: downloadTimeoutMs, + }); + return { + ok: true, + path: targetPath, + cleanup: async () => { + await fs.rm(createdTmpDir, { recursive: true, force: true }).catch(() => undefined); + }, + }; + } finally { + await release().catch(() => undefined); + } + } catch (error) { + if (tmpDir) { + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined); + } return { - ok: true, - path: targetPath, - cleanup: async () => { - await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined); - }, + ok: false, + error: formatMarketplaceDownloadError(url, formatErrorMessage(error)), }; - } finally { - await release(); } } @@ -704,7 +885,7 @@ async function resolveMarketplaceEntryInstallPath(params: { if (params.source.kind === "path") { if (isHttpUrl(params.source.path)) { if (resolveArchiveKind(params.source.path)) { - return await downloadUrlToTempFile(params.source.path); + return await downloadUrlToTempFile(params.source.path, params.timeoutMs); } return { ok: false, @@ -754,7 +935,7 @@ async function resolveMarketplaceEntryInstallPath(params: { } if (resolveArchiveKind(params.source.url)) { - return await downloadUrlToTempFile(params.source.url); + return await downloadUrlToTempFile(params.source.url, params.timeoutMs); } if (!normalizeGitCloneSource(params.source.url)) {