From ceda706ca019fb41d6b12d3d32fddbe70080ddee Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:39:37 +0200 Subject: [PATCH 1/3] fix(realunit): bound the tax-residence payload and stop writing user_data.tin on a rejected request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the tax-residence contract: - user_data.tin is varchar(256), but neither CountryAndTin.tin nor countryAndTINs was length-bounded and the client allows an unbounded number of tax residences. Seven entries with an 11-digit TIN already serialize to 260 characters, so the UPDATE threw 'value too long for type character varying(256)' — AFTER forwardRegistration had already persisted the registration, turning a completed registration into a 500 that every retry reproduced. Bound the input (10 entries, 64-character TIN), widen the column to varchar(1024) to cover that bounded maximum, and reject an oversized payload up front in validateRegistrationDto so it can never fail once the registration is durable. - The idempotent branch of completeRegistration wrote user_data.tin BEFORE idempotentRegistrationResult rejects a mismatching signature, so a request answered with a 400 had already mutated the column. countryAndTINs is not part of the EIP-712 envelope, so the body cannot describe what was actually registered either. Check the signature first and sync the snapshot from the durable registration row instead of the request body. - @ValidateIf(!swissTaxResidence) disabled every validator on countryAndTINs — including @IsArray — whenever swissTaxResidence was true, so a non-array body reached the service and crashed it with a TypeError (500 instead of 400). Validate whenever the field is required or present, and keep a defensive Array.isArray guard in the service. Also replace the unguarded JSON.parse(user_data.tin) in the registration prefill: a malformed legacy value took down the only endpoint through which the user could submit a corrected declaration. It now degrades loudly (PII-free error log) to 'no known tax residences'. --- migration/1784000000000-WidenUserDataTin.js | 41 ++++ .../user/models/user-data/user-data.entity.ts | 3 +- .../realunit-registration.dto.spec.ts | 42 ++++ .../__tests__/realunit.service.spec.ts | 201 +++++++++++++++++- .../realunit/dto/realunit-registration.dto.ts | 25 ++- .../supporting/realunit/realunit.service.ts | 90 +++++++- 6 files changed, 389 insertions(+), 13 deletions(-) create mode 100644 migration/1784000000000-WidenUserDataTin.js diff --git a/migration/1784000000000-WidenUserDataTin.js b/migration/1784000000000-WidenUserDataTin.js new file mode 100644 index 0000000000..193973707a --- /dev/null +++ b/migration/1784000000000-WidenUserDataTin.js @@ -0,0 +1,41 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Widens `user_data.tin` from varchar(256) to varchar(1024). The RealUnit multi-tax-residence + * declaration is stored as a JSON array of {country, tin} in this column; 256 characters is no + * longer enough from ~7 entries on (seven 11-digit TINs already serialize past 256). Widening a + * varchar length in Postgres is a catalog-only change with no table rewrite. + * + * down() fails loud if longer values already exist — that is intentional (fail-closed: do not + * silently truncate tax-residence snapshots). + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class WidenUserDataTin1784000000000 { + name = 'WidenUserDataTin1784000000000' + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + // Fail fast instead of head-of-queue-blocking every "user_data" write if the ALTER is + // contended at deploy time: the timeout aborts with a clear lock_timeout error instead of + // hanging app boot. + await queryRunner.query(`SET LOCAL lock_timeout = '5s'`); + await queryRunner.query(`ALTER TABLE "user_data" ALTER COLUMN "tin" TYPE character varying(1024)`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + // Fails loud if any row already stores more than 256 characters — intentional; do not + // silently truncate a multi-tax-residence snapshot. + await queryRunner.query(`SET LOCAL lock_timeout = '5s'`); + await queryRunner.query(`ALTER TABLE "user_data" ALTER COLUMN "tin" TYPE character varying(256)`); + } +} diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index 0e1971db80..ca17e47d1f 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -114,7 +114,8 @@ export class UserData extends IEntity { @Column({ type: 'timestamp', nullable: true }) birthday?: Date; - @Column({ length: 256, nullable: true }) + // RealUnit stores a JSON list of tax residences here (not a single TIN); 1024 covers the DTO-bounded max. + @Column({ length: 1024, nullable: true }) tin?: string; // --- ORGANIZATION DATA --- // diff --git a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts index 8321f43b5b..0d0a86d88e 100644 --- a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts +++ b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts @@ -71,6 +71,48 @@ describe('RealUnitRegistrationDto (class-validator decorators)', () => { expect(errors.find((e) => e.property === 'countryAndTINs')).toBeUndefined(); }); + // When swissTaxResidence is true BUT countryAndTINs is present, validators must still run + // (previously @ValidateIf skipped @IsArray and a non-array body reached the service as a 500). + it('rejects a non-array countryAndTINs when swissTaxResidence is true (F3 regression)', async () => { + const errors = await validate(build({ swissTaxResidence: true, countryAndTINs: 'pwned' })); + const tinError = errors.find((e) => e.property === 'countryAndTINs'); + expect(tinError).toBeDefined(); + expect(tinError!.constraints).toHaveProperty('isArray'); + }); + + it('rejects a TIN longer than 64 characters via @MaxLength', async () => { + const errors = await validate( + build({ swissTaxResidence: true, countryAndTINs: [{ country: 'DE', tin: 'x'.repeat(65) }] }), + ); + const tinError = errors.find((e) => e.property === 'countryAndTINs'); + expect(tinError).toBeDefined(); + expect(tinError!.children?.length).toBeGreaterThan(0); + const nestedTinError = tinError!.children![0].children?.find((e) => e.property === 'tin'); + expect(nestedTinError?.constraints).toHaveProperty('maxLength'); + }); + + it('rejects more than 10 countryAndTINs entries via @ArrayMaxSize', async () => { + // Valid 2-letter codes so only ArrayMaxSize (not nested Matches) is the failure mode. + const countries = ['DE', 'FR', 'US', 'AT', 'IT', 'ES', 'NL', 'BE', 'PT', 'IE', 'PL']; + const countryAndTINs = countries.map((country, i) => ({ country, tin: `TIN${i}` })); + const errors = await validate(build({ swissTaxResidence: false, countryAndTINs })); + const tinError = errors.find((e) => e.property === 'countryAndTINs'); + expect(tinError).toBeDefined(); + expect(tinError!.constraints).toHaveProperty('arrayMaxSize'); + }); + + it('trims TIN whitespace via @Transform(Util.trim)', () => { + const dto = build({ swissTaxResidence: false, countryAndTINs: [{ country: 'DE', tin: ' DE123 ' }] }); + expect(dto.countryAndTINs![0].tin).toBe('DE123'); + }); + + it('accepts exactly 10 countryAndTINs entries (ArrayMaxSize boundary)', async () => { + const countries = ['DE', 'FR', 'US', 'AT', 'IT', 'ES', 'NL', 'BE', 'PT', 'IE']; + const countryAndTINs = countries.map((country, i) => ({ country, tin: `TIN${i}` })); + const errors = await validate(build({ swissTaxResidence: false, countryAndTINs })); + expect(errors.find((e) => e.property === 'countryAndTINs')).toBeUndefined(); + }); + // --- kycData nested factory (@Type(() => KycPersonalData)) --- // it('instantiates kycData through the KycPersonalData type factory', () => { diff --git a/src/subdomains/supporting/realunit/__tests__/realunit.service.spec.ts b/src/subdomains/supporting/realunit/__tests__/realunit.service.spec.ts index 09ed4c2808..db7ad7f339 100644 --- a/src/subdomains/supporting/realunit/__tests__/realunit.service.spec.ts +++ b/src/subdomains/supporting/realunit/__tests__/realunit.service.spec.ts @@ -2216,10 +2216,26 @@ describe('RealUnitService', () => { it('returns the idempotent result for an existing current-wallet registration', async () => { userService.getUserByAddress.mockResolvedValue({ - userData: { id: 1, kycLevel: KycLevel.LEVEL_10, mail: 'max@example.com' }, + userData: { id: 1, kycLevel: KycLevel.LEVEL_10, mail: 'max@example.com', tin: null }, } as any); + // After the F2 fix the isForCurrentWallet branch reconstructs the registration via + // toRegistrationDto (reads signedPayloadData) before syncing user_data.tin from the record. jest.spyOn(service as any, 'findRegistration').mockResolvedValue({ - registration: { id: 3, signature: '0xsig', status: ReviewStatus.COMPLETED }, + registration: { + id: 3, + signature: '0xsig', + status: ReviewStatus.COMPLETED, + signedPayloadData: { + email: 'max@example.com', + name: 'Max', + walletAddress: '0xabc', + signature: '0xsig', + registrationDate: '2026-01-01', + swissTaxResidence: true, + addressCountry: 'CH', + }, + kycDataObj: { accountType: 'Personal' }, + }, isForCurrentWallet: true, }); @@ -2305,6 +2321,111 @@ describe('RealUnitService', () => { expect(status).toBe(RealUnitRegistrationStatus.FORWARDING_FAILED); }); + + it('F1: does not call forwardRegistration when countryAndTINs is too large', async () => { + // Exercise the real tax-residence guard (serialized-length fail-closed) without the full EIP-712 path. + jest.spyOn(service as any, 'validateRegistrationDto').mockImplementation(async (d: any) => { + (service as any).validateTaxResidenceCoversAddress(d); + }); + jest.spyOn(service as any, 'serializeCountryAndTins').mockReturnValue('x'.repeat(1025)); + const forwardSpy = jest.spyOn(service as any, 'forwardRegistration').mockResolvedValue(true); + + const oversizedDto: any = { + walletAddress: '0xabc', + signature: '0xsig', + email: 'max@example.com', + addressCountry: 'DE', + swissTaxResidence: false, + countryAndTINs: [{ country: 'DE', tin: 'DE123456789' }], + kycData: {}, + }; + userService.getUserByAddress.mockResolvedValue({ + userData: { id: 1, kycLevel: KycLevel.LEVEL_10, mail: 'max@example.com', firstname: 'Max' }, + } as any); + + await expect(service.completeRegistration(1, oversizedDto)).rejects.toThrow(/countryAndTINs is too large/); + expect(forwardSpy).not.toHaveBeenCalled(); + }); + + it('F2: does not write user_data.tin when the idempotent path rejects a signature mismatch', async () => { + userService.getUserByAddress.mockResolvedValue({ + userData: { id: 1, kycLevel: KycLevel.LEVEL_10, mail: 'max@example.com', tin: null }, + } as any); + jest.spyOn(service as any, 'findRegistration').mockResolvedValue({ + registration: { + id: 3, + signature: '0xSTORED_SIGNATURE', + status: ReviewStatus.COMPLETED, + signedPayloadData: { + email: 'max@example.com', + name: 'Max', + walletAddress: '0xabc', + signature: '0xSTORED_SIGNATURE', + registrationDate: '2026-01-01', + swissTaxResidence: true, + addressCountry: 'CH', + countryAndTINs: [{ country: 'DE', tin: 'DE-STORED' }], + }, + kycDataObj: { accountType: 'Personal' }, + }, + isForCurrentWallet: true, + }); + // Incoming signature does NOT match the stored one; body carries a different TIN set. + const mismatchDto: any = { + ...dto, + signature: '0xMISMATCHING_SIGNATURE', + countryAndTINs: [{ country: 'FR', tin: 'FR-ATTACKER' }], + }; + + await expect(service.completeRegistration(1, mismatchDto)).rejects.toThrow(BadRequestException); + + const tinWrites = (userDataService.updateUserDataInternal as jest.Mock).mock.calls + .map((c) => c[1]) + .filter((u) => u && Object.prototype.hasOwnProperty.call(u, 'tin')); + expect(tinWrites).toHaveLength(0); + expect(logService.create).not.toHaveBeenCalledWith(expect.objectContaining({ subsystem: 'UserDataTin' })); + }); + + it('F2: syncs user_data.tin from the stored registration, not the request body, on idempotent retry', async () => { + const storedTins = [{ country: 'DE', tin: 'DE-STORED' }]; + const bodyTins = [{ country: 'FR', tin: 'FR-FROM-BODY' }]; + const userData: any = { + id: 1, + kycLevel: KycLevel.LEVEL_10, + mail: 'max@example.com', + tin: null, + }; + userService.getUserByAddress.mockResolvedValue({ userData } as any); + jest.spyOn(service as any, 'findRegistration').mockResolvedValue({ + registration: { + id: 3, + signature: '0xsig', + status: ReviewStatus.COMPLETED, + signedPayloadData: { + email: 'max@example.com', + name: 'Max', + walletAddress: '0xabc', + signature: '0xsig', + registrationDate: '2026-01-01', + swissTaxResidence: true, + addressCountry: 'CH', + countryAndTINs: storedTins, + }, + kycDataObj: { accountType: 'Personal' }, + }, + isForCurrentWallet: true, + }); + logService.create.mockResolvedValue({} as any); + + const status = await service.completeRegistration(1, { ...dto, countryAndTINs: bodyTins }); + + expect(status).toBe(RealUnitRegistrationStatus.ALREADY_REGISTERED); + const tinWrites = (userDataService.updateUserDataInternal as jest.Mock).mock.calls + .map((c) => c[1]) + .filter((u) => u && Object.prototype.hasOwnProperty.call(u, 'tin')); + expect(tinWrites).toEqual([{ tin: JSON.stringify(storedTins) }]); + expect(tinWrites[0].tin).not.toContain('FR-FROM-BODY'); + }); }); describe('ponder queries (GraphQL injection protection)', () => { @@ -3205,6 +3326,55 @@ describe('RealUnitService', () => { ); }); + // --- F1/F3 defense-in-depth bounds on countryAndTINs (fail closed before forward) --- + + it('F1: rejects countryAndTINs whose serialized length exceeds MAX_SERIALIZED_TIN_LENGTH', async () => { + // Under the DTO bounds the worst-case serialization is ~890 chars; the serialized-length + // check is a fail-closed safety net for constant drift. Force it by stubbing serialize. + const dto = attachTins( + await buildDto(humanFields({ addressCountry: 'DE', swissTaxResidence: false }), humanKyc()), + [{ country: 'DE', tin: 'DE123456789' }], + ); + jest.spyOn(service as any, 'serializeCountryAndTins').mockReturnValue('x'.repeat(1025)); + await expect((service as any).validateRegistrationDto(dto)).rejects.toThrow(/countryAndTINs is too large/); + }); + + it('F1: rejects more than 10 countryAndTINs entries at the service layer', async () => { + const countries = ['DE', 'FR', 'US', 'AT', 'IT', 'ES', 'NL', 'BE', 'PT', 'IE', 'PL']; + const dto = attachTins( + await buildDto(humanFields({ addressCountry: 'DE', swissTaxResidence: false }), humanKyc()), + countries.map((country, i) => ({ country, tin: `TIN${i}` })), + ); + await expect((service as any).validateRegistrationDto(dto)).rejects.toThrow(/more than 10 entries/); + }); + + it('F1: rejects a TIN longer than 64 characters at the service layer', async () => { + const dto = attachTins( + await buildDto(humanFields({ addressCountry: 'DE', swissTaxResidence: false }), humanKyc()), + [{ country: 'DE', tin: 'x'.repeat(65) }], + ); + await expect((service as any).validateRegistrationDto(dto)).rejects.toThrow(/must not exceed 64 characters/); + }); + + it('F3: rejects a non-array countryAndTINs with BadRequestException (not TypeError)', () => { + for (const countryAndTINs of ['pwned', { a: 1 }, 42] as any[]) { + expect(() => + (service as any).validateTaxResidenceCoversAddress({ + swissTaxResidence: true, + addressCountry: 'CH', + countryAndTINs, + }), + ).toThrow(BadRequestException); + expect(() => + (service as any).validateTaxResidenceCoversAddress({ + swissTaxResidence: true, + addressCountry: 'CH', + countryAndTINs, + }), + ).toThrow(/countryAndTINs must be an array/); + } + }); + it('resolveSignedRegistrationMessage normalizes a signature that lacks the 0x prefix', async () => { const fields = humanFields(); const signature = await wallet._signTypedData(domain, types, fields); @@ -3612,6 +3782,33 @@ describe('RealUnitService', () => { country: { symbol: 'CH', id: 7 }, }); }); + + // F4: malformed / contract-violating user_data.tin must degrade the prefill loudly, never throw. + it('F4: degrades prefill to empty countryAndTINs when tin is not valid JSON', () => { + const userData: any = { id: 99, firstname: 'Erika', tin: 'not json' }; + const dto = (service as any).toUserDataDtoFromUserData(userData); + expect(dto).toBeDefined(); + expect(dto.countryAndTINs).toBeUndefined(); + expect((service as any).logger.error).toHaveBeenCalledWith(expect.stringContaining('not valid JSON')); + }); + + it('F4: degrades prefill to empty countryAndTINs when tin is a non-array JSON value', () => { + const userData: any = { id: 99, firstname: 'Erika', tin: '{"a":1}' }; + const dto = (service as any).toUserDataDtoFromUserData(userData); + expect(dto).toBeDefined(); + expect(dto.countryAndTINs).toBeUndefined(); + expect((service as any).logger.error).toHaveBeenCalledWith(expect.stringContaining('not an array')); + }); + + it('F4: drops a contract-violating CH entry from the prefill without throwing', () => { + const userData: any = { id: 99, firstname: 'Erika', tin: '[{"country":"CH","tin":"x"}]' }; + const dto = (service as any).toUserDataDtoFromUserData(userData); + expect(dto).toBeDefined(); + expect(dto.countryAndTINs).toBeUndefined(); + expect((service as any).logger.error).toHaveBeenCalledWith( + expect.stringContaining('violating the registration contract'), + ); + }); }); describe('isPersonalDataMatching (organization account branch)', () => { diff --git a/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts b/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts index 4076b596d7..2c5c187230 100644 --- a/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts +++ b/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts @@ -1,6 +1,7 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { Transform, Type } from 'class-transformer'; import { + ArrayMaxSize, IsArray, IsBoolean, IsEmail, @@ -18,6 +19,14 @@ import { Util } from 'src/shared/utils/util'; import { KycPersonalData } from 'src/subdomains/generic/kyc/dto/input/kyc-data.dto'; import { DfxPhoneTransform, IsDfxPhone } from 'src/subdomains/generic/user/models/user-data/is-dfx-phone.validator'; +// Bounds for the `user_data.tin` snapshot, which stores countryAndTINs as a JSON array of +// {country, tin}. The column is varchar(1024); the bounded worst case (10 entries x 64-char TIN) +// serializes to ~890 characters and therefore always fits. Keeping the three limits together makes +// the DB invariant explicit instead of implicit in the JSON envelope arithmetic. +export const MAX_TAX_RESIDENCES = 10; +export const MAX_TIN_LENGTH = 64; +export const MAX_SERIALIZED_TIN_LENGTH = 1024; + export enum RealUnitUserType { HUMAN = 'HUMAN', CORPORATION = 'CORPORATION', @@ -84,6 +93,8 @@ export class CountryAndTin { @ApiProperty({ description: 'Tax identification number' }) @IsNotEmpty() @IsString() + @MaxLength(MAX_TIN_LENGTH) + @Transform(Util.trim) tin: string; } @@ -191,15 +202,19 @@ export class AktionariatRegistrationDto { description: 'Tax residences with TINs for non-CH countries only (never CH — use swissTaxResidence). ' + 'Required when swissTaxResidence is false. Multi-residence TINs may also be sent when ' + - 'swissTaxResidence is true; nested shape is enforced by the service. addressCountry must be ' + + 'swissTaxResidence is true (max 10 entries, each TIN max 64 chars). addressCountry must be ' + 'covered: CH via swissTaxResidence, any other country via an entry here.', }) - // Presence required only for non-Swiss. Nested shape for multi-residence with - // swissTaxResidence=true is enforced in RealUnitService.validateTaxResidenceCoversAddress - // (class-validator ValidateIf cannot both require-when-false and nested-check-when-present cleanly). - @ValidateIf((o: AktionariatRegistrationDto) => !o.swissTaxResidence) + // Validate whenever the field is REQUIRED (no Swiss tax residence declared) OR whenever it is + // present at all. The previous condition (`!o.swissTaxResidence` alone) skipped EVERY validator — + // including @IsArray — as soon as swissTaxResidence was true, so a non-array body slipped through + // the pipe and crashed the service with a TypeError (HTTP 500 instead of 400). + @ValidateIf((o: AktionariatRegistrationDto) => !o.swissTaxResidence || o.countryAndTINs !== undefined) @IsNotEmpty({ message: 'countryAndTINs is required when swissTaxResidence is false' }) @IsArray() + @ArrayMaxSize(MAX_TAX_RESIDENCES, { + message: `countryAndTINs must not contain more than ${MAX_TAX_RESIDENCES} entries`, + }) @ValidateNested({ each: true }) @Type(() => CountryAndTin) countryAndTINs?: CountryAndTin[]; diff --git a/src/subdomains/supporting/realunit/realunit.service.ts b/src/subdomains/supporting/realunit/realunit.service.ts index ac293c8719..a989c1b195 100644 --- a/src/subdomains/supporting/realunit/realunit.service.ts +++ b/src/subdomains/supporting/realunit/realunit.service.ts @@ -90,6 +90,9 @@ import { import { RealUnitDtoMapper } from './dto/realunit-dto.mapper'; import { AktionariatRegistrationDto, + MAX_SERIALIZED_TIN_LENGTH, + MAX_TAX_RESIDENCES, + MAX_TIN_LENGTH, RealUnitEmailRegistrationDto, RealUnitEmailRegistrationStatus, RealUnitLanguage, @@ -745,10 +748,19 @@ export class RealUnitService { dto.walletAddress, ); if (isForCurrentWallet) { - // Registration row is already durable — still sync user_data.tin (e.g. retry after a - // previous attempt that forwarded successfully but failed the tin audit write). - await this.persistUserDataTinAfterRegistration(userData, dto); - return this.idempotentRegistrationResult(userData, existingRegistration!, dto.signature); + // Signature check FIRST: idempotentRegistrationResult rejects a mismatching signature with a 400, + // and a request that is about to be rejected must not have mutated the database on its way out. + const status = await this.idempotentRegistrationResult(userData, existingRegistration!, dto.signature); + + // The snapshot follows the event of record, never the request body. countryAndTINs is NOT part of + // the EIP-712 envelope, so a replay could otherwise rewrite user_data.tin to something that was + // never registered with Aktionariat. Syncing from the durable registration row also self-heals a + // retry whose forward succeeded but whose tin audit write failed. + const registrationData = this.toRegistrationDto(existingRegistration!); + if (!registrationData) throw new BadRequestException('Invalid registration data'); + await this.persistUserDataTinAfterRegistration(userData, registrationData); + + return status; } // validate personal data @@ -899,6 +911,17 @@ export class RealUnitService { private validateTaxResidenceCoversAddress(dto: RealUnitRegistrationDto): void { const entries = dto.countryAndTINs ?? []; + // Defense in depth: the DTO enforces this too, but the service must never index into a non-array. + // A non-array body used to reach here (the DTO's @ValidateIf skipped @IsArray whenever + // swissTaxResidence was true) and crashed with a TypeError => HTTP 500 instead of a 400. + if (!Array.isArray(entries)) { + throw new BadRequestException('countryAndTINs must be an array'); + } + + if (entries.length > MAX_TAX_RESIDENCES) { + throw new BadRequestException(`countryAndTINs must not contain more than ${MAX_TAX_RESIDENCES} entries`); + } + if (entries.some((e) => e.country === 'CH')) { throw new BadRequestException( 'countryAndTINs must not include CH; set swissTaxResidence for Swiss tax residence', @@ -914,6 +937,9 @@ export class RealUnitService { if (typeof entry.tin !== 'string' || !entry.tin.trim()) { throw new BadRequestException('countryAndTINs.tin must be a non-empty string'); } + if (entry.tin.length > MAX_TIN_LENGTH) { + throw new BadRequestException(`countryAndTINs.tin must not exceed ${MAX_TIN_LENGTH} characters`); + } } const tinCountries = entries.map((e) => e.country); @@ -931,6 +957,14 @@ export class RealUnitService { if (!taxCountries.has(dto.addressCountry)) { throw new BadRequestException(`Tax residence must include the residence country (${dto.addressCountry})`); } + + // Fail closed BEFORE anything is forwarded. user_data.tin is a bounded column and is written only + // AFTER forwardRegistration has persisted the registration — an oversized payload must therefore be + // rejected here with a 400, never blow up as a 500 on a registration that is already durable. + const serialized = this.serializeCountryAndTins(entries); + if (serialized && serialized.length > MAX_SERIALIZED_TIN_LENGTH) { + throw new BadRequestException('countryAndTINs is too large'); + } } // Canonical shape for user_data.tin: JSON array of {country, tin}, or null when the @@ -1254,6 +1288,52 @@ export class RealUnitService { return userDataDto as RealUnitUserDataDto; } + // user_data.tin is written exclusively by persistUserDataTinAfterRegistration, as a JSON array of + // {country, tin} with no CH entry. A malformed or contract-violating legacy value must not take this + // prefill down: GET registration-info is the only channel through which the user can submit a + // CORRECTED declaration, so throwing here would lock the account out of the very fix it needs. The + // bad value is therefore dropped LOUDLY (error log, no PII) and the prefill degrades to "no known tax + // residences" — it never hands the client back a payload that validateTaxResidenceCoversAddress would + // reject. + private parseStoredTin(userData: UserData): { country: string; tin: string }[] { + if (!userData.tin) return []; + + let parsed: unknown; + try { + parsed = JSON.parse(userData.tin); + } catch { + this.logger.error( + `Malformed user_data.tin for userData ${userData.id}: not valid JSON — prefill degraded to empty`, + ); + return []; + } + + if (!Array.isArray(parsed)) { + this.logger.error( + `Malformed user_data.tin for userData ${userData.id}: not an array — prefill degraded to empty`, + ); + return []; + } + + const entries = parsed.filter( + (e): e is { country: string; tin: string } => + !!e && + typeof e.country === 'string' && + /^[A-Z]{2}$/.test(e.country) && + e.country !== 'CH' && + typeof e.tin === 'string' && + !!e.tin.trim(), + ); + + if (entries.length !== parsed.length) { + this.logger.error( + `user_data.tin for userData ${userData.id} contains ${parsed.length - entries.length} entr(ies) violating the registration contract — dropped from the prefill`, + ); + } + + return entries; + } + // Pre-fill source for first-time RealUnit registrations: maps the user's existing DFX KYC data into // the Aktionariat-shaped DTO. The corresponding `completeRegistration` validation // (`isPersonalDataMatching`) compares the submitted KycPersonalData/address against the same @@ -1265,7 +1345,7 @@ export class RealUnitService { const lang = Object.values(RealUnitLanguage).find((l) => l === userData.language?.symbol?.toUpperCase()); const addressStreet = [userData.street, userData.houseNumber].filter((s) => s).join(' '); - const tinEntries: { country: string; tin: string }[] = userData.tin ? JSON.parse(userData.tin) : []; + const tinEntries = this.parseStoredTin(userData); return { email: userData.mail ?? '', From aabb42244f025be9c0c739d85d5ee3a66d4fcad2 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 15 Jul 2026 08:10:18 +0200 Subject: [PATCH 2/3] fix(realunit): trim tin without crashing on a non-string value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @Transform(Util.trim) added to CountryAndTin.tin runs in class-transformer before class-validator, so a non-string tin (e.g. a number) made value.trim() throw a TypeError => HTTP 500 before @IsString could reject it as a 400 — the exact 500-instead-of-400 regression this change set exists to remove. Trim only actual strings and let any other type fall through to @IsString. --- .../__tests__/realunit-registration.dto.spec.ts | 15 ++++++++++++++- .../realunit/dto/realunit-registration.dto.ts | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts index 0d0a86d88e..d12695a6c7 100644 --- a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts +++ b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts @@ -101,11 +101,24 @@ describe('RealUnitRegistrationDto (class-validator decorators)', () => { expect(tinError!.constraints).toHaveProperty('arrayMaxSize'); }); - it('trims TIN whitespace via @Transform(Util.trim)', () => { + it('trims TIN whitespace via string-safe @Transform', () => { const dto = build({ swissTaxResidence: false, countryAndTINs: [{ country: 'DE', tin: ' DE123 ' }] }); expect(dto.countryAndTINs![0].tin).toBe('DE123'); }); + // Non-string tin must not crash @Transform (Util.trim would throw TypeError => HTTP 500); + // the value must fall through so @IsString can reject it as a clean 400. + it('rejects a non-string tin as isString without throwing (400 not 500 regression)', async () => { + const errors = await validate( + build({ swissTaxResidence: true, countryAndTINs: [{ country: 'DE', tin: 12345 }] }), + ); + const tinError = errors.find((e) => e.property === 'countryAndTINs'); + expect(tinError).toBeDefined(); + expect(tinError!.children?.length).toBeGreaterThan(0); + const nestedTinError = tinError!.children![0].children?.find((e) => e.property === 'tin'); + expect(nestedTinError?.constraints).toHaveProperty('isString'); + }); + it('accepts exactly 10 countryAndTINs entries (ArrayMaxSize boundary)', async () => { const countries = ['DE', 'FR', 'US', 'AT', 'IT', 'ES', 'NL', 'BE', 'PT', 'IE']; const countryAndTINs = countries.map((country, i) => ({ country, tin: `TIN${i}` })); diff --git a/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts b/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts index 2c5c187230..29417a7ec0 100644 --- a/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts +++ b/src/subdomains/supporting/realunit/dto/realunit-registration.dto.ts @@ -94,7 +94,10 @@ export class CountryAndTin { @IsNotEmpty() @IsString() @MaxLength(MAX_TIN_LENGTH) - @Transform(Util.trim) + // NOT Util.trim: class-transformer runs this before class-validator, so a non-string tin (e.g. a + // number) would make `value.trim()` throw a TypeError => HTTP 500 before @IsString can reject it as + // a 400. Trim only actual strings and let any other type fall through to @IsString. + @Transform(({ value }) => (typeof value === 'string' ? value.trim() : value)) tin: string; } From 7f778c43488a4ce8a3e9ec90bf1515bc32285719 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 15 Jul 2026 08:13:36 +0200 Subject: [PATCH 3/3] style(realunit): apply prettier to the non-string-tin regression test --- .../realunit/__tests__/realunit-registration.dto.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts index d12695a6c7..f99e9a8fd2 100644 --- a/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts +++ b/src/subdomains/supporting/realunit/__tests__/realunit-registration.dto.spec.ts @@ -109,9 +109,7 @@ describe('RealUnitRegistrationDto (class-validator decorators)', () => { // Non-string tin must not crash @Transform (Util.trim would throw TypeError => HTTP 500); // the value must fall through so @IsString can reject it as a clean 400. it('rejects a non-string tin as isString without throwing (400 not 500 regression)', async () => { - const errors = await validate( - build({ swissTaxResidence: true, countryAndTINs: [{ country: 'DE', tin: 12345 }] }), - ); + const errors = await validate(build({ swissTaxResidence: true, countryAndTINs: [{ country: 'DE', tin: 12345 }] })); const tinError = errors.find((e) => e.property === 'countryAndTINs'); expect(tinError).toBeDefined(); expect(tinError!.children?.length).toBeGreaterThan(0);