fix(plugins): guard marketplace archive downloads (#58267)

* Plugins: guard marketplace archive downloads

* Plugins: harden marketplace download cleanup

* Plugins: bound marketplace archive downloads

* Plugins: harden marketplace archive failures

* Plugins: reject drive-relative marketplace archives

* Plugins: stream marketplace archive downloads
This commit is contained in:
Jacob Tomlinson 2026-03-31 04:59:42 -07:00 committed by GitHub
parent 607076d164
commit 2ce44ca6a1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 592 additions and 35 deletions

View File

@ -55,6 +55,16 @@ async function withTempDir<T>(fn: (dir: string) => Promise<T>): Promise<T> {
}
}
async function listMarketplaceDownloadTempDirs(): Promise<string[]> {
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<string> {
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();
});

View File

@ -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<Uint8Array> } {
return Boolean(
response.body && typeof (response.body as { getReader?: unknown }).getReader === "function",
);
}
async function readMarketplaceChunkWithTimeout(
reader: ReadableStreamDefaultReader<Uint8Array>,
chunkTimeoutMs: number,
): Promise<Awaited<ReturnType<typeof reader.read>>> {
let timeoutId: ReturnType<typeof setTimeout> | 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<ReturnType<typeof fs.open>>,
chunk: Uint8Array,
): Promise<void> {
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<Uint8Array> };
targetPath: string;
maxBytes: number;
chunkTimeoutMs: number;
}): Promise<void> {
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)) {