fix(webhook): return 401 when signature header is missing#4278
Open
mixelburg wants to merge 4 commits intoDokploy:canaryfrom
Open
fix(webhook): return 401 when signature header is missing#4278mixelburg wants to merge 4 commits intoDokploy:canaryfrom
mixelburg wants to merge 4 commits intoDokploy:canaryfrom
Conversation
Comment on lines
26
to
+30
| const signature = req.headers["x-hub-signature-256"]; | ||
| if (!signature) { | ||
| res.status(401).json({ message: "Missing signature header" }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
string[] case unhandled after removing the cast
After the if (!signature) guard, TypeScript narrows signature to string | string[] — a non-empty array is truthy and passes the check. webhooks.verify expects a string, so if a request arrives with multiple x-hub-signature-256 headers (or TypeScript strict mode is enabled) this could cause a compilation error or a runtime failure. The previous as string cast silently suppressed this, but removing it without handling the array case may break the build.
Consider extracting a definite string value:
Suggested change
| const signature = req.headers["x-hub-signature-256"]; | |
| if (!signature) { | |
| res.status(401).json({ message: "Missing signature header" }); | |
| return; | |
| } | |
| const rawSig = req.headers["x-hub-signature-256"]; | |
| const signature = Array.isArray(rawSig) ? rawSig[0] : rawSig; | |
| if (!signature) { | |
| res.status(401).json({ message: "Missing signature header" }); | |
| return; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4275
When the signature header is absent, webhooks.verify() receives undefined and throws, causing a 500. Now we check for the header early and return 401 with a clear message.
Greptile Summary
Adds an early
!signatureguard to return 401 when thex-hub-signature-256header is absent, fixing the 500 thrown bywebhooks.verify(body, undefined). Also removes theas stringtype cast at theverifycall site.Confidence Score: 5/5
Safe to merge; the core fix is correct and the remaining note is minor.
The 401 early-return for a missing signature is clearly correct and consistent with the existing 401 for a bad signature. The only concern (handling
string[]header values) is pre-existing behaviour that was previously hidden by theas stringcast, and GitHub realistically never sends multiple signature headers.No files require special attention.
Reviews (1): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile