Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions packages/core/src/extension/claude-converter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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');
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/extension/http-client.ts
Original file line number Diff line number Diff line change
@@ -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}`);
}
113 changes: 109 additions & 4 deletions packages/core/src/extension/marketplace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}));

Expand All @@ -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 = {
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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 () => {
Expand Down
44 changes: 31 additions & 13 deletions packages/core/src/extension/marketplace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -141,11 +141,17 @@ function fetchUrl(
url: string,
headers: Record<string, string>,
): Promise<string | null> {
const protocol = new URL(url).protocol;
const client = protocol === 'http:' ? http : https;

return new Promise((resolve) => {
let client: ReturnType<typeof clientForUrl>;
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;
Expand All @@ -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);
Expand All @@ -170,18 +177,25 @@ function fetchUrl(
res.on('data', (chunk) => {
total += chunk.length;
if (total > MARKETPLACE_MAX_BODY_BYTES) {
req.destroy();
req?.destroy();
done(null);
return;
}
chunks.push(chunk);
});
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);
});
});
Expand Down Expand Up @@ -266,6 +280,7 @@ export async function loadMarketplaceConfigFromSource(
source: string,
): Promise<ClaudeMarketplaceConfig | null> {
const trimmed = source.trim();
const lowerTrimmed = trimmed.toLowerCase();

// Priority 1: local path (directory with .claude-plugin/marketplace.json,
// or a direct marketplace.json file).
Expand All @@ -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);
Expand All @@ -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]);
Expand Down
17 changes: 1 addition & 16 deletions packages/core/src/extension/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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<T>(url: string, authToken?: string): Promise<T> {
const headers: Record<string, string> = {
Accept: 'application/json',
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/extension/sourceRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This table now covers uppercase HTTPS:// / HTTP:// well, but parseExtensionSourceType also made the git@ / sso:// branch case-insensitive (lower.startsWith('git@') || lower.startsWith('sso://')) and that half is only tested in lowercase (rows 33-34). Reverting just that branch to trimmed.startsWith(...) leaves every test green — so the uppercase git@/sso:// classification is unguarded. Two rows close it:

Suggested change
['HTTP://example.com/marketplace.json', 'http'],
['HTTP://example.com/marketplace.json', 'http'],
['GIT@github.com:owner/repo.git', 'git'],
['SSO://team/repo', 'git'],
中文

这个表对大写 HTTPS:// / HTTP:// 覆盖得很好,但 parseExtensionSourceType 同样把 git@ / sso:// 分支改成了大小写不敏感(lower.startsWith('git@') || lower.startsWith('sso://')),而这一半只测了小写(第 33-34 行)。把该分支退回 trimmed.startsWith(...) 时所有测试仍全绿——说明大写 git@/sso:// 的分类没有被测试守住。补两行即可。

— claude-opus-4-8 via Claude Code /qreview

['./local/marketplace', 'local'],
['/abs/path/marketplace', 'local'],
] as const)('classifies %s as %s', (input, expected) => {
Expand Down
Loading
Loading