Skip to content

feat(typescript): make verify a type-guard #376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

feat(typescript): make verify a type-guard #376

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion src/verify/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
import { timingSafeEqual } from "crypto";
import { Buffer } from "buffer";
import { EventTypesPayload } from "../generated/get-webhook-payload-type-from-event";
import { sign } from "../sign/index";

type WebhookEvents = Exclude<
keyof EventTypesPayload,
`${string}.${string}` | "errors" | "*"
Copy link
Contributor

@gr2m gr2m Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woah I didn't know you can do the template literals with string types! Very cool

Copy link
Member Author

@G-Rath G-Rath Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thats the nice hotness that landed with 4.1 - it only arrived about 2 weeks ago.

I'm actually very happy with this bit of code, because it's my first sensible reason to use it in production code, and it works like a treat 😍

>;

type GithubEvent<TName extends WebhookEvents = WebhookEvents> = Omit<
EventTypesPayload[TName],
"name"
> & { name: TName };

const getAlgorithm = (signature: string) => {
return signature.startsWith("sha256=") ? "sha256" : "sha1";
};
Expand All @@ -10,7 +21,7 @@ export function verify(
secret: string,
eventPayload: object,
signature: string
): boolean {
): eventPayload is GithubEvent {
Copy link
Contributor

@gr2m gr2m Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain how that helps exactly? I'm not familiar with that notion

Copy link
Member Author

@G-Rath G-Rath Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem - type guards allow you to define functions that narrow types based on if they return true or false. We actually now also have assertion guards which are the same except the function throws (they landed a few months ago, and very cool).

So by making this a type-guard, you can do this:

const signature = event.headers['X-Hub-Signature'];
const { GITHUB_WEBHOOK_SECRET } = process.env;
const body = JSON.parse(event.body ?? '{}') as object;

if (verify(GITHUB_WEBHOOK_SECRET, body, signature)) {
  console.log('Got an event named', body.name, 'from GitHub');
}

In the above, because verify is a type guard, TypeScript knows that in the scope of the if statement body has to be of type GithubEvent when that code is running since it only runs when its true, and so treats it as such while within the braces of the if statement.

Array.isArray is an example of a built-in type guard - that function is defined as isArray<T>(arg: T | {}): arg is T[].

(it's actually a bit bigger, as it has a conditional check to say "arg is (if T is readonly ? readonly T[] : T[])", but I've removed that to keep things simple)

Copy link
Member Author

@G-Rath G-Rath Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the code in action, vs the current method.

You'll see that the first use of body.name has an error, whereas the second doesn't:

image

Copy link
Contributor

@gr2m gr2m Dec 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, I got it now, makes a lot of sense. Thank you for the thorough explanation!

if (!secret || !eventPayload || !signature) {
throw new TypeError(
"[@octokit/webhooks] secret, eventPayload & signature required"
Expand Down