diff --git a/.changeset/spotty-steaks-notice.md b/.changeset/spotty-steaks-notice.md new file mode 100644 index 0000000000000..6a47834f69d47 --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/apps/server/bridges/listeners.ts b/apps/meteor/app/apps/server/bridges/listeners.ts index 6abf276d9f926..4e1c899072851 100644 --- a/apps/meteor/app/apps/server/bridges/listeners.ts +++ b/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,16 +164,23 @@ 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) {} @@ -178,6 +188,8 @@ export class AppListenerBridge { // eslint-disable-next-line complexity async handleEvent(args: HandleEvent): Promise { 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 { + 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 { const [message] = args.payload; diff --git a/apps/meteor/ee/server/apps/orchestrator.js b/apps/meteor/ee/server/apps/orchestrator.js index e59c6b2444052..7188695ac3f43 100644 --- a/apps/meteor/ee/server/apps/orchestrator.js +++ b/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(); diff --git a/apps/meteor/tests/data/apps/app-packages/README.md b/apps/meteor/tests/data/apps/app-packages/README.md new file mode 100644 index 0000000000000..03da4595a87c5 --- /dev/null +++ b/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. + +
+App source code + +```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 { + 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'); + } +} +``` + +
diff --git a/apps/meteor/tests/data/apps/app-packages/file-upload-test_0.0.1.zip b/apps/meteor/tests/data/apps/app-packages/file-upload-test_0.0.1.zip new file mode 100644 index 0000000000000..05183f3f05cb2 Binary files /dev/null and b/apps/meteor/tests/data/apps/app-packages/file-upload-test_0.0.1.zip differ diff --git a/apps/meteor/tests/data/apps/app-packages/index.ts b/apps/meteor/tests/data/apps/app-packages/index.ts new file mode 100644 index 0000000000000..d221164b6a5cb --- /dev/null +++ b/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'); diff --git a/apps/meteor/tests/data/apps/helper.ts b/apps/meteor/tests/data/apps/helper.ts index dfa945a470dc1..7871b51dafb3d 100644 --- a/apps/meteor/tests/data/apps/helper.ts +++ b/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((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((resolve, reject) => { + void request + .post(apps()) + .set(credentials) + .attach('app', path) + .end((err, res) => { + if (err) { + return reject(err); + } + return resolve(res.body.app); + }); + }); diff --git a/apps/meteor/tests/end-to-end/apps/apps-hooks-file-upload.ts b/apps/meteor/tests/end-to-end/apps/apps-hooks-file-upload.ts new file mode 100644 index 0000000000000..9cd3576705f52 --- /dev/null +++ b/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); + }); + }); + }); +}); diff --git a/packages/apps-engine/deno-runtime/deno.jsonc b/packages/apps-engine/deno-runtime/deno.jsonc index cefd268cf8856..525eda492feb4 100644 --- a/packages/apps-engine/deno-runtime/deno.jsonc +++ b/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:acorn@8.10.0", "acorn-walk": "npm:acorn-walk@8.2.0", "astring": "npm:astring@1.8.6", diff --git a/packages/apps-engine/deno-runtime/deno.lock b/packages/apps-engine/deno-runtime/deno.lock index 61763f056cce3..6dbfd05882fe2 100644 --- a/packages/apps-engine/deno-runtime/deno.lock +++ b/packages/apps-engine/deno-runtime/deno.lock @@ -2,7 +2,9 @@ "version": "3", "packages": { "specifiers": { + "jsr:@std/bytes@^1.0.6": "jsr:@std/bytes@1.0.6", "jsr:@std/cli@^1.0.9": "jsr:@std/cli@1.0.13", + "jsr:@std/streams@^1.0.16": "jsr:@std/streams@1.0.16", "npm:@msgpack/msgpack@3.0.0-beta2": "npm:@msgpack/msgpack@3.0.0-beta2", "npm:@rocket.chat/ui-kit@^0.31.22": "npm:@rocket.chat/ui-kit@0.31.25_@rocket.chat+icons@0.32.0", "npm:acorn-walk@8.2.0": "npm:acorn-walk@8.2.0", @@ -14,8 +16,17 @@ "npm:uuid@8.3.2": "npm:uuid@8.3.2" }, "jsr": { + "@std/bytes@1.0.6": { + "integrity": "f6ac6adbd8ccd99314045f5703e23af0a68d7f7e58364b47d2c7f408aeb5820a" + }, "@std/cli@1.0.13": { "integrity": "5db2d95ab2dca3bca9fb6ad3c19908c314e93d6391c8b026725e4892d4615a69" + }, + "@std/streams@1.0.16": { + "integrity": "85030627befb1767c60d4f65cb30fa2f94af1d6ee6e5b2515b76157a542e89c4", + "dependencies": [ + "jsr:@std/bytes@^1.0.6" + ] } }, "npm": { @@ -102,6 +113,7 @@ "workspace": { "dependencies": [ "jsr:@std/cli@^1.0.9", + "jsr:@std/streams@^1.0.16", "npm:@msgpack/msgpack@3.0.0-beta2", "npm:@rocket.chat/ui-kit@^0.31.22", "npm:acorn-walk@8.2.0", diff --git a/packages/apps-engine/deno-runtime/handlers/app/handleUploadEvents.ts b/packages/apps-engine/deno-runtime/handlers/app/handleUploadEvents.ts new file mode 100644 index 0000000000000..bb39c75e3d236 --- /dev/null +++ b/packages/apps-engine/deno-runtime/handlers/app/handleUploadEvents.ts @@ -0,0 +1,73 @@ +import { Buffer } from 'node:buffer'; + +import type { App } from '@rocket.chat/apps-engine/definition/App.ts'; +import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions/AppsEngineException.ts'; +import type { IFileUploadContext } from '@rocket.chat/apps-engine/definition/uploads/IFileUploadContext.ts' +import type { IUploadDetails } from '@rocket.chat/apps-engine/definition/uploads/IUploadDetails.ts' +import { toArrayBuffer } from '@std/streams'; +import { Defined, JsonRpcError } from 'jsonrpc-lite'; + +import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; +import { assertAppAvailable, assertHandlerFunction, isRecord } from '../lib/assertions.ts'; +import { AppAccessorsInstance } from '../../lib/accessors/mod.ts'; + +export const uploadEvents = ['executePreFileUpload'] as const; + +function assertIsUpload(v: unknown): asserts v is IUploadDetails { + if (isRecord(v) && !!v.rid && (!!v.userId || !!v.visitorToken)) return; + + throw JsonRpcError.invalidParams({ err: `Invalid 'file' parameter. Expected IUploadDetails, got`, value: v }); +} + +function assertString(v: unknown): asserts v is string { + if (v && typeof v === 'string') return; + + throw JsonRpcError.invalidParams({ err: `Invalid 'path' parameter. Expected string, got`, value: v }); +} + +export default async function handleUploadEvents(method: typeof uploadEvents[number], params: unknown): Promise { + const [{ file, path }] = params as [{ file?: IUpload, path?: string }]; + + const app = AppObjectRegistry.get('app'); + const handlerFunction = app?.[method as keyof App] as unknown; + + try { + assertAppAvailable(app); + assertHandlerFunction(handlerFunction); + assertIsUpload(file); + assertString(path); + + using tempFile = await Deno.open(path, { read: true, create: false }); + let context: IFileUploadContext; + + switch (method) { + case 'executePreFileUpload': { + const fileContents = await toArrayBuffer(tempFile.readable); + context = { file, content: Buffer.from(fileContents) }; + break; + } + } + + return await handlerFunction.call( + app, + context, + AppAccessorsInstance.getReader(), + AppAccessorsInstance.getHttp(), + AppAccessorsInstance.getPersistence(), + AppAccessorsInstance.getModifier(), + ); + } catch(e) { + if (e?.name === AppsEngineException.name) { + return new JsonRpcError(e.message, AppsEngineException.JSONRPC_ERROR_CODE, { name: e.name }); + } + + if (e instanceof JsonRpcError) { + return e; + } + + return JsonRpcError.internalError({ + err: e.message, + ...(e.code && { code: e.code }), + }); + } +} diff --git a/packages/apps-engine/deno-runtime/handlers/app/handler.ts b/packages/apps-engine/deno-runtime/handlers/app/handler.ts index 171d179a2d05d..cfb2df08cfb69 100644 --- a/packages/apps-engine/deno-runtime/handlers/app/handler.ts +++ b/packages/apps-engine/deno-runtime/handlers/app/handler.ts @@ -15,6 +15,8 @@ import handleListener from '../listener/handler.ts'; import handleUIKitInteraction, { uikitInteractions } from '../uikit/handler.ts'; import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; import handleOnUpdate from './handleOnUpdate.ts'; +import handleUploadEvents, { uploadEvents } from './handleUploadEvents.ts'; +import { isOneOf } from '../lib/assertions.ts'; export default async function handleApp(method: string, params: unknown): Promise { const [, appMethod] = method.split(':'); @@ -30,46 +32,35 @@ export default async function handleApp(method: string, params: unknown): Promis app?.getLogger().debug({ msg: `A method is being called...`, appMethod }); - if (uikitInteractions.includes(appMethod)) { - return handleUIKitInteraction(appMethod, params).then((result) => { - if (result instanceof JsonRpcError) { - app?.getLogger().debug({ - msg: `Method call was unsuccessful.`, - appMethod, - err: result, - errorMessage: result.message, - }); - } else { - app?.getLogger().debug({ - msg: `Method was successfully called! The result is:`, - appMethod, - result, - }); - } - - return result; - }); + const formatResult = (result: Defined | JsonRpcError): Defined | JsonRpcError => { + if (result instanceof JsonRpcError) { + app?.getLogger().debug({ + msg: `'${appMethod}' was unsuccessful.`, + appMethod, + err: result, + errorMessage: result.message, + }); + } else { + app?.getLogger().debug({ + msg: `'${appMethod}' was successfully called! The result is:`, + appMethod, + result, + }); + } + + return result; + }; + + if (app && isOneOf(appMethod, uploadEvents)) { + return handleUploadEvents(appMethod, params).then(formatResult); } - if (appMethod.startsWith('check') || appMethod.startsWith('execute')) { - return handleListener(appMethod, params).then((result) => { - if (result instanceof JsonRpcError) { - app?.getLogger().debug({ - msg: `'${appMethod}' was unsuccessful.`, - appMethod, - err: result, - errorMessage: result.message, - }); - } else { - app?.getLogger().debug({ - msg: `'${appMethod}' was successfully called! The result is:`, - appMethod, - result, - }); - } - - return result; - }); + if (app && isOneOf(appMethod, uikitInteractions)) { + return handleUIKitInteraction(appMethod, params).then(formatResult); + } + + if (app && (appMethod.startsWith('check') || appMethod.startsWith('execute'))) { + return handleListener(appMethod, params).then(formatResult); } let result: Defined | JsonRpcError; diff --git a/packages/apps-engine/deno-runtime/handlers/lib/assertions.ts b/packages/apps-engine/deno-runtime/handlers/lib/assertions.ts new file mode 100644 index 0000000000000..c154015d24b02 --- /dev/null +++ b/packages/apps-engine/deno-runtime/handlers/lib/assertions.ts @@ -0,0 +1,33 @@ +import type { App } from '@rocket.chat/apps-engine/definition/App.ts'; +import { JsonRpcError } from 'jsonrpc-lite'; + +export function isRecord(v: unknown): v is Record { + if (!v || typeof v !== 'object') { + return false; + } + + const prototype = Object.getPrototypeOf(v); + + return prototype === null || prototype.constructor === Object; +} + +/** + * Type guard function to check if a value is included in a readonly array + * and narrow its type accordingly. + */ +export function isOneOf(value: unknown, array: readonly T[]): value is T { + return array.includes(value as T); +} + +export function assertAppAvailable(v: unknown): asserts v is App { + if (v && typeof (v as App)['extendConfiguration'] === 'function') return; + + throw JsonRpcError.internalError({ err: 'App object not available' }); +} + +// deno-lint-ignore ban-types -- Function is the best we can do at this time +export function assertHandlerFunction(v: unknown): asserts v is Function { + if (v instanceof Function) return; + + throw JsonRpcError.internalError({ err: `Expected handler function, got ${v}` }); +} diff --git a/packages/apps-engine/src/definition/uploads/IFileUploadContext.ts b/packages/apps-engine/src/definition/uploads/IFileUploadContext.ts index cd0de6127029b..3aa78e24bcc03 100644 --- a/packages/apps-engine/src/definition/uploads/IFileUploadContext.ts +++ b/packages/apps-engine/src/definition/uploads/IFileUploadContext.ts @@ -1,5 +1,10 @@ import type { IUploadDetails } from './IUploadDetails'; +export interface IFileUploadInternalContext { + file: IUploadDetails; + path: string; +} + export interface IFileUploadContext { file: IUploadDetails; content: Buffer; diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 4c8348b726653..826b2b1b95148 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -54,6 +54,12 @@ export interface IAppManagerDeps { logStorage: AppLogStorage; bridges: AppBridges; sourceStorage: AppSourceStorage; + /** + * Path to temporary file storage. + * + * Needs to be accessible for reading and writing. + */ + tempFilePath: string; } interface IPurgeAppConfigOpts { @@ -106,9 +112,11 @@ export class AppManager { private readonly runtime: AppRuntimeManager; + private readonly tempFilePath: string; + private isLoaded: boolean; - constructor({ metadataStorage, logStorage, bridges, sourceStorage }: IAppManagerDeps) { + constructor({ metadataStorage, logStorage, bridges, sourceStorage, tempFilePath }: IAppManagerDeps) { // Singleton style. There can only ever be one AppManager instance if (typeof AppManager.Instance !== 'undefined') { throw new Error('There is already a valid AppManager instance'); @@ -138,6 +146,8 @@ export class AppManager { throw new Error('Invalid instance of the AppSourceStorage'); } + this.tempFilePath = tempFilePath; + this.apps = new Map(); this.parser = new AppPackageParser(); @@ -160,6 +170,15 @@ export class AppManager { AppManager.Instance = this; } + /** + * Gets the path to the temporary file storage. + * + * Mainly used for upload events + */ + public getTempFilePath(): string { + return this.tempFilePath; + } + /** Gets the instance of the storage connector. */ public getStorage(): AppMetadataStorage { return this.appMetadataStorage; diff --git a/packages/apps-engine/src/server/ProxiedApp.ts b/packages/apps-engine/src/server/ProxiedApp.ts index fb49615b736dc..d4eb00bbb6666 100644 --- a/packages/apps-engine/src/server/ProxiedApp.ts +++ b/packages/apps-engine/src/server/ProxiedApp.ts @@ -1,3 +1,5 @@ +import { inspect } from 'util'; + import * as mem from 'mem'; import type { AppManager } from './AppManager'; @@ -78,7 +80,7 @@ export class ProxiedApp { // Range of JSON-RPC error codes: https://www.jsonrpc.org/specification#error_object if (e.code >= -32999 || e.code <= -32000) { // we really need to receive a logger from rocket.chat - console.error('JSON-RPC error received: ', e); + console.error('JSON-RPC error received: ', inspect(e, { depth: 10 })); } } } diff --git a/packages/apps-engine/src/server/compiler/AppImplements.ts b/packages/apps-engine/src/server/compiler/AppImplements.ts index eafbb2d2dd411..ba9be27b678bc 100644 --- a/packages/apps-engine/src/server/compiler/AppImplements.ts +++ b/packages/apps-engine/src/server/compiler/AppImplements.ts @@ -2,26 +2,31 @@ import { AppInterface } from '../../definition/metadata/AppInterface'; import { Utilities } from '../misc/Utilities'; export class AppImplements { - private implemented: { [key: string]: boolean }; + private implemented: Record; constructor() { - this.implemented = {}; - Object.keys(AppInterface).forEach((int) => { + this.implemented = {} as Record; + + Object.keys(AppInterface).forEach((int: AppInterface) => { this.implemented[int] = false; }); } - public doesImplement(int: string): void { + public setImplements(int: AppInterface): void { if (int in AppInterface) { this.implemented[int] = true; } } - public getValues(): { [int: string]: boolean } { + public doesImplement(int: AppInterface): boolean { + return this.implemented[int]; + } + + public getValues(): Record { return Utilities.deepCloneAndFreeze(this.implemented); } - public toJSON(): { [int: string]: boolean } { + public toJSON(): Record { return this.getValues(); } } diff --git a/packages/apps-engine/src/server/compiler/AppPackageParser.ts b/packages/apps-engine/src/server/compiler/AppPackageParser.ts index 073d3dbffd646..757fe60878bed 100644 --- a/packages/apps-engine/src/server/compiler/AppPackageParser.ts +++ b/packages/apps-engine/src/server/compiler/AppPackageParser.ts @@ -85,7 +85,7 @@ export class AppPackageParser { const implemented = new AppImplements(); if (Array.isArray(info.implements)) { - info.implements.forEach((interfaceName) => implemented.doesImplement(interfaceName)); + info.implements.forEach((interfaceName) => implemented.setImplements(interfaceName)); } return { diff --git a/packages/apps-engine/src/server/managers/AppListenerManager.ts b/packages/apps-engine/src/server/managers/AppListenerManager.ts index e2ce55691b88b..327bc949ba0ac 100644 --- a/packages/apps-engine/src/server/managers/AppListenerManager.ts +++ b/packages/apps-engine/src/server/managers/AppListenerManager.ts @@ -25,7 +25,7 @@ import type { IUIKitIncomingInteractionModalContainer, } from '../../definition/uikit/UIKitIncomingInteractionContainer'; import type { IUIKitLivechatBlockIncomingInteraction, IUIKitLivechatIncomingInteraction } from '../../definition/uikit/livechat'; -import type { IFileUploadContext } from '../../definition/uploads/IFileUploadContext'; +import type { IFileUploadInternalContext } from '../../definition/uploads/IFileUploadContext'; import type { IUser, IUserContext, IUserStatusContext, IUserUpdateContext } from '../../definition/users'; import type { AppManager } from '../AppManager'; import type { ProxiedApp } from '../ProxiedApp'; @@ -205,7 +205,7 @@ export interface IListenerExecutor { }; // FileUpload [AppInterface.IPreFileUpload]: { - args: [IFileUploadContext]; + args: [IFileUploadInternalContext]; result: void; }; // Email @@ -448,7 +448,7 @@ export class AppListenerManager { return this.executePostLivechatGuestSaved(data as IVisitor); // FileUpload case AppInterface.IPreFileUpload: - return this.executePreFileUpload(data as IFileUploadContext); + return this.executePreFileUpload(data as IFileUploadInternalContext); // Email case AppInterface.IPreEmailSent: return this.executePreEmailSent(data as IPreEmailSentContext); @@ -1170,7 +1170,7 @@ export class AppListenerManager { } // FileUpload - private async executePreFileUpload(data: IFileUploadContext): Promise { + private async executePreFileUpload(data: IFileUploadInternalContext): Promise { for (const appId of this.listeners.get(AppInterface.IPreFileUpload)) { const app = this.manager.getOneById(appId); diff --git a/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts b/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts index 5b62d31051557..8b4eb385a88f8 100644 --- a/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts +++ b/packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts @@ -11,7 +11,7 @@ import { ProcessMessenger } from './ProcessMessenger'; import { bundleLegacyApp } from './bundler'; import { newDecoder } from './codec'; import { AppStatus, AppStatusUtils } from '../../../definition/AppStatus'; -import type { AppMethod } from '../../../definition/metadata'; +import { AppInterface, AppMethod } from '../../../definition/metadata'; import type { AppManager } from '../../AppManager'; import type { AppBridges } from '../../bridges'; import type { IParseAppPackageResult } from '../../compiler'; @@ -115,6 +115,8 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu private readonly livenessManager: LivenessManager; + private readonly tempFilePath: string; + // We need to keep the appSource around in case the Deno process needs to be restarted constructor( manager: AppManager, @@ -137,6 +139,7 @@ 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 { @@ -151,10 +154,17 @@ export class DenoRuntimeSubprocessController extends EventEmitter implements IRu // 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 options = [ 'run', '--cached-only', - `--allow-read=${appsEngineDir},${parentNodeModulesDir}`, + `--allow-read=${allowedDirs.join(',')}`, `--allow-env=${ALLOWED_ENVIRONMENT_VARIABLES.join(',')}`, denoWrapperPath, '--subprocess', diff --git a/packages/apps-engine/tests/server/compiler/AppImplements.spec.ts b/packages/apps-engine/tests/server/compiler/AppImplements.spec.ts index a848fbd7a0769..e3e6240ffba93 100644 --- a/packages/apps-engine/tests/server/compiler/AppImplements.spec.ts +++ b/packages/apps-engine/tests/server/compiler/AppImplements.spec.ts @@ -11,9 +11,10 @@ export class AppImplementsTestFixture { const impls = new AppImplements(); Expect(impls.getValues()).toBeDefined(); - Expect(() => impls.doesImplement(AppInterface.IPreMessageSentPrevent)).not.toThrow(); + Expect(() => impls.setImplements(AppInterface.IPreMessageSentPrevent)).not.toThrow(); + Expect(impls.doesImplement(AppInterface.IPreMessageSentPrevent)).toBe(true); + Expect(impls.doesImplement(AppInterface.IPostMessageDeleted)).toBe(false); Expect(impls.getValues()[AppInterface.IPreMessageSentPrevent]).toBe(true); - Expect(() => impls.doesImplement('Something')).not.toThrow(); - Expect(impls.getValues().Something).not.toBeDefined(); + Expect(impls.getValues()[AppInterface.IPostMessageDeleted]).toBe(false); } } diff --git a/packages/apps-engine/tests/test-data/utilities.ts b/packages/apps-engine/tests/test-data/utilities.ts index 9cd3135ed061c..7d976d08113e2 100644 --- a/packages/apps-engine/tests/test-data/utilities.ts +++ b/packages/apps-engine/tests/test-data/utilities.ts @@ -130,6 +130,7 @@ export class TestInfastructureSetup { getRuntime: () => { return this.runtimeManager; }, + getTempFilePath: () => 'temp-file-path', } as unknown as AppManager; }