refactor(apps): prepare deno-runtime for Deno upgrade (RocketChat#39701)

Author: d-gubert

packages/apps-engine/deno-runtime/deno.jsonc

@@ -12,6 +12,7 @@
 		"stack-trace": "npm:stack-trace@0.0.10",
 		"uuid": "npm:uuid@8.3.2"
 	},
+	"unstable": ["detect-cjs"],
 	"tasks": {
 		"test": "deno test --no-check --allow-read=../../../,/tmp --allow-write=/tmp"
 	},

packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts

@@ -1,4 +1,5 @@
 import * as child_process from 'child_process';
+import * as fs from 'fs';
 import * as path from 'path';
 import { type Readable, EventEmitter } from 'stream';
 import { inspect as utilInspect } from 'util';
@@ -11,7 +12,7 @@ import { ProcessMessenger } from './ProcessMessenger';
 import { bundleLegacyApp } from './bundler';
 import { newDecoder } from './codec';
 import { AppStatus, AppStatusUtils } from '../../../definition/AppStatus';
-import { AppInterface, AppMethod } from '../../../definition/metadata';
+import { AppMethod } from '../../../definition/metadata';
 import type { AppManager } from '../../AppManager';
 import type { AppBridges } from '../../bridges';
 import type { IParseAppPackageResult } from '../../compiler';
@@ -75,13 +76,13 @@ export function isValidOrigin(accessor: string): accessor is (typeof ALLOWED_ACC
 	return ALLOWED_ACCESSOR_METHODS.includes(accessor as any);
 }
 
-export function getDenoWrapperPath(): string {
+export function getDenoConfigPath(): string {
 	try {
 		// This path is relative to the compiled version of the Apps-Engine source
-		return require.resolve('../../../deno-runtime/main.ts');
+		return require.resolve('../../../deno-runtime/deno.jsonc');
 	} catch {
-		// This path is relative to the original Apps-Engine files
-		return require.resolve('../../../../deno-runtime/main.ts');
+		// This path is relative to the original Apps-Engine files - used during tests
+		return require.resolve('../../../../deno-runtime/deno.jsonc');
 	}
 }
 
@@ -117,14 +118,38 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu
 
 	private readonly tempFilePath: string;
 
-	// We need to keep the appSource around in case the Deno process needs to be restarted
+	private readonly denoRuntimePath: string;
+
+	private readonly denoConfigPath: string;
+
 	constructor(
 		manager: AppManager,
+		// We need to keep the appSource around in case the Deno process needs to be restarted
 		private readonly appPackage: IParseAppPackageResult,
 		private readonly storageItem: IAppStorageItem,
 	) {
 		super();
 
+		this.tempFilePath = manager.getTempFilePath();
+		this.denoRuntimePath = path.join(this.tempFilePath, 'deno-runtime', 'main.ts');
+		this.denoConfigPath = getDenoConfigPath();
+
+		/**
+		 * Deno 2.x refuses to run scripts inside the node_modules, so we create a symlink to the deno runtime files in the temp directory
+		 * The temp directory is the same we are given by the host to store temporary upload files
+		 */
+		try {
+			fs.symlinkSync(
+				path.dirname(this.denoConfigPath),
+				path.dirname(this.denoRuntimePath),
+				'dir'
+			);
+		} catch (reason: unknown) {
+			if ((reason as NodeJS.ErrnoException).code !== 'EEXIST') {
+				throw reason;
+			}
+		}
+
 		this.debug = baseDebug.extend(appPackage.info.id);
 		this.messenger = new ProcessMessenger();
 		this.livenessManager = new LivenessManager({
@@ -139,31 +164,26 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu
 		this.api = manager.getApiManager();
 		this.logStorage = manager.getLogStorage();
 		this.bridges = manager.getBridges();
-		this.tempFilePath = manager.getTempFilePath();
 	}
 
 	public spawnProcess(): void {
 		try {
 			const denoExePath = 'deno';
 
-			const denoWrapperPath = getDenoWrapperPath();
+			const denoWrapperPath = this.denoRuntimePath;
 			// During development, the appsEngineDir is enough to run the deno process
-			const appsEngineDir = path.dirname(path.join(denoWrapperPath, '..'));
+			const appsEngineDir = path.dirname(path.join(this.denoConfigPath, '..'));
 			const DENO_DIR = process.env.DENO_DIR ?? path.join(appsEngineDir, '.deno-cache');
 			// When running in production, we're likely inside a node_modules which the Deno
 			// process must be able to read in order to include files that use NPM packages
 			const parentNodeModulesDir = path.dirname(path.join(appsEngineDir, '..'));
 
-			const allowedDirs = [appsEngineDir, parentNodeModulesDir];
-
-			// If the app handles file upload events, it needs to be able to read the temp dir
-			if (this.appPackage.implemented.doesImplement(AppInterface.IPreFileUpload)) {
-				allowedDirs.push(this.tempFilePath);
-			}
+			const allowedDirs = [appsEngineDir, parentNodeModulesDir, this.tempFilePath];
 
 			const options = [
 				'run',
 				'--cached-only',
+				`--config=${this.denoConfigPath}`,
 				`--allow-read=${allowedDirs.join(',')}`,
 				`--allow-env=${ALLOWED_ENVIRONMENT_VARIABLES.join(',')}`,
 				denoWrapperPath,
@@ -188,6 +208,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu
 				},
 			};
 
+			// SECURITY: We control the command, the arguments and the script that will be executed.
 			this.deno = child_process.spawn(denoExePath, options, environment);
 			this.messenger.setReceiver(this.deno);
 			this.livenessManager.attach(this.deno);

packages/apps-engine/tests/server/AppManager.spec.ts

@@ -36,6 +36,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		Expect(manager.getStorage()).toBe(this.testingInfastructure.getAppStorage());
@@ -51,6 +52,7 @@ export class AppManagerTestFixture {
 					logStorage: {} as AppLogStorage,
 					bridges: {} as AppBridges,
 					sourceStorage: {} as AppSourceStorage,
+					tempFilePath: 'temp-file-path',
 				}),
 		).toThrowError(Error, 'There is already a valid AppManager instance');
 	}
@@ -66,6 +68,7 @@ export class AppManagerTestFixture {
 					logStorage: invalid as any,
 					bridges: invalid as any,
 					sourceStorage: invalid as any,
+					tempFilePath: 'temp-file-path',
 				}),
 		).toThrowError(Error, 'Invalid instance of the AppMetadataStorage');
 
@@ -76,6 +79,7 @@ export class AppManagerTestFixture {
 					logStorage: invalid as any,
 					bridges: invalid as any,
 					sourceStorage: invalid as any,
+					tempFilePath: 'temp-file-path',
 				}),
 		).toThrowError(Error, 'Invalid instance of the AppLogStorage');
 
@@ -86,6 +90,7 @@ export class AppManagerTestFixture {
 					logStorage: this.testingInfastructure.getLogStorage(),
 					bridges: invalid as any,
 					sourceStorage: invalid as any,
+					tempFilePath: 'temp-file-path',
 				}),
 		).toThrowError(Error, 'Invalid instance of the AppBridges');
 
@@ -96,6 +101,7 @@ export class AppManagerTestFixture {
 					logStorage: this.testingInfastructure.getLogStorage(),
 					bridges: this.testingInfastructure.getAppBridges(),
 					sourceStorage: invalid as any,
+					tempFilePath: this.testingInfastructure.getTempFilePath(),
 				}),
 		).toThrowError(Error, 'Invalid instance of the AppSourceStorage');
 	}
@@ -107,6 +113,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		Expect(manager.getParser() instanceof AppPackageParser).toBe(true);
@@ -129,6 +136,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		const appsOverview = TestData.getAppsOverview();
@@ -153,6 +161,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		const appsOverview = TestData.getAppsOverview();
@@ -177,6 +186,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		const sameLicenseData = 'same-license-data';
@@ -217,6 +227,7 @@ export class AppManagerTestFixture {
 			logStorage: this.testingInfastructure.getLogStorage(),
 			bridges: this.testingInfastructure.getAppBridges(),
 			sourceStorage: this.testingInfastructure.getSourceStorage(),
+			tempFilePath: this.testingInfastructure.getTempFilePath(),
 		});
 
 		const existingSubscriptionInfo = TestData.getMarketplaceSubscriptionInfo({

packages/apps-engine/tests/server/managers/AppRuntimeManager.spec.ts

@@ -6,6 +6,7 @@ import type { IParseAppPackageResult } from '../../../src/server/compiler';
 import { AppRuntimeManager } from '../../../src/server/managers/AppRuntimeManager';
 import type { IRuntimeController } from '../../../src/server/runtime/IRuntimeController';
 import type { IAppStorageItem } from '../../../src/server/storage';
+import { TestInfastructureSetup } from '../../test-data/utilities';
 
 export class AppRuntimeManagerTestFixture {
 	private mockManager: AppManager;
@@ -20,20 +21,9 @@ export class AppRuntimeManagerTestFixture {
 
 	@SetupFixture
 	public setupFixture() {
-		this.mockManager = {
-			getAccessorManager() {
-				return {} as any;
-			},
-			getApiManager() {
-				return {} as any;
-			},
-			getLogStorage() {
-				return {} as any;
-			},
-			getBridges() {
-				return {} as any;
-			},
-		} as AppManager;
+		const testInfrastructure = new TestInfastructureSetup();
+
+		this.mockManager = testInfrastructure.getMockManager();
 
 		this.mockAppPackage = {
 			info: {

packages/apps-engine/tests/server/runtime/DenoRuntimeSubprocessController.spec.ts

@@ -1,7 +1,8 @@
 import * as fs from 'fs/promises';
+import * as os from 'os';
 import * as path from 'path';
 
-import { TestFixture, Setup, Expect, AsyncTest, SpyOn, Any, AsyncSetupFixture, Teardown } from 'alsatian';
+import { TestFixture, Setup, Expect, AsyncTest, SpyOn, Any, AsyncSetupFixture, Teardown, TeardownFixture } from 'alsatian';
 import type { SuccessObject } from 'jsonrpc-lite';
 
 import { AppStatus } from '../../../src/definition/AppStatus';
@@ -57,6 +58,13 @@ export class DenuRuntimeSubprocessControllerTestFixture {
 		this.controller.stopApp();
 	}
 
+	@TeardownFixture
+	public async teardownFixture() {
+		await fs.unlink(path.join(os.tmpdir(), 'deno-runtime')).catch((reason) => {
+			console.warn('Failed to delete temporary Deno runtime symlink', reason);
+		});
+	}
+
 	@AsyncTest('correctly identifies a call to the HTTP accessor')
 	public async testHttpAccessor() {
 		const spy = SpyOn(this.manager.getBridges().getHttpBridge(), 'doCall');

packages/apps-engine/tests/test-data/storage/storage.ts

@@ -10,7 +10,17 @@ const Datastore = require('@seald-io/nedb') as typeof import('@seald-io/nedb').d
 export class TestsAppStorage extends AppMetadataStorage {
 	private db: InstanceType<typeof Datastore>;
 
-	constructor() {
+	private static instance: TestsAppStorage;
+
+	public static getInstance(): TestsAppStorage {
+		if (!TestsAppStorage.instance) {
+			TestsAppStorage.instance = new TestsAppStorage();
+		}
+
+		return TestsAppStorage.instance;
+	}
+
+	private constructor() {
 		super('nedb');
 		this.db = new Datastore({ filename: 'tests/test-data/dbs/apps.nedb', autoload: true });
 		this.db.ensureIndex({ fieldName: 'id', unique: true });

packages/apps-engine/tests/test-data/utilities.ts

@@ -1,3 +1,5 @@
+import * as os from 'os';
+
 import { TestsAppBridges } from './bridges/appBridges';
 import { TestSourceStorage } from './storage/TestSourceStorage';
 import { TestsAppLogStorage } from './storage/logStorage';
@@ -73,7 +75,7 @@ export class TestInfastructureSetup {
 	private runtimeManager: AppRuntimeManager;
 
 	constructor() {
-		this.appStorage = new TestsAppStorage();
+		this.appStorage = TestsAppStorage.getInstance();
 		this.logStorage = new TestsAppLogStorage();
 		this.bridges = new TestsAppBridges();
 		this.sourceStorage = new TestSourceStorage();
@@ -130,10 +132,14 @@ export class TestInfastructureSetup {
 			getRuntime: () => {
 				return this.runtimeManager;
 			},
-			getTempFilePath: () => 'temp-file-path',
+			getTempFilePath: this.getTempFilePath,
 		} as unknown as AppManager;
 	}
 
+	public getTempFilePath(): string {
+		return os.tmpdir();
+	}
+
 	public getAppStorage(): AppMetadataStorage {
 		return this.appStorage;
 	}