Skip to content

Commit

Permalink
Add an expiry time to inbound webhooks (#984)
Browse files Browse the repository at this point in the history
* Add logic to enable generic hook expiry

* Add storage for hook expiry warnings.

* Migrate generic hooks / add expiry field

* Allow reporting a specific error and status code for generic webhooks

* Report the specific error when a message fails to send

* Refactor input class to better support datetime

* Remove single use of innerChild

* Add UI support for expiry configuration

* Add new packages

* Add warnings when the timer is about to expire.

* Add send expiry notice config option

* lint

* document new option s

* Fixup test

* Add tests for expiry

* Add textual command for setting a duration on a webhook.

* Add e2e test for inbound hooks.

* changelog

* Add a configuration option to force webhooks to expire.

* update config.sample.yml

* fix field not working
  • Loading branch information
Half-Shot authored Nov 18, 2024
1 parent 6571b9f commit 80c7d35
Show file tree
Hide file tree
Showing 30 changed files with 1,002 additions and 657 deletions.
1 change: 1 addition & 0 deletions changelog.d/985.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for setting an expiry time on a webhook. See the documentation on [Generic Webhooks](https://matrix-org.github.io/matrix-hookshot/latest/setup/webhooks.html) for more information.
3 changes: 3 additions & 0 deletions config.sample.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,13 @@ listeners:
# enabled: false
# outbound: false
# enableHttpGet: false
# sendExpiryNotice: false
# requireExpiryTime: false
# urlPrefix: https://example.com/webhook/
# userIdPrefix: _webhooks_
# allowJsTransformationFunctions: false
# waitForComplete: false
# maxExpiryTime: 30d

#feeds:
# # (Optional) Configure this to enable RSS/Atom feed support
Expand Down
16 changes: 16 additions & 0 deletions docs/setup/webhooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ generic:
allowJsTransformationFunctions: false
waitForComplete: false
enableHttpGet: false
# maxExpiryTime: 30d
# sendExpiryNotice: false
# userIdPrefix: webhook_
```

Expand Down Expand Up @@ -43,6 +45,13 @@ has been sent (`true`). By default this is `false`.
`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.

`maxExpiryTime` sets an upper limit on how long a webhook can be valid for before the bridge expires it. By default this is unlimited. This
takes a duration represented by a string. E.g. "30d" is 30 days. See [this page](https://github.com/jkroso/parse-duration?tab=readme-ov-file#available-unit-types-are)
for available units. Additionally:

- `sendExpiryNotice` configures whether a message is sent into a room when the connection is close to expiring.
- `requireExpiryTime` forbids creating a webhook without a expiry time. This does not apply to existing webhooks.

You may set a `userIdPrefix` to create a specific user for each new webhook connection in a room. For example, a connection with a name
like `example` for a prefix of `webhook_` will create a user called `@webhook_example:example.com`. If you enable this option,
you need to configure the user to be part of your registration file e.g.:
Expand Down Expand Up @@ -117,6 +126,13 @@ can specify this either globally in your config, or on the widget with `waitForC
If you make use of the `webhookResponse` feature, you will need to enable `waitForComplete` as otherwise hookshot will
immeditately respond with it's default response values.


#### Expiring webhooks

Webhooks can be configured to expire, such that beyond a certain date they will fail any incoming requests. Currently this expiry time
is mutable, so anybody able to configure connections will be able to change the expiry date. Hookshot will send a notice to the room
at large when the webhook has less than 3 days until it's due to expire (if `sendExpiryNotice` is set).

### JavaScript Transformations

<section class="notice">
Expand Down
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@
"@octokit/rest": "^20.0.2",
"@octokit/webhooks": "^12.0.10",
"@sentry/node": "^7.52.1",
"@vector-im/compound-design-tokens": "^1.3.0",
"@vector-im/compound-web": "^4.8.0",
"@vector-im/compound-design-tokens": "^2.0.1",
"@vector-im/compound-web": "^7.3.0",
"ajv": "^8.11.0",
"axios": "^1.7.4",
"clsx": "^2.1.1",
"cors": "^2.8.5",
"date-fns": "^4.1.0",
"express": "^4.20.0",
"figma-js": "^1.14.0",
"helmet": "^7.1.0",
Expand All @@ -67,6 +69,7 @@
"mime": "^4.0.1",
"node-emoji": "^2.1.3",
"p-queue": "^6.6.2",
"parse-duration": "^1.1.0",
"preact-render-to-string": "^6.3.1",
"prom-client": "^15.1.0",
"quickjs-emscripten": "^0.26.0",
Expand Down
119 changes: 119 additions & 0 deletions spec/generic-hooks.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { E2ESetupTestTimeout, E2ETestEnv, E2ETestMatrixClient } from "./util/e2e-test";
import { describe, it } from "@jest/globals";
import { GenericHookConnection } from "../src/Connections";
import { TextualMessageEventContent } from "matrix-bot-sdk";
import { add } from "date-fns/add";

async function createInboundConnection(user: E2ETestMatrixClient, botMxid: string, roomId: string, duration?: string) {
const join = user.waitForRoomJoin({ sender: botMxid, roomId });
const connectionEvent = user.waitForRoomEvent({
eventType: GenericHookConnection.CanonicalEventType,
stateKey: 'test',
sender: botMxid
});
await user.inviteUser(botMxid, roomId);
await user.setUserPowerLevel(botMxid, roomId, 50);
await join;

// Note: Here we create the DM proactively so this works across multiple
// tests.
// Get the DM room so we can get the token.
const dmRoomId = await user.dms.getOrCreateDm(botMxid);

await user.sendText(roomId, '!hookshot webhook test' + (duration ? ` ${duration}` : ""));
// Test the contents of this.
await connectionEvent;

const msgPromise = user.waitForRoomEvent({ sender: botMxid, eventType: "m.room.message", roomId: dmRoomId });
const { data: msgData } = await msgPromise;
const msgContent = msgData.content as unknown as TextualMessageEventContent;
const [_unused1, _unused2, url] = msgContent.body.split('\n');
return url;
}

describe('Inbound (Generic) Webhooks', () => {
let testEnv: E2ETestEnv;

beforeAll(async () => {
const webhooksPort = 9500 + E2ETestEnv.workerId;
testEnv = await E2ETestEnv.createTestEnv({
matrixLocalparts: ['user'],
config: {
generic: {
enabled: true,
// Prefer to wait for complete as it reduces the concurrency of the test.
waitForComplete: true,
urlPrefix: `http://localhost:${webhooksPort}`
},
listeners: [{
port: webhooksPort,
bindAddress: '0.0.0.0',
// Bind to the SAME listener to ensure we don't have conflicts.
resources: ['webhooks'],
}],
}
});
await testEnv.setUp();
}, E2ESetupTestTimeout);

afterAll(() => {
return testEnv?.tearDown();
});

it('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'});
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.');

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).toEqual({
msgtype: 'm.notice',
body: 'Received webhook data: Hello world',
formatted_body: '<p>Received webhook data: Hello world</p>',
format: 'org.matrix.custom.html',
'uk.half-shot.hookshot.webhook_data': 'Hello world'
});
});

it('should be able to create a new expiring webhook and handle valid requests.', async () => {
jest.useFakeTimers();
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, '2h');
expect((await okMsg).data.content.body).toEqual('Room configured to bridge webhooks. See admin room for secret url.');

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).toEqual({
msgtype: 'm.notice',
body: 'Received webhook data: Hello world',
formatted_body: '<p>Received webhook data: Hello world</p>',
format: 'org.matrix.custom.html',
'uk.half-shot.hookshot.webhook_data': 'Hello world'
});
jest.setSystemTime(add(new Date(), { hours: 3 }));
const expiredReq = await fetch(url, {
method: "PUT",
body: "Hello world"
});
expect(expiredReq.status).toEqual(404);
expect(await expiredReq.json()).toEqual({
ok: false,
error: "This hook has expired",
});
});
});
14 changes: 6 additions & 8 deletions src/Bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { GetIssueResponse, GetIssueOpts } from "./Gitlab/Types"
import { GithubInstance } from "./github/GithubInstance";
import { IBridgeStorageProvider } from "./Stores/StorageProvider";
import { IConnection, GitHubDiscussionSpace, GitHubDiscussionConnection, GitHubUserSpace, JiraProjectConnection, GitLabRepoConnection,
GitHubIssueConnection, GitHubProjectConnection, GitHubRepoConnection, GitLabIssueConnection, FigmaFileConnection, FeedConnection, GenericHookConnection, WebhookResponse } from "./Connections";
GitHubIssueConnection, GitHubProjectConnection, GitHubRepoConnection, GitLabIssueConnection, FigmaFileConnection, FeedConnection, GenericHookConnection } from "./Connections";
import { IGitLabWebhookIssueStateEvent, IGitLabWebhookMREvent, IGitLabWebhookNoteEvent, IGitLabWebhookPushEvent, IGitLabWebhookReleaseEvent, IGitLabWebhookTagPushEvent, IGitLabWebhookWikiPageEvent } from "./Gitlab/WebhookTypes";
import { JiraIssueEvent, JiraIssueUpdatedEvent, JiraVersionEvent } from "./jira/WebhookTypes";
import { JiraOAuthResult } from "./jira/Types";
Expand Down Expand Up @@ -606,7 +606,7 @@ export class Bridge {

if (!connections.length) {
await this.queue.push<GenericWebhookEventResult>({
data: {notFound: true},
data: {successful: true, notFound: true},
sender: "Bridge",
messageId: messageId,
eventName: "response.generic-webhook.event",
Expand All @@ -621,21 +621,19 @@ export class Bridge {
await c.onGenericHook(data.hookData);
return;
}
let successful: boolean|null = null;
let response: WebhookResponse|undefined;
if (this.config.generic?.waitForComplete || c.waitForComplete) {
const result = await c.onGenericHook(data.hookData);
successful = result.successful;
response = result.response;
await this.queue.push<GenericWebhookEventResult>({
data: {successful, response},
data: result,
sender: "Bridge",
messageId,
eventName: "response.generic-webhook.event",
});
} else {
await this.queue.push<GenericWebhookEventResult>({
data: {},
data: {
successful: null,
},
sender: "Bridge",
messageId,
eventName: "response.generic-webhook.event",
Expand Down
Loading

0 comments on commit 80c7d35

Please sign in to comment.