Conversation
…yload flexible Use rawBody buffer in DosOrgSyncWebhookController for exact signature matching, and update DosOrgSyncDto to support flexible ecosystem and ping event payloads. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_989a106e-67a1-4f0d-b4a0-c8af570ba3f0) |
There was a problem hiding this comment.
Code Review
This pull request updates the webhook signature verification in DosOrgSyncWebhookController to prioritize (req as any).rawBody if available, and relaxes several validation constraints in DosOrgSyncDto and DosOrgSyncDataDto (making org_id, id, and data optional, and changing event to a string). Feedback suggests using req.body instead of payload in the fallback stringification to ensure the stringified payload is as close to the original request as possible, avoiding issues with stripped or transformed properties.
| const rawBody = | ||
| (req as any).rawBody || | ||
| (typeof req.body === 'string' ? req.body : JSON.stringify(payload)); |
There was a problem hiding this comment.
When falling back to stringifying the parsed body, using req.body is more reliable than payload. payload is the validated and transformed DTO instance, which may have properties stripped (if whitelist: true is enabled in the global ValidationPipe) or transformed by class-transformer. Using req.body ensures that the fallback stringification is as close to the original request payload as possible.
| const rawBody = | |
| (req as any).rawBody || | |
| (typeof req.body === 'string' ? req.body : JSON.stringify(payload)); | |
| const rawBody = | |
| (req as any).rawBody || | |
| (typeof req.body === 'string' ? req.body : JSON.stringify(req.body)); |
What kind of change does this PR introduce?
Bug fix & Webhook verification optimization
Why was this change needed?
(req as any).rawBodybuffer directly inDosOrgSyncWebhookControllerto ensure the exact incoming byte stream is verified againstx-dos-signature, preventing HMAC mismatch caused by JSON re-serialization differences.DosOrgSyncDtoandDosOrgSyncDataDtowith optional fields to support health/ping checks and diverse ecosystem event schemas.Other information:
Checklist:
Note
Medium Risk
Changes webhook HMAC input and weakens request validation on a signed endpoint; correctness depends on
rawBodybeing populated by upstream middleware when present.Overview
Fixes DOS org-sync webhook signature checks by preferring
(req as any).rawBodywhen computing the HMAC, so verification uses the original request bytes instead ofJSON.stringify(payload)(which can diverge from what the sender signed).Relaxes
DosOrgSyncDto/DosOrgSyncDataDtovalidation:org_idand top-leveldataare optional, adds optionalid, and typeseventas a plainstringinstead ofDosSyncEvent—so health/ping payloads and varied event shapes pass validation while unhandled events still fall through to the controller’signoredpath.Reviewed by Cursor Bugbot for commit 6422c52. Bugbot is set up for automated code reviews on this repo. Configure here.