Skip to content

fix(backend): migrate webhooks to use canonical timingsafekey comparison - #1747

Merged
LucasSantana-Dev merged 2 commits into
mainfrom
fix/1699-timingsafe-webhooks
Jul 9, 2026
Merged

LucasSantana-Dev merged 2 commits into
mainfrom
fix/1699-timingsafe-webhooks

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented Jul 9, 2026 •

Copy link
Copy Markdown
Owner

fix: #1699

Replace two call sites of custom safeEqualString() with timingSafeKeyCompare() to match the canonical constant-time comparison used in other internal routes (serviceAnnounce.ts, internalNotify.ts, serviceGuild.ts). Removes now-unused safeEqualString function definition and its timingSafeEqual import.


Summary by cubic

Standardizes webhook auth to use timingSafeKeyCompare for constant-time key checks in Top.gg and internal notify endpoints, aligning with other internal routes and addressing #1699. Removes the custom safeEqualString helper and the timingSafeEqual import.

Written for commit d0c3fe2. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved webhook request verification for better protection against invalid or tampered requests.
    • Kept vote streak and duplicate-vote behavior unchanged while making the handling more reliable.

replace two call sites of custom safeEqualString() with timingSafeKeyCompare()
to match the canonical constant-time comparison used in other internal routes
(serviceAnnounce.ts, internalNotify.ts, serviceGuild.ts). removes now-unused
safeEqualString function definition and its timingSafeEqual import.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Webhook token verification in webhooks.ts is refactored to use a shared timingSafeKeyCompare utility instead of a local safeEqualString helper combined with node:crypto's timingSafeEqual. Vote streak computation logic is reformatted without semantic changes.

Changes

Webhook Auth Refactor

Layer / File(s) Summary
Constant-time key comparison
packages/backend/src/routes/webhooks.ts
Imports timingSafeKeyCompare, removes the local safeEqualString helper, and updates verifyTopggAuth and verifyInternalKey to use the new comparison function for TOPGG_AUTH_TOKEN and LUCKY_NOTIFY_API_KEY checks.
Vote streak formatting
packages/backend/src/routes/webhooks.ts
Reformats the expiredStreak computation in readVoteState and the newStreak assignment in recordVote into multi-line conditionals, preserving existing streak semantics.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: switching backend webhooks to the canonical timingSafeKeyCompare approach.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1699-timingsafe-webhooks

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Failed to generate code suggestions for PR

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Auto-approved: Replaces custom safeEqualString with canonical timingSafeKeyCompare for webhook auth, matching other internal routes. Removes unused code. Low risk.

Re-trigger cubic

@github-actions

github-actions Bot commented Jul 9, 2026 •

Copy link
Copy Markdown
Warnings
⚠️

User-facing change without a CHANGELOG.md update. Add a line under ## [Unreleased] if this should appear in release notes. (Or apply the skip-changelog label if this PR does not affect end users.)

Generated by 🚫 dangerJS against d0c3fe2

@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@LucasSantana-Dev
LucasSantana-Dev merged commit eef5aee into main Jul 9, 2026
44 checks passed
@LucasSantana-Dev
LucasSantana-Dev deleted the fix/1699-timingsafe-webhooks branch July 9, 2026 18:06
LucasSantana-Dev added a commit that referenced this pull request Jul 10, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>2.34.0</summary>

##
[2.34.0](v2.33.1...v2.34.0)
(2026-07-10)


### Features

* **twitch:** use Promise.allSettled for per-event subscription error
logging ([#1749](#1749))
([6691305](6691305))


### Bug Fixes

* [#1699](#1699)
([eef5aee](eef5aee))
* **backend:** migrate webhooks to use canonical timingsafekey
comparison
([#1747](#1747))
([eef5aee](eef5aee))
* **backend:** wrap lastfm routes with asynchandler
([#1726](#1726))
([ce51d86](ce51d86))
* **batch-move:** graceful attachment-fetch degradation + mid-loop
client re-check
([#1750](#1750))
([f21a0ce](f21a0ce))
* **bot:** approve @discordjs/opus install script — P0 music playback
outage ([#1757](#1757))
([9d894e4](9d894e4))
* **ci:** add missing packages field to pnpm-workspace.yaml
([#1760](#1760))
([a4c585d](a4c585d))
* **ci:** remove pnpm shim from bundle-size workflow
([#1759](#1759))
([eaf676f](eaf676f))
* **deploy:** increase validation timeout to 10min
([#1743](#1743))
([07891ec](07891ec))
* **docker:** copy+chown [@prisma](https://github.com/prisma) engines in
production-backend — P0 deploy pipeline blocker
([#1758](#1758))
([a70d0e8](a70d0e8))
* eliminate mock state pollution in bot tests and remove resetMocks
config ([#1741](#1741))
([2e5fd94](2e5fd94))
* **frontend:** prevent state updates after unmount
([#1748](#1748))
([f4e7c45](f4e7c45))
* pin file-type to resolve CI flake
[#1740](#1740)
([#1753](#1753))
([6b8e527](6b8e527))
* reduce Jest maxWorkers and add DB pool config for test stability
([#1751](#1751))
([cfead33](cfead33))
* use fake timers in ReminderService.spec to prevent race condition
([#1745](#1745))
([ba2908c](ba2908c))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(backend): unify internal-endpoint key comparison on timingSafeKeyCompare

1 participant