fix: return 400 instead of 500 for invalid eventTypeId in booking flow (#26732)

* fix: return 400 instead of 500 for invalid eventTypeId in booking flow

Co-Authored-By: benny@cal.com <sldisek783@gmail.com>

* refactor: use ErrorWithCode instead of HttpError in BotDetectionService

Co-Authored-By: benny@cal.com <sldisek783@gmail.com>

* refactor: use vi.stubEnv for safer environment variable handling in tests

Co-Authored-By: benny@cal.com <sldisek783@gmail.com>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

packages/features/bot-detection/BotDetectionService.test.ts

@@ -1,8 +1,10 @@
 import type { IncomingHttpHeaders } from "node:http";
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 import type { EventTypeRepository } from "@calcom/features/eventtypes/repositories/eventTypeRepository";
 import type { FeaturesRepository } from "@calcom/features/flags/features.repository";
+import { ErrorCode } from "@calcom/lib/errorCodes";
+import { ErrorWithCode } from "@calcom/lib/errors";
 import { HttpError } from "@calcom/lib/http-error";
 
 import { BotDetectionService } from "./BotDetectionService";
@@ -30,10 +32,8 @@ describe("BotDetectionService", () => {
   let mockHeaders: IncomingHttpHeaders;
 
   beforeEach(() => {
-    // Reset all mocks before each test
     vi.clearAllMocks();
 
-    // Setup mock repositories
     mockFeaturesRepository = {
       checkIfTeamHasFeature: vi.fn(),
     } as unknown as FeaturesRepository;
@@ -48,14 +48,15 @@ describe("BotDetectionService", () => {
     };
 
     botDetectionService = new BotDetectionService(mockFeaturesRepository, mockEventTypeRepository);
+  });
 
-    // Reset environment variable
-    delete process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER;
+  afterEach(() => {
+    vi.unstubAllEnvs();
   });
 
   describe("checkBotDetection", () => {
     it("should return early if BotID is not enabled at instance level", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "0";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "0");
 
       await botDetectionService.checkBotDetection({
         eventTypeId: 123,
@@ -66,7 +67,7 @@ describe("BotDetectionService", () => {
     });
 
     it("should return early if no eventTypeId is provided", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
 
       await botDetectionService.checkBotDetection({
         headers: mockHeaders,
@@ -75,8 +76,43 @@ describe("BotDetectionService", () => {
       expect(mockEventTypeRepository.getTeamIdByEventTypeId).not.toHaveBeenCalled();
     });
 
+    it("should throw ErrorWithCode for invalid eventTypeId", async () => {
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
+
+      await expect(
+        botDetectionService.checkBotDetection({
+          eventTypeId: "3823346KiEg1Zk6') OR 370=(SELECT 370 FROM PG_SLEEP(15))--" as unknown as number,
+          headers: mockHeaders,
+        })
+      ).rejects.toThrow(ErrorWithCode);
+
+      await expect(
+        botDetectionService.checkBotDetection({
+          eventTypeId: -1,
+          headers: mockHeaders,
+        })
+      ).rejects.toThrow(ErrorWithCode);
+
+      await expect(
+        botDetectionService.checkBotDetection({
+          eventTypeId: 1.5,
+          headers: mockHeaders,
+        })
+      ).rejects.toThrow(ErrorWithCode);
+
+      try {
+        await botDetectionService.checkBotDetection({
+          eventTypeId: -1,
+          headers: mockHeaders,
+        });
+      } catch (error) {
+        expect(error).toBeInstanceOf(ErrorWithCode);
+        expect((error as ErrorWithCode).code).toBe(ErrorCode.BadRequest);
+      }
+    });
+
     it("should return early if event type has no teamId", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
       vi.mocked(mockEventTypeRepository.getTeamIdByEventTypeId).mockResolvedValue({
         teamId: null,
       });
@@ -90,7 +126,7 @@ describe("BotDetectionService", () => {
     });
 
     it("should return early if BotID feature is not enabled for the team", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
       vi.mocked(mockEventTypeRepository.getTeamIdByEventTypeId).mockResolvedValue({
         teamId: 456,
       });
@@ -107,7 +143,7 @@ describe("BotDetectionService", () => {
     });
 
     it("should throw HttpError when a bot is detected", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
       vi.mocked(mockEventTypeRepository.getTeamIdByEventTypeId).mockResolvedValue({
         teamId: 456,
       });
@@ -140,7 +176,7 @@ describe("BotDetectionService", () => {
     });
 
     it("should pass when a human is detected", async () => {
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
       vi.mocked(mockEventTypeRepository.getTeamIdByEventTypeId).mockResolvedValue({
         teamId: 456,
       });
@@ -167,7 +203,7 @@ describe("BotDetectionService", () => {
 
     it("should check feature flag with correct teamId", async () => {
       const teamId = 789;
-      process.env.NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER = "1";
+      vi.stubEnv("NEXT_PUBLIC_VERCEL_USE_BOTID_IN_BOOKER", "1");
       vi.mocked(mockEventTypeRepository.getTeamIdByEventTypeId).mockResolvedValue({
         teamId,
       });

packages/features/bot-detection/BotDetectionService.ts

@@ -3,6 +3,8 @@ import type { IncomingHttpHeaders } from "node:http";
 
 import type { EventTypeRepository } from "@calcom/features/eventtypes/repositories/eventTypeRepository";
 import type { FeaturesRepository } from "@calcom/features/flags/features.repository";
+import { ErrorCode } from "@calcom/lib/errorCodes";
+import { ErrorWithCode } from "@calcom/lib/errors";
 import { HttpError } from "@calcom/lib/http-error";
 import logger from "@calcom/lib/logger";
 
@@ -34,7 +36,10 @@ export class BotDetectionService {
     }
 
     if (!Number.isInteger(eventTypeId) || eventTypeId <= 0) {
-      throw new Error(`Invalid eventTypeId: ${eventTypeId}. Must be a positive integer.`);
+      throw new ErrorWithCode(
+        ErrorCode.BadRequest,
+        `Invalid eventTypeId: ${eventTypeId}. Must be a positive integer.`
+      );
     }
 
     // Fetch only the teamId from the event type