Skip to content

Conversation

@brkalow
Copy link
Member

@brkalow brkalow commented Jul 3, 2025

Description

Aligning on the standardwebhooks library for webhook verification.

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Refactor

    • Improved webhook verification by adopting a standardized verification method for greater consistency and security.
  • Tests

    • Enhanced test coverage for webhook signature validation with additional scenarios and refined error handling.
  • Chores

    • Added a new dependency to support the updated webhook verification process.

@changeset-bot
Copy link

changeset-bot bot commented Jul 3, 2025

🦋 Changeset detected

Latest commit: f414767

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@clerk/backend Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/remix Patch
@clerk/tanstack-react-start Patch
@clerk/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jul 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2025 8:07pm

@brkalow brkalow changed the title chore(backend): refactor verifyWebhook() to use standardwebhooks library chore(backend): Refactor verifyWebhook() to use standardwebhooks library Jul 3, 2025
@brkalow brkalow marked this pull request as ready for review July 3, 2025 18:10
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 3, 2025

📝 Walkthrough

Walkthrough

This change refactors the webhook verification logic in the @clerk/backend package by replacing the manual Svix-based verification with the standardwebhooks library. It introduces a helper function to map Svix-specific headers to standard webhook headers and updates the verifyWebhook function to use the Webhook class from standardwebhooks for signature verification. The test suite is updated to generate valid signatures using the new library, and additional tests are added to improve coverage of signature validation and error handling. The standardwebhooks dependency is added to the package configuration.

Possibly related PRs

  • test(backend): Remove mocked svix module #6227: Focuses on replacing manual Svix webhook verification with the standardwebhooks library and updates related tests, sharing a direct connection through changes in webhook verification and testing logic.

Suggested reviewers

  • jacekradko

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa76abb and f414767.

📒 Files selected for processing (1)
  • packages/backend/src/webhooks.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's conf...

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Use Prettier for consistent code formatting
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Validate all inputs and sanitize outputs
Implement proper logging with different levels

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
`packages/**/*.ts`: TypeScript is required for all packages

packages/**/*.ts: TypeScript is required for all packages

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
`packages/**/*.{ts,tsx,d.ts}`: Packages should export TypeScript types alongside runtime code

packages/**/*.{ts,tsx,d.ts}: Packages should export TypeScript types alongside runtime code

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
`**/*.{ts,tsx}`: Use proper TypeScript error types

**/*.{ts,tsx}: Use proper TypeScript error types

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
`**/*.{ts,tsx}`: Always define explicit return types for functions, especially p...

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Define custom error classes for domain-specific errors
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining and nullish coalescing for safe property access
Let TypeScript infer types when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use readonly arrays and objects for immutability
Use immutable update patterns (spread, etc.) for objects and arrays
Use lazy loading for large types
Prefer unknown over any for performance
Use type-only imports: import type { ... }
Use branded types for domain safety
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints in TypeScript generics
No unused type parameters in generics
Proper use of utility types instead of manual type construction
Type-only imports where possible for performance
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
packages/backend/src/webhooks.ts (11)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper event types for handlers
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{ts,tsx} : Use proper TypeScript error types
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Avoid `any` type - prefer `unknown` when type is uncertain, then narrow with type guards
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type definitions for props and state
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Always define explicit return types for functions, especially public APIs
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper use of utility types instead of manual type construction
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : No `any` types without justification
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type guards for conditional rendering
Learnt from: wobsoriano
PR: clerk/javascript#6123
File: packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts:63-75
Timestamp: 2025-06-16T01:27:54.563Z
Learning: In packages/nextjs/src/server/data/getAuthDataFromRequest.ts, the tokenType behavior on mismatch is intentionally different between array and single acceptsToken values: when acceptsToken is an array and the token type doesn't match any in the array, tokenType returns null; when acceptsToken is a single value and the token type doesn't match, tokenType returns the requested single value. This design aligns with developer intent and provides a more ergonomic API for common use cases.
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/backend/src/webhooks.ts (5)

3-3: LGTM: Clean import statement.

The import of Webhook from standardwebhooks is properly structured and aligns with the refactoring objective.


17-25: LGTM: Well-organized constants with clear naming.

The separation of standard webhook headers and Svix headers with descriptive comments makes the code more maintainable and the mapping logic clearer.


32-51: LGTM: Solid helper function with proper validation.

The createStandardWebhookHeaders function correctly maps Svix headers to standard webhook headers with proper trimming and null checking. The explicit return type and defensive programming approach are excellent.


94-113: LGTM: Excellent defensive programming with detailed error reporting.

The explicit validation of required headers with individual checking and descriptive error messages is a significant improvement over the previous implementation. This provides better debugging information when webhooks fail.


117-135: LGTM: Secure and standardized webhook verification.

The refactoring to use the standardwebhooks library provides several benefits:

  • Constant-time signature comparison for security
  • Standardized webhook handling approach
  • Built-in timestamp verification (as mentioned in PR comments)
  • Proper error handling with descriptive messages

The type casting approach is justified given the implicit guarantee from signature verification.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/backend/src/__tests__/webhooks.test.ts (1)

14-19: Add validation for timestamp parsing.

The parseInt call could return NaN if the timestamp is malformed, leading to an Invalid Date.

 const createValidSignature = (id: string, timestamp: string, body: string) => {
   const webhook = new Webhook(mockSecret);
+  const timestampNum = parseInt(timestamp);
+  if (isNaN(timestampNum)) {
+    throw new Error(`Invalid timestamp: ${timestamp}`);
+  }
   // Create a signature using the Standard Webhooks library
-  return webhook.sign(id, new Date(parseInt(timestamp) * 1000), body);
+  return webhook.sign(id, new Date(timestampNum * 1000), body);
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c642d42 and 11ca202.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • .changeset/chubby-tires-end.md (1 hunks)
  • packages/backend/package.json (1 hunks)
  • packages/backend/src/__tests__/webhooks.test.ts (3 hunks)
  • packages/backend/src/webhooks.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
`packages/*/package.json`: All packages must be published under the @clerk namespace

packages/*/package.json: All packages must be published under the @clerk namespace

📄 Source: CodeRabbit Inference Engine (.cursor/rules/global.mdc)

List of files the instruction was applied to:

  • packages/backend/package.json
`packages/*/package.json`: All publishable packages must be located in the 'pack...

packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.

📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

List of files the instruction was applied to:

  • packages/backend/package.json
`**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's conf...

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Use Prettier for consistent code formatting
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Validate all inputs and sanitize outputs
Implement proper logging with different levels

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
  • packages/backend/src/__tests__/webhooks.test.ts
`packages/**/*.ts`: TypeScript is required for all packages

packages/**/*.ts: TypeScript is required for all packages

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
  • packages/backend/src/__tests__/webhooks.test.ts
`packages/**/*.{ts,tsx,d.ts}`: Packages should export TypeScript types alongside runtime code

packages/**/*.{ts,tsx,d.ts}: Packages should export TypeScript types alongside runtime code

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
  • packages/backend/src/__tests__/webhooks.test.ts
`**/*.{ts,tsx}`: Use proper TypeScript error types

**/*.{ts,tsx}: Use proper TypeScript error types

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
  • packages/backend/src/__tests__/webhooks.test.ts
`**/*.{ts,tsx}`: Always define explicit return types for functions, especially p...

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Define custom error classes for domain-specific errors
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining and nullish coalescing for safe property access
Let TypeScript infer types when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use readonly arrays and objects for immutability
Use immutable update patterns (spread, etc.) for objects and arrays
Use lazy loading for large types
Prefer unknown over any for performance
Use type-only imports: import type { ... }
Use branded types for domain safety
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints in TypeScript generics
No unused type parameters in generics
Proper use of utility types instead of manual type construction
Type-only imports where possible for performance
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)

List of files the instruction was applied to:

  • packages/backend/src/webhooks.ts
  • packages/backend/src/__tests__/webhooks.test.ts
`**/{__tests__,**/__tests__}/**/*.{js,jsx,ts,tsx}`: Test files should be co-located with source files or in `__tests__` directories

**/{__tests__,**/__tests__}/**/*.{js,jsx,ts,tsx}: Test files should be co-located with source files or in __tests__ directories

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
`**/*.{test,spec}.{js,ts,tsx}`: Unit tests should use Jest or Vitest as the test runner.

**/*.{test,spec}.{js,ts,tsx}: Unit tests should use Jest or Vitest as the test runner.

📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
`**/__tests__/**/*.{ts,tsx}`: Use Vitest for type-safe testing in TypeScript Cre...

**/__tests__/**/*.{ts,tsx}: Use Vitest for type-safe testing in TypeScript
Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces in tests

📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
.changeset/chubby-tires-end.md (5)
Learnt from: wobsoriano
PR: clerk/javascript#6099
File: packages/backend/src/api/endpoints/IdPOAuthAccessTokenApi.ts:7-14
Timestamp: 2025-06-10T20:38:08.982Z
Learning: Methods in `packages/backend/src/api/endpoints` (e.g., `IdPOAuthAccessTokenApi.verifySecret`) are currently not exposed publicly, so renaming them does not constitute a breaking change.
Learnt from: wobsoriano
PR: clerk/javascript#6163
File: packages/backend/src/api/endpoints/APIKeysApi.ts:60-70
Timestamp: 2025-06-20T17:44:17.570Z
Learning: The Clerk codebase uses POST method for API key update operations instead of the typical PATCH method, as clarified by the maintainer wobsoriano.
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
packages/backend/package.json (8)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace on npm.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/global.mdc:0-0
Timestamp: 2025-06-30T10:30:13.397Z
Learning: Applies to packages/*/package.json : All packages must be published under the @clerk namespace
Learnt from: dstaley
PR: clerk/javascript#6100
File: packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx:121-124
Timestamp: 2025-06-16T17:08:58.414Z
Learning: The @clerk/clerk-js package only supports browsers released in the last two years (since May 8, 2023), so modern CSS features like color-mix() are fully supported across all target browsers without requiring fallbacks.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to packages/**/*.ts : TypeScript is required for all packages
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: wobsoriano
PR: clerk/javascript#6229
File: packages/backend/src/api/endpoints/MachineTokensApi.ts:47-89
Timestamp: 2025-07-01T15:20:41.834Z
Learning: In the Clerk JavaScript repository, for the MachineTokensApi class (packages/backend/src/api/endpoints/MachineTokensApi.ts), the maintainers prefer to rely on TypeScript types and readable property names for API documentation rather than JSDoc comments.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/*/tsup.config.{js,ts} : TypeScript compilation and bundling must use tsup.
packages/backend/src/webhooks.ts (1)
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
packages/backend/src/__tests__/webhooks.test.ts (11)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Implement proper mock types that match interfaces in tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use branded types for test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: wobsoriano
PR: clerk/javascript#6099
File: packages/backend/src/api/endpoints/IdPOAuthAccessTokenApi.ts:7-14
Timestamp: 2025-06-10T20:38:08.982Z
Learning: Methods in `packages/backend/src/api/endpoints` (e.g., `IdPOAuthAccessTokenApi.verifySecret`) are currently not exposed publicly, so renaming them does not constitute a breaking change.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Create type-safe test builders/factories
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use Vitest for type-safe testing in TypeScript
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test assertions
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Jest for unit testing, Vitest for some packages
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Test authentication flows end-to-end
🔇 Additional comments (4)
packages/backend/package.json (1)

114-114: standardwebhooks dependency verified

  • The latest published version on npm is 1.0.0.
  • Specifying "^1.0.0" correctly allows future minor and patch updates while preserving API stability.

✅ Approved and ready to merge.

.changeset/chubby-tires-end.md (1)

1-5: LGTM!

The changeset correctly marks this as a patch since it's an internal refactoring without API changes. The description clearly explains the change.

packages/backend/src/__tests__/webhooks.test.ts (1)

84-189: Excellent test coverage additions!

The new tests comprehensively cover:

  • Valid and invalid signature scenarios
  • Multiple signatures in headers
  • Backward compatibility without version prefixes
  • Standard Webhooks specification compliance
  • Realistic Clerk webhook payloads

This significantly improves the test suite's robustness.

packages/backend/src/webhooks.ts (1)

31-53: Header mapping implementation looks good.

The function correctly maps Svix headers to Standard Webhooks format while maintaining backward compatibility.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jul 3, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@6252

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@6252

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@6252

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@6252

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@6252

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@6252

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@6252

@clerk/clerk-expo

npm i https://pkg.pr.new/@clerk/clerk-expo@6252

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@6252

@clerk/express

npm i https://pkg.pr.new/@clerk/express@6252

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@6252

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@6252

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@6252

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@6252

@clerk/clerk-react

npm i https://pkg.pr.new/@clerk/clerk-react@6252

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@6252

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@6252

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@6252

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@6252

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@6252

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@6252

@clerk/types

npm i https://pkg.pr.new/@clerk/types@6252

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@6252

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@6252

commit: f414767

@bratsos
Copy link
Member

bratsos commented Jul 3, 2025

❓ According to standardwebhooks (TIL), it's a good practice to verify the webhook timestamp, making sure it's within a reasonable timeframe to prevent replay attacks. Do you think we should implement this here as well, or add this in a follow-up PR?

@brkalow
Copy link
Member Author

brkalow commented Jul 3, 2025

According to standardwebhooks (TIL), it's a good practice to verify the webhook timestamp, making sure it's within a reasonable timeframe to prevent replay attacks. Do you think we should implement this here as well, or add this in a follow-up PR?

Good call! The standardwebhooks package takes care of this internally already 👍

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/backend/src/webhooks.ts (1)

125-137: Verify the type casting approach aligns with security practices.

The type casting on line 128 bypasses TypeScript's type safety. However, given the previous discussion about trusting the payload after signature verification from the webhook provider, this approach is reasonable.

The implementation correctly handles errors and provides descriptive error messages. The use of standardwebhooks library ensures proper signature verification with constant-time comparison.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11ca202 and aa76abb.

📒 Files selected for processing (2)
  • packages/backend/src/__tests__/webhooks.test.ts (3 hunks)
  • packages/backend/src/webhooks.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
`**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's conf...

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Use Prettier for consistent code formatting
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Validate all inputs and sanitize outputs
Implement proper logging with different levels

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
  • packages/backend/src/webhooks.ts
`packages/**/*.ts`: TypeScript is required for all packages

packages/**/*.ts: TypeScript is required for all packages

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
  • packages/backend/src/webhooks.ts
`**/{__tests__,**/__tests__}/**/*.{js,jsx,ts,tsx}`: Test files should be co-located with source files or in `__tests__` directories

**/{__tests__,**/__tests__}/**/*.{js,jsx,ts,tsx}: Test files should be co-located with source files or in __tests__ directories

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
`packages/**/*.{ts,tsx,d.ts}`: Packages should export TypeScript types alongside runtime code

packages/**/*.{ts,tsx,d.ts}: Packages should export TypeScript types alongside runtime code

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
  • packages/backend/src/webhooks.ts
`**/*.{ts,tsx}`: Use proper TypeScript error types

**/*.{ts,tsx}: Use proper TypeScript error types

📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
  • packages/backend/src/webhooks.ts
`**/*.{test,spec}.{js,ts,tsx}`: Unit tests should use Jest or Vitest as the test runner.

**/*.{test,spec}.{js,ts,tsx}: Unit tests should use Jest or Vitest as the test runner.

📄 Source: CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
`**/*.{ts,tsx}`: Always define explicit return types for functions, especially p...

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document public functions and APIs with JSDoc-style comments including @param, @returns, @throws, and @example
Define custom error classes for domain-specific errors
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining and nullish coalescing for safe property access
Let TypeScript infer types when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use readonly arrays and objects for immutability
Use immutable update patterns (spread, etc.) for objects and arrays
Use lazy loading for large types
Prefer unknown over any for performance
Use type-only imports: import type { ... }
Use branded types for domain safety
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints in TypeScript generics
No unused type parameters in generics
Proper use of utility types instead of manual type construction
Type-only imports where possible for performance
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
  • packages/backend/src/webhooks.ts
`**/__tests__/**/*.{ts,tsx}`: Use Vitest for type-safe testing in TypeScript Cre...

**/__tests__/**/*.{ts,tsx}: Use Vitest for type-safe testing in TypeScript
Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces in tests

📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)

List of files the instruction was applied to:

  • packages/backend/src/__tests__/webhooks.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/package.json : Framework packages must depend on '@clerk/clerk-js' for core functionality.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
packages/backend/src/__tests__/webhooks.test.ts (12)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use branded types for test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Implement proper mock types that match interfaces in tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Create type-safe test builders/factories
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use Vitest for type-safe testing in TypeScript
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test assertions
Learnt from: wobsoriano
PR: clerk/javascript#6099
File: packages/backend/src/api/endpoints/IdPOAuthAccessTokenApi.ts:7-14
Timestamp: 2025-06-10T20:38:08.982Z
Learning: Methods in `packages/backend/src/api/endpoints` (e.g., `IdPOAuthAccessTokenApi.verifySecret`) are currently not exposed publicly, so renaming them does not constitute a breaking change.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Include tests for all new features
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use Jest for unit testing, Vitest for some packages
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
packages/backend/src/webhooks.ts (11)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper event types for handlers
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{ts,tsx} : Use proper TypeScript error types
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Avoid `any` type - prefer `unknown` when type is uncertain, then narrow with type guards
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Always define explicit return types for functions, especially public APIs
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper use of utility types instead of manual type construction
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type definitions for props and state
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : No `any` types without justification
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper type guards for conditional rendering
Learnt from: wobsoriano
PR: clerk/javascript#6123
File: packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts:63-75
Timestamp: 2025-06-16T01:27:54.563Z
Learning: In packages/nextjs/src/server/data/getAuthDataFromRequest.ts, the tokenType behavior on mismatch is intentionally different between array and single acceptsToken values: when acceptsToken is an array and the token type doesn't match any in the array, tokenType returns null; when acceptsToken is a single value and the token type doesn't match, tokenType returns the requested single value. This design aligns with developer intent and provides a more ergonomic API for common use cases.
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
🪛 ESLint
packages/backend/src/webhooks.ts

[error] 27-27: 'REQUIRED_WEBHOOK_HEADERS' is assigned a value but never used. Allowed unused vars must match /^_/u.

(@typescript-eslint/no-unused-vars)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
packages/backend/src/webhooks.ts (4)

3-3: Good choice of standardized library.

The switch to standardwebhooks library provides better standardization and security with constant-time signature comparison.


34-53: Well-implemented header mapping function.

The createStandardWebhookHeaders function properly maps Svix headers to Standard Webhooks format with appropriate trimming and null-checking.


96-115: Excellent header validation with detailed error reporting.

The header validation logic properly trims whitespace and provides detailed error messages listing all missing headers. This is a significant improvement over the previous implementation.


56-56: Add explicit return type annotation.

The function lacks an explicit return type annotation, which is a TypeScript best practice for public APIs.

-export async function verifyWebhook(request: Request, options: VerifyWebhookOptions = {}): Promise<WebhookEvent> {
+export async function verifyWebhook(request: Request, options: VerifyWebhookOptions = {}): Promise<WebhookEvent> {

Wait, the return type is already present. Let me check again... Yes, it's there on line 87. This is not an issue.

packages/backend/src/__tests__/webhooks.test.ts (11)

1-1: Proper integration with standardwebhooks library.

The import of Webhook from standardwebhooks aligns with the refactored implementation and enables proper signature generation for testing.


7-7: Mock secret format updated correctly.

The mock secret now uses the whsec_ prefix with base64 encoding, which is the proper format expected by the standardwebhooks library.


14-19: Excellent helper function for signature generation.

The createValidSignature helper function properly uses the standardwebhooks library to generate valid signatures for testing, replacing the previous hardcoded approach.


30-30: Updated error messages reflect generic validation.

The test expectations now use generic "Missing required webhook headers" messages instead of Svix-specific errors, which aligns with the refactored validation logic.

Also applies to: 42-42, 58-58


61-82: Main validation test updated with dynamic signatures.

The core validation test now uses dynamically generated signatures instead of hardcoded ones, which is more robust and realistic.


84-103: Comprehensive test coverage for valid signatures.

This test case specifically verifies that valid signatures are accepted, providing clear test isolation and coverage.


105-122: Good coverage of invalid signature rejection.

The test properly verifies that invalid signatures are rejected with appropriate error messages.


124-144: Excellent test for multiple signature handling.

This test verifies that the webhook verification accepts requests when at least one signature in a space-separated list is valid, which is important for the Standard Webhooks specification.


146-166: Good backward compatibility testing.

The test ensures that signatures work regardless of version prefixes, maintaining compatibility with different signature formats.


168-189: Comprehensive Standard Webhooks compliance test.

This test uses a realistic Clerk webhook payload and verifies compliance with the Standard Webhooks specification, including the explicit signingSecret parameter.


191-219: Excellent edge case testing for whitespace handling.

These tests verify the enhanced header validation that treats empty strings and whitespace-only values as missing headers. This addresses the previous review comment about whitespace handling.

@brkalow brkalow merged commit c2f24da into main Jul 3, 2025
38 of 39 checks passed
@brkalow brkalow deleted the brk.chore/refactor-webhook-verify branch July 3, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants