fix: 14 bug-scan findings across persistence, engine, and API routes

src/app/api/auth/register/__tests__/route.test.ts

@@ -0,0 +1,64 @@
+/**
+ * Tests for /api/auth/register input validation.
+ *
+ * The route previously accepted any non-empty password (even 1 character)
+ * and any string containing '@' as an "email", with no format check.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const { prismaMock, bcryptMock } = vi.hoisted(() => ({
+  prismaMock: {
+    user: {
+      findUnique: vi.fn(),
+      create: vi.fn(),
+    },
+  },
+  bcryptMock: {
+    hash: vi.fn(),
+  },
+}));
+
+vi.mock('@/lib/prisma', () => ({
+  prisma: prismaMock,
+}));
+
+vi.mock('bcryptjs', () => ({
+  default: bcryptMock,
+}));
+
+import { POST } from '../route';
+
+function makeRequest(body: unknown): Request {
+  return new Request('http://localhost/api/auth/register', {
+    method: 'POST',
+    headers: { 'content-type': 'application/json' },
+    body: JSON.stringify(body),
+  });
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  prismaMock.user.findUnique.mockResolvedValue(null);
+  prismaMock.user.create.mockResolvedValue({ id: 'user-1' });
+  bcryptMock.hash.mockResolvedValue('hashed');
+});
+
+describe('POST /api/auth/register', () => {
+  it('rejects a password shorter than 8 characters', async () => {
+    const res = await POST(makeRequest({ email: 'a@b.com', password: '1' }));
+    expect(res.status).toBe(400);
+    expect(prismaMock.user.create).not.toHaveBeenCalled();
+  });
+
+  it('rejects a malformed email', async () => {
+    const res = await POST(makeRequest({ email: 'not-an-email', password: 'longenough' }));
+    expect(res.status).toBe(400);
+    expect(prismaMock.user.create).not.toHaveBeenCalled();
+  });
+
+  it('accepts a valid email and an 8+ character password', async () => {
+    const res = await POST(makeRequest({ email: 'a@b.com', password: 'longenough' }));
+    expect(res.status).toBe(201);
+    expect(prismaMock.user.create).toHaveBeenCalledTimes(1);
+  });
+});

src/app/api/auth/register/route.ts

@@ -13,6 +13,20 @@ export async function POST(request: Request) {
       );
     }
 
+    if (typeof email !== 'string' || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) {
+      return NextResponse.json(
+        { error: 'Enter a valid email address' },
+        { status: 400 }
+      );
+    }
+
+    if (typeof password !== 'string' || password.length < 8) {
+      return NextResponse.json(
+        { error: 'Password must be at least 8 characters' },
+        { status: 400 }
+      );
+    }
+
     const existingUser = await prisma.user.findUnique({
       where: { email },
     });

src/app/api/campaigns/[id]/__tests__/route.test.ts

@@ -0,0 +1,88 @@
+/**
+ * Tests for /api/campaigns/[id] ownership scoping.
+ *
+ * PUT and DELETE verified ownership via a separate `findFirst({ id, userId })`
+ * check, but the mutating `update`/`delete` calls used `where: { id }` alone —
+ * a defense-in-depth gap where any future refactor that separates the check
+ * from the mutation could let one user modify another user's campaign by id.
+ * The fix scopes the mutation itself by `userId` too.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const { prismaMock, authMock } = vi.hoisted(() => {
+  const prismaMock = {
+    campaign: {
+      findFirst: vi.fn(),
+      update: vi.fn(),
+      delete: vi.fn(),
+    },
+  };
+  const authMock = vi.fn();
+  return { prismaMock, authMock };
+});
+
+vi.mock('@/lib/prisma', () => ({
+  prisma: prismaMock,
+}));
+
+vi.mock('@/lib/auth', () => ({
+  auth: authMock,
+}));
+
+import { PUT, DELETE } from '../route';
+
+const USER = { user: { id: 'user-1' } };
+const params = Promise.resolve({ id: 'campaign-1' });
+
+function putRequest(body: unknown): Request {
+  return new Request('http://localhost/api/campaigns/campaign-1', {
+    method: 'PUT',
+    headers: { 'content-type': 'application/json' },
+    body: JSON.stringify(body),
+  });
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  authMock.mockResolvedValue(USER);
+  prismaMock.campaign.findFirst.mockResolvedValue({ id: 'campaign-1', userId: 'user-1' });
+  prismaMock.campaign.update.mockResolvedValue({ id: 'campaign-1' });
+  prismaMock.campaign.delete.mockResolvedValue({ id: 'campaign-1' });
+});
+
+describe('PUT /api/campaigns/[id]', () => {
+  it('scopes the update to the authenticated user, not just the ownership check', async () => {
+    await PUT(putRequest({ name: 'Renamed' }), { params });
+    expect(prismaMock.campaign.update).toHaveBeenCalledWith(
+      expect.objectContaining({ where: { id: 'campaign-1', userId: 'user-1' } }),
+    );
+  });
+
+  it('rejects unauthenticated requests with 401 and never touches the DB', async () => {
+    authMock.mockResolvedValueOnce(null);
+    const res = await PUT(putRequest({ name: 'x' }), { params });
+    expect(res.status).toBe(401);
+    expect(prismaMock.campaign.update).not.toHaveBeenCalled();
+  });
+
+  it('returns 404 when the campaign is not owned by the caller', async () => {
+    prismaMock.campaign.findFirst.mockResolvedValueOnce(null);
+    const res = await PUT(putRequest({ name: 'x' }), { params });
+    expect(res.status).toBe(404);
+    expect(prismaMock.campaign.update).not.toHaveBeenCalled();
+  });
+});
+
+describe('DELETE /api/campaigns/[id]', () => {
+  it('scopes the delete to the authenticated user, not just the ownership check', async () => {
+    await DELETE(new Request('http://localhost/api/campaigns/campaign-1', { method: 'DELETE' }), { params });
+    expect(prismaMock.campaign.delete).toHaveBeenCalledWith({ where: { id: 'campaign-1', userId: 'user-1' } });
+  });
+
+  it('returns 404 when the campaign is not owned by the caller', async () => {
+    prismaMock.campaign.findFirst.mockResolvedValueOnce(null);
+    const res = await DELETE(new Request('http://localhost/api/campaigns/campaign-1', { method: 'DELETE' }), { params });
+    expect(res.status).toBe(404);
+    expect(prismaMock.campaign.delete).not.toHaveBeenCalled();
+  });
+});

src/app/api/campaigns/[id]/route.ts

@@ -1,6 +1,7 @@
 import { NextResponse } from 'next/server';
 import { prisma } from '@/lib/prisma';
 import { auth } from '@/lib/auth';
+import { safeJsonParse } from '@/lib/json';
 
 export async function GET(
   request: Request,
@@ -23,19 +24,20 @@ export async function GET(
     return NextResponse.json({ error: 'Not found' }, { status: 404 });
   }
 
-  // Parse JSON fields
+  // Parse JSON fields — a corrupted value falls back to an empty default
+  // instead of throwing and 500ing the request.
   const parsed = {
     ...campaign,
-    placements: campaign.placements ? JSON.parse(campaign.placements) : null,
-    products: campaign.products ? JSON.parse(campaign.products) : [],
-    creative: campaign.creative ? JSON.parse(campaign.creative) : null,
-    metrics: campaign.metrics ? JSON.parse(campaign.metrics) : null,
-    adGroups: campaign.adGroups ? JSON.parse(campaign.adGroups) : [],
-    targets: campaign.targets ? JSON.parse(campaign.targets) : [],
-    searchTerms: campaign.searchTerms ? JSON.parse(campaign.searchTerms) : [],
-    negatives: campaign.negatives ? JSON.parse(campaign.negatives) : [],
-    budgetRules: campaign.budgetRules ? JSON.parse(campaign.budgetRules) : [],
-    history: campaign.history ? JSON.parse(campaign.history) : [],
+    placements: safeJsonParse(campaign.placements, null),
+    products: safeJsonParse(campaign.products, []),
+    creative: safeJsonParse(campaign.creative, null),
+    metrics: safeJsonParse(campaign.metrics, null),
+    adGroups: safeJsonParse(campaign.adGroups, []),
+    targets: safeJsonParse(campaign.targets, []),
+    searchTerms: safeJsonParse(campaign.searchTerms, []),
+    negatives: safeJsonParse(campaign.negatives, []),
+    budgetRules: safeJsonParse(campaign.budgetRules, []),
+    history: safeJsonParse(campaign.history, []),
   };
 
   return NextResponse.json(parsed);
@@ -86,7 +88,7 @@ export async function PUT(
   }
 
   const campaign = await prisma.campaign.update({
-    where: { id },
+    where: { id, userId: session.user.id },
     data: updateData,
   });
 
@@ -116,7 +118,7 @@ export async function DELETE(
   }
 
   await prisma.campaign.delete({
-    where: { id },
+    where: { id, userId: session.user.id },
   });
 
   return NextResponse.json({ message: 'Deleted' });

src/app/api/campaigns/route.ts

@@ -1,6 +1,7 @@
 import { NextResponse } from 'next/server';
 import { prisma } from '@/lib/prisma';
 import { auth } from '@/lib/auth';
+import { safeJsonParse } from '@/lib/json';
 
 export async function GET() {
   const session = await auth();
@@ -13,19 +14,20 @@ export async function GET() {
     orderBy: { createdAt: 'desc' },
   });
 
-  // Parse JSON fields
+  // Parse JSON fields — a corrupted value in one row falls back to an
+  // empty default instead of throwing and 500ing the entire list.
   const parsed = campaigns.map((c: any) => ({
     ...c,
-    placements: c.placements ? JSON.parse(c.placements) : null,
-    products: c.products ? JSON.parse(c.products) : [],
-    creative: c.creative ? JSON.parse(c.creative) : null,
-    metrics: c.metrics ? JSON.parse(c.metrics) : null,
-    adGroups: c.adGroups ? JSON.parse(c.adGroups) : [],
-    targets: c.targets ? JSON.parse(c.targets) : [],
-    searchTerms: c.searchTerms ? JSON.parse(c.searchTerms) : [],
-    negatives: c.negatives ? JSON.parse(c.negatives) : [],
-    budgetRules: c.budgetRules ? JSON.parse(c.budgetRules) : [],
-    history: c.history ? JSON.parse(c.history) : [],
+    placements: safeJsonParse(c.placements, null),
+    products: safeJsonParse(c.products, []),
+    creative: safeJsonParse(c.creative, null),
+    metrics: safeJsonParse(c.metrics, null),
+    adGroups: safeJsonParse(c.adGroups, []),
+    targets: safeJsonParse(c.targets, []),
+    searchTerms: safeJsonParse(c.searchTerms, []),
+    negatives: safeJsonParse(c.negatives, []),
+    budgetRules: safeJsonParse(c.budgetRules, []),
+    history: safeJsonParse(c.history, []),
   }));
 
   return NextResponse.json(parsed);

src/app/api/sync/__tests__/route.test.ts

@@ -220,4 +220,66 @@ describe('GET /api/sync', () => {
     const res = await GET();
     expect(res.status).toBe(401);
   });
+
+  it('falls back to empty defaults for a row with corrupted JSON instead of 500ing the whole list', async () => {
+    prismaMock.campaign.findMany.mockResolvedValueOnce([
+      {
+        campaignId: 'corrupted',
+        type: 'SP',
+        name: 'Corrupted Row',
+        portfolio: null,
+        status: 'Enabled',
+        dailyBudget: 25,
+        defaultBid: 0.75,
+        startDate: null,
+        endDate: null,
+        targetingMode: null,
+        adFormat: null,
+        campaignGoal: null,
+        bidStrategy: null,
+        placements: '{not valid json',
+        products: null,
+        creative: null,
+        metrics: null,
+        adGroups: '{also not valid',
+        targets: null,
+        searchTerms: null,
+        negatives: null,
+        budgetRules: null,
+        history: null,
+      },
+      {
+        campaignId: 'healthy',
+        type: 'SP',
+        name: 'Healthy Row',
+        portfolio: null,
+        status: 'Enabled',
+        dailyBudget: 25,
+        defaultBid: 0.75,
+        startDate: null,
+        endDate: null,
+        targetingMode: null,
+        adFormat: null,
+        campaignGoal: null,
+        bidStrategy: null,
+        placements: null,
+        products: null,
+        creative: null,
+        metrics: null,
+        adGroups: null,
+        targets: null,
+        searchTerms: null,
+        negatives: null,
+        budgetRules: null,
+        history: null,
+      },
+    ]);
+    const res = await GET();
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    expect(body).toHaveLength(2);
+    expect(body[0].placements).toEqual({ top: 0, product: 0, rest: 0 });
+    expect(body[0].adGroups).toEqual([]);
+    expect(body[1].id).toBe('healthy');
+  });
 });

src/app/api/sync/route.ts

@@ -2,6 +2,7 @@ import { NextResponse } from 'next/server';
 import type { Prisma } from '@/generated/prisma/client';
 import { prisma } from '@/lib/prisma';
 import { auth } from '@/lib/auth';
+import { safeJsonParse } from '@/lib/json';
 
 /**
  * Wire shape of a campaign as the browser sends it in the sync payload.
@@ -133,7 +134,8 @@ export async function GET() {
     orderBy: { createdAt: 'desc' },
   });
 
-  // Parse JSON fields
+  // Parse JSON fields — a corrupted value in one row falls back to an
+  // empty default instead of throwing and 500ing the entire list.
   const parsed = campaigns.map((c: any) => ({
     id: c.campaignId,
     type: c.type,
@@ -148,16 +150,16 @@ export async function GET() {
     adFormat: c.adFormat,
     campaignGoal: c.campaignGoal,
     bidStrategy: c.bidStrategy,
-    placements: c.placements ? JSON.parse(c.placements) : { top: 0, product: 0, rest: 0 },
-    products: c.products ? JSON.parse(c.products) : [],
-    creative: c.creative ? JSON.parse(c.creative) : null,
-    metrics: c.metrics ? JSON.parse(c.metrics) : { impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 },
-    adGroups: c.adGroups ? JSON.parse(c.adGroups) : [],
-    targets: c.targets ? JSON.parse(c.targets) : [],
-    searchTerms: c.searchTerms ? JSON.parse(c.searchTerms) : [],
-    negatives: c.negatives ? JSON.parse(c.negatives) : [],
-    budgetRules: c.budgetRules ? JSON.parse(c.budgetRules) : [],
-    history: c.history ? JSON.parse(c.history) : [],
+    placements: safeJsonParse(c.placements, { top: 0, product: 0, rest: 0 }),
+    products: safeJsonParse(c.products, []),
+    creative: safeJsonParse(c.creative, null),
+    metrics: safeJsonParse(c.metrics, { impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }),
+    adGroups: safeJsonParse(c.adGroups, []),
+    targets: safeJsonParse(c.targets, []),
+    searchTerms: safeJsonParse(c.searchTerms, []),
+    negatives: safeJsonParse(c.negatives, []),
+    budgetRules: safeJsonParse(c.budgetRules, []),
+    history: safeJsonParse(c.history, []),
   }));
 
   return NextResponse.json(parsed);