fix(findings): allow admins/owners to set ready_for_review status

apps/api/src/admin-organizations/admin-findings.controller.ts

@@ -87,7 +87,7 @@ export class AdminFindingsController {
       orgId,
       findingId,
       updateDto,
-      [],
+      true,
       true,
       req.userId,
       null,

apps/api/src/findings/findings.controller.ts

@@ -11,7 +11,6 @@ import {
   UsePipes,
   ValidationPipe,
   BadRequestException,
-  ForbiddenException,
 } from '@nestjs/common';
 import {
   ApiBody,
@@ -34,6 +33,7 @@ import { PermissionGuard } from '../auth/permission.guard';
 import { RequirePermission } from '../auth/require-permission.decorator';
 import { AuthContext } from '../auth/auth-context.decorator';
 import type { AuthContext as AuthContextType } from '../auth/types';
+import { RolesService } from '../roles/roles.service';
 import { FindingsService } from './findings.service';
 import { CreateFindingDto } from './dto/create-finding.dto';
 import { UpdateFindingDto } from './dto/update-finding.dto';
@@ -46,7 +46,10 @@ import { evidenceFormTypeSchema } from '@/evidence-forms/evidence-forms.definiti
 @UseGuards(HybridAuthGuard)
 @ApiSecurity('apikey')
 export class FindingsController {
-  constructor(private readonly findingsService: FindingsService) {}
+  constructor(
+    private readonly findingsService: FindingsService,
+    private readonly rolesService: RolesService,
+  ) {}
 
   /**
    * List findings for the organization. Supports optional target/status filters.
@@ -203,15 +206,6 @@ export class FindingsController {
       throw new BadRequestException('User ID is required');
     }
 
-    const isAuditor = authContext.userRoles?.includes('auditor');
-    const isPlatformAdmin = await this.checkPlatformAdmin(authContext.userId);
-
-    if (!isAuditor && !isPlatformAdmin) {
-      throw new ForbiddenException(
-        'Only auditors or platform admins can create findings',
-      );
-    }
-
     const member = await db.member.findFirst({
       where: {
         userId: authContext.userId,
@@ -258,27 +252,34 @@ export class FindingsController {
       throw new BadRequestException('User ID is required');
     }
 
-    const isPlatformAdmin = await this.checkPlatformAdmin(authContext.userId);
-
-    const member = await db.member.findFirst({
-      where: {
-        userId: authContext.userId,
-        organizationId: authContext.organizationId,
-        deactivated: false,
-      },
-    });
+    const [isPlatformAdmin, member, permissions] = await Promise.all([
+      this.checkPlatformAdmin(authContext.userId),
+      db.member.findFirst({
+        where: {
+          userId: authContext.userId,
+          organizationId: authContext.organizationId,
+          deactivated: false,
+        },
+      }),
+      this.rolesService.resolvePermissions(
+        authContext.organizationId,
+        authContext.userRoles || [],
+      ),
+    ]);
 
     if (!member) {
       throw new BadRequestException(
         'User is not a member of this organization',
       );
     }
 
+    const canCreateFindings = permissions.finding?.includes('create') ?? false;
+
     return await this.findingsService.update(
       authContext.organizationId,
       id,
       updateDto,
-      authContext.userRoles || [],
+      canCreateFindings,
       isPlatformAdmin,
       authContext.userId,
       member.id,
@@ -298,15 +299,6 @@ export class FindingsController {
       throw new BadRequestException('User ID is required');
     }
 
-    const isAuditor = authContext.userRoles?.includes('auditor');
-    const isPlatformAdmin = await this.checkPlatformAdmin(authContext.userId);
-
-    if (!isAuditor && !isPlatformAdmin) {
-      throw new ForbiddenException(
-        'Only auditors or platform admins can delete findings',
-      );
-    }
-
     const member = await db.member.findFirst({
       where: {
         userId: authContext.userId,

apps/api/src/findings/findings.module.ts

@@ -1,5 +1,6 @@
 import { Module } from '@nestjs/common';
 import { AuthModule } from '../auth/auth.module';
+import { RolesModule } from '../roles/roles.module';
 import { TimelinesModule } from '../timelines/timelines.module';
 import { NovuService } from '../notifications/novu.service';
 import { FindingAuditService } from './finding-audit.service';
@@ -8,7 +9,7 @@ import { FindingsController } from './findings.controller';
 import { FindingsService } from './findings.service';
 
 @Module({
-  imports: [AuthModule, TimelinesModule],
+  imports: [AuthModule, RolesModule, TimelinesModule],
   controllers: [FindingsController],
   providers: [
     FindingsService,

apps/api/src/findings/findings.service.spec.ts

@@ -1,10 +1,22 @@
-import { BadRequestException, NotFoundException } from '@nestjs/common';
+import {
+  BadRequestException,
+  ForbiddenException,
+  NotFoundException,
+} from '@nestjs/common';
 
 jest.mock('@trycompai/company', () => ({
   toDbEvidenceFormType: (v: string) => v,
   toExternalEvidenceFormType: (v: string | null) => v,
 }));
 
+jest.mock('../timelines/timelines.service', () => ({
+  TimelinesService: jest.fn(),
+}));
+
+jest.mock('../frameworks/frameworks-timeline.helper', () => ({
+  checkAutoCompletePhases: jest.fn().mockResolvedValue(undefined),
+}));
+
 const mockDb = {
   task: { findFirst: jest.fn() },
   evidenceSubmission: { findFirst: jest.fn(), findUnique: jest.fn() },
@@ -13,6 +25,7 @@ const mockDb = {
   risk: { findFirst: jest.fn() },
   member: { findFirst: jest.fn(), findUnique: jest.fn() },
   device: { findFirst: jest.fn(), findUnique: jest.fn() },
+  user: { findUnique: jest.fn() },
   findingTemplate: { findUnique: jest.fn() },
   finding: {
     findFirst: jest.fn(),
@@ -57,6 +70,7 @@ describe('FindingsService.create (target validator)', () => {
   const svc = new FindingsService(
     auditService as never,
     notifier as never,
+    {} as never,
   );
   const baseDto = { content: 'Example finding' };
 
@@ -134,3 +148,84 @@ describe('FindingsService.create (target validator)', () => {
     expect(createArgs.data.taskId).toBeNull();
   });
 });
+
+describe('FindingsService.update (status transition rules)', () => {
+  const auditService = {
+    logFindingStatusChanged: jest.fn(),
+    logFindingContentChanged: jest.fn(),
+    logFindingTypeChanged: jest.fn(),
+  };
+  const notifier = {
+    notifyFindingCreated: jest.fn(),
+    notifyStatusChanged: jest.fn(),
+  };
+  const svc = new FindingsService(auditService as never, notifier as never, {} as never);
+  const existingFinding = {
+    id: 'fnd_1',
+    organizationId: 'org_1',
+    content: 'Test finding',
+    status: 'open',
+    type: 'soc2',
+    severity: 'medium',
+    revisionNote: null,
+    area: null,
+    taskId: null,
+    policyId: null,
+    vendorId: null,
+    riskId: null,
+    memberId: null,
+    deviceId: null,
+    evidenceSubmissionId: null,
+    evidenceFormType: null,
+    createdBy: null,
+    createdByAdmin: null,
+    createdAt: new Date(),
+    updatedAt: new Date(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockDb.finding.findFirst.mockResolvedValue(existingFinding);
+    mockDb.finding.update.mockResolvedValue({
+      ...existingFinding,
+      createdBy: null,
+      createdByAdmin: null,
+    });
+    mockDb.user.findUnique.mockResolvedValue({
+      name: 'Test User',
+      email: 'test@example.com',
+    });
+  });
+
+  it('allows ready_for_review regardless of canCreateFindings', async () => {
+    await svc.update('org_1', 'fnd_1', { status: 'ready_for_review' as never }, false, false, 'usr_1', 'mem_1');
+    expect(mockDb.finding.update).toHaveBeenCalled();
+  });
+
+  it('blocks needs_revision without canCreateFindings', async () => {
+    await expect(
+      svc.update('org_1', 'fnd_1', { status: 'needs_revision' as never }, false, false, 'usr_1', 'mem_1'),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+  });
+
+  it('allows needs_revision with canCreateFindings', async () => {
+    await svc.update('org_1', 'fnd_1', { status: 'needs_revision' as never }, true, false, 'usr_1', 'mem_1');
+    expect(mockDb.finding.update).toHaveBeenCalled();
+  });
+
+  it('blocks closed without canCreateFindings', async () => {
+    await expect(
+      svc.update('org_1', 'fnd_1', { status: 'closed' as never }, false, false, 'usr_1', 'mem_1'),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+  });
+
+  it('allows closed with canCreateFindings', async () => {
+    await svc.update('org_1', 'fnd_1', { status: 'closed' as never }, true, false, 'usr_1', 'mem_1');
+    expect(mockDb.finding.update).toHaveBeenCalled();
+  });
+
+  it('allows platform admin to set any status', async () => {
+    await svc.update('org_1', 'fnd_1', { status: 'closed' as never }, false, true, 'usr_1', 'mem_1');
+    expect(mockDb.finding.update).toHaveBeenCalled();
+  });
+});

apps/api/src/findings/findings.service.ts

@@ -364,7 +364,7 @@ export class FindingsService {
     organizationId: string,
     findingId: string,
     updateDto: UpdateFindingDto,
-    userRoles: string[],
+    canCreateFindings: boolean,
     isPlatformAdmin: boolean,
     userId: string,
     memberId: string | null,
@@ -375,26 +375,14 @@ export class FindingsService {
     const previousContent = finding.content;
 
     if (updateDto.status) {
-      const isAuditor = userRoles.includes('auditor');
-      const canSetRestrictedStatus = isPlatformAdmin || isAuditor;
-
       if (
         (updateDto.status === FindingStatus.needs_revision ||
           updateDto.status === FindingStatus.closed) &&
-        !canSetRestrictedStatus
-      ) {
-        throw new ForbiddenException(
-          `Only auditors or platform admins can set status to '${updateDto.status}'`,
-        );
-      }
-
-      if (
-        updateDto.status === FindingStatus.ready_for_review &&
-        isAuditor &&
+        !canCreateFindings &&
         !isPlatformAdmin
       ) {
         throw new ForbiddenException(
-          `Auditors cannot set status to 'ready_for_review'. This status is for clients to signal readiness.`,
+          `Only auditors or platform admins can set status to '${updateDto.status}'`,
         );
       }
     }

apps/app/src/app/(app)/[orgId]/overview/components/FindingDetailSheet.tsx

@@ -64,18 +64,18 @@ interface FindingDetailSheetProps {
  */
 function allowedStatusOptions({
   current,
-  isAuditor,
+  canCreateFindings,
   isPlatformAdmin,
 }: {
   current: FindingStatus;
-  isAuditor: boolean;
+  canCreateFindings: boolean;
   isPlatformAdmin: boolean;
 }): FindingStatus[] {
-  const canSetRestricted = isAuditor || isPlatformAdmin;
-  const canSetReadyForReview = !isAuditor || isPlatformAdmin;
-  const options: FindingStatus[] = [FindingStatus.open];
-  if (canSetReadyForReview) options.push(FindingStatus.ready_for_review);
-  if (canSetRestricted) {
+  const options: FindingStatus[] = [
+    FindingStatus.open,
+    FindingStatus.ready_for_review,
+  ];
+  if (canCreateFindings || isPlatformAdmin) {
     options.push(FindingStatus.needs_revision, FindingStatus.closed);
   }
   if (!options.includes(current)) options.push(current);
@@ -156,18 +156,13 @@ export function FindingDetailSheet({
   onSaved,
   onDeleted,
 }: FindingDetailSheetProps) {
-  const { hasPermission, roles } = usePermissions();
+  const { hasPermission } = usePermissions();
   const { data: session } = useSession();
   const canUpdate = hasPermission('finding', 'update');
   const canDelete = hasPermission('finding', 'delete');
-  // Match the API's literal role check (`userRoles.includes('auditor')` in
-  // findings.service.ts). A custom role granting `finding:create` does NOT
-  // count as auditor on the server, so we can't proxy via permissions here.
-  const isAuditor = roles.includes('auditor');
+  const canCreateFindings = hasPermission('finding', 'create');
   const isPlatformAdmin = session?.user?.role === 'admin';
-  // Only auditors can rewrite a finding's content; owners/admins can still
-  // move the status forward but the audit narrative belongs to the auditor.
-  const canEditContent = canUpdate && isAuditor;
+  const canEditContent = canUpdate && canCreateFindings;
   const { updateFinding, deleteFinding } = useFindingActions();
   const { data: historyData } = useFindingHistory(finding?.id ?? null);
 
@@ -353,7 +348,7 @@ export function FindingDetailSheet({
                   <SelectContent>
                     {allowedStatusOptions({
                       current: status,
-                      isAuditor,
+                      canCreateFindings,
                       isPlatformAdmin,
                     }).map((s) => (
                       <SelectItem key={s} value={s}>