feat: Mercury native webhook with HMAC signature verification#99
feat: Mercury native webhook with HMAC signature verification#99chitcommit merged 4 commits intomainfrom
Conversation
|
@coderabbitai review Please evaluate:
|
📝 WalkthroughWalkthroughAdds a tenant-scoped Mercury webhook POST that consumes raw bodies, optionally verifies per-tenant HMAC signatures from KV, KV-deduplicates events (7d), validates envelopes, ingests Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Webhook Client
participant Endpoint as Webhook Endpoint
participant KV as KV Store (secrets & dedupe)
participant Verifier as Signature Verifier
participant Validator as Schema Validator
participant Classifier as Account Resolver/Classifier
participant DB as Storage / DB
participant Logger as Ledger Logger
Client->>Endpoint: POST /api/webhooks/mercury/:tenantId (raw body + Mercury-Signature)
Endpoint->>KV: fetch tenant secret
Endpoint->>Verifier: verify HMAC-SHA256 over "<timestamp>.<raw_body>"
Verifier-->>Endpoint: valid / invalid / no-secret
Endpoint->>Validator: parse & validate envelope
Validator-->>Endpoint: valid / invalid
Endpoint->>KV: check/set dedupe key for event.id (7d TTL)
alt already processed
KV-->>Endpoint: exists
Endpoint-->>Client: 200 OK
else new event
KV-->>Endpoint: not found (set dedupe)
Endpoint->>Endpoint: if resourceType !== "transaction" => 200 OK
Endpoint->>Classifier: extract mergePatch, findAccountCode(), resolve/create account (may call SystemStorage.lookupAccountByExternalId)
Classifier-->>Endpoint: accountRef
Endpoint->>Validator: advisory validateRow() (schema advisory only)
Endpoint->>DB: storage.createTransaction(...)
DB-->>Endpoint: persisted transaction
Endpoint->>Logger: ledgerLog(...)
Endpoint-->>Client: 201 Created (transaction + classification + schemaAdvisory)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyfinance | 4230de4 | Apr 23 2026, 05:54 PM |
Code Review — PR #99: Mercury Native Webhook with HMAC Signature VerificationOverviewThis PR adds a tenant-scoped Mercury native webhook endpoint ( 🔴 Critical Issues1. Missing timestamp replay protection ( The function extracts // After extracting timestamp:
const now = Math.floor(Date.now() / 1000);
if (Math.abs(now - parseInt(timestamp, 10)) > 300) return false; // 5-minute window2. Signature skip is too permissive in production if (secret) {
// verify...
} else {
console.warn('[webhook:mercury] MERCURY_WEBHOOK_SECRET not set — signature verification skipped');
}The comment says this allows Mercury's verification ping to succeed during registration — but Mercury's verification ping is just a GET or an empty-body POST, not a signed transaction event. Skipping verification entirely (rather than just allowing empty-body pings through) means any actor can POST arbitrary transaction data to this endpoint whenever 🟡 Bugs / Logic Issues3. Race condition in account auto-creation const accounts = await storage.getAccounts(tenantId);
const active = accounts.find((a) => a.isActive);
if (active) {
accountId = active.id;
} else {
const created = await storage.createAccount({ ... });If two webhook events for the same tenant arrive simultaneously and neither finds an active account, both will call 4. Zero-amount transactions slip through if (amount === null) {
return c.json({ received: true, operationType: event.operationType }, 200);
}
5. Empty const description = (patch.bankDescription as string) ?? (patch.counterpartyName as string) ?? '';If both fields are absent, 🟡 Security6. async lookupAccountByExternalId(externalId: string) {
const [row] = await this.db.select().from(schema.accounts).where(eq(schema.accounts.externalId, externalId));
return row;
}The method returns an account row from any tenant. The caller does check async lookupAccountByExternalId(externalId: string, tenantId: string) {
const [row] = await this.db.select().from(schema.accounts)
.where(and(eq(schema.accounts.externalId, externalId), eq(schema.accounts.tenantId, tenantId)));
return row;
}🟢 What's Done Well
🔵 Minor / Nits
Summary
The replay protection fix is the most important — the rest are lower-risk given that |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
server/routes/webhooks.ts (1)
230-245: Unsafeas stringcasts and classifier drops the category hint.Two small things in the extraction block:
patch.bankDescription as string,patch.counterpartyName as string,patch.postedAt as string,patch.accountId as stringlie to the compiler —mergePatchisz.record(z.unknown()), so these fields can benull, numbers, objects, etc. If Mercury sendscounterpartyName: null,descriptionbecomes the string"null". Prefer a narrow schema ortypeof x === 'string' ? x : nullguards (you already do this correctly foramount).- The legacy path calls
findAccountCode(tx.description, tx.category ?? undefined); the native path callsfindAccountCode(description)only and ignorespatch.category/patch.mercuryCategory. This will give worse L0 classification for any Mercury event that carries a category.♻️ Proposed fix
- const patch = (event.mergePatch ?? {}) as Record<string, unknown>; - const amount = typeof patch.amount === 'number' ? patch.amount : null; - const description = (patch.bankDescription as string) ?? (patch.counterpartyName as string) ?? ''; - const counterpartyName = (patch.counterpartyName as string) ?? null; - const postedAt = (patch.postedAt as string) ?? (event.occurredAt as string) ?? null; - const mercuryAccountId = (patch.accountId as string) ?? null; + const patch = event.mergePatch ?? {}; + const asStr = (v: unknown): string | null => (typeof v === 'string' ? v : null); + const amount = typeof patch.amount === 'number' ? patch.amount : null; + const description = asStr(patch.bankDescription) ?? asStr(patch.counterpartyName) ?? ''; + const counterpartyName = asStr(patch.counterpartyName); + const postedAt = asStr(patch.postedAt) ?? asStr(event.occurredAt) ?? null; + const mercuryAccountId = asStr(patch.accountId); + const mercuryCategory = asStr(patch.category) ?? asStr(patch.mercuryCategory); ... - const suggestedCoaCode = findAccountCode(description); + const suggestedCoaCode = findAccountCode(description, mercuryCategory ?? 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 230 - 245, The extraction uses unsafe `as string` casts and drops Mercury's category hint; replace casts with type guards (e.g., derive description = typeof patch.bankDescription === 'string' ? patch.bankDescription : typeof patch.counterpartyName === 'string' ? patch.counterpartyName : '' and similarly set counterpartyName = typeof patch.counterpartyName === 'string' ? patch.counterpartyName : null, postedAt = typeof patch.postedAt === 'string' ? patch.postedAt : typeof event.occurredAt === 'string' ? event.occurredAt : null, mercuryAccountId = typeof patch.accountId === 'string' ? patch.accountId : null) so you never end up with the string "null"; also capture a categoryHint from patch.category or patch.mercuryCategory using the same typeof string checks and pass that as the second argument to findAccountCode(description, categoryHint) so L0 classification uses Mercury's category when available.
🤖 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 215-222: The KV dedup marker is written before processing which
causes transient failures to become permanent duplicates; change the flow in the
webhook handler so the KV put to c.env.FINANCE_KV (dedupKey) only happens after
a successful database insert (i.e., after createTransaction and after
getTransactionByExternalId confirms the row exists), or implement an "in-flight"
marker (short TTL) first and then replace/upgrade it to the 7-day TTL only upon
successful createTransaction; update references to kv, dedupKey,
createTransaction, and getTransactionByExternalId accordingly.
- Around line 181-182: The route handler starting at webhookRoutes.post where
tenantId is read directly from c.req.param('tenantId') must validate and verify
the tenant before any createAccount/createTransaction/audit actions: parse and
validate tenantId as a UUID using the Zod schemas from `@shared/schema`, then call
storage.getTenant(tenantId) (or the SystemStorage equivalent) and return a
404/unknown_tenant JSON response if missing; when resolving accounts use
getAccountByExternalId(externalId, tenantId) (only fall back to
lookupAccountByExternalId for diagnostics) so all account creation/transaction
insertion and audit logging include the tenant context and avoid the
cross-tenant fallback.
- Around line 184-196: Current logic allows unsigned webhooks when
MERCURY_WEBHOOK_SECRET is unset, which is a security risk; change the handler so
signature verification is required for all non-ping requests by making
MERCURY_WEBHOOK_SECRET mandatory (i.e., if secret is falsy, reject with 401) or,
if you must allow unsigned registration pings only, make the fallback explicit
behind a dedicated env flag like MERCURY_WEBHOOK_ALLOW_UNSIGNED and only skip
verifyMercurySignature(rawBody, signatureHeader, secret) when that flag is true
and the request shape matches the known registration ping (empty/non-JSON) —
reference MERCURY_WEBHOOK_SECRET, signatureHeader, rawBody,
verifyMercurySignature and the webhook handler code to implement this.
- Around line 32-71: The verifyMercurySignature function parses the
t=<timestamp> but never checks freshness; modify verifyMercurySignature to parse
timestamp as an integer (e.g., const ts = parseInt(timestamp, 10)), compute the
difference against Date.now() (convert ts to ms), and return false if the
timestamp is not numeric or is older than 5 minutes (5 * 60 * 1000 ms) or
unreasonably in the future (allow small clock skew, e.g., 1 minute); keep the
existing HMAC computation and constant-time comparison but add this early
timestamp validation using the existing timestamp variable to prevent replay
attacks.
In `@server/storage/system.ts`:
- Around line 77-85: The helper lookupAccountByExternalId currently queries
schema.accounts without tenant isolation; change its signature to accept an
optional tenantId parameter, apply a .where(eq(schema.accounts.tenantId,
tenantId)) when tenantId is provided, and keep a deterministic
.orderBy(schema.accounts.id) with .limit(1) to ensure consistent results; update
the JSDoc for lookupAccountByExternalId to document the cross-tenant risk and
the new optional tenantId behavior so callers (e.g., server/routes/webhooks.ts)
must pass tenant context or handle multiple matches explicitly.
---
Nitpick comments:
In `@server/routes/webhooks.ts`:
- Around line 230-245: The extraction uses unsafe `as string` casts and drops
Mercury's category hint; replace casts with type guards (e.g., derive
description = typeof patch.bankDescription === 'string' ? patch.bankDescription
: typeof patch.counterpartyName === 'string' ? patch.counterpartyName : '' and
similarly set counterpartyName = typeof patch.counterpartyName === 'string' ?
patch.counterpartyName : null, postedAt = typeof patch.postedAt === 'string' ?
patch.postedAt : typeof event.occurredAt === 'string' ? event.occurredAt : null,
mercuryAccountId = typeof patch.accountId === 'string' ? patch.accountId : null)
so you never end up with the string "null"; also capture a categoryHint from
patch.category or patch.mercuryCategory using the same typeof string checks and
pass that as the second argument to findAccountCode(description, categoryHint)
so L0 classification uses Mercury's category when available.
🪄 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: 01a8e277-7342-4ab0-92d0-65190bbbdd06
📒 Files selected for processing (2)
server/routes/webhooks.tsserver/storage/system.ts
There was a problem hiding this comment.
Pull request overview
Adds a Mercury-native webhook ingestion path to the public webhook router, including HMAC signature verification and tenant-scoped routing, while preserving the legacy ChittyConnect-normalized Mercury webhook endpoint.
Changes:
- Introduces
/api/webhooks/mercury/:tenantIdto ingest Mercury’s native event envelope, dedup events (KV + DB), and persist/classify transactions. - Adds Web Crypto-based HMAC-SHA256 verification for
Mercury-Signature. - Adds a cross-tenant account lookup helper and refactors the legacy Mercury webhook schema naming/logging.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| server/routes/webhooks.ts | Adds Mercury native webhook endpoint with HMAC verification, merge-patch field extraction, dedup, persistence + classification; keeps legacy endpoint with minor refactors. |
| server/storage/system.ts | Adds lookupAccountByExternalId() helper used by the new webhook path for account resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93de098267
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review Please evaluate:
|
Code Review — PR #99: Mercury native webhook with HMAC signature verificationOverviewThis PR adds a native Mercury webhook endpoint ( 🔴 Security Issues1. Missing timestamp replay-attack validation in The function extracts // Add after extracting timestamp:
const ts = parseInt(timestamp, 10);
const now = Math.floor(Date.now() / 1000);
if (isNaN(ts) || Math.abs(now - ts) > 300) return false; // 5-minute window2.
// Safer: filter by tenantId in the query
async lookupAccountByExternalId(externalId: string, tenantId: string) {
const [row] = await this.db
.select()
.from(schema.accounts)
.where(and(eq(schema.accounts.externalId, externalId), eq(schema.accounts.tenantId, tenantId)));
return row;
}Then remove the 3. Fallback account creation is not idempotent If the KV write succeeds but the DB write fails (or vice versa), a retry will re-enter the account-creation fallback and attempt to create a second "Mercury Checking" account. The KV dedup key was already written so the retry would bypass it. Consider at minimum adding a 🟡 Code Quality Issues4. Legacy path: This reduces classification accuracy for the new path. Pass 5. Hardcoded confidence magic numbers should be constants
const CONFIDENCE_KEYWORD_MATCH = '0.700';
const CONFIDENCE_SUSPENSE = '0.100';6. Status code inconsistency between legacy and native paths
Pick one and be consistent. 7. When Mercury sends an update event (e.g., a description or status change on an existing transaction), the handler acks with // TODO: handle 'update' events — patch description/status on existing transaction
return c.json({ received: true, operationType: event.operationType }, 200);🟢 What's Done Well
Test CoverageThe critical test path (real transaction event → classification → DB persistence) is still unchecked in the test plan. Before this goes to production for non-ARIBIA tenants, consider adding:
Summary
The replay-attack gap and cross-tenant query are the blockers — everything else can be a follow-up. Happy to pair on the fixes if useful. |
|
@coderabbitai review Please evaluate:
|
🧩 Analysis chain🏁 Script executed: Length of output: 57698
🔐 Security Review — PR
|
| Severity | Finding |
|---|---|
| 🚨 Critical | Signature bypass — unauthenticated transaction writes when no secret configured |
| 🔴 High | No HMAC timestamp replay-window validation |
| 🔴 High | tenantId URL param not validated against DB |
| 🟠 Medium | Cross-tenant lookupAccountByExternalId (timing oracle) |
| 🟠 Medium | Service token non-constant-time comparison in PUT /secret |
| 🟠 Medium | Full raw financial payload stored in KV (store sentinel instead) |
| 🟡 Low | No body size guard before c.req.text() |
| Potential breaking schema change on legacy Mercury path | |
| ✅ | No credential exposure in code |
| ✅ | No new supply chain dependencies |
The critical signature bypass (item 1) should be resolved before this is deployed to production tenants beyond the single test registration already in place.
🧠 Learnings used
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-18T01:23:18.963Z
Learning: Applies to server/routes/webhooks.ts : Implement idempotent webhook event processing by deduplicating events using KV storage with 7-day TTL for Stripe and Mercury webhook handlers
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
🧩 Analysis chain🏁 Script executed: Length of output: 57668
🔐 Security Review — PR
|
| # | Severity | Finding |
|---|---|---|
| 1 | 🔴 Critical | Signature verification fully bypassed when no KV secret exists |
| 2 | 🟠 High | No timestamp replay window — 7-day dedup bypass possible |
| 3 | 🟠 High | Auto-account creation behind unauthenticated path |
| 4 | 🟡 Medium | Cross-tenant DB scan in lookupAccountByExternalId |
| 5 | 🟡 Medium | Per-tenant webhook secrets stored in plain-text KV |
| 6 | 🟡 Medium | Full request body (potential PII) logged on unrecognized payloads |
| 7 | 🟡 Medium | Legacy endpoint schema change may break existing ChittyConnect adapters |
| 8 | ✅ Clean | No new external dependencies; supply chain risk is low |
🧠 Learnings used
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-18T01:23:18.963Z
Learning: Applies to server/routes/webhooks.ts : Implement idempotent webhook event processing by deduplicating events using KV storage with 7-day TTL for Stripe and Mercury webhook handlers
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
Code Review — PR #99: Mercury Native Webhook with HMAC VerificationGood direction overall — moving to per-tenant native webhook registration is the right architecture. The Web Crypto HMAC implementation is solid. A few security and correctness issues to address before merge. 🔴 Critical: Signature Skip Creates an Open Injection Windowif (secret) {
if (!signatureHeader || !(await verifyMercurySignature(...))) {
return c.json({ error: 'invalid_signature' }, 401);
}
} else {
console.warn('[webhook:mercury] No secret for tenant', tenantId, '— signature verification skipped');
}Any unauthenticated POST to Fix: Enforce a strict registration flow — reject with 401 if no secret exists, and document a separate one-time registration ping mechanism if Mercury requires it. Or only skip verification when the body is an empty/known Mercury ping pattern before any DB writes. 🔴 No Timestamp Replay Protection
Fix: Add a tolerance check (Stripe uses ±5 minutes): const ts = parseInt(timestamp, 10);
if (isNaN(ts) || Math.abs(Date.now() / 1000 - ts) > 300) return false;🔴 KV Dedup Written Before DB Write — Silent Loss on DB Failureawait kv.put(dedupKey, rawBody, { expirationTtl: 604800 });
// ... later ...
const created = await storage.createTransaction(...);If the DB write throws after KV is written, Mercury will retry but KV will return a duplicate hit and the transaction is permanently lost — no retry, no alert. Fix: Write to KV only after a successful DB persist, or wrap in a try/catch that deletes the KV key on DB failure. 🟡
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
server/routes/webhooks.ts (4)
201-202:⚠️ Potential issue | 🟠 MajorValidate the tenant and make account lookup tenant-scoped.
tenantIdis trusted from the URL, andlookupAccountByExternalIdsearches across all tenants before the tenant check. Validate the UUID, verify the tenant exists before any create/insert path, and resolve Mercury accounts with a tenant-scoped storage method.🛡️ Suggested direction
- const tenantId = c.req.param('tenantId'); + const tenantIdResult = z.string().uuid().safeParse(c.req.param('tenantId')); + if (!tenantIdResult.success) return c.json({ error: 'invalid_tenant_id' }, 400); + const tenantId = tenantIdResult.data;const db = createDb(c.env.DATABASE_URL); const storage = new SystemStorage(db); + + const tenant = await storage.getTenant(tenantId); + if (!tenant) return c.json({ error: 'unknown_tenant' }, 404);- const acct = await storage.lookupAccountByExternalId(`mercury:${mercuryAccountId}`); - if (acct && acct.tenantId === tenantId) { + const acct = await storage.getAccountByExternalId(`mercury:${mercuryAccountId}`, tenantId); + if (acct) { accountId = acct.id; }As per coding guidelines: “For multi-tenant operations in system mode, always include tenant context in queries to ensure data isolation and prevent cross-tenant data leaks”.
Also applies to: 272-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/webhooks.ts` around lines 201 - 202, The route handler starting at webhookRoutes.post('/api/webhooks/mercury/:tenantId') currently trusts tenantId from the URL and calls lookupAccountByExternalId across all tenants; fix this by (1) validating tenantId is a proper UUID, (2) resolving and asserting the tenant exists before any create/insert logic, and (3) changing the Mercury account resolution to use a tenant-scoped lookup (e.g., call lookupAccountByExternalId with a tenantId parameter or replace it with a tenant-aware method such as lookupAccountByExternalIdForTenant / resolveMercuryAccountWithTenant) so all queries include tenant context and prevent cross-tenant lookups or inserts.
239-245:⚠️ Potential issue | 🟠 MajorWrite a tenant-scoped sentinel after successful processing.
The current KV marker is global to
event.id, stores the full financial payload, and is written before DB persistence. Scope the key by tenant, store a minimal sentinel, and only promote it aftercreateTransactionsucceeds; otherwise Mercury retries can be incorrectly treated as duplicates.🛡️ Safer dedup pattern
- const dedupKey = `webhook:mercury:${event.id}`; + const dedupKey = `webhook:mercury:${tenantId}:${event.id}`; const existing = await kv.get(dedupKey); if (existing) { return c.json({ received: true, duplicate: true }, 200); } - await kv.put(dedupKey, rawBody, { expirationTtl: 604800 }); + // Put `await kv.put(dedupKey, '1', { expirationTtl: 604800 })` + // only after the transaction insert succeeds, or after confirming a DB duplicate.Based on learnings: “Implement idempotent webhook event processing by deduplicating events using KV storage with 7-day TTL for Stripe and Mercury webhook handlers”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/webhooks.ts` around lines 239 - 245, The dedup logic currently writes a global KV entry with the full payload before DB persistence; change it to first check a tenant-scoped key (e.g., `webhook:mercury:${tenantId}:${event.id}`) and only write a minimal sentinel (not rawBody) after createTransaction returns success. Specifically: use the existing kv.get(dedupKey) read but build dedupKey to include tenant identifier, avoid storing rawBody (store a small constant or timestamp), and move the kv.put call to run only after the createTransaction(...) call completes successfully so retries aren't marked duplicate before DB persistence.
32-50:⚠️ Potential issue | 🟠 MajorReject stale Mercury signatures before computing the HMAC.
timestampis parsed but never checked, so a captured valid(Mercury-Signature, body)can be replayed indefinitely once KV dedup expires or misses. Add a short replay window and small future-skew allowance.🔒 Proposed replay-window check
if (!timestamp || !signature) return false; + + const ts = Number.parseInt(timestamp, 10); + if (!Number.isFinite(ts)) return false; + + const nowMs = Date.now(); + const timestampMs = ts * 1000; + const maxAgeMs = 5 * 60 * 1000; + const maxFutureSkewMs = 60 * 1000; + if (nowMs - timestampMs > maxAgeMs || timestampMs - nowMs > maxFutureSkewMs) { + return false; + } const signedPayload = `${timestamp}.${rawBody}`;Please verify the exact current tolerance against Mercury’s webhook docs:
Mercury webhook Mercury-Signature timestamp replay tolerance t v1 HMAC SHA-256🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/webhooks.ts` around lines 32 - 50, In verifyMercurySignature, reject stale or far-future timestamps before computing the HMAC: parse the numeric timestamp variable and compare Date.now()/Math.floor to enforce a short replay window (e.g., 5 minutes) and a small future-skew allowance (e.g., 1-2 minutes) — if timestamp is outside [now - replayWindow, now + futureSkew] return false immediately; only then build signedPayload and compute/compare the v1 HMAC. Update the function signature verifyMercurySignature and the local variables timestamp/signature handling accordingly and confirm the exact window values against Mercury’s webhook docs.
204-220:⚠️ Potential issue | 🔴 CriticalFail closed when the tenant webhook secret is missing.
Line 219 allows unsigned JSON payloads to continue into transaction creation. Keep unsigned handling only for a provable registration ping, and reject all processable events when no tenant secret exists. Also update the stale “fallback” comment because line 211 only reads KV.
🔒 Proposed fail-closed gate
- // Falls back to MERCURY_WEBHOOK_SECRET env var (shared/legacy). - // Skips verification entirely if no secret found (allows registration ping). + // Skips verification only for empty registration pings; processable events require a per-tenant secret. const rawBody = await c.req.text(); const kv = c.env.FINANCE_KV; const secret = await kv.get(`webhook:mercury:secret:${tenantId}`); const signatureHeader = c.req.header('Mercury-Signature') ?? ''; - if (secret) { - if (!signatureHeader || !(await verifyMercurySignature(rawBody, signatureHeader, secret))) { - return c.json({ error: 'invalid_signature' }, 401); - } - } else { - console.warn('[webhook:mercury] No secret for tenant', tenantId, '— signature verification skipped'); + if (!secret) { + if (!rawBody.trim()) return c.json({ received: true }, 200); + console.error('[webhook:mercury] No secret configured for tenant', tenantId); + return c.json({ error: 'webhook_not_configured' }, 503); + } + + if (!signatureHeader || !(await verifyMercurySignature(rawBody, signatureHeader, secret))) { + return c.json({ error: 'invalid_signature' }, 401); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/webhooks.ts` around lines 204 - 220, The current webhook verification falls back to allowing unsigned payloads when no tenant secret exists; change this to fail closed: after retrieving secret from kv.get(`webhook:mercury:secret:${tenantId}`) and reading rawBody/signatureHeader, if secret is missing only allow the request to proceed when the payload is a provable registration ping (parse rawBody and check the event/type field explicitly for the registration ping value), otherwise return c.json({ error: 'invalid_signature' }, 401); keep the existing verifyMercurySignature(secret, rawBody, signatureHeader) path when secret exists, and update the comment above the block to remove the stale “fallback” mention of MERCURY_WEBHOOK_SECRET.
🤖 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 230-234: The current webhook handler logs the entire incoming body
on schema parse failure which may leak sensitive fields; update the failure
branch around mercuryEventSchema.safeParse(body) (variables: parsed, body,
tenantId) to stop logging body and instead log only non-sensitive validation
metadata such as parsed.error or parsed.success=false plus tenantId and a short
reason; remove or redact any direct body output and, if helpful, add a minimal
checksum/size flag (not contents) to aid debugging while preserving privacy.
- Around line 252-259: Validate event.mergePatch before using its fields:
replace the unchecked casting around patch (event.mergePatch ?? {}) and
downstream uses of amount, description, counterpartyName, postedAt, and
mercuryAccountId with a Zod schema (imported from `@shared/schema` or create one
matching the expected shape) that enforces types and optionality, parse/validate
mergePatch using schema.safeParse or parse, handle validation failures (log and
abort/ignore malformed payload), and then read the validated result to set
amount, description, counterpartyName, postedAt, and mercuryAccountId so no raw
"as string" or unchecked number casts are used.
- Around line 166-179: The webhook PUT handler
(webhookRoutes.put('/api/webhooks/mercury/:tenantId/secret', async (c) => { ...
})) must validate inputs and harden auth before writing KV: import and use Zod
schemas from `@shared/schema` to validate c.req.param('tenantId') and the parsed
body { secret } (wrap c.req.json(...) in try/catch to return a 400 with
consistent JSON on malformed JSON), perform authorization using a constant-time
comparison (e.g., crypto.timingSafeEqual) against
c.env.CHITTY_AUTH_SERVICE_TOKEN instead of direct token !== expected, and only
call kv.put (using c.env.FINANCE_KV) after both auth and validation succeed;
ensure all error responses use the same JSON error shape and correct HTTP status
codes.
---
Duplicate comments:
In `@server/routes/webhooks.ts`:
- Around line 201-202: The route handler starting at
webhookRoutes.post('/api/webhooks/mercury/:tenantId') currently trusts tenantId
from the URL and calls lookupAccountByExternalId across all tenants; fix this by
(1) validating tenantId is a proper UUID, (2) resolving and asserting the tenant
exists before any create/insert logic, and (3) changing the Mercury account
resolution to use a tenant-scoped lookup (e.g., call lookupAccountByExternalId
with a tenantId parameter or replace it with a tenant-aware method such as
lookupAccountByExternalIdForTenant / resolveMercuryAccountWithTenant) so all
queries include tenant context and prevent cross-tenant lookups or inserts.
- Around line 239-245: The dedup logic currently writes a global KV entry with
the full payload before DB persistence; change it to first check a tenant-scoped
key (e.g., `webhook:mercury:${tenantId}:${event.id}`) and only write a minimal
sentinel (not rawBody) after createTransaction returns success. Specifically:
use the existing kv.get(dedupKey) read but build dedupKey to include tenant
identifier, avoid storing rawBody (store a small constant or timestamp), and
move the kv.put call to run only after the createTransaction(...) call completes
successfully so retries aren't marked duplicate before DB persistence.
- Around line 32-50: In verifyMercurySignature, reject stale or far-future
timestamps before computing the HMAC: parse the numeric timestamp variable and
compare Date.now()/Math.floor to enforce a short replay window (e.g., 5 minutes)
and a small future-skew allowance (e.g., 1-2 minutes) — if timestamp is outside
[now - replayWindow, now + futureSkew] return false immediately; only then build
signedPayload and compute/compare the v1 HMAC. Update the function signature
verifyMercurySignature and the local variables timestamp/signature handling
accordingly and confirm the exact window values against Mercury’s webhook docs.
- Around line 204-220: The current webhook verification falls back to allowing
unsigned payloads when no tenant secret exists; change this to fail closed:
after retrieving secret from kv.get(`webhook:mercury:secret:${tenantId}`) and
reading rawBody/signatureHeader, if secret is missing only allow the request to
proceed when the payload is a provable registration ping (parse rawBody and
check the event/type field explicitly for the registration ping value),
otherwise return c.json({ error: 'invalid_signature' }, 401); keep the existing
verifyMercurySignature(secret, rawBody, signatureHeader) path when secret
exists, and update the comment above the block to remove the stale “fallback”
mention of MERCURY_WEBHOOK_SECRET.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Add tenant-scoped Mercury webhook endpoint (POST /api/webhooks/mercury/:tenantId) that accepts Mercury's native event envelope format (JSON Merge Patch) with HMAC-SHA256 signature verification via Mercury-Signature header. - Web Crypto HMAC verification (Workers-compatible, constant-time comparison) - Graceful fallback when MERCURY_WEBHOOK_SECRET not yet configured - Auto-classify transactions at ingest (L0→L1 keyword match) - Auto-create Mercury checking account if none exists for tenant - KV-based idempotency + DB-level external_id dedup - Legacy ChittyConnect-normalized path preserved at /api/webhooks/mercury - Add lookupAccountByExternalId storage method (cross-tenant) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mercury returns a unique signing secret per webhook registration, so a single MERCURY_WEBHOOK_SECRET env var doesn't work for multiple tenants. - Look up secret from KV at `webhook:mercury:secret:<tenantId>` - Fall back to MERCURY_WEBHOOK_SECRET env var (legacy/shared) - Skip verification if no secret found (allows registration ping) - Add PUT /api/webhooks/mercury/:tenantId/secret for storing secrets Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion When no per-tenant KV secret exists, skip verification instead of falling back to MERCURY_WEBHOOK_SECRET (which belongs to a different tenant and causes 401 on registration pings). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
chitcommit
force-pushed
the
feat/mercury-native-webhooks
branch
from
2ae2263 to
8a2926b
Compare
|
@coderabbitai review Please evaluate:
|
|
Tip For best results, initiate chat on the files or code changes.
🔐 Security Review — PR
|
| Vector | Current State | Risk |
|---|---|---|
MERCURY_WEBHOOK_SECRET env var |
Consumed from c.env, not logged |
✅ Safe |
| Per-tenant KV secrets | Stored as plain-text KV values | wrangler kv key get |
PUT /secret token comparison |
token !== expected — non-constant-time |
|
| KV dedup values | Stores full rawBody (financial payload) for 7 days |
|
| Unrecognized payload log | console.warn(..., { tenantId, body }) — full body logged |
|
| HMAC key material | Never logged | ✅ Safe |
PUT /secret response |
Returns only { stored: true, tenantId } |
✅ Safe |
Recommended fixes:
// 1. Constant-time token comparison for PUT /secret
function timingSafeEqual(a: string, b: string): boolean {
if (a.length !== b.length) return false;
let r = 0;
for (let i = 0; i < a.length; i++) r |= a.charCodeAt(i) ^ b.charCodeAt(i);
return r === 0;
}
// 2. Store sentinel in KV, not raw body
await kv.put(dedupKey, '1', { expirationTtl: 604800 });
// 3. Redact body from unrecognized-payload log
console.warn('[webhook:mercury] Unrecognized payload, acking', { tenantId, bodyKeys: Object.keys(body) });3. Dependency Supply Chain
✅ No new external dependencies introduced. The implementation relies on:
hono— already in the dependency treezod— already in the dependency treedrizzle-orm— already in the dependency tree- Web Crypto API (
crypto.subtle.importKey,crypto.subtle.sign) — built into the Cloudflare Workers runtime; no third-party HMAC library added
The HMAC construction (${timestamp}.${rawBody}, SHA-256, constant-time comparison via charCodeAt XOR) is implemented correctly and follows the pattern used for Stripe in this codebase. Supply chain risk is low.
4. Breaking API Changes
| Endpoint | Change | Risk |
|---|---|---|
POST /api/webhooks/mercury (legacy) |
Input schema replaced: old Mercury envelope → normalizedEnvelopeSchema + normalizedTransactionSchema |
🟠 Breaking for any ChittyConnect version still sending the old envelope shape |
POST /api/webhooks/mercury (legacy) |
Duplicate response: now returns 202; native path returns 200 |
|
POST /api/webhooks/mercury/:tenantId |
New endpoint — additive | ✅ Non-breaking |
PUT /api/webhooks/mercury/:tenantId/secret |
New endpoint — additive | ✅ Non-breaking |
Recommendation: Verify ChittyConnect's outbound payload schema matches normalizedEnvelopeSchema before merging. If the schema of the legacy endpoint changed, a contract test between ChittyConnect and this endpoint is advisable. Also align duplicate-event response codes across both paths for consistency.
Priority Summary
| Priority | Item |
|---|---|
| 🚨 Fix before production | Signature bypass — return 401 when no secret configured |
| 🔴 Fix before production | Add 5-minute timestamp replay window in verifyMercurySignature |
| 🔴 Fix before production | Validate tenantId against the tenants table |
| 🟠 Address soon | Scope lookupAccountByExternalId to the tenant |
| 🟠 Address soon | Constant-time comparison in PUT /secret |
| 🟠 Address soon | Store '1' sentinel in KV instead of full rawBody |
| 🟠 Address soon | Redact body from unrecognized-payload log |
| 🟡 Nice to have | Body size guard before c.req.text() |
| 🟡 Verify | ChittyConnect payload compatibility with updated legacy schema |
[review]
🧠 Learnings used
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-18T01:23:18.963Z
Learning: Applies to server/routes/webhooks.ts : Implement idempotent webhook event processing by deduplicating events using KV storage with 7-day TTL for Stripe and Mercury webhook handlers
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-18T01:23:18.963Z
Learning: Applies to server/routes/stripe.ts : For Stripe webhooks, verify signatures using `STRIPE_WEBHOOK_SECRET` before processing events in route handlers
Code Review — PR #99: Mercury Native Webhook + HMAC VerificationOverall this is a solid addition. The Web Crypto HMAC implementation is Workers-compatible, the dual-layer dedup (KV + DB) is the right pattern, and preserving the legacy ChittyConnect path is the correct backwards-compat call. A few things need attention before merge. 🔴 Security Issues1. No timestamp replay protection in The function parses the ```typescript 2. Signature skip silently accepts all traffic when no KV secret is present ```typescript This is only safe during Mercury's registration ping. But after registration, if the KV entry is accidentally evicted or the 3. ```typescript The route checks 🟡 Potential Bugs4. Race condition between KV dedup write and DB unique constraint The sequence is:
If step 4 throws a unique-constraint violation, the webhook returns 500 instead of the correct 200/duplicate response. Since the DB dedup is the real safety net, wrapping 5. Silent drop on unrecognized payloads ```typescript A verification ping and a malformed real event both land here. This is acceptable, but note that the event is acknowledged and dropped with no record in the DB. If Mercury's schema evolves (new required fields), real transactions could be silently lost. Consider at least logging the Zod error details alongside the body so post-incident analysis is possible. 🟡 Code Quality6. Hardcoded confidence strings duplicated
```typescript 7. Auto-creating accounts on fallback is high-blast-radius ```typescript Creating a new account in the fallback path could lead to duplicate accounts over time (e.g., account resolution misses on the first request, then 8. Removed comments on the legacy path reduce readability Several genuinely useful comments were stripped from 🟢 What's Done Well
Summary
The timestamp replay check (#1) and the unguarded auto-create (#7) are the most important fixes before this goes to production. The cross-tenant lookup (#3) and the silent-skip auth (#2) are close seconds. The race condition (#4) is low-probability but the fix is trivial. Happy to pair on any of these if useful. |
- Add JSON parse guard on PUT /api/webhooks/mercury/:tenantId/secret - Truncate unrecognized payload logs to keys-only (avoid leaking financial data) Addresses review comments from Copilot and CodeRabbit on PR #99. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
|
Tip For best results, initiate chat on the files or code changes.
🔐 Security Review — PR
|
| Vector | Current State | Risk |
|---|---|---|
MERCURY_WEBHOOK_SECRET env var |
Consumed from c.env, never logged |
✅ Safe |
| Per-tenant KV secrets | Stored as plain-text KV values | wrangler kv key get |
PUT /secret token comparison |
token !== expected — non-constant-time |
|
| KV dedup values | Stores full rawBody (financial payload) 7 days |
|
| Unrecognized payload log | console.warn(..., { tenantId, body }) — full body emitted |
|
| HMAC key material | Never logged | ✅ Safe |
PUT /secret response |
Returns only { stored: true, tenantId } |
✅ Safe |
Recommended fixes:
// 1. Constant-time token comparison for PUT /secret
function timingSafeEqual(a: string, b: string): boolean {
if (a.length !== b.length) return false;
let r = 0;
for (let i = 0; i < a.length; i++) r |= a.charCodeAt(i) ^ b.charCodeAt(i);
return r === 0;
}
if (!token || !timingSafeEqual(token, expected)) return c.json({ error: 'unauthorized' }, 401);
// 2. Store a sentinel in KV — not the full raw body
await kv.put(dedupKey, '1', { expirationTtl: 604800 });
// 3. Redact body from unrecognized-payload log
console.warn('[webhook:mercury] Unrecognized payload, acking', {
tenantId,
bodyKeys: Object.keys(body),
});3. Dependency Supply Chain
✅ No new external dependencies introduced. The implementation relies exclusively on:
hono,zod,drizzle-orm— already in the dependency tree- Web Crypto API (
crypto.subtle.importKey+crypto.subtle.sign) — built into the Cloudflare Workers runtime; no third-party HMAC library added
The HMAC construction (${timestamp}.${rawBody}, SHA-256, constant-time XOR comparison) follows the same pattern used for Stripe in this codebase. Supply chain risk is low.
4. Breaking API Changes
| Endpoint | Change | Risk |
|---|---|---|
POST /api/webhooks/mercury (legacy) |
Input schema replaced: old Mercury envelope → normalizedEnvelopeSchema + normalizedTransactionSchema |
🟠 Breaking for any ChittyConnect version still sending the old shape |
POST /api/webhooks/mercury (legacy) |
Duplicate response code: 202; native path returns 200 |
|
POST /api/webhooks/mercury/:tenantId |
New endpoint — additive | ✅ Non-breaking |
PUT /api/webhooks/mercury/:tenantId/secret |
New endpoint — additive | ✅ Non-breaking |
Recommendation: Verify ChittyConnect's outbound payload schema matches normalizedEnvelopeSchema before merging, and align duplicate-event response codes (200 vs 202) across both paths.
Priority Summary
| Priority | Item |
|---|---|
| 🚨 Fix before production | Signature bypass — return 401 when no secret is configured |
| 🔴 Fix before production | Add 5-minute timestamp replay window in verifyMercurySignature |
| 🔴 Fix before production | Validate tenantId against the tenants table |
| 🟠 Address soon | Scope lookupAccountByExternalId to the tenant |
| 🟠 Address soon | Constant-time comparison in PUT /secret |
| 🟠 Address soon | Store '1' sentinel in KV instead of full rawBody |
| 🟠 Address soon | Redact body from unrecognized-payload log |
| 🟡 Nice to have | Body size guard before c.req.text() |
| 🟡 Verify | ChittyConnect payload compatibility with updated legacy schema |
🧠 Learnings used
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-23T17:50:57.008Z
Learning: Validate all webhook signatures using provider-specific secrets (STRIPE_WEBHOOK_SECRET, MERCURY service auth) and implement idempotent processing with KV deduplication using 7-day TTL
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-23T17:50:57.008Z
Learning: Applies to **/server/routes/webhooks.ts : For Stripe webhook events, verify signatures using webhook.constructEvent() with STRIPE_WEBHOOK_SECRET and store raw request body as string (not parsed JSON)
Learnt from: CR
Repo: chittyapps/chittyfinance PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-23T17:50:57.008Z
Learning: Implement OAuth security using HMAC-SHA256 signed state tokens from server/lib/oauth-state-edge.ts with 10-minute expiration for all third-party OAuth integrations (Wave, Stripe, Mercury)
Code Review — PR #99: Mercury Native Webhook with HMAC VerificationOverviewThis PR introduces a tenant-scoped Mercury native webhook endpoint ( 🔴 Security Issues1. Signature skip creates an unauthenticated ingestion window if (secret) {
if (!signatureHeader || !(await verifyMercurySignature(...))) {
return c.json({ error: 'invalid_signature' }, 401);
}
} else {
console.warn('[webhook:mercury] No secret for tenant', tenantId, '— signature verification skipped');
}Skipping verification when no KV secret exists means any request with a known (or guessed) tenant UUID is accepted and persisted as real transactions. Tenant UUIDs are referenced in other API responses, making them guessable. The registration-ping use case doesn't justify this — Mercury's verification ping is typically an empty body (already handled: the 2. No timestamp freshness check = replay attacks
const now = Math.floor(Date.now() / 1000);
if (Math.abs(now - parseInt(timestamp, 10)) > 300) return false;3. async lookupAccountByExternalId(externalId: string) {
const [row] = await this.db.select().from(schema.accounts)
.where(eq(schema.accounts.externalId, externalId));
return row;
}The project convention (enforced throughout 🟡 Correctness Issues4. Auto-creating accounts races on concurrent webhooks const created = await storage.createAccount({
tenantId,
name: 'Mercury Checking',
type: 'checking',
institution: 'Mercury',
externalId: mercuryAccountId ? `mercury:${mercuryAccountId}` : undefined,
});If two transaction webhooks arrive simultaneously for a tenant with no accounts, both pass the 5. KV dedup written before DB write succeeds await kv.put(dedupKey, rawBody, { expirationTtl: 604800 });
// ... (several operations later)
const created = await storage.createTransaction(...);If the DB write fails after the KV entry is stored, the event is permanently deduplicated — the transaction is lost but Mercury will never successfully retry. Move the KV write to after the successful 6. HTTP 200 vs 202 inconsistency between paths The legacy path returns 🟡 Code Quality Issues7. Type annotation mismatch on secret parsing let parsed: { secret?: string };
try {
parsed = await c.req.json<{ secret: string }>();The outer annotation makes 8. const accounts = await storage.getAccounts(tenantId);
const active = accounts.find((a) => a.isActive);This fetches all accounts for the tenant to find the first active one. For tenants with many accounts this is wasteful. Consider adding a 9. KV dedup key namespace is shared between both paths Both the new native path and the legacy path use 🟢 What's Good
Summary
The unauthenticated ingestion window (#1) and missing timestamp validation (#2) should be addressed before merge — both are exploitable from the public internet. The cross-tenant account lookup (#3) and race on account creation (#4) are lower-urgency but worth fixing before this path sees production load.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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`:
- Line 318: The transaction-type ternary in webhooks.ts that sets type: amount
>= 0 ? 'income' : 'expense' misclassifies zero-value transactions as 'income';
change the condition to use amount > 0 so zero amounts are not treated as income
(update the expression in the function/handler that builds the transaction
object where the ternary is used, and audit any downstream logic/tests that rely
on transaction.type to ensure zero-amount behaviors are handled explicitly).
- Around line 215-251: Before calling c.req.text() and storing rawBody into KV,
validate the request size by checking the Content-Length header (or reject if
absent/untrusted) and reject requests larger than a safe threshold (e.g., 1 MB)
with a 413 response; update the handler around c.req.text(), signature
verification and the KV write (dedupKey/kv.put) to perform this early check and
avoid reading/keeping oversized bodies or putting them into KV. Ensure the logic
uses c.req.header('Content-Length') to parse and compare against the limit,
returns c.json({ error: 'payload_too_large' }, 413) when exceeded, and only then
proceeds to call c.req.text(), verifyMercurySignature, parse JSON, and
kv.put(dedupKey, rawBody,...).
- Line 249: The native webhook handler currently returns c.json({ received:
true, duplicate: true }, 200) (seen in server/routes/webhooks.ts) which is
inconsistent with the legacy path that returns 202 for duplicates; change the
duplicate-response status in the native path to 202 to match the legacy/Wave
behavior (update all native duplicate return sites, e.g., the occurrences with
the exact object { received: true, duplicate: true }), and run/update any tests
or integration checks that assert the previous 200 to expect 202.
- Around line 302-310: The current flow creates a Mercury Checking account via
storage.createAccount without externalId when mercuryAccountId is null, causing
future events with an external id to not match lookupAccountByExternalId and
possibly attach to the wrong account; change the logic so we either (A) do not
auto-create an account unless mercuryAccountId is present (i.e., early-return or
postpone creation) or (B) if you must create a placeholder, persist a flag or
placeholder externalId and ensure subsequent handlers update that record’s
externalId when mercuryAccountId arrives; implement the fix by updating the
branch that assigns accountId (the code calling storage.createAccount) and
add/update a path in the handler that receives a mercuryAccountId to find an
existing placeholder (by name/institution or a pending flag) and call
storage.updateAccount to set externalId so lookupAccountByExternalId will match,
and ensure lookupAccountByExternalId remains the primary match when
mercuryAccountId is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
/api/webhooks/mercury/:tenantId) accepting Mercury's native JSON Merge Patch event formatMercury-Signatureheader (Web Crypto, Workers-compatible)MERCURY_WEBHOOK_SECRETnot configured (allows Mercury verification ping during registration)/api/webhooks/mercuryTest plan
MERCURY_WEBHOOK_SECRETstored in Cloudflare Secrets Store + Workers secrets🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements