[pull] develop from RocketChat:develop

.changeset/spotty-steaks-notice.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/apps-engine': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes an issue that caused a spike in memory usage when apps handled the IPreFileUpload event

apps/meteor/app/apps/server/bridges/listeners.ts

@@ -1,3 +1,7 @@
+import crypto from 'crypto';
+import fs from 'fs';
+import path from 'path';
+
 import type { IAppServerOrchestrator, IAppsRoom, IAppsLivechatRoom, IAppsMessage } from '@rocket.chat/apps';
 import type { IPreEmailSentContext } from '@rocket.chat/apps-engine/definition/email';
 import type { IExternalComponent } from '@rocket.chat/apps-engine/definition/externalComponent';
@@ -6,9 +10,8 @@ import { isLivechatRoom } from '@rocket.chat/apps-engine/definition/livechat/ILi
 import { AppInterface } from '@rocket.chat/apps-engine/definition/metadata';
 import type { UIKitIncomingInteraction } from '@rocket.chat/apps-engine/definition/uikit';
 import type { IUIKitLivechatIncomingInteraction } from '@rocket.chat/apps-engine/definition/uikit/livechat';
-import type { IFileUploadContext } from '@rocket.chat/apps-engine/definition/uploads';
 import type { IUserContext, IUserUpdateContext } from '@rocket.chat/apps-engine/definition/users';
-import type { IMessage, IRoom, IUser, ILivechatDepartment } from '@rocket.chat/core-typings';
+import type { IMessage, IRoom, IUser, ILivechatDepartment, IUpload } from '@rocket.chat/core-typings';
 
 type LivechatTransferData = {
 	type: LivechatTransferEventType;
@@ -161,23 +164,32 @@ type HandleDefaultEvent =
 			event: AppInterface.IUIKitLivechatInteractionHandler;
 			payload: [IUIKitLivechatIncomingInteraction];
 	  }
-	| {
-			event: AppInterface.IPreFileUpload;
-			payload: [IFileUploadContext];
-	  }
 	| {
 			event: AppInterface.IPreEmailSent;
 			payload: [IPreEmailSentContext];
 	  };
 
-type HandleEvent = HandleMessageEvent | HandleRoomEvent | HandleLivechatEvent | HandleUserEvent | HandleDefaultEvent;
+type HandleFileUploadEvent = {
+	event: AppInterface.IPreFileUpload;
+	payload: [{ file: IUpload; content: Buffer }];
+};
+
+type HandleEvent =
+	| HandleMessageEvent
+	| HandleRoomEvent
+	| HandleLivechatEvent
+	| HandleUserEvent
+	| HandleFileUploadEvent
+	| HandleDefaultEvent;
 
 export class AppListenerBridge {
 	constructor(private readonly orch: IAppServerOrchestrator) {}
 
 	// eslint-disable-next-line complexity
 	async handleEvent(args: HandleEvent): Promise<any> {
 		switch (args.event) {
+			case AppInterface.IPreFileUpload:
+				return this.uploadEvent(args);
 			case AppInterface.IPostMessageDeleted:
 			case AppInterface.IPostMessageReacted:
 			case AppInterface.IPostMessageFollowed:
@@ -237,6 +249,40 @@ export class AppListenerBridge {
 		return this.orch.getManager().getListenerManager().executeListener(args.event, args.payload[0]);
 	}
 
+	async uploadEvent(args: HandleFileUploadEvent): Promise<void> {
+		const [{ file, content }] = args.payload;
+
+		const tmpfile = path.join(this.orch.getManager().getTempFilePath(), crypto.randomUUID());
+		await fs.promises.writeFile(tmpfile, content).catch((err) => {
+			this.orch.getRocketChatLogger().error({ msg: `AppListenerBridge: Could not write temporary file at ${tmpfile}`, err });
+
+			throw new Error('Error sending file to apps', { cause: err });
+		});
+
+		try {
+			const uploadDetails = {
+				name: file.name || '',
+				size: file.size || 0,
+				type: file.type || '',
+				rid: file.rid || '',
+				userId: file.userId || '',
+				visitorToken: file.token,
+			};
+
+			// Execute both events for backward compatibility
+			await this.orch.getManager().getListenerManager().executeListener(AppInterface.IPreFileUpload, {
+				file: uploadDetails,
+				path: tmpfile,
+			});
+		} finally {
+			await fs.promises
+				.unlink(tmpfile)
+				.catch((err) =>
+					this.orch.getRocketChatLogger().warn({ msg: `AppListenerBridge: Could not delete temporary file at ${tmpfile}`, err }),
+				);
+		}
+	}
+
 	async messageEvent(args: HandleMessageEvent): Promise<boolean | IMessage | undefined> {
 		const [message] = args.payload;
 

apps/meteor/ee/server/apps/orchestrator.js

@@ -1,3 +1,7 @@
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
 import { registerOrchestrator } from '@rocket.chat/apps';
 import { EssentialAppDisabledException } from '@rocket.chat/apps-engine/definition/exceptions';
 import { AppManager } from '@rocket.chat/apps-engine/server/AppManager';
@@ -68,11 +72,24 @@ export class AppServerOrchestrator {
 
 		this._bridges = new RealAppBridges(this);
 
+		const tempFilePath = path.join(os.tmpdir(), 'apps-engine-temp');
+
+		try {
+			// We call this only once at server startup, so using the synchronous version is fine
+			fs.mkdirSync(tempFilePath);
+		} catch (err) {
+			// If the temp directory already exists, we can continue
+			if (err.code !== 'EEXIST') {
+				throw new Error('Failed to initialize the Apps-Engine', { cause: err });
+			}
+		}
+
 		this._manager = new AppManager({
 			metadataStorage: this._storage,
 			logStorage: this._logStorage,
 			bridges: this._bridges,
 			sourceStorage: this._appSourceStorage,
+			tempFilePath,
 		});
 
 		this._communicators = new Map();

apps/meteor/tests/data/apps/app-packages/README.md

@@ -0,0 +1,54 @@
+# Test App Packages
+
+Includes pre-built app packages that are designed to test specific APIs exposed by the Apps-engine.
+
+## How to use
+
+In your tests, add a `before` step and call the `installLocalTestPackage` function, passing the path of your desired package. For instance:
+
+```javascript
+import { appImplementsIPreFileUpload } from '../../data/apps/app-packages';
+import { installLocalTestPackage } from '../../data/apps/helper';
+
+describe('My tests', () => {
+	before(async () => {
+		await installLocalTestPackage(appImplementsIPreFileUpload);
+	});
+});
+```
+
+### Available apps
+
+#### IPreFileUpload handler
+
+File name: `file-upload-test_0.0.1.zip`
+
+An app that handles the `IPreFileUpload` event. If the file name starts with `"test-should-reject"`, the app will prevent the upload from happening. The error message will contain the contents of the uploaded file as evidence that the app could successfully read them.
+
+<details>
+<summary>App source code</summary>
+
+```typescript
+import {
+    IHttp,
+    IModify,
+    IPersistence,
+    IRead,
+} from '@rocket.chat/apps-engine/definition/accessors';
+import { App } from '@rocket.chat/apps-engine/definition/App';
+import { FileUploadNotAllowedException } from '@rocket.chat/apps-engine/definition/exceptions';
+import { IPreFileUpload } from '@rocket.chat/apps-engine/definition/uploads';
+import { IFileUploadContext } from '@rocket.chat/apps-engine/definition/uploads/IFileUploadContext';
+
+export class TestIPreFileUpload extends App implements IPreFileUpload {
+    public async executePreFileUpload(context: IFileUploadContext, read: IRead, http: IHttp, persis: IPersistence, modify: IModify): Promise<void> {
+        if (context.file.name.startsWith('test-should-reject')) {
+            console.log('[executePreFileUpload] Rejecting file which name starts with "test-should-reject"');
+            throw new FileUploadNotAllowedException(`Test file rejected ${context.content.toString()}`);
+        }
+        console.log('[executePreFileUpload] Did not reject file');
+    }
+}
+```
+
+</details>

apps/meteor/tests/data/apps/app-packages/index.ts

@@ -0,0 +1,3 @@
+import * as path from 'path';
+
+export const appImplementsIPreFileUpload = path.resolve(__dirname, './file-upload-test_0.0.1.zip');

apps/meteor/tests/data/apps/helper.ts

@@ -1,7 +1,7 @@
 import type { App } from '@rocket.chat/core-typings';
 
 import { request, credentials } from '../api-data';
-import { apps, APP_URL, APP_NAME, installedApps } from './apps-data';
+import { apps, APP_URL, installedApps } from './apps-data';
 
 const getApps = () =>
 	new Promise<App[]>((resolve) => {
@@ -23,10 +23,7 @@ const removeAppById = (id: App['id']) =>
 
 export const cleanupApps = async () => {
 	const apps = await getApps();
-	const testApp = apps.find((app) => app.name === APP_NAME);
-	if (testApp) {
-		await removeAppById(testApp.id);
-	}
+	await Promise.all(apps.map((testApp) => removeAppById(testApp.id)));
 };
 
 export const installTestApp = () =>
@@ -41,3 +38,17 @@ export const installTestApp = () =>
 				resolve(res.body.app);
 			});
 	});
+
+export const installLocalTestPackage = (path: string) =>
+	new Promise<App>((resolve, reject) => {
+		void request
+			.post(apps())
+			.set(credentials)
+			.attach('app', path)
+			.end((err, res) => {
+				if (err) {
+					return reject(err);
+				}
+				return resolve(res.body.app);
+			});
+	});

apps/meteor/tests/end-to-end/apps/apps-hooks-file-upload.ts

@@ -0,0 +1,56 @@
+import type { IRoom } from '@rocket.chat/core-typings';
+import { expect } from 'chai';
+import { after, before, describe, it } from 'mocha';
+import type { Response } from 'supertest';
+
+import { getCredentials, request, credentials, api } from '../../data/api-data';
+import { appImplementsIPreFileUpload } from '../../data/apps/app-packages';
+import { cleanupApps, installLocalTestPackage } from '../../data/apps/helper';
+import { createRoom, deleteRoom } from '../../data/rooms.helper';
+import { IS_EE } from '../../e2e/config/constants';
+
+(IS_EE ? describe : describe.skip)('[Apps Hooks - File Upload]', () => {
+	before((done) => getCredentials(done));
+
+	describe('IPreFileUpload', () => {
+		let room: IRoom;
+
+		before(async () => {
+			await cleanupApps();
+			await installLocalTestPackage(appImplementsIPreFileUpload);
+			room = await createRoom({ type: 'c', name: `file-upload-hook-${Date.now()}` }).then((res) => res.body.channel);
+		});
+
+		after(() => Promise.all([deleteRoom({ type: 'c' as const, roomId: room._id }), cleanupApps()]));
+
+		it('should be capable of rejecting an upload based on app logic', async () => {
+			const fileContents = 'I want to be rejected by the app';
+
+			await request
+				.post(api(`rooms.media/${room._id}`))
+				.set(credentials)
+				.attach('file', Buffer.from(fileContents), { filename: 'test-should-reject' })
+				.expect('Content-Type', 'application/json')
+				.expect(400)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', false);
+					expect(res.body).to.have.property('errorType', 'error-app-prevented');
+					expect(res.body).to.have.property('error').that.is.string(fileContents);
+				});
+		});
+
+		it('should not prevent an unrelated file upload', async () => {
+			const fileContents = 'I should not be rejected';
+
+			await request
+				.post(api(`rooms.media/${room._id}`))
+				.set(credentials)
+				.attach('file', Buffer.from(fileContents), { filename: 'test-file' })
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+				});
+		});
+	});
+});

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

@@ -4,6 +4,7 @@
 		"@rocket.chat/apps-engine/": "./../src/",
 		"@rocket.chat/ui-kit": "npm:@rocket.chat/ui-kit@^0.31.22",
 		"@std/cli": "jsr:@std/cli@^1.0.9",
+		"@std/streams": "jsr:@std/streams@^1.0.16",
 		"acorn": "npm:[email protected]",
 		"acorn-walk": "npm:[email protected]",
 		"astring": "npm:[email protected]",