From dd668ad9a09d3df2a750b308e98720a980fa90df Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:00:01 +1000 Subject: [PATCH 1/8] feat(api,app): relax BoundedStr min and max length --- api/dbschema/migrations/00008-m13ngaz.edgeql | 10 ++++++++++ api/dbschema/scalars.esdl | 2 +- api/src/feat/transfers/insert-transfer.query.ts | 2 -- app/src/lib/zod.ts | 6 +++--- 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 api/dbschema/migrations/00008-m13ngaz.edgeql diff --git a/api/dbschema/migrations/00008-m13ngaz.edgeql b/api/dbschema/migrations/00008-m13ngaz.edgeql new file mode 100644 index 000000000..53f372955 --- /dev/null +++ b/api/dbschema/migrations/00008-m13ngaz.edgeql @@ -0,0 +1,10 @@ +CREATE MIGRATION m13ngazbt3dkkbegsr7e2te4y7j4jltf56szjupnmb3ngzd6yq33zq + ONTO m15qj3htiny7zuqceuvj46wrm7yza5xyqmfuadsywsbc6tvh6vb34a +{ + ALTER SCALAR TYPE default::BoundedStr { + DROP CONSTRAINT std::regexp(r'^(?![0oO][xX])[^\n\t]{3,50}$'); + }; + ALTER SCALAR TYPE default::BoundedStr { + CREATE CONSTRAINT std::regexp(r'^(?![0oO][xX])[^\n\t]{2,70}$'); + }; +}; diff --git a/api/dbschema/scalars.esdl b/api/dbschema/scalars.esdl index 47688e1b5..6f99eee0c 100644 --- a/api/dbschema/scalars.esdl +++ b/api/dbschema/scalars.esdl @@ -12,7 +12,7 @@ module default { }; scalar type BoundedStr extending str { - constraint regexp(r'^(?![0oO][xX])[^\n\t]{3,50}$'); + constraint regexp(r'^(?![0oO][xX])[^\n\t]{2,70}$'); } scalar type Bytes extending str { diff --git a/api/src/feat/transfers/insert-transfer.query.ts b/api/src/feat/transfers/insert-transfer.query.ts index 261ac0665..7c44a9069 100644 --- a/api/src/feat/transfers/insert-transfer.query.ts +++ b/api/src/feat/transfers/insert-transfer.query.ts @@ -18,7 +18,6 @@ export type InsertTransferArgs = { export type InsertTransferReturns = { "id": string; "internal": boolean; - "fee": boolean; "accountUsers": Array; } | null; @@ -52,7 +51,6 @@ with accountAddress := $account, select transfer { id, internal, - fee, accountUsers := .account.approvers.user.id }`, args); diff --git a/app/src/lib/zod.ts b/app/src/lib/zod.ts index 6375eaf3f..4147854ed 100644 --- a/app/src/lib/zod.ts +++ b/app/src/lib/zod.ts @@ -60,9 +60,9 @@ export function zBool() { export function zBoundStr() { return z .string() - .min(3) - .max(50) - .regex(/(?![0oO][xX])[^\n\t]{3,50}$/, 'Must not start with 0x'); + .min(2) + .max(70) + .regex(/(?![0oO][xX])[^\n\t]{2,70}$/, 'Must not start with 0x'); } export function zNonEmptyStr() { From 2b786d7eb240352ec4863d341d239d59f1856726 Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:01:29 +1000 Subject: [PATCH 2/8] fix(app): only allow valid cloud approver name when creating automatically --- app/src/hooks/cloud/useGetCloudApprover.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/hooks/cloud/useGetCloudApprover.ts b/app/src/hooks/cloud/useGetCloudApprover.ts index dae9133ed..a5a10e4a6 100644 --- a/app/src/hooks/cloud/useGetCloudApprover.ts +++ b/app/src/hooks/cloud/useGetCloudApprover.ts @@ -15,6 +15,7 @@ import { useGetCloudApproverQuery } from '~/api/__generated__/useGetCloudApprove import { signAuthHeaders } from '~/api/auth-manager'; import { UpdateApproverInput } from '~/api/__generated__/useGetCloudApproverMutation.graphql'; import { withHeaders } from '~/api/network/auth'; +import { zBoundStr } from '~/lib/zod'; const PK_PATH = '/approver.private-key'; const SCOPE = CloudStorageScope.AppData; @@ -84,9 +85,7 @@ export function useGetCloudApprover() { await fetchQuery( environment, Query, - { - approver: approver.address, - }, + { approver: approver.address }, { networkCacheConfig: withHeaders(authHeaders) }, ).toPromise() )?.approver.details; @@ -95,7 +94,11 @@ export function useGetCloudApprover() { { input: { address: approver.address, - name: !e?.name ? details?.name : undefined, + name: !e?.name + ? zBoundStr().safeParse( + details?.name?.slice(0, zBoundStr().maxLength ?? undefined), + ) + : undefined, cloud: !e?.cloud ? details?.cloud : undefined, }, }, From b89f43713ecce42973aa0b4ea7b9ec6d78640d0e Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:19:32 +1000 Subject: [PATCH 3/8] fix(api): don't cache database client - globals may change --- api/src/core/context/context.util.ts | 2 -- api/src/core/database/database.service.ts | 4 +--- api/src/feat/auth/accounts.cache.service.ts | 13 +++++-------- api/src/feat/transfers/insert-transfer.edgeql | 1 + api/src/feat/transfers/insert-transfer.query.ts | 2 ++ 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/api/src/core/context/context.util.ts b/api/src/core/context/context.util.ts index e00e778aa..4c0309644 100644 --- a/api/src/core/context/context.util.ts +++ b/api/src/core/context/context.util.ts @@ -2,12 +2,10 @@ import { AsyncLocalStorage } from 'async_hooks'; import { uuid } from 'edgedb/dist/codecs/ifaces'; import { Address, UAddress, UUID } from 'lib'; import { GqlContext } from '~/core/apollo/ctx'; -import { type Client as DatabaseClient } from 'edgedb'; export interface Context { afterRequestHooks: AfterRequestHook[]; user?: UserContext; - db?: DatabaseClient; } export interface UserContext { diff --git a/api/src/core/database/database.service.ts b/api/src/core/database/database.service.ts index eb82457d2..ea7122257 100644 --- a/api/src/core/database/database.service.ts +++ b/api/src/core/database/database.service.ts @@ -42,13 +42,11 @@ export class DatabaseService implements OnModuleInit { const reqCtx = getContextUnsafe(); if (!reqCtx?.user) return this.DANGEROUS_superuserClient; - reqCtx.db ??= this.__client.withGlobals({ + return this.__client.withGlobals({ current_accounts: reqCtx.user.accounts.map((a) => a.id), current_approver_address: reqCtx.user.approver, // current_user_id: reqCtx.user.id, } satisfies Globals); - - return reqCtx.db; } private async run(f: () => Promise, name = 'inline'): Promise { diff --git a/api/src/feat/auth/accounts.cache.service.ts b/api/src/feat/auth/accounts.cache.service.ts index 2ff14b47e..c2e2cd08c 100644 --- a/api/src/feat/auth/accounts.cache.service.ts +++ b/api/src/feat/auth/accounts.cache.service.ts @@ -55,8 +55,9 @@ export class AccountsCacheService implements OnModuleInit { accounts: JSON.parse(cachedAccounts) as UserAccountContext[], }; - const { user } = await this.db.queryWith( + const { user } = await this.db.queryWith2( { approver: e.Address }, + { approver }, ({ approver }) => e.select( e.insert(e.Approver, { address: approver }).unlessConflict((a) => ({ @@ -71,7 +72,6 @@ export class AccountsCacheService implements OnModuleInit { }, }), ), - { approver }, ); const accounts = user.accounts.map( @@ -99,14 +99,11 @@ export class AccountsCacheService implements OnModuleInit { if (!getUserCtx().accounts.find((a) => a.id === account.id)) getUserCtx().accounts.push(account); - const user = await this.db.queryWith( + const user = await this.db.queryWith2( { approver: e.Address }, - ({ approver }) => - e.select(e.Approver, () => ({ - filter_single: { address: approver }, - user: { id: true }, - })).user.id, { approver }, + ({ approver }) => + e.select(e.Approver, () => ({ filter_single: { address: approver } })).user.id, ); if (user) { diff --git a/api/src/feat/transfers/insert-transfer.edgeql b/api/src/feat/transfers/insert-transfer.edgeql index ad199f9fd..43fb27ec5 100644 --- a/api/src/feat/transfers/insert-transfer.edgeql +++ b/api/src/feat/transfers/insert-transfer.edgeql @@ -26,5 +26,6 @@ with accountAddress := $account, select transfer { id, internal, + fee, accountUsers := .account.approvers.user.id } \ No newline at end of file diff --git a/api/src/feat/transfers/insert-transfer.query.ts b/api/src/feat/transfers/insert-transfer.query.ts index 7c44a9069..261ac0665 100644 --- a/api/src/feat/transfers/insert-transfer.query.ts +++ b/api/src/feat/transfers/insert-transfer.query.ts @@ -18,6 +18,7 @@ export type InsertTransferArgs = { export type InsertTransferReturns = { "id": string; "internal": boolean; + "fee": boolean; "accountUsers": Array; } | null; @@ -51,6 +52,7 @@ with accountAddress := $account, select transfer { id, internal, + fee, accountUsers := .account.approvers.user.id }`, args); From fd75f324b1f19eea91714777d9bee26dd424bb34 Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:22:52 +1000 Subject: [PATCH 4/8] feat(api): attach edgeql params to sentry error --- api/src/core/database/database.service.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/api/src/core/database/database.service.ts b/api/src/core/database/database.service.ts index ea7122257..2e79a28ed 100644 --- a/api/src/core/database/database.service.ts +++ b/api/src/core/database/database.service.ts @@ -72,8 +72,13 @@ export class DatabaseService implements OnModuleInit { getExpr: (params: paramsToParamExprs) => Expr, params: paramsToParamArgs, ) { - const expression = e.params(paramsDef, getExpr as any); - return this.run(() => expression.run(this.client, params as any)) as Promise<$infer>; + try { + const expression = e.params(paramsDef, getExpr as any); + return this.run(() => expression.run(this.client, params as any)) as Promise<$infer>; + } catch (e) { + if (e instanceof EdgeDBError) Sentry.setExtra('EdgeQL Params', params); + throw e; + } } async queryWith2< @@ -84,8 +89,7 @@ export class DatabaseService implements OnModuleInit { params: paramsToParamArgs, getExpr: (params: paramsToParamExprs) => Expr, ) { - const expression = e.params(paramsDef, getExpr as any); - return this.run(() => expression.run(this.client, params as any)) as Promise<$infer>; + return this.queryWith(paramsDef, getExpr, params); } async exec Promise>( From e24012f17c2ec53020fc6ba0657d5fc718e97933 Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:28:58 +1000 Subject: [PATCH 5/8] feat(api): use parameterized query for account insertion --- api/src/core/database/database.service.ts | 4 +++- api/src/feat/accounts/accounts.service.ts | 17 ++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/api/src/core/database/database.service.ts b/api/src/core/database/database.service.ts index 2e79a28ed..3cde1ae72 100644 --- a/api/src/core/database/database.service.ts +++ b/api/src/core/database/database.service.ts @@ -74,7 +74,9 @@ export class DatabaseService implements OnModuleInit { ) { try { const expression = e.params(paramsDef, getExpr as any); - return this.run(() => expression.run(this.client, params as any)) as Promise<$infer>; + return (await this.run(() => expression.run(this.client, params as any))) as Promise< + $infer + >; } catch (e) { if (e instanceof EdgeDBError) Sentry.setExtra('EdgeQL Params', params); throw e; diff --git a/api/src/feat/accounts/accounts.service.ts b/api/src/feat/accounts/accounts.service.ts index 6fd85eecb..db73a73a8 100644 --- a/api/src/feat/accounts/accounts.service.ts +++ b/api/src/feat/accounts/accounts.service.ts @@ -35,6 +35,7 @@ import { AccountEvent } from './accounts.model'; import { PolicyInput } from '../policies/policies.input'; import { utils as zkUtils } from 'zksync-ethers'; import { toHex } from 'viem'; +import { insertAccount } from './insert-account.query'; const accountTrigger = (account: UAddress) => `account.updated:${account}`; const accountApproverTrigger = (approver: Address) => `account.updated:approver:${approver}`; @@ -148,15 +149,13 @@ export class AccountsService { ); await this.db.transaction(async () => { - await this.db.query( - e.insert(e.Account, { - id, - address, - name, - implementation, - initialization: { salt, bytecodeHash, aaVersion: 1 }, - }), - ); + await this.db.exec(insertAccount, { + id, + address, + name, + implementation, + initialization: { salt, bytecodeHash, aaVersion: 1 }, + }); await this.policies.propose( { From cc9e4c50b230ef1e970ab85227b4f514cc00f18a Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 10:35:09 +1000 Subject: [PATCH 6/8] feat(api): attach user context to sentry exception scope --- api/src/core/sentry/sentry.interceptor.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/core/sentry/sentry.interceptor.ts b/api/src/core/sentry/sentry.interceptor.ts index 4dd0a1695..80cf4c15b 100644 --- a/api/src/core/sentry/sentry.interceptor.ts +++ b/api/src/core/sentry/sentry.interceptor.ts @@ -54,6 +54,7 @@ export class SentryInterceptor implements NestInterceptor { scope.setExtra('exceptionData', JSON.stringify(exception, null, 2)); this.addContextExceptionMetadata(scope, context); + scope.setExtra('userContext', getContextUnsafe()?.user); Sentry.captureException(exception); }, From 4d5202e8f518a8abd3d59d9d302f486eaf79b1a7 Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 11:18:56 +1000 Subject: [PATCH 7/8] fix(app): handle gasUsed initially undefined for some reason --- app/src/components/transaction/TransactionDetails.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/components/transaction/TransactionDetails.tsx b/app/src/components/transaction/TransactionDetails.tsx index 4c9499457..3e9944915 100644 --- a/app/src/components/transaction/TransactionDetails.tsx +++ b/app/src/components/transaction/TransactionDetails.tsx @@ -48,7 +48,7 @@ export function TransactionDetails(props: TransactionDetailsProps) { } trailing={ } /> From e76872ea16b2d5d37b9636dcf168076dc6c8f5ba Mon Sep 17 00:00:00 2001 From: Hayden Briese Date: Fri, 23 Aug 2024 11:38:42 +1000 Subject: [PATCH 8/8] fix(api): 'hidden by access policy' error when updating policies This is an edgedb issue fixed by using backlinks --- api/src/feat/policies/existing-policies.edgeql | 5 ++--- api/src/feat/policies/existing-policies.query.ts | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/api/src/feat/policies/existing-policies.edgeql b/api/src/feat/policies/existing-policies.edgeql index afb279a4b..aae17324d 100644 --- a/api/src/feat/policies/existing-policies.edgeql +++ b/api/src/feat/policies/existing-policies.edgeql @@ -1,6 +1,6 @@ with account := (select Account filter .address = $account), keys := array_unpack(>$policyKeys) -select Policy { +select account.$account), keys := array_unpack(>$policyKeys) -select Policy { +select account.