improved org security (#146)

src/__tests__/multitenancy/customer-inventory-isolation.test.ts

@@ -193,16 +193,14 @@ describe("getCustomers — org scoping", () => {
 // ---------------------------------------------------------------------------
 
 describe("updateCustomer — cross-org isolation", () => {
-  it("🐛 BUG-3: silently returns success when targeting another org's customer — nothing is actually changed", async () => {
+  it("returns error when targeting another org's customer", async () => {
     setupOrgAOwner();
     // updateMany with org filter matches 0 rows for a cross-org customer id
     vi.mocked(db.customer.updateMany).mockResolvedValue({ count: 0 } as any);
 
     const result = await updateCustomer({ id: `${ORG_B}-customer-id`, name: "Hacked" });
 
-    // BUG: should be { success: false, error: "Customer not found" }
-    // Fix: check `result.count === 0` and throw (same pattern as inventoryActions).
-    expect(result.success).toBe(false); // FAILS until bug is fixed
+    expect(result.success).toBe(false);
   });
 
   it("update query always includes organizationId to prevent cross-org writes", async () => {
@@ -233,16 +231,14 @@ describe("updateCustomer — cross-org isolation", () => {
 // ---------------------------------------------------------------------------
 
 describe("deleteCustomer — cross-org isolation", () => {
-  it("🐛 BUG-4: silently returns success when deleting another org's customer — nothing is actually deleted", async () => {
+  it("returns error when deleting another org's customer", async () => {
     setupOrgAOwner();
     // deleteMany with org filter matches 0 rows for a cross-org customer id
     vi.mocked(db.customer.deleteMany).mockResolvedValue({ count: 0 } as any);
 
     const result = await deleteCustomer(`${ORG_B}-customer-id`);
 
-    // BUG: should be { success: false, error: "Customer not found" }
-    // Fix: check `result.count === 0` and throw (same pattern as inventoryActions).
-    expect(result.success).toBe(false); // FAILS until bug is fixed
+    expect(result.success).toBe(false);
   });
 
   it("delete query always includes organizationId to prevent cross-org deletes", async () => {

src/__tests__/multitenancy/inspection-isolation.test.ts

@@ -0,0 +1,192 @@
+/**
+ * Multi-tenancy isolation tests: Inspections
+ *
+ * Verifies that a user from Org A cannot read, update, or delete inspections
+ * belonging to Org B.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+vi.mock("@/lib/cached-session", () => ({
+  getCachedSession: vi.fn(),
+  getCachedMembership: vi.fn(),
+}));
+
+vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
+
+vi.mock("@/lib/notification-bus", () => ({
+  notificationBus: { emit: vi.fn() },
+}));
+
+vi.mock("@/lib/db", () => ({
+  db: {
+    user: { findUnique: vi.fn() },
+    inspection: {
+      findFirst: vi.fn(),
+      updateMany: vi.fn(),
+      deleteMany: vi.fn(),
+    },
+    inspectionItem: {
+      findFirst: vi.fn(),
+      update: vi.fn(),
+    },
+  },
+}));
+
+import { getCachedSession, getCachedMembership } from "@/lib/cached-session";
+import { db } from "@/lib/db";
+import {
+  getInspection,
+  updateInspectionItem,
+  completeInspection,
+  deleteInspection,
+} from "@/features/inspections/Actions/inspectionActions";
+
+const mockSession = vi.mocked(getCachedSession);
+const mockMembership = vi.mocked(getCachedMembership);
+const mockUserFindUnique = vi.mocked(db.user.findUnique);
+
+const ORG_A = "org-a";
+const ORG_B = "org-b";
+
+function setupOrgAOwner() {
+  mockSession.mockResolvedValue({ user: { id: "user-a", email: "a@example.com" } } as any);
+  mockMembership.mockResolvedValue({
+    organizationId: ORG_A,
+    role: "owner",
+    roleId: null,
+    customRole: null,
+  } as any);
+  mockUserFindUnique.mockResolvedValue({ isSuperAdmin: false } as any);
+}
+
+beforeEach(() => {
+  vi.resetAllMocks();
+});
+
+describe("getInspection — cross-org isolation", () => {
+  it("returns error when requesting another org's inspection", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue(null);
+
+    const result = await getInspection(`${ORG_B}-inspection-id`);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Inspection not found");
+  });
+
+  it("read query is scoped to organizationId", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue({
+      id: "insp-a",
+      organizationId: ORG_A,
+    } as any);
+
+    await getInspection("insp-a");
+
+    expect(vi.mocked(db.inspection.findFirst)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+});
+
+describe("updateInspectionItem — cross-org isolation", () => {
+  it("returns error when targeting another org's inspection item", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspectionItem.findFirst).mockResolvedValue(null);
+
+    const result = await updateInspectionItem(`${ORG_B}-item-id`, {
+      condition: "fail",
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Inspection item not found");
+  });
+
+  it("update query includes organizationId via inspection relation", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspectionItem.findFirst).mockResolvedValue({ id: "item-a" } as any);
+    vi.mocked(db.inspectionItem.update).mockResolvedValue({ id: "item-a" } as any);
+
+    await updateInspectionItem("item-a", { condition: "pass" });
+
+    expect(vi.mocked(db.inspectionItem.findFirst)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({
+          inspection: expect.objectContaining({ organizationId: ORG_A }),
+        }),
+      })
+    );
+  });
+
+  it("successfully updates the caller's own inspection item", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspectionItem.findFirst).mockResolvedValue({ id: "item-a" } as any);
+    vi.mocked(db.inspectionItem.update).mockResolvedValue({ id: "item-a", condition: "pass" } as any);
+
+    const result = await updateInspectionItem("item-a", { condition: "pass" });
+
+    expect(result.success).toBe(true);
+  });
+});
+
+describe("completeInspection — cross-org isolation", () => {
+  it("returns error when targeting another org's inspection", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue(null);
+
+    const result = await completeInspection(`${ORG_B}-inspection-id`);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Inspection not found");
+  });
+
+  it("update query includes organizationId", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue({
+      id: "insp-a",
+      vehicleId: "veh-a",
+      organizationId: ORG_A,
+    } as any);
+    vi.mocked(db.inspection.updateMany).mockResolvedValue({ count: 1 } as any);
+
+    await completeInspection("insp-a");
+
+    expect(vi.mocked(db.inspection.updateMany)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+});
+
+describe("deleteInspection — cross-org isolation", () => {
+  it("returns error when deleting another org's inspection", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue(null);
+
+    const result = await deleteInspection(`${ORG_B}-inspection-id`);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Inspection not found");
+  });
+
+  it("delete query includes organizationId", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.inspection.findFirst).mockResolvedValue({
+      id: "insp-a",
+      vehicleId: "veh-a",
+      organizationId: ORG_A,
+    } as any);
+    vi.mocked(db.inspection.deleteMany).mockResolvedValue({ count: 1 } as any);
+
+    await deleteInspection("insp-a");
+
+    expect(vi.mocked(db.inspection.deleteMany)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+});

src/__tests__/multitenancy/labor-preset-isolation.test.ts

@@ -0,0 +1,196 @@
+/**
+ * Multi-tenancy isolation tests: Labor Presets
+ *
+ * Verifies that a user from Org A cannot read, update, or delete labor presets
+ * belonging to Org B.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+vi.mock("@/lib/cached-session", () => ({
+  getCachedSession: vi.fn(),
+  getCachedMembership: vi.fn(),
+}));
+
+vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
+
+vi.mock("@/lib/db", () => ({
+  db: {
+    user: { findUnique: vi.fn() },
+    laborPreset: {
+      findFirst: vi.fn(),
+      findMany: vi.fn(),
+      count: vi.fn(),
+      deleteMany: vi.fn(),
+    },
+    laborPresetItem: {
+      deleteMany: vi.fn(),
+    },
+    $transaction: vi.fn(),
+  },
+}));
+
+import { getCachedSession, getCachedMembership } from "@/lib/cached-session";
+import { db } from "@/lib/db";
+import {
+  getLaborPreset,
+  updateLaborPreset,
+  deleteLaborPreset,
+} from "@/features/labor-presets/Actions/laborPresetActions";
+
+const mockSession = vi.mocked(getCachedSession);
+const mockMembership = vi.mocked(getCachedMembership);
+const mockUserFindUnique = vi.mocked(db.user.findUnique);
+
+const ORG_A = "org-a";
+const ORG_B = "org-b";
+
+function setupOrgAOwner() {
+  mockSession.mockResolvedValue({ user: { id: "user-a", email: "a@example.com" } } as any);
+  mockMembership.mockResolvedValue({
+    organizationId: ORG_A,
+    role: "owner",
+    roleId: null,
+    customRole: null,
+  } as any);
+  mockUserFindUnique.mockResolvedValue({ isSuperAdmin: false } as any);
+}
+
+beforeEach(() => {
+  vi.resetAllMocks();
+});
+
+describe("getLaborPreset — cross-org isolation", () => {
+  it("returns error when requesting another org's preset", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue(null);
+
+    const result = await getLaborPreset(`${ORG_B}-preset-id`);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Preset not found");
+  });
+
+  it("read query is scoped to organizationId", async () => {
+    setupOrgAOwner();
+    const preset = { id: "preset-a", organizationId: ORG_A, name: "Standard Labor", items: [] };
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue(preset as any);
+
+    await getLaborPreset("preset-a");
+
+    expect(vi.mocked(db.laborPreset.findFirst)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+
+  it("returns preset data for the caller's own preset", async () => {
+    setupOrgAOwner();
+    const preset = { id: "preset-a", organizationId: ORG_A, name: "Standard Labor", items: [] };
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue(preset as any);
+
+    const result = await getLaborPreset("preset-a");
+
+    expect(result.success).toBe(true);
+    expect((result.data as any).id).toBe("preset-a");
+  });
+});
+
+describe("updateLaborPreset — cross-org isolation", () => {
+  it("returns error when targeting another org's preset", async () => {
+    setupOrgAOwner();
+    // findFirst with organizationId filter returns null for cross-org preset
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue(null);
+
+    const result = await updateLaborPreset({
+      id: `${ORG_B}-preset-id`,
+      name: "Hacked",
+      items: [{ description: "test", hours: 1, rate: 50 }],
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Preset not found");
+  });
+
+  it("update includes organizationId check via findFirst ownership lookup", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue({ id: "preset-a" } as any);
+    const mockTx = {
+      laborPresetItem: { deleteMany: vi.fn().mockResolvedValue({ count: 0 }) },
+      laborPreset: { update: vi.fn().mockResolvedValue({ id: "preset-a", items: [] }) },
+    };
+    vi.mocked(db.$transaction).mockImplementation(async (fn: any) => fn(mockTx));
+
+    await updateLaborPreset({
+      id: "preset-a",
+      name: "Updated Labor",
+      items: [{ description: "Oil change", hours: 1, rate: 75 }],
+    });
+
+    expect(vi.mocked(db.laborPreset.findFirst)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+
+  it("successfully updates the caller's own preset", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.findFirst).mockResolvedValue({ id: "preset-a" } as any);
+    const mockTx = {
+      laborPresetItem: { deleteMany: vi.fn().mockResolvedValue({ count: 1 }) },
+      laborPreset: {
+        update: vi.fn().mockResolvedValue({
+          id: "preset-a",
+          name: "Updated Labor",
+          items: [{ description: "Oil change", hours: 1, rate: 75 }],
+        }),
+      },
+    };
+    vi.mocked(db.$transaction).mockImplementation(async (fn: any) => fn(mockTx));
+
+    const result = await updateLaborPreset({
+      id: "preset-a",
+      name: "Updated Labor",
+      items: [{ description: "Oil change", hours: 1, rate: 75 }],
+    });
+
+    expect(result.success).toBe(true);
+    expect((result.data as any).updated).toBe(true);
+  });
+});
+
+describe("deleteLaborPreset — cross-org isolation", () => {
+  it("returns error when deleting another org's preset", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.deleteMany).mockResolvedValue({ count: 0 } as any);
+
+    const result = await deleteLaborPreset(`${ORG_B}-preset-id`);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Preset not found");
+  });
+
+  it("delete query includes organizationId", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.deleteMany).mockResolvedValue({ count: 1 } as any);
+
+    await deleteLaborPreset("preset-a");
+
+    expect(vi.mocked(db.laborPreset.deleteMany)).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ organizationId: ORG_A }),
+      })
+    );
+  });
+
+  it("successfully deletes the caller's own preset", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.laborPreset.deleteMany).mockResolvedValue({ count: 1 } as any);
+
+    const result = await deleteLaborPreset("preset-a");
+
+    expect(result.success).toBe(true);
+    expect((result.data as any).deleted).toBe(true);
+  });
+});

src/__tests__/multitenancy/service-record-isolation.test.ts

@@ -0,0 +1,152 @@
+/**
+ * Multi-tenancy isolation tests: Service Record CRUD
+ *
+ * Verifies that getServiceRecord, updateServiceRecord, and deleteServiceRecord
+ * are properly scoped to the caller's organization and reject cross-org access.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+vi.mock("@/lib/cached-session", () => ({
+  getCachedSession: vi.fn(),
+  getCachedMembership: vi.fn(),
+}));
+vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
+vi.mock("@/lib/resolve-upload-path", () => ({
+  resolveUploadPath: vi.fn((url: string) => `/uploads/${url}`),
+}));
+vi.mock("@/lib/notification-bus", () => ({
+  notificationBus: { emit: vi.fn() },
+}));
+vi.mock("@/lib/db", () => ({
+  db: {
+    user: { findUnique: vi.fn() },
+    vehicle: { findFirst: vi.fn(), update: vi.fn() },
+    serviceRecord: { findFirst: vi.fn(), findMany: vi.fn(), update: vi.fn(), delete: vi.fn() },
+    $transaction: vi.fn(),
+  },
+}));
+
+import { getCachedSession, getCachedMembership } from "@/lib/cached-session";
+import { db } from "@/lib/db";
+import {
+  getServiceRecord,
+  updateServiceRecord,
+  deleteServiceRecord,
+} from "@/features/vehicles/Actions/serviceActions";
+
+const mockSession = vi.mocked(getCachedSession);
+const mockMembership = vi.mocked(getCachedMembership);
+const mockUserFindUnique = vi.mocked(db.user.findUnique);
+const ORG_A = "org-a";
+const ORG_B = "org-b";
+
+function setupOrgAOwner() {
+  mockSession.mockResolvedValue({ user: { id: "user-a", email: "a@example.com" } } as any);
+  mockMembership.mockResolvedValue({
+    organizationId: ORG_A, role: "owner", roleId: null, customRole: null,
+  } as any);
+  mockUserFindUnique.mockResolvedValue({ isSuperAdmin: false } as any);
+}
+
+function setupOrgAMember() {
+  mockSession.mockResolvedValue({ user: { id: "user-m", email: "m@example.com" } } as any);
+  mockMembership.mockResolvedValue({
+    organizationId: ORG_A, role: "member", roleId: null, customRole: null,
+  } as any);
+  mockUserFindUnique.mockResolvedValue({ isSuperAdmin: false } as any);
+}
+
+const ORG_A_RECORD = {
+  id: "sr-a", vehicleId: "veh-a", title: "Oil Change", status: "pending", attachments: [],
+  vehicle: { id: "veh-a", mileage: 50000, make: "Toyota", model: "Camry", year: 2020, licensePlate: "ABC123" },
+};
+
+const orgWhereClause = expect.objectContaining({
+  where: expect.objectContaining({
+    vehicle: expect.objectContaining({ organizationId: ORG_A }),
+  }),
+});
+
+beforeEach(() => { vi.resetAllMocks(); });
+
+describe("getServiceRecord — cross-org isolation", () => {
+  it("returns null data when querying another org's service record", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    const result = await getServiceRecord(`${ORG_B}-record-id`);
+    expect(result.success).toBe(true);
+    expect(result.data).toBeNull();
+  });
+
+  it("scopes the query to the caller's organizationId via vehicle relation", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    await getServiceRecord("sr-a");
+    expect(vi.mocked(db.serviceRecord.findFirst)).toHaveBeenCalledWith(orgWhereClause);
+  });
+
+  it("returns the service record when it belongs to the caller's org", async () => {
+    setupOrgAMember();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(ORG_A_RECORD as any);
+    const result = await getServiceRecord("sr-a");
+    expect(result.success).toBe(true);
+    expect((result.data as any)?.id).toBe("sr-a");
+  });
+});
+
+describe("updateServiceRecord — cross-org isolation", () => {
+  it("returns error when targeting another org's service record", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    const result = await updateServiceRecord({ id: `${ORG_B}-record-id` });
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Service record not found");
+  });
+
+  it("ownership check always includes organizationId via vehicle relation", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    await updateServiceRecord({ id: "sr-x" });
+    expect(vi.mocked(db.serviceRecord.findFirst)).toHaveBeenCalledWith(orgWhereClause);
+  });
+
+  it("successfully updates a service record belonging to the caller's org", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(ORG_A_RECORD as any);
+    const updatedRecord = { ...ORG_A_RECORD, title: "Brake Pads" };
+    vi.mocked(db.$transaction).mockImplementation(async (fn: any) =>
+      fn({ serviceRecord: { update: vi.fn().mockResolvedValue(updatedRecord) } })
+    );
+    const result = await updateServiceRecord({ id: "sr-a", title: "Brake Pads" });
+    expect(result.success).toBe(true);
+    expect((result.data as any)?.title).toBe("Brake Pads");
+  });
+});
+
+describe("deleteServiceRecord — cross-org isolation", () => {
+  it("returns error when deleting another org's service record", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    const result = await deleteServiceRecord(`${ORG_B}-record-id`);
+    expect(result.success).toBe(false);
+    expect(result.error).toBe("Record not found");
+  });
+
+  it("ownership check always includes organizationId via vehicle relation", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue(null);
+    await deleteServiceRecord("sr-x");
+    expect(vi.mocked(db.serviceRecord.findFirst)).toHaveBeenCalledWith(orgWhereClause);
+  });
+
+  it("successfully deletes a service record belonging to the caller's org", async () => {
+    setupOrgAOwner();
+    vi.mocked(db.serviceRecord.findFirst).mockResolvedValue({
+      id: "sr-a", vehicleId: "veh-a", attachments: [],
+    } as any);
+    vi.mocked(db.serviceRecord.delete).mockResolvedValue({} as any);
+    const result = await deleteServiceRecord("sr-a");
+    expect(result.success).toBe(true);
+    expect((result.data as any)?.recordId).toBe("sr-a");
+  });
+});

src/__tests__/multitenancy/team-invite-isolation.test.ts

@@ -110,8 +110,8 @@ function setupOrgAMember() {
 
 /**
  * User whose built-in role is "member" but whose custom role has isAdmin:true.
- * withAuth grants them access (roleIsAdmin=true), but actions that check the
- * built-in role string will incorrectly block them (BUG-6, BUG-7).
+ * withAuth properly grants isAdmin from customRole.isAdmin, so these actions
+ * work correctly.
  */
 function setupOrgACustomAdmin() {
   mockSession.mockResolvedValue({ user: { id: "user-a", email: "cadmin@orgA.com" } } as any);

src/__tests__/multitenancy/vehicle-service-isolation.test.ts

@@ -156,16 +156,14 @@ describe("getVehicles — org scoping", () => {
 // ---------------------------------------------------------------------------
 
 describe("updateVehicle — cross-org isolation", () => {
-  it("🐛 BUG-1: silently returns success when targeting another org's vehicle — nothing is actually changed", async () => {
+  it("returns error when targeting another org's vehicle", async () => {
     setupOrgAOwner();
     // updateMany with org filter matches 0 rows for a cross-org id
     vi.mocked(db.vehicle.updateMany).mockResolvedValue({ count: 0 } as any);
 
     const result = await updateVehicle({ id: `${ORG_B}-vehicle-id`, make: "Hacked" });
 
-    // BUG: should be { success: false, error: "Vehicle not found" }
-    // Fix: check `result.count === 0` in updateVehicle and throw.
-    expect(result.success).toBe(false); // FAILS until bug is fixed
+    expect(result.success).toBe(false);
   });
 
   it("update query always includes organizationId to prevent cross-org writes", async () => {
@@ -198,7 +196,7 @@ describe("updateVehicle — cross-org isolation", () => {
 // ---------------------------------------------------------------------------
 
 describe("deleteVehicle — cross-org isolation", () => {
-  it("🐛 BUG-2: silently returns success when targeting another org's vehicle — nothing is actually deleted", async () => {
+  it("returns error when deleting another org's vehicle", async () => {
     setupOrgAOwner();
     // findFirst returns null (cross-org vehicle invisible to caller)
     vi.mocked(db.vehicle.findFirst).mockResolvedValue(null);
@@ -207,9 +205,7 @@ describe("deleteVehicle — cross-org isolation", () => {
 
     const result = await deleteVehicle(`${ORG_B}-vehicle-id`);
 
-    // BUG: should be { success: false, error: "Vehicle not found" }
-    // Fix: throw when findFirst returns null (before deleteMany).
-    expect(result.success).toBe(false); // FAILS until bug is fixed
+    expect(result.success).toBe(false);
   });
 
   it("delete query always includes organizationId to prevent cross-org deletes", async () => {

src/features/labor-presets/Actions/laborPresetActions.ts

@@ -124,6 +124,11 @@ export async function updateLaborPreset(input: unknown) {
       updateData.name = updateData.name?.trim() || items?.[0]?.description || "Untitled";
     }
 
+    const existing = await db.laborPreset.findFirst({
+      where: { id, organizationId },
+    });
+    if (!existing) throw new Error("Preset not found");
+
     await db.$transaction(async (tx) => {
       // Delete old items
       await tx.laborPresetItem.deleteMany({ where: { presetId: id } });

src/features/vehicles/Actions/addPartToServiceRecord.ts

@@ -70,10 +70,11 @@ export async function addPartToServiceRecord(input: {
 
       // Deduct inventory stock if linked
       if (input.inventoryPartId) {
-        await db.inventoryPart.update({
-          where: { id: input.inventoryPartId },
+        const invResult = await db.inventoryPart.updateMany({
+          where: { id: input.inventoryPartId, organizationId },
           data: { quantity: { decrement: input.quantity } },
         });
+        if (invResult.count === 0) throw new Error("Inventory part not found");
       }
 
       revalidatePath(`/vehicles/${record.vehicleId}/service/${record.id}`);

src/features/vehicles/Actions/archiveVehicle.ts

@@ -8,10 +8,11 @@ import { revalidatePath } from "next/cache";
 export async function archiveVehicle(vehicleId: string, reason?: string) {
   return withAuth(
     async ({ organizationId }) => {
-      await db.vehicle.updateMany({
+      const result = await db.vehicle.updateMany({
         where: { id: vehicleId, organizationId },
         data: { isArchived: true, archiveReason: reason || null },
       });
+      if (result.count === 0) throw new Error("Vehicle not found");
 
       revalidatePath("/");
       revalidatePath("/vehicles");

src/features/vehicles/Actions/deleteVehicle.ts

@@ -8,9 +8,10 @@ import { revalidatePath } from "next/cache";
 export async function deleteVehicle(vehicleId: string) {
   return withAuth(
     async ({ organizationId }) => {
-      await db.vehicle.deleteMany({
+      const result = await db.vehicle.deleteMany({
         where: { id: vehicleId, organizationId },
       });
+      if (result.count === 0) throw new Error("Vehicle not found");
 
       revalidatePath("/");
       revalidatePath("/vehicles");

src/features/vehicles/Actions/unarchiveVehicle.ts

@@ -8,10 +8,11 @@ import { revalidatePath } from "next/cache";
 export async function unarchiveVehicle(vehicleId: string) {
   return withAuth(
     async ({ organizationId }) => {
-      await db.vehicle.updateMany({
+      const result = await db.vehicle.updateMany({
         where: { id: vehicleId, organizationId },
         data: { isArchived: false, archiveReason: null },
       });
+      if (result.count === 0) throw new Error("Vehicle not found");
 
       revalidatePath("/");
       revalidatePath("/vehicles");