From 87be38ea735bd7e008f07805bbc34ee12086d7fb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 1 Aug 2026 07:46:40 -0700 Subject: [PATCH] fix(plugins): resolve marketplace aliases without recursive cycles (#117440) Co-authored-by: Peter Steinberger --- src/plugins/marketplace.test.ts | 162 ++++++++++++++++++++++++++++++++ src/plugins/marketplace.ts | 34 ++++--- 2 files changed, 185 insertions(+), 11 deletions(-) diff --git a/src/plugins/marketplace.test.ts b/src/plugins/marketplace.test.ts index 6f7d6badac62..9aa3b3512bd7 100644 --- a/src/plugins/marketplace.test.ts +++ b/src/plugins/marketplace.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import * as jsonFiles from "../infra/json-files.js"; import { withEnvAsync } from "../test-utils/env.js"; import { withTempDir } from "../test-utils/temp-dir.js"; import { @@ -102,6 +103,22 @@ async function writeLocalMarketplaceFixture(params: { return writeMarketplaceManifest(params.rootDir, params.manifest); } +async function withKnownMarketplaceRegistry( + marketplaces: Record, + run: (homeDir: string) => Promise, +): Promise { + return await withTempDir("openclaw-marketplace-known-", async (homeDir) => { + const openClawHome = path.join(homeDir, "openclaw-home"); + const registryPath = path.join(homeDir, ".claude", "plugins", "known_marketplaces.json"); + await fs.mkdir(path.dirname(registryPath), { recursive: true }); + await fs.mkdir(openClawHome, { recursive: true }); + await fs.writeFile(registryPath, JSON.stringify(marketplaces)); + return await withEnvAsync({ HOME: homeDir, OPENCLAW_HOME: openClawHome }, async () => + run(homeDir), + ); + }); +} + function mockRemoteMarketplaceClone(params: { manifest: unknown; pluginDir?: string; @@ -533,6 +550,151 @@ describe("marketplace plugins", () => { }); }); + const cyclicKnownMarketplaces = [ + { + label: "self-referential", + marketplace: "loop", + marketplaces: { + loop: { source: { source: "path", path: "loop" } }, + }, + cycle: "loop -> loop", + }, + { + label: "two-marketplace", + marketplace: "alpha", + marketplaces: { + alpha: { source: { source: "path", path: "beta" } }, + beta: { source: { source: "path", path: "alpha" } }, + }, + cycle: "alpha -> beta -> alpha", + }, + ] as const; + + it.each(cyclicKnownMarketplaces)( + "rejects a $label known marketplace alias cycle while listing", + async ({ marketplace, marketplaces, cycle }) => { + await withKnownMarketplaceRegistry(marketplaces, async () => { + const registryRead = vi.spyOn(jsonFiles, "tryReadJson"); + for (let read = 0; read <= Object.keys(marketplaces).length; read += 1) { + registryRead.mockResolvedValueOnce(marketplaces); + } + // Bound the unfixed recursive loader so the regression never leaves a runaway task. + registryRead.mockResolvedValueOnce({}); + + try { + const result = await listMarketplacePlugins({ marketplace }); + + expect(result).toEqual({ + ok: false, + error: `known marketplace source cycle: ${cycle}`, + }); + expect(registryRead).toHaveBeenCalledTimes(1); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled(); + } finally { + registryRead.mockRestore(); + } + }); + }, + ); + + it.each(cyclicKnownMarketplaces)( + "rejects a $label known marketplace alias cycle before installing", + async ({ marketplace, marketplaces, cycle }) => { + await withKnownMarketplaceRegistry(marketplaces, async () => { + const registryRead = vi.spyOn(jsonFiles, "tryReadJson"); + for (let read = 0; read <= Object.keys(marketplaces).length; read += 1) { + registryRead.mockResolvedValueOnce(marketplaces); + } + // Bound the unfixed recursive loader so the lifecycle-owning call always settles. + registryRead.mockResolvedValueOnce({}); + + try { + const result = await installPluginFromMarketplace({ + marketplace, + plugin: "frontend-design", + }); + + expect(result).toEqual({ + ok: false, + error: `known marketplace source cycle: ${cycle}`, + }); + expect(registryRead).toHaveBeenCalledTimes(1); + expect(installPluginFromPathMock).not.toHaveBeenCalled(); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled(); + } finally { + registryRead.mockRestore(); + } + }); + }, + ); + + it("resolves independent known marketplace alias calls from one registry snapshot each", async () => { + await withKnownMarketplaceRegistry({}, async (homeDir) => { + const marketplaceRoot = path.join(homeDir, "known-marketplace"); + const pluginDir = path.join(marketplaceRoot, "plugins", "frontend-design"); + await writeLocalMarketplaceFixture({ + rootDir: marketplaceRoot, + pluginDir, + manifest: { + plugins: [{ name: "frontend-design", source: "./plugins/frontend-design" }], + }, + }); + await fs.writeFile( + path.join(homeDir, ".claude", "plugins", "known_marketplaces.json"), + JSON.stringify({ + alpha: { source: { source: "path", path: "beta" } }, + beta: { + installLocation: marketplaceRoot, + source: { source: "path", path: "alpha" }, + }, + }), + ); + installPluginFromPathMock.mockResolvedValue({ + ok: true, + pluginId: "frontend-design", + targetDir: "/tmp/frontend-design", + version: "0.1.0", + extensions: ["index.ts"], + }); + const registryRead = vi.spyOn(jsonFiles, "tryReadJson"); + + try { + const listed = await listMarketplacePlugins({ marketplace: "alpha" }); + const installed = await installPluginFromMarketplace({ + marketplace: "alpha", + plugin: "frontend-design", + }); + + expect(listed).toMatchObject({ ok: true, sourceLabel: "beta" }); + expectLocalMarketplaceInstallResult({ + result: installed, + pluginDir, + marketplaceSource: "alpha", + }); + expect(registryRead).toHaveBeenCalledTimes(2); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled(); + } finally { + registryRead.mockRestore(); + } + }); + }); + + it("preserves ordinary source fallback for an invalid known marketplace alias", async () => { + await withKnownMarketplaceRegistry({ broken: { source: { source: "path" } } }, async () => { + const result = await listMarketplacePlugins({ marketplace: "broken" }); + + expect(result).toEqual({ + ok: false, + error: "unsupported marketplace source: broken", + }); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + expect(fetchWithSsrFGuardMock).not.toHaveBeenCalled(); + }); + }); + it("installs remote marketplace plugins from relative paths inside the cloned repo", async () => { mockRemoteMarketplaceClone({ pluginDir: path.join("plugins", "frontend-design"), diff --git a/src/plugins/marketplace.ts b/src/plugins/marketplace.ts index c670aa2882b0..3dbe6857ace1 100644 --- a/src/plugins/marketplace.ts +++ b/src/plugins/marketplace.ts @@ -672,27 +672,39 @@ async function loadMarketplace(params: { return undefined; }; + // Resolve aliases against one snapshot so a cycle cannot retain a plugin lifecycle lease. const knownMarketplaces = await readClaudeKnownMarketplaces(); - const known = knownMarketplaces[params.source]; - if (known) { + const visitedKnownMarketplaces = new Set(); + let source = params.source; + + while (true) { + const known = knownMarketplaces[source]; + if (!known) { + break; + } + if (visitedKnownMarketplaces.has(source)) { + return { + ok: false, + error: `known marketplace source cycle: ${[...visitedKnownMarketplaces, source].join(" -> ")}`, + }; + } + visitedKnownMarketplaces.add(source); + if (known.installLocation) { const local = await resolveLocalMarketplaceSource(known.installLocation); if (local?.ok) { - return await loadResolvedLocalMarketplace(local, params.source); + return await loadResolvedLocalMarketplace(local, source); } } const normalizedSource = normalizeEntrySource(known.source); - if (normalizedSource.ok) { - return await loadMarketplace({ - source: marketplaceEntrySourceToInput(normalizedSource.source), - logger: params.logger, - timeoutMs: params.timeoutMs, - }); + if (!normalizedSource.ok) { + break; } + source = marketplaceEntrySourceToInput(normalizedSource.source); } - const local = await resolveLocalMarketplaceSource(params.source); + const local = await resolveLocalMarketplaceSource(source); if (local?.ok === false) { return local; } @@ -702,7 +714,7 @@ async function loadMarketplace(params: { } const cloned = await cloneMarketplaceRepo({ - source: params.source, + source, timeoutMs: params.timeoutMs, logger: params.logger, });