feat(webhooks): Wave native webhook receiver + per-business secret storage#113
feat(webhooks): Wave native webhook receiver + per-business secret storage#113chitcommit wants to merge 1 commit intomainfrom
Conversation
…orage
Adds Wave's native webhook flow to chittyfinance, scoped per
(tenant, business) pair to support tenants with multiple connected Wave
businesses. Wave webhook subscriptions are configured in the Wave
dashboard (no programmatic API exists per Wave's GraphQL schema), so
operators paste the per-business URL into the Wave Webhooks page and
seed the corresponding secret via the admin endpoint.
New endpoints (all under /api/webhooks/wave/:tenantId/:businessId):
- POST — receiver (HMAC verified, dedup'd, audit-logged)
- PUT /secret — admin secret-storage (service token auth)
- GET /secret/exists — admin existence check (never returns secret)
- DELETE /secret — admin secret removal
Verification matches Wave's documented signature scheme:
- Header: x-wave-signature: t=<unix_ts>,v1=<hex_hmac_sha256>
- Signed payload: <timestamp>.<raw_body> (raw bytes, not re-serialized)
- 5-minute replay window enforced
- Constant-time hex compare
Receiver behaviour:
- Skips signature verification when no secret stored (allows initial
Wave dashboard ping during setup)
- Validates payload business_id matches URL parameter (defense in depth)
- KV idempotency keyed on event_id, 7-day TTL
- Audit-logs each recognized event via ledgerLog (ChittyLedger)
- Treats empty body or unrecognized JSON shape as a setup ping (200 ack)
Currently supported event types per Wave's Webhooks Setup Guide:
invoice.overdue, invoice.viewed, invoice.approved
("More supported events will be available soon" — receiver is generic
and audit-logs all conformant events; specific business logic for
future events is added as Wave expands the surface.)
KV layout:
webhook:wave:secret:<tenantId>:<businessId> → HMAC secret
webhook:wave:dedup:<eventId> → 7d TTL idempotency marker
Tests: +28 (16 secret-storage + 12 receiver) — full suite 281 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyfinance | dd153c0 | May 01 2026, 01:17 PM |
|
@coderabbitai review Please evaluate:
|
📝 WalkthroughWalkthroughThe PR introduces Wave native webhook integration with per-tenant/business secret management. It adds HMAC-SHA256 signature verification with replay protection, KV-backed event deduplication, and secret admin endpoints (PUT, GET, DELETE). Two comprehensive test suites validate the receiver and secret management functionality. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Server as Server<br/>(Webhook Handler)
participant KV as KV Storage
participant Audit as Audit Log
Client->>Server: POST webhook<br/>(x-wave-signature header)
Server->>Server: Parse signature<br/>(timestamp + v1)
Server->>Server: Validate timestamp<br/>(5-min window)
Server->>Server: Retrieve secret from cache
Server->>Server: Compute HMAC-SHA256<br/>(timestamp.rawBody)
Server->>Server: Constant-time compare
alt Signature Invalid or Outside Window
Server-->>Client: 401 Unauthorized
else Signature Valid
Server->>Server: Parse JSON body
alt Empty/Unrecognized Payload
Server-->>Client: 200 OK (setup ping)
else Valid Payload
Server->>Server: Validate business_id
alt ID Mismatch
Server-->>Client: 400 Bad Request
else ID Match
Server->>KV: Check event_id<br/>(deduplication)
alt Duplicate event_id
KV-->>Server: Exists
Server-->>Client: 202 Accepted (duplicate)
else New event_id
KV-->>Server: Not found
Server->>KV: Store event_id<br/>(7-day TTL)
Server->>Audit: Log accepted event
Server-->>Client: 202 Accepted (new)
end
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Code Review — PR #113: Wave Native Webhook ReceiverOverall this is a well-structured implementation. The signature verification, KV dedup pattern, and test coverage are all solid. A few items worth addressing before merge, with one security concern that I'd flag as blocking. 🔴 Security — Unauthenticated event injection during setup window
When no secret is stored, signature verification is skipped entirely and the event is processed through the full handler (business_id check, dedup write, The intent — allowing Wave's initial test ping — is reasonable, but the current approach is too permissive. A targeted fix: // Option A: always ack setup-ping 200 when no secret; never reach ledgerLog
if (!secret) {
console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— treating as setup ping');
return c.json({ received: true }, 200);
}
// signature verification from here on is unconditionalThis still ACKs Wave's test ping (200 OK) but prevents audit-log pollution from unauthenticated events. The tradeoff is that the initial test webhook from Wave won't produce a ledger entry, which is fine since it's a synthetic ping rather than a real business event. 🟡 Stale comment in secret-storage blockNear the top of the new secret-storage section (around line 572): The receiver is implemented in this PR, so this comment is outdated. Delete or update it before merge — it will mislead future readers looking for where the receiver lives. 🟡 Dedup key not scoped to tenant/businessconst dedupKey = `webhook:wave:dedup:${event.event_id}`;The dedup key is global across all tenants and businesses. If Wave ever reuses const dedupKey = `webhook:wave:dedup:${tenantId}:${businessId}:${event.event_id}`;The TTL storage cost is minimal and this is unambiguously correct. 🟡
|
There was a problem hiding this comment.
Pull request overview
Adds Wave’s native webhook receiver and per-(tenant, business) KV secret management to the existing server/routes/webhooks.ts webhook surface, with Vitest coverage for admin secret endpoints and receiver behavior.
Changes:
- Added Wave webhook secret CRUD endpoints (PUT/GET exists/DELETE) guarded by service-token auth.
- Added Wave native receiver endpoint with HMAC-SHA256 signature verification, replay-window enforcement, payload validation, KV dedup, and ledger audit logging.
- Added new Vitest suites covering secret storage and receiver behaviors (signature/replay/dedup/business binding/setup ping).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| server/routes/webhooks.ts | Adds Wave per-business secret storage + native webhook receiver with signature verification and KV dedup/audit logging. |
| server/tests/webhooks-wave-secret.test.ts | Tests Wave secret storage endpoints (auth, validation, KV isolation, idempotency). |
| server/tests/webhooks-wave-receiver.test.ts | Tests Wave receiver signature verification, replay window, payload handling, and dedup behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // subscription is created and must be supplied to the verification step | ||
| // of the receiver (added in a follow-up PR once Wave's signature schema | ||
| // is verified). |
There was a problem hiding this comment.
The block comment says the Wave receiver is “added in a follow-up PR”, but the receiver is implemented later in this same file. Please update/remove this note so the documentation matches the current implementation (and doesn’t mislead operators about where signature verification lives).
| // subscription is created and must be supplied to the verification step | |
| // of the receiver (added in a follow-up PR once Wave's signature schema | |
| // is verified). | |
| // subscription is created and are supplied to the receiver's signature | |
| // verification step implemented later in this file. |
| function isAuthorizedWaveSecretCaller(env: { CHITTY_AUTH_SERVICE_TOKEN?: string }, authHeader: string): boolean { | ||
| const expected = env.CHITTY_AUTH_SERVICE_TOKEN; | ||
| if (!expected) return false; | ||
| const token = authHeader.startsWith('Bearer ') ? authHeader.slice(7) : ''; | ||
| return token.length > 0 && token === expected; | ||
| } |
There was a problem hiding this comment.
isAuthorizedWaveSecretCaller does a direct string equality check against CHITTY_AUTH_SERVICE_TOKEN. Elsewhere in the codebase, service-token auth uses constant-time comparison via tokenEqual (see server/middleware/auth.ts). Consider reusing serviceAuth middleware for these endpoints or using tokenEqual here to avoid timing leaks and keep auth behavior consistent.
| const [key, ...rest] = part.split('='); | ||
| const value = rest.join('='); | ||
| if (key === 't') timestamp = value; | ||
| if (key === 'v1') signature = value; | ||
| } | ||
|
|
||
| if (!timestamp || !signature) return false; |
There was a problem hiding this comment.
Signature header parsing here doesn’t trim whitespace. If Wave (or an intermediary) formats the header like t=..., v1=... (note the space after the comma), key becomes ' v1' and verification will always fail. Trim each part/key/value before comparison (and consider normalizing hex casing) to make verification robust.
| const [key, ...rest] = part.split('='); | |
| const value = rest.join('='); | |
| if (key === 't') timestamp = value; | |
| if (key === 'v1') signature = value; | |
| } | |
| if (!timestamp || !signature) return false; | |
| const trimmedPart = part.trim(); | |
| const [rawKey, ...rest] = trimmedPart.split('='); | |
| const key = rawKey.trim(); | |
| const value = rest.join('=').trim(); | |
| if (key === 't') timestamp = value; | |
| if (key === 'v1') signature = value; | |
| } | |
| if (!timestamp || !signature) return false; | |
| signature = signature.toLowerCase(); |
| if (secret) { | ||
| if (!signatureHeader || !(await verifyWaveSignature(rawBody, signatureHeader, secret))) { | ||
| return c.json({ error: 'invalid_signature' }, 401); | ||
| } | ||
| } else { | ||
| console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— signature verification skipped'); | ||
| } | ||
|
|
There was a problem hiding this comment.
When no secret is stored, the handler skips signature verification but continues on to parse schema-conformant JSON, write a 7-day dedup marker, and emit an audit ledgerLog. This makes the unauthenticated public endpoint writable during setup (anyone can spam audit logs / burn KV keys) until the secret is configured. Consider short-circuiting to a “setup ping” ack (no side effects) unless a secret exists, or only allowing side effects when a valid signature has been verified.
| if (secret) { | |
| if (!signatureHeader || !(await verifyWaveSignature(rawBody, signatureHeader, secret))) { | |
| return c.json({ error: 'invalid_signature' }, 401); | |
| } | |
| } else { | |
| console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— signature verification skipped'); | |
| } | |
| if (!secret) { | |
| console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— acking setup ping without side effects'); | |
| return c.json({ received: true, setup: true }, 200); | |
| } | |
| if (!signatureHeader || !(await verifyWaveSignature(rawBody, signatureHeader, secret))) { | |
| return c.json({ error: 'invalid_signature' }, 401); | |
| } |
Empirical verification of Wave's webhook surfaceQueried the live Wave GraphQL endpoint ( curl -sS -X POST "https://gql.waveapps.com/graphql/public" \
-H "Content-Type: application/json" \
-d '{"query":"{ __schema { mutationType { fields { name } } } }"}'Result confirmed:
So the dashboard-only design in this PR is the only viable approach — there's no GraphQL surface to add |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes/webhooks.ts`:
- Around line 779-790: The audit entry currently passes raw event.data to
ledgerLog which may leak sensitive information; instead implement and call a
sanitizer (e.g., sanitizeWebhookData) and/or extract a whitelist of safe
identifiers (invoiceId, customerId, amount, status) and a short summary, mask
any account numbers/API keys, then pass the sanitized object to ledgerLog
(replace event.data with the return of sanitizeWebhookData(event.data) and
include event.event_type, event.event_id, tenantId, businessId as before).
- Around line 739-745: The code currently skips signature verification when the
stored Wave secret is missing, allowing any schema-valid payload to proceed to
deduplication and ledgerLog; change this so that when secret is falsy you only
accept explicit setup/health-check payloads (e.g., an empty body or a known safe
"ping"/"setup" marker) and otherwise return 401. Concretely: in the webhook
handler around verifyWaveSignature/signatureHeader/secret, replace the
unconditional else that logs and proceeds with a guard that inspects
rawBody/parsed body and only allows the specific safe setup payload shape
(explicitly check for empty body or a recognized ping/setup event) to bypass
verification; for all other payloads return c.json({ error: 'invalid_signature'
}, 401) and log the rejection. Apply the same change to the other webhook
handling block that spans the later section (the 755-804 area) so dedup +
ledgerLog are never reached without a secret unless the payload is the allowed
setup ping.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f721c93b-f404-4335-9313-f2d648220157
📒 Files selected for processing (3)
server/__tests__/webhooks-wave-receiver.test.tsserver/__tests__/webhooks-wave-secret.test.tsserver/routes/webhooks.ts
| if (secret) { | ||
| if (!signatureHeader || !(await verifyWaveSignature(rawBody, signatureHeader, secret))) { | ||
| return c.json({ error: 'invalid_signature' }, 401); | ||
| } | ||
| } else { | ||
| console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— signature verification skipped'); | ||
| } |
There was a problem hiding this comment.
Don't accept real webhook events before a secret exists.
The current else path skips verification and still lets any schema-valid payload reach dedup + ledgerLog. That leaves a spoofing window during setup where forged Wave events are accepted as real deliveries. The ping exception should stay limited to empty/unrecognized setup payloads only.
🛡️ Suggested guard
const kv = c.env.FINANCE_KV;
const secret = await kv.get(waveSecretKey(tenantId, businessId));
+ const hasSecret = typeof secret === 'string' && secret.length > 0;
const signatureHeader = c.req.header('x-wave-signature') ?? '';
- if (secret) {
+ if (hasSecret) {
if (!signatureHeader || !(await verifyWaveSignature(rawBody, signatureHeader, secret))) {
return c.json({ error: 'invalid_signature' }, 401);
}
} else {
console.warn('[webhook:wave] No secret stored for', { tenantId, businessId }, '— signature verification skipped');
@@
const parsed = waveNativeEventSchema.safeParse(body);
if (!parsed.success) {
@@
return c.json({ received: true }, 200);
}
+
+ if (!hasSecret) {
+ return c.json({ error: 'secret_not_configured' }, 503);
+ }As per coding guidelines "Implement webhook verification using provided signing secrets (Stripe: STRIPE_WEBHOOK_SECRET, Mercury: service auth middleware). Use KV-based deduplication with 7-day TTL for idempotent event processing."
Also applies to: 755-804
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/webhooks.ts` around lines 739 - 745, The code currently skips
signature verification when the stored Wave secret is missing, allowing any
schema-valid payload to proceed to deduplication and ledgerLog; change this so
that when secret is falsy you only accept explicit setup/health-check payloads
(e.g., an empty body or a known safe "ping"/"setup" marker) and otherwise return
401. Concretely: in the webhook handler around
verifyWaveSignature/signatureHeader/secret, replace the unconditional else that
logs and proceeds with a guard that inspects rawBody/parsed body and only allows
the specific safe setup payload shape (explicitly check for empty body or a
recognized ping/setup event) to bypass verification; for all other payloads
return c.json({ error: 'invalid_signature' }, 401) and log the rejection. Apply
the same change to the other webhook handling block that spans the later section
(the 755-804 area) so dedup + ledgerLog are never reached without a secret
unless the payload is the allowed setup ping.
| ledgerLog( | ||
| c, | ||
| { | ||
| entityType: 'audit', | ||
| action: `webhook.wave.${event.event_type.replace(/[^a-z0-9_]/gi, '_')}`, | ||
| metadata: { | ||
| tenantId, | ||
| businessId, | ||
| eventId: event.event_id, | ||
| eventType: event.event_type, | ||
| data: event.data, | ||
| }, |
There was a problem hiding this comment.
Avoid sending raw event.data to the audit ledger.
event.data is unbounded webhook payload content and can include invoice/customer details. Shipping it verbatim through ledgerLog widens the audit surface with unnecessary sensitive data. Prefer whitelisted identifiers or a sanitized summary.
🧹 Suggested narrowing
{
entityType: 'audit',
action: `webhook.wave.${event.event_type.replace(/[^a-z0-9_]/gi, '_')}`,
metadata: {
tenantId,
businessId,
eventId: event.event_id,
eventType: event.event_type,
- data: event.data,
+ dataKeys: Object.keys(event.data),
+ invoiceId: typeof event.data['invoice_id'] === 'string' ? event.data['invoice_id'] : undefined,
},
},As per coding guidelines "Always sanitize financial data in logs by masking account numbers and sensitive information. Never log full API keys or credentials."
📝 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.
| ledgerLog( | |
| c, | |
| { | |
| entityType: 'audit', | |
| action: `webhook.wave.${event.event_type.replace(/[^a-z0-9_]/gi, '_')}`, | |
| metadata: { | |
| tenantId, | |
| businessId, | |
| eventId: event.event_id, | |
| eventType: event.event_type, | |
| data: event.data, | |
| }, | |
| ledgerLog( | |
| c, | |
| { | |
| entityType: 'audit', | |
| action: `webhook.wave.${event.event_type.replace(/[^a-z0-9_]/gi, '_')}`, | |
| metadata: { | |
| tenantId, | |
| businessId, | |
| eventId: event.event_id, | |
| eventType: event.event_type, | |
| dataKeys: Object.keys(event.data), | |
| invoiceId: typeof event.data['invoice_id'] === 'string' ? event.data['invoice_id'] : undefined, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/webhooks.ts` around lines 779 - 790, The audit entry currently
passes raw event.data to ledgerLog which may leak sensitive information; instead
implement and call a sanitizer (e.g., sanitizeWebhookData) and/or extract a
whitelist of safe identifiers (invoiceId, customerId, amount, status) and a
short summary, mask any account numbers/API keys, then pass the sanitized object
to ledgerLog (replace event.data with the return of
sanitizeWebhookData(event.data) and include event.event_type, event.event_id,
tenantId, businessId as before).
Summary
Adds Wave's native webhook flow to chittyfinance, scoped per (tenant, business) pair. Implements the receiver, full HMAC-SHA256 signature verification per Wave's Webhooks Setup Guide, and admin endpoints for per-business secret storage.
Wave's webhook subscriptions are configured only via the Wave dashboard (no programmatic API exists in Wave's GraphQL schema), so operators paste the per-business URL into the Wave Webhooks page and seed the corresponding secret via the admin endpoint added here.
New endpoints (under
/api/webhooks/wave/:tenantId/:businessId)//secret/secret/exists/secretSignature verification (per Wave's docs)
x-wave-signature: t=<unix_ts>,v1=<hex_hmac_sha256><timestamp>.<raw_body>(raw body as-is — not re-serialized)Receiver behaviour
business_idmatches URL parameter (defense in depth)event_id, 7-day TTLledgerLog(ChittyLedger)Currently supported events (per Wave Webhooks Setup Guide)
invoice.overdueinvoice.viewedinvoice.approvedThe receiver is generic and audit-logs all schema-conformant events; specific business logic for future events is added as Wave expands the surface.
KV layout
Operator setup runbook (post-merge)
For each tenant + Wave business:
https://finance.chitty.cc/api/webhooks/wave/<tenantId>/<businessId>and select trigger eventswebhook.wave.<event_type>Why no
wave-api.tschanges / no auto-register / no registration scriptWave's GraphQL schema has no webhook-related mutations (verified end-to-end against the public Wave schema). All webhook subscription management is dashboard-only. Adding fake registration code would be dead/never-callable.
Test plan
npx tsc --noEmitcleanfinance.chitty.ccwebhook.wave.invoice_*lands when Wave fires a real eventOut of scope (future PRs)
invoice.approved) — invoice events don't currently map cleanly to thetransactionstable, so for now we audit-log onlytransaction:*/account:*events once Wave releases them🤖 Generated with Claude Code
Summary by CodeRabbit