reported by @not-an-aardvark. Teddy, okay to share your gist here? /cc @bkeepers

See https://unpkg.com/browse/@octokit/[email protected]/

The problem is that we forgot to adapt the release settings and build the package with pika before release. This will be fixed via #175

For the upcoming octokit we need to be able to inject a pre-authenticated octokit client to the webhooks handler, see the Webhooks section

const webhooks = client.webhooks({secret})
webhooks.on('issue.opened', ({id, name, payload, client}) => {
  return client.rest.issues.createComment({body: 'Hello, World!'})
})
require('http').createServer(webhooks.middleware).listen(3000)

The client parameter in the code snippet above needs to be injected, it is not part of @octokit/webhooks API. We need to be able to run code after a webhook is received, but before it is passed to the webhook handlers.

One approach would be to implement middleware similar to what Express does: https://expressjs.com/en/guide/using-middleware.html. There is a related issue on Probot: probot/probot#598

An alternative would be to leverage the hook API that we already use in @octokit/rest and that will be part of octokit: https://github.com/octokit/rest.js#hooks.

Summary

verifyAndReceive uses a parsed representation of the passed json data. This can lead to false negative signature verifications. One Example is the signature verification failure for ANSI/ISO-6429 codes a.k.a. escape sequences. If Any GitHub event body contains an escape sequence, the verification will always fail, even though everything is valid.

Details

When the ESC sequence is json-encoded it might be encoded as \u001b OR \u001B.
The node json parser will always encode this sequence with a lowercase b, which means that the verification will fail if the signature was generated with a capital B (which is already the case).
Since this (and other encoding details) are ambiguous in the json standard, the only safe option is to verify the signature with the raw body we receive and not with the parsed representation, since JSON.stringify(JSON.parse(x)) does not necessarily yield x, thus causing a false negative.
This however is only one example, that already happens. In general, the only way that will guarantee that we properly verify the signature header is, using the data in the format it was encoded in the first place. This means, before parsing it.

Suggestion

In order to solve the problem, middleware/middleware might need to be adjusted, together with get-payload and verify-and-receive.
I was already working on that when a realised the following compatibility issues:

Compatibility concerns

1. verifyAndReceived is part of the public API, meaning people are using it directly and outside of the middleware context. (I saw it in some PR discussions). This means this function has to stay and still work with parsed data instead of json.
2. Since this PR octokit is supporting the usage in combination with body-parser, which will also store parsed information in request.body

It seems as if we need to keep the verification based on parsed data, if we want to support the mix of body-parser and webhook.js. We could also store the raw body in the request object.

A compromise could be:
testing if req.body is defined and if that is the case, we could use the current implementation.
Also keep the current implementation of verifyAndReceive exposed and (maybe?) deprecate it.
In order to fix our bug, we could just add a second verify and receive implementation that does the verification with the raw data.
This however would duplicate a lot of code and I am not sure what you think about this.

I hope we can figure out a way to fix this bug. This is just my view on things, maybe somebody has a better suggestion on how to address this issue.

Related issues

• This issue might be related to: #61
• There are also several slack discussions raising this topic (Non ANSI code support)

/cc @gr2m

Limiting the events that can be received to this list has two issues:

1. This library can't be used by beta testers for webhooks that are not yet unreleased
2. I think there are some undocumented webhooks for GitHub Enterprise. I can't find the list right now, but will try to locate it

It'd be great if the assertions could be turned off, or turned into just warnings.

Hey guys 🙂

First off, thanks for the awesome project! After #168 got merged I started using generated types for workflow_dispatch event and it works great. However having to type cast inputs of the event through any pains me a lot:

type WebhookPayloadWorkflowDispatchInputs = {};
type WebhookPayloadWorkflowDispatch = {
  inputs: WebhookPayloadWorkflowDispatchInputs;
  ref: string;
  repository: PayloadRepository;
  organization: WebhookPayloadWorkflowDispatchOrganization;
  sender: WebhookPayloadWorkflowDispatchSender;
  workflow: string;
};

type FooBarInput = { foo: string; bar: string };

const onWorkflowDispatch = (event: Webhooks.WebhookPayloadWorkflowDispatch) => {
  const { foo, bar } = (event.inputs as any) as FooBarInput;
  ...
}

However, I do believe that a better developer experience can be achieved by introducing generics in generated types:

type WebhookPayloadWorkflowDispatch<T> = {
  inputs: T;
  ref: string;
  repository: PayloadRepository;
  organization: WebhookPayloadWorkflowDispatchOrganization;
  sender: WebhookPayloadWorkflowDispatchSender;
  workflow: string;
};

type FooBarInput = { foo: string; bar: string };

const onWorkflowDispatch = (event: Webhooks.WebhookPayloadWorkflowDispatch<FooBarInput>) => {
  const { foo, bar } = event.inputs;
  ...
}

It is something that should be raised here or in gimenete/type-writer? I'd be happy to move this issue there if that's the case. Anyway, I would be happy to assist with a PR if/when we figure out some kind of a solution for this.

I ran into this issue and though it was never resolved, it appears tha the underlying problem is that @octokit/wehooks is assuming that the incoming request body is application/json. However, github can also deliver payloads in application/x-www-form-url-encoded which is, in fact, the default delivery format when creating a webhook via the github UI

In the event that the payload is encoded as application/x-www-form-urlencoded, the middleware will try to parse it as JSON.

I could see two possible solutions:

1. asserting that the content-type header is aapplication/json, and rejecting with a 400 and a status text that clearly indicates that x-form-url-encoded is not acceptable, and that the webhook configuration should be updated.
2. checking the content-type and decoding the payload appropriately for both application/json and application/x-form-www-url-encoded.

Would be happy to help implement either.

[❓question] Would have sense to use enums for event names?

A use case where it would be valuable is to make sure the events to be listened webhooks.on([<eventName>]) are the right ones on compile time.

If you think is a valid proposal I would be more than happy to propose a pull-request for it! :)

Hello! I attempted to build a new TypeScript project that consumes this module. I created a package.json and installed the following dependencies:

{
  "dependencies": {
    "@octokit/webhooks": "5.3.3"
  },
  "devDependencies": {
    "@types/node": "^11.12.0",
    "typescript": "^3.3.4000"
  }
}

I took the example from the readme and fixed the imports to work with TypeScript but it fails to compile:

// TypeScript default import syntax
import * as WebhooksApi from '@octokit/webhooks';
import * as http from 'http';

const webhooks = new WebhooksApi({
  secret: 'mysecret'
});

webhooks.on('*', ({id, name, payload}) => {
  console.log(name, 'event received');
});

http.createServer(webhooks.middleware).listen(3000);
//                ~~~~~~~~~~~~~~~~~~~
// TS2559: Type '(request: ClientRequest, response: ServerResponse, next: (err?: any) => void) => (request: IncomingMessage, response: ServerResponse) => void' has no properties in common with type 'ServerOptions'.

I would expect that this code example would successfully compile. What is going on here? It looks like middleware is not a http.RequestListener, but a function that returns a http.RequestListener. Is this merely an issue with the definition of middleware in index.d.ts?

Thanks for your help!

When trying to send a request with correct payload from PHP I am stuck with the issue that a correctly signed payload doesn't pass signature verification due to its conversion from string to object in middleware/get-payload.js. Original payload is coming as a string and the string is correctly signed with SHA1. But after parsing the string it becomes an object and after that the signature to be compared is calculated on the object being converted back to the string with formatting. I cannot emulate the same string structure.

There are several references to receiver in the docs and dead links linking to /receiver. I tried to find the relevant directory/package, but didn't find anything.

This will help avoid problems like #64 in the future

🚨 The automated release from the master branch failed. 🚨

I recommend you give this issue a high priority, so other packages depending on you could benefit from your bug fixes and new features.

You can find below the list of errors reported by semantic-release. Each one of them has to be resolved in order to automatically publish your package. I’m sure you can resolve this 💪.

Errors are usually caused by a misconfiguration or an authentication problem. With each error reported below you will find explanation and guidance to help you to resolve it.

Once all the errors are resolved, semantic-release will release your package the next time you push a commit to the master branch. You can also manually restart the failed CI job that runs semantic-release.

If you are not sure how to resolve this, here is some links that can help you:

• Usage documentation
• Frequently Asked Questions
• Support channels

If those don’t help, or if this issue is reporting something you think isn’t right, you can always ask the humans behind semantic-release.

No npm token specified.

An npm token must be created and set in the NPM_TOKEN environment variable on your CI environment.

Please make sure to create an npm token and to set it in the NPM_TOKEN environment variable on your CI environment. The token must allow to publish to the registry https://registry.npmjs.org/.

Good luck with your project ✨

Your semantic-release bot 📦🚀

I don't see a reason why we should restrict what the .on(event, callback) callback function returns. People use early returns or want to return promises in non-async callback functions.

This code currently emits TypeScript errors

webhooks.on("issues", () => {
  return 1;
});
webhooks.on("issues", () => {
  return Promise.resolve(1);
});
webhooks.on("issues", async () => {
  return "foo";
});

I don't think it should. We should allow for any return type, both synchronous and asynchronous

Should signature process be included here as well as it is dependent upon this? Am I reading this wrong? Maybe I do not understand. The hole scope of what is needed to verify a webhook..

Hi there 👋
I am opening this regarding some findings from the issue probot/probot#568

While debugging to understand the error, I noticed that if the header X-Hub-Signature is missing (or any header actually) a 400 error response is sent, but the error is never propagated to the error handler (in that case, probot):

Sorry if I got it wrong, but shouldn't it be calling the error handler like the case with an invalid signature?

Hello! I've come across an interesting bug in the type definitions when recently updating

$ tsc
node_modules/@octokit/webhooks/index.d.ts:5004:17 - error TS7010: 'createWebhooksApi', which lacks return-type annotation, implicitly has an 'any' return type.

5004 export function createWebhooksApi(options?: Options);
                     ~~~~~~~~~~~~~~~~~


Found 1 error.

I can work around this using the skipLibCheck option for tsc, but skipping all 3rd party type defs is clearly not ideal 😄

Any chance you'd consider updating the types here?

moving from gr2m/octokit-webhooks.js-legacy#8 @rtsao, @JasonEtco

Per probot/probot#316 (comment), for testing probot apps, it would be ideal to have a node-github-like API to programmatically generate webhook fixtures, for example something like:

let robot = /* ... */

let github = new MockGitHub();
let repo = github.mocks.repo({
  org: 'test-org',
  name: 'test-repo'
});

robot.auth = () => Promise.resolve(github);

let webhook = github.issues.edited(
  repo.issue({
    body: 'new issue body'
  })
);

await robot.recieve(webhook);

/* ... assertions ... */

Hi,
I am not sure whether this is the right place to post this issue (or whether this is intentional), fell free to guide me there.
I am creating a github app for tracking security vulnerabilities in the repos of my organisation. I was checking the security advisory event, but the event payload is missing the repository field for which the advisory was issued. I wanted to create automated issues on repos when the advisory is published, but the missing field makes it really difficult.
Another way would be to fetch security advisories of all the repos in the organisation, but it looks very counter-productive (Not to mention we have around 2K repos in the organisation).

🚨 The automated release from the master branch failed. 🚨

I recommend you give this issue a high priority, so other packages depending on you could benefit from your bug fixes and new features.

You can find below the list of errors reported by semantic-release. Each one of them has to be resolved in order to automatically publish your package. I’m sure you can resolve this 💪.

Errors are usually caused by a misconfiguration or an authentication problem. With each error reported below you will find explanation and guidance to help you to resolve it.

Once all the errors are resolved, semantic-release will release your package the next time you push a commit to the master branch. You can also manually restart the failed CI job that runs semantic-release.

If you are not sure how to resolve this, here is some links that can help you:

• Usage documentation
• Frequently Asked Questions
• Support channels

If those don’t help, or if this issue is reporting something you think isn’t right, you can always ask the humans behind semantic-release.

Invalid npm token.

The npm token configured in the NPM_TOKEN environment variable must be a valid token allowing to publish to the registry https://registry.npmjs.org/.

If you are using Two-Factor Authentication, make configure the auth-only level is supported. semantic-release cannot publish with the default auth-and-writes level.

Please make sure to set the NPM_TOKEN environment variable in your CI with the exact value of the npm token.

Good luck with your project ✨

Your semantic-release bot 📦🚀

Version 15.5.1 of semantic-release was just published.

This version is covered by your current version range and after updating it in your project the build failed.

semantic-release is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

index.d.ts is missing exports for several items that the README specifies as something that can be used standalone: verify, sign, and middleware

I have this code

const SmeeClient = require('smee-client')

process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = 0;

const smee = new SmeeClient({
  source: 'https://smee.io/ou0EqBOIPJ1QuwDQ',
  target: 'https://localhost:453/webhook/documentsigning',
  logger: console
})

const events = smee.start()

When I have a request from docusign to 'https://smee.io/ou0EqBOIPJ1QuwDQ' this throws and app stop. Error message is attached:

_http_outgoing.js:690
      throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk);
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be one of type string or Buffer. Received type object
    at ClientRequest.end (_http_outgoing.js:690:13)
    at Request._end (C:\Users\User\Desktop\proxy\node_modules\superagent\lib\node\index.js:1021:9)
    at Request.end (C:\Users\User\Desktop\proxy\node_modules\superagent\lib\node\index.js:777:15)
    at Client.onmessage (C:\Users\User\Desktop\proxy\node_modules\smee-client\index.js:35:9)
    at EventSource.emit (events.js:189:13)
    at _emit (C:\Users\User\Desktop\proxy\node_modules\eventsource\lib\eventsource.js:242:17)
    at parseEventStreamLine (C:\Users\User\Desktop\proxy\node_modules\eventsource\lib\eventsource.js:257:9)
    at IncomingMessage.<anonymous> (C:\Users\User\Desktop\proxy\node_modules\eventsource\lib\eventsource.js:217:11)
    at IncomingMessage.emit (events.js:189:13)
    at addChunk (_stream_readable.js:284:12)

👋 It'd be really helpful to expose the list of available webhook events as a JSON-consumable object or array. Type definitions are great, but in a use-case where, for example, I want to expose that list via a <select> dropdown, I'd need to loop through the list. So something auto generated like the TypeScript definitions, but require-able.

Something like:

const events = require('@octokit/webhooks/events')
console.log(events)

// Actions as array items
// -> { issues: ['opened', 'closed'], pull_request: ['synchronize'] }

// Just a big ol' array
// -> ['issues', 'issues.opened', 'issues.closed', 'pull_request', 'pull_request.synchronize']

I can carve out some time to open this as a PR in a couple weeks if that'd help!

Version 10 of Node.js (code name Dubnium) has been released! 🎊

To see what happens to your code in Node.js 10, Greenkeeper has created a branch with the following changes:

• Added the new Node.js version to your .travis.yml

If you’re interested in upgrading this repo to Node.js 10, you can open a PR with these changes. Please note that this issue is just intended as a friendly reminder and the PR as a possible starting point for getting your code running on Node.js 10.

Your Greenkeeper Bot 🌴

The TypeScript typings for the installation field on WebHookPayloadPullRequest is missing:

It's there at runtime, and the documentation suggests it should always be there for pull request events.

When using default import it works:

But when using new named import it does not work:

More details here

The README has a handful of links to standalone services that appear to be dead links:

• https://github.com/octokit/webhooks.js/blame/master/README.md#L148
• https://github.com/octokit/webhooks.js/blame/master/README.md#L189
• https://github.com/octokit/webhooks.js/blame/master/README.md#L499

Do you think it would be an interesting nice to have? It would be a way to clean remote unused git remote references :)

Details: https://github.blog/changelog/2019-07-31-automatically-delete-head-branches-of-pull-requests/

(I open it here but I guess it would be nice across all the repos of octokit)

Hello,

Can you please shed some light on how I am supposed to integrate @octokit/webhoooks with HapiJS server plugins? The @octokit/webhook middleware function currently accepts 3 arguments (http request obj, http response obj, and nodejs next function).

However the handler function for HapiJS 17 has a http request object, h response toolkit object, and does not have a nodejs next function.
https://hapi.dev/api/?v=18.4.0#-routeoptionshandler

I am having a hard time trying to integrate with hapijs, has there been an example of this before?

Thanks

Galdino Rosas

Typescript definitions are mostly generated in scripts/generate-types.js.

But most of the methods described in the README do not yet have definitions, we only have .sign() so far

I’d be grateful for any help completing the typescript definitions.

Using typescript, the webhook event payload type definitions do not match the actual payloads sent by Github.

Example

The handler for the check_suite.completed event receives an argument of type Webhook.WebhookEvent<Webhook.WebhookPayloadCheckSuite>

The Webhook.WebhookPayloadCheckSuite type is defined as:

type WebhookPayloadCheckSuite = {
    action: string;
    check_suite: WebhookPayloadCheckSuiteCheckSuite;
    repository: PayloadRepository;
    sender: WebhookPayloadCheckSuiteSender;
};

The actual received payload contains two extra fields which are missing from the type definition, these fields are installation and organization:

{
  "action": "completed",
  "check_suite": { ... },
  "repository": { ... },
  "sender": { ... },
  "organization": { ... },
  "installation": {
    "id": 1234,
    "node_id": "dummy_node_id"
  }
}

The type definitions should be updated to match the responses provided by Github.

Great project!

I'm using body-parser to transform requests to json form. Seems like this will consume the body stream before webhooks gets a chance to look at it.

Webhooks will then hang the request until the express timeout triggers and cancels the request. No error messages are thrown.

Perhaps a quick note in the README that this package must come before any body consumers such as body parser? Maybe an error thrown by checking if req.body already exists?

I can put together a PR as well.

Problem

A Pull Request on this repository is marked as Ready to Merge even if the index.d.ts has introduced a regression.

• Example PR introducing regression: #134

• Bug introduced: #136

Proposal

To make a required check to run npm run validate:ts on Pull Requests for this repository

smee.io has been created by the Probot team to simplify delivery of webhooks to your local development environment.

I’m not sure how much I’d build it into @octokit/webhooks or how much could be just done by documentation. It’s fairly simple:

const source = new EventSource('https://smee.io/gMOmZA1hb7gzyo')
source.onmessage = (event) => {
  const webhookEvent = JSON.parse(event.data)
  webhooks.verifyAndReceive({
    id: webhookEvent['x-request-id'],
    name: webhookEvent['x-github-event'],
    signature: webhookEvent['x-hub-signature'],
    payload: webhookEvent.body
  }).catch(console.error)
}

What I find most intriguing is that this makes @octokit/webhooks work for browsers :) There is a polyfil for Node as well as browsers without EventSource support: https://github.com/EventSource/eventsource

Going all in on making it part of @octokit/webhooks could looks something like this

webhooks.subscribe('https://smee.io/gMOmZA1hb7gzyo')

Or maybe a better approach that would avoid having a dependency on the eventsource package and the problem of polyfiling in older browsers:

const source = new EventSource('https://smee.io/gMOmZA1hb7gzyo')
source.onmessage = webhooks.verifyAndReceive

And then add checks within verifyAndReceive to see if the passed object is coming from smee or not.

I’ll give the last approach a try

Version 15.6.0 of semantic-release was just published.

This version is covered by your current version range and after updating it in your project the build failed.

semantic-release is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

It seems that the new property located at pull_request.draft is not in the TypeScript definitions.

I have looked in the docs and it seems to be missing there too.

We had issues verifying the signature of some payloads here #71 due to the way it is being calculated (JSON.parse + JSON.stringify) and I made a quick and dirty suggestion #71 (comment)

Nevertheless, the long term solution should be to always calculate the signature from the raw, original, payload.

I know that there are some circumstances where that's not possible (e.g. seems like Firebase doesn't expose the raw original body) but the library should always use the raw payload whenever possible and warn the developer if it's not possible.

We haven't seen any new mismatch between the way webhooks.js calculates the signature, but it could happen since the implementation assumes things that shouldn't rely on.

This is an awesome lib. Intellisense works great handling single events but when the argument is an array it doesn't show.

This issue depends on octokit/webhooks#43. It is related to #30 .

In the generated webhook schema there is only name, actions, and examples for each webhook. So this library generates the TypeScript types using @gimenete/type-writer from the examples in the schema.

This means that critical payload values that exist in the webhook but are not in the examples are not accessible in TypeScript without some terrible escape hatches.

For example, in an IssuesEvent, a developer trying to access the label payload property would not get prompted that it exists:

And entering it anyway results in a compiler error, even though the property exists in the webhook:

The only workaround I've found is to override the types to add the missing properties when they are needed, or escape hatch with any:

webhooks.on('issues.labeled', ({payload}: {payload: any}) => {
  const label = payload.label
})

Since using any removes any type checking, it kinda defeats the purpose of having a schema. So it would be great if the schema could be complete enough to include all the payload parameters in the documentation, and this library could leverage those parameters to build the types.

Thanks!

They are documented in the GitHub events docs, so I'm not sure why they are missing from the TS definition file

In order to make it simpler to contribute, I’d like to split up static Typescript definitions and the generated ones into separate files

follow up to #54 (comment)_ by @wolfy1339

This is what GitHub tells me: https://ibb.co/Cmw9wBp

My setup: https://gist.github.com/YChebotaev/ecd701c195fa4bd1383126969336bdda

Help, please.

Similar to https://github.com/octokit/routes, it would be great to have a published schema for webhook payloads.

probot/probot#626 extracts the type definition we use for Probot. Once we've made a little more progress on that I'll submit a PR here to add basic types, but that will only include hand-crafted types for each payload.

Long-term, we hope to publish official webhook payload schemas as part of Octokit. In the near term though, we may need to do something more manual to generate fixtures based documentation or sample payloads.

cc @gimenete @tcbyrd @kytrinyx

I'm running into a problem when trying to deploy on Firebase: the debug package is being required at runtime but is installed as a devDependency so it is not found. Is it possible to require this as a dependency instead? Thanks!

Discussed here: #72 (comment)

Some integrations need to be able to use the fix from #72 in older versions of Probot (at least 7.5.0 that I know of), so wanted to discuss here if we need to backport webhooks.js in order to facilitate this.

cc @scepticulous @gimenete @gr2m

Thanks a lot for providing this library. I was wondering if there was anyway to return a custom response when a handler is matched? Currently it only returns "ok" however I'd like to return a custom response body if possible. Thanks!

The types for the event installation_repositories are wrong and invalid, thus they don't work.

This is how they should look like

type WebhookPayloadInstallationRepositories = {
    action: string;
    installation: WebhookPayloadInstallationRepositoriesInstallation;
    repository_selection: string;
    repositories_added: Array<any>;
    repositories_removed: Array<
      WebhookPayloadInstallationRepositoriesRepositoriesRemovedItem
    >;
    sender: WebhookPayloadInstallationRepositoriesSender;
  };

When using webhooks.on('github_app_authorization'...

follow up for #1 (comment)

Use case

I want to build a GitHub app which works for both, github.com and GitHub Enterprise (GHE) installations. For GHE the available APIs (and probably the event payloads) differ per version, so it would be nice to know as the receiver of webhooks what I can do with them.