Skip to content
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.

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

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

Open
WyriHaximus opened this issue Feb 14, 2023 · 0 comments
Labels
enhancement New feature or request P4

Comments

@WyriHaximus
Copy link

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

@becco becco added enhancement New feature or request P4 labels Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P4
Projects
None yet
Development

No branches or pull requests

2 participants