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.tsexport - 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:
isValidUrlexists in bothcommon/validation.ts:119andtoolkit/validation/url.ts:10validateUrlexists in bothcommon/validation.ts:107andtoolkit/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:
paginationParamsSchemahas 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:
-
Move SIM account extraction logic to domain:
packages/domain/sim/validation.ts - extractSimAccountFromSubscription() - getSimAccountFromCustomFields() -
Move field name mappings to domain contracts:
packages/domain/sim/contract.ts - SIM_ACCOUNT_FIELD_NAMES constant -
Keep only infrastructure concerns in BFF:
- Database calls
- HTTP requests
- Logging
🟢 Good Practices Observed
✅ Proper Validation Patterns
-
Controllers use ZodValidationPipe (e.g.,
orders.controller.ts:21,auth.controller.ts:125)@UsePipes(new ZodValidationPipe(createOrderRequestSchema)) -
Domain schemas are reused (no duplication in controllers)
import { createOrderRequestSchema } from "@customer-portal/domain/orders"; -
Business validation separated from format validation:
- Format:
orderBusinessValidationSchema(domain) - Business:
OrderValidator.validateBusinessRules()(BFF service)
- Format:
-
Sanitization functions don't perform validation (e.g.,
mappings/validation.ts:106-112)
📋 Summary of Recommendations
Immediate Actions (High Priority)
-
Remove duplicate
isValidUuidfrom toolkit/validation/helpers.ts- Keep only the Zod-based version in common/validation.ts
-
Remove duplicate
uuidSchemaexport from toolkit/validation/helpers.ts- Use common/validation.ts as single source
-
Remove duplicate
isValidEmailfrom toolkit/validation/email.ts- Keep only the Zod-based version in common/validation.ts
-
Consolidate URL validation:
- Remove
isValidUrlandvalidateUrlfrom toolkit/validation/helpers.ts - Keep validation in common/validation.ts
- Keep only utility functions in toolkit/validation/url.ts
- Remove
Medium Priority
-
Move SIM business logic to domain
- Extract SIM account extraction to
packages/domain/sim/validation.ts - Move field mappings to domain constants
- Extract SIM account extraction to
-
Document pagination schema usage
- Clarify when to use
paginationParamsSchemavscreatePaginationSchema()
- Clarify when to use
Low Priority
- Review toolkit/validation organization
- Consider if toolkit should only contain utilities (not validation)
- Maybe rename to
toolkit/helpersortoolkit/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)
Recommended Structure
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
booleanor throws errors) →common/validation.tsor domain-specificvalidation.ts - Utilities (transformations, formatting, parsing) →
toolkit/utilities/
🎯 Implementation Checklist
- Remove
isValidUuidfromtoolkit/validation/helpers.ts:23 - Remove
uuidSchemaexport fromtoolkit/validation/helpers.ts:202 - Remove
isValidEmailfromtoolkit/validation/email.ts:10 - Remove
validateUrlfromtoolkit/validation/helpers.ts:169 - Remove
isValidHttpUrlfromtoolkit/validation/helpers.ts:181(or move to common) - Remove
isValidUrlfromtoolkit/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
- Security: Inconsistent validation can lead to security vulnerabilities
- Maintainability: Multiple implementations mean multiple places to update
- Testing: Easier to test validation when it's in one place
- 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:
-
All imports still resolve:
pnpm build -
No validation regressions:
pnpm test:unit pnpm test:e2e -
Consistent validation behavior:
- UUID validation rejects malformed UUIDs
- Email validation handles edge cases
- URL validation handles relative URLs correctly
📚 References
- VALIDATION_PATTERNS.md - Existing validation guidelines
- VALIDATION_CLEANUP_SUMMARY.md - Previous cleanup work
- Domain README: packages/domain/README.md