You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
verifyWebhook() accepts an optional toleranceSec, but does not validate it. With NaN or Infinity, the timestamp age comparison does not reject stale signatures, effectively disabling the freshness check.
Expected behavior
Require toleranceSec to be a finite, non-negative safe integer in seconds. Reject invalid values with a clear error, while preserving the default of 300 seconds and allowing 0 for callers who want no clock-skew window.
Tests
Invalid values (NaN, Infinity, negative values, and fractions) are rejected.
0, a valid custom tolerance, and the default work as documented.
Expired and future-dated signatures remain rejected outside the configured tolerance.
Update the webhook verification docs and add an appropriate @chatcops/server changeset.
@anurag629 I'd like to take this one if it's free (@piyusshhjangid, you filed it, so say if you want it yourself). I have one free slot (ogcops#362 is in review). Could you assign it to me? I'll start once it shows as assigned.
Hi @anurag629, if this issue makes sense, could you please add the appropriate label and assign it to me? I’d be happy to work on it after #504 is merged.
Sorry, @Saifjafri212213—I opened this issue because I’d like to take it on myself.
@Saifjafri212213 yours, and labeled. piyusshhjangid filed it but has one slot and took #519.
Confirmed: toleranceSec is read at packages/server/src/webhook.ts:202 with no check, and Math.abs(nowSec - timestamp) > toleranceSec at line 215 is always false for NaN, so a stale signature passes. Infinity turns the check off the same way.
verifyWebhook is a function called on each request, not a config option, so throw a RangeError for a bad value instead of returning false. A wrong setting should be loud, not look like a bad signature. The issue's rule is right: a finite, non-negative whole number, with 0 allowed. Tests go in packages/server/tests/webhook.test.ts. Mention it where toleranceSec is described at website/src/content/docs/server/setup.mdx:380, and add a patch changeset for @chatcops/server.
Activity
Saifjafri212213 commentedon Oct 31, 2026
@anurag629 I'd like to take this one if it's free (@piyusshhjangid, you filed it, so say if you want it yourself). I have one free slot (ogcops#362 is in review). Could you assign it to me? I'll start once it shows as assigned.
piyusshhjangid commentedon Oct 31, 2026
Hi @anurag629, if this issue makes sense, could you please add the appropriate label and assign it to me? I’d be happy to work on it after #504 is merged.
Sorry, @Saifjafri212213—I opened this issue because I’d like to take it on myself.
anurag629 commentedon Oct 31, 2026
@Saifjafri212213 yours, and labeled. piyusshhjangid filed it but has one slot and took #519.
Confirmed:
toleranceSecis read atpackages/server/src/webhook.ts:202with no check, andMath.abs(nowSec - timestamp) > toleranceSecat line 215 is always false forNaN, so a stale signature passes.Infinityturns the check off the same way.verifyWebhookis a function called on each request, not a config option, so throw aRangeErrorfor a bad value instead of returningfalse. A wrong setting should be loud, not look like a bad signature. The issue's rule is right: a finite, non-negative whole number, with0allowed. Tests go inpackages/server/tests/webhook.test.ts. Mention it wheretoleranceSecis described atwebsite/src/content/docs/server/setup.mdx:380, and add a patch changeset for@chatcops/server.fix(server): reject a toleranceSec that would switch the webhook age …