From b4fa945be472f07f211ae5d0e3005ea7ca22c446 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 20 Dec 2023 17:16:22 -0300 Subject: [PATCH 1/5] Move handleInitializeApp to handlers/app/ folder --- deno-runtime/handlers/app/construct.ts | 96 +++++++++++++++++++++++ deno-runtime/handlers/app/handler.ts | 28 +++++++ deno-runtime/main.ts | 102 +++---------------------- 3 files changed, 133 insertions(+), 93 deletions(-) create mode 100644 deno-runtime/handlers/app/construct.ts create mode 100644 deno-runtime/handlers/app/handler.ts diff --git a/deno-runtime/handlers/app/construct.ts b/deno-runtime/handlers/app/construct.ts new file mode 100644 index 000000000..2158da4d2 --- /dev/null +++ b/deno-runtime/handlers/app/construct.ts @@ -0,0 +1,96 @@ +import { IParseAppPackageResult } from '@rocket.chat/apps-engine/server/compiler/IParseAppPackageResult.ts'; + +import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; +import { require } from '../../lib/require.ts'; +import { sanitizeDeprecatedUsage } from '../../lib/sanitizeDeprecatedUsage.ts'; +import { AppAccessorsInstance } from '../../lib/accessors/mod.ts'; + +const ALLOWED_NATIVE_MODULES = ['path', 'url', 'crypto', 'buffer', 'stream', 'net', 'http', 'https', 'zlib', 'util', 'punycode', 'os', 'querystring']; +const ALLOWED_EXTERNAL_MODULES = ['uuid']; + +function buildRequire(): (module: string) => unknown { + return (module: string): unknown => { + if (ALLOWED_NATIVE_MODULES.includes(module)) { + return require(`node:${module}`); + } + + if (ALLOWED_EXTERNAL_MODULES.includes(module)) { + return require(`npm:${module}`); + } + + if (module.startsWith('@rocket.chat/apps-engine')) { + const path = module.replace('@rocket.chat/apps-engine/', new URL('..', import.meta.url).pathname).concat('.js'); + return require(path); + } + + throw new Error(`Module ${module} is not allowed`); + }; +} + +function wrapAppCode(code: string): (require: (module: string) => unknown) => Promise> { + return new Function( + 'require', + ` + const exports = {}; + const module = { exports }; + const result = (async (exports,module,require,globalThis,Deno) => { + ${code}; + })(exports,module,require); + return result.then(() => module.exports);`, + ) as (require: (module: string) => unknown) => Promise>; +} + +export async function handlInitializeApp(params: unknown): Promise { + if (!Array.isArray(params)) { + throw new Error('Invalid params', { cause: 'invalid_param_type' }); + } + + const [appPackage] = params as [IParseAppPackageResult]; + + if (!appPackage?.info?.id || !appPackage?.info?.classFile || !appPackage?.files) { + throw new Error('Invalid params', { cause: 'invalid_param_type'}); + } + + AppObjectRegistry.set('id', appPackage.info.id); + const source = sanitizeDeprecatedUsage(appPackage.files[appPackage.info.classFile]); + + const require = buildRequire(); + const exports = await wrapAppCode(source)(require); + + // This is the same naive logic we've been using in the App Compiler + // Applying the correct type here is quite difficult because of the dynamic nature of the code + // deno-lint-ignore no-explicit-any + const appClass = Object.values(exports)[0] as any; + const logger = AppObjectRegistry.get('logger'); + + const app = new appClass(appPackage.info, logger, AppAccessorsInstance.getDefaultAppAccessors()); + + if (typeof app.getName !== 'function') { + throw new Error('App must contain a getName function'); + } + + if (typeof app.getNameSlug !== 'function') { + throw new Error('App must contain a getNameSlug function'); + } + + if (typeof app.getVersion !== 'function') { + throw new Error('App must contain a getVersion function'); + } + + if (typeof app.getID !== 'function') { + throw new Error('App must contain a getID function'); + } + + if (typeof app.getDescription !== 'function') { + throw new Error('App must contain a getDescription function'); + } + + if (typeof app.getRequiredApiVersion !== 'function') { + throw new Error('App must contain a getRequiredApiVersion function'); + } + + AppObjectRegistry.set('app', app); + + return true; +} + diff --git a/deno-runtime/handlers/app/handler.ts b/deno-runtime/handlers/app/handler.ts new file mode 100644 index 000000000..465ffc87f --- /dev/null +++ b/deno-runtime/handlers/app/handler.ts @@ -0,0 +1,28 @@ +import { Defined, JsonRpcError } from "jsonrpc-lite"; +import { handlInitializeApp } from "./construct.ts"; + +export default async function handleApp(method: string, params: unknown): Promise { + const [, appMethod] = method.split(':'); + + let result: Defined; + + try { + if (appMethod === 'construct') { + result = await handlInitializeApp(params); + } else { + result = null; + } + } catch (e: unknown) { + if (!(e instanceof Error)) { + return new JsonRpcError('Unknown error', -32000, e); + } + + if ((e.cause as string)?.includes('invalid_param_type')) { + return JsonRpcError.invalidParams(null); + } + + return new JsonRpcError(e.message, -32000, e); + } + + return result; +} diff --git a/deno-runtime/main.ts b/deno-runtime/main.ts index 10eb3faab..3ac4fb448 100644 --- a/deno-runtime/main.ts +++ b/deno-runtime/main.ts @@ -8,93 +8,14 @@ if (!Deno.args.includes('--subprocess')) { Deno.exit(1001); } -import { sanitizeDeprecatedUsage } from './lib/sanitizeDeprecatedUsage.ts'; -import { AppAccessorsInstance } from './lib/accessors/mod.ts'; +import { JsonRpcError } from 'jsonrpc-lite'; + import * as Messenger from './lib/messenger.ts'; import { AppObjectRegistry } from './AppObjectRegistry.ts'; import { Logger } from './lib/logger.ts'; -import { require } from './lib/require.ts'; -import type { IParseAppPackageResult } from '@rocket.chat/apps-engine/server/compiler/IParseAppPackageResult.ts'; import slashcommandHandler from './handlers/slashcommand-handler.ts'; -import { JsonRpcError } from "jsonrpc-lite"; - -const ALLOWED_NATIVE_MODULES = ['path', 'url', 'crypto', 'buffer', 'stream', 'net', 'http', 'https', 'zlib', 'util', 'punycode', 'os', 'querystring']; -const ALLOWED_EXTERNAL_MODULES = ['uuid']; - -function buildRequire(): (module: string) => unknown { - return (module: string): unknown => { - if (ALLOWED_NATIVE_MODULES.includes(module)) { - return require(`node:${module}`); - } - - if (ALLOWED_EXTERNAL_MODULES.includes(module)) { - return require(`npm:${module}`); - } - - if (module.startsWith('@rocket.chat/apps-engine')) { - const path = module.replace('@rocket.chat/apps-engine/', new URL('..', import.meta.url).pathname).concat('.js'); - return require(path); - } - - throw new Error(`Module ${module} is not allowed`); - }; -} - -function wrapAppCode(code: string): (require: (module: string) => unknown) => Promise> { - return new Function( - 'require', - ` - const exports = {}; - const module = { exports }; - const result = (async (exports,module,require,globalThis,Deno) => { - ${code}; - })(exports,module,require); - return result.then(() => module.exports);`, - ) as (require: (module: string) => unknown) => Promise>; -} - -async function handlInitializeApp(appPackage: IParseAppPackageResult): Promise { - AppObjectRegistry.set('id', appPackage.info.id); - const source = sanitizeDeprecatedUsage(appPackage.files[appPackage.info.classFile]); - - const require = buildRequire(); - const exports = await wrapAppCode(source)(require); - - // This is the same naive logic we've been using in the App Compiler - // Applying the correct type here is quite difficult because of the dynamic nature of the code - // deno-lint-ignore no-explicit-any - const appClass = Object.values(exports)[0] as any; - const logger = AppObjectRegistry.get('logger'); - - const app = new appClass(appPackage.info, logger, AppAccessorsInstance.getDefaultAppAccessors()); - - if (typeof app.getName !== 'function') { - throw new Error('App must contain a getName function'); - } - - if (typeof app.getNameSlug !== 'function') { - throw new Error('App must contain a getNameSlug function'); - } - - if (typeof app.getVersion !== 'function') { - throw new Error('App must contain a getVersion function'); - } - - if (typeof app.getID !== 'function') { - throw new Error('App must contain a getID function'); - } - - if (typeof app.getDescription !== 'function') { - throw new Error('App must contain a getDescription function'); - } - - if (typeof app.getRequiredApiVersion !== 'function') { - throw new Error('App must contain a getRequiredApiVersion function'); - } - - AppObjectRegistry.set('app', app); -} +import handleApp from './handlers/app/handler.ts'; async function handleRequest({ type, payload }: Messenger.JsonRpcRequest): Promise { // We're not handling notifications at the moment @@ -108,22 +29,17 @@ async function handleRequest({ type, payload }: Messenger.JsonRpcRequest): Promi AppObjectRegistry.set('logger', logger); switch (true) { - case method.includes('app:construct'): { - const [appPackage] = params as [IParseAppPackageResult]; + case method.startsWith('app:'): { + const result = await handleApp(method, params); - if (!appPackage?.info?.id || !appPackage?.info?.classFile || !appPackage?.files) { - return Messenger.sendInvalidParamsError(id); + if (result instanceof JsonRpcError) { + return Messenger.errorResponse({ id, error: result }); } - await handlInitializeApp(appPackage); - - Messenger.successResponse({ - id, - result: 'logs should go here as a response', - }); + Messenger.successResponse({ id, result }); break; } - case method.includes('slashcommand:'): { + case method.startsWith('slashcommand:'): { const result = await slashcommandHandler(method, params); if (result instanceof JsonRpcError) { From e9e8a0356f0b3e72b29c3b977aa88393e7e4e93a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 13:56:17 -0300 Subject: [PATCH 2/5] Fix Deno require and subprocess controller flow --- deno-runtime/handlers/app/construct.ts | 2 +- src/server/runtime/AppsEngineDenoRuntime.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deno-runtime/handlers/app/construct.ts b/deno-runtime/handlers/app/construct.ts index 2158da4d2..d87efc682 100644 --- a/deno-runtime/handlers/app/construct.ts +++ b/deno-runtime/handlers/app/construct.ts @@ -19,7 +19,7 @@ function buildRequire(): (module: string) => unknown { } if (module.startsWith('@rocket.chat/apps-engine')) { - const path = module.replace('@rocket.chat/apps-engine/', new URL('..', import.meta.url).pathname).concat('.js'); + const path = module.concat('.js'); return require(path); } diff --git a/src/server/runtime/AppsEngineDenoRuntime.ts b/src/server/runtime/AppsEngineDenoRuntime.ts index 5cbef43bf..c88651979 100644 --- a/src/server/runtime/AppsEngineDenoRuntime.ts +++ b/src/server/runtime/AppsEngineDenoRuntime.ts @@ -120,7 +120,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter { public async setupApp() { await this.waitUntilReady(); - this.sendRequest({ method: 'app:construct', params: [this.appPackage] }); + await this.sendRequest({ method: 'app:construct', params: [this.appPackage] }); } public async stopApp() { @@ -155,7 +155,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter { private waitForResponse(id: string): Promise { return new Promise((resolve, reject) => { - const timeoutId = setTimeout(() => reject(new Error('Request timed out')), this.options.timeout); + const timeoutId = setTimeout(() => reject(new Error(`Request "${id}" timed out`)), this.options.timeout); this.once(`result:${id}`, (result: unknown, error: jsonrpc.IParsedObjectError['payload']['error']) => { clearTimeout(timeoutId); @@ -169,7 +169,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter { }); } - private async onReady(): Promise { + private onReady(): void { this.state = 'ready'; } From f800c096f42f76db22c7606944ebb49a99f6e7a0 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 14:11:40 -0300 Subject: [PATCH 3/5] Refactor app method call initialize --- deno-runtime/handlers/app/initialize.ts | 18 ++++++++++++++++++ src/server/AppManager.ts | 4 +--- src/server/ProxiedApp.ts | 16 +--------------- 3 files changed, 20 insertions(+), 18 deletions(-) create mode 100644 deno-runtime/handlers/app/initialize.ts diff --git a/deno-runtime/handlers/app/initialize.ts b/deno-runtime/handlers/app/initialize.ts new file mode 100644 index 000000000..d592d2970 --- /dev/null +++ b/deno-runtime/handlers/app/initialize.ts @@ -0,0 +1,18 @@ +import type { App } from '@rocket.chat/apps-engine/definition/App.ts'; + +import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; +import { AppAccessorsInstance } from '../../lib/accessors/mod.ts'; + +export default async function handleInitialize(): Promise { + const app = AppObjectRegistry.get('app'); + + if (typeof app?.initialize !== 'function') { + throw new Error('App must contain an initialize function', { + cause: 'invalid_app', + }); + } + + await app.initialize(AppAccessorsInstance.getConfigurationExtend(), AppAccessorsInstance.getEnvironmentRead()); + + return true; +} diff --git a/src/server/AppManager.ts b/src/server/AppManager.ts index 6c6183b4e..96471290b 100644 --- a/src/server/AppManager.ts +++ b/src/server/AppManager.ts @@ -914,14 +914,12 @@ export class AppManager { private async initializeApp(storageItem: IAppStorageItem, app: ProxiedApp, saveToDb = true, silenceStatus = false): Promise { let result: boolean; - const configExtend = this.getAccessorManager().getConfigurationExtend(storageItem.id); - const envRead = this.getAccessorManager().getEnvironmentRead(storageItem.id); try { await app.validateLicense(); await app.validateInstallation(); - await app.call(AppMethod.INITIALIZE, configExtend, envRead); + await app.call(AppMethod.INITIALIZE); await app.setStatus(AppStatus.INITIALIZED, silenceStatus); result = true; diff --git a/src/server/ProxiedApp.ts b/src/server/ProxiedApp.ts index 69c84a50e..ee28e73d7 100644 --- a/src/server/ProxiedApp.ts +++ b/src/server/ProxiedApp.ts @@ -55,21 +55,7 @@ export class ProxiedApp implements IApp { } public async call(method: `${AppMethod}`, ...args: Array): Promise { - const logger = this.setupLogger(method); - - try { - const result = await this.appRuntime.sendRequest({ method, params: args }); - - logger.debug('Result:', result); - - return result; - } catch (e) { - logger.error('Error:', e); - - throw e; - } finally { - await this.manager.getLogStorage().storeEntries(AppConsole.toStorageEntry(this.getID(), logger)); - } + return this.appRuntime.sendRequest({ method: `app:${method }`, params: args }); } public getStatus(): AppStatus { From 8c3e16584c2045efe4841160961e1f521441e88d Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 14:15:32 -0300 Subject: [PATCH 4/5] Add app method initialize handler on Deno --- deno-runtime/handlers/app/construct.ts | 2 +- deno-runtime/handlers/app/handler.ts | 24 ++++++++++++++---------- deno-runtime/main.ts | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/deno-runtime/handlers/app/construct.ts b/deno-runtime/handlers/app/construct.ts index d87efc682..85e1ce482 100644 --- a/deno-runtime/handlers/app/construct.ts +++ b/deno-runtime/handlers/app/construct.ts @@ -40,7 +40,7 @@ function wrapAppCode(code: string): (require: (module: string) => unknown) => Pr ) as (require: (module: string) => unknown) => Promise>; } -export async function handlInitializeApp(params: unknown): Promise { +export default async function handleConstructApp(params: unknown): Promise { if (!Array.isArray(params)) { throw new Error('Invalid params', { cause: 'invalid_param_type' }); } diff --git a/deno-runtime/handlers/app/handler.ts b/deno-runtime/handlers/app/handler.ts index 465ffc87f..a8f7d9bf6 100644 --- a/deno-runtime/handlers/app/handler.ts +++ b/deno-runtime/handlers/app/handler.ts @@ -1,16 +1,18 @@ -import { Defined, JsonRpcError } from "jsonrpc-lite"; -import { handlInitializeApp } from "./construct.ts"; +import { Defined, JsonRpcError } from 'jsonrpc-lite'; +import handleConstructApp from './construct.ts'; +import handleInitialize from './initialize.ts'; export default async function handleApp(method: string, params: unknown): Promise { const [, appMethod] = method.split(':'); - let result: Defined; - try { - if (appMethod === 'construct') { - result = await handlInitializeApp(params); - } else { - result = null; + switch (appMethod) { + case 'construct': + return await handleConstructApp(params); + case 'initialize': + return await handleInitialize(); + default: + throw new JsonRpcError('Method not found', -32601); } } catch (e: unknown) { if (!(e instanceof Error)) { @@ -21,8 +23,10 @@ export default async function handleApp(method: string, params: unknown): Promis return JsonRpcError.invalidParams(null); } + if ((e.cause as string)?.includes('invalid_app')) { + return JsonRpcError.internalError({ message: 'App unavailable' }); + } + return new JsonRpcError(e.message, -32000, e); } - - return result; } diff --git a/deno-runtime/main.ts b/deno-runtime/main.ts index 3ac4fb448..3a3d63221 100644 --- a/deno-runtime/main.ts +++ b/deno-runtime/main.ts @@ -17,7 +17,7 @@ import { Logger } from './lib/logger.ts'; import slashcommandHandler from './handlers/slashcommand-handler.ts'; import handleApp from './handlers/app/handler.ts'; -async function handleRequest({ type, payload }: Messenger.JsonRpcRequest): Promise { +async function requestRouter({ type, payload }: Messenger.JsonRpcRequest): Promise { // We're not handling notifications at the moment if (type === 'notification') { return Messenger.sendInvalidRequestError(); @@ -97,7 +97,7 @@ async function main() { } if (Messenger.isRequest(JSONRPCMessage)) { - await handleRequest(JSONRPCMessage); + await requestRouter(JSONRPCMessage); } if (Messenger.isResponse(JSONRPCMessage)) { From c29d10c7500dcca7c2dd990b630a04d7a2605c98 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 17:58:01 -0300 Subject: [PATCH 5/5] Add message separator between Deno and Node This is necessary because Node's read stream of Deno's stdout seems to be buffering data and the handler ended up getting a string with two jsonrpc messages in it --- deno-runtime/lib/messenger.ts | 2 +- deno-runtime/main.ts | 2 + src/server/runtime/AppsEngineDenoRuntime.ts | 60 +++++++++++++-------- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/deno-runtime/lib/messenger.ts b/deno-runtime/lib/messenger.ts index eb2d8bd6e..88409abf9 100644 --- a/deno-runtime/lib/messenger.ts +++ b/deno-runtime/lib/messenger.ts @@ -37,7 +37,7 @@ export const Transport = new class Transporter { } private async stdoutTransport(message: jsonrpc.JsonRpc): Promise { - const encoded = encoder.encode(message.serialize()); + const encoded = encoder.encode(message.serialize() + AppObjectRegistry.get('MESSAGE_SEPARATOR')); await Deno.stdout.write(encoded); } diff --git a/deno-runtime/main.ts b/deno-runtime/main.ts index 3a3d63221..342246322 100644 --- a/deno-runtime/main.ts +++ b/deno-runtime/main.ts @@ -17,6 +17,8 @@ import { Logger } from './lib/logger.ts'; import slashcommandHandler from './handlers/slashcommand-handler.ts'; import handleApp from './handlers/app/handler.ts'; +AppObjectRegistry.set('MESSAGE_SEPARATOR', Deno.args.at(-1)); + async function requestRouter({ type, payload }: Messenger.JsonRpcRequest): Promise { // We're not handling notifications at the moment if (type === 'notification') { diff --git a/src/server/runtime/AppsEngineDenoRuntime.ts b/src/server/runtime/AppsEngineDenoRuntime.ts index c88651979..d20d98b0d 100644 --- a/src/server/runtime/AppsEngineDenoRuntime.ts +++ b/src/server/runtime/AppsEngineDenoRuntime.ts @@ -39,6 +39,8 @@ export function isValidOrigin(accessor: string): accessor is typeof ALLOWED_ACCE return ALLOWED_ACCESSOR_METHODS.includes(accessor as any); } +const MESSAGE_SEPARATOR = 'OkFQUF9TRVA6'; + /** * Resolves the absolute path of the Deno executable * installed by deno-bin. @@ -86,7 +88,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter { const denoWrapperPath = getDenoWrapperPath(); const denoWrapperDir = path.dirname(path.join(denoWrapperPath, '..')); - this.deno = child_process.spawn(denoExePath, ['run', `--allow-read=${denoWrapperDir}/`, denoWrapperPath, '--subprocess', appPackage.info.id]); + this.deno = child_process.spawn(denoExePath, ['run', `--allow-read=${denoWrapperDir}/`, denoWrapperPath, '--subprocess', MESSAGE_SEPARATOR]); this.setupListeners(); } catch { @@ -99,6 +101,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter { this.bridges = manager.getBridges(); } + // Debug purposes, could be deleted later emit(eventName: string | symbol, ...args: any[]): boolean { const hadListeners = super.emit(eventName, ...args); @@ -132,7 +135,9 @@ export class DenoRuntimeSubprocessController extends EventEmitter { public async sendRequest(message: Pick): Promise { const id = String(Math.random()).substring(2); - this.deno.stdin.write(jsonrpc.request(id, message.method, message.params).serialize()); + const request = jsonrpc.request(id, message.method, message.params); + + this.deno.stdin.write(request.serialize()); return this.waitForResponse(id); } @@ -339,35 +344,48 @@ export class DenoRuntimeSubprocessController extends EventEmitter { logs = message.payload.error.data?.logs as ILoggerStorageEntry; } - this.logStorage.storeEntries(logs); + await this.logStorage.storeEntries(logs); this.emit(`result:${id}`, result, error); } private async parseOutput(chunk: Buffer): Promise { - let message; + // Chunk can be multiple JSONRpc messages as the stdout read stream can buffer multiple messages + const messages = chunk.toString().split(MESSAGE_SEPARATOR); - try { - message = jsonrpc.parse(chunk.toString()); + if (messages.length < 2) { + console.error('Invalid message format', messages); + return; + } - if (Array.isArray(message)) { - throw new Error('Invalid message format'); - } + messages.forEach(async (m) => { + if (!m.length) return; - if (message.type === 'request' || message.type === 'notification') { - return this.handleIncomingMessage(message); - } + try { + const message = jsonrpc.parse(m); - if (message.type === 'success' || message.type === 'error') { - return this.handleResultMessage(message); - } + if (Array.isArray(message)) { + throw new Error('Invalid message format'); + } - throw new Error(); - } catch { - console.error('Invalid message format. What to do?', chunk.toString()); - } finally { - console.log({ message }); - } + if (message.type === 'request' || message.type === 'notification') { + return await this.handleIncomingMessage(message); + } + + if (message.type === 'success' || message.type === 'error') { + return await this.handleResultMessage(message); + } + + console.error('Unrecognized message type', message); + } catch (e) { + // SyntaxError is thrown when the message is not a valid JSON + if (e instanceof SyntaxError) { + return console.error('Failed to parse message', m); + } + + console.error('Error executing handler', e); + } + }); } private async parseError(chunk: Buffer): Promise {