Add webhook event bus for async processing#35
Open
cb-srinaths wants to merge 1 commit into
Open
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughThe PR adds Chargebee webhook event-bus support, updates webhook event types and options, refactors webhook handling into shared dispatch and publish paths, adds a webhook processor factory, updates route wiring, and extends tests for publish and processor behavior. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/better-auth/src/webhook-handler.ts`:
- Around line 47-55: buildRequestValidator currently falls back to undefined
when only one of webhookUsername or webhookPassword is set, which disables Basic
Auth and leaves webhooks unauthenticated. Update buildRequestValidator in
webhook-handler.ts to fail closed by validating the webhook auth config up
front: if either credential is provided without the other, treat it as an error
instead of returning undefined. Keep basicAuthValidator only for the fully
configured case, and ensure the webhook-handler path rejects partial/empty auth
configuration rather than accepting requests without auth.
- Around line 298-323: The webhook handlers for handled and unhandled events
currently await eventBus.publish(event) and rely on the shared
handler.on("error") path, which converts every non-authentication failure into
200 OK. Update the event publishing flow in webhook-handler.ts so publish
failures are not acknowledged: handle errors from eventBus.publish(event)
explicitly in the HANDLED_EVENT_TYPES and "unhandled_event" listeners, and
ensure those queueing failures either rethrow or send a non-2xx response instead
of falling through to the generic 200 OK path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91ba27d8-d462-4cb3-b3f1-f26664642907
📒 Files selected for processing (6)
packages/better-auth/src/index.tspackages/better-auth/src/routes.tspackages/better-auth/src/types.tspackages/better-auth/src/webhook-handler.tspackages/better-auth/src/webhook-processor.tspackages/better-auth/test/webhook-queue.test.ts
Comment on lines
+47
to
+55
| function buildRequestValidator(options: ChargebeeOptions) { | ||
| return options.webhookUsername && options.webhookPassword | ||
| ? basicAuthValidator((username, password) => { | ||
| return ( | ||
| username === options.webhookUsername && | ||
| password === options.webhookPassword | ||
| ); | ||
| }) | ||
| : undefined; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed on partial webhook credentials.
If only one credential is configured, this returns undefined and disables Basic Auth entirely. Treat partial/empty webhook auth config as an error instead of accepting unauthenticated webhooks.
Proposed fix
function buildRequestValidator(options: ChargebeeOptions) {
- return options.webhookUsername && options.webhookPassword
- ? basicAuthValidator((username, password) => {
+ const hasWebhookAuthConfig =
+ options.webhookUsername !== undefined ||
+ options.webhookPassword !== undefined;
+
+ if (!hasWebhookAuthConfig) {
+ return undefined;
+ }
+
+ if (!options.webhookUsername || !options.webhookPassword) {
+ throw new Error(
+ "Both webhookUsername and webhookPassword must be configured for Chargebee webhook Basic Auth.",
+ );
+ }
+
+ return basicAuthValidator((username, password) => {
return (
username === options.webhookUsername &&
password === options.webhookPassword
);
- })
- : undefined;
+ });
}As per path instructions, this is a blocking security risk.
📝 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.
Suggested change
| function buildRequestValidator(options: ChargebeeOptions) { | |
| return options.webhookUsername && options.webhookPassword | |
| ? basicAuthValidator((username, password) => { | |
| return ( | |
| username === options.webhookUsername && | |
| password === options.webhookPassword | |
| ); | |
| }) | |
| : undefined; | |
| function buildRequestValidator(options: ChargebeeOptions) { | |
| const hasWebhookAuthConfig = | |
| options.webhookUsername !== undefined || | |
| options.webhookPassword !== undefined; | |
| if (!hasWebhookAuthConfig) { | |
| return undefined; | |
| } | |
| if (!options.webhookUsername || !options.webhookPassword) { | |
| throw new Error( | |
| "Both webhookUsername and webhookPassword must be configured for Chargebee webhook Basic Auth.", | |
| ); | |
| } | |
| return basicAuthValidator((username, password) => { | |
| return ( | |
| username === options.webhookUsername && | |
| password === options.webhookPassword | |
| ); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/better-auth/src/webhook-handler.ts` around lines 47 - 55,
buildRequestValidator currently falls back to undefined when only one of
webhookUsername or webhookPassword is set, which disables Basic Auth and leaves
webhooks unauthenticated. Update buildRequestValidator in webhook-handler.ts to
fail closed by validating the webhook auth config up front: if either credential
is provided without the other, treat it as an error instead of returning
undefined. Keep basicAuthValidator only for the fully configured case, and
ensure the webhook-handler path rejects partial/empty auth configuration rather
than accepting requests without auth.
Source: Path instructions
Comment on lines
+298
to
+323
| for (const eventType of HANDLED_EVENT_TYPES) { | ||
| handler.on(eventType, async ({ event, response }) => { | ||
| await eventBus.publish(event); | ||
| response?.status(200).send("OK"); | ||
| }); | ||
| } | ||
|
|
||
| // Forward all other events too, so the application receives every webhook. | ||
| handler.on("unhandled_event", async ({ event, response }) => { | ||
| await eventBus.publish(event); | ||
| response?.status(200).send("OK"); | ||
| }); | ||
|
|
||
| handler.on("error", (error: Error, { response }) => { | ||
| const webhookResponse = response as WebhookResponse | undefined; | ||
| if (error instanceof WebhookAuthenticationError) { | ||
| logger.warn( | ||
| `Webhook rejected: ${error.message}. Please verify webhookUsername and webhookPassword are correctly configured in your plugin options and that the webhook in Chargebee dashboard has matching Basic Auth credentials.`, | ||
| ); | ||
| webhookResponse?.status(401).send("Unauthorized"); | ||
| return; | ||
| } | ||
|
|
||
| // Log other errors and send 200 to prevent Chargebee retries | ||
| logger.error("Error processing webhook event:", error); | ||
| webhookResponse?.status(200).send("OK"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 'eventBus\.publish|Error processing webhook event|response: undefined' packages/better-auth/src packages/better-auth/testRepository: chargebee/js-framework-adapters
Length of output: 6411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant handler implementation and its tests with line numbers.
sed -n '240,330p' packages/better-auth/src/webhook-handler.ts
printf '\n--- TESTS ---\n'
sed -n '780,870p' packages/better-auth/test/webhook-events.test.ts
printf '\n--- QUEUE TESTS ---\n'
sed -n '80,150p' packages/better-auth/test/webhook-queue.test.tsRepository: chargebee/js-framework-adapters
Length of output: 7810
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the code paths that route webhook events and how errors are surfaced.
sed -n '1,120p' packages/better-auth/src/routes.ts
printf '\n--- WEBHOOK HANDLER ---\n'
sed -n '1,360p' packages/better-auth/src/webhook-handler.tsRepository: chargebee/js-framework-adapters
Length of output: 14178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether publish failures are expected to be caught by the shared error handler.
rg -n -C3 'handler\.on\("error"|publish\(event\)|eventBus\.publish|WebhookAuthenticationError|unhandled_event' packages/better-auth/src packages/better-auth/testRepository: chargebee/js-framework-adapters
Length of output: 8657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '240,330p' packages/better-auth/src/webhook-handler.ts
printf '\n--- TESTS ---\n'
sed -n '780,870p' packages/better-auth/test/webhook-events.test.ts
printf '\n--- QUEUE TESTS ---\n'
sed -n '80,150p' packages/better-auth/test/webhook-queue.test.tsRepository: chargebee/js-framework-adapters
Length of output: 7810
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the top of the file for the handler/event-emitter implementation.
sed -n '1,240p' packages/better-auth/src/webhook-handler.ts
printf '\n--- RELATED TYPES/USAGE ---\n'
rg -n -C3 'new .*Handler|create.*Webhook|on\("error"|on\("unhandled_event"|HANDLED_EVENT_TYPES' packages/better-auth/src packages/better-auth/testRepository: chargebee/js-framework-adapters
Length of output: 50023
Don't ACK queueing failures. eventBus.publish(event) in both webhook listeners can reject, but the shared "error" handler turns every non-authentication error into 200 OK. That drops events on transient queue failures; return a non-2xx or rethrow for publish errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/better-auth/src/webhook-handler.ts` around lines 298 - 323, The
webhook handlers for handled and unhandled events currently await
eventBus.publish(event) and rely on the shared handler.on("error") path, which
converts every non-authentication failure into 200 OK. Update the event
publishing flow in webhook-handler.ts so publish failures are not acknowledged:
handle errors from eventBus.publish(event) explicitly in the HANDLED_EVENT_TYPES
and "unhandled_event" listeners, and ensure those queueing failures either
rethrow or send a non-2xx response instead of falling through to the generic 200
OK path.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current implementation only supports synchronous processing of webhook events. If an application was to follow our best practices and save the incoming events to an event bus and then process it, all the default subscription handlers will have to be duplicated. This PR abstracts the processing of the webhook from the way the events are triggered. This will let apps implement an async flow: Chargebee -> App -> Queue -> Worker -> Process Better Auth related events.
Adds asynchronous Chargebee webhook processing via a new webhook event bus and processor. Webhooks can now be published to a queue for later handling, while preserving synchronous handling when no bus is configured. Also updates webhook event typing, centralizes dispatch logic, and adds tests for publish and processor flows.