From e629027c0ce0065c2d07b2558b791a685a623a60 Mon Sep 17 00:00:00 2001 From: Dread Date: Fri, 7 Aug 2026 23:18:42 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat(fygaro):=20payment=20webhook=20?= =?UTF-8?q?=E2=80=94=20record,=20notify,=20and=20flag-gated=20auto-credit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fygaro card top-ups previously left no server-side record: the app opened Fygaro's hosted payment button and nothing ever came back to Flash, so every top-up required manual PayPal-email archaeology to attribute and a manual treasury transfer to credit. This implements the missing recording path (TopUp V1 design), mirroring the Bridge webhook server structure. - fygaro-webhook server (default port 4010): raw-body capture, /health, enabled guard, rate limit, HMAC-SHA-256 signature verification per Fygaro's hook spec (Fygaro-Signature t=..,v1=..; Fygaro-Key-ID selects the shared secret; multiple secrets supported for rotation) - /payment handler: attributes the payment via customReference (Flash username), writes the ERPNext audit row (Bridge Transfer Request, provider=Fygaro, Fiat Received) for every payment — including unattributed ones, which alert ops with the payer email instead of being silently dropped — and posts to the Discord ops activity feed - fygaro.credit.enabled (default OFF): when on, USD payments are auto-credited from the bankowner treasury to the user's cash wallet via intraledger send, idempotent on the Fygaro transaction id (ENG-530 pattern), then the audit row is promoted to Completed. Credit failures alert critical and leave the row at Fiat Received for manual follow-up. - config: fygaro block (enabled / webhook.port / webhook.secrets / webhook.timestampSkewMs / credit.enabled), default-off baseline - unit tests: signature verification, enabled guard, and the full payment handler matrix (attribution, dedupe, credit on/off, non-USD, failures) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- dev/config/base-config.yaml | 13 + package.json | 1 + src/config/schema.ts | 35 +++ src/config/schema.types.d.ts | 15 + src/config/yaml.ts | 2 + src/servers/fygaro-webhook-server.ts | 9 + src/services/alerts/dedup-key.ts | 5 + src/services/alerts/index.types.ts | 7 +- .../frappe/BridgeTransferRequestWriter.ts | 76 +++++ .../frappe/models/BridgeTransferRequest.ts | 2 +- .../fygaro/webhook-server/credit-topup.ts | 118 ++++++++ src/services/fygaro/webhook-server/index.ts | 76 +++++ .../middleware/enabled-guard.ts | 27 ++ .../middleware/verify-signature.ts | 113 +++++++ .../fygaro/webhook-server/routes/payment.ts | 282 ++++++++++++++++++ .../webhook-server/enabled-guard.spec.ts | 61 ++++ .../fygaro/webhook-server/payment.spec.ts | 262 ++++++++++++++++ .../webhook-server/verify-signature.spec.ts | 215 +++++++++++++ 18 files changed, 1317 insertions(+), 2 deletions(-) create mode 100644 src/servers/fygaro-webhook-server.ts create mode 100644 src/services/fygaro/webhook-server/credit-topup.ts create mode 100644 src/services/fygaro/webhook-server/index.ts create mode 100644 src/services/fygaro/webhook-server/middleware/enabled-guard.ts create mode 100644 src/services/fygaro/webhook-server/middleware/verify-signature.ts create mode 100644 src/services/fygaro/webhook-server/routes/payment.ts create mode 100644 test/flash/unit/services/fygaro/webhook-server/enabled-guard.spec.ts create mode 100644 test/flash/unit/services/fygaro/webhook-server/payment.spec.ts create mode 100644 test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts diff --git a/dev/config/base-config.yaml b/dev/config/base-config.yaml index 0567a692f..a37e6c70a 100644 --- a/dev/config/base-config.yaml +++ b/dev/config/base-config.yaml @@ -15,6 +15,19 @@ ibex: # production overrides to enforce the allowlist; empty disables it (ISL-112). allowedIps: [] +fygaro: + enabled: false # flags-off baseline; enable per environment via config overrides + webhook: + port: 4010 + timestampSkewMs: 300000 + # HMAC shared secrets keyed by the Fygaro-Key-ID header value. + # Real secrets injected via config overrides; never commit a real secret. + secrets: {} + credit: + # Phase gate: with credit OFF the webhook only records the payment and + # notifies ops; the treasury -> user transfer stays manual. + enabled: false + bridge: enabled: false # flags-off baseline; enable per environment via config overrides apiKey: "" # real key injected via config overrides; never commit a real key diff --git a/package.json b/package.json index 8d366cb15..a3832d2ae 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "migrate-mongo-up": "migrate-mongo up -f './src/migrations/migrate-mongo-config.js'", "gen-test-jwt": "ts-node ./dev/bin/gen-test-jwt.ts", "bridge-webhook": ". ./.env && ts-node --transpile-only -r tsconfig-paths/register src/servers/bridge-webhook-server.ts --configPath dev/config/base-config.yaml", + "fygaro-webhook": ". ./.env && ts-node --transpile-only -r tsconfig-paths/register src/servers/fygaro-webhook-server.ts --configPath dev/config/base-config.yaml", "replay-bridge-events": "yarn build && node lib/scripts/replay-bridge-events.js", "reconcile-bridge-ibex-deposits": "yarn build && node lib/scripts/reconcile-bridge-ibex-deposits.js" }, diff --git a/src/config/schema.ts b/src/config/schema.ts index b6cd15a04..0da095190 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -772,6 +772,41 @@ export const configSchema = { // stay hidden unless explicitly enabled per the v0.6.0 flag ramp. default: { enabled: false }, }, + fygaro: { + type: "object", + properties: { + enabled: { type: "boolean" }, + webhook: { + type: "object", + properties: { + port: { type: "integer", default: 4010 }, + // HMAC shared secrets keyed by the Fygaro-Key-ID header value. + // Multiple entries support secret rotation. + secrets: { + type: "object", + additionalProperties: { type: "string" }, + default: {}, + }, + timestampSkewMs: { type: "integer", default: 300000 }, + }, + additionalProperties: false, + default: {}, + }, + credit: { + type: "object", + properties: { + enabled: { type: "boolean", default: false }, + }, + additionalProperties: false, + // Phase gate: with credit OFF the webhook only records + notifies; + // the treasury -> user transfer stays manual. + default: { enabled: false }, + }, + }, + additionalProperties: false, + // Default OFF baseline (mirrors topup); enable per environment via overrides. + default: { enabled: false }, + }, frappe: { type: "object", properties: { diff --git a/src/config/schema.types.d.ts b/src/config/schema.types.d.ts index 1de97bec3..2fd2de7c2 100644 --- a/src/config/schema.types.d.ts +++ b/src/config/schema.types.d.ts @@ -68,6 +68,20 @@ type BridgeConfig = { webhook: BridgeWebhook } +type FygaroWebhookConfig = { + port: number + secrets: Record + timestampSkewMs: number +} + +type FygaroConfig = { + enabled: boolean + webhook: FygaroWebhookConfig + credit: { + enabled: boolean + } +} + type CashoutEmail = { to: string from: string @@ -237,6 +251,7 @@ type YamlSchema = { topup: { enabled: boolean } + fygaro: FygaroConfig sendgrid: SendGridConfig frappe: FrappeConfig fcmTopics: { diff --git a/src/config/yaml.ts b/src/config/yaml.ts index b4a2e7f14..1b03623d8 100644 --- a/src/config/yaml.ts +++ b/src/config/yaml.ts @@ -419,4 +419,6 @@ export const IbexConfig = yamlConfig.ibex as IbexConfig export const BridgeConfig = yamlConfig.bridge as BridgeConfig +export const FygaroConfig = yamlConfig.fygaro as FygaroConfig + export const FrappeConfig = yamlConfig.frappe as FrappeConfig diff --git a/src/servers/fygaro-webhook-server.ts b/src/servers/fygaro-webhook-server.ts new file mode 100644 index 000000000..9ed19bc29 --- /dev/null +++ b/src/servers/fygaro-webhook-server.ts @@ -0,0 +1,9 @@ +import { startFygaroWebhookServer } from "@services/fygaro/webhook-server" +import { baseLogger } from "@services/logger" +import { setupMongoConnection } from "@services/mongodb" + +if (require.main === module) { + setupMongoConnection() + .then(async () => startFygaroWebhookServer()) + .catch((err) => baseLogger.error(err, "fygaro webhook server error")) +} diff --git a/src/services/alerts/dedup-key.ts b/src/services/alerts/dedup-key.ts index 2a1cb2584..b9d5245f9 100644 --- a/src/services/alerts/dedup-key.ts +++ b/src/services/alerts/dedup-key.ts @@ -28,6 +28,11 @@ export const generateDedupKey = { `ibex:reconcile:ibex-without-bridge:${txHash.toLowerCase()}`, ibexReconcileFailed: (txHash: string) => `ibex:reconcile:failed:${txHash.toLowerCase()}`, + erpnextFygaroAudit: (transactionId: string) => `erpnext-audit:fygaro:${transactionId}`, + fygaroWebhookPayment: (transactionId: string) => + `fygaro-webhook:payment:${transactionId}`, + fygaroUnattributed: (transactionId: string) => `fygaro:unattributed:${transactionId}`, + fygaroCreditFailed: (transactionId: string) => `fygaro:credit-failed:${transactionId}`, } export const normalizeDedupKey = (key: string): string => diff --git a/src/services/alerts/index.types.ts b/src/services/alerts/index.types.ts index b93b6c91c..54b56ff72 100644 --- a/src/services/alerts/index.types.ts +++ b/src/services/alerts/index.types.ts @@ -2,7 +2,12 @@ export type AlertSeverity = "critical" | "warning" -export type AlertSource = "bridge-webhook" | "bridge-api" | "ibex" | "erpnext-audit" +export type AlertSource = + | "bridge-webhook" + | "bridge-api" + | "ibex" + | "erpnext-audit" + | "fygaro-webhook" export interface BridgeAlert { dedupKey: string diff --git a/src/services/frappe/BridgeTransferRequestWriter.ts b/src/services/frappe/BridgeTransferRequestWriter.ts index 432d40089..9caa99732 100644 --- a/src/services/frappe/BridgeTransferRequestWriter.ts +++ b/src/services/frappe/BridgeTransferRequestWriter.ts @@ -205,6 +205,82 @@ export const writeIbexCryptoReceiveRequest = async ({ ) } +// Fygaro card top-up audit row: fiat captured on Fygaro's side, recorded the +// moment the payment webhook lands. `fygaro:` prefixed request ids keep these +// rows disjoint from Bridge deposit ids and IBEX settle rows. +export const writeFygaroTopupRequest = async ({ + transactionId, + amount, + currency, + accountId, + createdAt, + rawPayload, +}: { + transactionId: string + amount: string + currency: string + accountId?: AccountId | string + createdAt?: string + rawPayload: unknown +}): Promise => { + return upsert( + new BridgeTransferRequest({ + requestId: `fygaro:${transactionId}`, + transactionType: BridgeTransferRequestTransactionType.Topup, + status: BridgeTransferRequestStatus.FiatReceived, + provider: "Fygaro", + asset: "USD", + network: "Card", + amount: String(amount), + currency: String(currency), + accountId, + sourceEventId: transactionId, + sourceEventType: "fygaro.payment", + sourceSystemsSeen: ["fygaro_webhook"], + firstSeenAt: createdAt, + rawPayload, + }), + ) +} + +// Called after the treasury -> user intraledger credit succeeds: promotes the +// Fygaro topup row to Completed and stamps the credited wallet on it. The +// upsert's monotonic status guard makes this safe to repeat. +export const completeFygaroTopup = async ({ + transactionId, + accountId, + walletId, + amount, + currency, + rawPayload, +}: { + transactionId: string + accountId: AccountId | string + walletId: WalletId | string + amount: string + currency: string + rawPayload: unknown +}): Promise => { + return upsert( + new BridgeTransferRequest({ + requestId: `fygaro:${transactionId}`, + transactionType: BridgeTransferRequestTransactionType.Topup, + status: BridgeTransferRequestStatus.Completed, + provider: "Fygaro", + asset: "USD", + network: "Card", + amount: String(amount), + currency: String(currency), + accountId, + walletId, + sourceEventId: transactionId, + sourceEventType: "fygaro.payment", + sourceSystemsSeen: ["fygaro_webhook", "ibex_intraledger_credit"], + rawPayload, + }), + ) +} + type BridgeCashoutWriteInput = { transferId: string amount: string diff --git a/src/services/frappe/models/BridgeTransferRequest.ts b/src/services/frappe/models/BridgeTransferRequest.ts index 32dcc675f..a4341180d 100644 --- a/src/services/frappe/models/BridgeTransferRequest.ts +++ b/src/services/frappe/models/BridgeTransferRequest.ts @@ -17,7 +17,7 @@ export type BridgeTransferRequestInput = { status: BridgeTransferRequestStatus amount: string currency: string - provider?: "Bridge" + provider?: "Bridge" | "Fygaro" asset?: string network?: string developerFee?: string diff --git a/src/services/fygaro/webhook-server/credit-topup.ts b/src/services/fygaro/webhook-server/credit-topup.ts new file mode 100644 index 000000000..20ac03a19 --- /dev/null +++ b/src/services/fygaro/webhook-server/credit-topup.ts @@ -0,0 +1,118 @@ +import { PaymentSendStatus } from "@domain/bitcoin/lightning" +import { WalletCurrency } from "@domain/shared" +import { AccountsRepository, WalletsRepository } from "@services/mongoose" +import { baseLogger } from "@services/logger" + +/** + * Credits a verified Fygaro card payment to the payer's Flash account: + * an intraledger USD send from the bank-owner treasury to the user's cash + * wallet. Mirrors the referral-reward payout path (award-referral-reward.ts), + * the in-repo precedent for treasury -> user credits. + * + * Safety properties: + * - The send runs under withPaymentIdempotency keyed on the Fygaro + * transaction id, so a webhook replay can never double-credit (ENG-530). + * - PaymentSendStatus.Pending means money has probably left the treasury — + * it is reported as credited (never retried) and left to ops to confirm, + * the same never-risk-a-double-pay stance the referral payout takes. + */ +const TREASURY_ROLE = "bankowner" + +export class FygaroCreditError extends Error { + step: string + constructor(step: string, message: string) { + super(message) + this.name = "FygaroCreditError" + this.step = step + } +} + +const walletsFor = async (accountId: AccountId): Promise => { + const wallets = await WalletsRepository().listByAccountId(accountId) + return wallets instanceof Error ? [] : wallets +} + +export const creditFygaroTopup = async ({ + recipientAccountId, + amountCents, + transactionId, +}: { + recipientAccountId: AccountId + amountCents: number + transactionId: string +}): Promise< + { walletId: WalletId; status: "success" | "pending" } | FygaroCreditError +> => { + if (!Number.isInteger(amountCents) || amountCents <= 0) { + return new FygaroCreditError("validate-amount", `invalid amount: ${amountCents}`) + } + + const treasuryAccount = await AccountsRepository().findByRole(TREASURY_ROLE) + if (treasuryAccount instanceof Error) { + return new FygaroCreditError( + "resolve-treasury", + `no account holds the '${TREASURY_ROLE}' role`, + ) + } + + // Prefer the USDT wallet (the active cash wallet on every account — see + // accounts/create-account.ts), falling back to the legacy USD wallet. + const treasuryWallets = await walletsFor(treasuryAccount.id) + const fundingWallet = + treasuryWallets.find((w) => w.currency === WalletCurrency.Usdt) ?? + treasuryWallets.find((w) => w.currency === WalletCurrency.Usd) + if (!fundingWallet) { + return new FygaroCreditError( + "resolve-treasury-wallet", + "treasury account has no USDT or USD wallet", + ) + } + + // Recipients must hold a wallet in the funding wallet's currency — + // send-intraledger rejects cross-currency sends. + const recipientWallet = (await walletsFor(recipientAccountId)).find( + (w) => w.currency === fundingWallet.currency, + ) + if (!recipientWallet) { + return new FygaroCreditError( + "resolve-recipient-wallet", + `recipient has no ${fundingWallet.currency} wallet`, + ) + } + + // Lazy-import so merely importing this module doesn't pull the IBEX client + // (and its module-load side effects) into unrelated code paths. + const { intraledgerPaymentSendWalletIdForUsdWallet } = await import( + "@app/payments/send-intraledger" + ) + const result = await intraledgerPaymentSendWalletIdForUsdWallet({ + senderWalletId: fundingWallet.id, + recipientWalletId: recipientWallet.id, + amount: amountCents, + memo: `Card top-up (Fygaro ${transactionId})`, + idempotencyKey: `fygaro:${transactionId}` as IdempotencyKey, + }) + + if (result instanceof Error) { + baseLogger.error( + { err: result, transactionId, recipientAccountId }, + "fygaro credit: intraledger send returned an error", + ) + return new FygaroCreditError("intraledger-send", result.message) + } + if (result === PaymentSendStatus.Success) { + return { walletId: recipientWallet.id, status: "success" } + } + if (result === PaymentSendStatus.Pending) { + // Money has probably left the treasury: report credited, never re-pay. + baseLogger.warn( + { transactionId, recipientAccountId }, + "fygaro credit: send pending — treating as credited for idempotency", + ) + return { walletId: recipientWallet.id, status: "pending" } + } + return new FygaroCreditError( + "intraledger-send", + `unexpected payment status: ${String(result)}`, + ) +} diff --git a/src/services/fygaro/webhook-server/index.ts b/src/services/fygaro/webhook-server/index.ts new file mode 100644 index 000000000..652756eac --- /dev/null +++ b/src/services/fygaro/webhook-server/index.ts @@ -0,0 +1,76 @@ +/** + * Fygaro Webhook Server + * Standalone Express server for handling Fygaro payment-button hook events. + * Mirrors the Bridge webhook server structure. + * + * Runs on port configured in FygaroConfig.webhook.port (default: 4010) + * Routes: /payment + */ + +import express from "express" +import rateLimitMiddleware from "express-rate-limit" + +import { FygaroConfig } from "@config" +import { baseLogger } from "@services/logger" + +import { verifyFygaroSignature } from "./middleware/verify-signature" +import { fygaroEnabledGuard } from "./middleware/enabled-guard" +import { paymentHandler } from "./routes/payment" + +type RawBodyRequest = express.Request & { rawBody?: string } + +// `validate: { xForwardedForHeader: false }`: without Express `trust proxy`, +// express-rate-limit v7 throws ERR_ERL_UNEXPECTED_X_FORWARDED_FOR on any +// request carrying X-Forwarded-For (i.e. anything behind an LB), turning +// every webhook into a 500. `trust proxy` is set on the app (see below); the +// skip stays so a future trust-proxy misconfiguration degrades to one shared +// bucket instead of an outage. +const webhookRateLimit = rateLimitMiddleware({ + windowMs: 60_000, + limit: 120, + standardHeaders: true, + legacyHeaders: false, + validate: { xForwardedForHeader: false }, +}) + +export const startFygaroWebhookServer = () => { + const app = express() + + // Exactly one XFF-writing hop sits in front of the pod: the nginx ingress + // (the DO load balancer is L4 and does not touch headers). + app.set("trust proxy", 1) + + // Middleware - MUST capture raw body for signature verification + app.use( + express.json({ + verify: (req, res, buf) => { + if (res.writableEnded) { + return + } + const rawReq = req as RawBodyRequest + rawReq.rawBody = buf.toString("utf8") + }, + }), + ) + + // Health check + app.get("/health", (req, res) => { + res.status(200).json({ status: "ok", service: "fygaro-webhook" }) + }) + + app.use(fygaroEnabledGuard) + + app.post("/payment", webhookRateLimit, verifyFygaroSignature, paymentHandler) + + if (Object.keys(FygaroConfig.webhook?.secrets ?? {}).length === 0) { + baseLogger.warn( + "No Fygaro webhook secrets configured (fygaro.webhook.secrets) — /payment will reject all requests with 401", + ) + } + + // Start server + const port = FygaroConfig.webhook?.port ?? 4010 + app.listen(port, () => { + baseLogger.info({ port }, "Fygaro webhook server started") + }) +} diff --git a/src/services/fygaro/webhook-server/middleware/enabled-guard.ts b/src/services/fygaro/webhook-server/middleware/enabled-guard.ts new file mode 100644 index 000000000..df31b22f1 --- /dev/null +++ b/src/services/fygaro/webhook-server/middleware/enabled-guard.ts @@ -0,0 +1,27 @@ +import express from "express" + +import { FygaroConfig } from "@config" +import { baseLogger } from "@services/logger" + +/** + * Defense in depth (mirrors the bridge webhook's ENG-466 guard): the chart + * gates the fygaro-webhook workload on its own enabled flag, but if the + * process ever starts with the feature OFF (chart/config drift, a local run, + * a misconfig) it must not mutate the DB. /health stays up for k8s probes; + * every other route rejects. + */ +export const fygaroEnabledGuard = ( + req: express.Request, + res: express.Response, + next: express.NextFunction, +) => { + if (req.path === "/health") return next() + if (!FygaroConfig.enabled) { + baseLogger.warn( + { path: req.path }, + "Fygaro webhook received while fygaro is disabled — rejecting", + ) + return res.status(503).json({ error: "Fygaro is disabled" }) + } + return next() +} diff --git a/src/services/fygaro/webhook-server/middleware/verify-signature.ts b/src/services/fygaro/webhook-server/middleware/verify-signature.ts new file mode 100644 index 000000000..613814e91 --- /dev/null +++ b/src/services/fygaro/webhook-server/middleware/verify-signature.ts @@ -0,0 +1,113 @@ +import crypto from "crypto" + +import express from "express" + +import { FygaroConfig } from "@config" +import { baseLogger } from "@services/logger" + +type RawBodyRequest = express.Request & { rawBody?: string } + +/** + * Fygaro webhook signature verification. + * + * Fygaro signs every hook request with HMAC-SHA-256 over + * `${timestamp}.${rawBody}` and sends: + * - `Fygaro-Signature`: `t=,v1=[,v1=...]` + * (multiple v1 entries appear during secret rotation) + * - `Fygaro-Key-ID`: identifies which shared secret signed the request + * + * Secrets live in FygaroConfig.webhook.secrets keyed by that key id. When the + * key id header is absent (or unknown ids should not hard-fail during + * rotation), every configured secret is tried — the same approach Fygaro's + * official `@fygaro/webhook` helper takes. + */ +const parseSignatureHeader = ( + header: string, +): { timestamp?: string; hashes: string[] } => { + let timestamp: string | undefined + const hashes: string[] = [] + for (const part of header.split(",")) { + const [key, ...rest] = part.trim().split("=") + const value = rest.join("=") + if (key === "t" && value) timestamp = value + if (key === "v1" && value) hashes.push(value) + } + return { timestamp, hashes } +} + +const timingSafeHexEqual = (expectedHex: string, providedHex: string): boolean => { + const expected = Buffer.from(expectedHex, "hex") + const provided = Buffer.from(providedHex, "hex") + // Buffer.from(_, "hex") stops at the first invalid character, so malformed + // input degrades to a length mismatch rather than a throw. + if (expected.length === 0 || expected.length !== provided.length) return false + return crypto.timingSafeEqual(expected, provided) +} + +export const verifyFygaroSignature = ( + req: express.Request, + res: express.Response, + next: express.NextFunction, +) => { + try { + const signatureHeader = req.headers["fygaro-signature"] + if (!signatureHeader || typeof signatureHeader !== "string") { + return res.status(401).json({ error: "Missing signature header" }) + } + + const keyId = req.headers["fygaro-key-id"] + const secretsById = FygaroConfig.webhook?.secrets ?? {} + const candidateSecrets = + typeof keyId === "string" && keyId && secretsById[keyId] + ? [secretsById[keyId]] + : Object.values(secretsById) + if (candidateSecrets.length === 0) { + baseLogger.error( + { keyId }, + "Fygaro webhook rejected: no webhook secrets configured", + ) + return res.status(401).json({ error: "Webhook secret not configured" }) + } + + const { timestamp, hashes } = parseSignatureHeader(signatureHeader) + if (!timestamp || hashes.length === 0) { + return res.status(401).json({ error: "Invalid signature header" }) + } + + const timestampNum = Number(timestamp) + if (!Number.isFinite(timestampNum)) { + return res.status(401).json({ error: "Invalid signature timestamp" }) + } + // Fygaro documents epoch seconds; tolerate milliseconds too (11+ digits + // is past the year 5138 as seconds, so length disambiguates safely). + const timestampMs = timestamp.length > 11 ? timestampNum : timestampNum * 1000 + const skewMs = FygaroConfig.webhook?.timestampSkewMs ?? 300000 + if (Math.abs(Date.now() - timestampMs) > skewMs) { + baseLogger.warn({ timestamp }, "Fygaro webhook rejected: timestamp outside skew") + return res.status(401).json({ error: "Signature timestamp outside tolerance" }) + } + + const rawBody = (req as RawBodyRequest).rawBody + if (!rawBody) { + return res.status(400).json({ error: "Missing request body" }) + } + + const signedPayload = `${timestamp}.${rawBody}` + const valid = candidateSecrets.some((secret) => { + const expected = crypto + .createHmac("sha256", secret) + .update(signedPayload) + .digest("hex") + return hashes.some((hash) => timingSafeHexEqual(expected, hash)) + }) + if (!valid) { + baseLogger.warn({ keyId }, "Fygaro webhook rejected: signature mismatch") + return res.status(401).json({ error: "Invalid signature" }) + } + + return next() + } catch (error) { + baseLogger.error({ error }, "Fygaro webhook signature verification error") + return res.status(500).json({ error: "Signature verification failed" }) + } +} diff --git a/src/services/fygaro/webhook-server/routes/payment.ts b/src/services/fygaro/webhook-server/routes/payment.ts new file mode 100644 index 000000000..224eef9e1 --- /dev/null +++ b/src/services/fygaro/webhook-server/routes/payment.ts @@ -0,0 +1,282 @@ +/** + * Fygaro Payment Webhook Handler + * Handles payment notifications from Fygaro's payment-button hook. + * + * A payment here is fiat that has already been captured on Fygaro's side + * (card or PayPal). The handler: + * 1. attributes the payment to a Flash account via customReference + * (the app sends the Flash username there — see flash-mobile + * CardPayment.tsx), + * 2. writes the ERPNext audit row (Bridge Transfer Request, + * provider=Fygaro) so every card top-up is recorded, + * 3. posts to the ops activity feed, and + * 4. when fygaro.credit.enabled: credits the user's cash wallet from the + * bank-owner treasury (idempotent on the Fygaro transaction id). + * + * Unattributed payments (blank/unknown customReference — every pre-fix app + * build sends a blank one) are still recorded and alerted so ops can resolve + * them manually; they are never silently dropped. + */ + +import { Request, Response } from "express" + +import { FygaroConfig } from "@config" +import { LockService } from "@services/lock" +import { baseLogger } from "@services/logger" +import { AccountsRepository } from "@services/mongoose" +import { + writeFygaroTopupRequest, + completeFygaroTopup, +} from "@services/frappe/BridgeTransferRequestWriter" +import { alertBridge, generateDedupKey } from "@services/alerts" +import { notifyOpsEvent } from "@services/alerts/ops-events" + +import { creditFygaroTopup, FygaroCreditError } from "../credit-topup" + +type FygaroPaymentPayload = { + transactionId?: string + reference?: string + customReference?: string | null + amount?: string + currency?: string + authCode?: string | null + createdAt?: string + client?: { name?: string; email?: string } +} + +export const paymentHandler = async (req: Request, res: Response) => { + const payload = (req.body ?? {}) as FygaroPaymentPayload + const { transactionId, createdAt } = payload + const currency = (payload.currency ?? "USD").toUpperCase() + const username = payload.customReference?.trim() || undefined + + if (!transactionId || !payload.amount) { + baseLogger.warn( + { transactionId, has_amount: Boolean(payload.amount) }, + "Fygaro payment webhook rejected: missing required fields", + ) + return res.status(400).json({ + error: "Invalid payload", + detail: "Missing one or more required fields: transactionId, amount", + }) + } + + try { + baseLogger.info( + { + transactionId, + amount: payload.amount, + currency, + username, + reference: payload.reference, + }, + "Fygaro payment event", + ) + + // Attribution: customReference carries the Flash username. A blank or + // unknown reference still gets recorded — that IS the failure mode this + // webhook exists to surface. + let accountId: AccountId | undefined + if (username) { + const account = await AccountsRepository().findByUsername(username as Username) + if (account instanceof Error) { + baseLogger.warn( + { transactionId, username }, + "Fygaro payment: customReference does not match any account", + ) + } else { + accountId = account.id + } + } + + const auditResult = await writeFygaroTopupRequest({ + transactionId, + amount: String(payload.amount), + currency, + accountId, + createdAt, + rawPayload: req.body, + }) + if (auditResult instanceof Error) { + baseLogger.error( + { error: auditResult, transactionId }, + "Failed to persist Fygaro ERPNext audit row", + ) + alertBridge({ + dedupKey: generateDedupKey.erpnextFygaroAudit(transactionId), + source: "erpnext-audit", + severity: "critical", + title: "Fygaro payment ERPNext audit write failed", + detail: auditResult.message, + context: { transaction_id: transactionId }, + }) + notifyOpsEvent({ + flow: "deposit", + phase: "failed", + status: "failed", + step: "erpnext-audit", + error: auditResult.constructor.name, + amount: { value: String(payload.amount), currency }, + meta: { provider: "Fygaro", transactionId }, + }) + // 500 so Fygaro retries and the audit gap can self-heal. + return res.status(500).json({ error: "Failed to persist audit row" }) + } + + if (!accountId) { + alertBridge({ + dedupKey: generateDedupKey.fygaroUnattributed(transactionId), + source: "fygaro-webhook", + severity: "warning", + title: "Fygaro payment could not be attributed to an account", + detail: `customReference=${username ?? ""} — manual attribution needed`, + context: { + transaction_id: transactionId, + amount: String(payload.amount), + client_email: payload.client?.email, + }, + }) + notifyOpsEvent({ + flow: "deposit", + phase: "fygaro-unattributed", + status: "pending", + amount: { value: String(payload.amount), currency }, + meta: { + provider: "Fygaro", + transactionId, + reference: username ?? "blank", + email: payload.client?.email ?? "unknown", + }, + }) + return res.status(200).json({ status: "recorded", attributed: false }) + } + + // Mark processed only after the audit write succeeds, so provider retries + // can recover audit gaps after transient persistence failures. Everything + // past this point runs at most once per transaction. + const lockResult = await LockService().lockIdempotencyKey( + `fygaro-payment:${transactionId}` as IdempotencyKey, + ) + if (lockResult instanceof Error) { + baseLogger.info({ transactionId }, "Duplicate Fygaro payment webhook") + return res.status(200).json({ status: "already_processed" }) + } + + if (!FygaroConfig.credit?.enabled || currency !== "USD") { + if (currency !== "USD") { + // The payment button is USD-only; a non-USD payment is unexpected + // enough to demand human eyes before any crediting. + alertBridge({ + dedupKey: generateDedupKey.fygaroCreditFailed(transactionId), + source: "fygaro-webhook", + severity: "warning", + title: "Fygaro payment in unexpected currency — not auto-credited", + detail: `currency=${currency}`, + context: { transaction_id: transactionId, amount: String(payload.amount) }, + }) + } + notifyOpsEvent({ + flow: "deposit", + phase: "fygaro-recorded", + status: "pending", + accountId, + amount: { value: String(payload.amount), currency }, + meta: { provider: "Fygaro", transactionId, username: username ?? "" }, + }) + return res.status(200).json({ status: "recorded", credited: false }) + } + + const amountCents = Math.round(Number(payload.amount) * 100) + const creditResult = await creditFygaroTopup({ + recipientAccountId: accountId, + amountCents, + transactionId, + }) + if (creditResult instanceof FygaroCreditError) { + baseLogger.error( + { error: creditResult, transactionId, accountId }, + "Fygaro payment recorded but auto-credit failed", + ) + alertBridge({ + dedupKey: generateDedupKey.fygaroCreditFailed(transactionId), + source: "fygaro-webhook", + severity: "critical", + title: "Fygaro auto-credit failed — manual credit needed", + detail: `${creditResult.step}: ${creditResult.message}`, + context: { + transaction_id: transactionId, + account_id: accountId, + amount: String(payload.amount), + }, + }) + notifyOpsEvent({ + flow: "deposit", + phase: "failed", + status: "failed", + step: `credit:${creditResult.step}`, + error: creditResult.constructor.name, + accountId, + amount: { value: String(payload.amount), currency }, + meta: { provider: "Fygaro", transactionId, username: username ?? "" }, + }) + // The payment IS recorded; a 500 would only re-run the (now locked) + // handler. Ops resolves the credit manually from the alert. + return res.status(200).json({ status: "recorded", credited: false }) + } + + const completeResult = await completeFygaroTopup({ + transactionId, + accountId, + walletId: creditResult.walletId, + amount: String(payload.amount), + currency, + rawPayload: req.body, + }) + if (completeResult instanceof Error) { + // The money moved; only the audit promotion failed. Alert, don't fail. + alertBridge({ + dedupKey: generateDedupKey.erpnextFygaroAudit(transactionId), + source: "erpnext-audit", + severity: "warning", + title: "Fygaro credit succeeded but ERPNext promotion failed", + detail: completeResult.message, + context: { transaction_id: transactionId }, + }) + } + + notifyOpsEvent({ + flow: "deposit", + phase: "succeeded", + status: "success", + accountId, + amount: { value: String(payload.amount), currency }, + meta: { + provider: "Fygaro", + transactionId, + username: username ?? "", + creditStatus: creditResult.status, + }, + }) + + return res.status(200).json({ status: "success", credited: true }) + } catch (error) { + baseLogger.error({ error, transactionId }, "Error processing Fygaro payment webhook") + alertBridge({ + dedupKey: generateDedupKey.fygaroWebhookPayment(transactionId), + source: "fygaro-webhook", + severity: "critical", + title: "Fygaro payment webhook processing error", + detail: error instanceof Error ? error.message : String(error), + context: { transaction_id: transactionId }, + }) + notifyOpsEvent({ + flow: "deposit", + phase: "failed", + status: "failed", + step: "exception", + error: error instanceof Error ? error.constructor.name : String(error), + meta: { provider: "Fygaro", transactionId }, + }) + return res.status(500).json({ error: "Internal server error" }) + } +} diff --git a/test/flash/unit/services/fygaro/webhook-server/enabled-guard.spec.ts b/test/flash/unit/services/fygaro/webhook-server/enabled-guard.spec.ts new file mode 100644 index 000000000..6318ec5f6 --- /dev/null +++ b/test/flash/unit/services/fygaro/webhook-server/enabled-guard.spec.ts @@ -0,0 +1,61 @@ +import { Request, Response } from "express" + +const mockFygaroConfig = { enabled: false } + +jest.mock("@config", () => ({ + get FygaroConfig() { + return mockFygaroConfig + }, +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +import { fygaroEnabledGuard } from "@services/fygaro/webhook-server/middleware/enabled-guard" + +const makeRes = (): Response => { + const res = { status: jest.fn(), json: jest.fn() } as unknown as Response + ;(res.status as jest.Mock).mockReturnValue(res) + ;(res.json as jest.Mock).mockReturnValue(res) + return res +} + +const makeReq = (path: string): Request => ({ path }) as unknown as Request + +beforeEach(() => { + jest.clearAllMocks() + mockFygaroConfig.enabled = false +}) + +describe("fygaroEnabledGuard", () => { + it("rejects non-health routes with 503 while disabled", () => { + const res = makeRes() + const next = jest.fn() + + fygaroEnabledGuard(makeReq("/payment"), res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(503) + }) + + it("lets /health through while disabled (k8s probes)", () => { + const res = makeRes() + const next = jest.fn() + + fygaroEnabledGuard(makeReq("/health"), res, next) + + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it("lets all routes through when enabled", () => { + mockFygaroConfig.enabled = true + const res = makeRes() + const next = jest.fn() + + fygaroEnabledGuard(makeReq("/payment"), res, next) + + expect(next).toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts new file mode 100644 index 000000000..07d8ac8f6 --- /dev/null +++ b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts @@ -0,0 +1,262 @@ +import { Request, Response } from "express" + +const mockFygaroConfig = { + enabled: true, + webhook: { port: 4010, secrets: {}, timestampSkewMs: 300000 }, + credit: { enabled: false }, +} + +jest.mock("@config", () => ({ + get FygaroConfig() { + return mockFygaroConfig + }, +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +jest.mock("@services/lock", () => ({ + LockService: jest.fn(() => ({ + lockIdempotencyKey: (...args: unknown[]) => mockLockIdempotencyKey(...args), + })), +})) + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ + findByUsername: (...args: unknown[]) => mockFindByUsername(...args), + }), +})) + +jest.mock("@services/frappe/BridgeTransferRequestWriter", () => ({ + writeFygaroTopupRequest: (...args: unknown[]) => mockWriteFygaroTopup(...args), + completeFygaroTopup: (...args: unknown[]) => mockCompleteFygaroTopup(...args), +})) + +jest.mock("@services/alerts", () => ({ + alertBridge: (...args: unknown[]) => mockAlertBridge(...args), + generateDedupKey: new Proxy({}, { get: () => jest.fn(() => "dedup") }), +})) + +jest.mock("@services/alerts/ops-events", () => ({ + notifyOpsEvent: (...args: unknown[]) => mockNotifyOpsEvent(...args), +})) + +jest.mock("@services/fygaro/webhook-server/credit-topup", () => { + class FygaroCreditError extends Error { + step: string + constructor(step: string, message: string) { + super(message) + this.name = "FygaroCreditError" + this.step = step + } + } + return { + FygaroCreditError, + creditFygaroTopup: (...args: unknown[]) => mockCreditFygaroTopup(...args), + } +}) + +const mockLockIdempotencyKey = jest.fn() +const mockFindByUsername = jest.fn() +const mockWriteFygaroTopup = jest.fn() +const mockCompleteFygaroTopup = jest.fn() +const mockAlertBridge = jest.fn() +const mockNotifyOpsEvent = jest.fn() +const mockCreditFygaroTopup = jest.fn() + +import { paymentHandler } from "@services/fygaro/webhook-server/routes/payment" +import { FygaroCreditError } from "@services/fygaro/webhook-server/credit-topup" + +const ACCOUNT_ID = "account-1" as AccountId +const WALLET_ID = "wallet-1" as WalletId + +const VALID_BODY = { + transactionId: "0e2f2c1a-6f6e-4f2b-9b1e-3f1a2b3c4d5e", + reference: "FG-1042", + customReference: "civilizedbarbarian", + amount: "10.00", + currency: "USD", + authCode: null, + createdAt: "2026-08-07T15:00:00Z", + client: { name: "Regina Bailey", email: "regina@example.com" }, +} + +const makeRes = (): Response => { + const res = { status: jest.fn(), json: jest.fn() } as unknown as Response + ;(res.status as jest.Mock).mockReturnValue(res) + ;(res.json as jest.Mock).mockReturnValue(res) + return res +} + +const makeReq = (body: Record): Request => + ({ body }) as unknown as Request + +beforeEach(() => { + jest.clearAllMocks() + mockFygaroConfig.credit = { enabled: false } + mockLockIdempotencyKey.mockResolvedValue(true) + mockFindByUsername.mockResolvedValue({ id: ACCOUNT_ID }) + mockWriteFygaroTopup.mockResolvedValue(true) + mockCompleteFygaroTopup.mockResolvedValue(true) + mockCreditFygaroTopup.mockResolvedValue({ walletId: WALLET_ID, status: "success" }) +}) + +describe("fygaro paymentHandler", () => { + it("rejects a payload without transactionId or amount with 400", async () => { + const res = makeRes() + + await paymentHandler(makeReq({ amount: "10.00" }), res) + + expect(res.status).toHaveBeenCalledWith(400) + expect(mockWriteFygaroTopup).not.toHaveBeenCalled() + }) + + it("records an attributed payment and reports pending when credit is disabled", async () => { + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockWriteFygaroTopup).toHaveBeenCalledWith( + expect.objectContaining({ + transactionId: VALID_BODY.transactionId, + amount: "10.00", + currency: "USD", + accountId: ACCOUNT_ID, + }), + ) + expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(mockNotifyOpsEvent).toHaveBeenCalledWith( + expect.objectContaining({ phase: "fygaro-recorded", status: "pending" }), + ) + expect(res.status).toHaveBeenCalledWith(200) + expect(res.json).toHaveBeenCalledWith({ status: "recorded", credited: false }) + }) + + it("still records a payment with a blank customReference and alerts as unattributed", async () => { + const res = makeRes() + + await paymentHandler(makeReq({ ...VALID_BODY, customReference: "" }), res) + + expect(mockFindByUsername).not.toHaveBeenCalled() + expect(mockWriteFygaroTopup).toHaveBeenCalledWith( + expect.objectContaining({ accountId: undefined }), + ) + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "warning" }), + ) + expect(mockNotifyOpsEvent).toHaveBeenCalledWith( + expect.objectContaining({ phase: "fygaro-unattributed", status: "pending" }), + ) + expect(res.json).toHaveBeenCalledWith({ status: "recorded", attributed: false }) + }) + + it("treats an unknown username as unattributed", async () => { + mockFindByUsername.mockResolvedValue(new Error("CouldNotFindError")) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockWriteFygaroTopup).toHaveBeenCalledWith( + expect.objectContaining({ accountId: undefined }), + ) + expect(res.json).toHaveBeenCalledWith({ status: "recorded", attributed: false }) + }) + + it("returns 500 when the ERPNext audit write fails so Fygaro retries", async () => { + mockWriteFygaroTopup.mockResolvedValue(new Error("erpnext down")) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "critical" }), + ) + expect(res.status).toHaveBeenCalledWith(500) + expect(mockLockIdempotencyKey).not.toHaveBeenCalled() + }) + + it("acknowledges a duplicate delivery without reprocessing", async () => { + mockLockIdempotencyKey.mockResolvedValue(new Error("already locked")) + mockFygaroConfig.credit = { enabled: true } + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(200) + expect(res.json).toHaveBeenCalledWith({ status: "already_processed" }) + }) + + describe("with credit enabled", () => { + beforeEach(() => { + mockFygaroConfig.credit = { enabled: true } + }) + + it("credits the account in cents and promotes the audit row to Completed", async () => { + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCreditFygaroTopup).toHaveBeenCalledWith({ + recipientAccountId: ACCOUNT_ID, + amountCents: 1000, + transactionId: VALID_BODY.transactionId, + }) + expect(mockCompleteFygaroTopup).toHaveBeenCalledWith( + expect.objectContaining({ + transactionId: VALID_BODY.transactionId, + accountId: ACCOUNT_ID, + walletId: WALLET_ID, + }), + ) + expect(mockNotifyOpsEvent).toHaveBeenCalledWith( + expect.objectContaining({ phase: "succeeded", status: "success" }), + ) + expect(res.json).toHaveBeenCalledWith({ status: "success", credited: true }) + }) + + it("records without crediting and alerts critical when the credit fails", async () => { + mockCreditFygaroTopup.mockResolvedValue( + new FygaroCreditError("intraledger-send", "insufficient balance"), + ) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "critical" }), + ) + expect(mockNotifyOpsEvent).toHaveBeenCalledWith( + expect.objectContaining({ status: "failed", step: "credit:intraledger-send" }), + ) + expect(res.status).toHaveBeenCalledWith(200) + expect(res.json).toHaveBeenCalledWith({ status: "recorded", credited: false }) + }) + + it("never auto-credits a non-USD payment", async () => { + const res = makeRes() + + await paymentHandler(makeReq({ ...VALID_BODY, currency: "JMD" }), res) + + expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "warning" }), + ) + expect(res.json).toHaveBeenCalledWith({ status: "recorded", credited: false }) + }) + + it("still reports success when only the ERPNext promotion fails after a credit", async () => { + mockCompleteFygaroTopup.mockResolvedValue(new Error("erpnext down")) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "warning" }), + ) + expect(res.json).toHaveBeenCalledWith({ status: "success", credited: true }) + }) + }) +}) diff --git a/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts b/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts new file mode 100644 index 000000000..305cb9ed1 --- /dev/null +++ b/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts @@ -0,0 +1,215 @@ +import crypto from "crypto" + +import { Request, Response } from "express" + +const mockFygaroConfig = { + enabled: true, + webhook: { + port: 4010, + secrets: { key1: "secret-one", key2: "secret-two" } as Record, + timestampSkewMs: 300000, + }, + credit: { enabled: false }, +} + +jest.mock("@config", () => ({ + get FygaroConfig() { + return mockFygaroConfig + }, +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +import { verifyFygaroSignature } from "@services/fygaro/webhook-server/middleware/verify-signature" + +const RAW_BODY = JSON.stringify({ transactionId: "tx-1", amount: "10.00" }) + +const sign = (timestamp: string, rawBody: string, secret: string): string => + crypto.createHmac("sha256", secret).update(`${timestamp}.${rawBody}`).digest("hex") + +const nowSeconds = () => String(Math.floor(Date.now() / 1000)) + +const makeRes = (): Response => { + const res = { status: jest.fn(), json: jest.fn() } as unknown as Response + ;(res.status as jest.Mock).mockReturnValue(res) + ;(res.json as jest.Mock).mockReturnValue(res) + return res +} + +const makeReq = ({ + signature, + keyId, + rawBody = RAW_BODY, +}: { + signature?: string + keyId?: string + rawBody?: string +}): Request => + ({ + headers: { + ...(signature !== undefined ? { "fygaro-signature": signature } : {}), + ...(keyId !== undefined ? { "fygaro-key-id": keyId } : {}), + }, + rawBody, + }) as unknown as Request + +beforeEach(() => { + jest.clearAllMocks() + mockFygaroConfig.webhook.secrets = { key1: "secret-one", key2: "secret-two" } +}) + +describe("verifyFygaroSignature", () => { + it("accepts a valid signature for the key id's secret", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it("accepts a valid signature without a key id by trying all secrets", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-two")}`, + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).toHaveBeenCalled() + }) + + it("accepts when one of multiple v1 hashes matches (secret rotation)", () => { + const t = nowSeconds() + const stale = sign(t, RAW_BODY, "retired-secret") + const good = sign(t, RAW_BODY, "secret-one") + const req = makeReq({ signature: `t=${t},v1=${stale},v1=${good}`, keyId: "key1" }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).toHaveBeenCalled() + }) + + it("accepts a millisecond timestamp within skew", () => { + const t = String(Date.now()) + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).toHaveBeenCalled() + }) + + it("rejects a wrong signature with 401", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "wrong-secret")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects a signature computed over a different body", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, `{"tampered":true}`, "secret-one")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects a missing signature header with 401", () => { + const req = makeReq({}) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects a malformed signature header with 401", () => { + const req = makeReq({ signature: "not-a-signature" }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects a timestamp outside the allowed skew with 401", () => { + const t = String(Math.floor(Date.now() / 1000) - 3600) + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects with 401 when no secrets are configured", () => { + mockFygaroConfig.webhook.secrets = {} + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it("rejects with 400 when the raw body was not captured", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + rawBody: "", + }) + const res = makeRes() + const next = jest.fn() + + verifyFygaroSignature(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(400) + }) +}) From d7021262599eec5dccbd7e9f84960b41de5e5ba6 Mon Sep 17 00:00:00 2001 From: Dread Date: Fri, 7 Aug 2026 23:55:17 -0700 Subject: [PATCH 2/3] =?UTF-8?q?fix(fygaro):=20review=20fixes=20=E2=80=94?= =?UTF-8?q?=20releasing=20credit=20lock,=20credit-topup=20tests,=20hardeni?= =?UTF-8?q?ng?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the strict-review findings on the payment webhook: - credit path now serializes deliveries with a RELEASING lock (lockPaymentIdempotencyKey) and uses the audit row's Completed status as the processed marker, instead of consuming a non-releasing timelock before the send. A crash mid-credit releases the lock so the next provider retry re-runs the credit — withPaymentIdempotency keeps the send exactly-once — eliminating the stranded paid-but-uncredited window. Promotion failures now also self-heal on retry. - new isFygaroTopupCompleted helper (degrades to false on lookup failure; a false negative replays the cached send, never double-pays) - unattributed path takes the dedupe timelock before emitting, so Fygaro re-deliveries no longer spam a fresh ops-feed line per attempt - enabled guard optional-chains FygaroConfig (fail closed 503, never a per-request throw); signature verify documents the hex-digest assumption to check against the first real signed payment - NEW: credit-topup.spec.ts — direct tests for the money-moving path: treasury role/wallet resolution, USDT-preference + USD fallback, recipient currency matching, cents validation matrix, Pending semantics, send errors, unexpected-status rejection - payment.spec.ts updated for the new lock semantics: Completed-row short-circuit, incomplete-attempt re-run, lock contention, and unattributed re-delivery dedupe Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- .../frappe/BridgeTransferRequestWriter.ts | 18 ++ .../middleware/enabled-guard.ts | 5 +- .../middleware/verify-signature.ts | 4 + .../fygaro/webhook-server/routes/payment.ts | 199 +++++++++++------- .../webhook-server/credit-topup.spec.ts | 163 ++++++++++++++ .../fygaro/webhook-server/payment.spec.ts | 62 +++++- 6 files changed, 368 insertions(+), 83 deletions(-) create mode 100644 test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts diff --git a/src/services/frappe/BridgeTransferRequestWriter.ts b/src/services/frappe/BridgeTransferRequestWriter.ts index 9caa99732..b4b0a1931 100644 --- a/src/services/frappe/BridgeTransferRequestWriter.ts +++ b/src/services/frappe/BridgeTransferRequestWriter.ts @@ -243,6 +243,24 @@ export const writeFygaroTopupRequest = async ({ ) } +// Whether this Fygaro payment was already fully processed (its audit row +// promoted to Completed by a prior delivery). Used as the processed-marker for +// webhook re-deliveries. A lookup failure degrades to false — the credit +// itself is exactly-once under withPaymentIdempotency, so a false negative +// can never double-pay; it only costs a redundant cached-send replay. +export const isFygaroTopupCompleted = async (transactionId: string): Promise => { + if (!ErpNext?.findBridgeTransferRequest) return false + const doc = await ErpNext.findBridgeTransferRequest(`fygaro:${transactionId}`) + if (doc instanceof Error) { + baseLogger.warn( + { transactionId, error: doc }, + "Failed to check Fygaro topup completion; treating as not completed", + ) + return false + } + return doc?.status === BridgeTransferRequestStatus.Completed +} + // Called after the treasury -> user intraledger credit succeeds: promotes the // Fygaro topup row to Completed and stamps the credited wallet on it. The // upsert's monotonic status guard makes this safe to repeat. diff --git a/src/services/fygaro/webhook-server/middleware/enabled-guard.ts b/src/services/fygaro/webhook-server/middleware/enabled-guard.ts index df31b22f1..c8ab29072 100644 --- a/src/services/fygaro/webhook-server/middleware/enabled-guard.ts +++ b/src/services/fygaro/webhook-server/middleware/enabled-guard.ts @@ -16,7 +16,10 @@ export const fygaroEnabledGuard = ( next: express.NextFunction, ) => { if (req.path === "/health") return next() - if (!FygaroConfig.enabled) { + // Optional-chained: if the fygaro config block is ever absent (schema + // default not applied on some load path), fail closed with the 503 rather + // than throwing a per-request 500. + if (!FygaroConfig?.enabled) { baseLogger.warn( { path: req.path }, "Fygaro webhook received while fygaro is disabled — rejecting", diff --git a/src/services/fygaro/webhook-server/middleware/verify-signature.ts b/src/services/fygaro/webhook-server/middleware/verify-signature.ts index 613814e91..7236a6089 100644 --- a/src/services/fygaro/webhook-server/middleware/verify-signature.ts +++ b/src/services/fygaro/webhook-server/middleware/verify-signature.ts @@ -92,6 +92,10 @@ export const verifyFygaroSignature = ( return res.status(400).json({ error: "Missing request body" }) } + // ASSUMPTION (verify against a real signed payment before trusting in + // prod): Fygaro's docs and official helper libraries compare hex-encoded + // HMAC digests. If a correctly-configured secret still 401s here, check + // whether the digest encoding is base64 before suspecting the secret. const signedPayload = `${timestamp}.${rawBody}` const valid = candidateSecrets.some((secret) => { const expected = crypto diff --git a/src/services/fygaro/webhook-server/routes/payment.ts b/src/services/fygaro/webhook-server/routes/payment.ts index 224eef9e1..00951050c 100644 --- a/src/services/fygaro/webhook-server/routes/payment.ts +++ b/src/services/fygaro/webhook-server/routes/payment.ts @@ -27,6 +27,7 @@ import { AccountsRepository } from "@services/mongoose" import { writeFygaroTopupRequest, completeFygaroTopup, + isFygaroTopupCompleted, } from "@services/frappe/BridgeTransferRequestWriter" import { alertBridge, generateDedupKey } from "@services/alerts" import { notifyOpsEvent } from "@services/alerts/ops-events" @@ -124,6 +125,17 @@ export const paymentHandler = async (req: Request, res: Response) => { } if (!accountId) { + // Dedupe re-deliveries before emitting: alertBridge is TTL-deduped but + // the ops feed is not, so a Fygaro retry of an unattributed payment + // would otherwise spam a feed line per delivery. Taken only after the + // audit write succeeds so retries can still repair transient failures. + const dedupe = await LockService().lockIdempotencyKey( + `fygaro-payment:${transactionId}` as IdempotencyKey, + ) + if (dedupe instanceof Error) { + baseLogger.info({ transactionId }, "Duplicate Fygaro payment webhook") + return res.status(200).json({ status: "already_processed" }) + } alertBridge({ dedupKey: generateDedupKey.fygaroUnattributed(transactionId), source: "fygaro-webhook", @@ -151,18 +163,18 @@ export const paymentHandler = async (req: Request, res: Response) => { return res.status(200).json({ status: "recorded", attributed: false }) } - // Mark processed only after the audit write succeeds, so provider retries - // can recover audit gaps after transient persistence failures. Everything - // past this point runs at most once per transaction. - const lockResult = await LockService().lockIdempotencyKey( - `fygaro-payment:${transactionId}` as IdempotencyKey, - ) - if (lockResult instanceof Error) { - baseLogger.info({ transactionId }, "Duplicate Fygaro payment webhook") - return res.status(200).json({ status: "already_processed" }) - } - if (!FygaroConfig.credit?.enabled || currency !== "USD") { + // Record-only path: nothing money-moving happens here, so a + // non-releasing timelock is the right dedupe. Taken only after the + // audit write succeeds so provider retries can recover audit gaps + // after transient persistence failures. + const lockResult = await LockService().lockIdempotencyKey( + `fygaro-payment:${transactionId}` as IdempotencyKey, + ) + if (lockResult instanceof Error) { + baseLogger.info({ transactionId }, "Duplicate Fygaro payment webhook") + return res.status(200).json({ status: "already_processed" }) + } if (currency !== "USD") { // The payment button is USD-only; a non-USD payment is unexpected // enough to demand human eyes before any crediting. @@ -186,79 +198,108 @@ export const paymentHandler = async (req: Request, res: Response) => { return res.status(200).json({ status: "recorded", credited: false }) } + // Credit path: serialize deliveries with a RELEASING lock and use the + // audit row's Completed status as the processed marker. A crash between + // here and the promotion releases the lock, so the next provider retry + // re-runs this block — withPaymentIdempotency (keyed fygaro:) makes + // the send itself exactly-once — instead of stranding a paid-but- + // uncredited payment behind a consumed timelock. Distinct resource from + // the lock withPaymentIdempotency takes internally (that one is scoped to + // the sender wallet), so there is no nested-acquire collision. + const creditAccountId = accountId const amountCents = Math.round(Number(payload.amount) * 100) - const creditResult = await creditFygaroTopup({ - recipientAccountId: accountId, - amountCents, - transactionId, - }) - if (creditResult instanceof FygaroCreditError) { - baseLogger.error( - { error: creditResult, transactionId, accountId }, - "Fygaro payment recorded but auto-credit failed", - ) - alertBridge({ - dedupKey: generateDedupKey.fygaroCreditFailed(transactionId), - source: "fygaro-webhook", - severity: "critical", - title: "Fygaro auto-credit failed — manual credit needed", - detail: `${creditResult.step}: ${creditResult.message}`, - context: { - transaction_id: transactionId, - account_id: accountId, + const outcome = await LockService().lockPaymentIdempotencyKey( + `fygaro-payment:${transactionId}` as IdempotencyKey, + async () => { + if (await isFygaroTopupCompleted(transactionId)) { + baseLogger.info({ transactionId }, "Duplicate Fygaro payment webhook") + return { code: 200, body: { status: "already_processed" } } + } + + const creditResult = await creditFygaroTopup({ + recipientAccountId: creditAccountId, + amountCents, + transactionId, + }) + if (creditResult instanceof FygaroCreditError) { + baseLogger.error( + { error: creditResult, transactionId, accountId: creditAccountId }, + "Fygaro payment recorded but auto-credit failed", + ) + alertBridge({ + dedupKey: generateDedupKey.fygaroCreditFailed(transactionId), + source: "fygaro-webhook", + severity: "critical", + title: "Fygaro auto-credit failed — manual credit needed", + detail: `${creditResult.step}: ${creditResult.message}`, + context: { + transaction_id: transactionId, + account_id: creditAccountId, + amount: String(payload.amount), + }, + }) + notifyOpsEvent({ + flow: "deposit", + phase: "failed", + status: "failed", + step: `credit:${creditResult.step}`, + error: creditResult.constructor.name, + accountId: creditAccountId, + amount: { value: String(payload.amount), currency }, + meta: { provider: "Fygaro", transactionId, username: username ?? "" }, + }) + // The payment IS recorded and the row stays Fiat Received. A failed + // send is not cached, so a provider retry re-attempts the credit + // (self-healing for transient failures); ops has the critical alert + // for the deterministic ones. + return { code: 200, body: { status: "recorded", credited: false } } + } + + const completeResult = await completeFygaroTopup({ + transactionId, + accountId: creditAccountId, + walletId: creditResult.walletId, amount: String(payload.amount), - }, - }) - notifyOpsEvent({ - flow: "deposit", - phase: "failed", - status: "failed", - step: `credit:${creditResult.step}`, - error: creditResult.constructor.name, - accountId, - amount: { value: String(payload.amount), currency }, - meta: { provider: "Fygaro", transactionId, username: username ?? "" }, - }) - // The payment IS recorded; a 500 would only re-run the (now locked) - // handler. Ops resolves the credit manually from the alert. - return res.status(200).json({ status: "recorded", credited: false }) - } + currency, + rawPayload: req.body, + }) + if (completeResult instanceof Error) { + // The money moved; only the audit promotion failed. Alert, don't + // fail: the row stays Fiat Received, so a provider retry replays + // the cached send result and re-attempts this promotion. + alertBridge({ + dedupKey: generateDedupKey.erpnextFygaroAudit(transactionId), + source: "erpnext-audit", + severity: "warning", + title: "Fygaro credit succeeded but ERPNext promotion failed", + detail: completeResult.message, + context: { transaction_id: transactionId }, + }) + } - const completeResult = await completeFygaroTopup({ - transactionId, - accountId, - walletId: creditResult.walletId, - amount: String(payload.amount), - currency, - rawPayload: req.body, - }) - if (completeResult instanceof Error) { - // The money moved; only the audit promotion failed. Alert, don't fail. - alertBridge({ - dedupKey: generateDedupKey.erpnextFygaroAudit(transactionId), - source: "erpnext-audit", - severity: "warning", - title: "Fygaro credit succeeded but ERPNext promotion failed", - detail: completeResult.message, - context: { transaction_id: transactionId }, - }) - } + notifyOpsEvent({ + flow: "deposit", + phase: "succeeded", + status: "success", + accountId: creditAccountId, + amount: { value: String(payload.amount), currency }, + meta: { + provider: "Fygaro", + transactionId, + username: username ?? "", + creditStatus: creditResult.status, + }, + }) - notifyOpsEvent({ - flow: "deposit", - phase: "succeeded", - status: "success", - accountId, - amount: { value: String(payload.amount), currency }, - meta: { - provider: "Fygaro", - transactionId, - username: username ?? "", - creditStatus: creditResult.status, + return { code: 200, body: { status: "success", credited: true } } }, - }) - - return res.status(200).json({ status: "success", credited: true }) + ) + if (outcome instanceof Error) { + // Another delivery of this payment holds the credit lock right now. + baseLogger.info({ transactionId }, "Fygaro payment already being processed") + return res.status(200).json({ status: "already_processing" }) + } + return res.status(outcome.code).json(outcome.body) } catch (error) { baseLogger.error({ error, transactionId }, "Error processing Fygaro payment webhook") alertBridge({ diff --git a/test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts b/test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts new file mode 100644 index 000000000..f730b6548 --- /dev/null +++ b/test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts @@ -0,0 +1,163 @@ +import { PaymentSendStatus } from "@domain/bitcoin/lightning" +import { WalletCurrency } from "@domain/shared" + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ + findByRole: (...args: unknown[]) => mockFindByRole(...args), + }), + WalletsRepository: () => ({ + listByAccountId: (...args: unknown[]) => mockListByAccountId(...args), + }), +})) + +// credit-topup lazy-imports this module; jest's registry intercepts dynamic +// imports the same as static ones. +jest.mock("@app/payments/send-intraledger", () => ({ + intraledgerPaymentSendWalletIdForUsdWallet: (...args: unknown[]) => + mockIntraledgerSend(...args), +})) + +const mockFindByRole = jest.fn() +const mockListByAccountId = jest.fn() +const mockIntraledgerSend = jest.fn() + +import { + creditFygaroTopup, + FygaroCreditError, +} from "@services/fygaro/webhook-server/credit-topup" + +const TREASURY_ACCOUNT_ID = "treasury-account" as AccountId +const RECIPIENT_ACCOUNT_ID = "recipient-account" as AccountId +const TX_ID = "0e2f2c1a-6f6e-4f2b-9b1e-3f1a2b3c4d5e" + +const usdtWallet = (id: string) => ({ id, currency: WalletCurrency.Usdt }) +const usdWallet = (id: string) => ({ id, currency: WalletCurrency.Usd }) +const btcWallet = (id: string) => ({ id, currency: WalletCurrency.Btc }) + +// Wallet fixtures keyed by account: default = USDT treasury, USDT recipient. +const walletsByAccount: Record = {} +const setWallets = (accountId: string, wallets: unknown[]) => { + walletsByAccount[accountId] = wallets +} + +const credit = (amountCents = 1000) => + creditFygaroTopup({ + recipientAccountId: RECIPIENT_ACCOUNT_ID, + amountCents, + transactionId: TX_ID, + }) + +beforeEach(() => { + jest.clearAllMocks() + for (const key of Object.keys(walletsByAccount)) delete walletsByAccount[key] + setWallets(TREASURY_ACCOUNT_ID, [btcWallet("t-btc"), usdtWallet("t-usdt")]) + setWallets(RECIPIENT_ACCOUNT_ID, [btcWallet("r-btc"), usdtWallet("r-usdt")]) + mockFindByRole.mockResolvedValue({ id: TREASURY_ACCOUNT_ID }) + mockListByAccountId.mockImplementation(async (accountId: string) => { + return walletsByAccount[accountId] ?? [] + }) + mockIntraledgerSend.mockResolvedValue(PaymentSendStatus.Success) +}) + +describe("creditFygaroTopup", () => { + it("sends the amount in cents from the treasury USDT wallet with the fygaro idempotency key", async () => { + const result = await credit(1000) + + expect(mockFindByRole).toHaveBeenCalledWith("bankowner") + expect(mockIntraledgerSend).toHaveBeenCalledWith({ + senderWalletId: "t-usdt", + recipientWalletId: "r-usdt", + amount: 1000, + memo: `Card top-up (Fygaro ${TX_ID})`, + idempotencyKey: `fygaro:${TX_ID}`, + }) + expect(result).toEqual({ walletId: "r-usdt", status: "success" }) + }) + + it("falls back to the legacy USD wallets when the treasury has no USDT wallet", async () => { + setWallets(TREASURY_ACCOUNT_ID, [usdWallet("t-usd")]) + setWallets(RECIPIENT_ACCOUNT_ID, [usdWallet("r-usd"), usdtWallet("r-usdt")]) + + const result = await credit() + + expect(mockIntraledgerSend).toHaveBeenCalledWith( + expect.objectContaining({ senderWalletId: "t-usd", recipientWalletId: "r-usd" }), + ) + expect(result).toEqual({ walletId: "r-usd", status: "success" }) + }) + + it("treats a Pending send as credited and never retries", async () => { + mockIntraledgerSend.mockResolvedValue(PaymentSendStatus.Pending) + + const result = await credit() + + expect(mockIntraledgerSend).toHaveBeenCalledTimes(1) + expect(result).toEqual({ walletId: "r-usdt", status: "pending" }) + }) + + it.each([ + ["zero", 0], + ["negative", -100], + ["non-integer", 1000.5], + ["NaN", NaN], + ])("rejects a %s amount without touching any wallet", async (_label, amountCents) => { + const result = await credit(amountCents) + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("validate-amount") + expect(mockFindByRole).not.toHaveBeenCalled() + expect(mockIntraledgerSend).not.toHaveBeenCalled() + }) + + it("fails when no account holds the bankowner role", async () => { + mockFindByRole.mockResolvedValue(new Error("CouldNotFindError")) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("resolve-treasury") + expect(mockIntraledgerSend).not.toHaveBeenCalled() + }) + + it("fails when the treasury has neither a USDT nor a USD wallet", async () => { + setWallets(TREASURY_ACCOUNT_ID, [btcWallet("t-btc")]) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("resolve-treasury-wallet") + expect(mockIntraledgerSend).not.toHaveBeenCalled() + }) + + it("fails when the recipient has no wallet in the funding currency", async () => { + setWallets(RECIPIENT_ACCOUNT_ID, [usdWallet("r-usd")]) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("resolve-recipient-wallet") + expect(mockIntraledgerSend).not.toHaveBeenCalled() + }) + + it("surfaces a send error as an intraledger-send failure", async () => { + mockIntraledgerSend.mockResolvedValue(new Error("InsufficientBalanceError")) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("intraledger-send") + }) + + it("fails on an unexpected payment status instead of assuming success", async () => { + mockIntraledgerSend.mockResolvedValue(PaymentSendStatus.AlreadyPaid) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("intraledger-send") + }) +}) diff --git a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts index 07d8ac8f6..c4cac9d6c 100644 --- a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts @@ -19,6 +19,8 @@ jest.mock("@services/logger", () => ({ jest.mock("@services/lock", () => ({ LockService: jest.fn(() => ({ lockIdempotencyKey: (...args: unknown[]) => mockLockIdempotencyKey(...args), + lockPaymentIdempotencyKey: (key: unknown, fn: unknown) => + mockLockPaymentIdempotencyKey(key, fn), })), })) @@ -31,6 +33,7 @@ jest.mock("@services/mongoose", () => ({ jest.mock("@services/frappe/BridgeTransferRequestWriter", () => ({ writeFygaroTopupRequest: (...args: unknown[]) => mockWriteFygaroTopup(...args), completeFygaroTopup: (...args: unknown[]) => mockCompleteFygaroTopup(...args), + isFygaroTopupCompleted: (...args: unknown[]) => mockIsFygaroTopupCompleted(...args), })) jest.mock("@services/alerts", () => ({ @@ -58,9 +61,11 @@ jest.mock("@services/fygaro/webhook-server/credit-topup", () => { }) const mockLockIdempotencyKey = jest.fn() +const mockLockPaymentIdempotencyKey = jest.fn() const mockFindByUsername = jest.fn() const mockWriteFygaroTopup = jest.fn() const mockCompleteFygaroTopup = jest.fn() +const mockIsFygaroTopupCompleted = jest.fn() const mockAlertBridge = jest.fn() const mockNotifyOpsEvent = jest.fn() const mockCreditFygaroTopup = jest.fn() @@ -96,6 +101,11 @@ beforeEach(() => { jest.clearAllMocks() mockFygaroConfig.credit = { enabled: false } mockLockIdempotencyKey.mockResolvedValue(true) + // Releasing lock: default to acquiring and running the wrapped callback. + mockLockPaymentIdempotencyKey.mockImplementation( + async (_key: unknown, fn: () => Promise) => fn(), + ) + mockIsFygaroTopupCompleted.mockResolvedValue(false) mockFindByUsername.mockResolvedValue({ id: ACCOUNT_ID }) mockWriteFygaroTopup.mockResolvedValue(true) mockCompleteFygaroTopup.mockResolvedValue(true) @@ -176,18 +186,28 @@ describe("fygaro paymentHandler", () => { expect(mockLockIdempotencyKey).not.toHaveBeenCalled() }) - it("acknowledges a duplicate delivery without reprocessing", async () => { + it("acknowledges a duplicate record-only delivery without reprocessing", async () => { mockLockIdempotencyKey.mockResolvedValue(new Error("already locked")) - mockFygaroConfig.credit = { enabled: true } const res = makeRes() await paymentHandler(makeReq(VALID_BODY), res) - expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(mockNotifyOpsEvent).not.toHaveBeenCalled() expect(res.status).toHaveBeenCalledWith(200) expect(res.json).toHaveBeenCalledWith({ status: "already_processed" }) }) + it("dedupes re-deliveries of an unattributed payment before emitting the ops event", async () => { + mockLockIdempotencyKey.mockResolvedValue(new Error("already locked")) + const res = makeRes() + + await paymentHandler(makeReq({ ...VALID_BODY, customReference: "" }), res) + + expect(mockNotifyOpsEvent).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + expect(res.json).toHaveBeenCalledWith({ status: "already_processed" }) + }) + describe("with credit enabled", () => { beforeEach(() => { mockFygaroConfig.credit = { enabled: true } @@ -235,6 +255,42 @@ describe("fygaro paymentHandler", () => { expect(res.json).toHaveBeenCalledWith({ status: "recorded", credited: false }) }) + it("short-circuits when the audit row is already Completed (processed re-delivery)", async () => { + mockIsFygaroTopupCompleted.mockResolvedValue(true) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() + expect(res.json).toHaveBeenCalledWith({ status: "already_processed" }) + }) + + it("re-runs the credit when a retry arrives after an incomplete first attempt", async () => { + // Row still Fiat Received (crash or promotion failure last time): + // the credit path must run again — withPaymentIdempotency makes the + // send replay-safe — so the retry self-heals instead of stranding. + mockIsFygaroTopupCompleted.mockResolvedValue(false) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCreditFygaroTopup).toHaveBeenCalledTimes(1) + expect(mockCompleteFygaroTopup).toHaveBeenCalledTimes(1) + expect(res.json).toHaveBeenCalledWith({ status: "success", credited: true }) + }) + + it("acknowledges without crediting when another delivery holds the credit lock", async () => { + mockLockPaymentIdempotencyKey.mockResolvedValue(new Error("lock contention")) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCreditFygaroTopup).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(200) + expect(res.json).toHaveBeenCalledWith({ status: "already_processing" }) + }) + it("never auto-credits a non-USD payment", async () => { const res = makeRes() From c6530c4db245dd3b3b2e32c710c9bef05c1aa1ee Mon Sep 17 00:00:00 2001 From: Dread Date: Sat, 8 Aug 2026 07:58:27 -0700 Subject: [PATCH 3/3] fix(fygaro): distinguish lock contention from a swallowed credit-block throw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit redlock reports a throw from inside the wrapped callback as a generic lock error; acking that as "already_processing" (200) would stop Fygaro's retries and strand the payment at Fiat Received with no alert. Only ResourceAttemptsLockServiceError (real contention) is acked now — any other lock error is rethrown so the catch-all returns 500 + critical alert and the provider retries. Test added for the throw path; the contention test now uses the real error class. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- .../fygaro/webhook-server/routes/payment.ts | 10 +++++++- .../fygaro/webhook-server/payment.spec.ts | 23 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/services/fygaro/webhook-server/routes/payment.ts b/src/services/fygaro/webhook-server/routes/payment.ts index 00951050c..7fa30a0b7 100644 --- a/src/services/fygaro/webhook-server/routes/payment.ts +++ b/src/services/fygaro/webhook-server/routes/payment.ts @@ -21,6 +21,7 @@ import { Request, Response } from "express" import { FygaroConfig } from "@config" +import { ResourceAttemptsLockServiceError } from "@domain/lock" import { LockService } from "@services/lock" import { baseLogger } from "@services/logger" import { AccountsRepository } from "@services/mongoose" @@ -294,11 +295,18 @@ export const paymentHandler = async (req: Request, res: Response) => { return { code: 200, body: { status: "success", credited: true } } }, ) - if (outcome instanceof Error) { + if (outcome instanceof ResourceAttemptsLockServiceError) { // Another delivery of this payment holds the credit lock right now. baseLogger.info({ transactionId }, "Fygaro payment already being processed") return res.status(200).json({ status: "already_processing" }) } + if (outcome instanceof Error) { + // Any other lock error means redlock swallowed a throw from inside the + // credit block (UnknownLockServiceError). Rethrow so the catch-all + // below returns 500 + critical alert and Fygaro retries, instead of + // acking a stranded payment as "already_processing". + throw outcome + } return res.status(outcome.code).json(outcome.body) } catch (error) { baseLogger.error({ error, transactionId }, "Error processing Fygaro payment webhook") diff --git a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts index c4cac9d6c..0ce1f134b 100644 --- a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts @@ -70,6 +70,8 @@ const mockAlertBridge = jest.fn() const mockNotifyOpsEvent = jest.fn() const mockCreditFygaroTopup = jest.fn() +import { ResourceAttemptsLockServiceError } from "@domain/lock" + import { paymentHandler } from "@services/fygaro/webhook-server/routes/payment" import { FygaroCreditError } from "@services/fygaro/webhook-server/credit-topup" @@ -281,7 +283,9 @@ describe("fygaro paymentHandler", () => { }) it("acknowledges without crediting when another delivery holds the credit lock", async () => { - mockLockPaymentIdempotencyKey.mockResolvedValue(new Error("lock contention")) + mockLockPaymentIdempotencyKey.mockResolvedValue( + new ResourceAttemptsLockServiceError(), + ) const res = makeRes() await paymentHandler(makeReq(VALID_BODY), res) @@ -291,6 +295,23 @@ describe("fygaro paymentHandler", () => { expect(res.json).toHaveBeenCalledWith({ status: "already_processing" }) }) + it("returns 500 with a critical alert when the credit block threw (not contention)", async () => { + // redlock reports a swallowed callback throw as a generic lock error — + // that must NOT be acked as already_processing, or Fygaro stops + // retrying and the payment strands at Fiat Received silently. + mockLockPaymentIdempotencyKey.mockResolvedValue( + new Error("UnknownLockServiceError"), + ) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ severity: "critical" }), + ) + expect(res.status).toHaveBeenCalledWith(500) + }) + it("never auto-credits a non-USD payment", async () => { const res = makeRes()