fix: restrict remote marketplace plugin sources

This commit is contained in:
Peter Steinberger
2026-03-22 22:44:55 -07:00
parent 09faed6bd8
commit c036e4d176
4 changed files with 250 additions and 3 deletions

View File

@@ -5,11 +5,16 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { withEnvAsync } from "../test-utils/env.js";
const installPluginFromPathMock = vi.fn();
const runCommandWithTimeoutMock = vi.hoisted(() => vi.fn());
vi.mock("./install.js", () => ({
installPluginFromPath: (...args: unknown[]) => installPluginFromPathMock(...args),
}));
vi.mock("../process/exec.js", () => ({
runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args),
}));
async function withTempDir<T>(fn: (dir: string) => Promise<T>): Promise<T> {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-marketplace-test-"));
try {
@@ -22,6 +27,7 @@ async function withTempDir<T>(fn: (dir: string) => Promise<T>): Promise<T> {
describe("marketplace plugins", () => {
afterEach(() => {
installPluginFromPathMock.mockReset();
runCommandWithTimeoutMock.mockReset();
});
it("lists plugins from a local marketplace root", async () => {
@@ -141,4 +147,159 @@ describe("marketplace plugins", () => {
});
});
});
it("installs remote marketplace plugins from relative paths inside the cloned repo", async () => {
runCommandWithTimeoutMock.mockImplementationOnce(async (argv: string[]) => {
const repoDir = argv.at(-1);
expect(typeof repoDir).toBe("string");
await fs.mkdir(path.join(repoDir as string, ".claude-plugin"), { recursive: true });
await fs.mkdir(path.join(repoDir as string, "plugins", "frontend-design"), {
recursive: true,
});
await fs.writeFile(
path.join(repoDir as string, ".claude-plugin", "marketplace.json"),
JSON.stringify({
plugins: [
{
name: "frontend-design",
source: "./plugins/frontend-design",
},
],
}),
);
return { code: 0, stdout: "", stderr: "", killed: false };
});
installPluginFromPathMock.mockResolvedValue({
ok: true,
pluginId: "frontend-design",
targetDir: "/tmp/frontend-design",
version: "0.1.0",
extensions: ["index.ts"],
});
const { installPluginFromMarketplace } = await import("./marketplace.js");
const result = await installPluginFromMarketplace({
marketplace: "owner/repo",
plugin: "frontend-design",
});
expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1);
expect(runCommandWithTimeoutMock).toHaveBeenCalledWith(
["git", "clone", "--depth", "1", "https://github.com/owner/repo.git", expect.any(String)],
{ timeoutMs: 120_000 },
);
expect(installPluginFromPathMock).toHaveBeenCalledWith(
expect.objectContaining({
path: expect.stringMatching(/[\\/]repo[\\/]plugins[\\/]frontend-design$/),
}),
);
expect(result).toMatchObject({
ok: true,
pluginId: "frontend-design",
marketplacePlugin: "frontend-design",
marketplaceSource: "owner/repo",
});
});
it("rejects remote marketplace git plugin sources before cloning nested remotes", async () => {
runCommandWithTimeoutMock.mockImplementationOnce(async (argv: string[]) => {
const repoDir = argv.at(-1);
expect(typeof repoDir).toBe("string");
await fs.mkdir(path.join(repoDir as string, ".claude-plugin"), { recursive: true });
await fs.writeFile(
path.join(repoDir as string, ".claude-plugin", "marketplace.json"),
JSON.stringify({
plugins: [
{
name: "frontend-design",
source: {
type: "git",
url: "https://evil.example/repo.git",
},
},
],
}),
);
return { code: 0, stdout: "", stderr: "", killed: false };
});
const { listMarketplacePlugins } = await import("./marketplace.js");
const result = await listMarketplacePlugins({ marketplace: "owner/repo" });
expect(result).toEqual({
ok: false,
error:
'invalid marketplace entry "frontend-design" in owner/repo: ' +
"remote marketplaces may not use git plugin sources",
});
expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1);
});
it("rejects remote marketplace absolute plugin paths", async () => {
runCommandWithTimeoutMock.mockImplementationOnce(async (argv: string[]) => {
const repoDir = argv.at(-1);
expect(typeof repoDir).toBe("string");
await fs.mkdir(path.join(repoDir as string, ".claude-plugin"), { recursive: true });
await fs.writeFile(
path.join(repoDir as string, ".claude-plugin", "marketplace.json"),
JSON.stringify({
plugins: [
{
name: "frontend-design",
source: {
type: "path",
path: "/tmp/frontend-design",
},
},
],
}),
);
return { code: 0, stdout: "", stderr: "", killed: false };
});
const { listMarketplacePlugins } = await import("./marketplace.js");
const result = await listMarketplacePlugins({ marketplace: "owner/repo" });
expect(result).toEqual({
ok: false,
error:
'invalid marketplace entry "frontend-design" in owner/repo: ' +
"remote marketplaces may only use relative plugin paths",
});
expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1);
});
it("rejects remote marketplace HTTP plugin paths", async () => {
runCommandWithTimeoutMock.mockImplementationOnce(async (argv: string[]) => {
const repoDir = argv.at(-1);
expect(typeof repoDir).toBe("string");
await fs.mkdir(path.join(repoDir as string, ".claude-plugin"), { recursive: true });
await fs.writeFile(
path.join(repoDir as string, ".claude-plugin", "marketplace.json"),
JSON.stringify({
plugins: [
{
name: "frontend-design",
source: {
type: "path",
path: "https://evil.example/plugin.tgz",
},
},
],
}),
);
return { code: 0, stdout: "", stderr: "", killed: false };
});
const { listMarketplacePlugins } = await import("./marketplace.js");
const result = await listMarketplacePlugins({ marketplace: "owner/repo" });
expect(result).toEqual({
ok: false,
error:
'invalid marketplace entry "frontend-design" in owner/repo: ' +
"remote marketplaces may not use HTTP(S) plugin paths",
});
expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1);
});
});

View File

@@ -51,6 +51,8 @@ type LoadedMarketplace = {
cleanup?: () => Promise<void>;
};
type MarketplaceManifestOrigin = "local" | "remote";
type KnownMarketplaceRecord = {
installLocation?: string;
source?: unknown;
@@ -473,10 +475,19 @@ async function loadMarketplace(params: {
if (!parsed.ok) {
return parsed;
}
const validated = validateMarketplaceManifest({
manifest: parsed.manifest,
sourceLabel: local.manifestPath,
rootDir: local.rootDir,
origin: "local",
});
if (!validated.ok) {
return validated;
}
return {
ok: true,
marketplace: {
manifest: parsed.manifest,
manifest: validated.manifest,
rootDir: local.rootDir,
sourceLabel: params.source,
},
@@ -505,10 +516,19 @@ async function loadMarketplace(params: {
if (!parsed.ok) {
return parsed;
}
const validated = validateMarketplaceManifest({
manifest: parsed.manifest,
sourceLabel: local.manifestPath,
rootDir: local.rootDir,
origin: "local",
});
if (!validated.ok) {
return validated;
}
return {
ok: true,
marketplace: {
manifest: parsed.manifest,
manifest: validated.manifest,
rootDir: local.rootDir,
sourceLabel: local.manifestPath,
},
@@ -543,11 +563,21 @@ async function loadMarketplace(params: {
await cloned.cleanup();
return parsed;
}
const validated = validateMarketplaceManifest({
manifest: parsed.manifest,
sourceLabel: cloned.label,
rootDir: cloned.rootDir,
origin: "remote",
});
if (!validated.ok) {
await cloned.cleanup();
return validated;
}
return {
ok: true,
marketplace: {
manifest: parsed.manifest,
manifest: validated.manifest,
rootDir: cloned.rootDir,
sourceLabel: cloned.label,
cleanup: cloned.cleanup,
@@ -600,6 +630,56 @@ function ensureInsideMarketplaceRoot(
return { ok: true, path: resolved };
}
function validateMarketplaceManifest(params: {
manifest: MarketplaceManifest;
sourceLabel: string;
rootDir: string;
origin: MarketplaceManifestOrigin;
}): { ok: true; manifest: MarketplaceManifest } | { ok: false; error: string } {
if (params.origin === "local") {
return { ok: true, manifest: params.manifest };
}
for (const plugin of params.manifest.plugins) {
const source = plugin.source;
if (source.kind === "path") {
if (isHttpUrl(source.path)) {
return {
ok: false,
error:
`invalid marketplace entry "${plugin.name}" in ${params.sourceLabel}: ` +
"remote marketplaces may not use HTTP(S) plugin paths",
};
}
if (path.isAbsolute(source.path)) {
return {
ok: false,
error:
`invalid marketplace entry "${plugin.name}" in ${params.sourceLabel}: ` +
"remote marketplaces may only use relative plugin paths",
};
}
const resolved = ensureInsideMarketplaceRoot(params.rootDir, source.path);
if (!resolved.ok) {
return {
ok: false,
error: `invalid marketplace entry "${plugin.name}" in ${params.sourceLabel}: ${resolved.error}`,
};
}
continue;
}
return {
ok: false,
error:
`invalid marketplace entry "${plugin.name}" in ${params.sourceLabel}: ` +
`remote marketplaces may not use ${source.kind} plugin sources`,
};
}
return { ok: true, manifest: params.manifest };
}
async function resolveMarketplaceEntryInstallPath(params: {
source: MarketplaceEntrySource;
marketplaceRootDir: string;