mirror of
https://github.com/matrix-org/matrix-hookshot.git
synced 2025-03-10 13:17:08 +00:00
Replace vm2 with quickjs (#817)
* quickjs test * Replace vm2 with quickjs * initalise -> initialise * Remove unused transformation timeout time * Don't assume quickModule is set Also use whether it's set as the indicator of whether transformation functions are allowed, instead of checking the config * Refactor GenericHookConnectionState validation - Do it in the constructor instead of in callers - Make hookId mandatory so as to not drop it on some state updates - Conflate a state event's state key with a connection state's name, which was already the case in practice * Refactor validateState * Drop explicit any Better to infer the type instead * Always validate transformation fn * Fix test * Add changelog * Fix disposal, validation, and printing * Fix transformation error string formatting Also refactor similar code * Let invalid transformations run & fail instead of pretending that one was never set * Restore transformation timeout time * Don't execute transformation fn when validating it Instead, only compile it * Revert unrelated changes --------- Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>
This commit is contained in:
Will Hunt
GitHub
parent
d744974493
commit
dc126afa6a
1
changelog.d/817.misc
Normal file
1
changelog.d/817.misc
Normal file
@ -0,0 +1 @@
|
||||
Use quickjs instead of vm2 for evaluating JS transformation functions.
|
||||
@ -65,11 +65,11 @@
|
||||
"nyc": "^15.1.0",
|
||||
"p-queue": "^6.6.2",
|
||||
"prom-client": "^14.2.0",
|
||||
"quickjs-emscripten": "^0.23.0",
|
||||
"reflect-metadata": "^0.1.13",
|
||||
"source-map-support": "^0.5.21",
|
||||
"string-argv": "^0.3.1",
|
||||
"tiny-typed-emitter": "^2.1.0",
|
||||
"vm2": "^3.9.18",
|
||||
"winston": "^3.3.3",
|
||||
"xml2js": "^0.5.0",
|
||||
"yaml": "^2.2.2"
|
||||
|
||||
@ -10,6 +10,7 @@ import { LogService } from "matrix-bot-sdk";
|
||||
import { getAppservice } from "../appservice";
|
||||
import BotUsersManager from "../Managers/BotUsersManager";
|
||||
import * as Sentry from '@sentry/node';
|
||||
import { GenericHookConnection } from "../Connections";
|
||||
|
||||
Logger.configure({console: "info"});
|
||||
const log = new Logger("App");
|
||||
@ -49,6 +50,10 @@ async function start() {
|
||||
log.info("Sentry reporting enabled");
|
||||
}
|
||||
|
||||
if (config.generic?.allowJsTransformationFunctions) {
|
||||
await GenericHookConnection.initialiseQuickJS();
|
||||
}
|
||||
|
||||
const botUsersManager = new BotUsersManager(config, appservice);
|
||||
|
||||
const bridgeApp = new Bridge(config, listener, appservice, storage, botUsersManager);
|
||||
|
||||
@ -2,7 +2,7 @@ import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, P
|
||||
import { Logger } from "matrix-appservice-bridge";
|
||||
import { MessageSenderClient } from "../MatrixSender"
|
||||
import markdownit from "markdown-it";
|
||||
import { VMScript as Script, NodeVM } from "vm2";
|
||||
import { QuickJSRuntime, QuickJSWASMModule, newQuickJSWASMModule, shouldInterruptAfterDeadline } from "quickjs-emscripten";
|
||||
import { MatrixEvent } from "../MatrixEvent";
|
||||
import { Appservice, Intent, StateEvent } from "matrix-bot-sdk";
|
||||
import { ApiError, ErrCode } from "../api";
|
||||
@ -65,6 +65,11 @@ const SANITIZE_MAX_BREADTH = 50;
|
||||
*/
|
||||
@Connection
|
||||
export class GenericHookConnection extends BaseConnection implements IConnection {
|
||||
private static quickModule?: QuickJSWASMModule;
|
||||
|
||||
public static async initialiseQuickJS() {
|
||||
GenericHookConnection.quickModule = await newQuickJSWASMModule();
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures a JSON payload is compatible with Matrix JSON requirements, such
|
||||
@ -107,27 +112,26 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
return obj;
|
||||
}
|
||||
|
||||
static validateState(state: Record<string, unknown>, allowJsTransformationFunctions?: boolean): GenericHookConnectionState {
|
||||
static validateState(state: Record<string, unknown>): GenericHookConnectionState {
|
||||
const {name, transformationFunction} = state;
|
||||
let transformationFunctionResult: string|undefined;
|
||||
if (transformationFunction) {
|
||||
if (allowJsTransformationFunctions !== undefined && !allowJsTransformationFunctions) {
|
||||
throw new ApiError('Transformation functions are not allowed', ErrCode.DisabledFeature);
|
||||
}
|
||||
if (typeof transformationFunction !== "string") {
|
||||
throw new ApiError('Transformation functions must be a string', ErrCode.BadValue);
|
||||
}
|
||||
transformationFunctionResult = transformationFunction;
|
||||
}
|
||||
if (!name) {
|
||||
throw new ApiError('Missing name', ErrCode.BadValue);
|
||||
}
|
||||
if (typeof name !== "string" || name.length < 3 || name.length > 64) {
|
||||
throw new ApiError("'name' must be a string between 3-64 characters long", ErrCode.BadValue);
|
||||
}
|
||||
// Use !=, not !==, to check for both undefined and null
|
||||
if (transformationFunction != undefined) {
|
||||
if (!this.quickModule) {
|
||||
throw new ApiError('Transformation functions are not allowed', ErrCode.DisabledFeature);
|
||||
}
|
||||
if (typeof transformationFunction !== "string") {
|
||||
throw new ApiError('Transformation functions must be a string', ErrCode.BadValue);
|
||||
}
|
||||
}
|
||||
return {
|
||||
name,
|
||||
...(transformationFunctionResult && {transformationFunction: transformationFunctionResult}),
|
||||
...(transformationFunction && {transformationFunction}),
|
||||
};
|
||||
}
|
||||
|
||||
@ -163,7 +167,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
throw Error('Generic Webhooks are not configured');
|
||||
}
|
||||
const hookId = randomUUID();
|
||||
const validState = GenericHookConnection.validateState(data, config.generic.allowJsTransformationFunctions || false);
|
||||
const validState = GenericHookConnection.validateState(data);
|
||||
await GenericHookConnection.ensureRoomAccountData(roomId, intent, hookId, validState.name);
|
||||
await intent.underlyingClient.sendStateEvent(roomId, this.CanonicalEventType, validState.name, validState);
|
||||
const connection = new GenericHookConnection(roomId, validState, hookId, validState.name, messageClient, config.generic, as, intent);
|
||||
@ -197,8 +201,11 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
GenericHookConnection.LegacyCanonicalEventType,
|
||||
];
|
||||
|
||||
private transformationFunction?: Script;
|
||||
private transformationFunction?: string;
|
||||
private cachedDisplayname?: string;
|
||||
/**
|
||||
* @param state Should be a pre-validated state object returned by {@link validateState}
|
||||
*/
|
||||
constructor(
|
||||
roomId: string,
|
||||
private state: GenericHookConnectionState,
|
||||
@ -210,8 +217,8 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
private readonly intent: Intent,
|
||||
) {
|
||||
super(roomId, stateKey, GenericHookConnection.CanonicalEventType);
|
||||
if (state.transformationFunction && config.allowJsTransformationFunctions) {
|
||||
this.transformationFunction = new Script(state.transformationFunction);
|
||||
if (state.transformationFunction && GenericHookConnection.quickModule) {
|
||||
this.transformationFunction = state.transformationFunction;
|
||||
}
|
||||
}
|
||||
|
||||
@ -259,12 +266,26 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
}
|
||||
|
||||
public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
|
||||
const validatedConfig = GenericHookConnection.validateState(stateEv.content as Record<string, unknown>, this.config.allowJsTransformationFunctions || false);
|
||||
const validatedConfig = GenericHookConnection.validateState(stateEv.content as Record<string, unknown>);
|
||||
if (validatedConfig.transformationFunction) {
|
||||
try {
|
||||
this.transformationFunction = new Script(validatedConfig.transformationFunction);
|
||||
} catch (ex) {
|
||||
await this.messageClient.sendMatrixText(this.roomId, 'Could not compile transformation function:' + ex, "m.text", this.intent.userId);
|
||||
const ctx = GenericHookConnection.quickModule!.newContext();
|
||||
const codeEvalResult = ctx.evalCode(`function f(data) {${validatedConfig.transformationFunction}}`, undefined, { compileOnly: true });
|
||||
if (codeEvalResult.error) {
|
||||
const errorString = JSON.stringify(ctx.dump(codeEvalResult.error), null, 2);
|
||||
codeEvalResult.error.dispose();
|
||||
ctx.dispose();
|
||||
|
||||
const errorPrefix = "Could not compile transformation function:";
|
||||
await this.intent.sendEvent(this.roomId, {
|
||||
msgtype: "m.text",
|
||||
body: errorPrefix + "\n\n```json\n\n" + errorString + "\n\n```",
|
||||
formatted_body: `<p>${errorPrefix}</p><p><pre><code class=\\"language-json\\">${errorString}</code></pre></p>`,
|
||||
format: "org.matrix.custom.html",
|
||||
});
|
||||
} else {
|
||||
codeEvalResult.value.dispose();
|
||||
ctx.dispose();
|
||||
this.transformationFunction = validatedConfig.transformationFunction;
|
||||
}
|
||||
} else {
|
||||
this.transformationFunction = undefined;
|
||||
@ -281,8 +302,10 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
} else if (typeof safeData?.text === "string") {
|
||||
msg.plain = safeData.text;
|
||||
} else {
|
||||
msg.plain = "Received webhook data:\n\n" + "```json\n\n" + JSON.stringify(data, null, 2) + "\n\n```";
|
||||
msg.html = `<p>Received webhook data:</p><p><pre><code class=\\"language-json\\">${JSON.stringify(data, null, 2)}</code></pre></p>`
|
||||
const dataString = JSON.stringify(data, null, 2);
|
||||
const dataPrefix = "Received webhook data:";
|
||||
msg.plain = dataPrefix + "\n\n```json\n\n" + dataString + "\n\n```";
|
||||
msg.html = `<p>${dataPrefix}</p><p><pre><code class=\\"language-json\\">${dataString}</code></pre></p>`
|
||||
}
|
||||
|
||||
if (typeof safeData?.html === "string") {
|
||||
@ -304,17 +327,27 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
if (!this.transformationFunction) {
|
||||
throw Error('Transformation function not defined');
|
||||
}
|
||||
const vm = new NodeVM({
|
||||
console: 'off',
|
||||
wrapper: 'none',
|
||||
wasm: false,
|
||||
eval: false,
|
||||
timeout: TRANSFORMATION_TIMEOUT_MS,
|
||||
});
|
||||
vm.setGlobal('HookshotApiVersion', 'v2');
|
||||
vm.setGlobal('data', data);
|
||||
vm.run(this.transformationFunction);
|
||||
const result = vm.getGlobal('result');
|
||||
let result;
|
||||
const ctx = GenericHookConnection.quickModule!.newContext();
|
||||
ctx.runtime.setInterruptHandler(shouldInterruptAfterDeadline(Date.now() + TRANSFORMATION_TIMEOUT_MS));
|
||||
try {
|
||||
ctx.setProp(ctx.global, 'HookshotApiVersion', ctx.newString('v2'));
|
||||
const ctxResult = ctx.evalCode(`const data = ${JSON.stringify(data)};\n\n${this.state.transformationFunction}`);
|
||||
|
||||
if (ctxResult.error) {
|
||||
const e = Error(`Transformation failed to run: ${JSON.stringify(ctx.dump(ctxResult.error))}`);
|
||||
ctxResult.error.dispose();
|
||||
throw e;
|
||||
} else {
|
||||
const value = ctx.getProp(ctx.global, 'result');
|
||||
result = ctx.dump(value);
|
||||
value.dispose();
|
||||
ctxResult.value.dispose();
|
||||
}
|
||||
} finally {
|
||||
ctx.global.dispose();
|
||||
ctx.dispose();
|
||||
}
|
||||
|
||||
// Legacy v1 api
|
||||
if (typeof result === "string") {
|
||||
@ -437,7 +470,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection
|
||||
public async provisionerUpdateConfig(userId: string, config: Record<string, unknown>) {
|
||||
// Apply previous state to the current config, as provisioners might not return "unknown" keys.
|
||||
config = { ...this.state, ...config };
|
||||
const validatedConfig = GenericHookConnection.validateState(config, this.config.allowJsTransformationFunctions || false);
|
||||
const validatedConfig = GenericHookConnection.validateState(config);
|
||||
await this.intent.underlyingClient.sendStateEvent(this.roomId, GenericHookConnection.CanonicalEventType, this.stateKey,
|
||||
{
|
||||
...validatedConfig,
|
||||
|
||||
@ -56,6 +56,9 @@ function handleMessage(mq: LocalMQ): Promise<IMatrixSendMessage> {
|
||||
}
|
||||
|
||||
describe("GenericHookConnection", () => {
|
||||
before(async () => {
|
||||
await GenericHookConnection.initialiseQuickJS();
|
||||
})
|
||||
it("will handle simple hook events", async () => {
|
||||
const [connection, mq] = createGenericHook();
|
||||
await testSimpleWebhook(connection, mq, "data");
|
||||
|
||||
@ -20,7 +20,7 @@ const EXAMPLE_SCRIPT = `if (data.counter === undefined) {
|
||||
};
|
||||
} else {
|
||||
result = {
|
||||
plain: \`*Everything is fine*, the counter is under by\${data.maxValue - data.counter}\`,
|
||||
plain: \`*Everything is fine*, the counter is under by \${data.maxValue - data.counter}\`,
|
||||
version: "v2"
|
||||
};
|
||||
}`;
|
||||
|
||||
17
yarn.lock
17
yarn.lock
@ -1754,12 +1754,12 @@ acorn-jsx@^5.3.2:
|
||||
resolved "https://registry.yarnpkg.com/acorn-jsx/-/acorn-jsx-5.3.2.tgz#7ed5bb55908b3b2f1bc55c6af1653bada7f07937"
|
||||
integrity sha512-rq9s+JNhf0IChjtDXxllJ7g41oZk5SlXtp0LHwyA5cejwn7vKmKp4pPri6YEePv2PU65sAsegbXtIinmDFDXgQ==
|
||||
|
||||
acorn-walk@^8.1.1, acorn-walk@^8.2.0:
|
||||
acorn-walk@^8.1.1:
|
||||
version "8.2.0"
|
||||
resolved "https://registry.yarnpkg.com/acorn-walk/-/acorn-walk-8.2.0.tgz#741210f2e2426454508853a2f44d0ab83b7f69c1"
|
||||
integrity sha512-k+iyHEuPgSw6SbuDpGQM+06HQUa04DZ3o+F6CSzXMvvI5KMvnaEqXe+YVe555R9nn6GPt404fos4wcgpw12SDA==
|
||||
|
||||
acorn@^8.4.1, acorn@^8.7.0:
|
||||
acorn@^8.4.1:
|
||||
version "8.7.1"
|
||||
resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.7.1.tgz#0197122c843d1bf6d0a5e83220a788f278f63c30"
|
||||
integrity sha512-Xx54uLJQZ19lKygFXOWsscKUbsBZW0CPykPhVQdhIeIwrbPmJzqeASDInc8nKBnp/JT6igTs82qPXz069H8I/A==
|
||||
@ -5240,6 +5240,11 @@ queue-microtask@^1.2.2:
|
||||
resolved "https://registry.yarnpkg.com/queue-microtask/-/queue-microtask-1.2.3.tgz#4929228bbc724dfac43e0efb058caf7b6cfb6243"
|
||||
integrity sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A==
|
||||
|
||||
quickjs-emscripten@^0.23.0:
|
||||
version "0.23.0"
|
||||
resolved "https://registry.yarnpkg.com/quickjs-emscripten/-/quickjs-emscripten-0.23.0.tgz#94f412d0ee5f3021fc12ddf6c0b00bd8ce0d28b9"
|
||||
integrity sha512-CIP+NDRYDDqbT3cTiN8Bon1wsZ7IgISVYCJHYsPc86oxszpepVMPXFfttyQgn1u1okg1HPnCnM7Xv1LrCO/VmQ==
|
||||
|
||||
railroad-diagrams@^1.0.0:
|
||||
version "1.0.0"
|
||||
resolved "https://registry.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz#eb7e6267548ddedfb899c1b90e57374559cddb7e"
|
||||
@ -6166,14 +6171,6 @@ vite@^4.1.5:
|
||||
optionalDependencies:
|
||||
fsevents "~2.3.2"
|
||||
|
||||
vm2@^3.9.18:
|
||||
version "3.9.18"
|
||||
resolved "https://registry.yarnpkg.com/vm2/-/vm2-3.9.18.tgz#d919848bee191a0410c5cc1c5aac58adfd03ce9a"
|
||||
integrity sha512-iM7PchOElv6Uv6Q+0Hq7dcgDtWWT6SizYqVcvol+1WQc+E9HlgTCnPozbQNSP3yDV9oXHQOEQu530w2q/BCVZg==
|
||||
dependencies:
|
||||
acorn "^8.7.0"
|
||||
acorn-walk "^8.2.0"
|
||||
|
||||
w3c-keyname@^2.2.4:
|
||||
version "2.2.6"
|
||||
resolved "https://registry.yarnpkg.com/w3c-keyname/-/w3c-keyname-2.2.6.tgz#8412046116bc16c5d73d4e612053ea10a189c85f"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user