[Schema Inaccuracy] WebHooks lacking discriminator based on X-Github-Event header

Open
#2,207 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
38/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
openapi
Domain
api

Research direction

Start at the webhooks.branch-protection-rule-created.post.parameters definition in the OpenAPI YAML shown in the issue, then compare the X-Github-Event header with the webhook event name. Add the event value as an enum and apply the same rule to the relevant webhook definitions; done means generated handlers can identify the event without validating only the request body.

Written by the indexing model from the issue text.

Description

enhancement feature fundamentals-and-support P4 shield

Schema Inaccuracy

While working on a generated webhook handler I noticed that the spec doesn't include a way to communicate which value X-Github-Event should hold. Currently this is the spec:

webhooks:
  branch-protection-rule-created:
    post:
      summary: |-
        This event occurs when there is activity relating to branch protection rules. For more information, see "[About protected branches](https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches)." For information about the APIs to manage branch protection rules, see [the GraphQL documentation](https://docs.github.com/graphql/reference/objects#branchprotectionrule) or "[Branch protection](https://docs.github.com/rest/branches/branch-protection)" in the REST API documentation.

        To subscribe to this event, a GitHub App must have at least read-level access for the "Administration" repository permission
      description: A branch protection rule was created.
      operationId: branch-protection-rule/created
      externalDocs:
        url: https://docs.github.com/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#branch-protection-rule
      parameters:
      - name: User-Agent
        in: header
        example: GitHub-Hookshot/123abc
        schema:
          type: string
      - name: X-Github-Hook-Id
        in: header
        example: 12312312
        schema:
          type: string
      - name: X-Github-Event
        in: header
        example: issues
        schema:
          type: string
      - name: X-Github-Hook-Installation-Target-Id
        in: header
        example: 123123
        schema:
          type: string
      - name: X-Github-Hook-Installation-Target-Type
        in: header
        example: repository
        schema:
          type: string
      - name: X-GitHub-Delivery
        in: header
        example: 0b989ba4-242f-11e5-81e1-c7b6966d2516
        schema:
          type: string
      - name: X-Hub-Signature-256
        in: header
        example: sha256=6dcb09b5b57875f334f61aebed695e2e4193db5e
        schema:
          type: string
      requestBody:
        required: true
        content:
          application/json:
            schema:
              "$ref": "#/components/schemas/webhook-branch-protection-rule-created"
      responses:
        '200':
          description: Return a 200 status to indicate that the data was received
            successfully
      x-github:
        githubCloudOnly: false
        category: webhooks
        subcategory: branch-protection-rule
        supported-webhook-types:
        - repository
        - organization
        - app

Expected

This is what I would have expected:

webhooks:
  branch-protection-rule-created:
    post:
      summary: |-
        This event occurs when there is activity relating to branch protection rules. For more information, see "[About protected branches](https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches)." For information about the APIs to manage branch protection rules, see [the GraphQL documentation](https://docs.github.com/graphql/reference/objects#branchprotectionrule) or "[Branch protection](https://docs.github.com/rest/branches/branch-protection)" in the REST API documentation.

        To subscribe to this event, a GitHub App must have at least read-level access for the "Administration" repository permission
      description: A branch protection rule was created.
      operationId: branch-protection-rule/created
      externalDocs:
        url: https://docs.github.com/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#branch-protection-rule
      parameters:
      - name: User-Agent
        in: header
        example: GitHub-Hookshot/123abc
        schema:
          type: string
      - name: X-Github-Hook-Id
        in: header
        example: 12312312
        schema:
          type: string
      - name: X-Github-Event
        in: header
        example: issues
        schema:
          type: string
          enum:
            - branch-protection-rule
      - name: X-Github-Hook-Installation-Target-Id
        in: header
        example: 123123
        schema:
          type: string
      - name: X-Github-Hook-Installation-Target-Type
        in: header
        example: repository
        schema:
          type: string
      - name: X-GitHub-Delivery
        in: header
        example: 0b989ba4-242f-11e5-81e1-c7b6966d2516
        schema:
          type: string
      - name: X-Hub-Signature-256
        in: header
        example: sha256=6dcb09b5b57875f334f61aebed695e2e4193db5e
        schema:
          type: string
      requestBody:
        required: true
        content:
          application/json:
            schema:
              "$ref": "#/components/schemas/webhook-branch-protection-rule-created"
      responses:
        '200':
          description: Return a 200 status to indicate that the data was received
            successfully
      x-github:
        githubCloudOnly: false
        category: webhooks
        subcategory: branch-protection-rule
        supported-webhook-types:
        - repository
        - organization
        - app

Notes

In theory it shouldn't matter that you don't specify which value the X-Github-Event holds. But by defining it for each webhook event, there is no need to validate each possible webhook schema against the request body. On top of that this will guarantee that we don't accidentally mix up two different webhooks. While in theory we could get this from the post.operationId, but that assumes we know we should ignore the / in there and everything after it, which isn't the case for all specs out there. Plus is uses dashes between words and not underscores so using that one on one also doesn't

Reproduction Steps

n/a

Dominant language
No language data
Stars
1.6k
Forks
342
Avg merge
3h 33m
Merged PRs (30d)
51

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from github/rest-api-description

All issues in github/rest-api-description

Similar issues

More Backend & API Design issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.