diff --git a/packages/cli/src/create/__tests__/org-manifest.spec.ts b/packages/cli/src/create/__tests__/org-manifest.spec.ts index b4d42d6117..d0741e592e 100644 --- a/packages/cli/src/create/__tests__/org-manifest.spec.ts +++ b/packages/cli/src/create/__tests__/org-manifest.spec.ts @@ -509,4 +509,30 @@ describe('readOrgManifest', () => { /version "9\.9\.9" not found/, ); }); + + it.each([ + ['latest', '../../outside-write'], + ['latest', '..'], + ['latest', '.'], + ['latest', 'not-a-version'], + // A tag named like a pinned version can also substitute an invalid target. + ['1.2.3', '../../outside-write'], + ])('rejects a non-semver dist-tags.%s target: %s', async (tag, version) => { + const body = packument([ + { name: 'web', description: 'v1', template: '@your-org/template-web' }, + ]); + // Keep matching metadata so only the version validation rejects it. + mockFetchJson({ + ...body, + 'dist-tags': { ...body['dist-tags'], [tag]: version }, + versions: { + ...body.versions, + [version]: { ...body.versions['1.0.0'], version }, + }, + }); + const requestedVersion = tag === 'latest' ? undefined : tag; + const manifest = readOrgManifest('@your-org', requestedVersion); + await expect(manifest).rejects.toThrow(OrgManifestSchemaError); + await expect(manifest).rejects.toThrow(/invalid version/); + }); }); diff --git a/packages/cli/src/create/__tests__/org-tarball.spec.ts b/packages/cli/src/create/__tests__/org-tarball.spec.ts index c97059f030..c4a95d22cb 100644 --- a/packages/cli/src/create/__tests__/org-tarball.spec.ts +++ b/packages/cli/src/create/__tests__/org-tarball.spec.ts @@ -2,16 +2,23 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { OrgManifest } from '../org-manifest.js'; import { cleanupStaleStagingDirs, + ensureOrgPackageExtracted, normalizeEntryName, parseEntryMode, resolveBundledPath, + resolveExtractionDir, sanitizeHostForPath, } from '../org-tarball.js'; +const { mockGetVpDirs } = vi.hoisted(() => ({ mockGetVpDirs: vi.fn() })); + +vi.mock('../../../binding/index.js', () => ({ getVpDirs: mockGetVpDirs })); + describe('resolveBundledPath', () => { const scratchDirs: string[] = []; @@ -88,6 +95,62 @@ describe('normalizeEntryName', () => { }); }); +function manifestFor(version: string): OrgManifest { + return { + scope: '@your-org', + packageName: '@your-org/create', + version, + tarballUrl: 'https://registry.npmjs.org/@your-org/create/-/create-1.0.0.tgz', + templates: [], + }; +} + +describe('resolveExtractionDir', () => { + const cacheRoot = path.resolve(os.tmpdir(), 'vp-cache-root'); + + it.each(['1.0.0', '0.0.0', '1.2.3', '2.0.0-beta.1', '10.20.30+build.5'])( + 'places version %s beneath the cache root', + (version) => { + expect(resolveExtractionDir(cacheRoot, manifestFor(version))).toBe( + path.join(cacheRoot, 'registry.npmjs.org', '@your-org', 'create', version), + ); + }, + ); + + // Three `..` segments reach the cache root; four escape it. + it.each(['../../..', '../../../../outside', '../../../../../../outside-write', '/absolute'])( + 'rejects a version that resolves to or outside the cache root: %s', + (version) => { + expect(() => resolveExtractionDir(cacheRoot, manifestFor(version))).toThrow( + /escapes the cache root/, + ); + }, + ); +}); + +describe('ensureOrgPackageExtracted', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('rejects the cache root before filesystem or network activity', async () => { + mockGetVpDirs.mockReturnValue({ cache: path.join(os.tmpdir(), 'vp-org-extraction-cache') }); + const exists = vi.spyOn(fs, 'existsSync').mockReturnValue(false); + const mkdir = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + const readdir = vi.spyOn(fs.promises, 'readdir').mockResolvedValue([]); + const fetch = vi.spyOn(globalThis, 'fetch').mockRejectedValue(new Error('unexpected download')); + + await expect(ensureOrgPackageExtracted(manifestFor('../../..'))).rejects.toThrow( + /escapes the cache root/, + ); + + expect(exists).not.toHaveBeenCalled(); + expect(mkdir).not.toHaveBeenCalled(); + expect(readdir).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + }); +}); + describe('sanitizeHostForPath', () => { it('passes through plain hostnames untouched', () => { expect(sanitizeHostForPath('registry.npmjs.org')).toBe('registry.npmjs.org'); diff --git a/packages/cli/src/create/org-manifest.ts b/packages/cli/src/create/org-manifest.ts index 70264273ce..37965e8440 100644 --- a/packages/cli/src/create/org-manifest.ts +++ b/packages/cli/src/create/org-manifest.ts @@ -1,5 +1,7 @@ import path from 'node:path'; +import semver from 'semver'; + import { fetchNpmResource, getNpmRegistry } from '../utils/npm-config.ts'; import { readPackageJsonFromTarball } from './org-tarball.ts'; @@ -333,6 +335,14 @@ export async function readOrgManifest( return null; } } + // Registry versions become cache-path components, so reject malformed + // values even when the registry has matching version metadata. + if (semver.valid(resolvedVersion) === null) { + throw new OrgManifestSchemaError( + `invalid version "${resolvedVersion}" (expected a semantic version)`, + packageName, + ); + } const meta = packument.versions?.[resolvedVersion]; if (!meta) { return null; diff --git a/packages/cli/src/create/org-tarball.ts b/packages/cli/src/create/org-tarball.ts index 0a2db67987..8efa3715ee 100644 --- a/packages/cli/src/create/org-tarball.ts +++ b/packages/cli/src/create/org-tarball.ts @@ -28,16 +28,24 @@ export function sanitizeHostForPath(host: string): string { * (via `.npmrc` scope mappings) don't share a cache slot. The registry * guarantees `manifest.tarballUrl` is a valid URL, so any parse failure * here is a real bug worth surfacing. + * + * Require a strict descendant of `cacheRoot` so sibling staging directories + * stay inside the cache, even if a caller skips `readOrgManifest` validation. */ -function getExtractionDir(manifest: OrgManifest): string { +export function resolveExtractionDir(cacheRoot: string, manifest: OrgManifest): string { const { host } = new URL(manifest.tarballUrl); - return path.join( - getCacheRoot(), + const resolvedRoot = path.resolve(cacheRoot); + const resolvedDir = path.resolve( + resolvedRoot, sanitizeHostForPath(host), manifest.scope, 'create', manifest.version, ); + if (!resolvedDir.startsWith(`${resolvedRoot}${path.sep}`)) { + throw new Error(`org template extraction path escapes the cache root: ${manifest.version}`); + } + return resolvedDir; } function parseIntegrity(integrity: string): { algorithm: string; expected: string } | null { @@ -295,7 +303,7 @@ export async function cleanupStaleStagingDirs(destDir: string): Promise { * cleans up and returns the existing directory. */ export async function ensureOrgPackageExtracted(manifest: OrgManifest): Promise { - const extractedRoot = getExtractionDir(manifest); + const extractedRoot = resolveExtractionDir(getCacheRoot(), manifest); if (fs.existsSync(path.join(extractedRoot, 'package.json'))) { return extractedRoot; }