From 02f86f04d19bf56b77f8dd29c222bc545eabcadc Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 21:54:21 -0700 Subject: [PATCH 1/7] feat(fygaro): signature-failure alerts + bankowner float monitoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close two silent-failure gaps in the Fygaro card-top-up path. 1. Signature-failure alerting. verify-signature previously only logged on an HMAC mismatch or a missing/empty secret set, so a rotated or wrong webhook secret 401s every real payment while the service looks healthy (this exact gap caused hours of silent failures during setup). Now those two cases fire alertBridge (warning, source fygaro-webhook) under a static dedup key so the built-in TTL suppression rate-limits the flood to one page per window. Plain missing/malformed/expired signatures (internet noise) stay silent. The alert carries only the (public) key id, never the secret. 2. Bankowner treasury float monitoring. a. A cron task (checkFygaroTreasuryFloat) reads the bankowner treasury USDT balance from IBEX each run (~15 min) and warns when it drops below a configurable floor (fygaro.float.floorUsd, default $2000 — ~4x the $500 auto-credit limit). Self-guards on FygaroConfig.enabled and never throws, so an IBEX read blip logs but cannot crash the cron. b. credit-topup now maps an insufficient-treasury-balance send failure (InsufficientIbexBalance / InsufficientBalanceError, matched by class) to a distinct "insufficient-treasury-float" step, and payment.ts raises a distinct critical ("float EXHAUSTED — top up bankowner immediately") under its own static dedup key so ops tops up rather than debugging a bug. The row still stays Fiat Received; idempotency and no-double-spend are unchanged. Tests cover: mismatch/no-secrets alert vs. noise stays silent; float below / at / above floor, drained-account zero, IBEX read failure and resolver throw don't crash, disabled feature skips; insufficient-balance -> float step and the distinct exhausted alert while other failures keep the generic one. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- dev/config/base-config.yaml | 5 + src/config/schema.ts | 11 ++ src/config/schema.types.d.ts | 3 + src/servers/cron.ts | 9 ++ src/services/alerts/dedup-key.ts | 5 + src/services/fygaro/float-monitor.ts | 66 ++++++++ .../fygaro/webhook-server/credit-topup.ts | 21 +++ .../middleware/verify-signature.ts | 32 ++++ .../fygaro/webhook-server/routes/payment.ts | 20 ++- .../services/fygaro/float-monitor.spec.ts | 153 ++++++++++++++++++ .../webhook-server/credit-topup.spec.ts | 26 ++- .../fygaro/webhook-server/payment.spec.ts | 30 +++- .../webhook-server/verify-signature.spec.ts | 78 +++++++++ 13 files changed, 452 insertions(+), 7 deletions(-) create mode 100644 src/services/fygaro/float-monitor.ts create mode 100644 test/flash/unit/services/fygaro/float-monitor.spec.ts diff --git a/dev/config/base-config.yaml b/dev/config/base-config.yaml index a37e6c70a..771d4eb32 100644 --- a/dev/config/base-config.yaml +++ b/dev/config/base-config.yaml @@ -27,6 +27,11 @@ fygaro: # Phase gate: with credit OFF the webhook only records the payment and # notifies ops; the treasury -> user transfer stays manual. enabled: false + float: + # Alert (cron) when the bankowner treasury USDT balance — the auto-credit + # funding source — drops below this floor, in USD. Default ~4x the $500 + # auto-credit limit so ops has runway to top up before the well runs dry. + floorUsd: 2000 bridge: enabled: false # flags-off baseline; enable per environment via config overrides diff --git a/src/config/schema.ts b/src/config/schema.ts index 0da095190..0edca3f5c 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -802,6 +802,17 @@ export const configSchema = { // the treasury -> user transfer stays manual. default: { enabled: false }, }, + float: { + type: "object", + properties: { + // Alert when the bankowner treasury USDT balance (the auto-credit + // funding source) drops below this floor, in USD. Default ~4x the + // $500 auto-credit limit so ops has runway to top up. + floorUsd: { type: "number", default: 2000 }, + }, + additionalProperties: false, + default: { floorUsd: 2000 }, + }, }, additionalProperties: false, // Default OFF baseline (mirrors topup); enable per environment via overrides. diff --git a/src/config/schema.types.d.ts b/src/config/schema.types.d.ts index 2fd2de7c2..74a14d8f8 100644 --- a/src/config/schema.types.d.ts +++ b/src/config/schema.types.d.ts @@ -80,6 +80,9 @@ type FygaroConfig = { credit: { enabled: boolean } + float: { + floorUsd: number + } } type CashoutEmail = { diff --git a/src/servers/cron.ts b/src/servers/cron.ts index b08488029..728c42d99 100644 --- a/src/servers/cron.ts +++ b/src/servers/cron.ts @@ -24,6 +24,7 @@ import { reconcileBridgeAndIbexDeposits, reconcileBridgeAndIbexWithdrawals, } from "@services/bridge/reconciliation" +import { checkFygaroTreasuryFloat } from "@services/fygaro/float-monitor" import { elapsedSinceTimestamp, sleep } from "@utils" import { rebalancingInternalChannels } from "@services/lnd/rebalancing" @@ -88,6 +89,13 @@ const reconcileBridgeWithdrawalsJob = async () => { if (result instanceof Error) throw result } +// Bankowner treasury float check for Fygaro auto-credit. Runs every cron +// invocation (~15 min via the k8s CronJob schedule, matching the reconcile +// cadence above); self-guards on FygaroConfig.enabled and never throws. +const checkFygaroFloatJob = async () => { + await checkFygaroTreasuryFloat() +} + const main = async () => { console.log("cronjob started") const start = new Date() @@ -125,6 +133,7 @@ const main = async () => { ...(cronConfig.swapEnabled ? [swapOutJob] : []), reconcileBridgeDepositsJob, reconcileBridgeWithdrawalsJob, + checkFygaroFloatJob, deleteExpiredPaymentFlows, deleteExpiredInvoices, ...(cronConfig.lndTasksEnabled diff --git a/src/services/alerts/dedup-key.ts b/src/services/alerts/dedup-key.ts index 4dd796c0f..47ce19fe2 100644 --- a/src/services/alerts/dedup-key.ts +++ b/src/services/alerts/dedup-key.ts @@ -37,6 +37,11 @@ export const generateDedupKey = { // non-USD, non-positive net). Distinct from fygaroCreditFailed so a "skipped" // warning never suppresses a later "credit attempt failed" critical. fygaroNotCredited: (transactionId: string) => `fygaro:not-credited:${transactionId}`, + // Static keys (no per-request suffix) so the built-in TTL dedup rate-limits + // these to one alert per window rather than one per failing request/poll. + fygaroSignatureFailure: () => "fygaro:signature-failure", + fygaroFloatLow: () => "fygaro:float-low", + fygaroFloatExhausted: () => "fygaro:float-exhausted", } export const normalizeDedupKey = (key: string): string => diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts new file mode 100644 index 000000000..829bb5dea --- /dev/null +++ b/src/services/fygaro/float-monitor.ts @@ -0,0 +1,66 @@ +import { FygaroConfig } from "@config" +import { USDTAmount, WalletCurrency } from "@domain/shared" +import { getBankOwnerWalletId } from "@services/ledger/caching" +import Ibex from "@services/ibex/client" +import { alertBridge, generateDedupKey } from "@services/alerts" +import { baseLogger } from "@services/logger" + +/** + * Proactive bankowner treasury float check (the main value of the float- + * monitoring work). Auto-credit sends card top-ups from the bankowner treasury; + * if that float quietly drains, every subsequent credit fails one-at-a-time. A + * periodic check that pages BEFORE the well runs dry turns that into a single + * "top up soon" heads-up instead of a stream of exhausted-credit incidents. + * + * Contract: this is registered as a cron task and MUST NOT throw — a thrown + * task fails the whole cron run (exit 99 -> CrashLoopBackOff). Every failure + * path here logs and returns. Only runs when the fygaro feature is enabled so + * an unconfigured instance never alerts. + */ +const DEFAULT_FLOOR_USD = 2000 + +export const checkFygaroTreasuryFloat = async (): Promise => { + if (!FygaroConfig.enabled) return + + const floorUsd = FygaroConfig.float?.floorUsd ?? DEFAULT_FLOOR_USD + + try { + // walletId IS the IBEX accountId (see docs/ledger caching). Read the + // treasury's USDT balance straight off the IBEX client. + const walletId = await getBankOwnerWalletId() + const details = await Ibex.getAccountDetails(walletId, WalletCurrency.Usdt) + + if (details instanceof Error) { + // An IBEX read blip must never crash the cron, and must never be mistaken + // for a low balance. Log and bail; the next run re-reads. (Per the "or + // logs" option — a distinct alert here would fight the float-low dedup.) + baseLogger.error( + { err: details }, + "Fygaro float check: could not read bankowner treasury balance", + ) + return + } + + // IBEX omits `balance` for a drained / never-funded account (absent means + // zero — see get-balance-for-wallet.ts). A genuinely empty treasury reads + // as 0 here and correctly trips the floor. + const balanceUsd = + details.balance instanceof USDTAmount ? Number(details.balance.asNumber()) : 0 + + if (balanceUsd < floorUsd) { + baseLogger.warn({ balanceUsd, floorUsd }, "Fygaro treasury float below floor") + alertBridge({ + dedupKey: generateDedupKey.fygaroFloatLow(), + source: "fygaro-webhook", + severity: "warning", + title: "Fygaro treasury float low — top up bankowner", + detail: `balance=$${balanceUsd.toFixed(2)} floor=$${floorUsd.toFixed(2)}`, + context: { balance_usd: balanceUsd, floor_usd: floorUsd }, + }) + } + } catch (err) { + // Belt-and-suspenders: a resolver throw or any unexpected error is + // swallowed so the cron run still succeeds. + baseLogger.error({ err }, "Fygaro float check errored") + } +} diff --git a/src/services/fygaro/webhook-server/credit-topup.ts b/src/services/fygaro/webhook-server/credit-topup.ts index 20ac03a19..901461bf0 100644 --- a/src/services/fygaro/webhook-server/credit-topup.ts +++ b/src/services/fygaro/webhook-server/credit-topup.ts @@ -1,7 +1,21 @@ import { PaymentSendStatus } from "@domain/bitcoin/lightning" import { WalletCurrency } from "@domain/shared" +import { InsufficientBalanceError } from "@domain/errors" import { AccountsRepository, WalletsRepository } from "@services/mongoose" import { baseLogger } from "@services/logger" +import { InsufficientIbexBalance } from "@services/ibex/errors" + +// The treasury can't cover the send. On the flash IBEX-custodial path an +// under-funded bankowner surfaces as InsufficientIbexBalance (ibex/errors.ts +// maps the IBEX "insufficient balance" ApiError); InsufficientBalanceError is +// the domain-level equivalent kept for defence in depth. Matched by class, not +// by message, so a generic send error with a coincidental wording is NOT +// misread as float exhaustion. +const isInsufficientTreasuryBalance = (err: Error): boolean => + err instanceof InsufficientIbexBalance || err instanceof InsufficientBalanceError + +/** Distinct credit-failure step signalling the treasury float is exhausted. */ +export const INSUFFICIENT_TREASURY_FLOAT_STEP = "insufficient-treasury-float" /** * Credits a verified Fygaro card payment to the payer's Flash account: @@ -98,6 +112,13 @@ export const creditFygaroTopup = async ({ { err: result, transactionId, recipientAccountId }, "fygaro credit: intraledger send returned an error", ) + // Distinguish "the treasury is empty" (an ops top-up problem) from every + // other send failure (a bug to debug) so payment.ts can raise the right + // alert. The payment is still recorded and the row stays Fiat Received + // either way — no double-spend risk. + if (isInsufficientTreasuryBalance(result)) { + return new FygaroCreditError(INSUFFICIENT_TREASURY_FLOAT_STEP, result.message) + } return new FygaroCreditError("intraledger-send", result.message) } if (result === PaymentSendStatus.Success) { diff --git a/src/services/fygaro/webhook-server/middleware/verify-signature.ts b/src/services/fygaro/webhook-server/middleware/verify-signature.ts index 7236a6089..6e8e8d140 100644 --- a/src/services/fygaro/webhook-server/middleware/verify-signature.ts +++ b/src/services/fygaro/webhook-server/middleware/verify-signature.ts @@ -4,9 +4,33 @@ import express from "express" import { FygaroConfig } from "@config" import { baseLogger } from "@services/logger" +import { alertBridge, generateDedupKey } from "@services/alerts" type RawBodyRequest = express.Request & { rawBody?: string } +/** + * Page ops when signature verification is failing for a reason that means our + * side is misconfigured — a rotated/wrong shared secret ("we hold a secret but + * the HMAC didn't match") or no secrets configured at all. Left silent, either + * one 401s every real payment while looking healthy: this exact gap caused + * hours of silent card-top-up failures during setup. The static dedup key lets + * the built-in TTL suppression collapse the flood to one alert per window + * rather than one per rejected request. Deliberately NOT fired for a plain + * missing/malformed/expired signature (random internet noise) — those never + * indicate a secret problem and would be pure alert spam. Never carries the + * secret itself — only the (public) key id. + */ +const alertSignatureFailure = (reason: string, keyId?: string): void => { + alertBridge({ + dedupKey: generateDedupKey.fygaroSignatureFailure(), + source: "fygaro-webhook", + severity: "warning", + title: "Fygaro webhook signature verification failing — check the webhook secret", + detail: reason, + context: { key_id: keyId }, + }) +} + /** * Fygaro webhook signature verification. * @@ -66,6 +90,10 @@ export const verifyFygaroSignature = ( { keyId }, "Fygaro webhook rejected: no webhook secrets configured", ) + alertSignatureFailure( + "no webhook secrets configured", + typeof keyId === "string" ? keyId : undefined, + ) return res.status(401).json({ error: "Webhook secret not configured" }) } @@ -106,6 +134,10 @@ export const verifyFygaroSignature = ( }) if (!valid) { baseLogger.warn({ keyId }, "Fygaro webhook rejected: signature mismatch") + alertSignatureFailure( + "HMAC signature mismatch — secret likely rotated or wrong", + typeof keyId === "string" ? keyId : undefined, + ) return res.status(401).json({ error: "Invalid signature" }) } diff --git a/src/services/fygaro/webhook-server/routes/payment.ts b/src/services/fygaro/webhook-server/routes/payment.ts index 70aca5d40..6330f25e9 100644 --- a/src/services/fygaro/webhook-server/routes/payment.ts +++ b/src/services/fygaro/webhook-server/routes/payment.ts @@ -33,7 +33,11 @@ import { import { alertBridge, generateDedupKey } from "@services/alerts" import { notifyOpsEvent } from "@services/alerts/ops-events" -import { creditFygaroTopup, FygaroCreditError } from "../credit-topup" +import { + creditFygaroTopup, + FygaroCreditError, + INSUFFICIENT_TREASURY_FLOAT_STEP, +} from "../credit-topup" import { getFygaroSettings } from "../fygaro-settings" import { evaluateCreditGate, RecordOnlyReason } from "../fees" @@ -324,11 +328,21 @@ export const paymentHandler = async (req: Request, res: Response) => { { error: creditResult, transactionId, accountId: creditAccountId }, "Fygaro payment recorded but auto-credit failed", ) + // Treasury-float exhaustion is an ops top-up problem, not a bug to + // debug — raise a distinct critical (with its own static dedup key so + // a run of exhausted credits collapses to one page) that names the + // fix. Every other credit failure keeps the generic per-transaction + // "manual credit needed" alert. + const floatExhausted = creditResult.step === INSUFFICIENT_TREASURY_FLOAT_STEP alertBridge({ - dedupKey: generateDedupKey.fygaroCreditFailed(transactionId), + dedupKey: floatExhausted + ? generateDedupKey.fygaroFloatExhausted() + : generateDedupKey.fygaroCreditFailed(transactionId), source: "fygaro-webhook", severity: "critical", - title: "Fygaro auto-credit failed — manual credit needed", + title: floatExhausted + ? "Fygaro treasury float EXHAUSTED — top up bankowner immediately" + : "Fygaro auto-credit failed — manual credit needed", detail: `${creditResult.step}: ${creditResult.message}`, context: { transaction_id: transactionId, diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts new file mode 100644 index 000000000..0290619c6 --- /dev/null +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -0,0 +1,153 @@ +import { USDTAmount, WalletCurrency } from "@domain/shared" + +const mockFygaroConfig = { + enabled: true, + float: { floorUsd: 2000 } as { floorUsd: number } | undefined, +} + +jest.mock("@config", () => ({ + get FygaroConfig() { + return mockFygaroConfig + }, +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +const mockGetBankOwnerWalletId = jest.fn() +jest.mock("@services/ledger/caching", () => ({ + getBankOwnerWalletId: (...args: unknown[]) => mockGetBankOwnerWalletId(...args), +})) + +const mockGetAccountDetails = jest.fn() +jest.mock("@services/ibex/client", () => ({ + __esModule: true, + default: { + getAccountDetails: (...args: unknown[]) => mockGetAccountDetails(...args), + }, +})) + +const mockAlertBridge = jest.fn() +jest.mock("@services/alerts", () => ({ + alertBridge: (...args: unknown[]) => mockAlertBridge(...args), + generateDedupKey: { + fygaroFloatLow: () => "fygaro:float-low", + }, +})) + +import { checkFygaroTreasuryFloat } from "@services/fygaro/float-monitor" + +const WALLET_ID = "bankowner-usdt-wallet" as WalletId + +const usdt = (dollars: string): USDTAmount => { + const amt = USDTAmount.fromNumber(dollars) + if (amt instanceof Error) throw amt + return amt +} + +const detailsWithBalance = (balance: USDTAmount | undefined) => ({ + id: WALLET_ID, + userId: "u", + name: "bankowner", + balance, +}) + +beforeEach(() => { + jest.clearAllMocks() + mockFygaroConfig.enabled = true + mockFygaroConfig.float = { floorUsd: 2000 } + mockGetBankOwnerWalletId.mockResolvedValue(WALLET_ID) + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("5000"))) +}) + +describe("checkFygaroTreasuryFloat", () => { + it("reads the bankowner USDT balance from IBEX", async () => { + await checkFygaroTreasuryFloat() + + expect(mockGetBankOwnerWalletId).toHaveBeenCalled() + expect(mockGetAccountDetails).toHaveBeenCalledWith(WALLET_ID, WalletCurrency.Usdt) + }) + + it("alerts (warning) when the balance is below the floor", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("1500"))) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + const alert = mockAlertBridge.mock.calls[0][0] + expect(alert).toMatchObject({ + dedupKey: "fygaro:float-low", + source: "fygaro-webhook", + severity: "warning", + }) + expect(alert.title).toMatch(/float low/i) + expect(alert.context).toEqual({ balance_usd: 1500, floor_usd: 2000 }) + }) + + it("treats an absent (drained account) balance as zero and alerts", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(undefined)) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + expect(mockAlertBridge.mock.calls[0][0].context).toEqual({ + balance_usd: 0, + floor_usd: 2000, + }) + }) + + it("does NOT alert when the balance equals the floor", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("2000"))) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does NOT alert when the balance is above the floor", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("2500"))) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does not crash and does not alert when the IBEX read fails", async () => { + mockGetAccountDetails.mockResolvedValue(new Error("ibex unreachable")) + + await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does not crash when the wallet resolver throws", async () => { + mockGetBankOwnerWalletId.mockRejectedValue(new Error("no bankowner")) + + await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("skips entirely when the fygaro feature is disabled", async () => { + mockFygaroConfig.enabled = false + + await checkFygaroTreasuryFloat() + + expect(mockGetBankOwnerWalletId).not.toHaveBeenCalled() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("falls back to the default floor when float config is absent", async () => { + mockFygaroConfig.float = undefined + // Default floor is 2000; 1000 is below it. + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("1000"))) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + expect(mockAlertBridge.mock.calls[0][0].context).toEqual({ + balance_usd: 1000, + floor_usd: 2000, + }) + }) +}) 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 index f730b6548..5fea42801 100644 --- a/test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/credit-topup.spec.ts @@ -29,6 +29,8 @@ import { creditFygaroTopup, FygaroCreditError, } from "@services/fygaro/webhook-server/credit-topup" +import { InsufficientIbexBalance } from "@services/ibex/errors" +import { InsufficientBalanceError } from "@domain/errors" const TREASURY_ACCOUNT_ID = "treasury-account" as AccountId const RECIPIENT_ACCOUNT_ID = "recipient-account" as AccountId @@ -143,7 +145,9 @@ describe("creditFygaroTopup", () => { expect(mockIntraledgerSend).not.toHaveBeenCalled() }) - it("surfaces a send error as an intraledger-send failure", async () => { + it("surfaces a generic send error as an intraledger-send failure", async () => { + // A plain Error whose message merely mentions balance must NOT be + // misclassified as float exhaustion — detection is by error class. mockIntraledgerSend.mockResolvedValue(new Error("InsufficientBalanceError")) const result = await credit() @@ -152,6 +156,26 @@ describe("creditFygaroTopup", () => { expect((result as FygaroCreditError).step).toBe("intraledger-send") }) + it("maps an IBEX insufficient-balance failure to the insufficient-treasury-float step", async () => { + mockIntraledgerSend.mockResolvedValue( + new InsufficientIbexBalance(new Error("insufficient balance")), + ) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("insufficient-treasury-float") + }) + + it("maps a domain InsufficientBalanceError to the insufficient-treasury-float step", async () => { + mockIntraledgerSend.mockResolvedValue(new InsufficientBalanceError("balance too low")) + + const result = await credit() + + expect(result).toBeInstanceOf(FygaroCreditError) + expect((result as FygaroCreditError).step).toBe("insufficient-treasury-float") + }) + it("fails on an unexpected payment status instead of assuming success", async () => { mockIntraledgerSend.mockResolvedValue(PaymentSendStatus.AlreadyPaid) 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 bf9a46f49..01103274d 100644 --- a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts @@ -56,6 +56,7 @@ jest.mock("@services/fygaro/webhook-server/credit-topup", () => { } return { FygaroCreditError, + INSUFFICIENT_TREASURY_FLOAT_STEP: "insufficient-treasury-float", creditFygaroTopup: (...args: unknown[]) => mockCreditFygaroTopup(...args), } }) @@ -280,9 +281,9 @@ describe("fygaro paymentHandler", () => { expect(res.json).toHaveBeenCalledWith({ status: "success", credited: true }) }) - it("records without crediting and alerts critical when the credit fails", async () => { + it("records without crediting and fires the generic critical when the credit fails", async () => { mockCreditFygaroTopup.mockResolvedValue( - new FygaroCreditError("intraledger-send", "insufficient balance"), + new FygaroCreditError("intraledger-send", "some send error"), ) const res = makeRes() @@ -290,7 +291,10 @@ describe("fygaro paymentHandler", () => { expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() expect(mockAlertBridge).toHaveBeenCalledWith( - expect.objectContaining({ severity: "critical" }), + expect.objectContaining({ + severity: "critical", + title: "Fygaro auto-credit failed — manual credit needed", + }), ) expect(mockNotifyOpsEvent).toHaveBeenCalledWith( expect.objectContaining({ status: "failed", step: "credit:intraledger-send" }), @@ -299,6 +303,26 @@ describe("fygaro paymentHandler", () => { expect(res.json).toHaveBeenCalledWith({ status: "recorded", credited: false }) }) + it("fires the distinct float-EXHAUSTED critical when the treasury can't cover the send", async () => { + mockCreditFygaroTopup.mockResolvedValue( + new FygaroCreditError("insufficient-treasury-float", "insufficient balance"), + ) + const res = makeRes() + + await paymentHandler(makeReq(VALID_BODY), res) + + expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() + expect(mockAlertBridge).toHaveBeenCalledWith( + expect.objectContaining({ + severity: "critical", + title: "Fygaro treasury float EXHAUSTED — top up bankowner immediately", + }), + ) + // The row still stays Fiat Received — the existing safety is unchanged. + expect(res.status).toHaveBeenCalledWith(200) + 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() 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 index 305cb9ed1..743cab4b9 100644 --- a/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts @@ -22,6 +22,14 @@ jest.mock("@services/logger", () => ({ baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })) +const mockAlertBridge = jest.fn() +jest.mock("@services/alerts", () => ({ + alertBridge: (...args: unknown[]) => mockAlertBridge(...args), + generateDedupKey: { + fygaroSignatureFailure: () => "fygaro:signature-failure", + }, +})) + import { verifyFygaroSignature } from "@services/fygaro/webhook-server/middleware/verify-signature" const RAW_BODY = JSON.stringify({ transactionId: "tx-1", amount: "10.00" }) @@ -212,4 +220,74 @@ describe("verifyFygaroSignature", () => { expect(next).not.toHaveBeenCalled() expect(res.status).toHaveBeenCalledWith(400) }) + + describe("signature-failure alerting", () => { + it("alerts (warning, static dedup key) on a signature mismatch, without the secret", () => { + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "wrong-secret")}`, + keyId: "key1", + }) + + verifyFygaroSignature(req, makeRes(), jest.fn()) + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + const alert = mockAlertBridge.mock.calls[0][0] + expect(alert).toMatchObject({ + dedupKey: "fygaro:signature-failure", + source: "fygaro-webhook", + severity: "warning", + }) + expect(alert.title).toMatch(/signature verification failing/i) + // The key id is safe to include; the secret never is. + expect(alert.context).toEqual({ key_id: "key1" }) + expect(JSON.stringify(alert)).not.toContain("secret-one") + expect(JSON.stringify(alert)).not.toContain("wrong-secret") + }) + + it("alerts when no webhook secrets are configured", () => { + mockFygaroConfig.webhook.secrets = {} + const t = nowSeconds() + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + + verifyFygaroSignature(req, makeRes(), jest.fn()) + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + expect(mockAlertBridge.mock.calls[0][0]).toMatchObject({ + dedupKey: "fygaro:signature-failure", + severity: "warning", + }) + }) + + it("does NOT alert on a missing signature header (random internet noise)", () => { + verifyFygaroSignature(makeReq({}), makeRes(), jest.fn()) + + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does NOT alert on a malformed signature header", () => { + verifyFygaroSignature( + makeReq({ signature: "not-a-signature" }), + makeRes(), + jest.fn(), + ) + + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does NOT alert on a timestamp outside the allowed skew", () => { + const t = String(Math.floor(Date.now() / 1000) - 3600) + const req = makeReq({ + signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, + keyId: "key1", + }) + + verifyFygaroSignature(req, makeRes(), jest.fn()) + + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + }) }) From 9f8d52c83e9ee80ae0cb2108b066c6e49556f9b3 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 22:09:09 -0700 Subject: [PATCH 2/7] fix(fygaro): monitor the treasury USDT funding wallet, not the default wallet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review found the proactive float monitor read the wrong account. It resolved the balance via getBankOwnerWalletId() — the bankowner account's default (USD) wallet — while auto-credit actually spends from the bankowner USDT wallet (credit-topup.ts selects find(Usdt) ?? find(Usd)). In flash's IBEX-custodial model each walletId is its own IBEX account, so the monitor was reading a different account's balance and mis-parsing it as USDT. A drained USDT float would hide behind a funded USD wallet (no page ever fires) and a low USD wallet would false-alarm as "USDT float low". Resolve the funding wallet exactly the way credit-topup does — findByRole ("bankowner") -> listByAccountId -> find(Usdt) ?? find(Usd) — and read THAT wallet's balance in its own currency, so the monitored account is provably the funding source. Parse both USDT and USD balances so the USD fallback is not scored as an empty USDT float. Tests now drive the real resolution path: assert getAccountDetails is called with the USDT wallet id (USD wallet listed first, proving currency- not order-based selection), add a USD-fallback case, and cover the account/wallet-resolution failure paths. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014QwZjNcKfkMhrBE33HcUVN --- src/services/fygaro/float-monitor.ts | 62 ++++++++-- .../services/fygaro/float-monitor.spec.ts | 117 ++++++++++++++++-- 2 files changed, 158 insertions(+), 21 deletions(-) diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts index 829bb5dea..d24304387 100644 --- a/src/services/fygaro/float-monitor.ts +++ b/src/services/fygaro/float-monitor.ts @@ -1,6 +1,6 @@ import { FygaroConfig } from "@config" -import { USDTAmount, WalletCurrency } from "@domain/shared" -import { getBankOwnerWalletId } from "@services/ledger/caching" +import { USDAmount, USDTAmount, WalletCurrency } from "@domain/shared" +import { AccountsRepository, WalletsRepository } from "@services/mongoose" import Ibex from "@services/ibex/client" import { alertBridge, generateDedupKey } from "@services/alerts" import { baseLogger } from "@services/logger" @@ -19,16 +19,55 @@ import { baseLogger } from "@services/logger" */ const DEFAULT_FLOOR_USD = 2000 +// The role whose account funds auto-credit sends (credit-topup.ts). +const TREASURY_ROLE = "bankowner" + export const checkFygaroTreasuryFloat = async (): Promise => { if (!FygaroConfig.enabled) return const floorUsd = FygaroConfig.float?.floorUsd ?? DEFAULT_FLOOR_USD try { - // walletId IS the IBEX accountId (see docs/ledger caching). Read the - // treasury's USDT balance straight off the IBEX client. - const walletId = await getBankOwnerWalletId() - const details = await Ibex.getAccountDetails(walletId, WalletCurrency.Usdt) + // Read the balance of the SAME wallet auto-credit actually spends from — + // resolved exactly the way credit-topup.ts does — so the monitored account + // is provably the funding source. In flash's IBEX-custodial model each + // walletId is its own IBEX account, so reading the bankowner account's + // default wallet would read a DIFFERENT account's balance (typically the USD + // wallet) and mis-parse it as the USDT float: a drained USDT float would + // hide behind a funded USD wallet (no page ever fires) and a low USD wallet + // would false-alarm as "USDT float low". + const treasury = await AccountsRepository().findByRole(TREASURY_ROLE) + if (treasury instanceof Error) { + baseLogger.error( + { err: treasury, role: TREASURY_ROLE }, + "Fygaro float check: could not resolve the bankowner treasury account", + ) + return + } + + const wallets = await WalletsRepository().listByAccountId(treasury.id) + if (wallets instanceof Error) { + baseLogger.error( + { err: wallets }, + "Fygaro float check: could not list bankowner treasury wallets", + ) + return + } + + // Prefer the USDT wallet (the active cash wallet), falling back to the + // legacy USD wallet — the exact selection credit-topup makes for the send. + const funding = + wallets.find((w) => w.currency === WalletCurrency.Usdt) ?? + wallets.find((w) => w.currency === WalletCurrency.Usd) + if (!funding) { + baseLogger.error( + { role: TREASURY_ROLE }, + "Fygaro float check: treasury account has no USDT or USD wallet", + ) + return + } + + const details = await Ibex.getAccountDetails(funding.id, funding.currency) if (details instanceof Error) { // An IBEX read blip must never crash the cron, and must never be mistaken @@ -43,9 +82,16 @@ export const checkFygaroTreasuryFloat = async (): Promise => { // IBEX omits `balance` for a drained / never-funded account (absent means // zero — see get-balance-for-wallet.ts). A genuinely empty treasury reads - // as 0 here and correctly trips the floor. + // as 0 here and correctly trips the floor. Read the balance in the funding + // wallet's own currency so the USD fallback is not scored as an empty USDT + // float (or vice versa). + const balance = details.balance const balanceUsd = - details.balance instanceof USDTAmount ? Number(details.balance.asNumber()) : 0 + balance instanceof USDTAmount + ? Number(balance.asNumber()) + : balance instanceof USDAmount + ? Number(balance.asDollars()) + : 0 if (balanceUsd < floorUsd) { baseLogger.warn({ balanceUsd, floorUsd }, "Fygaro treasury float below floor") diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts index 0290619c6..db46a46c1 100644 --- a/test/flash/unit/services/fygaro/float-monitor.spec.ts +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -1,4 +1,4 @@ -import { USDTAmount, WalletCurrency } from "@domain/shared" +import { USDAmount, USDTAmount, WalletCurrency } from "@domain/shared" const mockFygaroConfig = { enabled: true, @@ -15,9 +15,15 @@ jest.mock("@services/logger", () => ({ baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })) -const mockGetBankOwnerWalletId = jest.fn() -jest.mock("@services/ledger/caching", () => ({ - getBankOwnerWalletId: (...args: unknown[]) => mockGetBankOwnerWalletId(...args), +const mockFindByRole = jest.fn() +const mockListByAccountId = jest.fn() +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ + findByRole: (...args: unknown[]) => mockFindByRole(...args), + }), + WalletsRepository: () => ({ + listByAccountId: (...args: unknown[]) => mockListByAccountId(...args), + }), })) const mockGetAccountDetails = jest.fn() @@ -38,7 +44,26 @@ jest.mock("@services/alerts", () => ({ import { checkFygaroTreasuryFloat } from "@services/fygaro/float-monitor" -const WALLET_ID = "bankowner-usdt-wallet" as WalletId +const TREASURY_ACCOUNT_ID = "bankowner-account" as AccountId +const USDT_WALLET_ID = "bankowner-usdt-wallet" as WalletId +const USD_WALLET_ID = "bankowner-usd-wallet" as WalletId +const BTC_WALLET_ID = "bankowner-btc-wallet" as WalletId + +const usdtWallet = { + id: USDT_WALLET_ID, + accountId: TREASURY_ACCOUNT_ID, + currency: WalletCurrency.Usdt, +} as unknown as Wallet +const usdWallet = { + id: USD_WALLET_ID, + accountId: TREASURY_ACCOUNT_ID, + currency: WalletCurrency.Usd, +} as unknown as Wallet +const btcWallet = { + id: BTC_WALLET_ID, + accountId: TREASURY_ACCOUNT_ID, + currency: WalletCurrency.Btc, +} as unknown as Wallet const usdt = (dollars: string): USDTAmount => { const amt = USDTAmount.fromNumber(dollars) @@ -46,8 +71,14 @@ const usdt = (dollars: string): USDTAmount => { return amt } -const detailsWithBalance = (balance: USDTAmount | undefined) => ({ - id: WALLET_ID, +const usd = (dollars: string): USDAmount => { + const amt = USDAmount.dollars(dollars) + if (amt instanceof Error) throw amt + return amt +} + +const detailsWithBalance = (balance: USDTAmount | USDAmount | undefined) => ({ + id: USDT_WALLET_ID, userId: "u", name: "bankowner", balance, @@ -57,16 +88,50 @@ beforeEach(() => { jest.clearAllMocks() mockFygaroConfig.enabled = true mockFygaroConfig.float = { floorUsd: 2000 } - mockGetBankOwnerWalletId.mockResolvedValue(WALLET_ID) + mockFindByRole.mockResolvedValue({ id: TREASURY_ACCOUNT_ID }) + // USD wallet listed first on purpose: selection must pick the USDT wallet by + // currency, not by list order. + mockListByAccountId.mockResolvedValue([usdWallet, usdtWallet]) mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("5000"))) }) describe("checkFygaroTreasuryFloat", () => { - it("reads the bankowner USDT balance from IBEX", async () => { + it("reads the balance of the treasury's USDT funding wallet from IBEX", async () => { + await checkFygaroTreasuryFloat() + + expect(mockFindByRole).toHaveBeenCalledWith("bankowner") + expect(mockListByAccountId).toHaveBeenCalledWith(TREASURY_ACCOUNT_ID) + // The wallet queried MUST be the same one auto-credit spends from: the USDT + // wallet, read in its own currency — not the account's default (USD) wallet. + expect(mockGetAccountDetails).toHaveBeenCalledWith( + USDT_WALLET_ID, + WalletCurrency.Usdt, + ) + }) + + it("falls back to the legacy USD wallet when the treasury has no USDT wallet", async () => { + mockListByAccountId.mockResolvedValue([usdWallet]) + // A funded USD wallet above the floor must NOT alert (and must be read in + // USD, not mis-parsed as a zero USDT balance). + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usd("5000"))) + await checkFygaroTreasuryFloat() - expect(mockGetBankOwnerWalletId).toHaveBeenCalled() - expect(mockGetAccountDetails).toHaveBeenCalledWith(WALLET_ID, WalletCurrency.Usdt) + expect(mockGetAccountDetails).toHaveBeenCalledWith(USD_WALLET_ID, WalletCurrency.Usd) + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("alerts on a low USD-wallet fallback balance (scored in USD, not as zero)", async () => { + mockListByAccountId.mockResolvedValue([usdWallet]) + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usd("1500"))) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + expect(mockAlertBridge.mock.calls[0][0].context).toEqual({ + balance_usd: 1500, + floor_usd: 2000, + }) }) it("alerts (warning) when the balance is below the floor", async () => { @@ -120,8 +185,33 @@ describe("checkFygaroTreasuryFloat", () => { expect(mockAlertBridge).not.toHaveBeenCalled() }) + it("does not read IBEX or alert when the treasury account cannot be resolved", async () => { + mockFindByRole.mockResolvedValue(new Error("no bankowner")) + + await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() + expect(mockListByAccountId).not.toHaveBeenCalled() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does not read IBEX or alert when listing treasury wallets fails", async () => { + mockListByAccountId.mockResolvedValue(new Error("mongo down")) + + await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("does not read IBEX or alert when the treasury has no USDT or USD wallet", async () => { + mockListByAccountId.mockResolvedValue([btcWallet]) + + await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + it("does not crash when the wallet resolver throws", async () => { - mockGetBankOwnerWalletId.mockRejectedValue(new Error("no bankowner")) + mockFindByRole.mockRejectedValue(new Error("boom")) await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() expect(mockAlertBridge).not.toHaveBeenCalled() @@ -132,7 +222,8 @@ describe("checkFygaroTreasuryFloat", () => { await checkFygaroTreasuryFloat() - expect(mockGetBankOwnerWalletId).not.toHaveBeenCalled() + expect(mockFindByRole).not.toHaveBeenCalled() + expect(mockListByAccountId).not.toHaveBeenCalled() expect(mockGetAccountDetails).not.toHaveBeenCalled() expect(mockAlertBridge).not.toHaveBeenCalled() }) From fc01d8ebfbd770c385f794b35e1e98f447c9c525 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 22:20:11 -0700 Subject: [PATCH 3/7] fix(fygaro): gate float monitor on auto-credit and share treasury resolver Two code-review fixes to the Fygaro float monitor: - Gate the cron on the auto-credit master flag, not just the feature flag. During the record-only phase (fygaro.enabled=true, credit.enabled=false) nothing spends from the bankowner treasury, so paging "top up bankowner" every window was premature noise contradicting the alert's own instruction. Now returns early on `!FygaroConfig.enabled || !FygaroConfig.credit?.enabled`. - Extract `resolveFygaroTreasuryFundingWallet` in credit-topup and call it from both the credit path and the float monitor. The funding-wallet selection (bankowner -> USDT ?? USD) was duplicated verbatim; the "monitor the exact wallet auto-credit spends from" invariant is now enforced by shared code instead of two byte-identical copies that could silently drift apart. Tests: assert the monitor performs no repository/IBEX read and fires no alert when auto-credit is disabled (and when the credit block is absent), mirroring the feature-disabled test. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014QwZjNcKfkMhrBE33HcUVN --- src/services/fygaro/float-monitor.ts | 65 +++++++--------- .../fygaro/webhook-server/credit-topup.ts | 75 ++++++++++++++----- .../services/fygaro/float-monitor.spec.ts | 28 +++++++ 3 files changed, 111 insertions(+), 57 deletions(-) diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts index d24304387..fc9de01ae 100644 --- a/src/services/fygaro/float-monitor.ts +++ b/src/services/fygaro/float-monitor.ts @@ -1,10 +1,14 @@ import { FygaroConfig } from "@config" -import { USDAmount, USDTAmount, WalletCurrency } from "@domain/shared" -import { AccountsRepository, WalletsRepository } from "@services/mongoose" +import { USDAmount, USDTAmount } from "@domain/shared" import Ibex from "@services/ibex/client" import { alertBridge, generateDedupKey } from "@services/alerts" import { baseLogger } from "@services/logger" +import { + FygaroCreditError, + resolveFygaroTreasuryFundingWallet, +} from "./webhook-server/credit-topup" + /** * Proactive bankowner treasury float check (the main value of the float- * monitoring work). Auto-credit sends card top-ups from the bankowner treasury; @@ -14,60 +18,45 @@ import { baseLogger } from "@services/logger" * * Contract: this is registered as a cron task and MUST NOT throw — a thrown * task fails the whole cron run (exit 99 -> CrashLoopBackOff). Every failure - * path here logs and returns. Only runs when the fygaro feature is enabled so - * an unconfigured instance never alerts. + * path here logs and returns. Only runs when the fygaro feature is enabled AND + * auto-credit is on: during the record-only phase (fygaro.enabled=true, + * credit.enabled=false) nothing ever spends from the treasury, so paging "top + * up bankowner" would be premature noise that contradicts its own instruction. */ const DEFAULT_FLOOR_USD = 2000 -// The role whose account funds auto-credit sends (credit-topup.ts). -const TREASURY_ROLE = "bankowner" - export const checkFygaroTreasuryFloat = async (): Promise => { - if (!FygaroConfig.enabled) return + // Gate on both the feature flag and the auto-credit master gate. With + // credit.enabled=false the webhook only records payments and the treasury -> + // user transfer stays manual, so there is nothing for this monitor to fund + // yet — running it would page every window over a float no credit touches. + if (!FygaroConfig.enabled || !FygaroConfig.credit?.enabled) return const floorUsd = FygaroConfig.float?.floorUsd ?? DEFAULT_FLOOR_USD try { // Read the balance of the SAME wallet auto-credit actually spends from — - // resolved exactly the way credit-topup.ts does — so the monitored account - // is provably the funding source. In flash's IBEX-custodial model each - // walletId is its own IBEX account, so reading the bankowner account's + // resolved through the shared resolver credit-topup uses — so the monitored + // account is provably the funding source. In flash's IBEX-custodial model + // each walletId is its own IBEX account, so reading the bankowner account's // default wallet would read a DIFFERENT account's balance (typically the USD // wallet) and mis-parse it as the USDT float: a drained USDT float would // hide behind a funded USD wallet (no page ever fires) and a low USD wallet // would false-alarm as "USDT float low". - const treasury = await AccountsRepository().findByRole(TREASURY_ROLE) - if (treasury instanceof Error) { - baseLogger.error( - { err: treasury, role: TREASURY_ROLE }, - "Fygaro float check: could not resolve the bankowner treasury account", - ) - return - } - - const wallets = await WalletsRepository().listByAccountId(treasury.id) - if (wallets instanceof Error) { - baseLogger.error( - { err: wallets }, - "Fygaro float check: could not list bankowner treasury wallets", - ) - return - } - - // Prefer the USDT wallet (the active cash wallet), falling back to the - // legacy USD wallet — the exact selection credit-topup makes for the send. - const funding = - wallets.find((w) => w.currency === WalletCurrency.Usdt) ?? - wallets.find((w) => w.currency === WalletCurrency.Usd) - if (!funding) { + const funding = await resolveFygaroTreasuryFundingWallet() + if (funding instanceof FygaroCreditError) { + // Could not resolve the treasury or its funding wallet. Log and bail; the + // next run re-reads. Never alert here (a distinct alert would fight the + // float-low dedup) and never crash the cron. baseLogger.error( - { role: TREASURY_ROLE }, - "Fygaro float check: treasury account has no USDT or USD wallet", + { step: funding.step, detail: funding.message }, + "Fygaro float check: could not resolve the bankowner treasury funding wallet", ) return } - const details = await Ibex.getAccountDetails(funding.id, funding.currency) + const fundingWallet = funding.fundingWallet + const details = await Ibex.getAccountDetails(fundingWallet.id, fundingWallet.currency) if (details instanceof Error) { // An IBEX read blip must never crash the cron, and must never be mistaken diff --git a/src/services/fygaro/webhook-server/credit-topup.ts b/src/services/fygaro/webhook-server/credit-topup.ts index 901461bf0..10e03795f 100644 --- a/src/services/fygaro/webhook-server/credit-topup.ts +++ b/src/services/fygaro/webhook-server/credit-topup.ts @@ -46,6 +46,56 @@ const walletsFor = async (accountId: AccountId): Promise => { return wallets instanceof Error ? [] : wallets } +export type FygaroTreasuryFunding = { + account: Account + fundingWallet: Wallet +} + +/** + * Resolves the bankowner treasury account and the exact wallet auto-credit + * spends from: the USDT cash wallet on every account (see + * accounts/create-account.ts), falling back to the legacy USD wallet. + * + * This is the SINGLE source of truth for treasury funding-wallet selection. + * Both the credit path (creditFygaroTopup, below) and the proactive float + * monitor (float-monitor.ts) resolve the funding wallet through here, so the + * monitor can never silently drift onto a different wallet than the one credits + * actually drain — the round-1 "monitor the exact wallet auto-credit spends + * from" invariant is now enforced by shared code, not by two copies staying + * byte-identical. + */ +export const resolveFygaroTreasuryFundingWallet = async (): Promise< + FygaroTreasuryFunding | FygaroCreditError +> => { + const account = await AccountsRepository().findByRole(TREASURY_ROLE) + if (account instanceof Error) { + return new FygaroCreditError( + "resolve-treasury", + `no account holds the '${TREASURY_ROLE}' role`, + ) + } + + const wallets = await WalletsRepository().listByAccountId(account.id) + if (wallets instanceof Error) { + return new FygaroCreditError( + "list-treasury-wallets", + "could not list treasury wallets", + ) + } + + const fundingWallet = + wallets.find((w) => w.currency === WalletCurrency.Usdt) ?? + wallets.find((w) => w.currency === WalletCurrency.Usd) + if (!fundingWallet) { + return new FygaroCreditError( + "resolve-treasury-wallet", + "treasury account has no USDT or USD wallet", + ) + } + + return { account, fundingWallet } +} + export const creditFygaroTopup = async ({ recipientAccountId, amountCents, @@ -61,26 +111,13 @@ export const creditFygaroTopup = async ({ 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", - ) + // Resolve the treasury funding wallet through the shared resolver so the + // wallet credits drain from is provably the same one float-monitor watches. + const funding = await resolveFygaroTreasuryFundingWallet() + if (funding instanceof FygaroCreditError) { + return funding } + const { fundingWallet } = funding // Recipients must hold a wallet in the funding wallet's currency — // send-intraledger rejects cross-currency sends. diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts index db46a46c1..d398eb86e 100644 --- a/test/flash/unit/services/fygaro/float-monitor.spec.ts +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -2,6 +2,7 @@ import { USDAmount, USDTAmount, WalletCurrency } from "@domain/shared" const mockFygaroConfig = { enabled: true, + credit: { enabled: true } as { enabled: boolean } | undefined, float: { floorUsd: 2000 } as { floorUsd: number } | undefined, } @@ -87,6 +88,7 @@ const detailsWithBalance = (balance: USDTAmount | USDAmount | undefined) => ({ beforeEach(() => { jest.clearAllMocks() mockFygaroConfig.enabled = true + mockFygaroConfig.credit = { enabled: true } mockFygaroConfig.float = { floorUsd: 2000 } mockFindByRole.mockResolvedValue({ id: TREASURY_ACCOUNT_ID }) // USD wallet listed first on purpose: selection must pick the USDT wallet by @@ -228,6 +230,32 @@ describe("checkFygaroTreasuryFloat", () => { expect(mockAlertBridge).not.toHaveBeenCalled() }) + it("skips entirely during the record-only phase (auto-credit disabled)", async () => { + // fygaro.enabled=true but credit.enabled=false: the webhook only records + // payments, nothing spends from the treasury, so the monitor must do no + // repository/IBEX read and fire no page — paging "top up bankowner" here + // would be premature noise contradicting the alert's own instruction. + mockFygaroConfig.credit = { enabled: false } + + await checkFygaroTreasuryFloat() + + expect(mockFindByRole).not.toHaveBeenCalled() + expect(mockListByAccountId).not.toHaveBeenCalled() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("skips entirely when the credit config block is absent", async () => { + mockFygaroConfig.credit = undefined + + await checkFygaroTreasuryFloat() + + expect(mockFindByRole).not.toHaveBeenCalled() + expect(mockListByAccountId).not.toHaveBeenCalled() + expect(mockGetAccountDetails).not.toHaveBeenCalled() + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + it("falls back to the default floor when float config is absent", async () => { mockFygaroConfig.float = undefined // Default floor is 2000; 1000 is below it. From 3a1f4b2f5d05e7717c6962ed73bf968f6ce6f45f Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 22:29:22 -0700 Subject: [PATCH 4/7] fix(fygaro): rate-limit float-low alert across cron runs via Redis marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Residual review finding: the float monitor runs as a one-shot cron Job, so alertBridge's process-local in-memory dedup resets every ~15-min run and cannot suppress the float-low warning across runs — a treasury below the floor would page every run (~4/hr) instead of the ~1/hr the alert layer implies. Gate the alert on a Redis NX marker (1h TTL) that survives the process restarts. A Redis error falls through to alerting: over-notifying is the safe failure mode for a draining float, never silence. Tests: the marker suppresses across runs, and a Redis outage still alerts (fail-open). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- src/services/fygaro/float-monitor.ts | 51 ++++++++++++++++--- .../services/fygaro/float-monitor.spec.ts | 36 +++++++++++++ 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts index fc9de01ae..915758c7c 100644 --- a/src/services/fygaro/float-monitor.ts +++ b/src/services/fygaro/float-monitor.ts @@ -3,6 +3,7 @@ import { USDAmount, USDTAmount } from "@domain/shared" import Ibex from "@services/ibex/client" import { alertBridge, generateDedupKey } from "@services/alerts" import { baseLogger } from "@services/logger" +import { redis } from "@services/redis" import { FygaroCreditError, @@ -25,6 +26,36 @@ import { */ const DEFAULT_FLOOR_USD = 2000 +// Cross-run rate limit for the float-low warning. alertBridge's own TTL dedup +// is a process-local Map, but this monitor runs as a one-shot cron Job (a fresh +// process every ~15-min run), so that dedup resets each run and cannot suppress +// anything across runs. Without this a treasury sitting below the floor would +// page every run (~4/hr) instead of the ~1/hr the alert layer implies. A Redis +// NX marker with a 1h TTL gives real cross-run suppression that survives the +// process restarts. A Redis error falls through to alerting: over-notifying is +// the safe failure mode for a draining float, never staying silent. +const FLOAT_LOW_ALERT_WINDOW_SECONDS = 3600 +const FLOAT_LOW_ALERT_MARKER = "fygaro:float-low:alerted" + +const claimFloatLowAlertSlot = async (): Promise => { + try { + const set = await redis.set( + FLOAT_LOW_ALERT_MARKER, + "1", + "EX", + FLOAT_LOW_ALERT_WINDOW_SECONDS, + "NX", + ) + return set === "OK" + } catch (err) { + baseLogger.warn( + { err }, + "Fygaro float check: dedup marker unavailable, alerting anyway", + ) + return true + } +} + export const checkFygaroTreasuryFloat = async (): Promise => { // Gate on both the feature flag and the auto-credit master gate. With // credit.enabled=false the webhook only records payments and the treasury -> @@ -84,14 +115,18 @@ export const checkFygaroTreasuryFloat = async (): Promise => { if (balanceUsd < floorUsd) { baseLogger.warn({ balanceUsd, floorUsd }, "Fygaro treasury float below floor") - alertBridge({ - dedupKey: generateDedupKey.fygaroFloatLow(), - source: "fygaro-webhook", - severity: "warning", - title: "Fygaro treasury float low — top up bankowner", - detail: `balance=$${balanceUsd.toFixed(2)} floor=$${floorUsd.toFixed(2)}`, - context: { balance_usd: balanceUsd, floor_usd: floorUsd }, - }) + // Rate-limit across cron runs via the Redis marker (see above); without + // it a persistent low float would page every ~15-min run. + if (await claimFloatLowAlertSlot()) { + alertBridge({ + dedupKey: generateDedupKey.fygaroFloatLow(), + source: "fygaro-webhook", + severity: "warning", + title: "Fygaro treasury float low — top up bankowner", + detail: `balance=$${balanceUsd.toFixed(2)} floor=$${floorUsd.toFixed(2)}`, + context: { balance_usd: balanceUsd, floor_usd: floorUsd }, + }) + } } } catch (err) { // Belt-and-suspenders: a resolver throw or any unexpected error is diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts index d398eb86e..1e7f674ad 100644 --- a/test/flash/unit/services/fygaro/float-monitor.spec.ts +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -43,6 +43,13 @@ jest.mock("@services/alerts", () => ({ }, })) +// The cron is a one-shot Job, so cross-run rate limiting is a Redis NX marker, +// not the in-memory alert dedup. Mock redis.set so tests drive the claim path. +const mockRedisSet = jest.fn() +jest.mock("@services/redis", () => ({ + redis: { set: (...args: unknown[]) => mockRedisSet(...args) }, +})) + import { checkFygaroTreasuryFloat } from "@services/fygaro/float-monitor" const TREASURY_ACCOUNT_ID = "bankowner-account" as AccountId @@ -95,6 +102,8 @@ beforeEach(() => { // currency, not by list order. mockListByAccountId.mockResolvedValue([usdWallet, usdtWallet]) mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("5000"))) + // Default: the cross-run Redis marker is claimable (NX SET succeeds). + mockRedisSet.mockResolvedValue("OK") }) describe("checkFygaroTreasuryFloat", () => { @@ -136,6 +145,33 @@ describe("checkFygaroTreasuryFloat", () => { }) }) + it("suppresses the float-low alert across runs when the Redis marker is already set", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("1500"))) + // NX SET returns null when the key already exists (a prior run alerted + // within the window) — the one-shot cron must not re-page. + mockRedisSet.mockResolvedValue(null) + + await checkFygaroTreasuryFloat() + + expect(mockRedisSet).toHaveBeenCalledWith( + "fygaro:float-low:alerted", + "1", + "EX", + 3600, + "NX", + ) + expect(mockAlertBridge).not.toHaveBeenCalled() + }) + + it("alerts anyway when the Redis dedup marker is unavailable (fail-open, never silent)", async () => { + mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("1500"))) + mockRedisSet.mockRejectedValue(new Error("redis down")) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + }) + it("alerts (warning) when the balance is below the floor", async () => { mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usdt("1500"))) From 2c4c708e3282d033db6b8a08919bed8208cb07bf Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 22:40:21 -0700 Subject: [PATCH 5/7] style(fygaro): reword mis-parse -> misread (spell-check) The typos linter flags "mis-parse"/"mis-parsed" (wants miss/mist). No behavior change; comment wording only. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NcEjF6SS3Ci5D4CzZqdPjb --- src/services/fygaro/float-monitor.ts | 2 +- test/flash/unit/services/fygaro/float-monitor.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts index 915758c7c..a57699f23 100644 --- a/src/services/fygaro/float-monitor.ts +++ b/src/services/fygaro/float-monitor.ts @@ -71,7 +71,7 @@ export const checkFygaroTreasuryFloat = async (): Promise => { // account is provably the funding source. In flash's IBEX-custodial model // each walletId is its own IBEX account, so reading the bankowner account's // default wallet would read a DIFFERENT account's balance (typically the USD - // wallet) and mis-parse it as the USDT float: a drained USDT float would + // wallet) and misread it as the USDT float: a drained USDT float would // hide behind a funded USD wallet (no page ever fires) and a low USD wallet // would false-alarm as "USDT float low". const funding = await resolveFygaroTreasuryFundingWallet() diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts index 1e7f674ad..19cff2227 100644 --- a/test/flash/unit/services/fygaro/float-monitor.spec.ts +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -123,7 +123,7 @@ describe("checkFygaroTreasuryFloat", () => { it("falls back to the legacy USD wallet when the treasury has no USDT wallet", async () => { mockListByAccountId.mockResolvedValue([usdWallet]) // A funded USD wallet above the floor must NOT alert (and must be read in - // USD, not mis-parsed as a zero USDT balance). + // USD, not misread as a zero USDT balance). mockGetAccountDetails.mockResolvedValue(detailsWithBalance(usd("5000"))) await checkFygaroTreasuryFloat() From 686e5fb49bc833fff4afedb6ad7f0865f32f0dc6 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 22:55:15 -0700 Subject: [PATCH 6/7] fix(fygaro): surface drained-treasury 404 and clock-skew 401s as alerts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent-failure gaps the ops-hardening work was meant to close: - float-monitor read IBEX via Ibex.getAccountDetails directly, so a drained/never-funded treasury account (IBEX answers 404) came back as an IbexError, hit the `instanceof Error` bail, and returned WITHOUT paging — the exact empty-float condition the monitor exists to catch. Read through the shared getBalanceForWallet helper instead, which maps 404 -> ZERO (and absent balance -> ZERO), so a dry treasury trips the low-float alert. - verify-signature 401'd every real webhook on a systematic clock skew (server drift / NTP down) but only warn-logged it. Now it pages via a distinct static dedup key (fygaroClockSkew) so a stuck clock surfaces on its own and replayed old webhooks still collapse to one warning per window — without masking, or being masked by, the wrong-secret alert. Tests: 404-drain trips low-float with balance_usd 0; the read-failure case now models a real (non-404) IbexError; skew rejection asserts a page under its own dedup key. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014QwZjNcKfkMhrBE33HcUVN --- src/services/alerts/dedup-key.ts | 5 ++ src/services/fygaro/float-monitor.ts | 36 ++++++++----- .../middleware/verify-signature.ts | 52 ++++++++++++++----- .../services/fygaro/float-monitor.spec.ts | 43 ++++++++++++++- .../webhook-server/verify-signature.spec.ts | 20 ++++++- 5 files changed, 126 insertions(+), 30 deletions(-) diff --git a/src/services/alerts/dedup-key.ts b/src/services/alerts/dedup-key.ts index 47ce19fe2..f9a813eba 100644 --- a/src/services/alerts/dedup-key.ts +++ b/src/services/alerts/dedup-key.ts @@ -40,6 +40,11 @@ export const generateDedupKey = { // Static keys (no per-request suffix) so the built-in TTL dedup rate-limits // these to one alert per window rather than one per failing request/poll. fygaroSignatureFailure: () => "fygaro:signature-failure", + // Distinct from fygaroSignatureFailure so a stuck server clock (every real + // webhook 401ing on skew) surfaces as its own warning instead of being + // collapsed into — or masked by — the "wrong secret" alert. Still static so + // replayed old webhooks collapse to one warning per window. + fygaroClockSkew: () => "fygaro:clock-skew", fygaroFloatLow: () => "fygaro:float-low", fygaroFloatExhausted: () => "fygaro:float-exhausted", } diff --git a/src/services/fygaro/float-monitor.ts b/src/services/fygaro/float-monitor.ts index a57699f23..6480c1237 100644 --- a/src/services/fygaro/float-monitor.ts +++ b/src/services/fygaro/float-monitor.ts @@ -1,6 +1,6 @@ import { FygaroConfig } from "@config" import { USDAmount, USDTAmount } from "@domain/shared" -import Ibex from "@services/ibex/client" +import { getBalanceForWallet } from "@app/wallets/get-balance-for-wallet" import { alertBridge, generateDedupKey } from "@services/alerts" import { baseLogger } from "@services/logger" import { redis } from "@services/redis" @@ -87,25 +87,35 @@ export const checkFygaroTreasuryFloat = async (): Promise => { } const fundingWallet = funding.fundingWallet - const details = await Ibex.getAccountDetails(fundingWallet.id, fundingWallet.currency) + // Read through the SAME helper the rest of the app uses so this monitor + // inherits IBEX's documented drain semantics instead of re-deriving them. + // Crucially, IBEX returns HTTP 404 for a drained / never-funded account and + // getBalanceForWallet maps that 404 -> ZERO (get-balance-for-wallet.ts). The + // old direct Ibex.getAccountDetails read surfaced that 404 as an IbexError, + // hit the `instanceof Error` bail, and returned WITHOUT alerting — silently + // missing the very empty-float condition this monitor exists to catch. Via + // the helper both drain signals (404 and an absent `balance` field) collapse + // to ZERO, so a dry treasury correctly trips the floor. + const balance = await getBalanceForWallet({ + walletId: fundingWallet.id, + currency: fundingWallet.currency, + }) - if (details instanceof Error) { - // An IBEX read blip must never crash the cron, and must never be mistaken - // for a low balance. Log and bail; the next run re-reads. (Per the "or - // logs" option — a distinct alert here would fight the float-low dedup.) + if (balance instanceof Error) { + // A genuine read blip (a non-404 IBEX error, or an unexpected throw the + // helper wraps) must never crash the cron and must never be mistaken for a + // low balance. Log and bail; the next run re-reads. (A distinct alert here + // would fight the float-low dedup.) baseLogger.error( - { err: details }, + { err: balance }, "Fygaro float check: could not read bankowner treasury balance", ) return } - // IBEX omits `balance` for a drained / never-funded account (absent means - // zero — see get-balance-for-wallet.ts). A genuinely empty treasury reads - // as 0 here and correctly trips the floor. Read the balance in the funding - // wallet's own currency so the USD fallback is not scored as an empty USDT - // float (or vice versa). - const balance = details.balance + // Score in the funding wallet's own currency so the USD fallback is not + // read as an empty USDT float (or vice versa). A ZERO from either drain + // signal above lands here as 0 and correctly trips the floor. const balanceUsd = balance instanceof USDTAmount ? Number(balance.asNumber()) diff --git a/src/services/fygaro/webhook-server/middleware/verify-signature.ts b/src/services/fygaro/webhook-server/middleware/verify-signature.ts index 6e8e8d140..ef70e5b48 100644 --- a/src/services/fygaro/webhook-server/middleware/verify-signature.ts +++ b/src/services/fygaro/webhook-server/middleware/verify-signature.ts @@ -9,23 +9,35 @@ import { alertBridge, generateDedupKey } from "@services/alerts" type RawBodyRequest = express.Request & { rawBody?: string } /** - * Page ops when signature verification is failing for a reason that means our - * side is misconfigured — a rotated/wrong shared secret ("we hold a secret but - * the HMAC didn't match") or no secrets configured at all. Left silent, either - * one 401s every real payment while looking healthy: this exact gap caused - * hours of silent card-top-up failures during setup. The static dedup key lets - * the built-in TTL suppression collapse the flood to one alert per window - * rather than one per rejected request. Deliberately NOT fired for a plain - * missing/malformed/expired signature (random internet noise) — those never - * indicate a secret problem and would be pure alert spam. Never carries the - * secret itself — only the (public) key id. + * Page ops when a webhook rejection means OUR side is misconfigured — every one + * of these 401s legitimate payments while the service looks healthy, the exact + * gap that caused hours of silent card-top-up failures during setup. Three + * cases fire: + * - a rotated/wrong shared secret ("we hold a secret but the HMAC didn't match") + * - no secrets configured at all + * - the server clock drifted past the skew window (NTP down / stuck clock), + * which 401s every real webhook on timestamp tolerance + * Each uses a STATIC dedup key so the built-in TTL suppression collapses the + * flood to one alert per window rather than one per rejected request. Clock skew + * carries its OWN static key (generateDedupKey.fygaroClockSkew) so a stuck clock + * and a bad secret never mask one another. Deliberately NOT fired for a plain + * missing/malformed signature (random internet noise) — those never indicate a + * misconfiguration and would be pure alert spam. Never carries the secret + * itself — only the (public) key id. */ -const alertSignatureFailure = (reason: string, keyId?: string): void => { +const SIGNATURE_FAILURE_TITLE = + "Fygaro webhook signature verification failing — check the webhook secret" + +const alertSignatureFailure = ( + reason: string, + keyId?: string, + overrides?: { dedupKey?: string; title?: string }, +): void => { alertBridge({ - dedupKey: generateDedupKey.fygaroSignatureFailure(), + dedupKey: overrides?.dedupKey ?? generateDedupKey.fygaroSignatureFailure(), source: "fygaro-webhook", severity: "warning", - title: "Fygaro webhook signature verification failing — check the webhook secret", + title: overrides?.title ?? SIGNATURE_FAILURE_TITLE, detail: reason, context: { key_id: keyId }, }) @@ -112,6 +124,20 @@ export const verifyFygaroSignature = ( const skewMs = FygaroConfig.webhook?.timestampSkewMs ?? 300000 if (Math.abs(Date.now() - timestampMs) > skewMs) { baseLogger.warn({ timestamp }, "Fygaro webhook rejected: timestamp outside skew") + // A one-off stale/replayed webhook is noise, but a SYSTEMATIC skew — our + // clock drifted past the tolerance, or NTP is down — 401s every real + // payment while the service looks healthy, the same silent-misconfig class + // the secret alerts guard against. Fire with its own static dedup key so + // replayed old webhooks collapse to one warning per window and a stuck + // clock never masks (or is masked by) a bad-secret alert. + alertSignatureFailure( + "timestamp outside skew tolerance — check the server clock / NTP", + typeof keyId === "string" ? keyId : undefined, + { + dedupKey: generateDedupKey.fygaroClockSkew(), + title: "Fygaro webhook rejected: timestamp skew — check server clock/NTP", + }, + ) return res.status(401).json({ error: "Signature timestamp outside tolerance" }) } diff --git a/test/flash/unit/services/fygaro/float-monitor.spec.ts b/test/flash/unit/services/fygaro/float-monitor.spec.ts index 19cff2227..b63e4fa6f 100644 --- a/test/flash/unit/services/fygaro/float-monitor.spec.ts +++ b/test/flash/unit/services/fygaro/float-monitor.spec.ts @@ -1,4 +1,7 @@ +import { ApiError } from "ibex-client" + import { USDAmount, USDTAmount, WalletCurrency } from "@domain/shared" +import { IbexError } from "@services/ibex/errors" const mockFygaroConfig = { enabled: true, @@ -92,6 +95,22 @@ const detailsWithBalance = (balance: USDTAmount | USDAmount | undefined) => ({ balance, }) +// A drained / never-funded IBEX account answers getAccountDetails with an HTTP +// 404 IbexError — the codebase's documented empty-account drain signal +// (get-balance-for-wallet.ts maps 404 -> ZERO). The float monitor reads through +// that helper, so this MUST be scored as an empty float, not swallowed as a read +// blip. +const ibex404 = (): IbexError => + new IbexError( + new ApiError(Object.assign(new Error("account not found"), { status: 404 })), + ) + +// A genuine read blip: an IBEX error that is NOT a 404. getBalanceForWallet +// surfaces this as an error (never ZERO), so the monitor must bail without +// alerting. (Ibex.getAccountDetails returns IbexError on failure — never a bare +// Error — so this is what a real read failure looks like.) +const ibexReadBlip = (): IbexError => new IbexError(new Error("ibex unreachable")) + beforeEach(() => { jest.clearAllMocks() mockFygaroConfig.enabled = true @@ -216,13 +235,33 @@ describe("checkFygaroTreasuryFloat", () => { expect(mockAlertBridge).not.toHaveBeenCalled() }) - it("does not crash and does not alert when the IBEX read fails", async () => { - mockGetAccountDetails.mockResolvedValue(new Error("ibex unreachable")) + it("does not crash and does not alert when the IBEX read fails (non-404 blip)", async () => { + mockGetAccountDetails.mockResolvedValue(ibexReadBlip()) await expect(checkFygaroTreasuryFloat()).resolves.toBeUndefined() expect(mockAlertBridge).not.toHaveBeenCalled() }) + it("alerts with balance_usd 0 when IBEX 404s the drained treasury account", async () => { + // Regression: reading Ibex.getAccountDetails directly surfaced a 404 as an + // IbexError, hit the `instanceof Error` bail, and returned WITHOUT alerting — + // silently missing the empty-float condition. Routed through + // getBalanceForWallet the 404 collapses to ZERO and MUST trip the low-float + // alert. + mockGetAccountDetails.mockResolvedValue(ibex404()) + + await checkFygaroTreasuryFloat() + + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + const alert = mockAlertBridge.mock.calls[0][0] + expect(alert).toMatchObject({ + dedupKey: "fygaro:float-low", + source: "fygaro-webhook", + severity: "warning", + }) + expect(alert.context).toEqual({ balance_usd: 0, floor_usd: 2000 }) + }) + it("does not read IBEX or alert when the treasury account cannot be resolved", async () => { mockFindByRole.mockResolvedValue(new Error("no bankowner")) 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 index 743cab4b9..39e208883 100644 --- a/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/verify-signature.spec.ts @@ -27,6 +27,7 @@ jest.mock("@services/alerts", () => ({ alertBridge: (...args: unknown[]) => mockAlertBridge(...args), generateDedupKey: { fygaroSignatureFailure: () => "fygaro:signature-failure", + fygaroClockSkew: () => "fygaro:clock-skew", }, })) @@ -278,7 +279,12 @@ describe("verifyFygaroSignature", () => { expect(mockAlertBridge).not.toHaveBeenCalled() }) - it("does NOT alert on a timestamp outside the allowed skew", () => { + it("alerts on a timestamp outside skew under its OWN dedup key (stuck clock / NTP)", () => { + // A systematic skew 401s every real webhook while the service looks + // healthy — the same silent-misconfig class the secret alerts guard + // against. It must page, but under a DISTINCT static dedup key so replayed + // old webhooks collapse to one warning per window and it never masks (or is + // masked by) a bad-secret alert. const t = String(Math.floor(Date.now() / 1000) - 3600) const req = makeReq({ signature: `t=${t},v1=${sign(t, RAW_BODY, "secret-one")}`, @@ -287,7 +293,17 @@ describe("verifyFygaroSignature", () => { verifyFygaroSignature(req, makeRes(), jest.fn()) - expect(mockAlertBridge).not.toHaveBeenCalled() + expect(mockAlertBridge).toHaveBeenCalledTimes(1) + const alert = mockAlertBridge.mock.calls[0][0] + expect(alert).toMatchObject({ + dedupKey: "fygaro:clock-skew", + source: "fygaro-webhook", + severity: "warning", + }) + // Distinct from the signature-failure alerts so the two never collapse. + expect(alert.dedupKey).not.toBe("fygaro:signature-failure") + expect(alert.title).toMatch(/skew|clock/i) + expect(alert.context).toEqual({ key_id: "key1" }) }) }) }) From 75cb7a610fd9819a1c8da9e9043297b2e5ec5a99 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 10 Aug 2026 23:05:50 -0700 Subject: [PATCH 7/7] test(fygaro): pin dedupKey selection in credit-failure alerts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The float-exhausted and generic credit-failure tests asserted only severity+title, never the dedupKey. A regression of the branch that swaps the per-transaction key (fygaroCreditFailed) for the static key (fygaroFloatExhausted) would pass both title assertions while a real treasury outage floods on-call with one page per transaction — the exact flood this hardening exists to prevent. - Replace the constant `generateDedupKey` stub (Proxy returning "dedup" for every key) with the real, pure dedup-key generator so the branch actually produces distinct keys under test. - Exhausted test: assert dedupKey === "fygaro:float-exhausted". - Generic-failure test: assert the per-transaction "fygaro:credit-failed:" key is still used. Both sides of the ternary are now pinned; verified by mutating the source to always use the per-transaction key (float-exhausted test fails) with no false positive on the generic test. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014QwZjNcKfkMhrBE33HcUVN --- .../fygaro/webhook-server/payment.spec.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) 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 01103274d..f58691fcd 100644 --- a/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts +++ b/test/flash/unit/services/fygaro/webhook-server/payment.spec.ts @@ -38,7 +38,11 @@ jest.mock("@services/frappe/BridgeTransferRequestWriter", () => ({ jest.mock("@services/alerts", () => ({ alertBridge: (...args: unknown[]) => mockAlertBridge(...args), - generateDedupKey: new Proxy({}, { get: () => jest.fn(() => "dedup") }), + // Use the REAL dedup-key generator (a pure, side-effect-free module) rather + // than a constant stub. The static-vs-per-transaction key selection in the + // credit-failure branch is exactly what the dedupKey assertions below pin, so + // stubbing every key to one value would let a regressed ternary pass silently. + generateDedupKey: jest.requireActual("@services/alerts/dedup-key").generateDedupKey, })) jest.mock("@services/alerts/ops-events", () => ({ @@ -292,6 +296,11 @@ describe("fygaro paymentHandler", () => { expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() expect(mockAlertBridge).toHaveBeenCalledWith( expect.objectContaining({ + // A generic credit failure MUST keep the per-transaction dedup key so + // each stranded payment pages ops individually — pinned here so a + // regression to the static float-exhausted key (which would collapse + // distinct manual-credit failures into one page) fails the test. + dedupKey: `fygaro:credit-failed:${VALID_BODY.transactionId}`, severity: "critical", title: "Fygaro auto-credit failed — manual credit needed", }), @@ -314,6 +323,11 @@ describe("fygaro paymentHandler", () => { expect(mockCompleteFygaroTopup).not.toHaveBeenCalled() expect(mockAlertBridge).toHaveBeenCalledWith( expect.objectContaining({ + // The exhausted branch MUST swap the per-transaction key for the + // STATIC float-exhausted key so a treasury-outage run of failing + // credits collapses to ONE PagerDuty page instead of one-per-tx. + // Pinned so a regression back to the per-transaction key fails here. + dedupKey: "fygaro:float-exhausted", severity: "critical", title: "Fygaro treasury float EXHAUSTED — top up bankowner immediately", }),