Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1004.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an option in the config to disable hook bodies in Matrix messages.
1 change: 1 addition & 0 deletions config.sample.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ listeners:
# sendExpiryNotice: false
# requireExpiryTime: false
# maxExpiryTime: 30d
# includeHookBody: true

#figma:
# # (Optional) Configure this to enable Figma support
Expand Down
3 changes: 3 additions & 0 deletions docs/setup/webhooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
56 changes: 55 additions & 1 deletion spec/generic-hooks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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" });
Expand Down Expand Up @@ -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();
});
});
22 changes: 20 additions & 2 deletions src/Connections/GenericHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -164,6 +169,7 @@ export class GenericHookConnection
transformationFunction,
waitForComplete,
expirationDate: expirationDateStr,
includeHookBody,
} = state;
if (!name) {
throw new ApiError("Missing name", ErrCode.BadValue);
Expand All @@ -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) {
Expand Down Expand Up @@ -217,6 +229,7 @@ export class GenericHookConnection
transformationFunction: transformationFunction || undefined,
waitForComplete,
expirationDate,
includeHookBody,
};
}

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions src/config/sections/GenericHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface BridgeGenericWebhooksConfigYAML {
maxExpiryTime?: string;
sendExpiryNotice?: boolean;
requireExpiryTime?: boolean;
includeHookBody?: boolean;
}

export class BridgeConfigGenericWebhooks {
Expand All @@ -39,13 +40,15 @@ 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;
this.outbound = yaml.outbound || false;
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 = () => {
Expand Down
Loading