diff --git a/apps/api/src/common/guards/permission.guard.spec.ts b/apps/api/src/common/guards/permission.guard.spec.ts index ab3ccd1..cce4442 100644 --- a/apps/api/src/common/guards/permission.guard.spec.ts +++ b/apps/api/src/common/guards/permission.guard.spec.ts @@ -1,11 +1,11 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; -import { ExecutionContext, ForbiddenException } from "@nestjs/common"; +import { ExecutionContext, ForbiddenException, InternalServerErrorException } from "@nestjs/common"; import { Reflector } from "@nestjs/core"; +import { Prisma, WorkspaceMemberRole } from "@prisma/client"; import { PermissionGuard } from "./permission.guard"; import { PrismaService } from "../../prisma/prisma.service"; import { Permission } from "../decorators/permissions.decorator"; -import { WorkspaceMemberRole } from "@prisma/client"; describe("PermissionGuard", () => { let guard: PermissionGuard; @@ -208,13 +208,67 @@ describe("PermissionGuard", () => { ); }); - it("should handle database errors gracefully", async () => { + it("should throw InternalServerErrorException on database connection errors", async () => { const context = createMockExecutionContext({ id: userId }, { id: workspaceId }); mockReflector.getAllAndOverride.mockReturnValue(Permission.WORKSPACE_MEMBER); - mockPrismaService.workspaceMember.findUnique.mockRejectedValue(new Error("Database error")); + mockPrismaService.workspaceMember.findUnique.mockRejectedValue( + new Error("Database connection failed") + ); + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + await expect(guard.canActivate(context)).rejects.toThrow("Failed to verify permissions"); + }); + + it("should throw InternalServerErrorException on Prisma connection timeout", async () => { + const context = createMockExecutionContext({ id: userId }, { id: workspaceId }); + + mockReflector.getAllAndOverride.mockReturnValue(Permission.WORKSPACE_MEMBER); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Connection timed out", { + code: "P1001", // Authentication failed (connection error) + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + }); + + it("should return null role for Prisma not found error (P2025)", async () => { + const context = createMockExecutionContext({ id: userId }, { id: workspaceId }); + + mockReflector.getAllAndOverride.mockReturnValue(Permission.WORKSPACE_MEMBER); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Record not found", { + code: "P2025", // Record not found + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + // P2025 should be treated as "not a member" -> ForbiddenException await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); + await expect(guard.canActivate(context)).rejects.toThrow( + "You are not a member of this workspace" + ); + }); + + it("should NOT mask database pool exhaustion as permission denied", async () => { + const context = createMockExecutionContext({ id: userId }, { id: workspaceId }); + + mockReflector.getAllAndOverride.mockReturnValue(Permission.WORKSPACE_MEMBER); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Connection pool exhausted", { + code: "P2024", // Connection pool timeout + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + // Should NOT throw ForbiddenException for DB errors + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + await expect(guard.canActivate(context)).rejects.not.toThrow(ForbiddenException); }); }); }); diff --git a/apps/api/src/common/guards/permission.guard.ts b/apps/api/src/common/guards/permission.guard.ts index c0dc7a5..6c4e43d 100644 --- a/apps/api/src/common/guards/permission.guard.ts +++ b/apps/api/src/common/guards/permission.guard.ts @@ -3,8 +3,10 @@ import { CanActivate, ExecutionContext, ForbiddenException, + InternalServerErrorException, Logger, } from "@nestjs/common"; +import { Prisma } from "@prisma/client"; import { Reflector } from "@nestjs/core"; import { PrismaService } from "../../prisma/prisma.service"; import { PERMISSION_KEY, Permission } from "../decorators/permissions.decorator"; @@ -99,6 +101,10 @@ export class PermissionGuard implements CanActivate { /** * Fetches the user's role in a workspace + * + * SEC-API-3 FIX: Database errors are no longer swallowed as null role. + * Connection timeouts, pool exhaustion, and other infrastructure errors + * are propagated as 500 errors to avoid masking operational issues. */ private async getUserWorkspaceRole( userId: string, @@ -119,11 +125,23 @@ export class PermissionGuard implements CanActivate { return member?.role ?? null; } catch (error) { + // Only handle Prisma "not found" errors (P2025) as expected cases + // All other database errors (connection, timeout, pool) should propagate + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === "P2025" // Record not found + ) { + return null; + } + + // Log the error before propagating this.logger.error( - `Failed to fetch user role: ${error instanceof Error ? error.message : "Unknown error"}`, + `Database error during permission check: ${error instanceof Error ? error.message : "Unknown error"}`, error instanceof Error ? error.stack : undefined ); - return null; + + // Propagate infrastructure errors as 500s, not permission denied + throw new InternalServerErrorException("Failed to verify permissions"); } } diff --git a/apps/api/src/common/guards/workspace.guard.spec.ts b/apps/api/src/common/guards/workspace.guard.spec.ts index 8146ba6..5e1dea9 100644 --- a/apps/api/src/common/guards/workspace.guard.spec.ts +++ b/apps/api/src/common/guards/workspace.guard.spec.ts @@ -1,6 +1,12 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; -import { ExecutionContext, ForbiddenException, BadRequestException } from "@nestjs/common"; +import { + ExecutionContext, + ForbiddenException, + BadRequestException, + InternalServerErrorException, +} from "@nestjs/common"; +import { Prisma } from "@prisma/client"; import { WorkspaceGuard } from "./workspace.guard"; import { PrismaService } from "../../prisma/prisma.service"; @@ -253,14 +259,60 @@ describe("WorkspaceGuard", () => { ); }); - it("should handle database errors gracefully", async () => { + it("should throw InternalServerErrorException on database connection errors", async () => { const context = createMockExecutionContext({ id: userId }, { "x-workspace-id": workspaceId }); mockPrismaService.workspaceMember.findUnique.mockRejectedValue( new Error("Database connection failed") ); + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + await expect(guard.canActivate(context)).rejects.toThrow("Failed to verify workspace access"); + }); + + it("should throw InternalServerErrorException on Prisma connection timeout", async () => { + const context = createMockExecutionContext({ id: userId }, { "x-workspace-id": workspaceId }); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Connection timed out", { + code: "P1001", // Authentication failed (connection error) + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + }); + + it("should return false for Prisma not found error (P2025)", async () => { + const context = createMockExecutionContext({ id: userId }, { "x-workspace-id": workspaceId }); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Record not found", { + code: "P2025", // Record not found + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + // P2025 should be treated as "not a member" -> ForbiddenException await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); + await expect(guard.canActivate(context)).rejects.toThrow( + "You do not have access to this workspace" + ); + }); + + it("should NOT mask database pool exhaustion as access denied", async () => { + const context = createMockExecutionContext({ id: userId }, { "x-workspace-id": workspaceId }); + + const prismaError = new Prisma.PrismaClientKnownRequestError("Connection pool exhausted", { + code: "P2024", // Connection pool timeout + clientVersion: "5.0.0", + }); + + mockPrismaService.workspaceMember.findUnique.mockRejectedValue(prismaError); + + // Should NOT throw ForbiddenException for DB errors + await expect(guard.canActivate(context)).rejects.toThrow(InternalServerErrorException); + await expect(guard.canActivate(context)).rejects.not.toThrow(ForbiddenException); }); }); }); diff --git a/apps/api/src/common/guards/workspace.guard.ts b/apps/api/src/common/guards/workspace.guard.ts index d0f9dab..75d065f 100644 --- a/apps/api/src/common/guards/workspace.guard.ts +++ b/apps/api/src/common/guards/workspace.guard.ts @@ -4,8 +4,10 @@ import { ExecutionContext, ForbiddenException, BadRequestException, + InternalServerErrorException, Logger, } from "@nestjs/common"; +import { Prisma } from "@prisma/client"; import { PrismaService } from "../../prisma/prisma.service"; import type { AuthenticatedRequest } from "../types/user.types"; @@ -127,6 +129,10 @@ export class WorkspaceGuard implements CanActivate { /** * Verifies that a user is a member of the specified workspace + * + * SEC-API-2 FIX: Database errors are no longer swallowed as "access denied". + * Connection timeouts, pool exhaustion, and other infrastructure errors + * are propagated as 500 errors to avoid masking operational issues. */ private async verifyWorkspaceMembership(userId: string, workspaceId: string): Promise { try { @@ -141,11 +147,23 @@ export class WorkspaceGuard implements CanActivate { return member !== null; } catch (error) { + // Only handle Prisma "not found" errors (P2025) as expected cases + // All other database errors (connection, timeout, pool) should propagate + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === "P2025" // Record not found + ) { + return false; + } + + // Log the error before propagating this.logger.error( - `Failed to verify workspace membership: ${error instanceof Error ? error.message : "Unknown error"}`, + `Database error during workspace membership check: ${error instanceof Error ? error.message : "Unknown error"}`, error instanceof Error ? error.stack : undefined ); - return false; + + // Propagate infrastructure errors as 500s, not access denied + throw new InternalServerErrorException("Failed to verify workspace access"); } } }