Assist_Design/VALIDATION_AUDIT_REPORT.md

12 KiB

Validation Schema & Logic Audit Report

Date: October 8, 2025
Scope: Domain validation schemas and BFF validation logic
Status: 🔴 Issues Found - Recommendations Provided


Executive Summary

The codebase has undergone previous validation cleanup efforts (documented in docs/validation/), but there are still critical overlaps and inconsistencies that need to be addressed. This audit identifies:

  • 5 major duplication issues across validation utilities
  • 1 architectural concern with business logic placement
  • 3 potential naming/organizational improvements

🔴 Critical Issues

1. UUID Validation Duplication (Priority: HIGH)

Problem: Two different implementations of isValidUuid() exist:

Location Implementation Issue
packages/domain/common/validation.ts:76 Uses Zod's .uuid() Correct (Zod-based)
packages/domain/toolkit/validation/helpers.ts:23 Manual regex /^[0-9a-f]{8}-...$/i ⚠️ Duplicate logic

Impact:

  • Inconsistent validation behavior
  • Maintenance burden (changes needed in two places)
  • The regex version might accept invalid UUIDs that Zod would reject

Recommendation:

// ❌ Remove from toolkit/validation/helpers.ts
export function isValidUuid(uuid: string): boolean {
  const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
  return uuidRegex.test(uuid);
}

// ✅ Keep only in common/validation.ts (uses Zod)
export function isValidUuid(id: string): boolean {
  return uuidSchema.safeParse(id).success;
}

2. UUID Schema Duplication (Priority: HIGH)

Problem: uuidSchema is exported from two locations:

Location Definition
packages/domain/common/validation.ts:13 z.string().uuid()
packages/domain/toolkit/validation/helpers.ts:202 z.string().uuid()

Impact:

  • Import confusion (which one to use?)
  • Violates Single Source of Truth principle

Recommendation:

  • Keep: packages/domain/common/validation.ts (primary validation module)
  • Remove: packages/domain/toolkit/validation/helpers.ts export
  • Alternative: Re-export from common/validation if toolkit needs it

3. Email Validation Duplication (Priority: HIGH)

Problem: Two different implementations of isValidEmail():

Location Implementation Validation Quality
packages/domain/common/validation.ts:69 Uses Zod .email() Robust (RFC 5322)
packages/domain/toolkit/validation/email.ts:10 Manual regex /^[^\s@]+@[^\s@]+\.[^\s@]+$/ ⚠️ Basic (misses edge cases)

Impact:

  • Zod's .email() is more comprehensive and battle-tested
  • Manual regex misses many RFC 5322 edge cases
  • Risk of accepting invalid emails or rejecting valid ones

Recommendation:

// ❌ Remove basic regex version from toolkit/validation/email.ts
export function isValidEmail(email: string): boolean {
  const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
  return emailRegex.test(email);
}

// ✅ Keep Zod-based version in common/validation.ts
export function isValidEmail(email: string): boolean {
  return z.string().email().safeParse(email).success;
}

4. URL Validation Duplication (Priority: MEDIUM)

Problem: Three overlapping implementations:

Location Functions Notes
packages/domain/common/validation.ts urlSchema, validateUrlOrThrow, validateUrl, isValidUrl Comprehensive (Zod + helpers)
packages/domain/toolkit/validation/url.ts isValidUrl, ensureProtocol, getHostname Mixed (validation + utilities)
packages/domain/toolkit/validation/helpers.ts validateUrl, isValidHttpUrl Partial duplication

Specific Duplications:

  • isValidUrl exists in both common/validation.ts:119 and toolkit/validation/url.ts:10
  • validateUrl exists in both common/validation.ts:107 and toolkit/validation/helpers.ts:169

Impact:

  • Import confusion
  • Inconsistent validation (URL constructor vs Zod)

Recommendation:

// ✅ Keep in common/validation.ts (schema + validation)
export const urlSchema = z.string().url();
export function isValidUrl(url: string): boolean { /* ... */ }
export function validateUrlOrThrow(url: string): string { /* ... */ }

// ✅ Keep in toolkit/validation/url.ts (utility functions only)
export function ensureProtocol(url: string): string { /* ... */ }
export function getHostname(url: string): string | null { /* ... */ }

// ❌ Remove duplicates from toolkit/validation/helpers.ts

5. Pagination Schema Duplication (Priority: LOW)

Problem: Two pagination implementations:

Location Export Usage
packages/domain/common/schema.ts:99 paginationParamsSchema Used in domain schemas
packages/domain/toolkit/validation/helpers.ts:207 createPaginationSchema() Utility function with options

Analysis:

  • paginationParamsSchema has fixed defaults (page=1, limit=20, max=100)
  • createPaginationSchema() allows customizable min/max/default limits

Recommendation:

  • Keep both but clarify their purposes:
    • paginationParamsSchema → Standard pagination (most use cases)
    • createPaginationSchema() → Custom pagination (special cases)
  • Document when to use each in VALIDATION_PATTERNS.md

🟡 Architectural Concerns

6. Business Logic in BFF Service (Priority: MEDIUM)

File: apps/bff/src/modules/subscriptions/sim-management/services/sim-validation.service.ts

Problem: Contains hardcoded business rules that should be in domain:

// Hardcoded fallback SIM number
if (!account) {
  account = "02000331144508";
  this.logger.warn(`No SIM account identifier found, using test SIM: ${account}`);
}

Issues:

  • Magic numbers in BFF layer (should be in domain or config)
  • Business logic for extracting SIM account from custom fields (lines 110-195)
  • Hardcoded field name mappings for WHMCS

Impact:

  • Difficult to test in isolation
  • Cannot reuse logic if needed elsewhere
  • Violates domain-driven design principles

Recommendation:

  1. Move SIM account extraction logic to domain:

    packages/domain/sim/validation.ts
    - extractSimAccountFromSubscription()
    - getSimAccountFromCustomFields()
    
  2. Move field name mappings to domain contracts:

    packages/domain/sim/contract.ts
    - SIM_ACCOUNT_FIELD_NAMES constant
    
  3. Keep only infrastructure concerns in BFF:

    • Database calls
    • HTTP requests
    • Logging

🟢 Good Practices Observed

Proper Validation Patterns

  1. Controllers use ZodValidationPipe (e.g., orders.controller.ts:21, auth.controller.ts:125)

    @UsePipes(new ZodValidationPipe(createOrderRequestSchema))
    
  2. Domain schemas are reused (no duplication in controllers)

    import { createOrderRequestSchema } from "@customer-portal/domain/orders";
    
  3. Business validation separated from format validation:

    • Format: orderBusinessValidationSchema (domain)
    • Business: OrderValidator.validateBusinessRules() (BFF service)
  4. Sanitization functions don't perform validation (e.g., mappings/validation.ts:106-112)


📋 Summary of Recommendations

Immediate Actions (High Priority)

  1. Remove duplicate isValidUuid from toolkit/validation/helpers.ts

    • Keep only the Zod-based version in common/validation.ts
  2. Remove duplicate uuidSchema export from toolkit/validation/helpers.ts

    • Use common/validation.ts as single source
  3. Remove duplicate isValidEmail from toolkit/validation/email.ts

    • Keep only the Zod-based version in common/validation.ts
  4. Consolidate URL validation:

    • Remove isValidUrl and validateUrl from toolkit/validation/helpers.ts
    • Keep validation in common/validation.ts
    • Keep only utility functions in toolkit/validation/url.ts

Medium Priority

  1. Move SIM business logic to domain

    • Extract SIM account extraction to packages/domain/sim/validation.ts
    • Move field mappings to domain constants
  2. Document pagination schema usage

    • Clarify when to use paginationParamsSchema vs createPaginationSchema()

Low Priority

  1. Review toolkit/validation organization
    • Consider if toolkit should only contain utilities (not validation)
    • Maybe rename to toolkit/helpers or toolkit/utils

📁 Validation File Organization

Current Structure (Good)

packages/domain/
├── common/
│   ├── schema.ts         ✅ Shared Zod schemas
│   └── validation.ts     ✅ Shared validation functions
├── orders/
│   ├── schema.ts         ✅ Order-specific schemas
│   └── validation.ts     ✅ Order business validation
├── auth/
│   └── schema.ts         ✅ Auth schemas
├── mappings/
│   ├── schema.ts         ✅ Mapping schemas
│   └── validation.ts     ✅ Mapping business validation
└── toolkit/
    └── validation/       ⚠️ Overlaps with common/validation
        ├── helpers.ts    ⚠️ Duplicates from common/validation
        ├── email.ts      ⚠️ Duplicates isValidEmail
        ├── url.ts        ⚠️ Duplicates URL validation
        └── string.ts     ✅ String utilities (OK)
packages/domain/
├── common/
│   ├── schema.ts         → All shared Zod schemas
│   └── validation.ts     → All shared validation functions
├── [domain]/
│   ├── schema.ts         → Domain-specific schemas
│   └── validation.ts     → Domain business validation
└── toolkit/
    ├── formatting/       → Date, currency, text formatting
    ├── typing/          → Type guards and assertions
    └── utilities/       → Pure utility functions (no validation)
        ├── string.ts    → String manipulation (not validation)
        ├── url.ts       → URL utilities (ensureProtocol, getHostname)
        └── email.ts     → Email utilities (getEmailDomain, normalizeEmail)

Key Principle:

  • Validation (anything that returns boolean or throws errors) → common/validation.ts or domain-specific validation.ts
  • Utilities (transformations, formatting, parsing) → toolkit/utilities/

🎯 Implementation Checklist

  • Remove isValidUuid from toolkit/validation/helpers.ts:23
  • Remove uuidSchema export from toolkit/validation/helpers.ts:202
  • Remove isValidEmail from toolkit/validation/email.ts:10
  • Remove validateUrl from toolkit/validation/helpers.ts:169
  • Remove isValidHttpUrl from toolkit/validation/helpers.ts:181 (or move to common)
  • Remove isValidUrl from toolkit/validation/url.ts:10
  • Move SIM account extraction to packages/domain/sim/validation.ts
  • Document pagination schema usage patterns
  • Update imports across codebase to use common/validation
  • Add tests to verify validation consistency

📝 Notes

Why This Matters

  1. Security: Inconsistent validation can lead to security vulnerabilities
  2. Maintainability: Multiple implementations mean multiple places to update
  3. Testing: Easier to test validation when it's in one place
  4. Developer Experience: Clear where to find and how to use validation

Previous Cleanup Efforts

The codebase has already undergone significant validation cleanup:

  • Password validation duplication removed
  • SKU validation consolidated
  • Sanitization functions simplified
  • ZodValidationPipe adopted in controllers

This audit builds on that work to eliminate remaining overlaps.


🔍 Testing Recommendations

After implementing changes, verify:

  1. All imports still resolve:

    pnpm build
    
  2. No validation regressions:

    pnpm test:unit
    pnpm test:e2e
    
  3. Consistent validation behavior:

    • UUID validation rejects malformed UUIDs
    • Email validation handles edge cases
    • URL validation handles relative URLs correctly

📚 References