fix: wire validateTaskPayload into POST /github-webhook (closes #139) - #149
Open
Whiznificent wants to merge 1 commit into
Open
fix: wire validateTaskPayload into POST /github-webhook (closes #139)#149Whiznificent wants to merge 1 commit into
Whiznificent wants to merge 1 commit into
Conversation
…protocol#139) The strict zod-based TaskPayload validator already exists in src/middleware/validator.js but had no callers. This change wires it into the /github-webhook middleware chain between verifySignature and idempotencyMiddleware so that signature-valid but structurally invalid payloads (wrong types, unknown top-level keys) are rejected with a 400 response before reaching the idempotency dedupe or queue layer. Two new integration tests in test/webhook.test.js cover the end-to-end rejection path: - signature-valid but structurally invalid (non-integer PR number) -> 400 - signature-valid payload with an unknown top-level key -> 400 (injection guard) The /internal/webhooks/replay endpoint intentionally remains unvalidated; it reads already-validated payloads from the raw event store by Idempotency-Key.
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
1 participant
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.
closes #139
Summary
Wire the existing
validateTaskPayloadmiddleware intoPOST /github-webhookso that signature-valid but structurally invalid payloads are rejected with a 400 response before reaching the idempotency dedupe or queue layer.Changes
validateTaskPayloadand insert it into the/github-webhookExpress middleware chain betweenverifySignatureandidempotencyMiddleware. Replaced the previous single-line comment with a block that documents the full chain (rate-limit → signature verify → payload validate → idempotency dedupe → handler) and explains why validation comes before idempotency (malformed payloads get a consistent 400 each time instead of being silently cached as seen keys that later surface as confusing 409s). Added a one-line note on/internal/webhooks/replayexplaining that route is intentionally unvalidated because it reads already-validated payloads from the raw event store.github webhook rejects signature-valid but structurally invalid payloads with 400— sends a body with a valid HMAC signature butpull_request.numberset to a string instead of an integer and asserts the 400 + structureddetailsresponse.github webhook rejects payloads with unknown top-level keys (injection guard)— sends a body with an extra top-level property (evilProperty) and asserts the strict-schema rejection surfaces in the responsedetails.Verification
35 pass, 1 fail — the failing test is a pre-existing flake in this dev environment (default
enforceIdempotencyneeds Redis at runtime) that also fails onmainwithout my changes. Out of scope for #139; tracked separately.Notes
/internal/webhooks/replayis intentionally not affected; payloads there come from the pre-validated raw event store.zod-based validator.