Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/api/src/audit/audit-log.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('AuditLogController', () => {
where: { organizationId: 'org_1' },
include: {
user: {
select: { id: true, name: true, email: true, image: true },
select: { id: true, name: true, email: true, image: true, isPlatformAdmin: true },
},
member: true,
organization: true,
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/audit/audit-log.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class AuditLogController {
where,
include: {
user: {
select: { id: true, name: true, email: true, image: true },
select: { id: true, name: true, email: true, image: true, isPlatformAdmin: true },
},
member: true,
organization: true,
Expand Down
9 changes: 8 additions & 1 deletion apps/api/src/people/people-fleet.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ export async function getAllEmployeeDevices(
) {
try {
const employees = await db.member.findMany({
where: { organizationId, deactivated: false },
where: {
organizationId,
deactivated: false,
OR: [
{ user: { isPlatformAdmin: false } },
{ role: { contains: 'owner' } },
],
},
include: { user: true },
});

Expand Down
20 changes: 20 additions & 0 deletions apps/api/src/policies/policies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,15 @@ export class PoliciesService {

async create(organizationId: string, createData: CreatePolicyDto) {
try {
if (createData.assigneeId) {
const assignee = await db.member.findFirst({
where: { id: createData.assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (assignee?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Policy update path missing platform admin assignee check

High Severity

The create method in policies.service.ts validates that assigneeId doesn't belong to a platform admin, but the updateById method has no such check. Every other service (risks, vendors, tasks, task-management) consistently enforces this restriction in both create and update paths. This allows a platform admin to be assigned to a policy via the update API, bypassing the intended restriction.

Additional Locations (1)

Fix in Cursor Fix in Web

const contentValue = createData.content as Prisma.InputJsonValue[];

// Create policy with version 1 in a transaction
Expand Down Expand Up @@ -971,6 +980,17 @@ export class PoliciesService {
);
}

// Cannot assign a platform admin as approver
const approverUser = await db.user.findUnique({
where: { id: approver.userId },
select: { isPlatformAdmin: true },
});
if (approverUser?.isPlatformAdmin) {
throw new BadRequestException(
'Cannot assign a platform admin as approver',
);
}

await db.policy.update({
where: { id: policyId },
data: {
Expand Down
19 changes: 18 additions & 1 deletion apps/api/src/risks/risks.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable, NotFoundException, Logger } from '@nestjs/common';
import { BadRequestException, Injectable, NotFoundException, Logger } from '@nestjs/common';
import { db, Prisma } from '@trycompai/db';
import { CreateRiskDto } from './dto/create-risk.dto';
import { GetRisksQueryDto } from './dto/get-risks-query.dto';
Expand All @@ -25,6 +25,16 @@ export interface PaginatedRisksResult {
export class RisksService {
private readonly logger = new Logger(RisksService.name);

private async validateAssigneeNotPlatformAdmin(assigneeId: string, organizationId: string) {
const member = await db.member.findFirst({
where: { id: assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (member?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}

async findAllByOrganization(
organizationId: string,
assignmentFilter: Prisma.RiskWhereInput = {},
Expand Down Expand Up @@ -130,6 +140,9 @@ export class RisksService {

async create(organizationId: string, createRiskDto: CreateRiskDto) {
try {
if (createRiskDto.assigneeId) {
await this.validateAssigneeNotPlatformAdmin(createRiskDto.assigneeId, organizationId);
}
const risk = await db.risk.create({
data: {
...createRiskDto,
Expand Down Expand Up @@ -159,6 +172,10 @@ export class RisksService {
// First check if the risk exists in the organization
await this.findById(id, organizationId);

if (updateRiskDto.assigneeId) {
await this.validateAssigneeNotPlatformAdmin(updateRiskDto.assigneeId, organizationId);
}

const updatedRisk = await db.risk.update({
where: { id },
data: updateRiskDto,
Expand Down
21 changes: 21 additions & 0 deletions apps/api/src/soa/soa.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jest.mock('@db', () => ({
update: jest.fn(),
},
member: { findFirst: jest.fn() },
user: { findUnique: jest.fn() },
sOAAnswer: { findFirst: jest.fn(), create: jest.fn(), update: jest.fn() },
},
}));
Expand Down Expand Up @@ -214,18 +215,34 @@ describe('SOAService', () => {
it('throws ForbiddenException when approver is not owner/admin', async () => {
(mockDb.member.findFirst as jest.Mock).mockResolvedValue({
id: 'mem-1',
userId: 'user-1',
role: 'employee',
});
(mockDb.user.findUnique as jest.Mock).mockResolvedValue({ isPlatformAdmin: false });
await expect(service.submitForApproval(dto)).rejects.toThrow(
ForbiddenException,
);
});

it('throws BadRequestException when approver is platform admin', async () => {
(mockDb.member.findFirst as jest.Mock).mockResolvedValue({
id: 'mem-1',
userId: 'user-1',
role: 'admin',
});
(mockDb.user.findUnique as jest.Mock).mockResolvedValue({ isPlatformAdmin: true });
await expect(service.submitForApproval(dto)).rejects.toThrow(
BadRequestException,
);
});

it('throws NotFoundException when document not found', async () => {
(mockDb.member.findFirst as jest.Mock).mockResolvedValue({
id: 'mem-1',
userId: 'user-1',
role: 'admin',
});
(mockDb.user.findUnique as jest.Mock).mockResolvedValue({ isPlatformAdmin: false });
(mockDb.sOADocument.findFirst as jest.Mock).mockResolvedValue(null);
await expect(service.submitForApproval(dto)).rejects.toThrow(
NotFoundException,
Expand All @@ -235,8 +252,10 @@ describe('SOAService', () => {
it('throws BadRequestException when already pending approval', async () => {
(mockDb.member.findFirst as jest.Mock).mockResolvedValue({
id: 'mem-1',
userId: 'user-1',
role: 'admin',
});
(mockDb.user.findUnique as jest.Mock).mockResolvedValue({ isPlatformAdmin: false });
(mockDb.sOADocument.findFirst as jest.Mock).mockResolvedValue({
id: 'doc-1',
status: 'needs_review',
Expand All @@ -249,8 +268,10 @@ describe('SOAService', () => {
it('submits for approval successfully', async () => {
(mockDb.member.findFirst as jest.Mock).mockResolvedValue({
id: 'mem-1',
userId: 'user-1',
role: 'owner',
});
(mockDb.user.findUnique as jest.Mock).mockResolvedValue({ isPlatformAdmin: false });
(mockDb.sOADocument.findFirst as jest.Mock).mockResolvedValue({
id: 'doc-1',
status: 'draft',
Expand Down
9 changes: 9 additions & 0 deletions apps/api/src/soa/soa.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,15 @@ export class SOAService {
throw new NotFoundException('Approver not found in organization');
}

// Cannot assign a platform admin as approver
const approverUser = await db.user.findUnique({
where: { id: approverMember.userId },
select: { isPlatformAdmin: true },
});
if (approverUser?.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as approver');
}

const isOwnerOrAdmin =
approverMember.role.includes('owner') ||
approverMember.role.includes('admin');
Expand Down
19 changes: 19 additions & 0 deletions apps/api/src/task-management/task-management.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,16 @@ export class TaskManagementService {
);
}

if (createTaskItemDto.assigneeId) {
const assigneeMember = await db.member.findFirst({
where: { id: createTaskItemDto.assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (assigneeMember?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}

const taskItem = await db.taskItem.create({
data: {
...createTaskItemDto,
Expand Down Expand Up @@ -470,6 +480,15 @@ export class TaskManagementService {
updateData.priority = updateTaskItemDto.priority;
}
if (updateTaskItemDto.assigneeId !== undefined) {
if (updateTaskItemDto.assigneeId) {
const assigneeMember = await db.member.findFirst({
where: { id: updateTaskItemDto.assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (assigneeMember?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}
updateData.assigneeId = updateTaskItemDto.assigneeId;
}

Expand Down
8 changes: 8 additions & 0 deletions apps/api/src/tasks/task-notifier.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,10 @@ export class TaskNotifierService {
where: {
organizationId,
deactivated: false,
OR: [
{ user: { isPlatformAdmin: false } },
{ role: { contains: 'owner' } },
],
},
select: {
id: true,
Expand Down Expand Up @@ -1298,6 +1302,10 @@ export class TaskNotifierService {
where: {
organizationId,
deactivated: false,
OR: [
{ user: { isPlatformAdmin: false } },
{ role: { contains: 'owner' } },
],
},
select: {
id: true,
Expand Down
29 changes: 28 additions & 1 deletion apps/api/src/tasks/tasks.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export class TasksService {
where,
include: {
user: {
select: { id: true, name: true, email: true, image: true },
select: { id: true, name: true, email: true, image: true, isPlatformAdmin: true },
},
},
orderBy: { timestamp: 'desc' },
Expand Down Expand Up @@ -357,6 +357,16 @@ export class TasksService {
changedByUserId: string,
): Promise<{ updatedCount: number }> {
try {
if (assigneeId) {
const assigneeMember = await db.member.findFirst({
where: { id: assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (assigneeMember?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}

const result = await db.task.updateMany({
where: {
id: {
Expand Down Expand Up @@ -493,6 +503,15 @@ export class TasksService {
dataToUpdate.status = updateData.status;
}
if (updateData.assigneeId !== undefined) {
if (updateData.assigneeId !== null) {
const assigneeMember = await db.member.findFirst({
where: { id: updateData.assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (assigneeMember?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}
dataToUpdate.assigneeId =
updateData.assigneeId === null ? null : updateData.assigneeId;
}
Expand Down Expand Up @@ -826,6 +845,10 @@ export class TasksService {
throw new BadRequestException('Approver not found or is deactivated');
}

if (approver.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as approver');
}

const currentMember = await db.member.findFirst({
where: { userId, organizationId, deactivated: false },
});
Expand Down Expand Up @@ -899,6 +922,10 @@ export class TasksService {
throw new BadRequestException('Approver not found or is deactivated');
}

if (approver.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as approver');
}

const tasks = await db.task.findMany({
where: {
id: { in: taskIds },
Expand Down
19 changes: 18 additions & 1 deletion apps/api/src/vendors/vendors.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable, NotFoundException, Logger } from '@nestjs/common';
import { BadRequestException, Injectable, NotFoundException, Logger } from '@nestjs/common';
import { db, TaskItemPriority, TaskItemStatus } from '@trycompai/db';
import { CreateVendorDto } from './dto/create-vendor.dto';
import { UpdateVendorDto } from './dto/update-vendor.dto';
Expand Down Expand Up @@ -198,12 +198,25 @@ export class VendorsService {
}
}

private async validateAssigneeNotPlatformAdmin(assigneeId: string, organizationId: string) {
const member = await db.member.findFirst({
where: { id: assigneeId, organizationId },
include: { user: { select: { isPlatformAdmin: true } } },
});
if (member?.user.isPlatformAdmin) {
throw new BadRequestException('Cannot assign a platform admin as assignee');
}
}

async create(
organizationId: string,
createVendorDto: CreateVendorDto,
createdByUserId?: string,
) {
try {
if (createVendorDto.assigneeId) {
await this.validateAssigneeNotPlatformAdmin(createVendorDto.assigneeId, organizationId);
}
const vendor = await db.vendor.create({
data: {
...createVendorDto,
Expand Down Expand Up @@ -587,6 +600,10 @@ export class VendorsService {
// First check if the vendor exists in the organization
await this.findById(id, organizationId);

if (updateVendorDto.assigneeId) {
await this.validateAssigneeNotPlatformAdmin(updateVendorDto.assigneeId, organizationId);
}

const updatedVendor = await db.vendor.update({
where: { id },
data: updateVendorDto,
Expand Down
12 changes: 12 additions & 0 deletions apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export function MemberRow({
: 0;

const isOwner = currentRoles.includes('owner');
const isPlatformAdmin = member.user.isPlatformAdmin;
const canRemove = !isOwner;
const isDeactivated = member.deactivated || !member.isActive;
const profileHref = `/${orgId}/people/${memberId}`;
Expand Down Expand Up @@ -219,6 +220,9 @@ export function MemberRow({
<TableCell>
<div className="w-[160px]">
<div className="flex flex-wrap gap-1">
{member.user.isPlatformAdmin && (
<Badge>Comp AI</Badge>
)}
{currentRoles.map((role) => (
<Badge key={role} variant="outline">
{getRoleLabel(role)}
Expand Down Expand Up @@ -333,6 +337,14 @@ export function MemberRow({
<div className="space-y-4 py-4">
<div className="space-y-2">
<Label htmlFor={`role-${memberId}`}>Roles</Label>
{isPlatformAdmin && (
<div className="flex items-center gap-2 rounded-md border px-3 py-2">
<Badge>Comp AI</Badge>
<span className="text-muted-foreground text-xs">
This role is managed by the platform and cannot be removed.
</span>
</div>
)}
<MultiRoleCombobox
selectedRoles={selectedRoles}
onSelectedRolesChange={setSelectedRoles}
Expand Down
Loading