Release: develop -> main

src/shared/utils/__tests__/util.spec.ts

@@ -95,3 +95,47 @@ describe('sanitizeLogValue', () => {
     expect(Util.sanitizeLogValue('a=b', 64)).toBe('a?b');
   });
 });
+
+describe('toDbId', () => {
+  // Regression: request params were coerced with `!isNaN(+x)` / `Number.isInteger(+x)`, both of which
+  // accept values Postgres rejects as an integer. Reaching SQL, they surfaced as 500s on endpoints
+  // anonymous callers can reach (GET /v1/paymentLink/payment, /v1/plp, /v1/paymentLink/recipient).
+  it.each(['Infinity', '-Infinity', '1.9', '1e+21', '1E5', '0x10', '0b11', '-1', 'abc', '', '  '])(
+    'rejects %j',
+    (value) => {
+      expect(Util.toDbId(value)).toBeUndefined();
+    },
+  );
+
+  it('rejects NaN, which the old isNaN(+x) coercion also caught', () => {
+    expect(Util.toDbId('NaN')).toBeUndefined();
+  });
+
+  it('rejects 0, since SERIAL ids start at 1', () => {
+    expect(Util.toDbId('0')).toBeUndefined();
+  });
+
+  it('rejects ids beyond the Postgres INTEGER range', () => {
+    expect(Util.toDbId('2147483647')).toBe(2147483647);
+    expect(Util.toDbId('2147483648')).toBeUndefined();
+    expect(Util.toDbId('9'.repeat(309))).toBeUndefined();
+  });
+
+  // Query params are not guaranteed to be strings: `?id=1&id=2` arrives as an array, which
+  // `RegExp.test` would coerce to a passing value.
+  it.each([[['12']], [12], [null], [undefined], [{}]])('rejects the non-string %p', (value) => {
+    expect(Util.toDbId(value)).toBeUndefined();
+  });
+
+  it('returns the parsed id for a plain positive integer', () => {
+    expect(Util.toDbId('1')).toBe(1);
+    expect(Util.toDbId('42')).toBe(42);
+    expect(Util.toDbId('007')).toBe(7);
+  });
+
+  // A query string decodes `+` to a space, so `?id=+42` arrives padded and resolved before this
+  // existed. Trimming lives here so every call site treats padding the same way.
+  it.each([' 42', '42 ', ' 42 ', '\t42\n'])('tolerates the surrounding whitespace in %j', (value) => {
+    expect(Util.toDbId(value)).toBe(42);
+  });
+});

src/shared/utils/util.ts

@@ -15,6 +15,9 @@ export type KeyType<T, U> = {
 
 type CryptoAlgorithm = 'md5' | 'sha256' | 'sha384' | 'sha512';
 
+/** Postgres INTEGER / SERIAL upper bound (positive ids only). */
+export const PG_INTEGER_MAX = 2_147_483_647;
+
 export enum AmountType {
   ASSET = 'Asset',
   FIAT = 'Fiat',
@@ -513,6 +516,32 @@ export class Util {
     return ILike(`%${search}%`);
   }
 
+  /**
+   * Parses a request-supplied value into a usable Postgres INTEGER/SERIAL id, or `undefined` if it
+   * is not one.
+   *
+   * Deliberately stricter than `+value` / `Number.isInteger(+value)`: both of those accept values JS
+   * coerces to a finite-looking number but Postgres rejects, which surfaces as a 500 rather than a
+   * 400. Notably `+'Infinity'` is `Infinity` and `Number.isInteger(+'1e+21')` is `true`. Surrounding
+   * whitespace is tolerated because a query string decodes `+` to a space, so `?id=+42` resolved
+   * before this existed.
+   *
+   * Returns the id rather than a boolean so callers cannot re-derive it with a laxer coercion, and
+   * takes `unknown` because query params are not guaranteed to be strings at runtime (`?id=1&id=2`
+   * arrives as an array, which `RegExp.test` would coerce to something that passes).
+   */
+  static toDbId(value: unknown): number | undefined {
+    if (typeof value !== 'string') return undefined;
+
+    const trimmed = value.trim();
+    if (!/^\d+$/.test(trimmed)) return undefined;
+
+    // The digit-only test above already rules out anything non-integral or negative, so the range is
+    // the only remaining constraint — a value above it is also beyond Number.MAX_SAFE_INTEGER.
+    const id = Number(trimmed);
+    return id >= 1 && id <= PG_INTEGER_MAX ? id : undefined;
+  }
+
   // --- MISC --- //
 
   static async readFileFromDisk(fileName: string): Promise<string> {

src/subdomains/core/custody/controllers/custody-account.controller.ts

@@ -18,6 +18,7 @@ import { JwtPayload } from 'src/shared/auth/jwt-payload.interface';
 import { RoleGuard } from 'src/shared/auth/role.guard';
 import { UserActiveGuard } from 'src/shared/auth/user-active.guard';
 import { UserRole } from 'src/shared/auth/user-role.enum';
+import { Util } from 'src/shared/utils/util';
 import { PdfDto } from 'src/subdomains/core/buy-crypto/routes/buy/dto/pdf.dto';
 import { CreateCustodyAccountAccessDto } from '../dto/input/create-custody-account-access.dto';
 import { CreateCustodyAccountDto } from '../dto/input/create-custody-account.dto';
@@ -30,12 +31,7 @@ import { CustodyOrderHistoryDto } from '../dto/output/custody-order-history.dto'
 import { CustodyAccessLevel } from '../enums/custody';
 import { CustodyAccountReadGuard, CustodyAccountWriteGuard } from '../guards/custody-account-access.guard';
 import { CustodyAccountDtoMapper } from '../mappers/custody-account-dto.mapper';
-import {
-  CustodyAccountId,
-  CustodyAccountService,
-  LegacyAccountId,
-  PG_INTEGER_MAX,
-} from '../services/custody-account.service';
+import { CustodyAccountId, CustodyAccountService, LegacyAccountId } from '../services/custody-account.service';
 import { CustodyOrderService } from '../services/custody-order.service';
 import { CustodyPdfService } from '../services/custody-pdf.service';
 import { CustodyService } from '../services/custody.service';
@@ -253,15 +249,11 @@ export class CustodyAccountController {
    * Rejects non-digits, Infinity (e.g. 309 nines), zero, and values outside column range → 400.
    */
   private parsePositiveIntParam(value: string, name: string): number {
-    if (!/^\d+$/.test(value)) {
-      throw new BadRequestException(`Invalid ${name}`);
-    }
-
-    const n = Number(value);
-    if (!Number.isSafeInteger(n) || n < 1 || n > PG_INTEGER_MAX) {
+    const id = Util.toDbId(value);
+    if (!id) {
       throw new BadRequestException(`Invalid ${name}`);
     }
 
-    return n;
+    return id;
   }
 }

src/subdomains/core/custody/guards/custody-account-access.guard.ts

@@ -1,11 +1,7 @@
 import { CanActivate, ExecutionContext, ForbiddenException, HttpException, Injectable } from '@nestjs/common';
+import { Util } from 'src/shared/utils/util';
 import { CustodyAccessLevel } from '../enums/custody';
-import {
-  CustodyAccountId,
-  CustodyAccountService,
-  LegacyAccountId,
-  PG_INTEGER_MAX,
-} from '../services/custody-account.service';
+import { CustodyAccountId, CustodyAccountService, LegacyAccountId } from '../services/custody-account.service';
 
 abstract class CustodyAccountAccessGuard implements CanActivate {
   protected abstract readonly requiredLevel: CustodyAccessLevel;
@@ -42,12 +38,8 @@ abstract class CustodyAccountAccessGuard implements CanActivate {
 
     // Same constraints as the controller: digits only, finite safe positive int in SERIAL range.
     // (Guards map validation failures to 403; the controller answers 400 on grant routes.)
-    if (!/^\d+$/.test(id)) {
-      throw new ForbiddenException('Invalid custody account ID');
-    }
-
-    const parsed = Number(id);
-    if (!Number.isSafeInteger(parsed) || parsed < 1 || parsed > PG_INTEGER_MAX) {
+    const parsed = Util.toDbId(id);
+    if (!parsed) {
       throw new ForbiddenException('Invalid custody account ID');
     }
 

src/subdomains/core/custody/services/custody-account.service.ts

@@ -6,6 +6,7 @@ import {
   NotFoundException,
 } from '@nestjs/common';
 import { UserRole } from 'src/shared/auth/user-role.enum';
+import { PG_INTEGER_MAX } from 'src/shared/utils/util';
 import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity';
 import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service';
 import { EntityManager } from 'typeorm';
@@ -21,9 +22,6 @@ import { CustodyService } from './custody.service';
 export const LegacyAccountId = 'legacy';
 export type CustodyAccountId = number | typeof LegacyAccountId;
 
-/** Postgres INTEGER / SERIAL upper bound (positive ids only). */
-export const PG_INTEGER_MAX = 2_147_483_647;
-
 /**
  * Owner-scoped advisory lock key for ordinary creation vs legacy materialisation.
  * Must stay identical everywhere — a second key scheme would re-open races.

src/subdomains/core/history/__tests__/transaction-helper.spec.ts

@@ -428,4 +428,49 @@ describe('TransactionHelper', () => {
     expect(buyService.getBankInfoForRequest).not.toHaveBeenCalled();
     expect(buyService.getBankInfo).not.toHaveBeenCalled();
   });
+
+  // Regression: resolved as a single join this relation tree selects 1664 columns (61 joined nodes —
+  // Asset appears 10x and Country 9x, dragged in by eager relations), so one more @Column anywhere in
+  // it trips Postgres' "target lists can have at most 1664 entries" and 500s every invoice/receipt.
+  // Loading relations as separate queries keeps the statement path off that cliff for good.
+  it('loads statement relations as separate queries instead of one join', async () => {
+    const userData = createCustomUserData({
+      accountType: AccountType.PERSONAL,
+      firstname: 'Test',
+      surname: 'User',
+    });
+    const buyCrypto = createCustomBuyCrypto({
+      checkoutTx: createDefaultCheckoutTx(),
+      inputAmount: 125,
+      isComplete: true,
+      outputAmount: 0.005,
+    });
+    const transaction = createCustomTransaction({
+      buyCrypto,
+      sourceType: TransactionSourceType.CHECKOUT_TX,
+      userData,
+    });
+    const uid = 'T0123456789ABCDEF';
+    jest.spyOn(transactionService, 'getTransactionById').mockResolvedValue(transaction);
+    jest.spyOn(transactionService, 'getTransactionByUid').mockResolvedValue(transaction);
+    jest.spyOn(fiatService, 'getFiatByName').mockResolvedValue(createCustomFiat({ name: 'EUR' }));
+
+    await txHelper.getTxStatementDetails(userData.id, transaction.id, TxStatementType.INVOICE);
+    await txHelper.getTxStatementDetails(userData.id, uid, TxStatementType.INVOICE);
+
+    // The tree is pinned, not just the strategy: pruning is only safe because organization,
+    // buyFiat.outputAsset and cryptoInput.asset are eager, so a branch silently reappearing here
+    // (or an eager flag being dropped elsewhere) should fail loudly rather than cost round-trips.
+    const expectedRelations = {
+      userData: true,
+      buyCrypto: { buy: { user: { wallet: true } }, cryptoInput: true },
+      buyFiat: { cryptoInput: true },
+      refReward: true,
+      bankTxReturn: true,
+      request: true,
+    };
+
+    expect(transactionService.getTransactionById).toHaveBeenCalledWith(transaction.id, expectedRelations, 'query');
+    expect(transactionService.getTransactionByUid).toHaveBeenCalledWith(uid, expectedRelations, 'query');
+  });
 });

src/subdomains/core/history/__tests__/transaction.controller.spec.ts

@@ -1,4 +1,5 @@
 import { createMock } from '@golevelup/ts-jest';
+import { BadRequestException, NotFoundException } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
 import { Config } from 'src/config/config';
 import { JwtPayload } from 'src/shared/auth/jwt-payload.interface';
@@ -21,6 +22,10 @@ import { createCustomTransactionRequest } from 'src/subdomains/supporting/paymen
 import { createCustomTransaction } from 'src/subdomains/supporting/payment/__mocks__/transaction.entity.mock';
 import { VirtualIbanService } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.service';
 import { FiatPaymentMethod } from 'src/subdomains/supporting/payment/dto/payment-method.enum';
+import {
+  TxStatementDetails,
+  TxStatementType,
+} from 'src/subdomains/supporting/payment/dto/transaction-helper/tx-statement-details.dto';
 import { SwissQRService } from 'src/subdomains/supporting/payment/services/swiss-qr.service';
 import { TransactionHelper } from 'src/subdomains/supporting/payment/services/transaction-helper';
 import { TransactionRequestService } from 'src/subdomains/supporting/payment/services/transaction-request.service';
@@ -204,4 +209,78 @@ describe('TransactionController', () => {
       expect(transactionHelper.getTxStatementDetails).not.toHaveBeenCalled();
     });
   });
+
+  describe('getSingleTransactionDetails (malformed id / order-id)', () => {
+    // Both guards reject rather than drop, because id and order-id are the primary selectors here
+    // rather than one of several optional lookup keys.
+    it.each(['NaN', 'Infinity', '1.9', '1e+21', '-1', '0', '2147483648', 'abc'])(
+      'rejects id=%j before it reaches the lookup',
+      async (id) => {
+        await expect(controller.getSingleTransactionDetails(jwt, id)).rejects.toBeInstanceOf(BadRequestException);
+
+        expect(transactionService.getTransactionById).not.toHaveBeenCalled();
+      },
+    );
+
+    it.each(['NaN', 'Infinity', '1e+21', '2147483648', 'abc'])(
+      'rejects order-id=%j before it reaches the lookup',
+      async (orderId) => {
+        await expect(controller.getSingleTransactionDetails(jwt, undefined, undefined, orderId)).rejects.toBeInstanceOf(
+          BadRequestException,
+        );
+
+        expect(transactionService.getTransactionByRequestId).not.toHaveBeenCalled();
+      },
+    );
+
+    it('passes a well-formed id through as a number', async () => {
+      jest.spyOn(transactionService, 'getTransactionById').mockResolvedValue(undefined);
+
+      await expect(controller.getSingleTransactionDetails(jwt, ' 42 ')).rejects.toBeInstanceOf(NotFoundException);
+
+      expect(transactionService.getTransactionById).toHaveBeenCalledWith(42, expect.any(Object));
+    });
+  });
+
+  describe('generateInvoiceFromTransaction (malformed id)', () => {
+    // Regression: `isNaN(+id)` classified all of these as numeric ids, so they reached Postgres as
+    // integers and came back as `invalid input syntax for type integer` -> 500. None of them can be a
+    // real transaction id, so they must go down the UID path instead and never be coerced to a number.
+    const malformedIds = ['NaN', 'Infinity', '1.9', '1e+21', '-1', '99999999999', '0x10', 'abc'];
+
+    beforeEach(() => {
+      Config.invoice.currencies = ['EUR', 'CHF'];
+      jest.spyOn(transactionRequestService, 'getTransactionRequestByUid').mockResolvedValue(undefined);
+      // Resolves rather than rejects on purpose: a rejecting mock would make the assertions below
+      // pass regardless of how the id was classified, which is the bug this pins.
+      jest
+        .spyOn(transactionHelper, 'getTxStatementDetails')
+        .mockResolvedValue({ currency: 'EUR' } as unknown as TxStatementDetails);
+      jest.spyOn(swissQrService, 'createTxStatement').mockResolvedValue('pdf-data');
+    });
+
+    it.each(malformedIds)('passes %j through as a UID, never as a numeric id', async (id) => {
+      await controller.generateInvoiceFromTransaction(jwt, id);
+
+      expect(transactionHelper.getTxStatementDetails).toHaveBeenCalledWith(jwt.account, id, TxStatementType.INVOICE);
+    });
+
+    // A query string decodes `+` to a space, so `/v1/transaction/+42/invoice` arrives padded; it
+    // resolved as id 42 before the guard existed and still must.
+    it.each(['42', ' 42 '])('still resolves the well-formed numeric id %j to a number', async (id) => {
+      await controller.generateInvoiceFromTransaction(jwt, id);
+
+      expect(transactionHelper.getTxStatementDetails).toHaveBeenCalledWith(jwt.account, 42, TxStatementType.INVOICE);
+    });
+
+    it('guards the receipt endpoint the same way', async () => {
+      await controller.generateReceiptFromTransaction(jwt, 'Infinity');
+
+      expect(transactionHelper.getTxStatementDetails).toHaveBeenCalledWith(
+        jwt.account,
+        'Infinity',
+        TxStatementType.RECEIPT,
+      );
+    });
+  });
 });

src/subdomains/core/history/controllers/transaction.controller.ts

@@ -505,7 +505,7 @@ export class TransactionController {
   @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), IpGuard, UserActiveGuard())
   @ApiOkResponse({ type: PdfDto })
   async generateInvoiceFromTransaction(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise<PdfDto> {
-    const txIdOrUid = isNaN(+id) ? id : +id;
+    const txIdOrUid = Util.toDbId(id) ?? id;
 
     // For string UIDs, first try to find a TransactionRequest (for pending transactions)
     if (typeof txIdOrUid === 'string') {
@@ -572,7 +572,7 @@ export class TransactionController {
   @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), IpGuard, UserActiveGuard())
   @ApiOkResponse({ type: PdfDto })
   async generateReceiptFromTransaction(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise<PdfDto> {
-    const txIdOrUid = isNaN(+id) ? id : +id;
+    const txIdOrUid = Util.toDbId(id) ?? id;
     const txStatementDetails = await this.transactionHelper.getTxStatementDetails(
       jwt.account,
       txIdOrUid,
@@ -801,8 +801,8 @@ export class TransactionController {
 
     let tx: Transaction | TransactionRequest;
     if (id) {
-      const transactionId = +id;
-      if (!Number.isInteger(transactionId)) throw new BadRequestException('id must be an integer');
+      const transactionId = Util.toDbId(id);
+      if (!transactionId) throw new BadRequestException('id must be an integer');
       tx = await this.transactionService.getTransactionById(transactionId, baseRelations);
     }
 
@@ -815,8 +815,8 @@ export class TransactionController {
     }
 
     if (orderId) {
-      const requestId = +orderId;
-      if (!Number.isInteger(requestId)) throw new BadRequestException('order-id must be an integer');
+      const requestId = Util.toDbId(orderId);
+      if (!requestId) throw new BadRequestException('order-id must be an integer');
       tx =
         (await this.transactionService.getTransactionByRequestId(requestId, baseRelations)) ??
         (await this.transactionRequestService.getTransactionRequest(requestId, { user: { userData: true } }));