Skip to content

Commit

Permalink
fix: Handle verify error (#916)
Browse files Browse the repository at this point in the history
* update publish pattern to make sure maintenance branches are picked up

* fix: Handle verify error  (#915)

* fix: handle error thrown by `verify` method (#914)

* test: handle error thrown by `verify` method

* fix: handle error thrown by `verify` method

* test: remove `.only`

* lint fix

---------

Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>

---------

Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>
  • Loading branch information
nickfloyd and gr2m authored Nov 14, 2023
1 parent c5e041d commit 2618438
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ name: Release
- main
- next
- beta
- v*.x
- "+([0-9]).x"
jobs:
release:
name: release
Expand Down
2 changes: 1 addition & 1 deletion src/verify-and-receive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function verifyAndReceive(
? toNormalizedJsonString(event.payload)
: event.payload,
event.signature
);
).catch(() => false);

if (!matchesSignature) {
const error = new Error(
Expand Down
47 changes: 47 additions & 0 deletions test/integration/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,51 @@ describe("createNodeMiddleware(webhooks)", () => {

server.close();
});

test("Handles invalid signature", async () => {
expect.assertions(3);

const webhooks = new Webhooks({
secret: "mySecret",
});

webhooks.onError((error) => {
expect(error.message).toContain(
"signature does not match event payload and secret"
);
});

const log = {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const middleware = createNodeMiddleware(webhooks, { log });
const server = createServer(middleware).listen();

// @ts-expect-error complains about { port } although it's included in returned AddressInfo interface
const { port } = server.address();

const response = await fetch(
`http://localhost:${port}/api/github/webhooks`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
"X-GitHub-Delivery": "1",
"X-GitHub-Event": "push",
"X-Hub-Signature-256": "",
},
body: pushEventPayload,
}
);

expect(response.status).toEqual(400);
await expect(response.text()).resolves.toBe(
'{"error":"Error: [@octokit/webhooks] signature does not match event payload and secret"}'
);

server.close();
});
});

0 comments on commit 2618438

Please # to comment.