From 60a974b1a2938632135cccb71b0ccf843faeae4e Mon Sep 17 00:00:00 2001 From: tt-a1i <53142663+tt-a1i@users.noreply.github.com> Date: Sat, 20 Jun 2026 14:25:37 +0800 Subject: [PATCH] fix(extensions): accept uppercase marketplace source schemes --- .../src/extension/claude-converter.test.ts | 42 +++++++ packages/core/src/extension/http-client.ts | 21 ++++ .../core/src/extension/marketplace.test.ts | 113 +++++++++++++++++- packages/core/src/extension/marketplace.ts | 44 +++++-- packages/core/src/extension/npm.ts | 17 +-- .../core/src/extension/sourceRegistry.test.ts | 3 + packages/core/src/extension/sourceRegistry.ts | 5 +- 7 files changed, 210 insertions(+), 35 deletions(-) create mode 100644 packages/core/src/extension/http-client.ts diff --git a/packages/core/src/extension/claude-converter.test.ts b/packages/core/src/extension/claude-converter.test.ts index d0e23391cf1..5af3496a182 100644 --- a/packages/core/src/extension/claude-converter.test.ts +++ b/packages/core/src/extension/claude-converter.test.ts @@ -203,6 +203,8 @@ describe('convertClaudePluginPackage', () => { beforeEach(() => { // Create a temporary directory for test files testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-test-')); + vi.mocked(downloadFromGitHubRelease).mockReset(); + vi.mocked(cloneFromGit).mockReset(); }); afterEach(() => { @@ -378,6 +380,46 @@ describe('convertClaudePluginPackage', () => { fs.rmSync(secretDir, { recursive: true, force: true }); }); + it('treats uppercase HTTPS marketplace plugin sources as URLs', async () => { + const pluginSourceDir = path.join(testDir, 'plugin-uppercase-url'); + const marketplaceDir = path.join(pluginSourceDir, '.claude-plugin'); + fs.mkdirSync(marketplaceDir, { recursive: true }); + + const marketplaceConfig: ClaudeMarketplaceConfig = { + name: 'test-marketplace', + owner: { name: 'Test Owner', email: 'test@example.com' }, + plugins: [ + { + name: 'remote', + version: '1.0.0', + source: 'HTTPS://github.com/owner/plugin', + strict: false, + }, + ], + }; + fs.writeFileSync( + path.join(marketplaceDir, 'marketplace.json'), + JSON.stringify(marketplaceConfig, null, 2), + 'utf-8', + ); + vi.mocked(downloadFromGitHubRelease).mockResolvedValue(undefined as never); + + const result = await convertClaudePluginPackage(pluginSourceDir, 'remote'); + + expect(result.config.name).toBe('remote'); + expect(downloadFromGitHubRelease).toHaveBeenCalledWith( + { + source: 'HTTPS://github.com/owner/plugin', + type: 'git', + originSource: 'Claude', + }, + expect.any(String), + ); + expect(cloneFromGit).not.toHaveBeenCalled(); + + fs.rmSync(result.convertedDir, { recursive: true, force: true }); + }); + it('should use all skills from folder when config does not specify skills', async () => { // Setup: Create a plugin source with skills but no skills config const pluginSourceDir = path.join(testDir, 'plugin-source-default'); diff --git a/packages/core/src/extension/http-client.ts b/packages/core/src/extension/http-client.ts new file mode 100644 index 00000000000..bacd248c773 --- /dev/null +++ b/packages/core/src/extension/http-client.ts @@ -0,0 +1,21 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as http from 'node:http'; +import * as https from 'node:https'; + +export type HttpClient = typeof http | typeof https; + +export function clientForUrl(url: string): HttpClient { + const protocol = new URL(url).protocol.toLowerCase(); + if (protocol === 'https:') { + return https; + } + if (protocol === 'http:') { + return http; + } + throw new Error(`Unsupported URL protocol: ${protocol}`); +} diff --git a/packages/core/src/extension/marketplace.test.ts b/packages/core/src/extension/marketplace.test.ts index dfe0bf9b70f..4bf5e096e88 100644 --- a/packages/core/src/extension/marketplace.test.ts +++ b/packages/core/src/extension/marketplace.test.ts @@ -24,11 +24,11 @@ vi.mock('node:fs', () => ({ }, })); -vi.mock('node:https', () => ({ +vi.mock('node:http', () => ({ get: vi.fn(), })); -vi.mock('node:http', () => ({ +vi.mock('node:https', () => ({ get: vi.fn(), })); @@ -49,12 +49,13 @@ describe('parseInstallSource', () => { vi.mocked(https.get).mockImplementation((_url, _options, callback) => { const mockRes = { statusCode: 404, + resume: vi.fn(), on: vi.fn(), }; if (typeof callback === 'function') { callback(mockRes as never); } - return { on: vi.fn() } as never; + return { on: vi.fn(), setTimeout: vi.fn(), destroy: vi.fn() } as never; }); vi.mocked(http.get).mockImplementation((_url, _options, callback) => { const mockRes = { @@ -337,7 +338,7 @@ describe('parseInstallSource', () => { if (typeof callback === 'function') { callback(mockRes as never); } - return { on: vi.fn() } as never; + return { on: vi.fn(), setTimeout: vi.fn(), destroy: vi.fn() } as never; }); const result = await parseInstallSource('owner/repo'); @@ -430,6 +431,110 @@ describe('parseInstallSource', () => { expect(result).toEqual(cfg); }); + it('resolves a marketplace from an uppercase HTTPS GitHub source', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + const cfg = { + name: 'uppercase-url-marketplace', + owner: { name: 'Owner' }, + plugins: [{ name: 'p1' }], + }; + vi.mocked(https.get).mockImplementation((_url, _options, callback) => { + const mockRes = { + statusCode: 200, + resume: vi.fn(), + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(Buffer.from(JSON.stringify(cfg))); + } + if (event === 'end') { + handler(); + } + }), + }; + if (typeof callback === 'function') { + callback(mockRes as never); + } + return { on: vi.fn(), setTimeout: vi.fn(), destroy: vi.fn() } as never; + }); + + const result = await loadMarketplaceConfigFromSource( + 'HTTPS://github.com/owner/repo', + ); + + expect(result).toEqual(cfg); + }); + + it('resolves a direct JSON marketplace from an uppercase HTTPS source', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + const cfg = { + name: 'uppercase-direct-marketplace', + owner: { name: 'Owner' }, + plugins: [{ name: 'p1' }], + }; + vi.mocked(https.get).mockImplementation((_url, _options, callback) => { + const mockRes = { + statusCode: 200, + resume: vi.fn(), + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(Buffer.from(JSON.stringify(cfg))); + } + if (event === 'end') { + handler(); + } + }), + }; + if (typeof callback === 'function') { + callback(mockRes as never); + } + return { on: vi.fn(), setTimeout: vi.fn(), destroy: vi.fn() } as never; + }); + + const result = await loadMarketplaceConfigFromSource( + 'HTTPS://example.com/marketplace.json', + ); + + expect(result).toEqual(cfg); + }); + + it('resolves a direct JSON marketplace from an uppercase HTTP source', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + const cfg = { + name: 'uppercase-http-marketplace', + owner: { name: 'Owner' }, + plugins: [{ name: 'p1' }], + }; + vi.mocked(http.get).mockImplementation((_url, _options, callback) => { + const mockRes = { + statusCode: 200, + resume: vi.fn(), + on: vi.fn((event, handler) => { + if (event === 'data') { + handler(Buffer.from(JSON.stringify(cfg))); + } + if (event === 'end') { + handler(); + } + }), + }; + if (typeof callback === 'function') { + callback(mockRes as never); + } + return { on: vi.fn(), setTimeout: vi.fn(), destroy: vi.fn() } as never; + }); + + const result = await loadMarketplaceConfigFromSource( + 'HTTP://example.com/marketplace.json', + ); + + expect(result).toEqual(cfg); + expect(https.get).not.toHaveBeenCalledWith( + 'HTTP://example.com/marketplace.json', + expect.anything(), + expect.anything(), + ); + }); + // A non-GitHub https URL reaches fetchUrl via a single direct-JSON fetch, // so these exercise the fetchUrl security guards in isolation. it('aborts and returns null when the response body exceeds the size cap', async () => { diff --git a/packages/core/src/extension/marketplace.ts b/packages/core/src/extension/marketplace.ts index 8f87f69878d..6e3ee474c31 100644 --- a/packages/core/src/extension/marketplace.ts +++ b/packages/core/src/extension/marketplace.ts @@ -9,12 +9,12 @@ import type { ExtensionInstallMetadata } from '../config/config.js'; import type { ClaudeMarketplaceConfig } from './claude-converter.js'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import * as http from 'node:http'; -import * as https from 'node:https'; +import type { ClientRequest, IncomingMessage } from 'node:http'; import { stat } from 'node:fs/promises'; import { parseGitHubRepoForReleases } from './github.js'; import { isScopedNpmPackage } from './npm.js'; import { redactUrlCredentials } from './redaction.js'; +import { clientForUrl } from './http-client.js'; export interface MarketplaceInstallOptions { marketplaceUrl: string; @@ -141,11 +141,17 @@ function fetchUrl( url: string, headers: Record, ): Promise { - const protocol = new URL(url).protocol; - const client = protocol === 'http:' ? http : https; - return new Promise((resolve) => { + let client: ReturnType; + try { + client = clientForUrl(url); + } catch { + resolve(null); + return; + } + let settled = false; + let req: ClientRequest | undefined; const done = (value: string | null) => { if (settled) return; settled = true; @@ -156,10 +162,11 @@ function fetchUrl( // chunk, so a server trickling bytes can keep the request alive forever. // Pair it with an absolute wall-clock deadline. const hardDeadline = setTimeout(() => { - req.destroy(); + req?.destroy(); done(null); }, MARKETPLACE_FETCH_TIMEOUT_MS); - const req = client.get(url, { headers }, (res) => { + + const onResponse = (res: IncomingMessage) => { if (res.statusCode !== 200) { res.resume(); // drain so the socket can be freed done(null); @@ -170,7 +177,7 @@ function fetchUrl( res.on('data', (chunk) => { total += chunk.length; if (total > MARKETPLACE_MAX_BODY_BYTES) { - req.destroy(); + req?.destroy(); done(null); return; } @@ -178,10 +185,17 @@ function fetchUrl( }); res.on('end', () => done(Buffer.concat(chunks).toString())); res.on('error', () => done(null)); - }); + }; + + try { + req = client.get(url, { headers }, onResponse); + } catch { + done(null); + return; + } req.on('error', () => done(null)); req.setTimeout(MARKETPLACE_FETCH_TIMEOUT_MS, () => { - req.destroy(); + req?.destroy(); done(null); }); }); @@ -266,6 +280,7 @@ export async function loadMarketplaceConfigFromSource( source: string, ): Promise { const trimmed = source.trim(); + const lowerTrimmed = trimmed.toLowerCase(); // Priority 1: local path (directory with .claude-plugin/marketplace.json, // or a direct marketplace.json file). @@ -287,7 +302,10 @@ export async function loadMarketplaceConfigFromSource( } // Priority 2: http(s) URL — try GitHub repo first, then a direct JSON doc. - if (trimmed.startsWith('http://') || trimmed.startsWith('https://')) { + if ( + lowerTrimmed.startsWith('http://') || + lowerTrimmed.startsWith('https://') + ) { try { const { owner, repo } = parseGitHubRepoForReleases(trimmed); const ghConfig = await fetchGitHubMarketplaceConfig(owner, repo); @@ -309,11 +327,11 @@ export async function loadMarketplaceConfigFromSource( } // Priority 3: ssh/sso git URLs -> resolve owner/repo via github. - if (trimmed.startsWith('git@') || trimmed.startsWith('sso://')) { + if (lowerTrimmed.startsWith('git@') || lowerTrimmed.startsWith('sso://')) { // `git@github.com:owner/repo(.git)` isn't a parseable URL, so extract // owner/repo directly before falling back to the URL-based parser. const sshMatch = trimmed.match( - /^git@github\.com:([^/]+)\/(.+?)(?:\.git)?$/, + /^git@github\.com:([^/]+)\/(.+?)(?:\.git)?$/i, ); if (sshMatch) { return fetchGitHubMarketplaceConfig(sshMatch[1], sshMatch[2]); diff --git a/packages/core/src/extension/npm.ts b/packages/core/src/extension/npm.ts index f89887a6d11..fc8b749519d 100644 --- a/packages/core/src/extension/npm.ts +++ b/packages/core/src/extension/npm.ts @@ -5,13 +5,12 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; -import * as https from 'node:https'; -import * as http from 'node:http'; import * as tar from 'tar'; import type { ExtensionInstallMetadata } from '../config/config.js'; import { ExtensionUpdateState } from './extensionManager.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { redactUrlCredentials } from './redaction.js'; +import { clientForUrl } from './http-client.js'; const debugLogger = createDebugLogger('EXT_NPM'); @@ -179,20 +178,6 @@ function getNpmAuthToken(registryUrl: string): string | undefined { return undefined; } -/** - * Fetch JSON from a URL, handling both https and http. - */ -function clientForUrl(url: string): typeof https | typeof http { - const protocol = new URL(url).protocol.toLowerCase(); - if (protocol === 'https:') { - return https; - } - if (protocol === 'http:') { - return http; - } - throw new Error(`Unsupported npm registry URL protocol: ${protocol}`); -} - function fetchNpmJson(url: string, authToken?: string): Promise { const headers: Record = { Accept: 'application/json', diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 438c966849b..5d29b5e508d 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -29,9 +29,12 @@ describe('parseExtensionSourceType', () => { it.each([ ['anthropics/skills', 'github'], ['https://github.com/owner/repo', 'github'], + ['HTTPS://github.com/owner/repo', 'github'], ['git@github.com:owner/repo.git', 'git'], ['sso://team/repo', 'git'], ['https://example.com/marketplace.json', 'http'], + ['HTTPS://example.com/marketplace.json', 'http'], + ['HTTP://example.com/marketplace.json', 'http'], ['./local/marketplace', 'local'], ['/abs/path/marketplace', 'local'], ] as const)('classifies %s as %s', (input, expected) => { diff --git a/packages/core/src/extension/sourceRegistry.ts b/packages/core/src/extension/sourceRegistry.ts index f3ac7ba6f34..f26645c74a8 100644 --- a/packages/core/src/extension/sourceRegistry.ts +++ b/packages/core/src/extension/sourceRegistry.ts @@ -125,10 +125,11 @@ function pluginInstalls( */ export function parseExtensionSourceType(source: string): ExtensionSourceType { const trimmed = source.trim(); - if (trimmed.startsWith('git@') || trimmed.startsWith('sso://')) { + const lower = trimmed.toLowerCase(); + if (lower.startsWith('git@') || lower.startsWith('sso://')) { return 'git'; } - if (trimmed.startsWith('http://') || trimmed.startsWith('https://')) { + if (lower.startsWith('http://') || lower.startsWith('https://')) { return isGitHubHost(trimmed) ? 'github' : 'http'; } if (isOwnerRepoShorthand(trimmed)) {