From 551d5bd660aa2bed7de6c3a4412f3722168f59c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 19 Aug 2022 18:04:11 +0200 Subject: [PATCH 1/3] [N8N-4362] Load all nodes and credentials code in isolation --- packages/cli/src/LoadNodesAndCredentials.ts | 65 ++++++++------------- packages/cli/src/PackageHelper.ts | 7 +++ packages/cli/src/WorkflowRunnerProcess.ts | 36 ++++-------- 3 files changed, 41 insertions(+), 67 deletions(-) create mode 100644 packages/cli/src/PackageHelper.ts diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts index 8f5c62da9bc35..e3142d6e9e8c5 100644 --- a/packages/cli/src/LoadNodesAndCredentials.ts +++ b/packages/cli/src/LoadNodesAndCredentials.ts @@ -43,9 +43,18 @@ import { persistInstalledPackageData, removePackageFromDatabase, } from './CommunityNodes/packageModel'; +import { loadClassInIsolation } from './PackageHelper'; const CUSTOM_NODES_CATEGORY = 'Custom Nodes'; +function toJSON() { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return { + ...this, + authenticate: typeof this.authenticate === 'function' ? {} : this.authenticate, + }; +} + class LoadNodesAndCredentialsClass { nodeTypes: INodeTypeData = {}; @@ -194,24 +203,16 @@ class LoadNodesAndCredentialsClass { */ async loadCredentialsFromFile(credentialName: string, filePath: string): Promise { // eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires - const tempModule = require(filePath); let tempCredential: ICredentialType; try { + tempCredential = loadClassInIsolation(filePath, credentialName); + // Add serializer method "toJSON" to the class so that authenticate method (if defined) // gets mapped to the authenticate attribute before it is sent to the client. // The authenticate property is used by the client to decide whether or not to // include the credential type in the predefined credentials (HTTP node) - // eslint-disable-next-line func-names - tempModule[credentialName].prototype.toJSON = function () { - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return { - ...this, - authenticate: typeof this.authenticate === 'function' ? {} : this.authenticate, - }; - }; - - tempCredential = new tempModule[credentialName]() as ICredentialType; + Object.assign(tempCredential, { toJSON }); if (tempCredential.icon && tempCredential.icon.startsWith('file:')) { // If a file icon gets used add the full path @@ -363,9 +364,7 @@ class LoadNodesAndCredentialsClass { let nodeVersion = 1; try { - // eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires - const tempModule = require(filePath); - tempNode = new tempModule[nodeName](); + tempNode = loadClassInIsolation(filePath, nodeName); this.addCodex({ node: tempNode, filePath, isCustom: packageName === 'CUSTOM' }); } catch (error) { // eslint-disable-next-line no-console, @typescript-eslint/restrict-template-expressions @@ -385,13 +384,6 @@ class LoadNodesAndCredentialsClass { )}`; } - if (tempNode.hasOwnProperty('executeSingle')) { - this.logger.warn( - `"executeSingle" will get deprecated soon. Please update the code of node "${packageName}.${nodeName}" to use "execute" instead!`, - { filePath }, - ); - } - if (tempNode.hasOwnProperty('nodeVersions')) { const versionedNodeType = (tempNode as INodeVersionedType).getNodeType(); this.addCodex({ node: versionedNodeType, filePath, isCustom: packageName === 'CUSTOM' }); @@ -512,13 +504,10 @@ class LoadNodesAndCredentialsClass { async loadDataFromDirectory(setPackageName: string, directory: string): Promise { const files = await glob(path.join(directory, '**/*.@(node|credentials).js')); - let fileName: string; - let type: string; - // eslint-disable-next-line @typescript-eslint/no-explicit-any const loadPromises: any[] = []; for (const filePath of files) { - [fileName, type] = path.parse(filePath).name.split('.'); + const [fileName, type] = path.parse(filePath).name.split('.'); if (type === 'node') { loadPromises.push(this.loadNodeFromFile(setPackageName, fileName, filePath)); @@ -551,19 +540,14 @@ class LoadNodesAndCredentialsClass { } const packageName = packageFile.name; - - let tempPath: string; - let filePath: string; - + const { nodes, credentials } = packageFile.n8n; const returnData: INodeTypeNameVersion[] = []; // Read all node types - let fileName: string; - let type: string; - if (packageFile.n8n.hasOwnProperty('nodes') && Array.isArray(packageFile.n8n.nodes)) { - for (filePath of packageFile.n8n.nodes) { - tempPath = path.join(packagePath, filePath); - [fileName, type] = path.parse(filePath).name.split('.'); + if (Array.isArray(nodes)) { + for (const filePath of nodes) { + const tempPath = path.join(packagePath, filePath); + const [fileName] = path.parse(filePath).name.split('.'); const loadData = await this.loadNodeFromFile(packageName, fileName, tempPath); if (loadData) { returnData.push(loadData); @@ -572,14 +556,11 @@ class LoadNodesAndCredentialsClass { } // Read all credential types - if ( - packageFile.n8n.hasOwnProperty('credentials') && - Array.isArray(packageFile.n8n.credentials) - ) { - for (filePath of packageFile.n8n.credentials) { - tempPath = path.join(packagePath, filePath); + if (Array.isArray(credentials)) { + for (const filePath of credentials) { + const tempPath = path.join(packagePath, filePath); // eslint-disable-next-line @typescript-eslint/no-unused-vars - [fileName, type] = path.parse(filePath).name.split('.'); + const [fileName] = path.parse(filePath).name.split('.'); // eslint-disable-next-line @typescript-eslint/no-floating-promises this.loadCredentialsFromFile(fileName, tempPath); } diff --git a/packages/cli/src/PackageHelper.ts b/packages/cli/src/PackageHelper.ts new file mode 100644 index 0000000000000..40b5c736cb9c4 --- /dev/null +++ b/packages/cli/src/PackageHelper.ts @@ -0,0 +1,7 @@ +import { Script } from 'vm'; + +export const loadClassInIsolation = (filePath: string, className: string) => { + const script = new Script(`new (require('${filePath}').${className})()`); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return script.runInNewContext({ require }); +}; diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 477caf98b2cfa..d4eecad079d65 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -49,6 +49,7 @@ import { getLogger } from './Logger'; import config from '../config'; import { InternalHooksManager } from './InternalHooksManager'; import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; +import { loadClassInIsolation } from './PackageHelper'; export class WorkflowRunnerProcess { data: IWorkflowExecutionDataProcessWithExecution | undefined; @@ -92,41 +93,30 @@ export class WorkflowRunnerProcess { workflowId: this.data.workflowData.id, }); - let className: string; - let tempNode: INodeType; - let tempCredential: ICredentialType; - let filePath: string; - this.startedAt = new Date(); // Load the required nodes const nodeTypesData: INodeTypeData = {}; // eslint-disable-next-line no-restricted-syntax for (const nodeTypeName of Object.keys(this.data.nodeTypeData)) { - className = this.data.nodeTypeData[nodeTypeName].className; - - filePath = this.data.nodeTypeData[nodeTypeName].sourcePath; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires - const tempModule = require(filePath); + let tempNode: INodeType; + const { className, sourcePath } = this.data.nodeTypeData[nodeTypeName]; try { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - const nodeObject = new tempModule[className](); + const nodeObject = loadClassInIsolation(sourcePath, className); if (nodeObject.getNodeType !== undefined) { // eslint-disable-next-line @typescript-eslint/no-unsafe-call tempNode = nodeObject.getNodeType(); } else { tempNode = nodeObject; } - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - tempNode = new tempModule[className]() as INodeType; } catch (error) { - throw new Error(`Error loading node "${nodeTypeName}" from: "${filePath}"`); + throw new Error(`Error loading node "${nodeTypeName}" from: "${sourcePath}"`); } nodeTypesData[nodeTypeName] = { type: tempNode, - sourcePath: filePath, + sourcePath, }; } @@ -137,22 +127,18 @@ export class WorkflowRunnerProcess { const credentialsTypeData: ICredentialTypeData = {}; // eslint-disable-next-line no-restricted-syntax for (const credentialTypeName of Object.keys(this.data.credentialsTypeData)) { - className = this.data.credentialsTypeData[credentialTypeName].className; - - filePath = this.data.credentialsTypeData[credentialTypeName].sourcePath; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires - const tempModule = require(filePath); + let tempCredential: ICredentialType; + const { className, sourcePath } = this.data.credentialsTypeData[credentialTypeName]; try { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - tempCredential = new tempModule[className]() as ICredentialType; + tempCredential = loadClassInIsolation(sourcePath, className); } catch (error) { - throw new Error(`Error loading credential "${credentialTypeName}" from: "${filePath}"`); + throw new Error(`Error loading credential "${credentialTypeName}" from: "${sourcePath}"`); } credentialsTypeData[credentialTypeName] = { type: tempCredential, - sourcePath: filePath, + sourcePath, }; } From d8f8cafdb361616960bb4f9ce7f1122ff4c44e49 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 30 Aug 2022 09:50:07 +0200 Subject: [PATCH 2/3] Minor fixes and refactors --- packages/cli/src/CommunityNodes/helpers.ts | 8 ++++- packages/cli/src/LoadNodesAndCredentials.ts | 38 ++++++--------------- packages/cli/src/PackageHelper.ts | 7 ---- packages/cli/src/WorkflowRunnerProcess.ts | 2 +- 4 files changed, 19 insertions(+), 36 deletions(-) delete mode 100644 packages/cli/src/PackageHelper.ts diff --git a/packages/cli/src/CommunityNodes/helpers.ts b/packages/cli/src/CommunityNodes/helpers.ts index 38544efdb57c7..9fda8e0d6531b 100644 --- a/packages/cli/src/CommunityNodes/helpers.ts +++ b/packages/cli/src/CommunityNodes/helpers.ts @@ -4,7 +4,7 @@ import { promisify } from 'util'; import { exec } from 'child_process'; import { access as fsAccess, mkdir as fsMkdir } from 'fs/promises'; - +import { Script } from 'vm'; import axios from 'axios'; import { UserSettings } from 'n8n-core'; import { LoggerProxy, PublicInstalledPackage } from 'n8n-workflow'; @@ -235,3 +235,9 @@ export const isClientError = (error: Error): boolean => { export function isNpmError(error: unknown): error is { code: number; stdout: string } { return typeof error === 'object' && error !== null && 'code' in error && 'stdout' in error; } + +export const loadClassInIsolation = (filePath: string, className: string) => { + const script = new Script(`new (require('${filePath}').${className})()`); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return script.runInNewContext({ require }); +}; diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts index e3142d6e9e8c5..f302638d07156 100644 --- a/packages/cli/src/LoadNodesAndCredentials.ts +++ b/packages/cli/src/LoadNodesAndCredentials.ts @@ -37,13 +37,12 @@ import config from '../config'; import { NodeTypes } from '.'; import { InstalledPackages } from './databases/entities/InstalledPackages'; import { InstalledNodes } from './databases/entities/InstalledNodes'; -import { executeCommand } from './CommunityNodes/helpers'; +import { executeCommand, loadClassInIsolation } from './CommunityNodes/helpers'; import { RESPONSE_ERROR_MESSAGES } from './constants'; import { persistInstalledPackageData, removePackageFromDatabase, } from './CommunityNodes/packageModel'; -import { loadClassInIsolation } from './PackageHelper'; const CUSTOM_NODES_CATEGORY = 'Custom Nodes'; @@ -113,10 +112,8 @@ class LoadNodesAndCredentialsClass { await fsAccess(checkPath); // Folder exists, so use it. return path.dirname(checkPath); - } catch (error) { + } catch (_) { // Folder does not exist so get next one - // eslint-disable-next-line no-continue - continue; } } throw new Error('Could not find "node_modules" folder!'); @@ -153,8 +150,7 @@ class LoadNodesAndCredentialsClass { if (process.env[CUSTOM_EXTENSION_ENV] !== undefined) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV]!.split(';'); - // eslint-disable-next-line prefer-spread - customDirectories.push.apply(customDirectories, customExtensionFolders); + customDirectories.push(...customExtensionFolders); } for (const directory of customDirectories) { @@ -201,9 +197,7 @@ class LoadNodesAndCredentialsClass { * @param {string} filePath The file to read credentials from * @returns {Promise} */ - async loadCredentialsFromFile(credentialName: string, filePath: string): Promise { - // eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires - + loadCredentialsFromFile(credentialName: string, filePath: string): void { let tempCredential: ICredentialType; try { tempCredential = loadClassInIsolation(filePath, credentialName); @@ -354,13 +348,12 @@ class LoadNodesAndCredentialsClass { * @param {string} filePath The file to read node from * @returns {Promise} */ - async loadNodeFromFile( + loadNodeFromFile( packageName: string, nodeName: string, filePath: string, - ): Promise { + ): INodeTypeNameVersion | undefined { let tempNode: INodeType | INodeVersionedType; - let fullNodeName: string; let nodeVersion = 1; try { @@ -372,8 +365,7 @@ class LoadNodesAndCredentialsClass { throw error; } - // eslint-disable-next-line prefer-const - fullNodeName = `${packageName}.${tempNode.description.name}`; + const fullNodeName = `${packageName}.${tempNode.description.name}`; tempNode.description.name = fullNodeName; if (tempNode.description.icon !== undefined && tempNode.description.icon.startsWith('file:')) { @@ -483,8 +475,7 @@ class LoadNodesAndCredentialsClass { node.description.codex = codex; } catch (_) { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - this.logger.debug(`No codex available for: ${filePath.split('/').pop()}`); + this.logger.debug(`No codex available for: ${filePath.split('/').pop() ?? ''}`); if (isCustom) { node.description.codex = { @@ -504,19 +495,15 @@ class LoadNodesAndCredentialsClass { async loadDataFromDirectory(setPackageName: string, directory: string): Promise { const files = await glob(path.join(directory, '**/*.@(node|credentials).js')); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const loadPromises: any[] = []; for (const filePath of files) { const [fileName, type] = path.parse(filePath).name.split('.'); if (type === 'node') { - loadPromises.push(this.loadNodeFromFile(setPackageName, fileName, filePath)); + this.loadNodeFromFile(setPackageName, fileName, filePath); } else if (type === 'credentials') { - loadPromises.push(this.loadCredentialsFromFile(fileName, filePath)); + this.loadCredentialsFromFile(fileName, filePath); } } - - await Promise.all(loadPromises); } async readPackageJson(packagePath: string): Promise { @@ -534,7 +521,6 @@ class LoadNodesAndCredentialsClass { async loadDataFromPackage(packagePath: string): Promise { // Get the absolute path of the package const packageFile = await this.readPackageJson(packagePath); - // if (!packageFile.hasOwnProperty('n8n')) { if (!packageFile.n8n) { return []; } @@ -548,7 +534,7 @@ class LoadNodesAndCredentialsClass { for (const filePath of nodes) { const tempPath = path.join(packagePath, filePath); const [fileName] = path.parse(filePath).name.split('.'); - const loadData = await this.loadNodeFromFile(packageName, fileName, tempPath); + const loadData = this.loadNodeFromFile(packageName, fileName, tempPath); if (loadData) { returnData.push(loadData); } @@ -559,9 +545,7 @@ class LoadNodesAndCredentialsClass { if (Array.isArray(credentials)) { for (const filePath of credentials) { const tempPath = path.join(packagePath, filePath); - // eslint-disable-next-line @typescript-eslint/no-unused-vars const [fileName] = path.parse(filePath).name.split('.'); - // eslint-disable-next-line @typescript-eslint/no-floating-promises this.loadCredentialsFromFile(fileName, tempPath); } } diff --git a/packages/cli/src/PackageHelper.ts b/packages/cli/src/PackageHelper.ts deleted file mode 100644 index 40b5c736cb9c4..0000000000000 --- a/packages/cli/src/PackageHelper.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { Script } from 'vm'; - -export const loadClassInIsolation = (filePath: string, className: string) => { - const script = new Script(`new (require('${filePath}').${className})()`); - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return script.runInNewContext({ require }); -}; diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index d4eecad079d65..2c2b015ff79f6 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -49,7 +49,7 @@ import { getLogger } from './Logger'; import config from '../config'; import { InternalHooksManager } from './InternalHooksManager'; import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; -import { loadClassInIsolation } from './PackageHelper'; +import { loadClassInIsolation } from './CommunityNodes/helpers'; export class WorkflowRunnerProcess { data: IWorkflowExecutionDataProcessWithExecution | undefined; From 0dc47c8d51d5d481db6991e4bed3d79af247d673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 31 Aug 2022 16:07:51 +0200 Subject: [PATCH 3/3] reuse the same context for loading modules --- packages/cli/src/CommunityNodes/helpers.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/CommunityNodes/helpers.ts b/packages/cli/src/CommunityNodes/helpers.ts index 9fda8e0d6531b..cb634a4aa0399 100644 --- a/packages/cli/src/CommunityNodes/helpers.ts +++ b/packages/cli/src/CommunityNodes/helpers.ts @@ -4,7 +4,7 @@ import { promisify } from 'util'; import { exec } from 'child_process'; import { access as fsAccess, mkdir as fsMkdir } from 'fs/promises'; -import { Script } from 'vm'; +import { createContext, Script } from 'vm'; import axios from 'axios'; import { UserSettings } from 'n8n-core'; import { LoggerProxy, PublicInstalledPackage } from 'n8n-workflow'; @@ -236,8 +236,9 @@ export function isNpmError(error: unknown): error is { code: number; stdout: str return typeof error === 'object' && error !== null && 'code' in error && 'stdout' in error; } +const context = createContext({ require }); export const loadClassInIsolation = (filePath: string, className: string) => { const script = new Script(`new (require('${filePath}').${className})()`); // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return script.runInNewContext({ require }); + return script.runInContext(context); };