diff --git a/package.json b/package.json index 0e6f46b07..b342bcca3 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,7 @@ "lodash": "4.18.1", "marked": "18.0.5", "mpegts.js": "1.8.0", + "ms": "2.1.3", "ngx-indexed-db": "21.0.0", "ngx-skeleton-loader": "11.3.0", "rxjs": "7.8.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 12668170e..5fcf722a4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -143,6 +143,9 @@ importers: mpegts.js: specifier: 1.8.0 version: 1.8.0 + ms: + specifier: 2.1.3 + version: 2.1.3 ngx-indexed-db: specifier: 21.0.0 version: 21.0.0(@angular/common@21.2.9(@angular/core@21.2.9(@angular/compiler@21.2.9)(rxjs@7.8.2)(zone.js@0.15.1))(rxjs@7.8.2))(@angular/core@21.2.9(@angular/compiler@21.2.9)(rxjs@7.8.2)(zone.js@0.15.1)) diff --git a/tools/packaging/asar-dependency-closure.mjs b/tools/packaging/asar-dependency-closure.mjs new file mode 100644 index 000000000..7f7768a0f --- /dev/null +++ b/tools/packaging/asar-dependency-closure.mjs @@ -0,0 +1,225 @@ +/** + * Verifies that a packaged Electron `app.asar` ships a self-consistent set of + * runtime node_modules: every package inside the archive has its own + * non-optional dependencies shipped alongside it. + * + * electron-builder's pnpm dependency collector reads `pnpm list --json`, which + * deduplicates repeated packages and reports empty `dependencies` for all but + * one occurrence. That makes it silently drop deeply-nested transitive + * dependencies (for example `ms` under `debug`) from the packaged archive, which + * crashes the app on launch ("Cannot find module 'ms'", issue #1103). + * + * These helpers audit every package that is actually shipped inside the asar and + * report any non-optional dependency of a shipped package that is missing, + * resolving each one the way Node would inside the packaged tree. IO is injected + * so the core logic stays pure and unit-testable without a real archive. + */ + +import path from 'node:path'; + +const PACKAGE_MANIFEST = 'package.json'; +const NODE_MODULES_SEGMENT = '/node_modules/'; + +/** + * A genuine installed package lives directly under a node_modules directory as + * `` or `@scope/`. Nested folders that merely carry their own + * package.json (for example `fast-uri/benchmark` or `/test`) are not + * packages and must not be treated as dependency sources. + */ +function isPackageRootDir(dir) { + const boundary = dir.lastIndexOf(NODE_MODULES_SEGMENT); + if (boundary === -1) { + return false; + } + + const relative = dir.slice(boundary + NODE_MODULES_SEGMENT.length); + const segments = relative.split('/'); + + return segments[0].startsWith('@') + ? segments.length === 2 + : segments.length === 1; +} + +/** + * Collects every genuine installed package directory in the archive (those that + * sit directly under a node_modules as `` or `@scope/`). Entry paths + * are leading-slash posix. + */ +export function collectAsarPackageDirs(asarEntries) { + const packageDirs = new Set(); + + for (const entry of asarEntries) { + if (!entry.endsWith(`/${PACKAGE_MANIFEST}`)) { + continue; + } + + const dir = entry.slice(0, -(PACKAGE_MANIFEST.length + 1)); + if (isPackageRootDir(dir)) { + packageDirs.add(dir); + } + } + + return packageDirs; +} + +/** + * Mirrors Node's upward node_modules resolution across the packaged tree, which + * is what electron-builder's hoisting produces. Walks every ancestor directory + * (not just node_modules boundaries) so a dependency hoisted to the archive + * root still resolves for a package that lives under an app subdirectory such + * as `/electron-backend/node_modules/foo`. Returns the resolved package + * directory or null when the dependency is absent. + */ +export function resolvePackagedDependency(fromDir, dependencyName, packageDirs) { + let current = fromDir; + + while (true) { + // Node never looks inside `.../node_modules/node_modules`. + if (!current.endsWith(NODE_MODULES_SEGMENT.slice(0, -1))) { + const candidate = `${current}/node_modules/${dependencyName}`; + if (packageDirs.has(candidate)) { + return candidate; + } + } + + if (current === '') { + return null; + } + + current = current.slice(0, current.lastIndexOf('/')); + } +} + +/** + * Checks that every package physically shipped inside the asar has its own + * non-optional dependencies shipped too (node-resolvable from its location). + * Returns the list of missing dependencies. + * + * The app root manifest ('') is intentionally skipped: it declares frontend-only + * packages (Angular, zone.js, ...) that are compiled into the web bundle rather + * than shipped as runtime node_modules, so requiring them here would be a false + * positive. Auditing what is actually bundled still catches the real regression + * (a shipped `debug` whose transitive `ms` was dropped — issue #1103). + * + * @param {Set} packageDirs directories that contain a package.json + * @param {(packageDir: string) => object | null} readManifest manifest reader + * @returns {{ dependency: string, requiredBy: string }[]} + */ +export function findMissingPackagedDependencies(packageDirs, readManifest) { + const missing = []; + + for (const packageDir of packageDirs) { + if (packageDir === '') { + continue; + } + + const manifest = readManifest(packageDir); + if (!manifest) { + continue; + } + + const optionalDependencies = new Set( + Object.keys(manifest.optionalDependencies ?? {}) + ); + // Packages sometimes list a host-provided peer (e.g. `electron`) in + // `dependencies` as well so package managers install it during + // development; at runtime the host supplies it, so its absence from + // the archive is not a packaging defect. + const peerDependencies = new Set( + Object.keys(manifest.peerDependencies ?? {}) + ); + + for (const dependencyName of Object.keys(manifest.dependencies ?? {})) { + if ( + optionalDependencies.has(dependencyName) || + peerDependencies.has(dependencyName) + ) { + continue; + } + + if ( + !resolvePackagedDependency( + packageDir, + dependencyName, + packageDirs + ) + ) { + missing.push({ + dependency: dependencyName, + requiredBy: packageDir, + }); + } + } + } + + return missing; +} + +/** + * Inspects a real `app.asar` for missing runtime dependencies. `listPackage` and + * `extractFile` are injected (normally from `@electron/asar`). + * + * `@electron/asar` builds listing entries and resolves lookup paths with the + * HOST separator: on Windows `listPackage` returns entries like + * `\node_modules\debug\package.json` and `extractFile` splits its path on + * `path.sep`. The pure helpers above are posix-only, so listings are normalized + * to posix and lookup paths converted back to the host separator (`pathSep` is + * injectable for tests). Without this the guard silently audited nothing on + * Windows. + * + * Returns `{ missing, packageCount, manifestReadFailures }` so callers can + * reject a vacuous pass: a packaged app always ships node_modules, so + * `packageCount === 0` or read failures indicate the guard itself is broken, + * not a healthy archive. + */ +export function inspectPackagedDependencyClosure( + asarPath, + { listPackage, extractFile, pathSep = path.sep } +) { + const toPosix = (value) => + pathSep === '\\' ? value.replaceAll('\\', '/') : value; + const toHostPath = (value) => + pathSep === '\\' ? value.replaceAll('/', '\\') : value; + + const packageDirs = collectAsarPackageDirs( + listPackage(asarPath).map(toPosix) + ); + const manifestCache = new Map(); + const manifestReadFailures = []; + + const readManifest = (packageDir) => { + if (manifestCache.has(packageDir)) { + return manifestCache.get(packageDir); + } + + const relativePath = toHostPath( + (packageDir === '' ? '' : `${packageDir.slice(1)}/`) + + PACKAGE_MANIFEST + ); + + let manifest = null; + + try { + manifest = JSON.parse( + extractFile(asarPath, relativePath).toString('utf8') + ); + } catch (error) { + manifestReadFailures.push({ + packageDir, + message: error.message, + }); + manifest = null; + } + + manifestCache.set(packageDir, manifest); + return manifest; + }; + + const missing = findMissingPackagedDependencies(packageDirs, readManifest); + + return { + missing, + packageCount: packageDirs.size, + manifestReadFailures, + }; +} diff --git a/tools/packaging/asar-dependency-closure.test.mjs b/tools/packaging/asar-dependency-closure.test.mjs new file mode 100644 index 000000000..0c5fc81fe --- /dev/null +++ b/tools/packaging/asar-dependency-closure.test.mjs @@ -0,0 +1,286 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + collectAsarPackageDirs, + findMissingPackagedDependencies, + inspectPackagedDependencyClosure, + resolvePackagedDependency, +} from './asar-dependency-closure.mjs'; + +/** + * Builds a `readManifest(dir)` backed by an in-memory map of + * `{ packageDir: manifest }`, mirroring how manifests are read from an asar. + */ +function manifestReader(manifests) { + return (packageDir) => manifests[packageDir] ?? null; +} + +test('collectAsarPackageDirs keeps only genuine package roots', () => { + const dirs = collectAsarPackageDirs([ + '/package.json', + '/node_modules/debug/package.json', + '/node_modules/debug/src/index.js', + '/node_modules/@angular/core/package.json', + '/node_modules/fast-uri/node_modules/uri-js/package.json', + // Nested manifests that are NOT packages must be ignored. + '/node_modules/fast-uri/benchmark/package.json', + '/node_modules/@angular/core/schematics/package.json', + ]); + + assert.ok(dirs.has('/node_modules/debug')); + assert.ok(dirs.has('/node_modules/@angular/core')); + assert.ok(dirs.has('/node_modules/fast-uri/node_modules/uri-js')); + assert.equal(dirs.has(''), false); + assert.equal(dirs.has('/node_modules/fast-uri/benchmark'), false); + assert.equal(dirs.has('/node_modules/@angular/core/schematics'), false); +}); + +test('resolvePackagedDependency walks node_modules boundaries upward', () => { + const dirs = new Set([ + '/node_modules/ms', + '/node_modules/electron-updater/node_modules/lazy-val', + ]); + + // Hoisted to top level, requested from a deeply nested package. + assert.equal( + resolvePackagedDependency( + '/node_modules/electron-updater/node_modules/builder-util-runtime', + 'ms', + dirs + ), + '/node_modules/ms' + ); + + // Nested copy preferred over walking further up. + assert.equal( + resolvePackagedDependency( + '/node_modules/electron-updater', + 'lazy-val', + dirs + ), + '/node_modules/electron-updater/node_modules/lazy-val' + ); + + assert.equal(resolvePackagedDependency('', 'missing', dirs), null); +}); + +test('resolvePackagedDependency reaches root node_modules from app subdirectories', () => { + // A package under an app subdirectory (e.g. /electron-backend/node_modules) + // must still resolve a dependency hoisted to the archive root, exactly as + // Node's resolver walks every ancestor directory. + const dirs = new Set([ + '/electron-backend/node_modules/foo', + '/node_modules/bar', + ]); + + assert.equal( + resolvePackagedDependency( + '/electron-backend/node_modules/foo', + 'bar', + dirs + ), + '/node_modules/bar' + ); +}); + +test('reports a deduplicated transitive dependency dropped from the archive (issue #1103)', () => { + // `debug` is packaged but its transitive `ms` was dropped by the collector. + const packageDirs = new Set([ + '', + '/node_modules/electron-updater', + '/node_modules/debug', + ]); + const manifests = { + '': { dependencies: { 'electron-updater': '6.8.9' } }, + '/node_modules/electron-updater': { dependencies: { debug: '4.4.3' } }, + '/node_modules/debug': { dependencies: { ms: '2.1.3' } }, + }; + + const missing = findMissingPackagedDependencies( + packageDirs, + manifestReader(manifests) + ); + + assert.deepEqual(missing, [ + { dependency: 'ms', requiredBy: '/node_modules/debug' }, + ]); +}); + +test('passes when the full runtime closure is present', () => { + const packageDirs = new Set([ + '', + '/node_modules/electron-updater', + '/node_modules/debug', + '/node_modules/ms', + ]); + const manifests = { + '': { dependencies: { 'electron-updater': '6.8.9' } }, + '/node_modules/electron-updater': { dependencies: { debug: '4.4.3' } }, + '/node_modules/debug': { dependencies: { ms: '2.1.3' } }, + '/node_modules/ms': {}, + }; + + assert.deepEqual( + findMissingPackagedDependencies(packageDirs, manifestReader(manifests)), + [] + ); +}); + +test('does not flag frontend-only deps the app root declares but never ships', () => { + // The app package.json lists Angular etc. (compiled into the web bundle), + // which are intentionally absent from the shipped runtime node_modules. + const packageDirs = new Set(['', '/node_modules/electron-updater']); + const manifests = { + '': { + dependencies: { + '@angular/core': '21.2.9', + 'electron-updater': '6.8.9', + }, + }, + '/node_modules/electron-updater': {}, + }; + + assert.deepEqual( + findMissingPackagedDependencies(packageDirs, manifestReader(manifests)), + [] + ); +}); + +test('ignores missing optional dependencies', () => { + const packageDirs = new Set(['', '/node_modules/pkg']); + const manifests = { + '': { dependencies: { pkg: '1.0.0' } }, + '/node_modules/pkg': { + dependencies: { fsevents: '2.3.0' }, + optionalDependencies: { fsevents: '2.3.0' }, + }, + }; + + assert.deepEqual( + findMissingPackagedDependencies(packageDirs, manifestReader(manifests)), + [] + ); +}); + +test('ignores host-provided deps declared in both dependencies and peerDependencies', () => { + // e.g. a package listing `electron` in dependencies for dev installs while + // the Electron runtime provides it — never shipped inside app.asar. + const packageDirs = new Set(['', '/node_modules/pkg']); + const manifests = { + '': { dependencies: { pkg: '1.0.0' } }, + '/node_modules/pkg': { + dependencies: { electron: '41.0.0' }, + peerDependencies: { electron: '>=30' }, + }, + }; + + assert.deepEqual( + findMissingPackagedDependencies(packageDirs, manifestReader(manifests)), + [] + ); +}); + +test('tolerates dependency cycles without infinite recursion', () => { + const packageDirs = new Set(['', '/node_modules/a', '/node_modules/b']); + const manifests = { + '': { dependencies: { a: '1.0.0' } }, + '/node_modules/a': { dependencies: { b: '1.0.0' } }, + '/node_modules/b': { dependencies: { a: '1.0.0' } }, + }; + + assert.deepEqual( + findMissingPackagedDependencies(packageDirs, manifestReader(manifests)), + [] + ); +}); + +test('inspectPackagedDependencyClosure wires injected asar IO', () => { + const files = { + 'package.json': { dependencies: { debug: '4.4.3' } }, + 'node_modules/debug/package.json': { dependencies: { ms: '2.1.3' } }, + }; + const listPackage = () => [ + '/package.json', + '/node_modules/debug/package.json', + ]; + const extractFile = (_asarPath, relativePath) => { + const manifest = files[relativePath]; + if (!manifest) { + throw new Error(`not found: ${relativePath}`); + } + return Buffer.from(JSON.stringify(manifest)); + }; + + const { missing, packageCount, manifestReadFailures } = + inspectPackagedDependencyClosure('app.asar', { + listPackage, + extractFile, + pathSep: '/', + }); + + assert.deepEqual(missing, [ + { dependency: 'ms', requiredBy: '/node_modules/debug' }, + ]); + assert.equal(packageCount, 1); + assert.deepEqual(manifestReadFailures, []); +}); + +test('inspectPackagedDependencyClosure handles Windows-separator asar IO', () => { + // @electron/asar builds listing entries with the host separator and + // resolves extractFile paths by splitting on path.sep — on Windows both + // are backslash-based. The guard must normalize listings to posix and + // hand extractFile backslash paths, otherwise it audits nothing. + const files = { + 'package.json': { dependencies: { debug: '4.4.3' } }, + 'node_modules\\debug\\package.json': { + dependencies: { ms: '2.1.3' }, + }, + }; + const listPackage = () => [ + '\\package.json', + '\\node_modules\\debug\\package.json', + '\\node_modules\\debug\\src\\index.js', + ]; + const extractFile = (_asarPath, relativePath) => { + // Mimic asar on win32: forward-slash lookups do not resolve. + const manifest = files[relativePath]; + if (!manifest) { + throw new Error(`${relativePath} was not found in this archive`); + } + return Buffer.from(JSON.stringify(manifest)); + }; + + const { missing, packageCount, manifestReadFailures } = + inspectPackagedDependencyClosure('app.asar', { + listPackage, + extractFile, + pathSep: '\\', + }); + + assert.equal(packageCount, 1); + assert.deepEqual(manifestReadFailures, []); + assert.deepEqual(missing, [ + { dependency: 'ms', requiredBy: '/node_modules/debug' }, + ]); +}); + +test('inspectPackagedDependencyClosure surfaces manifest read failures instead of swallowing them', () => { + const listPackage = () => ['/node_modules/broken/package.json']; + const extractFile = () => { + throw new Error('corrupt entry'); + }; + + const { missing, packageCount, manifestReadFailures } = + inspectPackagedDependencyClosure('app.asar', { + listPackage, + extractFile, + pathSep: '/', + }); + + assert.deepEqual(missing, []); + assert.equal(packageCount, 1); + assert.deepEqual(manifestReadFailures, [ + { packageDir: '/node_modules/broken', message: 'corrupt entry' }, + ]); +}); diff --git a/tools/packaging/project.json b/tools/packaging/project.json index d9e35412c..8e5882191 100644 --- a/tools/packaging/project.json +++ b/tools/packaging/project.json @@ -17,11 +17,13 @@ "{workspaceRoot}/apps/electron-backend/native/src/embedded_mpv_win32.cc", "{workspaceRoot}/apps/electron-backend/src/app/options/maker.options.json", "{workspaceRoot}/tools/packaging/generate-electron-builder-metadata.mjs", + "{workspaceRoot}/tools/packaging/asar-dependency-closure.mjs", + "{workspaceRoot}/tools/packaging/asar-dependency-closure.test.mjs", "{workspaceRoot}/tools/embedded-mpv/stage-runtime.mjs", "{workspaceRoot}/tools/embedded-mpv/stage-windows-runtime-archive.mjs" ], "options": { - "command": "node --test tools/packaging/electron-package-identity.test.mjs", + "command": "node --test tools/packaging/electron-package-identity.test.mjs tools/packaging/asar-dependency-closure.test.mjs", "cwd": "{workspaceRoot}" } }, diff --git a/tools/packaging/verify-electron-package-layout.mjs b/tools/packaging/verify-electron-package-layout.mjs index 4af255e1a..de6b01ddd 100644 --- a/tools/packaging/verify-electron-package-layout.mjs +++ b/tools/packaging/verify-electron-package-layout.mjs @@ -3,9 +3,10 @@ import { createRequire } from 'module'; import path from 'path'; import { buildElectronBuilderMetadata } from './generate-electron-builder-metadata.mjs'; +import { inspectPackagedDependencyClosure } from './asar-dependency-closure.mjs'; const require = createRequire(import.meta.url); -const { extractFile } = require('@electron/asar'); +const { extractFile, listPackage } = require('@electron/asar'); const { validatePackagedEmbeddedMpv } = require('./embedded-mpv-packaging.cjs'); const args = process.argv.slice(2); const normalizedArgs = args[0] === '--' ? args.slice(1) : args; @@ -547,6 +548,68 @@ function verifySnapPackagingConfig(errors) { } } +function verifyPackagedDependencyClosure(resourceDir, errors) { + const asarPath = path.join(resourceDir, 'app.asar'); + + if (!fileExists(asarPath)) { + // Missing archive is already reported by verifyPackagedPackageMetadata. + return; + } + + let inspection; + + try { + inspection = inspectPackagedDependencyClosure(asarPath, { + listPackage, + extractFile, + }); + } catch (error) { + errors.push( + `Unable to inspect packaged app archive ${asarPath}: ${error.message}` + ); + return; + } + + const { missing, packageCount, manifestReadFailures } = inspection; + + // A packaged app always ships node_modules, so an empty audit means the + // guard itself failed (e.g. path-separator handling), not a healthy asar. + if (packageCount === 0) { + errors.push( + `Dependency-closure guard found no node_modules packages in ${asarPath}; the audit cannot have run against the real archive contents.` + ); + return; + } + + if (manifestReadFailures.length > 0) { + errors.push( + [ + `Dependency-closure guard could not read ${manifestReadFailures.length} package manifest(s) in ${asarPath}:`, + ...manifestReadFailures.map( + (failure) => `- ${failure.packageDir}: ${failure.message}` + ), + ].join('\n') + ); + } + + if (missing.length === 0) { + return; + } + + const details = missing + .map((entry) => `- ${entry.dependency} (required by ${entry.requiredBy})`) + .join('\n'); + + errors.push( + [ + `Packaged app.asar is missing runtime node_modules in ${asarPath}.`, + "electron-builder's pnpm collector likely dropped a deduplicated transitive dependency.", + 'Fix it by declaring the missing package as a direct dependency in package.json (see issue #1103).', + details, + ].join('\n') + ); +} + function verifyResourceDir(resourceDir) { const missingWorkers = workerFiles.filter( (workerFile) => @@ -563,6 +626,7 @@ function verifyResourceDir(resourceDir) { const errors = []; verifyPackagedPackageMetadata(resourceDir, errors); + verifyPackagedDependencyClosure(resourceDir, errors); if (missingWorkers.length > 0) { errors.push(