migrating packTester to leverage corepack for isolating text/fixture environments and using specific installation methods (+ snapshot verification) of package manager implementations

Author: mmaietta

.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
@@ -188,7 +188,7 @@ jobs:
       fail-fast: false
       matrix:
         testFiles:
-          - winCodeSignTest,differentialUpdateTest,squirrelWindowsTest
+          - winCodeSignTest,differentialUpdateTest,squirrelWindowsTest,HoistedNodeModuleTest
           - appxTest,msiTest,portableTest,assistedInstallerTest,protonTest
           - BuildTest,oneClickInstallerTest,winPackagerTest,nsisUpdaterTest,webInstallerTest
           - 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,149 @@
 import { NpmNodeModulesCollector } from "./npmNodeModulesCollector"
 import { PnpmNodeModulesCollector } from "./pnpmNodeModulesCollector"
 import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
-import { detectPackageManagerByLockfile, detectPackageManagerByEnv, PM, getPackageManagerCommand, detectYarnBerry } from "./packageManager"
+import { detectPackageManagerByFile, detectPackageManagerByEnv, PM, getPackageManagerCommand, detectYarnBerry as detectIfYarnBerry } from "./packageManager"
 import { NodeModuleInfo } from "./types"
 import { TmpDir } from "temp-file"
+import * as path from "path"
+import * as fs from "fs-extra"
+import { log, spawn } from "builder-util"
+import { YarnBerryNodeModulesCollector } from "./yarnBerryNodeModulesCollector"
+import { CancellationToken } from "builder-util-runtime"
 
-export async function getCollectorByPackageManager(pm: PM, rootDir: string, tempDirManager: TmpDir) {
+export { PM, getPackageManagerCommand }
+
+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,
+    packageVersion
+  }: {
+    rootDir: string
+    tempDirManager: TmpDir
+    cancellationToken: CancellationToken
+    packageName: string
+    packageVersion?: string
+  }
+): Promise<NodeModuleInfo[]> {
+  const collector = getCollectorByPackageManager(pm, rootDir, tempDirManager)
+  return collector.getNodeModules({ cancellationToken, packageName, packageVersion })
 }
 
-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 = fs.existsSync(packageJsonPath) ? JSON.parse(fs.readFileSync(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
+}