fix(create): reject non-semver versions in org-template manifests (#2665)

Malformed registry versions can place org-template files outside the
cache root. `readOrgManifest()` now rejects versions that fail
`semver.valid()`, including targets from `dist-tags.latest` and tags
that match a requested version.

`resolveExtractionDir()` also checks that the extraction path stays
within the cache root. Tests cover invalid versions, path traversal, and
valid versions with prerelease or build metadata.

Author: fengmk2

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/);
+  });
 });

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');

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;

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<void> {
  * cleans up and returns the existing directory.
  */
 export async function ensureOrgPackageExtracted(manifest: OrgManifest): Promise<string> {
-  const extractedRoot = getExtractionDir(manifest);
+  const extractedRoot = resolveExtractionDir(getCacheRoot(), manifest);
   if (fs.existsSync(path.join(extractedRoot, 'package.json'))) {
     return extractedRoot;
   }