feat: support corepack packageManager and add related unit tests (electron-userland#9309)

- This rewrites the `yarn` parser to no longer use `npm list` and
instead use `yarn list`. The JSON format is entirely different and
doesn't possess the same tree setup that `npm list` returns, but in this
migration, we can officially support `corepack` for yarn **classic** (as
`npm list` won't run if `corepack enable` has run).
- Adds new functionality, dedicated support for Yarn Berry. Only Yarn v1
can use CLI. We should use pnp.cjs for PnP, but we can't access the
files due to virtual file paths within zipped modules. (e.g.
`<package_name>.zip/<file_path>`) We fallback to npm node module
collection (since Yarn Berry could have npm-like structure OR pnpm-like
structure, depending on `nmHoistingLimits` configuration). In the latter
case, we still can't assume `pnpm` is installed, so we still try to use
npm collection as a best-effort attempt.
- If all options fail for npm or yarn berry....added new functionality
to parse the node_module tree manually as the ultimate fallback 😅
 
All filesystem operations are async and memoized.

Migrated `packTester` to leverage `corepack` for isolating text/fixture
environments and the various installation/hoisting setups they can have.
Using specific installation methods in new unit tests (+ snapshot
verification) of package manager implementations

Author: mmaietta

.changeset/legal-adults-fail.md

@@ -0,0 +1,6 @@
+---
+"electron-builder": minor
+"app-builder-lib": minor
+---
+
+feat: support `corepack` and `packageManager` field and add related unit tests

.github/workflows/test.yaml

@@ -70,7 +70,7 @@ jobs:
           - ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configurationValidationTest,filenameUtilTest,filesTest,globTest,httpExecutorTest,ignoreTest,macroExpanderTest,mainEntryTest,urlUtilTest,extraMetadataTest,linuxArchiveTest,linuxPackagerTest,HoistedNodeModuleTest,MemoLazyTest,HoistTest,ExtraBuildResourcesTest,utilTest
           - snapTest,debTest,fpmTest,protonTest
           - winPackagerTest,winCodeSignTest,webInstallerTest
-          - oneClickInstallerTest,assistedInstallerTest
+          - oneClickInstallerTest,assistedInstallerTest,packageManagerTest
           - concurrentBuildsTest
     steps:
       - name: Checkout code repository
@@ -218,7 +218,7 @@ jobs:
       fail-fast: false
       matrix:
         testFiles:
-          - oneClickInstallerTest,assistedInstallerTest,webInstallerTest
+          - oneClickInstallerTest,assistedInstallerTest,webInstallerTest,packageManagerTest,HoistedNodeModuleTest
           - winPackagerTest,winCodeSignTest,BuildTest,blackboxUpdateTest
           - masTest,dmgTest,filesTest,macPackagerTest,differentialUpdateTest,macArchiveTest
           - concurrentBuildsTest

.vscode/launch.json

@@ -1,18 +1,25 @@
 {
   "version": "0.2.0",
   "configurations": [
-      {
-          "type": "node",
-          "request": "launch",
-          "name": "Vitest TEST_FILES",
-          "runtimeExecutable": "pnpm",
-          "program": "ci:test",
-          "console": "integratedTerminal",
-          "internalConsoleOptions": "openOnFirstSessionStart",
-          "env": {
-            "TEST_FILES": "macPackagerTest",
-            "UPDATE_SNAPSHOT": "false"
-          }
-      }
+    {
+      "type": "node",
+      "request": "launch",
+      "name": "Vitest TEST_FILES",
+      "runtimeExecutable": "pnpm",
+      "program": "ci:test",
+      "console": "integratedTerminal",
+      "internalConsoleOptions": "openOnFirstSessionStart",
+      "env": {
+        "TEST_FILES": "packageManagerTest",
+        "UPDATE_SNAPSHOT": "false",
+        "DEBUG": "electron-builder",
+        "TEST_SEQUENTIAL": "true" // Run tests sequentially to debug issues that may be hidden by concurrency (console log pollution when DEBUG flag set)
+      },
+      "skipFiles": [
+        "<node_internals>/**", // Skip Node’s internal modules
+        "${workspaceFolder}/**/node_modules/**/*.js", // Skip libraries
+        "**/*.js" // Optionally skip all compiled JS
+      ]
+    }
   ]
 }

package.json

@@ -19,7 +19,7 @@
     "prettier": "prettier 'packages/**/*.{ts,js}' test/src/**/*.ts --write",
     "///": "Please see https://github.com/electron-userland/electron-builder/blob/master/CONTRIBUTING.md#run-test-using-cli how to run particular test instead full (and very slow) run",
     "test-all": "pnpm compile && pnpm pretest && pnpm ci:test",
-    "test-linux": "docker run --rm -e CI=${CI:-1} -e DEBUG=${DEBUG:-} -e UPDATE_SNAPSHOT=${UPDATE_SNAPSHOT:-false} -e TEST_FILES=\"${TEST_FILES:-HoistedNodeModuleTest}\" -w /project -v $(pwd):/project -v $(pwd)-node-modules:/project/node_modules -v $HOME/Library/Caches/electron:/root/.cache/electron -v $HOME/Library/Caches/electron-builder:/root/.cache/electron-builder ${ADDITIONAL_DOCKER_ARGS} ${TEST_RUNNER_IMAGE_TAG:-electronuserland/builder:22-wine-mono} /bin/bash -c \"corepack enable && pnpm install && pnpm ci:test\"",
+    "test-linux": "docker run --rm -e CI=${CI:-true} -e DEBUG=${DEBUG:-} -e UPDATE_SNAPSHOT=${UPDATE_SNAPSHOT:-false} -e TEST_FILES=\"${TEST_FILES:-HoistedNodeModuleTest}\" -w /project -v $(pwd):/project -v $(pwd)-node-modules:/project/node_modules -v $HOME/Library/Caches/electron:/root/.cache/electron -v $HOME/Library/Caches/electron-builder:/root/.cache/electron-builder ${ADDITIONAL_DOCKER_ARGS} ${TEST_RUNNER_IMAGE_TAG:-electronuserland/builder:22-wine-mono} /bin/bash -c \"corepack enable && pnpm install && pnpm ci:test\"",
     "test-update": "UPDATE_SNAPSHOT=true vitest run",
     "test-ui": "vitest --ui",
     "docker-images": "docker/build.sh",
@@ -28,7 +28,7 @@
     "generate-changeset": "pnpm changeset",
     "generate-schema": "typescript-json-schema packages/app-builder-lib/tsconfig-scheme.json Configuration --out packages/app-builder-lib/scheme.json --noExtraProps --useTypeOfKeyword --strictNullChecks --required && node ./scripts/fix-schema.js",
     "generate-all": "pnpm generate-schema && pnpm prettier",
-    "ci:test": "vitest run --no-update",
+    "ci:test": "vitest run --no-update --allowOnly",
     "ci:version": "pnpm i && pnpm changelog && changeset version && node scripts/update-package-version-export.js && pnpm compile && pnpm generate-all && git add .",
     "ci:publish": "pnpm i && pnpm compile && pnpm publish -r --tag next && changeset tag",
     "docs:prebuild": "docker build -t mkdocs-dockerfile -f mkdocs-dockerfile . ",

packages/app-builder-lib/src/asar/asarUtil.ts

@@ -9,6 +9,7 @@ import { PlatformPackager } from "../platformPackager"
 import { ResolvedFileSet, getDestinationPath } from "../util/appFileCopier"
 import { detectUnpackedDirs } from "./unpackDetector"
 import { Readable } from "stream"
+import * as os from "os"
 
 /** @internal */
 export class AsarPackager {
@@ -122,7 +123,7 @@ export class AsarPackager {
     transformedData: string | Buffer | undefined
     isUnpacked: (path: string) => boolean
   }): Promise<AsarStreamType> {
-    const { isUnpacked, transformedData, file, destination, stat, fileSet } = options
+    const { isUnpacked, transformedData, file, destination, stat } = options
     const unpacked = isUnpacked(destination)
 
     if (!stat.isFile() && !stat.isSymbolicLink()) {
@@ -143,13 +144,8 @@ export class AsarPackager {
       return { path: destination, streamGenerator, unpacked, type: "file", stat: { mode: stat.mode, size } }
     }
 
-    const realPathFile = await fs.realpath(file)
-    const realPathRelative = path.relative(fileSet.src, realPathFile)
-    const isOutsidePackage = realPathRelative.startsWith("..")
-    if (isOutsidePackage) {
-      log.error({ source: log.filePath(file), realPathFile: log.filePath(realPathFile) }, `unable to copy, file is symlinked outside the package`)
-      throw new Error(`Cannot copy file (${path.basename(file)}) symlinked to file (${path.basename(realPathFile)}) outside the package as that violates asar security integrity`)
-    }
+    // verify that the file is not a direct link or symlinked to access/copy a system file
+    await this.protectSystemAndUnsafePaths(file)
 
     const config = {
       path: destination,
@@ -158,14 +154,17 @@ export class AsarPackager {
       stat,
     }
 
-    // not a symlink, stream directly
-    if (file === realPathFile) {
+    // file, stream directly
+    if (!stat.isSymbolicLink()) {
       return {
         ...config,
         type: "file",
       }
     }
 
+    // guard against symlink pointing to outside workspace root
+    await this.protectSystemAndUnsafePaths(file, await this.packager.info.getWorkspaceRoot())
+
     // okay, it must be a symlink. evaluate link to be relative to source file in asar
     let link = await readlink(file)
     if (path.isAbsolute(link)) {
@@ -230,4 +229,86 @@ export class AsarPackager {
       transformedFiles,
     }
   }
+
+  private async getProtectedPaths(): Promise<string[]> {
+    const systemPaths = [
+      // Generic *nix
+      "/usr",
+      "/lib",
+      "/bin",
+      "/sbin",
+      "/System",
+      "/Library",
+      "/private/etc",
+      "/private/var/db",
+      "/private/var/root",
+      "/private/var/log",
+      "/private/tmp",
+
+      // macOS legacy symlinks
+      "/etc",
+      "/var",
+      "/tmp",
+
+      // Windows
+      process.env.SystemRoot,
+      process.env.WINDIR,
+      // process.env.ProgramFiles,
+      // process.env["ProgramFiles(x86)"],
+      // process.env.ProgramData,
+      // process.env.CommonProgramFiles,
+      // process.env["CommonProgramFiles(x86)"],
+    ]
+      .filter(Boolean)
+      .map(p => path.resolve(p as string))
+
+    // Normalize to real paths to prevent symlink bypasses
+    const resolvedPaths: string[] = []
+    for (const p of systemPaths) {
+      try {
+        resolvedPaths.push(await fs.realpath(p))
+      } catch {
+        resolvedPaths.push(path.resolve(p))
+      }
+    }
+
+    return resolvedPaths
+  }
+
+  private async protectSystemAndUnsafePaths(file: string, workspaceRoot?: string): Promise<boolean> {
+    const resolved = await fs.realpath(file).catch(() => path.resolve(file))
+
+    const scan = async () => {
+      if (workspaceRoot) {
+        const workspace = path.resolve(workspaceRoot)
+
+        if (!resolved.startsWith(workspace)) {
+          return true
+        }
+      }
+
+      // Allow temp & cache folders
+      const tmpdir = await fs.realpath(os.tmpdir())
+      if (resolved.startsWith(tmpdir)) {
+        return false
+      }
+
+      const blockedSystemPaths = await this.getProtectedPaths()
+      for (const sys of blockedSystemPaths) {
+        if (resolved.startsWith(sys)) {
+          return true
+        }
+      }
+
+      return false
+    }
+
+    const unsafe = await scan()
+
+    if (unsafe) {
+      log.error({ source: file, realPath: resolved }, `unable to copy, file is from outside the package to a system or unsafe path`)
+      throw new Error(`Cannot copy file [${file}] symlinked to file [${resolved}] outside the package to a system or unsafe path`)
+    }
+    return unsafe
+  }
 }

packages/app-builder-lib/src/node-module-collector/index.ts

@@ -1,56 +1,147 @@
+import { exists, log, spawn } from "builder-util"
+import { CancellationToken } from "builder-util-runtime"
+import * as fs from "fs-extra"
+import * as path from "path"
+import { TmpDir } from "temp-file"
 import { NpmNodeModulesCollector } from "./npmNodeModulesCollector"
+import { detectYarnBerry as detectIfYarnBerry, detectPackageManagerByEnv, detectPackageManagerByFile, getPackageManagerCommand, PM } from "./packageManager"
 import { PnpmNodeModulesCollector } from "./pnpmNodeModulesCollector"
-import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
-import { detectPackageManagerByLockfile, detectPackageManagerByEnv, PM, getPackageManagerCommand, detectYarnBerry } from "./packageManager"
 import { NodeModuleInfo } from "./types"
-import { TmpDir } from "temp-file"
+import { YarnBerryNodeModulesCollector } from "./yarnBerryNodeModulesCollector"
+import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
+
+export { getPackageManagerCommand, PM }
 
-export async function getCollectorByPackageManager(pm: PM, rootDir: string, tempDirManager: TmpDir) {
+export function getCollectorByPackageManager(pm: PM, rootDir: string, tempDirManager: TmpDir) {
   switch (pm) {
     case PM.PNPM:
-      if (await PnpmNodeModulesCollector.isPnpmProjectHoisted(rootDir)) {
-        return new NpmNodeModulesCollector(rootDir, tempDirManager)
-      }
       return new PnpmNodeModulesCollector(rootDir, tempDirManager)
-    case PM.NPM:
-    case PM.BUN:
-      return new NpmNodeModulesCollector(rootDir, tempDirManager)
     case PM.YARN:
       return new YarnNodeModulesCollector(rootDir, tempDirManager)
+    case PM.YARN_BERRY:
+      return new YarnBerryNodeModulesCollector(rootDir, tempDirManager)
+    case PM.BUN:
+    case PM.NPM:
     default:
       return new NpmNodeModulesCollector(rootDir, tempDirManager)
   }
 }
 
-export async function getNodeModules(pm: PM, rootDir: string, tempDirManager: TmpDir): Promise<NodeModuleInfo[]> {
-  const collector = await getCollectorByPackageManager(pm, rootDir, tempDirManager)
-  return collector.getNodeModules()
+export function getNodeModules(
+  pm: PM,
+  {
+    rootDir,
+    tempDirManager,
+    cancellationToken,
+    packageName,
+  }: {
+    rootDir: string
+    tempDirManager: TmpDir
+    cancellationToken: CancellationToken
+    packageName: string
+  }
+): Promise<NodeModuleInfo[]> {
+  const collector = getCollectorByPackageManager(pm, rootDir, tempDirManager)
+  return collector.getNodeModules({ cancellationToken, packageName })
 }
 
-export function detectPackageManager(dirs: string[]): PM {
+export async function detectPackageManager(searchPaths: string[]): Promise<{ pm: PM; corepackConfig: string | undefined; resolvedDirectory: string | undefined }> {
   let pm: PM | null = null
+  const dedupedPaths = Array.from(new Set(searchPaths)) // reduce file operations, dedupe paths since primary use case has projectDir === appDir
 
-  const resolveYarnVersion = (pm: PM) => {
-    if (pm === PM.YARN) {
-      return detectYarnBerry()
+  const resolveIfYarn = (pm: PM, version: string, cwd: string) => (pm === PM.YARN ? detectIfYarnBerry(cwd, version) : pm)
+
+  for (const dir of dedupedPaths) {
+    const packageJsonPath = path.join(dir, "package.json")
+    const packageManager = (await exists(packageJsonPath)) ? (await fs.readJson(packageJsonPath, "utf8"))?.packageManager : undefined
+    if (packageManager) {
+      const [pm, version] = packageManager.split("@")
+      if (Object.values(PM).includes(pm as PM)) {
+        const resolvedPackageManager = await resolveIfYarn(pm as PM, version, dir)
+        log.debug({ resolvedPackageManager, packageManager, cwd: dir }, "packageManager field detected in package.json")
+        return { pm: resolvedPackageManager, corepackConfig: packageManager, resolvedDirectory: dir }
+      }
     }
-    return pm
-  }
 
-  for (const dir of dirs) {
-    pm = detectPackageManagerByLockfile(dir)
+    pm = await detectPackageManagerByFile(dir)
     if (pm) {
-      return resolveYarnVersion(pm)
+      const resolvedPackageManager = await resolveIfYarn(pm, "", dir)
+      log.debug({ resolvedPackageManager, cwd: dir }, "packageManager detected by file")
+      return { pm: resolvedPackageManager, resolvedDirectory: dir, corepackConfig: undefined }
     }
   }
 
-  pm = detectPackageManagerByEnv()
-  if (pm) {
-    return resolveYarnVersion(pm)
+  pm = detectPackageManagerByEnv() || PM.NPM
+  const cwd = process.env.npm_package_json ? path.dirname(process.env.npm_package_json) : (process.env.INIT_CWD ?? process.cwd())
+  const resolvedPackageManager = await resolveIfYarn(pm, "", cwd)
+  log.debug({ resolvedPackageManager, detected: cwd }, "packageManager not detected by file, falling back to environment detection")
+  return { pm: resolvedPackageManager, resolvedDirectory: undefined, corepackConfig: undefined }
+}
+
+export async function findWorkspaceRoot(pm: PM, cwd: string): Promise<string | undefined> {
+  let command: { command: string; args: string[] } | undefined
+
+  switch (pm) {
+    case PM.PNPM:
+      command = { command: "pnpm", args: ["root", "-w"] }
+      break
+
+    case PM.YARN_BERRY:
+      command = { command: "yarn", args: ["config", "get", "workspaceRoot"] }
+      break
+
+    case PM.YARN: {
+      command = { command: "yarn", args: ["workspaces", "info", "--silent"] }
+      break
+    }
+
+    case PM.BUN:
+      command = { command: "bun", args: ["pm", "ls", "--json"] }
+      break
+
+    case PM.NPM:
+    default:
+      command = { command: "npm", args: ["prefix", "-w"] }
+      break
   }
 
-  // Default to npm
-  return PM.NPM
+  const output = await spawn(command.command, command.args, { cwd, stdio: ["ignore", "pipe", "ignore"] })
+    .then(it => {
+      const out = it?.trim()
+      if (pm === PM.YARN) {
+        JSON.parse(out) // if JSON valid, workspace detected
+        return findNearestWithWorkspacesField(cwd)
+      } else if (pm === PM.BUN) {
+        const json = JSON.parse(out)
+        if (Array.isArray(json) && json.length > 0) {
+          return findNearestWithWorkspacesField(cwd)
+        }
+      }
+      return !out?.length || out === "undefined" ? undefined : out
+    })
+    .catch(() => findNearestWithWorkspacesField(cwd))
+
+  log.debug({ root: output || cwd }, output ? "workspace root detected" : "workspace root not detected, using project root")
+  return output
 }
 
-export { PM, getPackageManagerCommand }
+async function findNearestWithWorkspacesField(dir: string): Promise<string | undefined> {
+  let current = dir
+  while (true) {
+    const pkgPath = path.join(current, "package.json")
+    try {
+      const pkg = JSON.parse(await fs.readFile(pkgPath, "utf8"))
+      if (pkg.workspaces) {
+        return current
+      }
+    } catch {
+      // ignore
+    }
+    const parent = path.dirname(current)
+    if (parent === current) {
+      break
+    }
+    current = parent
+  }
+  return undefined
+}