fix(create): reject extraction at the cache root

Author: fengmk2

packages/cli/src/create/__tests__/org-tarball.spec.ts

@@ -2,18 +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[] = [];
 
@@ -112,9 +117,9 @@ describe('resolveExtractionDir', () => {
     },
   );
 
-  // Escaping `<root>/<host>/<scope>/create` takes at least four `..` segments.
-  it.each(['../../../../outside', '../../../../../../outside-write', '/absolute'])(
-    'rejects a version that escapes the cache root: %s',
+  // 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/,
@@ -123,6 +128,29 @@ describe('resolveExtractionDir', () => {
   );
 });
 
+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-tarball.ts

@@ -29,8 +29,8 @@ export function sanitizeHostForPath(host: string): string {
  * guarantees `manifest.tarballUrl` is a valid URL, so any parse failure
  * here is a real bug worth surfacing.
  *
- * Check containment here as well, in case a caller skips the version
- * validation in `readOrgManifest`.
+ * Require a strict descendant of `cacheRoot` so sibling staging directories
+ * stay inside the cache, even if a caller skips `readOrgManifest` validation.
  */
 export function resolveExtractionDir(cacheRoot: string, manifest: OrgManifest): string {
   const { host } = new URL(manifest.tarballUrl);
@@ -42,7 +42,7 @@ export function resolveExtractionDir(cacheRoot: string, manifest: OrgManifest):
     'create',
     manifest.version,
   );
-  if (resolvedDir !== resolvedRoot && !resolvedDir.startsWith(`${resolvedRoot}${path.sep}`)) {
+  if (!resolvedDir.startsWith(`${resolvedRoot}${path.sep}`)) {
     throw new Error(`org template extraction path escapes the cache root: ${manifest.version}`);
   }
   return resolvedDir;