From 32cef4c4fa2f3c4c1e928df8a89e298525823d78 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:29:52 +0200 Subject: [PATCH 1/2] feat(custody): account-scoped read endpoints for a shared Safe (#4406) * feat(custody): account-scoped read endpoints for a shared Safe * fix(custody): refuse an account-scoped read when holdings span several accounts Reading resolves an account to its owner, because no per-account attribution of balances and orders exists. If the owner holds more than one active account, that would hand a grantee everything the owner holds, including what belongs to the accounts they were not granted. There is no source of truth to filter by, so the read refuses instead of returning either a fabricated subset or an over-broad Safe. The owner keeps the aggregate view through the caller-scoped endpoints. * fix(custody): apply the multi-account refusal only to grantees, and count every account The refusal exists because a grant covers one account while the data layer can only return the owner's whole Safe. It counted active accounts only, so an owner with one active and one closed account passed the check and a grantee received the closed account's holdings as well - closing an account moves nothing. It now counts every account of that owner. It also refused the owner, who holds all of those rows anyway and reaches them through the caller-scoped endpoints, which broke the equivalence these routes are meant to have for the owner. The check now applies only when someone else asks. * docs(custody): describe what the account-scoped responses return The three new read routes declared their response type without a description, the only routes in this controller that did. Each one now says that the data belongs to the addressed account rather than the caller - the distinction that separates them from the caller-scoped endpoints. --- .../controllers/custody-account.controller.ts | 72 ++++++++++++++++++- .../services/custody-account.service.ts | 49 +++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/subdomains/core/custody/controllers/custody-account.controller.ts b/src/subdomains/core/custody/controllers/custody-account.controller.ts index 45cc87ff8d..a8af59d97a 100644 --- a/src/subdomains/core/custody/controllers/custody-account.controller.ts +++ b/src/subdomains/core/custody/controllers/custody-account.controller.ts @@ -8,6 +8,7 @@ import { Param, Post, Put, + Query, UseGuards, } from '@nestjs/common'; import { AuthGuard } from '@nestjs/passport'; @@ -17,11 +18,15 @@ 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 { 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'; +import { GetCustodyPdfDto } from '../dto/input/get-custody-pdf.dto'; import { UpdateCustodyAccountAccessDto } from '../dto/input/update-custody-account-access.dto'; import { UpdateCustodyAccountDto } from '../dto/input/update-custody-account.dto'; import { CustodyAccountAccessDto, CustodyAccountDto } from '../dto/output/custody-account.dto'; +import { CustodyBalanceDto, CustodyHistoryDto } from '../dto/output/custody-balance.dto'; +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'; @@ -31,11 +36,19 @@ import { LegacyAccountId, PG_INTEGER_MAX, } 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'; @ApiTags('Custody') @Controller('custody/account') export class CustodyAccountController { - constructor(private readonly custodyAccountService: CustodyAccountService) {} + constructor( + private readonly custodyAccountService: CustodyAccountService, + private readonly custodyService: CustodyService, + private readonly custodyOrderService: CustodyOrderService, + private readonly custodyPdfService: CustodyPdfService, + ) {} @Get() @ApiBearerAuth() @@ -97,6 +110,63 @@ export class CustodyAccountController { return CustodyAccountDtoMapper.toDto(custodyAccount, CustodyAccessLevel.WRITE); } + @Get(':id/balance') + @ApiBearerAuth() + @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard(), CustodyAccountReadGuard) + @ApiOkResponse({ type: CustodyBalanceDto, description: 'Custody balance of the addressed account' }) + async getAccountBalance(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise { + const ownerAccountId = await this.custodyAccountService.resolveOwnerAccountId( + this.parseCustodyAccountId(id), + jwt.account, + ); + + return this.custodyService.getUserCustodyBalance(ownerAccountId); + } + + @Get(':id/history') + @ApiBearerAuth() + @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard(), CustodyAccountReadGuard) + @ApiOkResponse({ type: CustodyHistoryDto, description: 'Custody history of the addressed account' }) + async getAccountHistory(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise { + const ownerAccountId = await this.custodyAccountService.resolveOwnerAccountId( + this.parseCustodyAccountId(id), + jwt.account, + ); + + return this.custodyService.getUserCustodyHistory(ownerAccountId); + } + + @Get(':id/order') + @ApiBearerAuth() + @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard(), CustodyAccountReadGuard) + @ApiOkResponse({ type: CustodyOrderHistoryDto, isArray: true, description: 'Order history of the addressed account' }) + async getAccountOrders(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise { + const ownerAccountId = await this.custodyAccountService.resolveOwnerAccountId( + this.parseCustodyAccountId(id), + jwt.account, + ); + + return this.custodyOrderService.getOrdersByUserData(ownerAccountId); + } + + @Get(':id/pdf') + @ApiBearerAuth() + @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard(), CustodyAccountReadGuard) + @ApiOkResponse({ type: PdfDto, description: 'Custody balance PDF report (base64 encoded)' }) + async getAccountPdf( + @GetJwt() jwt: JwtPayload, + @Param('id') id: string, + @Query() dto: GetCustodyPdfDto, + ): Promise { + const ownerAccountId = await this.custodyAccountService.resolveOwnerAccountId( + this.parseCustodyAccountId(id), + jwt.account, + ); + + const pdfData = await this.custodyPdfService.generateCustodyPdf(ownerAccountId, dto); + return { pdfData }; + } + @Get(':id/access') @ApiBearerAuth() @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) diff --git a/src/subdomains/core/custody/services/custody-account.service.ts b/src/subdomains/core/custody/services/custody-account.service.ts index e7087673cd..6cf3d8e102 100644 --- a/src/subdomains/core/custody/services/custody-account.service.ts +++ b/src/subdomains/core/custody/services/custody-account.service.ts @@ -145,6 +145,55 @@ export class CustodyAccountService { return { custodyAccount, isLegacy: false }; } + /** + * Resolves a custody account to its owner's user_data id for data reads. + * Today balances/orders never set accountId, so a read returns the owner's entire Safe. + * + * The multi-account refusal is only for grantees: a grant covers one account, but the + * data layer can only return the owner's whole Safe. If that owner holds more than one + * custody account (any status — closed/blocked still hold assets), serving the Safe would + * disclose holdings outside the grant, so refuse with 409 instead of a fabricated subset + * or an over-broad full Safe. The owner already authorises every one of those rows and + * reaches them via the caller-scoped endpoints; their own authorisation is total, so the + * ambiguity check is skipped when the caller is the owner. + * + * Once balances and orders carry an account, callers filter by it and this multi-account + * refusal is no longer needed. Legacy is unaffected (caller has no accounts). + */ + async resolveOwnerAccountId(custodyAccountId: CustodyAccountId, callerAccountId: number): Promise { + const { custodyAccount, isLegacy } = await this.checkAccess( + custodyAccountId, + callerAccountId, + CustodyAccessLevel.READ, + ); + + if (isLegacy) { + return callerAccountId; + } + + if (!custodyAccount) { + throw new NotFoundException('Custody account not found'); + } + + const ownerId = custodyAccount.owner.id; + + // Owner already holds every Safe row; multi-account ambiguity only matters for grantees. + if (ownerId === callerAccountId) { + return ownerId; + } + + const ownedCount = await this.custodyAccountRepo.count({ + where: { owner: { id: ownerId } }, + }); + if (ownedCount > 1) { + throw new ConflictException( + 'The holdings of this Safe are not attributed to a single account, so the account cannot be read in isolation', + ); + } + + return ownerId; + } + // --- CREATE --- // async createCustodyAccount(accountId: number, title: string, description?: string): Promise { const owner = await this.userDataService.getActiveUserData(accountId); From 16c4223109df0bc6c5b9b4c57f56cf1ef8ec62aa Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:35:36 +0200 Subject: [PATCH 2/2] fix(storage): stabilize content verification scan race (#4408) --- scripts/storage/verify-content.ts | 154 +++++++++++++++++- .../storage/__tests__/verify-content.spec.ts | 97 +++++++++++ 2 files changed, 247 insertions(+), 4 deletions(-) diff --git a/scripts/storage/verify-content.ts b/scripts/storage/verify-content.ts index 3128071419..f7a4a30488 100644 --- a/scripts/storage/verify-content.ts +++ b/scripts/storage/verify-content.ts @@ -73,7 +73,13 @@ * VERIFY_IGNORE_BUCKETS (non-document / system buckets) — unaccounted buckets fail hard. */ -import { GetObjectCommand, ListBucketsCommand, ListObjectsV2Command, S3Client } from '@aws-sdk/client-s3'; +import { + GetObjectCommand, + HeadObjectCommand, + ListBucketsCommand, + ListObjectsV2Command, + S3Client, +} from '@aws-sdk/client-s3'; import { BlobServiceClient, ContainerClient } from '@azure/storage-blob'; import * as crypto from 'crypto'; import * as dotenv from 'dotenv'; @@ -640,6 +646,11 @@ export interface ContainerClassification { s3Map: Map; } +export interface StabilizedContentClassification { + report: ContainerClassification; + resolvedConcurrentMissingKeys: number; +} + export function classifyContainer( container: string, azureObjs: ContentObject[], @@ -701,6 +712,113 @@ export function classifyContainer( }; } +export function isS3NotFound(err: unknown): boolean { + const e = err as { $metadata?: { httpStatusCode?: number }; name?: string }; + return e?.$metadata?.httpStatusCode === 404 || e?.name === 'NoSuchKey' || e?.name === 'NotFound'; +} + +export function isAzureNotFound(err: unknown): boolean { + const e = err as { statusCode?: number; details?: { errorCode?: string } }; + return e?.statusCode === 404 || e?.details?.errorCode === 'BlobNotFound'; +} + +/** + * Close the non-atomic listing race without pausing uploads. Only keys classified as missing + * are re-checked on the allegedly absent target, then the complete container classification is + * rebuilt with the target metadata. No content is downloaded and no object is written or + * deleted. A large one-sided-empty inventory still fails before any per-key target request; + * only a new container's bounded first-object 0↔1 race is allowed through once. + */ +export async function stabilizeMissingKeys( + report: ContainerClassification, + azureContainer: ContainerClient, + s3: S3Client, + backfillCutoff: Date, + backfillContentProven: boolean, +): Promise { + const oneSideEmpty = (report.azureMap.size === 0) !== (report.s3Map.size === 0); + if (oneSideEmpty && Math.max(report.azureMap.size, report.s3Map.size) > 1) { + throw new Error( + `One-sided empty content inventory for container "${report.container}": ` + + `azureCount=${report.azureMap.size}, s3Count=${report.s3Map.size}. ` + + `Refusing target re-check fan-out.`, + ); + } + + let resolvedConcurrentMissingKeys = 0; + + for (const key of report.missingKeys) { + const azureObj = report.azureMap.get(key); + const s3Obj = report.s3Map.get(key); + if ((azureObj == null) === (s3Obj == null)) { + throw new Error( + `Invalid missing-key classification for ${safeObjectReference(report.container, key)}: ` + + `exactly one source object is required`, + ); + } + + if (azureObj) { + try { + const target = await s3.send(new HeadObjectCommand({ Bucket: report.container, Key: key })); + if (target.ContentLength == null || target.LastModified == null || target.ETag == null || target.ETag === '') { + throw new Error( + `Incomplete S3 HEAD response for ${safeObjectReference(report.container, key)}: ` + + `ContentLength, LastModified and ETag are required`, + ); + } + report.s3Map.set(key, { + key, + size: target.ContentLength, + lastModified: target.LastModified, + etag: target.ETag, + }); + resolvedConcurrentMissingKeys++; + } catch (err) { + if (!isS3NotFound(err)) { + throw new Error(`S3 missing-key re-check failed for ${safeObjectReference(report.container, key)}`, { + cause: err, + }); + } + } + } else { + try { + const target = await azureContainer.getBlockBlobClient(key).getProperties(); + if (target.contentLength == null || target.lastModified == null || target.etag == null || target.etag === '') { + throw new Error( + `Incomplete Azure properties response for ${safeObjectReference(report.container, key)}: ` + + `contentLength, lastModified and etag are required`, + ); + } + report.azureMap.set(key, { + key, + size: target.contentLength, + lastModified: target.lastModified, + etag: target.etag, + ...(target.contentMD5 != null ? { contentMd5: Buffer.from(target.contentMD5).toString('base64') } : {}), + }); + resolvedConcurrentMissingKeys++; + } catch (err) { + if (!isAzureNotFound(err)) { + throw new Error(`Azure missing-key re-check failed for ${safeObjectReference(report.container, key)}`, { + cause: err, + }); + } + } + } + } + + return { + report: classifyContainer( + report.container, + [...report.azureMap.values()], + [...report.s3Map.values()], + backfillCutoff, + backfillContentProven, + ), + resolvedConcurrentMissingKeys, + }; +} + function printCategory(label: string, keys: string[]): void { console.log(` ${label}: ${keys.length}`); } @@ -759,10 +877,18 @@ async function main(): Promise { const azureContainer = azure.getContainerClient(container); const azureObjs = await listAzure(azureContainer); const s3Objs = await listS3(s3, container); - const report = classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven); + const initialReport = classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven); + const { report, resolvedConcurrentMissingKeys } = await stabilizeMissingKeys( + initialReport, + azureContainer, + s3, + backfillCutoff, + backfillContentProven, + ); reports.push(report); console.log(`\n[${container}] azure=${azureObjs.length} s3=${s3Objs.length}`); + console.log(` missingKeyRecheckResolved: ${resolvedConcurrentMissingKeys}`); printCategory('metadata-match', report.metadataMatch); printCategory('backfill-covered', report.backfillCovered); printCategory('size-mismatch', report.sizeMismatch); @@ -930,7 +1056,18 @@ async function main(): Promise { const azureContainer = azure.getContainerClient(container); const azureObjs = await listAzure(azureContainer); const s3Objs = await listS3(s3, container); - const report = classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven); + const initialReport = classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven); + const { report, resolvedConcurrentMissingKeys } = await stabilizeMissingKeys( + initialReport, + azureContainer, + s3, + backfillCutoff, + backfillContentProven, + ); + console.log( + `Fixpoint iteration ${iteration} [${container}] ` + + `missingKeyRecheckResolved=${resolvedConcurrentMissingKeys}`, + ); // Replace report in place for intermediate logging / final path consistency. const idx = reports.findIndex((r) => r.container === container); @@ -1001,7 +1138,16 @@ async function main(): Promise { const azureContainer = azure.getContainerClient(container); const azureObjs = await listAzure(azureContainer); const s3Objs = await listS3(s3, container); - finalReports.push(classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven)); + const initialReport = classifyContainer(container, azureObjs, s3Objs, backfillCutoff, backfillContentProven); + const { report, resolvedConcurrentMissingKeys } = await stabilizeMissingKeys( + initialReport, + azureContainer, + s3, + backfillCutoff, + backfillContentProven, + ); + console.log(`[final ${container}] missingKeyRecheckResolved=${resolvedConcurrentMissingKeys}`); + finalReports.push(report); } catch (e) { throw new Error(`[container="${container}"] final re-list failed: ${e?.message ?? e}`, { cause: e }); } diff --git a/src/integration/infrastructure/storage/__tests__/verify-content.spec.ts b/src/integration/infrastructure/storage/__tests__/verify-content.spec.ts index 6bd104c737..3e66136bf6 100644 --- a/src/integration/infrastructure/storage/__tests__/verify-content.spec.ts +++ b/src/integration/infrastructure/storage/__tests__/verify-content.spec.ts @@ -1,3 +1,5 @@ +import { HeadObjectCommand, S3Client } from '@aws-sdk/client-s3'; +import { mockClient } from 'aws-sdk-client-mock'; import { assertHashedSize, assertHashVersionUnchanged, @@ -15,8 +17,18 @@ import { objectSignature, parseConfig, safeObjectReference, + stabilizeMissingKeys, } from '../../../../../scripts/storage/verify-content'; +const s3Mock = mockClient(S3Client); + +function makeS3Client(): S3Client { + return new S3Client({ + region: 'us-east-1', + credentials: { accessKeyId: 'x', secretAccessKey: 'x' }, + }); +} + function contentObject( key: string, size: number, @@ -200,6 +212,91 @@ describe('classifyContainer', () => { }); }); +describe('stabilizeMissingKeys', () => { + beforeEach(() => { + s3Mock.reset(); + }); + + it('reclassifies an Azure-only key that appeared concurrently on S3', async () => { + const azure = contentObject('k', 0, afterCutoff, { contentMd5: EMPTY_MD5_BASE64, etag: AZURE_ETAG }); + const initial = classifyContainer('support', [azure], [], cutoff, false); + s3Mock.on(HeadObjectCommand, { Bucket: 'support', Key: 'k' }).resolves({ + ContentLength: 0, + LastModified: afterCutoff, + ETag: S3_ETAG, + }); + + const result = await stabilizeMissingKeys(initial, {} as never, makeS3Client(), cutoff, false); + + expect(result.resolvedConcurrentMissingKeys).toBe(1); + expect(result.report.missingKeys).toEqual([]); + expect(result.report.metadataMatch).toEqual(['k']); + expect(result.report.s3Map.get('k')).toEqual(contentObject('k', 0, afterCutoff, { etag: S3_ETAG })); + }); + + it('reclassifies an S3-only key that appeared concurrently on Azure', async () => { + const s3 = contentObject('k', 0, afterCutoff, { etag: S3_ETAG }); + const initial = classifyContainer('support', [], [s3], cutoff, false); + const getProperties = jest.fn().mockResolvedValue({ + contentLength: 0, + lastModified: afterCutoff, + etag: AZURE_ETAG, + contentMD5: Buffer.from(EMPTY_MD5_BASE64, 'base64'), + }); + const azureContainer = { getBlockBlobClient: jest.fn().mockReturnValue({ getProperties }) } as never; + + const result = await stabilizeMissingKeys(initial, azureContainer, makeS3Client(), cutoff, false); + + expect(result.resolvedConcurrentMissingKeys).toBe(1); + expect(result.report.missingKeys).toEqual([]); + expect(result.report.metadataMatch).toEqual(['k']); + expect(result.report.azureMap.get('k')).toEqual( + contentObject('k', 0, afterCutoff, { contentMd5: EMPTY_MD5_BASE64, etag: AZURE_ETAG }), + ); + }); + + it('keeps a genuinely absent target missing and promotes a different-size target', async () => { + const absent = contentObject('absent', 1, afterCutoff, { contentMd5: EMPTY_MD5_BASE64, etag: AZURE_ETAG }); + const mismatch = contentObject('mismatch', 2, afterCutoff, { + contentMd5: EMPTY_MD5_BASE64, + etag: AZURE_ETAG, + }); + const sharedAzure = contentObject('shared', 0, afterCutoff, { + contentMd5: EMPTY_MD5_BASE64, + etag: AZURE_ETAG, + }); + const sharedS3 = contentObject('shared', 0, afterCutoff, { etag: S3_ETAG }); + const initial = classifyContainer('support', [absent, mismatch, sharedAzure], [sharedS3], cutoff, false); + s3Mock + .on(HeadObjectCommand, { Bucket: 'support', Key: 'absent' }) + .rejects(Object.assign(new Error('not found'), { name: 'NotFound' })); + s3Mock.on(HeadObjectCommand, { Bucket: 'support', Key: 'mismatch' }).resolves({ + ContentLength: 3, + LastModified: afterCutoff, + ETag: S3_ETAG, + }); + + const result = await stabilizeMissingKeys(initial, {} as never, makeS3Client(), cutoff, false); + + expect(result.resolvedConcurrentMissingKeys).toBe(1); + expect(result.report.missingKeys).toEqual(['absent']); + expect(result.report.sizeMismatch).toEqual(['mismatch']); + }); + + it('rejects large one-sided-empty inventories before any target request', async () => { + const azure = [ + contentObject('a', 1, afterCutoff, { etag: AZURE_ETAG }), + contentObject('b', 1, afterCutoff, { etag: AZURE_ETAG }), + ]; + const initial = classifyContainer('kyc', azure, [], cutoff, false); + + await expect(stabilizeMissingKeys(initial, {} as never, makeS3Client(), cutoff, false)).rejects.toThrow( + /One-sided empty content inventory/, + ); + expect(s3Mock.commandCalls(HeadObjectCommand)).toHaveLength(0); + }); +}); + describe('assertWithinHashCap', () => { it('throws when count exceeds cap (message mentions both values)', () => { expect(() => assertWithinHashCap(5001, 5000)).toThrow(/5001/);