diff --git a/changelog.d/1004.feature b/changelog.d/1004.feature new file mode 100644 index 000000000..c0317f945 --- /dev/null +++ b/changelog.d/1004.feature @@ -0,0 +1 @@ +Add an option in the config to disable hook bodies in Matrix messages. diff --git a/config.sample.yml b/config.sample.yml index a70efff34..6d8c4c5ad 100644 --- a/config.sample.yml +++ b/config.sample.yml @@ -118,6 +118,7 @@ listeners: # sendExpiryNotice: false # requireExpiryTime: false # maxExpiryTime: 30d +# includeHookBody: true #figma: # # (Optional) Configure this to enable Figma support diff --git a/docs/setup/webhooks.md b/docs/setup/webhooks.md index d01baa652..a61b91396 100644 --- a/docs/setup/webhooks.md +++ b/docs/setup/webhooks.md @@ -41,6 +41,9 @@ webhook requests from `https://example.com/mywebhookspath` to the bridge (on `/w respond with a 200 as soon as the webhook has entered processing (`false`) while others prefer to know if the resulting Matrix message has been sent (`true`). By default this is `false`. +`includeHookBody` will determine whether the bridge will include the full webhook request body +(`uk.half-shot.hookshot.webhook_data`) inside the Matrix event. By default this is `true`. + `enableHttpGet` means that webhooks can be triggered by `GET` requests, in addition to `POST` and `PUT`. This was previously on by default, but is now disabled due to concerns mentioned below. diff --git a/spec/generic-hooks.spec.ts b/spec/generic-hooks.spec.ts index 13678cb12..f9125a68a 100644 --- a/spec/generic-hooks.spec.ts +++ b/spec/generic-hooks.spec.ts @@ -3,7 +3,15 @@ import { E2ETestEnv, E2ETestMatrixClient, } from "./util/e2e-test"; -import { describe, test, beforeAll, afterAll, expect, vitest } from "vitest"; +import { + describe, + test, + beforeAll, + afterAll, + afterEach, + expect, + vitest, +} from "vitest"; import { GenericHookConnection } from "../src/Connections"; import { TextualMessageEventContent } from "matrix-bot-sdk"; import { add } from "date-fns/add"; @@ -78,6 +86,10 @@ describe("Inbound (Generic) Webhooks", () => { return testEnv?.tearDown(); }); + afterEach(() => { + vitest.useRealTimers(); + }); + test("should be able to create a new webhook and handle an incoming request.", async () => { const user = testEnv.getUser("user"); const roomId = await user.createRoom({ name: "My Test Webhooks room" }); @@ -159,4 +171,46 @@ describe("Inbound (Generic) Webhooks", () => { error: "This hook has expired", }); }); + test("should allow disabling hook data in matrix events.", async () => { + const user = testEnv.getUser("user"); + const roomId = await user.createRoom({ name: "My Test Webhooks room" }); + const okMsg = user.waitForRoomEvent({ + eventType: "m.room.message", + sender: testEnv.botMxid, + roomId, + }); + const url = await createInboundConnection(user, testEnv.botMxid, roomId); + expect((await okMsg).data.content.body).toEqual( + "Room configured to bridge webhooks. See admin room for secret url.", + ); + + await user.sendStateEvent( + roomId, + GenericHookConnection.CanonicalEventType, + "test", + { + ...(await user.getRoomStateEvent( + roomId, + GenericHookConnection.CanonicalEventType, + "test", + )), + includeHookBody: false, + }, + ); + + const expectedMsg = user.waitForRoomEvent({ + eventType: "m.room.message", + sender: testEnv.botMxid, + roomId, + }); + const req = await fetch(url, { + method: "PUT", + body: "Hello world", + }); + expect(req.status).toEqual(200); + expect(await req.json()).toEqual({ ok: true }); + expect( + (await expectedMsg).data.content["uk.half-shot.hookshot.webhook_data"], + ).toBeUndefined(); + }); }); diff --git a/src/Connections/GenericHook.ts b/src/Connections/GenericHook.ts index 5eb04b7cd..41aa20760 100644 --- a/src/Connections/GenericHook.ts +++ b/src/Connections/GenericHook.ts @@ -41,6 +41,11 @@ export interface GenericHookConnectionState extends IConnectionState { */ waitForComplete?: boolean | undefined; + /** + * Should the Matrix event include the `uk.half-shot.hookshot.webhook_data` property. + */ + includeHookBody?: boolean; + /** * If the webhook has an expriation date, then the date at which the webhook is no longer value * (in UTC) time. @@ -164,6 +169,7 @@ export class GenericHookConnection transformationFunction, waitForComplete, expirationDate: expirationDateStr, + includeHookBody, } = state; if (!name) { throw new ApiError("Missing name", ErrCode.BadValue); @@ -180,6 +186,12 @@ export class GenericHookConnection ErrCode.BadValue, ); } + if (includeHookBody !== undefined && typeof includeHookBody !== "boolean") { + throw new ApiError( + "'includeHookBody' must be a boolean", + ErrCode.BadValue, + ); + } // Use !=, not !==, to check for both undefined and null if (transformationFunction != undefined) { if (!WebhookTransformer.canTransform) { @@ -217,6 +229,7 @@ export class GenericHookConnection transformationFunction: transformationFunction || undefined, waitForComplete, expirationDate, + includeHookBody, }; } @@ -639,7 +652,10 @@ export class GenericHookConnection ); // Matrix cannot handle float data, so make sure we parse out any floats. - const safeData = GenericHookConnection.sanitiseObjectForMatrixJSON(data); + const safeData = + (this.state.includeHookBody ?? this.config.includeHookBody) + ? GenericHookConnection.sanitiseObjectForMatrixJSON(data) + : undefined; await this.messageClient.sendMatrixMessage( this.roomId, @@ -652,7 +668,9 @@ export class GenericHookConnection ? { "m.mentions": content.mentions } : undefined), format: "org.matrix.custom.html", - "uk.half-shot.hookshot.webhook_data": safeData, + ...(safeData + ? { "uk.half-shot.hookshot.webhook_data": safeData } + : undefined), }, "m.room.message", sender, diff --git a/src/config/sections/GenericHooks.ts b/src/config/sections/GenericHooks.ts index 14bfa04c9..06d3bdb7a 100644 --- a/src/config/sections/GenericHooks.ts +++ b/src/config/sections/GenericHooks.ts @@ -18,6 +18,7 @@ export interface BridgeGenericWebhooksConfigYAML { maxExpiryTime?: string; sendExpiryNotice?: boolean; requireExpiryTime?: boolean; + includeHookBody?: boolean; } export class BridgeConfigGenericWebhooks { @@ -39,6 +40,7 @@ export class BridgeConfigGenericWebhooks { public readonly requireExpiryTime: boolean; // Public facing value for config generator public readonly maxExpiryTime?: string; + public readonly includeHookBody: boolean; constructor(yaml: BridgeGenericWebhooksConfigYAML) { this.enabled = yaml.enabled || false; @@ -46,6 +48,7 @@ export class BridgeConfigGenericWebhooks { this.enableHttpGet = yaml.enableHttpGet || false; this.sendExpiryNotice = yaml.sendExpiryNotice || false; this.requireExpiryTime = yaml.requireExpiryTime || false; + this.includeHookBody = yaml.includeHookBody ?? true; try { this.parsedUrlPrefix = makePrefixedUrl(yaml.urlPrefix); this.urlPrefix = () => {