Address code review feedback: fix variant updates, security validations

Copilot · AshrafAbir · Copilot · commit 3730bd24f1b5 · 2025-11-26T18:25:52.000Z
Co-authored-by: AshrafAbir &lt;88766326+AshrafAbir@users.noreply.github.com&gt;
diff --git a/src/app/api/products/import/route.ts b/src/app/api/products/import/route.ts
@@ -5,6 +5,28 @@ import { NextRequest, NextResponse } from 'next/server';
 import { getServerSession } from 'next-auth/next';
 import { authOptions } from '@/lib/auth';
 import { ProductService } from '@/lib/services/product.service';
+import { z } from 'zod';
+
+// Zod schema for CSV record validation
+const csvRecordSchema = z.object({
+  name: z.string().min(1, 'Name is required'),
+  sku: z.string().min(1, 'SKU is required'),
+  price: z.union([z.string(), z.number()]).transform((val) => {
+    const num = typeof val === 'string' ? parseFloat(val) : val;
+    if (isNaN(num) || num < 0) throw new Error('Invalid price');
+    return num;
+  }),
+  description: z.string().optional(),
+  categoryId: z.string().optional(),
+  brandId: z.string().optional(),
+  inventoryQty: z.union([z.string(), z.number()]).optional().transform((val) => {
+    if (val === undefined || val === '') return 0;
+    const num = typeof val === 'string' ? parseInt(val) : val;
+    return isNaN(num) ? 0 : Math.max(0, num);
+  }),
+  status: z.string().optional(),
+  images: z.string().optional(),
+}).passthrough(); // Allow additional columns
 
 // Simple CSV parser (handles quoted fields and commas within quotes)
 function parseCSV(text: string): Array<Record<string, string>> {
@@ -127,10 +149,8 @@ export async function POST(request: NextRequest) {
       );
     }
 
-    // Import products
-    const productService = ProductService.getInstance();
-    // Cast records to the expected type - validation happens in bulkImport
-    const result = await productService.bulkImport(storeId, records as Array<{
+    // Validate and transform records using Zod schema
+    const validatedRecords: Array<{
       name: string;
       sku: string;
       price: number | string;
@@ -140,13 +160,47 @@ export async function POST(request: NextRequest) {
       inventoryQty?: number | string;
       status?: string;
       images?: string;
-    }>);
+    }> = [];
+    const validationErrors: Array<{ row: number; error: string }> = [];
+
+    for (let i = 0; i < records.length; i++) {
+      const result = csvRecordSchema.safeParse(records[i]);
+      if (result.success) {
+        validatedRecords.push(result.data);
+      } else {
+        // Zod 4 uses issues instead of errors
+        const errorMessages = result.error.issues 
+          ? result.error.issues.map((issue: { message: string }) => issue.message).join(', ')
+          : 'Validation failed';
+        validationErrors.push({
+          row: i + 1,
+          error: errorMessages,
+        });
+      }
+    }
+
+    // If all records failed validation, return early
+    if (validatedRecords.length === 0) {
+      return NextResponse.json({
+        success: false,
+        imported: 0,
+        total: records.length,
+        errors: validationErrors,
+      });
+    }
+
+    // Import validated products
+    const productService = ProductService.getInstance();
+    const result = await productService.bulkImport(storeId, validatedRecords);
+
+    // Combine validation errors with import errors
+    const allErrors = [...validationErrors, ...result.errors];
 
     return NextResponse.json({
-      success: true,
+      success: result.imported > 0,
       imported: result.imported,
       total: records.length,
-      errors: result.errors,
+      errors: allErrors,
     });
   } catch (error) {
     console.error('POST /api/products/import error:', error);
diff --git a/src/app/api/products/upload/route.ts b/src/app/api/products/upload/route.ts
@@ -22,12 +22,20 @@ const ALLOWED_TYPES = [
   'image/svg+xml',
 ];
 
+// Validate storeId to prevent directory traversal attacks
+function isValidStoreId(storeId: string): boolean {
+  // storeId should be alphanumeric with optional hyphens (cuid format)
+  return /^[a-zA-Z0-9-_]+$/.test(storeId) && storeId.length <= 100;
+}
+
 // Generate a unique filename
 function generateUniqueFileName(originalName: string): string {
   const timestamp = Date.now();
   const random = Math.random().toString(36).substring(2, 8);
   const extension = originalName.split('.').pop()?.toLowerCase() || 'jpg';
-  return `${timestamp}-${random}.${extension}`;
+  // Sanitize extension to prevent path traversal
+  const safeExtension = extension.replace(/[^a-z0-9]/g, '');
+  return `${timestamp}-${random}.${safeExtension || 'jpg'}`;
 }
 
 // POST /api/products/upload - Upload product image
@@ -61,6 +69,14 @@ export async function POST(request: NextRequest) {
       );
     }
 
+    // Validate storeId to prevent directory traversal attacks
+    if (!isValidStoreId(storeId)) {
+      return NextResponse.json(
+        { error: 'Invalid storeId format' },
+        { status: 400 }
+      );
+    }
+
     // Validate file size
     if (file.size > MAX_FILE_SIZE) {
       return NextResponse.json(
@@ -145,6 +161,14 @@ export async function PUT(request: NextRequest) {
       );
     }
 
+    // Validate storeId to prevent directory traversal attacks
+    if (!isValidStoreId(storeId)) {
+      return NextResponse.json(
+        { error: 'Invalid storeId format' },
+        { status: 400 }
+      );
+    }
+
     // Limit number of files per upload
     if (files.length > 10) {
       return NextResponse.json(
diff --git a/src/lib/services/product.service.ts b/src/lib/services/product.service.ts
@@ -107,7 +107,7 @@ export const createProductSchema = z.object({
   metaKeywords: z.string().optional().nullable(),
   status: z.nativeEnum(ProductStatus).default(ProductStatus.DRAFT),
   isFeatured: z.boolean().default(false),
-  // Variants support (min 1, max 100)
+  // Variants support (min 0, max 100)
   variants: z.array(variantSchema).min(0).max(100).optional(),
 });
 
@@ -582,42 +582,55 @@ export class ProductService {
         .filter(v => !variantIdsToKeep.has(v.id))
         .map(v => v.id);
 
-      // Build the variant update operations
-      updateData.variants = {
+      // Use a transaction to handle variant updates individually
+      await prisma.$transaction(async (tx) => {
         // Delete variants that are no longer in the list
-        deleteMany: variantIdsToDelete.length > 0 ? { id: { in: variantIdsToDelete } } : undefined,
-        // Update existing variants
-        updateMany: variantsToUpdate.map(variant => ({
-          where: { id: variant.id! },
-          data: {
-            name: variant.name,
-            sku: variant.sku,
-            barcode: variant.barcode,
-            price: variant.price,
-            compareAtPrice: variant.compareAtPrice,
-            inventoryQty: variant.inventoryQty ?? 0,
-            lowStockThreshold: variant.lowStockThreshold ?? 5,
-            weight: variant.weight,
-            image: variant.image,
-            options: typeof variant.options === 'string' ? variant.options : JSON.stringify(variant.options),
-            isDefault: variant.isDefault ?? false,
-          },
-        })),
+        if (variantIdsToDelete.length > 0) {
+          await tx.productVariant.deleteMany({
+            where: { id: { in: variantIdsToDelete } },
+          });
+        }
+
+        // Update existing variants individually (updateMany can't handle different data per record)
+        for (const variant of variantsToUpdate) {
+          await tx.productVariant.update({
+            where: { id: variant.id! },
+            data: {
+              name: variant.name,
+              sku: variant.sku,
+              barcode: variant.barcode,
+              price: variant.price,
+              compareAtPrice: variant.compareAtPrice,
+              inventoryQty: variant.inventoryQty ?? 0,
+              lowStockThreshold: variant.lowStockThreshold ?? 5,
+              weight: variant.weight,
+              image: variant.image,
+              options: typeof variant.options === 'string' ? variant.options : JSON.stringify(variant.options),
+              isDefault: variant.isDefault ?? false,
+            },
+          });
+        }
+
         // Create new variants
-        create: variantsToCreate.map((variant, index) => ({
-          name: variant.name,
-          sku: variant.sku,
-          barcode: variant.barcode,
-          price: variant.price,
-          compareAtPrice: variant.compareAtPrice,
-          inventoryQty: variant.inventoryQty ?? 0,
-          lowStockThreshold: variant.lowStockThreshold ?? 5,
-          weight: variant.weight,
-          image: variant.image,
-          options: typeof variant.options === 'string' ? variant.options : JSON.stringify(variant.options),
-          isDefault: variant.isDefault ?? (variantsToUpdate.length === 0 && index === 0),
-        })),
-      };
+        if (variantsToCreate.length > 0) {
+          await tx.productVariant.createMany({
+            data: variantsToCreate.map((variant, index) => ({
+              productId,
+              name: variant.name,
+              sku: variant.sku,
+              barcode: variant.barcode,
+              price: variant.price,
+              compareAtPrice: variant.compareAtPrice,
+              inventoryQty: variant.inventoryQty ?? 0,
+              lowStockThreshold: variant.lowStockThreshold ?? 5,
+              weight: variant.weight,
+              image: variant.image,
+              options: typeof variant.options === 'string' ? variant.options : JSON.stringify(variant.options),
+              isDefault: variant.isDefault ?? (variantsToUpdate.length === 0 && index === 0),
+            })),
+          });
+        }
+      });
     }
 
     // Remove id from update data
