[Schema Inaccuracy] WebHooks lacking discriminator based on X-Github-Event header
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
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/rest-api-description
-
feature
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
github/rest-api-description#7201 ·
-
feature
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
github/rest-api-description#7163 ·
-
feature
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
github/rest-api-description#7162 ·
-
feature
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
github/rest-api-description#7135 ·
-
feature
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
github/rest-api-description#7111 · 1 comment ·
All issues in github/rest-api-description
Similar issues
-
area/sessions comp/cron comp/gateway P2 sweeper:risk-message-delivery sweeper:risk-session-state type/bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
NousResearch/hermes-agent#118863 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
use-agent-os/agent-os#3312 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
-
needs-acceptance wg/data-plane-networking
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
vllm-project/semantic-router#4024 · 1 comment ·