fix: Critical bugs and centralized validation layer

src/commands/account/create.ts

@@ -4,7 +4,7 @@ import { getConfigStore } from '../../storage/config-store.js'
 import { getSafeStorage } from '../../storage/safe-store.js'
 import { getWalletStorage } from '../../storage/wallet-store.js'
 import { SafeService } from '../../services/safe-service.js'
-import { isValidAddress } from '../../utils/validation.js'
+import { getValidationService } from '../../services/validation-service.js'
 import { checksumAddress, shortenAddress } from '../../utils/ethereum.js'
 import { formatSafeAddress } from '../../utils/eip3770.js'
 import { logError } from '../../ui/messages.js'
@@ -67,6 +67,8 @@ export async function createSafe() {
 
   // Add more owners
   let addingOwners = true
+  const validator = getValidationService()
+
   while (addingOwners) {
     const addMore = await p.confirm({
       message: `Add ${owners.length > 0 ? 'another' : 'an'} owner?`,
@@ -87,9 +89,9 @@ export async function createSafe() {
       message: 'Owner address:',
       placeholder: '0x...',
       validate: (value) => {
-        if (!value) return 'Address is required'
-        if (!isValidAddress(value)) return 'Invalid Ethereum address'
-        const checksummed = checksumAddress(value)
+        const addressError = validator.validateAddress(value)
+        if (addressError) return addressError
+        const checksummed = checksumAddress(value as string)
         if (owners.includes(checksummed as Address)) return 'Owner already added'
         return undefined
       },
@@ -114,14 +116,7 @@ export async function createSafe() {
   const threshold = await p.text({
     message: `Signature threshold (1-${owners.length}):`,
     placeholder: Math.min(2, owners.length).toString(),
-    validate: (value) => {
-      if (!value) return 'Threshold is required'
-      const num = parseInt(value, 10)
-      if (isNaN(num)) return 'Must be a number'
-      if (num < 1) return 'Threshold must be at least 1'
-      if (num > owners.length) return `Threshold cannot exceed ${owners.length} owners`
-      return undefined
-    },
+    validate: (value) => validator.validateThreshold(value, 1, owners.length),
   })
 
   if (p.isCancel(threshold)) {
@@ -135,7 +130,7 @@ export async function createSafe() {
   const name = await p.text({
     message: 'Give this Safe a name:',
     placeholder: 'my-safe',
-    validate: (value) => (!value ? 'Name is required' : undefined),
+    validate: (value) => validator.validateRequired(value, 'Name'),
   })
 
   if (p.isCancel(name)) {

src/commands/config/chains.ts

@@ -1,7 +1,7 @@
 import * as p from '@clack/prompts'
 import { getConfigStore } from '../../storage/config-store.js'
 import type { ChainConfig } from '../../types/config.js'
-import { isValidChainId, isValidUrl } from '../../utils/validation.js'
+import { getValidationService } from '../../services/validation-service.js'
 import { logError } from '../../ui/messages.js'
 import { renderScreen } from '../../ui/render.js'
 import {
@@ -14,14 +14,15 @@ export async function addChain() {
   p.intro('Add Chain')
 
   const configStore = getConfigStore()
+  const validator = getValidationService()
 
   const chainId = await p.text({
     message: 'Chain ID:',
     placeholder: '1',
     validate: (value) => {
-      if (!value) return 'Chain ID is required'
-      if (!isValidChainId(value)) return 'Invalid chain ID'
-      if (configStore.chainExists(value)) return `Chain ${value} already exists`
+      const error = validator.validateChainId(value)
+      if (error) return error
+      if (configStore.chainExists(value as string)) return `Chain ${value} already exists`
       return undefined
     },
   })
@@ -34,7 +35,7 @@ export async function addChain() {
   const name = await p.text({
     message: 'Chain name:',
     placeholder: 'Ethereum Mainnet',
-    validate: (value) => (!value ? 'Chain name is required' : undefined),
+    validate: (value) => validator.validateRequired(value, 'Chain name'),
   })
 
   if (p.isCancel(name)) {
@@ -45,12 +46,7 @@ export async function addChain() {
   const shortName = await p.text({
     message: 'Short name (EIP-3770):',
     placeholder: 'eth',
-    validate: (value) => {
-      if (!value) return 'Short name is required'
-      if (!/^[a-z0-9-]+$/.test(value))
-        return 'Short name must be lowercase alphanumeric with hyphens'
-      return undefined
-    },
+    validate: (value) => validator.validateShortName(value),
   })
 
   if (p.isCancel(shortName)) {
@@ -61,11 +57,7 @@ export async function addChain() {
   const rpcUrl = await p.text({
     message: 'RPC URL:',
     placeholder: 'https://eth.llamarpc.com',
-    validate: (value) => {
-      if (!value) return 'RPC URL is required'
-      if (!isValidUrl(value)) return 'Invalid URL'
-      return undefined
-    },
+    validate: (value) => validator.validateUrl(value, true),
   })
 
   if (p.isCancel(rpcUrl)) {
@@ -86,7 +78,7 @@ export async function addChain() {
   const currency = await p.text({
     message: 'Native currency symbol:',
     placeholder: 'ETH',
-    validate: (value) => (!value ? 'Currency symbol is required' : undefined),
+    validate: (value) => validator.validateRequired(value, 'Currency symbol'),
   })
 
   if (p.isCancel(currency)) {
@@ -97,10 +89,7 @@ export async function addChain() {
   const transactionServiceUrl = await p.text({
     message: 'Safe Transaction Service URL (optional):',
     placeholder: 'https://safe-transaction-mainnet.safe.global',
-    validate: (value) => {
-      if (value && !isValidUrl(value)) return 'Invalid URL'
-      return undefined
-    },
+    validate: (value) => validator.validateUrl(value, false),
   })
 
   if (p.isCancel(transactionServiceUrl)) {

src/commands/tx/create.ts

@@ -1,5 +1,5 @@
 import * as p from '@clack/prompts'
-import { isAddress, type Address } from 'viem'
+import { type Address } from 'viem'
 import { getConfigStore } from '../../storage/config-store.js'
 import { getSafeStorage } from '../../storage/safe-store.js'
 import { getTransactionStore } from '../../storage/transaction-store.js'
@@ -8,9 +8,9 @@
 import { ContractService } from '../../services/contract-service.js'
 import { ABIService } from '../../services/abi-service.js'
 import { TransactionBuilder } from '../../services/transaction-builder.js'
+import { getValidationService } from '../../services/validation-service.js'
 import { SafeCLIError } from '../../utils/errors.js'
 import { formatSafeAddress } from '../../utils/eip3770.js'
-import { validateAndChecksumAddress } from '../../utils/validation.js'
 import { renderScreen } from '../../ui/render.js'
 import { TransactionCreateSuccessScreen } from '../../ui/screens/index.js'
 
@@ -22,6 +22,7 @@
     const configStore = getConfigStore()
     const walletStorage = getWalletStorage()
     const transactionStore = getTransactionStore()
+    const validator = getValidationService()
 
     const activeWallet = walletStorage.getActiveWallet()
     if (!activeWallet) {
@@ -105,27 +106,16 @@
     const toInput = await p.text({
       message: 'To address',
       placeholder: '0x...',
-      validate: (value) => {
-        if (!value) return 'Address is required'
-        if (!isAddress(value)) return 'Invalid Ethereum address'
-        return undefined
-      },
+      validate: (value) => validator.validateAddress(value),
     })
 
     if (p.isCancel(toInput)) {
       p.cancel('Operation cancelled')
       return
     }
 
-    // Checksum the address immediately
-    let to: Address
-    try {
-      to = validateAndChecksumAddress(toInput as string)
-    } catch (error) {
-      p.log.error(error instanceof Error ? error.message : 'Invalid address')
-      p.outro('Failed')
-      return
-    }
+    // Checksum the address
+    const to = validator.assertAddress(toInput as string, 'To address')
 
     // Check if address is a contract
     const contractService = new ContractService(chain)
@@ -139,7 +129,7 @@
     try {
       isContract = await contractService.isContract(to)
       spinner2.stop(isContract ? 'Contract detected' : 'EOA (regular address)')
     } catch (error) {
       spinner2.stop('Failed to check contract')
       p.log.warning('Could not determine if address is a contract, falling back to manual input')
     }
@@ -159,7 +149,7 @@
       }
 
       const abiService = new ABIService(chain, etherscanApiKey)
       let abi: any = null
       let contractName: string | undefined
 
       try {
@@ -204,16 +194,16 @@
             const combinedAbi = [...implAbi]
             const existingSignatures = new Set(
               implAbi
                 .filter((item: any) => item.type === 'function')
                 .map(
                   (item: any) =>
                     `${item.name}(${item.inputs?.map((i: any) => i.type).join(',') || ''})`
                 )
             )
 
             for (const item of abi) {
               if (item.type === 'function') {
                 const sig = `${item.name}(${item.inputs?.map((i: any) => i.type).join(',') || ''})`
                 if (!existingSignatures.has(sig)) {
                   combinedAbi.push(item)
                 }
@@ -307,15 +297,7 @@
         message: 'Value in wei (0 for token transfer)',
         placeholder: '0',
         initialValue: '0',
-        validate: (val) => {
-          if (!val) return 'Value is required'
-          try {
-            BigInt(val)
-            return undefined
-          } catch {
-            return 'Invalid number'
-          }
-        },
+        validate: (val) => validator.validateWeiValue(val),
       })) as string
 
       if (p.isCancel(value)) {
@@ -327,14 +309,7 @@
         message: 'Transaction data (hex)',
         placeholder: '0x',
         initialValue: '0x',
-        validate: (val) => {
-          if (!val) return 'Data is required (use 0x for empty)'
-          if (!val.startsWith('0x')) return 'Data must start with 0x'
-          if (val.length > 2 && !/^0x[0-9a-fA-F]*$/.test(val)) {
-            return 'Data must be valid hex'
-          }
-          return undefined
-        },
+        validate: (val) => validator.validateHexData(val),
       })) as `0x${string}`
 
       if (p.isCancel(data)) {
@@ -374,14 +349,7 @@
     const nonceInput = (await p.text({
       message: 'Transaction nonce (leave empty for default)',
       placeholder: `${currentNonce} (recommended: current nonce)`,
-      validate: (value) => {
-        if (!value) return undefined // Empty is OK (will use default)
-        const num = parseInt(value, 10)
-        if (isNaN(num) || num < 0) return 'Nonce must be a non-negative number'
-        if (num < currentNonce)
-          return `Nonce cannot be lower than current Safe nonce (${currentNonce})`
-        return undefined
-      },
+      validate: (value) => validator.validateNonce(value, currentNonce),
     })) as string
 
     if (p.isCancel(nonceInput)) {

src/commands/tx/pull.ts

@@ -151,7 +151,7 @@ export async function pullTransactions(account?: string) {
             transactionStore.addSignature(safeTxHash, {
               signer: confirmation.owner as Address,
               signature: confirmation.signature,
-              signedAt: new Date(confirmation.submissionDate),
+              signedAt: new Date(confirmation.submissionDate).toISOString(),
             })
           }
 
@@ -176,7 +176,7 @@ export async function pullTransactions(account?: string) {
               transactionStore.addSignature(safeTxHash, {
                 signer: confirmation.owner as Address,
                 signature: confirmation.signature,
-                signedAt: new Date(confirmation.submissionDate),
+                signedAt: new Date(confirmation.submissionDate).toISOString(),
               })
             }
 

src/commands/tx/sign.ts

@@ -182,7 +182,7 @@ export async function signTransaction(safeTxHash?: string) {
     transactionStore.addSignature(selectedSafeTxHash, {
       signer: activeWallet.address as Address,
       signature,
-      signedAt: new Date(),
+      signedAt: new Date().toISOString(),
     })
 
     // Update status to signed if not already

src/commands/tx/sync.ts

@@ -148,7 +148,7 @@ export async function syncTransactions(account?: string) {
             transactionStore.addSignature(safeTxHash, {
               signer: confirmation.owner as Address,
               signature: confirmation.signature,
-              signedAt: new Date(confirmation.submissionDate),
+              signedAt: new Date(confirmation.submissionDate).toISOString(),
             })
           }
 
@@ -166,7 +166,7 @@ export async function syncTransactions(account?: string) {
               transactionStore.addSignature(safeTxHash, {
                 signer: confirmation.owner as Address,
                 signature: confirmation.signature,
-                signedAt: new Date(confirmation.submissionDate),
+                signedAt: new Date(confirmation.submissionDate).toISOString(),
               })
             }
 

src/commands/wallet/import.ts

@@ -1,7 +1,7 @@
 import * as p from '@clack/prompts'
 import { type Address } from 'viem'
 import { getWalletStorage } from '../../storage/wallet-store.js'
-import { isValidPrivateKey } from '../../utils/validation.js'
+import { getValidationService } from '../../services/validation-service.js'
 import { logError } from '../../ui/messages.js'
 import { renderScreen } from '../../ui/render.js'
 import { WalletImportSuccessScreen } from '../../ui/screens/index.js'
@@ -10,15 +10,12 @@ export async function importWallet() {
   p.intro('Import Wallet')
 
   const walletStorage = getWalletStorage()
+  const validator = getValidationService()
 
   // Get password for encryption
   const password = await p.password({
     message: 'Create a password to encrypt your wallet:',
-    validate: (value) => {
-      if (!value) return 'Password is required'
-      if (value.length < 8) return 'Password must be at least 8 characters'
-      return undefined
-    },
+    validate: (value) => validator.validatePassword(value, 8),
   })
 
   if (p.isCancel(password)) {
@@ -28,10 +25,7 @@ export async function importWallet() {
 
   const confirmPassword = await p.password({
     message: 'Confirm password:',
-    validate: (value) => {
-      if (value !== password) return 'Passwords do not match'
-      return undefined
-    },
+    validate: (value) => validator.validatePasswordConfirmation(value, password as string),
   })
 
   if (p.isCancel(confirmPassword)) {
@@ -44,13 +38,7 @@ export async function importWallet() {
   // Get private key
   const privateKey = await p.password({
     message: 'Enter your private key:',
-    validate: (value) => {
-      if (!value) return 'Private key is required'
-      if (!isValidPrivateKey(value)) {
-        return 'Invalid private key format. Must be a 64-character hex string (with or without 0x prefix)'
-      }
-      return undefined
-    },
+    validate: (value) => validator.validatePrivateKey(value),
   })
 
   if (p.isCancel(privateKey)) {
@@ -62,7 +50,7 @@ export async function importWallet() {
   const name = await p.text({
     message: 'Give this wallet a name:',
     placeholder: 'my-wallet',
-    validate: (value) => (!value ? 'Wallet name is required' : undefined),
+    validate: (value) => validator.validateRequired(value, 'Wallet name'),
   })
 
   if (p.isCancel(name)) {