From db2bf1a0d1be6b81ca2d1a6b693b78753d3ba6ea Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 26 Jan 2024 17:29:04 +0900 Subject: [PATCH] fix: apply module search paths restriction on worker and child process (#41118) --- filenames.auto.gni | 4 -- lib/browser/init.ts | 3 -- lib/common/init.ts | 40 ++++++++++++++++ lib/common/reset-search-paths.ts | 69 ---------------------------- lib/node/init.ts | 18 ++++++++ lib/renderer/init.ts | 13 ++++-- lib/utility/init.ts | 3 -- lib/worker/init.ts | 3 -- spec/fixtures/module/module-paths.js | 2 + spec/node-spec.ts | 6 +++ 10 files changed, 76 insertions(+), 85 deletions(-) delete mode 100644 lib/common/reset-search-paths.ts create mode 100644 spec/fixtures/module/module-paths.js diff --git a/filenames.auto.gni b/filenames.auto.gni index bbf71f31cc..43bff50aca 100644 --- a/filenames.auto.gni +++ b/filenames.auto.gni @@ -260,7 +260,6 @@ auto_filenames = { "lib/common/deprecate.ts", "lib/common/init.ts", "lib/common/ipc-messages.ts", - "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.ts", "lib/common/webpack-globals-provider.ts", "package.json", @@ -277,7 +276,6 @@ auto_filenames = { "lib/common/define-properties.ts", "lib/common/init.ts", "lib/common/ipc-messages.ts", - "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.ts", "lib/common/webpack-provider.ts", "lib/renderer/api/clipboard.ts", @@ -316,7 +314,6 @@ auto_filenames = { "lib/common/define-properties.ts", "lib/common/init.ts", "lib/common/ipc-messages.ts", - "lib/common/reset-search-paths.ts", "lib/common/webpack-provider.ts", "lib/renderer/api/clipboard.ts", "lib/renderer/api/context-bridge.ts", @@ -352,7 +349,6 @@ auto_filenames = { "lib/common/api/net-client-request.ts", "lib/common/define-properties.ts", "lib/common/init.ts", - "lib/common/reset-search-paths.ts", "lib/common/webpack-globals-provider.ts", "lib/utility/api/exports/electron.ts", "lib/utility/api/module-list.ts", diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 79ea1406d0..47a694f5d5 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -10,9 +10,6 @@ const Module = require('module') as NodeJS.ModuleInternal; // we need to restore it here. process.argv.splice(1, 1); -// Clear search paths. -require('../common/reset-search-paths'); - // Import common settings. require('@electron/internal/common/init'); diff --git a/lib/common/init.ts b/lib/common/init.ts index 630a5f910e..097f8fa641 100644 --- a/lib/common/init.ts +++ b/lib/common/init.ts @@ -73,3 +73,43 @@ if (process.platform === 'win32') { } }); } + +const Module = require('module') as NodeJS.ModuleInternal; + +// Make a fake Electron module that we will insert into the module cache +const makeElectronModule = (name: string) => { + const electronModule = new Module('electron', null); + electronModule.id = 'electron'; + electronModule.loaded = true; + electronModule.filename = name; + Object.defineProperty(electronModule, 'exports', { + get: () => require('electron') + }); + Module._cache[name] = electronModule; +}; + +makeElectronModule('electron'); +makeElectronModule('electron/common'); +if (process.type === 'browser') { + makeElectronModule('electron/main'); +} +if (process.type === 'renderer') { + makeElectronModule('electron/renderer'); +} + +const originalResolveFilename = Module._resolveFilename; + +// 'electron/main', 'electron/renderer' and 'electron/common' are module aliases +// of the 'electron' module for TypeScript purposes, i.e., the types for +// 'electron/main' consist of only main process modules, etc. It is intentional +// that these can be `require()`-ed from both the main process as well as the +// renderer process regardless of the names, they're superficial for TypeScript +// only. +const electronModuleNames = new Set(['electron', 'electron/main', 'electron/renderer', 'electron/common']); +Module._resolveFilename = function (request, parent, isMain, options) { + if (electronModuleNames.has(request)) { + return 'electron'; + } else { + return originalResolveFilename(request, parent, isMain, options); + } +}; diff --git a/lib/common/reset-search-paths.ts b/lib/common/reset-search-paths.ts deleted file mode 100644 index 4268613532..0000000000 --- a/lib/common/reset-search-paths.ts +++ /dev/null @@ -1,69 +0,0 @@ -import * as path from 'path'; - -const Module = require('module') as NodeJS.ModuleInternal; - -// We do not want to allow use of the VM module in the renderer process as -// it conflicts with Blink's V8::Context internal logic. -if (process.type === 'renderer') { - const _load = Module._load; - Module._load = function (request: string) { - if (request === 'vm') { - console.warn('The vm module of Node.js is deprecated in the renderer process and will be removed.'); - } - return _load.apply(this, arguments as any); - }; -} - -// Prevent Node from adding paths outside this app to search paths. -const resourcesPathWithTrailingSlash = process.resourcesPath + path.sep; -const originalNodeModulePaths = Module._nodeModulePaths; -Module._nodeModulePaths = function (from: string) { - const paths: string[] = originalNodeModulePaths(from); - const fromPath = path.resolve(from) + path.sep; - // If "from" is outside the app then we do nothing. - if (fromPath.startsWith(resourcesPathWithTrailingSlash)) { - return paths.filter(function (candidate) { - return candidate.startsWith(resourcesPathWithTrailingSlash); - }); - } else { - return paths; - } -}; - -// Make a fake Electron module that we will insert into the module cache -const makeElectronModule = (name: string) => { - const electronModule = new Module('electron', null); - electronModule.id = 'electron'; - electronModule.loaded = true; - electronModule.filename = name; - Object.defineProperty(electronModule, 'exports', { - get: () => require('electron') - }); - Module._cache[name] = electronModule; -}; - -makeElectronModule('electron'); -makeElectronModule('electron/common'); -if (process.type === 'browser') { - makeElectronModule('electron/main'); -} -if (process.type === 'renderer') { - makeElectronModule('electron/renderer'); -} - -const originalResolveFilename = Module._resolveFilename; - -// 'electron/main', 'electron/renderer' and 'electron/common' are module aliases -// of the 'electron' module for TypeScript purposes, i.e., the types for -// 'electron/main' consist of only main process modules, etc. It is intentional -// that these can be `require()`-ed from both the main process as well as the -// renderer process regardless of the names, they're superficial for TypeScript -// only. -const electronModuleNames = new Set(['electron', 'electron/main', 'electron/renderer', 'electron/common']); -Module._resolveFilename = function (request, parent, isMain, options) { - if (electronModuleNames.has(request)) { - return 'electron'; - } else { - return originalResolveFilename(request, parent, isMain, options); - } -}; diff --git a/lib/node/init.ts b/lib/node/init.ts index 19bd28eccf..207d16108c 100644 --- a/lib/node/init.ts +++ b/lib/node/init.ts @@ -29,3 +29,21 @@ cp.fork = (modulePath: string, args: any, options: any) => { } return originalFork(modulePath, args, options); }; + +// Prevent Node from adding paths outside this app to search paths. +const path = require('path'); +const Module = require('module') as NodeJS.ModuleInternal; +const resourcesPathWithTrailingSlash = process.resourcesPath + path.sep; +const originalNodeModulePaths = Module._nodeModulePaths; +Module._nodeModulePaths = function (from: string) { + const paths: string[] = originalNodeModulePaths(from); + const fromPath = path.resolve(from) + path.sep; + // If "from" is outside the app then we do nothing. + if (fromPath.startsWith(resourcesPathWithTrailingSlash)) { + return paths.filter(function (candidate) { + return candidate.startsWith(resourcesPathWithTrailingSlash); + }); + } else { + return paths; + } +}; diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 3b151cc24b..b735ba29be 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -7,6 +7,16 @@ import type * as ipcRendererUtilsModule from '@electron/internal/renderer/ipc-re const Module = require('module') as NodeJS.ModuleInternal; +// We do not want to allow use of the VM module in the renderer process as +// it conflicts with Blink's V8::Context internal logic. +const originalModuleLoad = Module._load; +Module._load = function (request: string) { + if (request === 'vm') { + console.warn('The vm module of Node.js is deprecated in the renderer process and will be removed.'); + } + return originalModuleLoad.apply(this, arguments as any); +}; + // Make sure globals like "process" and "global" are always available in preload // scripts even after they are deleted in "loaded" script. // @@ -33,9 +43,6 @@ Module.wrapper = [ // init.js, we need to restore it here. process.argv.splice(1, 1); -// Clear search paths. -require('../common/reset-search-paths'); - // Import common settings. require('@electron/internal/common/init'); diff --git a/lib/utility/init.ts b/lib/utility/init.ts index 14cf0cf5ce..ef8ac74ef5 100644 --- a/lib/utility/init.ts +++ b/lib/utility/init.ts @@ -10,9 +10,6 @@ const entryScript: string = v8Util.getHiddenValue(process, '_serviceStartupScrip // we need to restore it here. process.argv.splice(1, 1, entryScript); -// Clear search paths. -require('../common/reset-search-paths'); - // Import common settings. require('@electron/internal/common/init'); diff --git a/lib/worker/init.ts b/lib/worker/init.ts index b293539355..0c92d61e88 100644 --- a/lib/worker/init.ts +++ b/lib/worker/init.ts @@ -6,9 +6,6 @@ const Module = require('module') as NodeJS.ModuleInternal; // init.js, we need to restore it here. process.argv.splice(1, 1); -// Clear search paths. -require('../common/reset-search-paths'); - // Import common settings. require('@electron/internal/common/init'); diff --git a/spec/fixtures/module/module-paths.js b/spec/fixtures/module/module-paths.js new file mode 100644 index 0000000000..895168dd39 --- /dev/null +++ b/spec/fixtures/module/module-paths.js @@ -0,0 +1,2 @@ +const Module = require('node:module'); +process.send(Module._nodeModulePaths(process.resourcesPath + '/test.js')); diff --git a/spec/node-spec.ts b/spec/node-spec.ts index e1d9c76d9e..df70e2ab2d 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -23,6 +23,12 @@ describe('node feature', () => { const [msg] = await message; expect(msg).to.equal('message'); }); + + it('Has its module searth paths restricted', async () => { + const child = childProcess.fork(path.join(fixtures, 'module', 'module-paths.js')); + const [msg] = await once(child, 'message'); + expect(msg.length).to.equal(2); + }); }); });