feat(backend): Add event_attributes to Webhook type #6162
Conversation
🦋 Changeset detectedLatest commit: 728110a The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
|
@jaredpiedt is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes introduce a new field, Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/backend/src/webhooks.ts (1)
83-85: Critical: Fix inverted signature verification logic.The signature verification logic is inverted. Currently, it throws an error when the constructed signature IS found in the svix signature list, which means it rejects valid signatures and accepts invalid ones. This is a critical security vulnerability.
Apply this fix:
- if (svixSignature.split(' ').includes(constructedSignature)) { + if (!svixSignature.split(' ').includes(constructedSignature)) { return errorThrower.throw('Incoming webhook does not have a valid signature'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/sixty-regions-camp.md(1 hunks)packages/backend/src/api/resources/Webhooks.ts(1 hunks)packages/backend/src/webhooks.ts(1 hunks)
🔇 Additional comments (3)
packages/backend/src/api/resources/Webhooks.ts (2)
16-21: Well-structured type definition for HTTP request metadata.The
WebhookEventAttributestype properly encapsulates HTTP request information with appropriate nesting underhttp_request.
23-23: Verify backward compatibility for required event_attributes property.Adding
event_attributesas a required property to theWebhooktype could be a breaking change if existing webhook payloads don't include this field. Consider making it optional initially or ensure all webhook sources provide this field.#!/bin/bash # Description: Search for existing Webhook type usage to assess potential breaking changes # Expected: Find usages that might be affected by the new required property echo "Searching for Webhook type usage patterns..." rg -A 3 -B 3 "Webhook<" --type ts echo -e "\nSearching for webhook payload parsing..." rg -A 5 -B 5 "JSON\.parse.*payload|payload.*JSON\.parse" --type ts echo -e "\nSearching for webhook event creation/construction..." rg -A 5 -B 5 "type.*event.*object.*data" --type ts.changeset/sixty-regions-camp.md (1)
1-6: Changeset documentation looks good.The changeset correctly documents this as a patch-level change with a clear description.
packages/backend/src/webhooks.ts
Outdated
| type: payload.type, | ||
| object: 'event', | ||
| data: payload.data, | ||
| event_attributes: payload.event_attributes, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety for event_attributes extraction.
The code assumes payload.event_attributes exists, but it should handle cases where this property might be missing from older webhook payloads to maintain backward compatibility.
Apply this diff to add null safety:
- event_attributes: payload.event_attributes,
+ event_attributes: payload.event_attributes || { http_request: { client_ip: '', user_agent: '' } },Or make the property optional in the type definition if it's acceptable for it to be undefined.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| event_attributes: payload.event_attributes, | |
| event_attributes: payload.event_attributes || { http_request: { client_ip: '', user_agent: '' } }, |
🤖 Prompt for AI Agents
In packages/backend/src/webhooks.ts at line 93, the code accesses
payload.event_attributes without checking if it exists, which can cause errors
with older webhook payloads. To fix this, add a null check or use optional
chaining when extracting event_attributes to safely handle cases where it might
be undefined. Alternatively, update the type definition to make event_attributes
optional if that aligns with the expected data structure.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
|
@jaredpiedt Can you ensure we also have an associated test case here? You can use the existing test cases as a reference. |
@brkalow definitely, I'll get that added today |
|
@brkalow test case has been added |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/backend/src/__tests__/webhooks.test.ts (1)
221-250: Well-structured test case for event_attributes functionality.The test correctly validates the new
event_attributesproperty and follows the established patterns in the test suite. The test structure, assertions, and usage of the helper function are appropriate.Consider adding a few more test cases to ensure comprehensive coverage:
- Test with empty event_attributes: Verify behavior when
event_attributesis an empty object or missing entirely- Test with partial http_request data: Test cases where only
client_iporuser_agentis present- Test with additional nested properties: If
event_attributesmight contain other properties beyondhttp_requestit('should handle missing event_attributes gracefully', async () => { const clerkPayload = JSON.stringify({ type: 'user.created', data: { id: 'user_123', email: '[email protected]' }, // event_attributes intentionally omitted }); const svixId = 'msg_123'; const svixTimestamp = (Date.now() / 1000).toString(); const validSignature = createValidSignature(svixId, svixTimestamp, clerkPayload); const mockRequest = new Request('https://clerk.com/webhooks', { method: 'POST', body: clerkPayload, headers: new Headers({ 'svix-id': svixId, 'svix-timestamp': svixTimestamp, 'svix-signature': validSignature, }), }); const result = await verifyWebhook(mockRequest, { signingSecret: mockSecret }); expect(result).toHaveProperty('type', 'user.created'); expect(result).not.toHaveProperty('event_attributes'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/backend/src/__tests__/webhooks.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
`**/*.{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/**/*.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
`**/{__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
`**/*.{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
`**/*.{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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor 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
Useconst assertionsfor literal types:as const
Usesatisfiesoperator 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
Preferunknownoveranyfor performance
Use type-only imports:import type { ... }
Use branded types for domain safety
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor 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
`**/__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
`**/*.ts`: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
**/*.ts: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
packages/backend/src/__tests__/webhooks.test.ts
🧠 Learnings (2)
📓 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: 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.
packages/backend/src/__tests__/webhooks.test.ts (2)
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/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
3 participants
Description
This adds the
event_attributesproperty to webhook events.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit