diff --git a/docs/users/extension/introduction.md b/docs/users/extension/introduction.md index 73f34dff480..6bc33e7120a 100644 --- a/docs/users/extension/introduction.md +++ b/docs/users/extension/introduction.md @@ -155,7 +155,9 @@ Only scoped packages (`@scope/package-name`) are supported to avoid ambiguity wi #### From Git Repository -Public Git repository installs and update checks require Git 2.37 or newer. Qwen Code uses the `http.curloptResolve` setting introduced in Git 2.37 to pin public network connections to validated DNS results. If your distribution ships an older Git version, upgrade Git or install a local/archive release instead. +Git 2.37 or newer is required for credentialed, non-GitHub, nested marketplace, submodule, and Git LFS sources because Qwen Code uses `http.curloptResolve` to pin Git connections to validated DNS results. On older Git versions, Qwen Code supports only anonymous public `https://github.com/{owner}/{repo}[.git]` root repositories by resolving the requested ref to a commit and downloading GitHub's source archive with the same public-network and archive-safety checks. + +Because the older-Git fallback installs from a source archive rather than a clone, it cannot install repositories that rely on symlinks, submodules, or Git LFS, and it caps downloads at 100 MiB compressed and archives at 100,000 entries / 1 GiB expanded. Release-based installs are still preferred when a repository publishes releases. ```bash qwen extensions install https://github.com/github/github-mcp-server diff --git a/packages/core/src/extension/archive-safety.test.ts b/packages/core/src/extension/archive-safety.test.ts index 52a001de6f6..fc53a0d06fb 100644 --- a/packages/core/src/extension/archive-safety.test.ts +++ b/packages/core/src/extension/archive-safety.test.ts @@ -4,12 +4,85 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { randomBytes } from 'node:crypto'; import { promises as fs } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import * as tar from 'tar'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { assertTarArchiveHasNoLinks } from './archive-safety.js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + MAX_ARCHIVE_ENTRIES, + MAX_ARCHIVE_EXPANDED_BYTES, + assertTarArchiveHasNoLinks, +} from './archive-safety.js'; + +// Passthrough wrapper around `fs.createReadStream` that tests can hook to +// observe how much of the archive the scan actually reads. +const streamProbe = vi.hoisted(() => ({ + onReadStream: undefined as + | (( + filePath: unknown, + options: unknown, + original: ( + filePath: unknown, + options: unknown, + ) => NodeJS.ReadableStream, + ) => NodeJS.ReadableStream) + | undefined, +})); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createReadStream: (filePath: unknown, options: unknown) => { + const original = ( + actual.createReadStream as ( + filePath: unknown, + options: unknown, + ) => NodeJS.ReadableStream + ).bind(actual); + if (streamProbe.onReadStream) { + return streamProbe.onReadStream(filePath, options, original); + } + return original(filePath, options); + }, + }; +}); + +// Builds a ustar header for a zero-content regular file. `tar.t` parses +// headers via `onReadEntry` without requiring entry content, so these +// crafted headers are enough to exercise the entry-count and expanded-size +// limits without writing gigabytes of data or hundreds of thousands of +// files to disk. +function createTarFileHeader(name: string, size: number): Buffer { + const header = Buffer.alloc(512); + header.write(name, 0, 100, 'utf8'); + header.write('0000644\0', 100, 8); // mode + header.write('0000000\0', 108, 8); // uid + header.write('0000000\0', 116, 8); // gid + header.write(`${size.toString(8).padStart(11, '0')}\0`, 124, 12); + header.write('14763423360\0', 136, 12); // mtime + header.write(' ', 148, 8); // checksum placeholder (spaces) + header.write('0', 156, 1); // typeflag: regular file + header.write('ustar\0', 257, 6); + header.write('00', 263, 2); + let checksum = 0; + for (const byte of header) { + checksum += byte; + } + header.write(`${checksum.toString(8).padStart(6, '0')}\0 `, 148, 8); + return header; +} + +const TAR_TRAILER = Buffer.alloc(1024); + +async function writeCraftedTar( + archive: string, + headers: Buffer[], +): Promise { + await fs.writeFile(archive, Buffer.concat([...headers, TAR_TRAILER])); +} describe('assertTarArchiveHasNoLinks', () => { let root: string; @@ -39,4 +112,155 @@ describe('assertTarArchiveHasNoLinks', () => { ); }, ); + + it.runIf(process.platform !== 'win32')( + 'stops reading the archive as soon as validation fails', + async () => { + const links = Array.from({ length: 101 }, (_, index) => `link-${index}`); + await Promise.all( + links.map(async (link) => { + await fs.symlink('missing-target', path.join(root, link)); + }), + ); + // A large trailing entry that a scan-to-end implementation would still + // consume after the link limit trips; an early abort never reaches it. + const tailBytes = 20 * 1024 * 1024; + await fs.writeFile(path.join(root, 'tail.bin'), randomBytes(tailBytes)); + const archive = path.join(root, 'abort-links.tar'); + await tar.c({ cwd: root, file: archive }, [...links, 'tail.bin']); + + let bytesRead = 0; + streamProbe.onReadStream = (filePath, options, original) => { + const stream = original(filePath, options); + stream.on('data', (chunk) => { + bytesRead += chunk.length; + }); + return stream; + }; + + try { + await expect(assertTarArchiveHasNoLinks(archive)).rejects.toThrow( + 'more than 100 unsupported link entries', + ); + } finally { + streamProbe.onReadStream = undefined; + } + + // Without the early abort the scan would read the whole ~20 MB tail. + expect(bytesRead).toBeLessThan(tailBytes / 2); + }, + ); + + it('rejects a pre-aborted signal without opening the archive stream', async () => { + const controller = new AbortController(); + const abortReason = new Error('install cancelled'); + controller.abort(abortReason); + let createReadStreamCalls = 0; + streamProbe.onReadStream = (filePath, options, original) => { + createReadStreamCalls += 1; + const stream = original(filePath, options); + // If the regression returns, the abandoned stream would emit an + // unhandled ENOENT 'error' event; swallow it so the assertion below + // fails the test cleanly instead of crashing the worker. + stream.on('error', () => {}); + return stream; + }; + + try { + await expect( + assertTarArchiveHasNoLinks( + path.join(root, 'missing.tar'), + controller.signal, + ), + ).rejects.toBe(abortReason); + } finally { + streamProbe.onReadStream = undefined; + } + + expect(createReadStreamCalls).toBe(0); + }); + + const resourceLimits = { enforceResourceLimits: true }; + + it('accepts an archive with exactly the entry-count limit', async () => { + const archive = path.join(root, 'exact-entries.tar'); + const header = createTarFileHeader('file', 0); + await writeCraftedTar( + archive, + Array.from({ length: MAX_ARCHIVE_ENTRIES }, () => header), + ); + + await expect( + assertTarArchiveHasNoLinks(archive, undefined, resourceLimits), + ).resolves.toBeUndefined(); + }); + + it('rejects an archive just over the entry-count limit', async () => { + const archive = path.join(root, 'too-many-entries.tar'); + const header = createTarFileHeader('file', 0); + await writeCraftedTar( + archive, + Array.from({ length: MAX_ARCHIVE_ENTRIES + 1 }, () => header), + ); + + await expect( + assertTarArchiveHasNoLinks(archive, undefined, resourceLimits), + ).rejects.toThrow( + `Tar archive contains more than ${MAX_ARCHIVE_ENTRIES} entries.`, + ); + }); + + it('skips resource limits for trusted archives by default', async () => { + const archive = path.join(root, 'huge-but-trusted.tar'); + await fs.writeFile( + archive, + Buffer.concat([ + createTarFileHeader('big.bin', MAX_ARCHIVE_EXPANDED_BYTES + 1), + TAR_TRAILER, + ]), + ); + + await expect(assertTarArchiveHasNoLinks(archive)).resolves.toBeUndefined(); + }); + + // The parser skips `size` content bytes after each header, so every entry + // except the last must carry its (padded) content; the final entry declares + // a huge size without backing bytes, which `tar.t` tolerates as a trailing + // truncation. The first entry's real content makes the two-entry sum an + // actual accumulation check. + async function writeByteLimitTar( + archive: string, + secondEntrySize: number, + ): Promise { + const firstContent = Buffer.alloc(512); + await fs.writeFile( + archive, + Buffer.concat([ + createTarFileHeader('first.bin', firstContent.length), + firstContent, + createTarFileHeader('second.bin', secondEntrySize), + TAR_TRAILER, + ]), + ); + } + + it('accepts an archive whose declared sizes sum exactly to the byte limit', async () => { + const archive = path.join(root, 'exact-bytes.tar'); + await writeByteLimitTar(archive, MAX_ARCHIVE_EXPANDED_BYTES - 512); + + await expect( + assertTarArchiveHasNoLinks(archive, undefined, resourceLimits), + ).resolves.toBeUndefined(); + }); + + it('rejects an archive whose declared sizes sum just over the byte limit', async () => { + const archive = path.join(root, 'too-many-bytes.tar'); + await writeByteLimitTar(archive, MAX_ARCHIVE_EXPANDED_BYTES - 512 + 1); + + await expect( + assertTarArchiveHasNoLinks(archive, undefined, resourceLimits), + ).rejects.toThrow( + `Tar archive expands beyond ${MAX_ARCHIVE_EXPANDED_BYTES} bytes.`, + ); + }); }); diff --git a/packages/core/src/extension/archive-safety.ts b/packages/core/src/extension/archive-safety.ts index 75a046f6443..845a2dd416c 100644 --- a/packages/core/src/extension/archive-safety.ts +++ b/packages/core/src/extension/archive-safety.ts @@ -12,6 +12,18 @@ import { stripAnsiAndControl } from '../utils/textUtils.js'; const MAX_REPORTED_ENTRY_PATH_LENGTH = 200; const MAX_REPORTED_LINK_ENTRIES = 10; const MAX_LINK_ENTRIES = 100; +export const MAX_ARCHIVE_ENTRIES = 100_000; +export const MAX_ARCHIVE_EXPANDED_BYTES = 1024 * 1024 * 1024; + +export interface TarArchiveSafetyOptions { + /** + * Enforce the entry-count and expanded-size ceilings. Kept off by default + * so local, npm, and release archives keep their pre-existing behavior; + * enable it only for untrusted network archives such as the older-Git + * public GitHub archive fallback. + */ + enforceResourceLimits?: boolean; +} function formatEntryPath(entryPath: string): string { const sanitized = stripAnsiAndControl(entryPath); @@ -22,11 +34,43 @@ function formatEntryPath(entryPath: string): string { export async function assertTarArchiveHasNoLinks( file: string, signal?: AbortSignal, + options: TarArchiveSafetyOptions = {}, ): Promise { + const enforceResourceLimits = options.enforceResourceLimits === true; const unsupportedLinkPaths: string[] = []; let unsupportedLinkCount = 0; - let linkLimitError: Error | undefined; + let entryCount = 0; + let expandedBytes = 0; + let validationError: Error | undefined; + // Stop reading as soon as validation fails instead of walking the rest of + // a potentially hostile archive. + const failValidation = (error: Error) => { + if (validationError) return; + validationError = error; + stream.destroy(); + }; const onReadEntry = (entry: tar.ReadEntry) => { + if (validationError) return; + if (enforceResourceLimits) { + entryCount += 1; + expandedBytes += entry.size; + if (entryCount > MAX_ARCHIVE_ENTRIES) { + failValidation( + new Error( + `Tar archive contains more than ${MAX_ARCHIVE_ENTRIES} entries.`, + ), + ); + return; + } + if (expandedBytes > MAX_ARCHIVE_EXPANDED_BYTES) { + failValidation( + new Error( + `Tar archive expands beyond ${MAX_ARCHIVE_EXPANDED_BYTES} bytes.`, + ), + ); + return; + } + } if (entry.type === 'SymbolicLink' || entry.type === 'Link') { unsupportedLinkCount += 1; const unsupportedLinkPath = @@ -34,31 +78,29 @@ export async function assertTarArchiveHasNoLinks( if (unsupportedLinkPaths.length < MAX_REPORTED_LINK_ENTRIES) { unsupportedLinkPaths.push(unsupportedLinkPath); } - if ( - unsupportedLinkCount > MAX_LINK_ENTRIES && - linkLimitError === undefined - ) { - linkLimitError = new Error( - `Tar archive contains more than ${MAX_LINK_ENTRIES} unsupported link entries: ${unsupportedLinkPaths.join(', ')}`, + if (unsupportedLinkCount > MAX_LINK_ENTRIES) { + failValidation( + new Error( + `Tar archive contains more than ${MAX_LINK_ENTRIES} unsupported link entries: ${unsupportedLinkPaths.join(', ')}`, + ), ); } } }; signal?.throwIfAborted(); - if (signal) { - try { - await pipeline(fs.createReadStream(file), tar.t({ onReadEntry }), { - signal, - }); - } catch (error) { - signal.throwIfAborted(); - throw error; - } - signal.throwIfAborted(); - } else { - await tar.t({ file, onReadEntry }); + // Open the stream only after the abort check: entering with a pre-aborted + // signal must not leave a live ReadStream behind (an unhandled ENOENT + // 'error' event for a missing file, or a leaked fd otherwise). + const stream = fs.createReadStream(file); + try { + await pipeline(stream, tar.t({ onReadEntry }), { signal }); + } catch (error) { + signal?.throwIfAborted(); + if (validationError) throw validationError; + throw error; } - if (linkLimitError) throw linkLimitError; + signal?.throwIfAborted(); + if (validationError) throw validationError; if (unsupportedLinkCount > 0) { const entryLabel = unsupportedLinkCount === 1 diff --git a/packages/core/src/extension/extensionManager.test.ts b/packages/core/src/extension/extensionManager.test.ts index c7f6f180e3b..d2243ccbed0 100644 --- a/packages/core/src/extension/extensionManager.test.ts +++ b/packages/core/src/extension/extensionManager.test.ts @@ -38,6 +38,7 @@ import { EXTENSION_GIT_CREDENTIAL_SELECTOR_FILENAME, resolveStoredGitCredential, } from './extension-git-credentials.js'; +import { resetLocalGitVersionCacheForTesting } from './github.js'; import { FileTokenStorage } from '../mcp/token-storage/file-token-storage.js'; const mockGit = { @@ -52,6 +53,12 @@ const mockGit = { path: vi.fn(), }; const mockDownloadFromArchiveUrl = vi.hoisted(() => vi.fn()); +const mockDownloadPublicGitHubArchiveFallback = vi.hoisted(() => vi.fn()); +const mockDownloadFromGitHubRelease = vi.hoisted(() => + vi + .fn() + .mockRejectedValue(new Error('Mocked GitHub release download failure')), +); const mockExtractArchiveFile = vi.hoisted(() => vi.fn()); const mockDownloadFromNpmRegistry = vi.hoisted(() => vi.fn()); @@ -68,9 +75,9 @@ vi.mock('./github.js', async (importOriginal) => { return { ...actual, downloadFromArchiveUrl: mockDownloadFromArchiveUrl, - downloadFromGitHubRelease: vi - .fn() - .mockRejectedValue(new Error('Mocked GitHub release download failure')), + downloadPublicGitHubArchiveFallback: + mockDownloadPublicGitHubArchiveFallback, + downloadFromGitHubRelease: mockDownloadFromGitHubRelease, extractArchiveFile: mockExtractArchiveFile, }; }); @@ -217,8 +224,14 @@ describe('extension tests', () => { mockHomedir.mockReturnValue(tempHomeDir); vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); + resetLocalGitVersionCacheForTesting(); Object.values(mockGit).forEach((fn) => fn.mockReset()); mockDownloadFromArchiveUrl.mockReset(); + mockDownloadPublicGitHubArchiveFallback.mockReset(); + mockDownloadFromGitHubRelease.mockReset(); + mockDownloadFromGitHubRelease.mockRejectedValue( + new Error('Mocked GitHub release download failure'), + ); mockExtractArchiveFile.mockReset(); mockDownloadFromNpmRegistry.mockReset(); mockGit.revparse.mockResolvedValue('sample-commit'); @@ -542,6 +555,71 @@ describe('extension tests', () => { ).toBe(false); }); + it('installs an anonymous public GitHub extension through the old-Git archive fallback', async () => { + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + mockDownloadPublicGitHubArchiveFallback.mockImplementation( + async (_metadata: ExtensionInstallMetadata, destination: string) => { + writeExtractedExtension(destination, 'old-git-extension'); + return '0123456789abcdef0123456789abcdef01234567'; + }, + ); + const manager = createExtensionManager({ networkPolicy: 'public' }); + await manager.refreshCache(); + + const installed = await manager.installExtension( + { + type: 'git', + source: 'https://github.com/obra/superpowers', + }, + async () => {}, + ); + + expect(installed.installMetadata).toMatchObject({ + type: 'git', + source: 'https://github.com/obra/superpowers', + gitCommit: '0123456789abcdef0123456789abcdef01234567', + }); + // Releases stay preferred over the archive fallback on older Git. + expect(mockDownloadFromGitHubRelease).toHaveBeenCalled(); + expect(mockDownloadPublicGitHubArchiveFallback).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'git', + source: 'https://github.com/obra/superpowers', + }), + expect.any(String), + undefined, + ); + expect(mockGit.clone).not.toHaveBeenCalled(); + }); + + it('keeps release installs ahead of the old-Git archive fallback', async () => { + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + mockDownloadFromGitHubRelease.mockImplementation( + async (_metadata: ExtensionInstallMetadata, destination: string) => { + writeExtractedExtension(destination, 'release-extension'); + return { tagName: 'v2.0.0', type: 'github-release' as const }; + }, + ); + const manager = createExtensionManager({ networkPolicy: 'public' }); + await manager.refreshCache(); + + const installed = await manager.installExtension( + { + type: 'git', + source: 'https://github.com/owner/repo', + }, + async () => {}, + ); + + expect(installed.installMetadata).toMatchObject({ + type: 'github-release', + source: 'https://github.com/owner/repo', + releaseTag: 'v2.0.0', + }); + expect(mockDownloadPublicGitHubArchiveFallback).not.toHaveBeenCalled(); + expect(mockGit.clone).not.toHaveBeenCalled(); + }); + it('persists a credentialed one-time install as a source-free snapshot', async () => { mockGit.env.mockReturnValue(mockGit); mockGit.clone.mockImplementation(async () => { @@ -3275,6 +3353,55 @@ describe('extension tests', () => { expect(mockGit.listRemote).not.toHaveBeenCalled(); }); + it('updates an old-Git public GitHub extension through a new archive SHA', async () => { + createExtension({ + extensionsDir: userExtensionsDir, + version: '1.0.0', + installMetadata: { + type: 'git', + source: 'https://github.com/owner/repo', + gitCommit: '0123456789abcdef0123456789abcdef01234567', + }, + }); + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + mockDownloadPublicGitHubArchiveFallback.mockImplementation( + async (_metadata: ExtensionInstallMetadata, destination: string) => { + fs.mkdirSync(destination, { recursive: true }); + fs.writeFileSync( + path.join(destination, EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'my-extension', version: '2.0.0' }), + ); + return '89abcdef0123456789abcdef0123456789abcdef'; + }, + ); + const manager = createExtensionManager({ networkPolicy: 'public' }); + await manager.refreshCache(); + const extension = manager.getLoadedExtensions()[0]!; + + await manager.updateExtension( + extension, + ExtensionUpdateState.UPDATE_AVAILABLE, + () => {}, + ); + + expect(manager.getLoadedExtensions()[0]?.installMetadata).toMatchObject({ + source: 'https://github.com/owner/repo', + type: 'git', + gitCommit: '89abcdef0123456789abcdef0123456789abcdef', + }); + // The manager mutates installMetadata in place (gitCommit gets the new + // SHA after the call), so pin only the immutable identity fields. + expect(mockDownloadPublicGitHubArchiveFallback).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'git', + source: 'https://github.com/owner/repo', + }), + expect.any(String), + undefined, + ); + expect(mockGit.clone).not.toHaveBeenCalled(); + }); + it('applies the update network policy without mutating cached metadata', async () => { createExtension({ extensionsDir: userExtensionsDir, diff --git a/packages/core/src/extension/extensionManager.ts b/packages/core/src/extension/extensionManager.ts index 3c4bd933b95..7ff23ae7e57 100644 --- a/packages/core/src/extension/extensionManager.ts +++ b/packages/core/src/extension/extensionManager.ts @@ -40,9 +40,11 @@ import { cloneFromGit, downloadFromArchiveUrl, downloadFromGitHubRelease, + downloadPublicGitHubArchiveFallback, extractArchiveFile, isSupportedArchivePath, parseGitHubRepoForReleases, + shouldUsePublicGitHubArchiveFallback, } from './github.js'; import { downloadFromNpmRegistry } from './npm.js'; import { redactUrlCredentials } from './redaction.js'; @@ -2059,13 +2061,24 @@ export class ExtensionManager { // Release extraction may leave a partial destination behind. await fs.promises.rm(tempDir, { recursive: true, force: true }); await fs.promises.mkdir(tempDir, { recursive: true }); - installMetadata.gitCommit = await cloneFromGit( - installMetadata, - tempDir, - signal, - ); - if (installMetadata.type === 'github-release') { - installMetadata.type = 'git'; + // Keep release-first for older Git too: the archive fallback is + // only a clone replacement, not a release replacement. + if (await shouldUsePublicGitHubArchiveFallback(installMetadata)) { + installMetadata.gitCommit = + await downloadPublicGitHubArchiveFallback( + installMetadata, + tempDir, + signal, + ); + } else { + installMetadata.gitCommit = await cloneFromGit( + installMetadata, + tempDir, + signal, + ); + if (installMetadata.type === 'github-release') { + installMetadata.type = 'git'; + } } } } diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index eef3edff711..96f48c29c90 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -10,12 +10,15 @@ import { cloneFromGit, downloadFromArchiveUrl, downloadFromGitHubRelease, + downloadPublicGitHubArchiveFallback, extractArchiveFile, extractFile, findReleaseAsset, isSupportedArchivePath, isSupportedArchiveUrl, parseGitHubRepoForReleases, + resetLocalGitVersionCacheForTesting, + shouldUsePublicGitHubArchiveFallback, } from './github.js'; import { simpleGit, type SimpleGit } from 'simple-git'; import * as os from 'node:os'; @@ -27,6 +30,7 @@ import * as path from 'node:path'; import { randomBytes } from 'node:crypto'; import { Readable } from 'node:stream'; import { promises as dns } from 'node:dns'; +import { gzipSync } from 'node:zlib'; import * as tar from 'tar'; import * as archiver from 'archiver'; import { @@ -35,6 +39,7 @@ import { type ExtensionManager, } from './extensionManager.js'; import { getErrorMessage } from '../utils/errors.js'; +import type { ExtensionInstallMetadata } from '../config/config.js'; import { EXTENSIONS_CONFIG_FILENAME } from './variables.js'; import { QODER_PLUGIN_MANIFEST } from './qoder-converter.js'; import { ExtensionStorage } from './storage.js'; @@ -65,6 +70,7 @@ vi.mock('simple-git'); describe('git extension helpers', () => { beforeEach(() => { vi.stubEnv('GITHUB_TOKEN', ''); + resetLocalGitVersionCacheForTesting(); }); afterEach(() => { @@ -113,6 +119,21 @@ describe('git extension helpers', () => { } as unknown as ReturnType; } + // Header names are case-insensitive, so anonymity checks must not depend + // on the exact casing the client used for a header key. + function headerNames( + options: + | https.RequestOptions + | ((res: IncomingMessage) => void) + | undefined, + ): string[] { + const headers = + typeof options === 'object' || options === undefined + ? options?.headers + : undefined; + return Object.keys(headers ?? {}).map((key) => key.toLowerCase()); + } + function mockHttpsResponses(...responses: Array): void { mockHttpsGet.mockImplementation((( _url: string | URL | https.RequestOptions, @@ -332,7 +353,7 @@ describe('git extension helpers', () => { '/dest', ), ).rejects.toThrow( - 'Public extension Git installs require Git 2.37 or newer for secure DNS pinning; found Git 2.34.1. Upgrade Git, or install the extension from a local path or archive instead.', + 'Public extension Git installs require Git 2.37 or newer unless the source is an anonymous public GitHub root repository; found Git 2.34.1. Upgrade Git for credentialed, non-GitHub, nested, submodule, or Git LFS installs.', ); expect(mockGit.clone).not.toHaveBeenCalled(); }); @@ -617,6 +638,638 @@ describe('git extension helpers', () => { }); }); + describe('old-Git public GitHub archive fallback', () => { + it.each([ + ['HEAD', undefined], + ['branch', 'feature/test'], + ['tag', 'v1.2.3'], + ['commit', '0123456789abcdef0123456789abcdef01234567'], + ])( + 'resolves %s to a commit and downloads anonymously', + async (_kind, ref) => { + vi.stubEnv('GITHUB_TOKEN', 'must-not-be-sent'); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'old-git-fallback-test-'), + ); + const sourceDir = path.join(tempDir, 'source'); + const destination = path.join(tempDir, 'destination'); + await fs.mkdir(path.join(sourceDir, 'repo-archive'), { + recursive: true, + }); + await fs.mkdir(destination); + await fs.writeFile( + path.join('' + sourceDir, 'repo-archive', EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'archive-extension', version: '1.0.0' }), + ); + const archivePath = path.join(tempDir, 'source.tar.gz'); + await tar.c({ gzip: true, file: archivePath, cwd: sourceDir }, [ + 'repo-archive', + ]); + const archive = await fs.readFile(archivePath); + const sha = 'abcdef0123456789abcdef0123456789abcdef01'; + mockHttpsGet + .mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toContain( + `/commits/${encodeURIComponent(ref || 'HEAD')}`, + ); + expect(headerNames(options)).not.toContain('authorization'); + expect(typeof options?.lookup).toBe('function'); + expect(options?.agent).toBe(false); + callResponseCallback( + options, + callback, + createResponse(JSON.stringify({ sha })), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toBe( + `https://api.github.com/repos/owner/repo/git/trees/${sha}?recursive=1`, + ); + expect(headerNames(options)).not.toContain('authorization'); + expect(typeof options?.lookup).toBe('function'); + expect(options?.agent).toBe(false); + callResponseCallback( + options, + callback, + createResponse(JSON.stringify({ tree: [], truncated: false })), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toBe( + `https://codeload.github.com/owner/repo/tar.gz/${sha}`, + ); + expect(headerNames(options)).not.toContain('authorization'); + expect(typeof options?.lookup).toBe('function'); + expect(options?.agent).toBe(false); + callResponseCallback(options, callback, createResponse(archive)); + return createRequestMock(); + }) as typeof https.get); + + try { + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ...(ref ? { ref } : {}), + networkPolicy: 'public', + }, + destination, + ), + ).resolves.toBe(sha); + expect( + fsSync.existsSync( + path.join(destination, EXTENSIONS_CONFIG_FILENAME), + ), + ).toBe(true); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it('follows a limited GitHub API redirect when resolving the commit SHA', async () => { + vi.stubEnv('GITHUB_TOKEN', 'must-not-be-sent'); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'old-git-fallback-redirect-test-'), + ); + const sourceDir = path.join(tempDir, 'source'); + const destination = path.join(tempDir, 'destination'); + await fs.mkdir(path.join(sourceDir, 'repo-archive'), { + recursive: true, + }); + await fs.mkdir(destination); + await fs.writeFile( + path.join(sourceDir, 'repo-archive', EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'archive-extension', version: '1.0.0' }), + ); + const archivePath = path.join(tempDir, 'source.tar.gz'); + await tar.c({ gzip: true, file: archivePath, cwd: sourceDir }, [ + 'repo-archive', + ]); + const archive = await fs.readFile(archivePath); + const sha = 'abcdef0123456789abcdef0123456789abcdef01'; + mockHttpsGet + .mockImplementationOnce(((_url, _options, callback) => { + callResponseCallback( + _options, + callback, + createResponse(undefined, 301, { + location: + 'https://api.github.com/repos/owner/renamed/commits/HEAD', + }), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toBe( + 'https://api.github.com/repos/owner/renamed/commits/HEAD', + ); + expect(headerNames(options)).not.toContain('authorization'); + callResponseCallback( + options, + callback, + createResponse(JSON.stringify({ sha })), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + // The tree check targets the source owner/repo; rename redirects + // are followed inside fetchJson, not by the caller. + expect(String(_url)).toBe( + `https://api.github.com/repos/owner/repo/git/trees/${sha}?recursive=1`, + ); + expect(headerNames(options)).not.toContain('authorization'); + callResponseCallback( + options, + callback, + createResponse(JSON.stringify({ tree: [], truncated: false })), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toBe( + `https://codeload.github.com/owner/repo/tar.gz/${sha}`, + ); + callResponseCallback(options, callback, createResponse(archive)); + return createRequestMock(); + }) as typeof https.get); + + try { + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + destination, + ), + ).resolves.toBe(sha); + expect( + fsSync.existsSync(path.join(destination, EXTENSIONS_CONFIG_FILENAME)), + ).toBe(true); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it('rejects an invalid commit SHA before downloading the archive', async () => { + vi.stubEnv('GITHUB_TOKEN', 'must-not-be-sent'); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + mockHttpsGet.mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toContain('/commits/HEAD'); + expect(headerNames(options)).not.toContain('authorization'); + callResponseCallback( + options, + callback, + createResponse(JSON.stringify({ sha: 'not-a-valid-sha' })), + ); + return createRequestMock(); + }) as typeof https.get); + + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + '/dest', + ), + ).rejects.toThrow('GitHub returned an invalid commit SHA.'); + expect(mockHttpsGet).toHaveBeenCalledTimes(1); + }); + + it('aborts the commit SHA resolution when the signal aborts mid-request', async () => { + vi.stubEnv('GITHUB_TOKEN', 'must-not-be-sent'); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const controller = new AbortController(); + const reason = new Error('download cancelled'); + // A response body that never completes: only the abort wiring can + // settle this request, so a dropped signal hangs instead of aborting. + const hangingResponse = Object.assign(new Readable({ read() {} }), { + statusCode: 200, + headers: {}, + }) as IncomingMessage; + const request = createRequestMock(); + mockHttpsGet.mockImplementationOnce(((_url, options, callback) => { + expect(String(_url)).toContain('/commits/HEAD'); + callResponseCallback(options, callback, hangingResponse); + // Abort while the commit SHA request is still in flight. + controller.abort(reason); + return request; + }) as typeof https.get); + + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + '/dest', + controller.signal, + ), + ).rejects.toBe(reason); + expect(request.destroy).toHaveBeenCalled(); + // The archive download and every later request must be skipped. + expect(mockHttpsGet).toHaveBeenCalledTimes(1); + }); + + it.each([ + 'http://github.com/owner/repo', + 'https://gitlab.com/owner/repo', + 'https://user:pass@github.com/owner/repo', + 'https://github.com:8443/owner/repo', + 'https://github.com/owner/repo/path', + 'https://github.com/owner/repo?ref=main', + 'https://github.com/owner/repo#readme', + ])( + 'rejects an ineligible source without network access: %s', + async (source) => { + await expect( + downloadPublicGitHubArchiveFallback( + { type: 'git', source, networkPolicy: 'public' }, + '/dest', + ), + ).rejects.toThrow('Older-Git fallback'); + expect(mockHttpsGet).not.toHaveBeenCalled(); + }, + ); + + const fallbackSha = 'abcdef0123456789abcdef0123456789abcdef01'; + const gitLfsPointer = [ + 'version https://git-lfs.github.com/spec/v1', + 'oid sha256:4d7a214614ab2935c943f9e0ff69d22eadbb8f32b1258daaa5e2ca24d17e2393', + 'size 12345', + '', + ].join('\n'); + + // Builds an extension archive whose repo root contains the given files + // (paths relative to that root), then runs the real fallback download + // against it. The mocked commit-tree listing mirrors `files` unless + // overridden, e.g. to model `.gitattributes` `export-ignore` hiding a + // path from the archive. Resolves with the pinned SHA; rejects when + // validation fails. + async function runFallbackAgainstArchive( + files: Record, + treeOverride?: { + tree: Array<{ path: string; type: string }>; + truncated?: boolean; + }, + ): Promise { + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'old-git-fallback-files-test-'), + ); + const sourceDir = path.join(tempDir, 'source'); + const destination = path.join(tempDir, 'destination'); + await fs.mkdir(path.join(sourceDir, 'repo-archive'), { + recursive: true, + }); + await fs.mkdir(destination); + await fs.writeFile( + path.join(sourceDir, 'repo-archive', EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'archive-extension', version: '1.0.0' }), + ); + for (const [relativePath, contents] of Object.entries(files)) { + const filePath = path.join(sourceDir, 'repo-archive', relativePath); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, contents); + } + const archivePath = path.join(tempDir, 'source.tar.gz'); + await tar.c({ gzip: true, file: archivePath, cwd: sourceDir }, [ + 'repo-archive', + ]); + const archive = await fs.readFile(archivePath); + const treeData = treeOverride ?? { + tree: Object.keys(files).map((filePath) => ({ + path: filePath.split(path.sep).join('/'), + type: 'blob', + })), + truncated: false, + }; + mockHttpsResponses( + JSON.stringify({ sha: fallbackSha }), + JSON.stringify(treeData), + archive, + ); + + try { + return await downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + destination, + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + } + + it.each([ + [ + 'a root .gitmodules file', + { + '.gitmodules': + '[submodule "nested"]\n\tpath = nested\n\turl = https://github.com/owner/nested.git', + }, + 'submodules', + ], + [ + // codeload archives honor `.gitattributes` `export-ignore`, so a + // repository can hide its attributes file from the archive; the raw + // pointer content must still be rejected (and no `.gitattributes` + // means no grammar-only check could have seen the LFS config). + 'a Git LFS pointer file without any .gitattributes', + { 'payload.bin': gitLfsPointer }, + 'Git LFS', + ], + [ + 'a nested Git LFS pointer file', + { 'assets/payload.bin': gitLfsPointer }, + 'Git LFS', + ], + ])( + 'rejects archives containing %s', + async (_label, files, expectedError) => { + await expect(runFallbackAgainstArchive(files)).rejects.toThrow( + expectedError, + ); + }, + ); + + it('rejects a repo whose root .gitmodules is hidden from the archive via export-ignore', async () => { + // codeload strips `export-ignore` paths from the archive, so the + // extracted tree carries no `.gitmodules`; the commit tree still + // lists it (alongside the submodule gitlink), and the tree-based + // check must fail closed on it. + await expect( + runFallbackAgainstArchive( + {}, + { + tree: [ + { path: '.gitmodules', type: 'blob' }, + { path: 'nested', type: 'commit' }, + ], + }, + ), + ).rejects.toThrow('submodules'); + }); + + it('rejects a repo with a bare submodule gitlink and no .gitmodules', async () => { + await expect( + runFallbackAgainstArchive( + {}, + { tree: [{ path: 'vendor/nested', type: 'commit' }] }, + ), + ).rejects.toThrow('submodules'); + }); + + it('rejects a repo when GitHub truncates the tree listing', async () => { + await expect( + runFallbackAgainstArchive({}, { tree: [], truncated: true }), + ).rejects.toThrow('tree listing'); + }); + + it('still rejects a root .gitmodules absent from the tree listing', async () => { + // Defense in depth: even if the tree listing under-reports, the + // extracted-tree scan must keep rejecting a root `.gitmodules`. + await expect( + runFallbackAgainstArchive( + { '.gitmodules': '[submodule "nested"]\n\tpath = nested\n' }, + { tree: [] }, + ), + ).rejects.toThrow('submodules'); + }); + + // Issue #8993's repro repository (obra/superpowers) carries a root + // symlink `AGENTS.md -> CLAUDE.md`, and GitHub codeload archives + // preserve repository symlinks. The fallback's extraction chain rejects + // any archive containing a link entry, so on older Git such repositories + // still fail closed — the honest outcome is a clear rejection naming the + // link entry. Safe in-archive symlink support is tracked in #9724; this + // test must fail if the rejection is ever silently removed. + it.runIf(process.platform !== 'win32')( + 'rejects archives containing a root symlink like the issue #8993 repro repo', + async () => { + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'old-git-fallback-symlink-test-'), + ); + const sourceDir = path.join(tempDir, 'source'); + const destination = path.join(tempDir, 'destination'); + const archiveRoot = path.join(sourceDir, 'repo-archive'); + await fs.mkdir(archiveRoot, { recursive: true }); + await fs.mkdir(destination); + await fs.writeFile( + path.join(archiveRoot, EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'archive-extension', version: '1.0.0' }), + ); + // Mirrors the obra/superpowers root symlink from issue #8993. + await fs.writeFile(path.join(archiveRoot, 'CLAUDE.md'), '# agents\n'); + await fs.symlink('CLAUDE.md', path.join(archiveRoot, 'AGENTS.md')); + const archivePath = path.join(tempDir, 'source.tar.gz'); + await tar.c({ gzip: true, file: archivePath, cwd: sourceDir }, [ + 'repo-archive', + ]); + const archive = await fs.readFile(archivePath); + mockHttpsResponses( + JSON.stringify({ sha: fallbackSha }), + JSON.stringify({ tree: [], truncated: false }), + archive, + ); + + try { + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + destination, + ), + ).rejects.toThrow( + /Tar archive contains unsupported link entry: .*AGENTS\.md/, + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }, + ); + + it('accepts an archive whose only .gitmodules file is nested', async () => { + await expect( + runFallbackAgainstArchive({ + 'fixtures/.gitmodules': '[submodule "inert"]', + }), + ).resolves.toBe(fallbackSha); + }); + + it('accepts an archive with commented-out LFS attributes and no pointer content', async () => { + await expect( + runFallbackAgainstArchive({ + '.gitattributes': '# *.bin filter=lfs diff=lfs merge=lfs -text\n', + }), + ).resolves.toBe(fallbackSha); + }); + + // Builds a ustar header for a zero-content regular file (same technique + // as archive-safety.test.ts): `tar.t` parses headers via `onReadEntry` + // without requiring entry content, so a header declaring a huge size + // exercises the expanded-size ceiling without gigabytes of data. + function createTarFileHeader(name: string, size: number): Buffer { + const header = Buffer.alloc(512); + header.write(name, 0, 100, 'utf8'); + header.write('0000644\0', 100, 8); // mode + header.write('0000000\0', 108, 8); // uid + header.write('0000000\0', 116, 8); // gid + header.write(`${size.toString(8).padStart(11, '0')}\0`, 124, 12); + header.write('14763423360\0', 136, 12); // mtime + header.write(' ', 148, 8); // checksum placeholder (spaces) + header.write('0', 156, 1); // typeflag: regular file + header.write('ustar\0', 257, 6); + header.write('00', 263, 2); + let checksum = 0; + for (const byte of header) { + checksum += byte; + } + header.write(`${checksum.toString(8).padStart(6, '0')}\0 `, 148, 8); + return header; + } + + it('enforces the expanded-size limit on the downloaded fallback archive', async () => { + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'old-git-fallback-size-test-'), + ); + const destination = path.join(tempDir, 'destination'); + await fs.mkdir(destination); + const maxExpandedBytes = 1024 * 1024 * 1024; + const archive = gzipSync( + Buffer.concat([ + createTarFileHeader('big.bin', maxExpandedBytes + 1), + Buffer.alloc(1024), // tar trailer + ]), + ); + mockHttpsResponses( + JSON.stringify({ sha: fallbackSha }), + JSON.stringify({ tree: [], truncated: false }), + archive, + ); + + try { + await expect( + downloadPublicGitHubArchiveFallback( + { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + destination, + ), + ).rejects.toThrow( + `Tar archive expands beyond ${maxExpandedBytes} bytes.`, + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + }); + + describe('shouldUsePublicGitHubArchiveFallback', () => { + const gateGit = { version: vi.fn() }; + + beforeEach(() => { + vi.mocked(simpleGit).mockReturnValue(gateGit as unknown as SimpleGit); + }); + + afterEach(() => { + gateGit.version.mockReset(); + }); + + function createMetadata( + overrides: Partial = {}, + ): ExtensionInstallMetadata { + return { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + ...overrides, + }; + } + + it('uses the fallback for old Git with an anonymous public GitHub root', async () => { + gateGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + await expect( + shouldUsePublicGitHubArchiveFallback(createMetadata()), + ).resolves.toBe(true); + }); + + it('stays on pinned Git when Git is modern enough', async () => { + gateGit.version.mockResolvedValue({ major: 2, minor: 52, patch: 0 }); + await expect( + shouldUsePublicGitHubArchiveFallback(createMetadata()), + ).resolves.toBe(false); + }); + + const failClosedCases: Array<[string, Partial]> = + [ + ['stored credentials', { credentialPersistence: 'stored' }], + [ + 'a Claude marketplace config', + { + marketplaceConfig: { + name: 'marketplace', + owner: { name: 'owner', email: 'owner@example.com' }, + plugins: [], + }, + }, + ], + ['a plugin name', { pluginName: 'sample-plugin' }], + ['external content', { externalContent: true }], + ['a missing public network policy', { networkPolicy: undefined }], + ['a non-git install type', { type: 'github-release' }], + ['a non-GitHub source', { source: 'https://gitlab.com/owner/repo' }], + [ + 'a nested GitHub path', + { source: 'https://github.com/owner/repo/nested' }, + ], + ]; + + it.each(failClosedCases)( + 'stays fail-closed on old Git for %s', + async (_label, overrides) => { + gateGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + await expect( + shouldUsePublicGitHubArchiveFallback(createMetadata(overrides)), + ).resolves.toBe(false); + }, + ); + }); + describe('checkForExtensionUpdate', () => { it.skipIf(process.platform === 'win32')( 'does not try to extract uploaded archive metadata sources', @@ -690,6 +1343,82 @@ describe('git extension helpers', () => { }; } + it.each([ + [ + 'same', + '0123456789abcdef0123456789abcdef01234567', + ExtensionUpdateState.UP_TO_DATE, + ], + [ + 'different', + '89abcdef0123456789abcdef0123456789abcdef', + ExtensionUpdateState.UPDATE_AVAILABLE, + ], + ])( + 'checks old-Git public GitHub SHA when remote is %s', + async (_case, remoteSha, expected) => { + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + mockHttpsResponses(JSON.stringify({ sha: remoteSha })); + const result = await checkForExtensionUpdate( + createExtension({ + installMetadata: { + type: 'git', + source: 'https://github.com/owner/repo', + gitCommit: '0123456789abcdef0123456789abcdef01234567', + networkPolicy: 'public', + }, + }), + mockExtensionManager, + ); + + expect(result).toBe(expected); + expect(mockGit.listRemote).not.toHaveBeenCalled(); + }, + ); + + it('returns ERROR when the old-Git update check receives an invalid SHA', async () => { + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + vi.spyOn(dns, 'lookup').mockResolvedValue([ + { address: '8.8.8.8', family: 4 }, + ] as never); + mockHttpsResponses(JSON.stringify({ sha: 'not-a-valid-sha' })); + const result = await checkForExtensionUpdate( + createExtension({ + installMetadata: { + type: 'git', + source: 'https://github.com/owner/repo', + gitCommit: '0123456789abcdef0123456789abcdef01234567', + networkPolicy: 'public', + }, + }), + mockExtensionManager, + ); + + expect(result).toBe(ExtensionUpdateState.ERROR); + expect(mockGit.listRemote).not.toHaveBeenCalled(); + }); + + it('returns NOT_UPDATABLE when the old-Git install has no stored commit', async () => { + mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 }); + const result = await checkForExtensionUpdate( + createExtension({ + installMetadata: { + type: 'git', + source: 'https://github.com/owner/repo', + networkPolicy: 'public', + }, + }), + mockExtensionManager, + ); + + expect(result).toBe(ExtensionUpdateState.NOT_UPDATABLE); + expect(mockHttpsGet).not.toHaveBeenCalled(); + expect(mockGit.listRemote).not.toHaveBeenCalled(); + }); + it('should return NOT_UPDATABLE for non-git extensions', async () => { const extension = createExtension({ installMetadata: { @@ -1596,6 +2325,248 @@ describe('git extension helpers', () => { ).rejects.toBeInstanceOf(SyntaxError); }); + // The release-metadata fetch exercises fetchJson's redirect handling + // (mirrors the downloadFile redirect matrix below). + const releaseMetadata = JSON.stringify({ + assets: [ + { + name: 'extension.zip', + browser_download_url: + 'https://github.com/owner/repo/releases/download/v1.0.0/extension.zip', + }, + ], + tag_name: 'v1.0.0', + }); + + async function createReleaseArchive(): Promise { + return createZipBuffer(tempDir, [ + { + name: EXTENSIONS_CONFIG_FILENAME, + content: JSON.stringify({ + name: 'redirected-metadata-extension', + version: '1.0.0', + }), + }, + ]); + } + + it('stops following GitHub API redirect loops', async () => { + // With a network policy every hop must be re-resolved through DNS, so + // the lookup spy counts the per-hop re-validations. + const lookupSpy = vi + .spyOn(dns, 'lookup') + .mockResolvedValue([{ address: '8.8.8.8', family: 4 }] as never); + mockHttpsGet.mockImplementation(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(undefined, 302, { + location: 'https://api.github.com/repos/owner/repo/releases/next', + }), + ); + return createRequestMock(); + }) as typeof https.get); + + await expect( + downloadFromGitHubRelease( + { + source: 'owner/repo', + type: 'github-release', + networkPolicy: 'public', + }, + tempDir, + ), + ).rejects.toThrow('Too many redirects while fetching GitHub API data'); + // The initial request plus MAX_API_REDIRECTS follow-ups. + expect(mockHttpsGet).toHaveBeenCalledTimes(6); + expect(lookupSpy).toHaveBeenCalledTimes(6); + }); + + it('rejects GitHub API redirects without a location and clears the timeout', async () => { + vi.useFakeTimers(); + const lookupSpy = vi + .spyOn(dns, 'lookup') + .mockResolvedValue([{ address: '8.8.8.8', family: 4 }] as never); + const response = createResponse(undefined, 302); + const resumeSpy = vi.spyOn(response, 'resume'); + mockHttpsGet.mockImplementationOnce(((_url, options, callback) => { + callResponseCallback(options, callback, response); + return createRequestMock(); + }) as typeof https.get); + + try { + await expect( + downloadFromGitHubRelease( + { + source: 'owner/repo', + type: 'github-release', + networkPolicy: 'public', + }, + tempDir, + ), + ).rejects.toThrow('Redirect response missing location header'); + expect(resumeSpy).toHaveBeenCalled(); + // The single hop is resolved once against the network policy. + expect(lookupSpy).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it('rejects GitHub API redirect scheme downgrades before following them', async () => { + vi.stubEnv('GITHUB_TOKEN', 'secret-token'); + const lookupSpy = vi + .spyOn(dns, 'lookup') + .mockResolvedValue([{ address: '8.8.8.8', family: 4 }] as never); + mockHttpsGet.mockImplementationOnce(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(undefined, 302, { + location: 'http://api.github.com/repos/owner/repo/releases/latest', + }), + ); + return createRequestMock(); + }) as typeof https.get); + + await expect( + downloadFromGitHubRelease( + { + source: 'owner/repo', + type: 'github-release', + networkPolicy: 'public', + }, + tempDir, + ), + ).rejects.toThrow('Unsupported redirect URL protocol: http:'); + + // The downgrade is rejected before any request to the http: URL, so + // only the initial hop is resolved against the network policy. + expect(mockHttpsGet).toHaveBeenCalledTimes(1); + expect(lookupSpy).toHaveBeenCalledTimes(1); + const originalOptions = mockHttpsGet.mock.calls[0][1] as + | https.RequestOptions + | undefined; + expect(originalOptions?.headers).toMatchObject({ + Authorization: 'token secret-token', + }); + }); + + it('does not forward the GitHub token to cross-host GitHub API redirects', async () => { + vi.stubEnv('GITHUB_TOKEN', 'secret-token'); + const lookupSpy = vi + .spyOn(dns, 'lookup') + .mockResolvedValue([{ address: '8.8.8.8', family: 4 }] as never); + const archive = await createReleaseArchive(); + mockHttpsGet + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(undefined, 302, { + location: 'https://objects.githubusercontent.com/metadata', + }), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(releaseMetadata), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback(options, callback, createResponse(archive)); + return createRequestMock(); + }) as typeof https.get); + + await downloadFromGitHubRelease( + { + source: 'owner/repo', + type: 'github-release', + networkPolicy: 'public', + }, + tempDir, + ); + + const originalOptions = mockHttpsGet.mock.calls[0][1] as + | https.RequestOptions + | undefined; + const redirectedOptions = mockHttpsGet.mock.calls[1][1] as + | https.RequestOptions + | undefined; + expect(originalOptions?.headers).toMatchObject({ + Authorization: 'token secret-token', + }); + expect(redirectedOptions?.headers).toEqual({ + 'User-Agent': 'gemini-cli', + }); + // Every hop (initial API, redirected API, archive download) is + // re-resolved against the network policy and carries the pinned + // lookup, so a redirect can never escape to a freshly resolved + // blocked address. + expect(lookupSpy).toHaveBeenCalledTimes(3); + for (const call of mockHttpsGet.mock.calls) { + const hopOptions = call[1] as https.RequestOptions | undefined; + expect(typeof hopOptions?.lookup).toBe('function'); + expect(hopOptions?.agent).toBe(false); + } + }); + + it('keeps the GitHub token for same-host GitHub API redirects', async () => { + vi.stubEnv('GITHUB_TOKEN', 'secret-token'); + const lookupSpy = vi + .spyOn(dns, 'lookup') + .mockResolvedValue([{ address: '8.8.8.8', family: 4 }] as never); + const archive = await createReleaseArchive(); + mockHttpsGet + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(undefined, 302, { + location: + 'https://api.github.com/repos/owner/renamed/releases/latest', + }), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback( + options, + callback, + createResponse(releaseMetadata), + ); + return createRequestMock(); + }) as typeof https.get) + .mockImplementationOnce(((_url, options, callback) => { + callResponseCallback(options, callback, createResponse(archive)); + return createRequestMock(); + }) as typeof https.get); + + await downloadFromGitHubRelease( + { + source: 'owner/repo', + type: 'github-release', + networkPolicy: 'public', + }, + tempDir, + ); + + const redirectedOptions = mockHttpsGet.mock.calls[1][1] as + | https.RequestOptions + | undefined; + expect(redirectedOptions?.headers).toMatchObject({ + Authorization: 'token secret-token', + }); + // Initial API hop + redirected hop + archive download, each + // re-resolved against the network policy. + expect(lookupSpy).toHaveBeenCalledTimes(3); + }); + it('should explain when a release archive is missing an extension manifest', async () => { const invalidArchive = await createZipBuffer(tempDir, [ { name: 'README.md', content: 'not an extension' }, diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index 2792d75418b..c2b02d458f2 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -6,6 +6,7 @@ import type { SimpleGit } from 'simple-git'; import { getErrorMessage } from '../utils/errors.js'; +import * as crypto from 'node:crypto'; import * as os from 'node:os'; import * as https from 'node:https'; import * as fs from 'node:fs'; @@ -30,7 +31,10 @@ import { AGENT_PLUGIN_MANIFEST, getAgentPluginSchemaStatus, } from './agent-plugins-v1/manifest.js'; -import { assertTarArchiveHasNoLinks } from './archive-safety.js'; +import { + assertTarArchiveHasNoLinks, + type TarArchiveSafetyOptions, +} from './archive-safety.js'; import { resolveNetworkTarget } from './network-policy.js'; import { extractZipArchive } from './zip-extraction.js'; import { loadSimpleGit } from '../utils/load-simple-git.js'; @@ -53,6 +57,20 @@ interface GithubReleaseData { zipball_url?: string; } +interface GitHubCommitData { + sha: string; +} + +interface GitHubTreeEntry { + path?: string; + type?: string; +} + +interface GitHubTreeData { + tree?: GitHubTreeEntry[]; + truncated?: boolean; +} + interface Asset { name: string; browser_download_url: string; @@ -136,19 +154,52 @@ function getGitHubCredential(source: string): GitCredential | undefined { return undefined; } -async function assertPinnedGitSupported(): Promise { - const { simpleGit } = await loadSimpleGit(); - const version = await simpleGit().version(); - if ( - version.major < MINIMUM_PINNED_GIT_VERSION.major || +type LocalGitVersion = { + major: number; + minor: number; + patch?: number | string; +}; + +// The local Git version cannot change within a process lifetime, so probe it +// once instead of spawning a `git version` subprocess from both the fallback +// gate and the pinned-Git assert for every extension. +let localGitVersionPromise: Promise | undefined; + +function getLocalGitVersion(): Promise { + localGitVersionPromise ??= (async () => { + const { simpleGit } = await loadSimpleGit(); + return await simpleGit().version(); + })(); + return localGitVersionPromise; +} + +export function resetLocalGitVersionCacheForTesting(): void { + localGitVersionPromise = undefined; +} + +function isPinnedGitVersionSupported(version: { + major: number; + minor: number; +}): boolean { + return ( + version.major > MINIMUM_PINNED_GIT_VERSION.major || (version.major === MINIMUM_PINNED_GIT_VERSION.major && - version.minor < MINIMUM_PINNED_GIT_VERSION.minor) - ) { + version.minor >= MINIMUM_PINNED_GIT_VERSION.minor) + ); +} + +async function isPinnedGitSupported(): Promise { + return isPinnedGitVersionSupported(await getLocalGitVersion()); +} + +async function assertPinnedGitSupported(): Promise { + const version = await getLocalGitVersion(); + if (!isPinnedGitVersionSupported(version)) { const detectedVersion = [version.major, version.minor, version.patch] .filter((component) => component !== undefined) .join('.'); throw new Error( - `Public extension Git installs require Git 2.37 or newer for secure DNS pinning; found Git ${detectedVersion}. Upgrade Git, or install the extension from a local path or archive instead.`, + `Public extension Git installs require Git 2.37 or newer unless the source is an anonymous public GitHub root repository; found Git ${detectedVersion}. Upgrade Git for credentialed, non-GitHub, nested, submodule, or Git LFS installs.`, ); } } @@ -327,6 +378,195 @@ export function parseGitHubRepoForReleases(source: string): { return { owner, repo }; } +function parseAnonymousPublicGitHubRepo(source: string): { + owner: string; + repo: string; +} { + let url: URL; + try { + url = new URL(source); + } catch { + throw new Error('Older-Git fallback requires a valid GitHub HTTPS URL.'); + } + if ( + url.protocol !== 'https:' || + url.hostname !== 'github.com' || + url.port || + url.username || + url.password || + url.search || + url.hash + ) { + throw new Error( + 'Older-Git fallback only supports anonymous https://github.com/{owner}/{repo}[.git] sources.', + ); + } + const match = /^\/([^/]+)\/([^/]+?)(?:\.git)?\/?$/.exec(url.pathname); + if (!match || !match[1] || !match[2]) { + throw new Error( + 'Older-Git fallback only supports a GitHub repository root URL.', + ); + } + return { owner: match[1], repo: match[2] }; +} + +export async function shouldUsePublicGitHubArchiveFallback( + installMetadata: ExtensionInstallMetadata, +): Promise { + if ( + installMetadata.type !== 'git' || + installMetadata.networkPolicy !== 'public' || + installMetadata.credentialPersistence || + installMetadata.marketplaceConfig || + installMetadata.pluginName || + installMetadata.externalContent + ) { + return false; + } + try { + parseAnonymousPublicGitHubRepo(installMetadata.source); + } catch { + return false; + } + return !(await isPinnedGitSupported()); +} + +// A Git LFS pointer is a small text file (~130 bytes). Only files small +// enough to plausibly be pointers are read, keeping the scan cheap. +const GIT_LFS_POINTER_PREFIX = 'version https://git-lfs.github.com/spec/v1'; +const MAX_LFS_POINTER_SCAN_BYTES = 512; + +async function assertArchivePreservesGitSemantics(destination: string) { + const pending = [destination]; + while (pending.length > 0) { + const directory = pending.pop()!; + const isArchiveRoot = directory === destination; + for (const entry of await fs.promises.readdir(directory, { + withFileTypes: true, + })) { + const entryPath = path.join(directory, entry.name); + if (entry.isDirectory()) { + pending.push(entryPath); + continue; + } + if (!entry.isFile()) { + continue; + } + // Git gives submodule semantics only to a root-level `.gitmodules`; + // a nested copy is an inert regular file. + if (isArchiveRoot && entry.name === '.gitmodules') { + throw new Error( + 'Older-Git fallback does not support repositories with submodules.', + ); + } + // Detect Git LFS by pointer-file content rather than `.gitattributes` + // grammar: codeload archives honor `.gitattributes` `export-ignore`, + // so the attributes file itself can be hidden from the extracted tree, + // and attribute macros or case-variant names also bypass a + // grammar-only check. Any LFS-tracked file arrives as a raw pointer + // file, so scanning for pointer content catches every variant. + const stats = await fs.promises.stat(entryPath); + if (stats.size > MAX_LFS_POINTER_SCAN_BYTES) { + continue; + } + const content = await fs.promises.readFile(entryPath, 'utf8'); + if (content.startsWith(GIT_LFS_POINTER_PREFIX)) { + throw new Error( + 'Older-Git fallback does not support repositories using Git LFS.', + ); + } + } + } +} + +// codeload archives honor `.gitattributes` `export-ignore`, so a repository +// can strip its root `.gitmodules` from the archive and slip past the +// extracted-tree presence check above. The commit's tree object still lists +// every path regardless of export-ignore, so verify it directly: a root +// `.gitmodules` blob or any gitlink (type `commit`) entry means the archive +// would silently drop submodule content. +async function assertGitHubTreeHasNoSubmodules( + owner: string, + repo: string, + commitSha: string, + signal?: AbortSignal, +): Promise { + const treeData = await fetchJson( + `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/git/trees/${encodeURIComponent(commitSha)}?recursive=1`, + signal, + 'public', + false, + ); + if (treeData.truncated) { + throw new Error( + 'Older-Git fallback cannot verify that the repository is free of submodules because GitHub truncated the tree listing.', + ); + } + const entries = Array.isArray(treeData.tree) ? treeData.tree : []; + const hasSubmoduleSemantics = entries.some( + (entry) => entry?.path === '.gitmodules' || entry?.type === 'commit', + ); + if (hasSubmoduleSemantics) { + throw new Error( + 'Older-Git fallback does not support repositories with submodules.', + ); + } +} + +async function resolvePublicGitHubCommitSha( + owner: string, + repo: string, + ref: string, + signal?: AbortSignal, +): Promise { + const commitData = await fetchJson( + `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/${encodeURIComponent(ref)}`, + signal, + 'public', + false, + ); + if (!/^[a-f0-9]{40}$/i.test(commitData.sha)) { + throw new Error('GitHub returned an invalid commit SHA.'); + } + return commitData.sha.toLowerCase(); +} + +export async function downloadPublicGitHubArchiveFallback( + installMetadata: ExtensionInstallMetadata, + destination: string, + signal?: AbortSignal, +): Promise { + const { owner, repo } = parseAnonymousPublicGitHubRepo( + installMetadata.source, + ); + const commitSha = await resolvePublicGitHubCommitSha( + owner, + repo, + installMetadata.ref || 'HEAD', + signal, + ); + await assertGitHubTreeHasNoSubmodules(owner, repo, commitSha, signal); + // A random staging name avoids clobbering (or being filtered out as) a + // repository file that happens to share the archive name. + const archivePath = path.join( + destination, + `github-source-${crypto.randomUUID()}.tar.gz`, + ); + await downloadFile( + `https://codeload.github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/tar.gz/${commitSha}`, + archivePath, + { includeGitHubToken: false, networkPolicy: 'public' }, + 0, + signal, + ); + await extractArchiveFile(archivePath, destination, signal, { + enforceResourceLimits: true, + }); + await fs.promises.unlink(archivePath); + await assertArchivePreservesGitSemantics(destination); + return commitSha; +} + async function fetchReleaseFromGithub( owner: string, repo: string, @@ -477,6 +717,23 @@ export async function checkForExtensionUpdate( installMetadata.credentialPersistence === 'stored' ? (await resolveStoredGitCredential(extension.path)).credential : undefined; + if (await shouldUsePublicGitHubArchiveFallback(installMetadata)) { + if (!installMetadata.gitCommit) { + return ExtensionUpdateState.NOT_UPDATABLE; + } + const { owner, repo } = parseAnonymousPublicGitHubRepo( + installMetadata.source, + ); + const remoteSha = await resolvePublicGitHubCommitSha( + owner, + repo, + installMetadata.ref || 'HEAD', + signal, + ); + return remoteSha === installMetadata.gitCommit + ? ExtensionUpdateState.UP_TO_DATE + : ExtensionUpdateState.UPDATE_AVAILABLE; + } const { simpleGit } = await loadSimpleGit(); if (installMetadata.networkPolicy === 'public') { await assertPinnedGitSupported(); @@ -731,6 +988,7 @@ export async function extractArchiveFile( archivePath: string, destination: string, signal?: AbortSignal, + options: TarArchiveSafetyOptions = {}, ): Promise { signal?.throwIfAborted(); if (!isSupportedArchivePath(archivePath)) { @@ -739,7 +997,7 @@ export async function extractArchiveFile( ); } try { - await extractFile(archivePath, destination, signal); + await extractFile(archivePath, destination, signal, options); } catch (error) { signal?.throwIfAborted(); throw new Error( @@ -790,10 +1048,14 @@ export function findReleaseAsset(assets: Asset[]): Asset | undefined { return undefined; } +const MAX_API_REDIRECTS = 5; + async function fetchJson( url: string, signal?: AbortSignal, networkPolicy?: ExtensionInstallMetadata['networkPolicy'], + includeGitHubToken = true, + redirectCount = 0, ): Promise { const timeoutError = new Error('Timed out fetching GitHub API response'); const timeoutController = new AbortController(); @@ -809,7 +1071,7 @@ async function fetchJson( 'User-Agent': 'gemini-cli', }; const token = getGitHubToken(); - if (token) { + if (includeGitHubToken && token) { headers.Authorization = `token ${token}`; } let target; @@ -856,6 +1118,52 @@ async function fetchJson( }, (res) => { res.on('error', fail); + if ( + res.statusCode === 301 || + res.statusCode === 302 || + res.statusCode === 307 || + res.statusCode === 308 + ) { + res.resume(); + if (redirectCount >= MAX_API_REDIRECTS) { + return fail( + new Error('Too many redirects while fetching GitHub API data'), + ); + } + if (!res.headers.location) { + return fail( + new Error('Redirect response missing location header'), + ); + } + let redirectUrl: URL; + try { + redirectUrl = new URL(res.headers.location, url); + } catch (error) { + return fail( + new Error(`Invalid redirect URL: ${getErrorMessage(error)}`), + ); + } + if (redirectUrl.protocol !== 'https:') { + return fail( + new Error( + `Unsupported redirect URL protocol: ${redirectUrl.protocol}`, + ), + ); + } + cleanup(); + // Every hop is re-resolved against the network policy above, and + // the token never follows a redirect to a different host. + fetchJson( + redirectUrl.toString(), + signal, + networkPolicy, + redirectUrl.host === target.url.host ? includeGitHubToken : false, + redirectCount + 1, + ) + .then(finish) + .catch(fail); + return; + } if (res.statusCode !== 200) { res.resume(); return fail( @@ -1055,10 +1363,11 @@ export async function extractFile( file: string, dest: string, signal?: AbortSignal, + options: TarArchiveSafetyOptions = {}, ): Promise { signal?.throwIfAborted(); if (file.endsWith('.tar.gz')) { - await assertTarArchiveHasNoLinks(file, signal); + await assertTarArchiveHasNoLinks(file, signal, options); signal?.throwIfAborted(); try { await pipeline(fs.createReadStream(file), tar.x({ cwd: dest }), { diff --git a/packages/core/src/extension/npm.test.ts b/packages/core/src/extension/npm.test.ts index 295a914e001..48460f222c4 100644 --- a/packages/core/src/extension/npm.test.ts +++ b/packages/core/src/extension/npm.test.ts @@ -18,6 +18,7 @@ import { promises as dns } from 'node:dns'; vi.mock('node:fs', () => ({ readFileSync: vi.fn(), existsSync: vi.fn(), + createReadStream: vi.fn(() => ({ destroy: vi.fn() }) as never), createWriteStream: vi.fn(), promises: { readdir: vi.fn(), @@ -28,6 +29,10 @@ vi.mock('node:fs', () => ({ }, })); +vi.mock('node:stream/promises', () => ({ + pipeline: vi.fn().mockResolvedValue(undefined), +})); + describe('parseNpmPackageSource', () => { it('should parse scoped package without version', () => { const result = parseNpmPackageSource('@ali/openclaw-tmcp-dingtalk'); @@ -906,6 +911,17 @@ describe('downloadFromNpmRegistry', () => { ), ).rejects.toThrow('more than 100 unsupported link entries'); expect(tar.x).not.toHaveBeenCalled(); + + // Tripping the link-count cap makes failValidation destroy the read + // stream; the mocked stream must support that cleanly. Before the + // createReadStream mock returned a destroyable object, this path raised + // a TypeError inside the tar.t mock instead of completing. + const createdStream = vi.mocked(fs.createReadStream).mock.results[0] + ?.value as { destroy: ReturnType } | undefined; + expect(createdStream?.destroy).toHaveBeenCalled(); + await expect( + vi.mocked(tar.t).mock.results[0]?.value as Promise, + ).resolves.toBeUndefined(); }); it('stops between tar inspection and extraction when cancelled', async () => {