fix(api): harden input and cron robustness (NaN params, stablecoin crons, LNURL)

[TaprootFreak]

What this is

A repo-wide robustness sweep closing three classes of pre-existing defects that all share the same shape: a malformed input or a transient failure produces an HTTP 500 (or a wrong value) where a clean 400 / a well-defined error is correct. Surfaced while reviewing #3954 / #3964 / #3978 / #3979 and a subsequent full repo pass.

Guiding principle throughout: fail-closed / fail-loud instead of silent fallbacks — a clear error beats a masked wrong value.

---

1. NaN → Postgres 500 on numeric parameters

Problem. Many endpoints took a numeric ID as @Param('id') id: string and coerced it with +id. For non-numeric input this yields NaN, which flows into an integer-column query (WHERE id = $1). Postgres then throws invalid input syntax for type integer: "NaN" → HTTP 500 + error log instead of a proper 400/404. Same class #3954/#3979 already addressed for the buy/sell/swap controllers — here consistently extended to the remaining genuinely numeric IDs.

Fix. ParseIntPipe on the genuinely numeric route params (and the required buyId query); optional numeric query params are validated with a Number.isInteger guard (no ParseIntPipe({ optional: true }), since that option does not exist in the pinned @nestjs/common@9.4.3 and would break the build).

Affected endpoints (user/account/custody-reachable, highest risk first):

Endpoint	File
GET /paymentLink/walletApp/:id (unauth)	wallet-app.controller.ts
PUT /realunit/buy/:id/confirm, sell/:id/confirm, sell/:id/unsigned-transactions, sell/:id/broadcast (USER)	realunit.controller.ts
GET /support/issue/:id/message/:messageId/file — messageId via pipe; the polymorphic numeric id branch in getIssueSearch is guarded with Number.isInteger (:id stays string for uid/quote prefixes)	support-issue.controller.ts, support-issue.service.ts
PUT /transaction/:id/target + ?buyId, GET /transaction/:id/refund, PUT /transaction/:id/refund (ACCOUNT)	transaction.controller.ts
PUT /recommendation/:id/confirm, :id/reject (ACCOUNT)	recommendation.controller.ts
PUT /bankAccount/:id (ACCOUNT)	bank-account.controller.ts
POST /custody/order/:id/confirm (CUSTODY)	custody.controller.ts
GET /transaction/detail/single (?id, ?order-id), PUT /paymentLink/assign (?linkId), GET /paymentLink/stickers (?ids) — query guards	transaction.controller.ts, payment-link.controller.ts

Deliberately NOT changed (string IDs — a numeric cast would break them): LNURL/Lightning routing IDs (lnurlp/lnurld/lnurlw), custody-account/:id (accepts the legacy sentinel), the polymorphic id-or-uid paths in transaction.controller, and externalLinkId / externalPaymentId / key / KYC code. These are intentionally string.

---

2. dEURO / juice / frankencoin — cron robustness

The stablecoin integrations run every 10 minutes (*_LOG_INFO cron) and feed the public GET /deuro/info, GET /juice/info and GET /frankencoin/info endpoints.

• TVL division by a missing price. getBridgePriceFallback() was missing an await, which made ?? 0 dead code and let the function return undefined; getBridgeTvl then computed mintedValue / undefined → NaN/Infinity in the TVL. Fix: skip and log the bridge position when the EUR token price is missing (mirroring the existing getTvlByCollaterals guard) — no silent 0/NaN. (dEURO only; juice's getBridgeTvl does not divide by a price.)
• getDEPS/getJuice/getFPS catch without return. On an RPC blip the catch block returned undefined; the cron then wrote a log without poolShares, and the next /info call read poolShares.marketCap → TypeError / HTTP 500 until the next successful cron. Fix: rethrow on error so no incomplete log is written — applied to all three sister integrations (dEURO, juice, frankencoin) for consistent error semantics.
• Asymmetric config guard. processLogInfo only checked graphUrl but also used apiUrl unguarded. Fix: both in the guard; the now-redundant silent 0-savings fallback in juice-client is removed.
• Unbounded pagination. getPositionV2s ran while (hasNextPage) with no cursor-progress or page limit — a faulty indexer could cause an infinite loop + growing array (OOM). Fix: seen cursor set + MAX_PAGES limit; reaching the cap is logged (visible truncation).

---

3. LNURL metadata — JSON injection

createLnurlMetadata built the LUD-06 metadata JSON via string interpolation from the merchant-controlled memo ([["text/plain", "${memo}"]]). A " or \ in the memo could break the JSON or inject entries — visible to the paying customer via GET /lnurlp/:id. Fix: JSON.stringify([['text/plain', memo]]) escapes correctly and always produces valid JSON; both callers use the same function, so the description_hash stays self-consistent.

---

Deliberately out of scope

• Admin/compliance/support-gated +id endpoints (e.g. kyc-admin, buy-crypto/buy-fiat process, asset, mros, support, user-data) share the same NaN→500 class but are only reachable by privileged internal roles — low risk, a separate follow-up sweep makes sense.
• bridgeToDeuro MaxUint256 allowance: reviewed and confirmed not a bug (it sits inside the if (allowance.lt(weiAmount)) guard) — left untouched.

Tests / gates

• npm run type-check ✅ · eslint (changed files) ✅ · prettier --check ✅ · affected jest specs ✅
• No entity/column change → no migration. No config/env change. API contracts unchanged (IDs were always numeric); the only observable change for malformed requests is 500 → 400. Conforming clients are unaffected.

---

Release Checklist

Pre-Release

• Check migrations — none (no entity/column change)
• Check for linter errors (in PR)
• Test basic user operations (on DFX services) — Login/logout, Buy/sell payment request, KYC page

Post-Release

• Test basic user operations
• Monitor application insights log

[github-actions]

❌ TypeScript: 1 errors

[Danswar]

Reviewed the diff against current develop (after #3954/#3979/#3978). The core is sound and the NaN→400 coverage genuinely holds for the user/account/custody tier — the remaining unguarded +id endpoints I checked (route.updateRoute, user.updateUserAdmin, bank-data) are all ADMIN/COMPLIANCE/SUPPORT-gated, matching the deferred bucket. A few points below, mostly inline.

No regression tests (PR-level). For a defensive change whose whole value is "these specific defects can't recur", nothing asserts the new behavior — that NaN now yields 400, that the cron rethrows instead of writing a partial log, that the LNURL memo is escaped. The only test delta is mechanically retyping one spec's id from '1' to 1. Nothing prevents a future refactor from reverting ParseIntPipe back to +id. A handful of targeted specs (one per defect class) would lock the fixes in.

Framing nit. The description says "no silent 0/NaN" / "a clear error beats a masked wrong value", but the continue in getBridgeTvl and the MAX_PAGES truncation both serve a silently-undercounted public TVL (warn-only). Better than a crash, but it's the same class of masked wrong value the PR says it eliminates — worth reconciling the wording with the behavior. Details inline.

[Danswar]

This continue skips the bridge position entirely, so getBridgeTvl returns a TVL that is silently undercounted (warn-only) when an EUR price is missing. That is a quieter wrong value than the NaN it replaces, but it is still a wrong value served to the public /deuro/info — at odds with the PR's stated "no masked wrong value" principle. Worth either making the whole TVL computation fail-closed here, or explicitly documenting that undercount-with-warn is the accepted behavior.

[Danswar]

MAX_PAGES = 100 is an unjustified magic number. The query is positionV2s(after: $after) with no explicit first:, so the real cap is 100 × <indexer default page size> — unknowable from this code. If the position count ever approaches it, getPositionV2s silently returns an incomplete list and the downstream public TVL/borrow totals are wrong, with only a logger.warn as signal. Suggestions: (a) log the actual page count on every run, not just on cap-hit, and/or (b) set an explicit page size and derive the cap from realistic data headroom. Same applies to juice-client.ts.

[Danswar]

Same MAX_PAGES concern as deuro-client.ts:77 — magic number plus silent truncation of a public-facing total.

[Danswar]

Rethrowing here aborts the whole processLogInfo cycle — collateralTvl, bridgeTvl, and positionV2s are already computed above and get discarded, and no log is written this cycle. The trade is correct (a stale-but-valid previous log beats a poisoned one that 500s every /info read), but note the blast radius: a transient RPC blip on the pool-shares call now costs the entire 10-minute sample, not just poolShares. Same pattern in deuro.service.ts:174 and frankencoin.service.ts:186. Fine to keep — just flagging it is a deliberate fail-bigger, worth a one-line code comment so the next reader does not "optimize" it back.

[Danswar]

Correct fix for the injection. One transient edge: this also changes the metadata string's whitespace (", " → ","), so description_hash (sha256 of this string) changes value. Within a single pay flow it stays self-consistent, since both the served metadata (payment-link.service.ts) and the invoice hash (lightning-client.ts) go through this function. But during a rolling deploy, an in-flight LNURL-pay where old-format metadata was already served and a new-format hash is then minted will mismatch and the wallet rejects the invoice. Low-probability and self-clearing, but worth being aware of when timing the deploy.

[Danswar]

Minor: ParseIntPipe's isNumeric is /^-?\d+$/, so negative ids pass the pipe (e.g. /walletApp/-5 → -5). Harmless here (yields a 404), but "validated as integer" is not "validated as a valid id" — if any of these routes ever assume a positive id, ParseIntPipe alone will not guard it.