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/config/edit.ts

@@ -7,6 +7,7 @@ import { getConfigStore } from '../../storage/config-store.js'
 import { SafeCLIError } from '../../utils/errors.js'
 import { renderScreen } from '../../ui/render.js'
 import { ChainEditSuccessScreen } from '../../ui/screens/index.js'
+import type { ChainConfig } from '../../types/config.js'
 
 export async function editChains() {
   p.intro('Edit Chain Configurations')
@@ -66,10 +67,10 @@ export async function editChains() {
     unlinkSync(tempFile) // Clean up temp file
 
     // Parse and validate
-    let parsedConfig: any
+    let parsedConfig: { chains?: Record<string, unknown> }
     try {
       parsedConfig = JSON.parse(editedContent)
-    } catch (error) {
+    } catch {
       throw new SafeCLIError('Invalid JSON format. Changes not saved.')
     }
 
@@ -81,7 +82,7 @@ export async function editChains() {
 
     // Validate each chain
     for (const [chainId, chain] of Object.entries(newChains)) {
-      const c = chain as any
+      const c = chain as ChainConfig
 
       if (!c.chainId || !c.name || !c.shortName || !c.rpcUrl || !c.currency) {
         throw new SafeCLIError(
@@ -135,9 +136,9 @@ export async function editChains() {
       (id) => oldChainIds.has(id) && JSON.stringify(chains[id]) !== JSON.stringify(newChains[id])
     )
 
-    const addedNames = added.map((id) => newChains[id].name)
+    const addedNames = added.map((id) => (newChains[id] as ChainConfig).name)
     const removedNames = removed.map((id) => chains[id].name)
-    const modifiedNames = modified.map((id) => newChains[id].name)
+    const modifiedNames = modified.map((id) => (newChains[id] as ChainConfig).name)
 
     const confirm = await p.confirm({
       message: 'Apply these changes?',
@@ -160,7 +161,7 @@ export async function editChains() {
 
     // Add/update chains
     for (const [chainId, chain] of Object.entries(newChains)) {
-      const c = chain as any
+      const c = chain as ChainConfig
       configStore.setChain(chainId, {
         chainId: c.chainId,
         name: c.name,

src/commands/tx/create.ts

@@ -1,16 +1,16 @@
 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'
 import { getWalletStorage } from '../../storage/wallet-store.js'
 import { TransactionService } from '../../services/transaction-service.js'
 import { ContractService } from '../../services/contract-service.js'
-import { ABIService } from '../../services/abi-service.js'
+import { ABIService, type ABI, type ABIFunction } 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 @@ export async function createTransaction() {
     const configStore = getConfigStore()
     const walletStorage = getWalletStorage()
     const transactionStore = getTransactionStore()
+    const validator = getValidationService()
 
     const activeWallet = walletStorage.getActiveWallet()
     if (!activeWallet) {
@@ -105,27 +106,16 @@ export async function createTransaction() {
     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 @@ export async function createTransaction() {
     try {
       isContract = await contractService.isContract(to)
       spinner2.stop(isContract ? 'Contract detected' : 'EOA (regular address)')
-    } catch (error) {
+    } catch {
       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 @@ export async function createTransaction() {
       }
 
       const abiService = new ABIService(chain, etherscanApiKey)
-      let abi: any = null
+      let abi: ABI | null = null
       let contractName: string | undefined
 
       try {
@@ -203,17 +193,15 @@ export async function createTransaction() {
             // Filter out duplicates by function signature
             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(',') || ''})`
-                )
+              (implAbi.filter((item) => item.type === 'function') as ABIFunction[]).map(
+                (item) => `${item.name}(${item.inputs?.map((i) => i.type).join(',') || ''})`
+              )
             )
 
             for (const item of abi) {
               if (item.type === 'function') {
-                const sig = `${item.name}(${item.inputs?.map((i: any) => i.type).join(',') || ''})`
+                const funcItem = item as ABIFunction
+                const sig = `${funcItem.name}(${funcItem.inputs?.map((i) => i.type).join(',') || ''})`
                 if (!existingSignatures.has(sig)) {
                   combinedAbi.push(item)
                 }
@@ -225,14 +213,14 @@ export async function createTransaction() {
 
             abi = combinedAbi
             console.log(`  Combined: ${abi.length} items total`)
-          } catch (error) {
+          } catch {
             console.log('⚠ Could not fetch implementation ABI, using proxy ABI only')
             console.log(`  Found ${abi.length} items in proxy ABI`)
           }
         } else {
           console.log(`  Found ${abi.length} items in ABI`)
         }
-      } catch (error) {
+      } catch {
         console.log('⚠ Could not fetch ABI')
         console.log('  Contract may not be verified. Falling back to manual input.')
       }
@@ -261,7 +249,7 @@ export async function createTransaction() {
             const selectedFuncSig = await p.select({
               message: 'Select function to call:',
               options: functions.map((func) => {
-                const signature = `${func.name}(${func.inputs?.map((i: any) => i.type).join(',') || ''})`
+                const signature = `${func.name}(${func.inputs?.map((i) => i.type).join(',') || ''})`
                 return {
                   value: signature,
                   label: abiService.formatFunctionSignature(func),
@@ -277,7 +265,7 @@ export async function createTransaction() {
             }
 
             const func = functions.find((f) => {
-              const sig = `${f.name}(${f.inputs?.map((i: any) => i.type).join(',') || ''})`
+              const sig = `${f.name}(${f.inputs?.map((i) => i.type).join(',') || ''})`
               return sig === selectedFuncSig
             })
             if (!func) {
@@ -307,15 +295,7 @@ export async function createTransaction() {
         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 +307,7 @@ export async function createTransaction() {
         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 +347,7 @@ export async function createTransaction() {
     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/execute.ts

@@ -173,7 +173,7 @@ export async function executeTransaction(safeTxHash?: string) {
     let privateKey: string
     try {
       privateKey = walletStorage.getPrivateKey(activeWallet.id, password)
-    } catch (error) {
+    } catch {
       spinner2.stop('Failed')
       p.log.error('Invalid password')
       p.outro('Failed')