From b4fa945be472f07f211ae5d0e3005ea7ca22c446 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 20 Dec 2023 17:16:22 -0300 Subject: [PATCH 1/3] 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 3c2a0c08a749328117d3d31d72150f6a3c3f8a5e Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 13:56:17 -0300 Subject: [PATCH 2/3] 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 92d7d93a78366b916c3b690df7021883c0f9092a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 21 Dec 2023 17:58:01 -0300 Subject: [PATCH 3/3] 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 3ac4fb448..9fc469549 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 handleRequest({ 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 {