From e4677926a2737d335dc7da54cd3665c8c8620a7c Mon Sep 17 00:00:00 2001 From: Matt Schoch Date: Wed, 27 Mar 2024 10:57:39 -0400 Subject: [PATCH] Adding zod dto to vault, initial zod-based scheam for Action types --- .../src/shared/filter/zod-exception.filter.ts | 16 ++-- .../vault/src/vault/__test__/e2e/sign.spec.ts | 15 ++-- .../http/rest/controller/sign.controller.ts | 13 +++- .../vault/http/rest/dto/sign-request.dto.ts | 20 ----- packages/policy-engine-shared/src/index.ts | 1 + .../src/lib/schema/action.schema.ts | 74 +++++++++++++++++++ .../lib/util/__test__/unit/evm.util.spec.ts | 4 + .../src/lib/util/evm.util.ts | 4 +- 8 files changed, 111 insertions(+), 36 deletions(-) delete mode 100644 apps/vault/src/vault/http/rest/dto/sign-request.dto.ts create mode 100644 packages/policy-engine-shared/src/lib/schema/action.schema.ts diff --git a/apps/vault/src/shared/filter/zod-exception.filter.ts b/apps/vault/src/shared/filter/zod-exception.filter.ts index aa9aff7f4..80c2e8a02 100644 --- a/apps/vault/src/shared/filter/zod-exception.filter.ts +++ b/apps/vault/src/shared/filter/zod-exception.filter.ts @@ -1,24 +1,28 @@ import { ArgumentsHost, Catch, ExceptionFilter, HttpStatus, Logger } from '@nestjs/common' import { ConfigService } from '@nestjs/config' import { Response } from 'express' +import { ZodValidationException } from 'nestjs-zod' import { ZodError } from 'zod' import { Config, Env } from '../../main.config' -@Catch(ZodError) +// Catch both types, because the zodToDto function will throw a wrapped ZodValidationError that otherwise isn't picked up here +@Catch(ZodError, ZodValidationException) export class ZodExceptionFilter implements ExceptionFilter { private logger = new Logger(ZodExceptionFilter.name) constructor(private configService: ConfigService) {} - catch(exception: ZodError, host: ArgumentsHost) { + catch(exception: ZodError | ZodValidationException, host: ArgumentsHost) { const ctx = host.switchToHttp() const response = ctx.getResponse() const status = HttpStatus.UNPROCESSABLE_ENTITY const isProduction = this.configService.get('env') === Env.PRODUCTION + const zodError = exception instanceof ZodValidationException ? exception.getZodError() : exception + // Log as error level because Zod issues should be handled by the caller. this.logger.error('Uncaught ZodError', { - exception + exception: zodError }) response.status(status).json( @@ -26,13 +30,13 @@ export class ZodExceptionFilter implements ExceptionFilter { ? { statusCode: status, message: 'Internal validation error', - context: exception.errors + context: zodError.flatten() } : { statusCode: status, message: 'Internal validation error', - context: exception.errors, - stacktrace: exception.stack + context: zodError.flatten(), + stacktrace: zodError.stack } ) } diff --git a/apps/vault/src/vault/__test__/e2e/sign.spec.ts b/apps/vault/src/vault/__test__/e2e/sign.spec.ts index 1b2f4695d..7b1c5d136 100644 --- a/apps/vault/src/vault/__test__/e2e/sign.spec.ts +++ b/apps/vault/src/vault/__test__/e2e/sign.spec.ts @@ -169,13 +169,14 @@ describe('Sign', () => { .set('authorization', `GNAP ${accessToken}`) .send(payload) - expect(status).toEqual(HttpStatus.BAD_REQUEST) - - expect(body).toEqual({ - error: 'Bad Request', - message: ['request.transactionRequest.to must be an Ethereum address'], - statusCode: HttpStatus.BAD_REQUEST - }) + expect(body).toEqual( + expect.objectContaining({ + context: expect.any(Object), + message: 'Internal validation error', + statusCode: HttpStatus.UNPROCESSABLE_ENTITY + }) + ) + expect(status).toEqual(HttpStatus.UNPROCESSABLE_ENTITY) }) it('signs', async () => { diff --git a/apps/vault/src/vault/http/rest/controller/sign.controller.ts b/apps/vault/src/vault/http/rest/controller/sign.controller.ts index c6b462b94..da03d3a43 100644 --- a/apps/vault/src/vault/http/rest/controller/sign.controller.ts +++ b/apps/vault/src/vault/http/rest/controller/sign.controller.ts @@ -1,10 +1,21 @@ import { Request } from '@narval/policy-engine-shared' import { Body, Controller, Post, UseGuards } from '@nestjs/common' +import { createZodDto } from 'nestjs-zod' +import { + SignMessageActionSchema, + SignTransactionActionSchema, + SignTypedDataActionSchema +} from 'packages/policy-engine-shared/src/lib/schema/action.schema' +import { z } from 'zod' import { ClientId } from '../../../../shared/decorator/client-id.decorator' import { AuthorizationGuard } from '../../../../shared/guard/authorization.guard' import { SigningService } from '../../../core/service/signing.service' -import { SignRequestDto } from '../dto/sign-request.dto' +const SignRequestSchema = z.object({ + request: z.union([SignTransactionActionSchema, SignMessageActionSchema, SignTypedDataActionSchema]) +}) + +class SignRequestDto extends createZodDto(SignRequestSchema) {} @Controller('/sign') @UseGuards(AuthorizationGuard) export class SignController { diff --git a/apps/vault/src/vault/http/rest/dto/sign-request.dto.ts b/apps/vault/src/vault/http/rest/dto/sign-request.dto.ts deleted file mode 100644 index acdb36c46..000000000 --- a/apps/vault/src/vault/http/rest/dto/sign-request.dto.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { SignMessageRequestDataDto, SignTransactionRequestDataDto } from '@narval/nestjs-shared' -import { Action } from '@narval/policy-engine-shared' -import { ApiExtraModels, ApiProperty, getSchemaPath } from '@nestjs/swagger' -import { Type } from 'class-transformer' -import { IsDefined, ValidateNested } from 'class-validator' - -@ApiExtraModels(SignTransactionRequestDataDto, SignMessageRequestDataDto) -export class SignRequestDto { - @Type((opts) => { - return opts?.object.request.action === Action.SIGN_TRANSACTION - ? SignTransactionRequestDataDto - : SignMessageRequestDataDto - }) - @IsDefined() - @ApiProperty({ - oneOf: [{ $ref: getSchemaPath(SignTransactionRequestDataDto) }, { $ref: getSchemaPath(SignMessageRequestDataDto) }] - }) - @ValidateNested() - request: SignTransactionRequestDataDto | SignMessageRequestDataDto -} diff --git a/packages/policy-engine-shared/src/index.ts b/packages/policy-engine-shared/src/index.ts index eaac8f8fd..9b53dee13 100644 --- a/packages/policy-engine-shared/src/index.ts +++ b/packages/policy-engine-shared/src/index.ts @@ -1,3 +1,4 @@ +export * from './lib/schema/action.schema' export * from './lib/schema/address.schema' export * from './lib/schema/data-store.schema' export * from './lib/schema/domain.schema' diff --git a/packages/policy-engine-shared/src/lib/schema/action.schema.ts b/packages/policy-engine-shared/src/lib/schema/action.schema.ts new file mode 100644 index 000000000..2b86094a2 --- /dev/null +++ b/packages/policy-engine-shared/src/lib/schema/action.schema.ts @@ -0,0 +1,74 @@ +import { z } from 'zod' +import { Action } from '../type/action.type' +import { addressSchema } from './address.schema' +import { hexSchema } from './hex.schema' + +export const AccessListSchema = z.array( + z.object({ + address: addressSchema, + storageKeys: z.array(hexSchema) + }) +) +// export type AccessList = z.infer + +export const ActionSchema = z.nativeEnum(Action) + +export const BaseActionSchema = z.object({ + action: ActionSchema, + nonce: z.string() +}) +// export type BaseActionSchema = z.infer + +export const TransactionRequestSchema = z.object({ + chainId: z.number(), + from: addressSchema, + nonce: z.number().optional(), + accessList: AccessListSchema.optional(), + data: hexSchema.optional(), + gas: z.coerce.bigint().optional(), + maxFeePerGas: z.coerce.bigint().optional(), + maxPriorityFeePerGas: z.coerce.bigint().optional(), + to: addressSchema.nullable().optional(), + type: z.literal('2').optional(), + value: hexSchema.optional() +}) +// export type TransactionRequest = z.infer + +export const SignTransactionActionSchema = z.intersection( + BaseActionSchema, + z.object({ + action: z.literal(Action.SIGN_TRANSACTION), + resourceId: z.string(), + transactionRequest: TransactionRequestSchema + }) +) +// export type SignTransactionAction = z.infer + +// Matching viem's SignableMessage options https://viem.sh/docs/actions/wallet/signMessage#message +export const SignableMessageSchema = z.union([ + z.string(), + z.object({ + raw: hexSchema + }) +]) +// export type SignableMessage = z.infer + +export const SignMessageActionSchema = z.intersection( + BaseActionSchema, + z.object({ + action: z.literal(Action.SIGN_MESSAGE), + resourceId: z.string(), + message: SignableMessageSchema + }) +) +// export type SignMessageAction = z.infer + +export const SignTypedDataActionSchema = z.intersection( + BaseActionSchema, + z.object({ + action: z.literal(Action.SIGN_TYPED_DATA), + resourceId: z.string(), + typedData: z.string() + }) +) +// export type SignTypedDataAction = z.infer diff --git a/packages/policy-engine-shared/src/lib/util/__test__/unit/evm.util.spec.ts b/packages/policy-engine-shared/src/lib/util/__test__/unit/evm.util.spec.ts index c839e648b..04fbf21fb 100644 --- a/packages/policy-engine-shared/src/lib/util/__test__/unit/evm.util.spec.ts +++ b/packages/policy-engine-shared/src/lib/util/__test__/unit/evm.util.spec.ts @@ -30,6 +30,10 @@ describe('evm', () => { expect(isAddress('foo')).toEqual(false) expect(isAddress(address.toUpperCase())).toEqual(false) }) + + it('requires 0x prefix', () => { + expect(isAddress(address.slice(2))).toEqual(false) + }) }) describe('getAddress', () => { diff --git a/packages/policy-engine-shared/src/lib/util/evm.util.ts b/packages/policy-engine-shared/src/lib/util/evm.util.ts index 342c342c7..bb776a340 100644 --- a/packages/policy-engine-shared/src/lib/util/evm.util.ts +++ b/packages/policy-engine-shared/src/lib/util/evm.util.ts @@ -10,9 +10,9 @@ import { Address } from '../type/domain.type' * returns false. */ export const isAddress = (address: string): boolean => { - if (!/^(0x)?[0-9a-fA-F]{40}$/.test(address)) { + if (!/^0x[0-9a-fA-F]{40}$/.test(address)) { return false - } else if (/^(0x)?[0-9a-f]{40}$/.test(address) || /^(0x)?[0-9A-F]{40}$/.test(address)) { + } else if (/^0x[0-9a-f]{40}$/.test(address) || /^0x[0-9A-F]{40}$/.test(address)) { return true } else { return viemIsAddress(address)