Skip to content

feat(webhooks): Wave native webhook receiver + per-business secret storage#113

Open
chitcommit wants to merge 1 commit intomainfrom
feat/wave-webhook-per-tenant
Open

feat(webhooks): Wave native webhook receiver + per-business secret storage#113
chitcommit wants to merge 1 commit intomainfrom
feat/wave-webhook-per-tenant

Conversation

@chitcommit
Copy link
Copy Markdown
Contributor

@chitcommit chitcommit commented May 1, 2026

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)

Method Path Purpose
POST / Wave native receiver (HMAC verified, dedup'd, audit-logged)
PUT /secret Admin secret storage (service-token auth)
GET /secret/exists Admin existence check (never returns the secret)
DELETE /secret Admin secret removal

Signature verification (per Wave's docs)

  • Header: x-wave-signature: t=<unix_ts>,v1=<hex_hmac_sha256>
  • Signed payload: <timestamp>.<raw_body> (raw body as-is — 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 configuration 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 events (per Wave Webhooks Setup Guide)

  • invoice.overdue
  • invoice.viewed
  • invoice.approved

"More supported events will be available soon"

The 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

webhook:wave:secret:<tenantId>:<businessId>  → HMAC secret
webhook:wave:dedup:<eventId>                 → 7d TTL idempotency marker

Operator setup runbook (post-merge)

For each tenant + Wave business:

  1. In Wave dashboard → Webhooks: paste https://finance.chitty.cc/api/webhooks/wave/<tenantId>/<businessId> and select trigger events
  2. Copy the secret Wave reveals
  3. curl -X PUT https://finance.chitty.cc/api/webhooks/wave/<tenantId>/<businessId>/secret \
      -H "Authorization: Bearer $CHITTY_AUTH_SERVICE_TOKEN" \
      -H "Content-Type: application/json" \
      -d "{\"secret\":\"<wave-hash-secret>\"}"
    
  4. Trigger a test event from Wave dashboard → should land in ledger as webhook.wave.<event_type>

Why no wave-api.ts changes / no auto-register / no registration script

Wave'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

  • Type check: npx tsc --noEmit clean
  • New tests: 28 passing (16 secret-storage + 12 receiver covering signature/replay/dedup/business-id-mismatch/setup-ping)
  • Full suite: 281/281 passing (was 253, +28)
  • Post-deploy: configure one Wave business webhook end-to-end against live finance.chitty.cc
  • Post-deploy: verify ledger entry webhook.wave.invoice_* lands when Wave fires a real event

Out of scope (future PRs)

  • Map invoice events to chittyfinance domain entities (e.g., create AR record on invoice.approved) — invoice events don't currently map cleanly to the transactions table, so for now we audit-log only
  • Surface Wave webhook config status in the integrations UI
  • Receiver for transaction:* / account:* events once Wave releases them

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Integrated Wave native webhook support with HMAC-SHA256 signature verification and 5-minute replay protection
    • Secure webhook secret management with dedicated endpoints to store, verify, and delete credentials per tenant-business pair
    • Automatic deduplication of duplicate webhook events with 7-day retention
    • Comprehensive audit logging for all accepted webhook events

…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>
Copilot AI review requested due to automatic review settings May 1, 2026 13:17
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
chittyfinance dd153c0 May 01 2026, 01:17 PM

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

@coderabbitai review

Please evaluate:

  • Security implications
  • Credential exposure risk
  • Dependency supply chain concerns
  • Breaking API changes

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

📝 Walkthrough

Walkthrough

The 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

Cohort / File(s) Summary
Wave Webhook Tests
server/__tests__/webhooks-wave-receiver.test.ts, server/__tests__/webhooks-wave-secret.test.ts
Comprehensive test coverage for webhook receiver end-to-end validation (signature verification, replay window enforcement, idempotency via event_id deduplication) and secret admin endpoints (CRUD operations with auth enforcement and KV isolation).
Wave Webhook Integration
server/routes/webhooks.ts
Complete Wave webhook integration including secret storage endpoints, HMAC-SHA256 signature verification with 5-minute replay window, raw body handling, Zod schema validation, KV-backed deduplication by event_id (7-day TTL), audit logging, and structured error responses.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Hop! Skip! Jump—webhooks now secure,
Wave signatures verified, replay windows pure,
KV deduplication keeps duplicates at bay,
Secrets stored safely—hoorah, hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: implementation of a Wave native webhook receiver with per-business secret storage, which directly matches the primary functionality added across the three files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wave-webhook-per-tenant

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

@claude
Copy link
Copy Markdown

claude Bot commented May 1, 2026

Code Review — PR #113: Wave Native Webhook Receiver

Overall 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

server/routes/webhooks.ts, receiver POST handler:

When no secret is stored, signature verification is skipped entirely and the event is processed through the full handler (business_id check, dedup write, ledgerLog). This means during the window between configuring the Wave webhook URL in the Wave dashboard and calling PUT /secret, any caller who knows the endpoint URL can inject arbitrary recognized events into the audit ledger.

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 unconditional

This 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 block

Near the top of the new secret-storage section (around line 572):

// Secrets are issued by Wave when a webhook 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).

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/business

const dedupKey = `webhook:wave:dedup:${event.event_id}`;

The dedup key is global across all tenants and businesses. If Wave ever reuses event_id values across business accounts (unlikely but undocumented), an event for tenant A could silently suppress the same event ID arriving for tenant B. Compare with the Mercury pattern to see if per-tenant scoping was intentional there. Suggested alternative:

const dedupKey = `webhook:wave:dedup:${tenantId}:${businessId}:${event.event_id}`;

The TTL storage cost is minimal and this is unambiguously correct.


🟡 isAuthorizedWaveSecretCaller uses non-constant-time token comparison

return token.length > 0 && token === expected;

Timing attacks over HTTP are difficult in practice, but the pattern elsewhere in this codebase (e.g. Mercury webhook HMAC) uses constant-time comparison. Given that service tokens are long random strings this is low risk, but worth aligning with project conventions:

// constant-time compare using the same XOR loop already used in verifyWaveSignature

Or, if you want to keep it simple, at minimum document why string equality is acceptable here.


🟡 No tenantId format validation on admin endpoints

The PUT/GET/DELETE secret endpoints extract tenantId from the URL with no format validation. A service-token caller who supplies a tenantId containing colons (e.g. "a:b") would write to a key like webhook:wave:secret:a:b:businessId, which has a different interpretation from webhook:wave:secret:a:b:businessId intended for tenant a:b. Since tenant IDs are UUIDs in the system schema, consider a quick UUID guard:

const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
if (!UUID_RE.test(tenantId)) return c.json({ error: 'invalid_tenant_id' }, 400);

✅ What's well done

  • verifyWaveSignature — correct constant-time hex comparison, proper replay-window enforcement, clean separation from Mercury's verification logic.
  • Raw body handling — reading rawBody once via c.req.text() before any parsing is exactly right; re-serializing JSON would break the HMAC.
  • event_type sanitization for the ledger action string (replace(/[^a-z0-9_]/gi, '_')) — good defensive practice.
  • Test coverage — 28 tests across signature verification, replay, business-ID mismatch, setup ping, dedup isolation, and tenant/business key isolation. The Map-backed KV stub pattern is clean and consistent with the Mercury tests.
  • GET /secret/exists confirming the response body never contains the secret value is a nice explicit test.
  • PR description is thorough and the KV layout diagram and operator runbook are genuinely useful for future operators.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread server/routes/webhooks.ts
Comment on lines +569 to +571
// 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).
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
Comment thread server/routes/webhooks.ts
Comment on lines +581 to +586
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;
}
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment thread server/routes/webhooks.ts
Comment on lines +610 to +616
const [key, ...rest] = part.split('=');
const value = rest.join('=');
if (key === 't') timestamp = value;
if (key === 'v1') signature = value;
}

if (!timestamp || !signature) return false;
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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();

Copilot uses AI. Check for mistakes.
Comment thread server/routes/webhooks.ts
Comment on lines +739 to +746
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');
}

Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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);
}

Copilot uses AI. Check for mistakes.
@chitcommit
Copy link
Copy Markdown
Contributor Author

Empirical verification of Wave's webhook surface

Queried the live Wave GraphQL endpoint (https://gql.waveapps.com/graphql/public) anonymously via introspection:

curl -sS -X POST "https://gql.waveapps.com/graphql/public" \
  -H "Content-Type: application/json" \
  -d '{"query":"{ __schema { mutationType { fields { name } } } }"}'

Result confirmed:

  • 0 webhook-related mutations (full mutation set is invoice/customer/account/estimate/product/salesTax/moneyTransaction CRUD)
  • 0 webhook-related queries
  • 0 webhook-related types (no WebhookSubscription, no Subscription, no Event)

So the dashboard-only design in this PR is the only viable approach — there's no GraphQL surface to add wave-api.ts methods or auto-register-on-callback against. Confirmed both via the published Webhooks Setup Guide and the live introspection check.

@chitcommit chitcommit enabled auto-merge (squash) May 1, 2026 13:22
Copy link
Copy Markdown
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: 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

📥 Commits

Reviewing files that changed from the base of the PR and between a7d2271 and dd153c0.

📒 Files selected for processing (3)
  • server/__tests__/webhooks-wave-receiver.test.ts
  • server/__tests__/webhooks-wave-secret.test.ts
  • server/routes/webhooks.ts

Comment thread server/routes/webhooks.ts
Comment on lines +739 to +745
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');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Comment thread server/routes/webhooks.ts
Comment on lines +779 to +790
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,
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants