Convert CAIP-10 address validation to schema (#2110)

* Don't allow empty/hex strings to be validated as numeric strings * Convert CAIP-10 address validation to schema --------- Co-authored-by: Hector Gómez Varela <[email protected]>
safe-global · Nov 14, 2024 · 83f24e9 · 83f24e9
1 parent 20e8be8
commit 83f24e9
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 137 deletions.
diff --git a/src/routes/safes/entities/caip-10-addresses.entity.spec.ts b/src/routes/safes/entities/caip-10-addresses.entity.spec.ts
@@ -0,0 +1,138 @@
+import { Caip10AddressesSchema } from '@/routes/safes/entities/caip-10-addresses.entity';
+import { faker } from '@faker-js/faker';
+import { getAddress } from 'viem';
+
+describe('Caip10AddressesSchema', () => {
+  const caip10Addresses = faker.helpers.multiple(
+    () => {
+      return {
+        chainId: faker.string.numeric(),
+        address: faker.finance.ethereumAddress(),
+      };
+    },
+    {
+      count: { min: 1, max: 5 },
+    },
+  );
+
+  it('should parse CAIP-10 addresses', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${address.chainId}:${address.address}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(result.success).toBe(true);
+  });
+
+  it('should checksum addresses', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${address.chainId}:${address.address}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(
+      result.success && result.data.map(({ address }) => address),
+    ).toStrictEqual(
+      caip10Addresses.map(({ address }) => {
+        return getAddress(address);
+      }),
+    );
+  });
+
+  it('should throw for incorrectly formatted CAIP-10 addresses', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${address.chainId}-${address.address}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(!result.success && result.error.issues).toStrictEqual(
+      Array.from({ length: caip10Addresses.length }).flatMap(() => [
+        {
+          code: 'custom',
+          message: 'Invalid base-10 numeric string',
+          path: ['chainId'],
+        },
+        {
+          code: 'invalid_type',
+          expected: 'string',
+          message: 'Required',
+          path: ['address'],
+          received: 'undefined',
+        },
+      ]),
+    );
+  });
+
+  it('should throw for non-numerical chainIds', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${faker.string.hexadecimal()}:${address.address}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(!result.success && result.error.issues).toStrictEqual(
+      Array.from({ length: caip10Addresses.length }).map(() => ({
+        code: 'custom',
+        message: 'Invalid base-10 numeric string',
+        path: ['chainId'],
+      })),
+    );
+  });
+
+  it('should throw for invalid addresses', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${address.chainId}:${faker.string.numeric()}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(!result.success && result.error.issues).toStrictEqual(
+      Array.from({ length: caip10Addresses.length }).flatMap(() => [
+        {
+          code: 'custom',
+          message: 'Invalid address',
+          path: ['address'],
+        },
+      ]),
+    );
+  });
+
+  it('should throw for missing chainIds', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `:${address.address}`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(!result.success && result.error.issues).toStrictEqual(
+      Array.from({ length: caip10Addresses.length }).flatMap(() => [
+        {
+          code: 'custom',
+          message: 'Invalid base-10 numeric string',
+          path: ['chainId'],
+        },
+      ]),
+    );
+  });
+
+  it('should throw for missing addresses', () => {
+    const caip10AddressesString = caip10Addresses
+      .map((address) => `${address.chainId}:`)
+      .join(',');
+
+    const result = Caip10AddressesSchema.safeParse(caip10AddressesString);
+
+    expect(!result.success && result.error.issues).toStrictEqual(
+      Array.from({ length: caip10Addresses.length }).flatMap(() => [
+        {
+          code: 'custom',
+          message: 'Invalid address',
+          path: ['address'],
+        },
+      ]),
+    );
+  });
+});
diff --git a/src/routes/safes/entities/caip-10-addresses.entity.ts b/src/routes/safes/entities/caip-10-addresses.entity.ts
@@ -0,0 +1,32 @@
+import { z } from 'zod';
+import { asError } from '@/logging/utils';
+import { AddressSchema } from '@/validation/entities/schemas/address.schema';
+import { NumericStringSchema } from '@/validation/entities/schemas/numeric-string.schema';
+
+export const Caip10AddressesSchema = z.string().transform((str, ctx) => {
+  return str.split(',').map((item) => {
+    try {
+      const [chainId, address] = item.split(':');
+      return z
+        .object({
+          chainId: NumericStringSchema,
+          address: AddressSchema,
+        })
+        .parse({ chainId, address });
+    } catch (e) {
+      if (e instanceof z.ZodError) {
+        e.issues.forEach((issue) => {
+          ctx.addIssue(issue);
+        });
+      } else {
+        ctx.addIssue({
+          code: z.ZodIssueCode.custom,
+          message: asError(e).message,
+        });
+      }
+      return z.NEVER;
+    }
+  });
+});
+
+export type Caip10Addresses = z.infer<typeof Caip10AddressesSchema>;
diff --git a/src/routes/safes/pipes/caip-10-addresses.pipe.spec.ts b/src/routes/safes/pipes/caip-10-addresses.pipe.spec.ts
diff --git a/src/routes/safes/pipes/caip-10-addresses.pipe.ts b/src/routes/safes/pipes/caip-10-addresses.pipe.ts
diff --git a/src/routes/safes/safes.controller.ts b/src/routes/safes/safes.controller.ts
@@ -11,7 +11,10 @@ import { SafeState } from '@/routes/safes/entities/safe-info.entity';
 import { SafesService } from '@/routes/safes/safes.service';
 import { SafeNonces } from '@/routes/safes/entities/nonces.entity';
 import { SafeOverview } from '@/routes/safes/entities/safe-overview.entity';
-import { Caip10AddressesPipe } from '@/routes/safes/pipes/caip-10-addresses.pipe';
+import {
+  Caip10AddressesSchema,
+  type Caip10Addresses,
+} from '@/routes/safes/entities/caip-10-addresses.entity';
 import { ValidationPipe } from '@/validation/pipes/validation.pipe';
 import { AddressSchema } from '@/validation/entities/schemas/address.schema';
 
@@ -50,8 +53,8 @@ export class SafesController {
   @Get('safes')
   async getSafeOverview(
     @Query('currency') currency: string,
-    @Query('safes', new Caip10AddressesPipe())
-    addresses: Array<{ chainId: string; address: `0x${string}` }>,
+    @Query('safes', new ValidationPipe(Caip10AddressesSchema))
+    addresses: Caip10Addresses,
     @Query('trusted', new DefaultValuePipe(false), ParseBoolPipe)
     trusted: boolean,
     @Query('exclude_spam', new DefaultValuePipe(true), ParseBoolPipe)

diff --git a/src/routes/safes/safes.service.ts b/src/routes/safes/safes.service.ts
@@ -25,6 +25,7 @@ import { SafeOverview } from '@/routes/safes/entities/safe-overview.entity';
 import { IConfigurationService } from '@/config/configuration.service.interface';
 import { LoggingService, ILoggingService } from '@/logging/logging.interface';
 import { asError } from '@/logging/utils';
+import { Caip10Addresses } from '@/routes/safes/entities/caip-10-addresses.entity';
 
 @Injectable()
 export class SafesService {
@@ -129,7 +130,7 @@ export class SafesService {
 
   async getSafeOverview(args: {
     currency: string;
-    addresses: Array<{ chainId: string; address: `0x${string}` }>;
+    addresses: Caip10Addresses;
     trusted: boolean;
     excludeSpam: boolean;
     walletAddress?: `0x${string}`;