Skip to content
Open
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
39 changes: 39 additions & 0 deletions apps/api/src/attachments/attachments.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
CopyObjectCommand,
DeleteObjectCommand,
GetObjectCommand,
PutObjectCommand,
Expand Down Expand Up @@ -334,6 +335,44 @@ export class AttachmentsService {
}
}

/**
* Copy a policy PDF to a new S3 key for versioning
*/
async copyPolicyVersionPdf(
sourceKey: string,
destinationKey: string,
): Promise<string | null> {
try {
await this.s3Client.send(
new CopyObjectCommand({
Bucket: this.bucketName,
CopySource: `${this.bucketName}/${sourceKey}`,
Key: destinationKey,
}),
);
return destinationKey;
} catch (error) {
console.error('Error copying policy PDF:', error);
return null;
}
}

/**
* Delete a policy version PDF from S3
*/
async deletePolicyVersionPdf(s3Key: string): Promise<void> {
try {
await this.s3Client.send(
new DeleteObjectCommand({
Bucket: this.bucketName,
Key: s3Key,
}),
);
} catch (error) {
console.error('Error deleting policy PDF:', error);
}
}

/**
* Generate signed URL for file download
*/
Expand Down
12 changes: 12 additions & 0 deletions apps/api/src/findings/dto/update-finding.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,16 @@ export class UpdateFindingDto {
@IsNotEmpty({ message: 'Content cannot be empty if provided' })
@MaxLength(5000)
content?: string;

@ApiProperty({
description: 'Auditor note when requesting revision (only for needs_revision status)',
example: 'Please provide clearer screenshots showing the timestamp.',
maxLength: 2000,
required: false,
nullable: true,
})
@IsString()
@IsOptional()
@MaxLength(2000)
revisionNote?: string | null;
Copy link

Choose a reason for hiding this comment

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

Missing explicit type causes incorrect OpenAPI schema

Low Severity

The @ApiProperty decorator for revisionNote is missing an explicit type: String specification. Combined with nullable: true and TypeScript type string | null, this causes NestJS Swagger to generate "type": "object" in the OpenAPI spec instead of "type": "string". Add type: String to the decorator options.

Fix in Cursor Fix in Web

}
54 changes: 43 additions & 11 deletions apps/api/src/findings/findings.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ export class FindingsService {
],
});

this.logger.log(
`Retrieved ${findings.length} findings for task ${taskId}`,
);
this.logger.log(`Retrieved ${findings.length} findings for task ${taskId}`);
return findings;
}

Expand Down Expand Up @@ -311,19 +309,38 @@ export class FindingsService {
}

// ready_for_review can only be set by non-auditor admins/owners (client signals to auditor)
if (updateDto.status === FindingStatus.ready_for_review && isAuditor && !isPlatformAdmin) {
if (
updateDto.status === FindingStatus.ready_for_review &&
isAuditor &&
!isPlatformAdmin
) {
throw new ForbiddenException(
`Auditors cannot set status to 'ready_for_review'. This status is for clients to signal readiness.`,
);
}
}

// Handle revisionNote logic:
// - Set revisionNote when status is needs_revision and a note is provided
// - Clear revisionNote when status changes to anything other than needs_revision
let revisionNoteUpdate: { revisionNote?: string | null } = {};
if (updateDto.status === FindingStatus.needs_revision) {
// Set revision note if provided (can be null to clear)
if (updateDto.revisionNote !== undefined) {
revisionNoteUpdate = { revisionNote: updateDto.revisionNote || null };
}
} else if (updateDto.status !== undefined) {
// Clear revision note when moving to a different status
revisionNoteUpdate = { revisionNote: null };
}

const updatedFinding = await db.finding.update({
where: { id: findingId },
data: {
...(updateDto.status !== undefined && { status: updateDto.status }),
...(updateDto.type !== undefined && { type: updateDto.type }),
...(updateDto.content !== undefined && { content: updateDto.content }),
...revisionNoteUpdate,
},
include: {
createdBy: {
Expand Down Expand Up @@ -411,22 +428,34 @@ export class FindingsService {

switch (updateDto.status) {
case FindingStatus.ready_for_review:
this.logger.log(`Triggering 'ready_for_review' notification for finding ${findingId}`);
this.logger.log(
`Triggering 'ready_for_review' notification for finding ${findingId}`,
);
void this.findingNotifierService.notifyReadyForReview({
...notificationParams,
findingCreatorMemberId: finding.createdById,
});
break;
case FindingStatus.needs_revision:
this.logger.log(`Triggering 'needs_revision' notification for finding ${findingId}`);
void this.findingNotifierService.notifyNeedsRevision(notificationParams);
this.logger.log(
`Triggering 'needs_revision' notification for finding ${findingId}`,
);
void this.findingNotifierService.notifyNeedsRevision(
notificationParams,
);
break;
case FindingStatus.closed:
this.logger.log(`Triggering 'closed' notification for finding ${findingId}`);
void this.findingNotifierService.notifyFindingClosed(notificationParams);
this.logger.log(
`Triggering 'closed' notification for finding ${findingId}`,
);
void this.findingNotifierService.notifyFindingClosed(
notificationParams,
);
break;
case FindingStatus.open:
this.logger.log(`Status changed to 'open' for finding ${findingId}. No notification sent.`);
this.logger.log(
`Status changed to 'open' for finding ${findingId}. No notification sent.`,
);
break;
default:
this.logger.warn(
Expand Down Expand Up @@ -489,6 +518,9 @@ export class FindingsService {
// Verify finding exists
await this.findById(organizationId, findingId);

return this.findingAuditService.getFindingActivity(findingId, organizationId);
return this.findingAuditService.getFindingActivity(
findingId,
organizationId,
);
}
}
22 changes: 17 additions & 5 deletions apps/api/src/lib/fleet.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,20 @@ export class FleetService {
}
}

/**
* Remove a single host from FleetDM by ID
* @param hostId - The FleetDM host ID
*/
async removeHostById(hostId: number): Promise<void> {
try {
await this.fleetInstance.delete(`/hosts/${hostId}`);
this.logger.debug(`Deleted host ${hostId} from FleetDM`);
} catch (error) {
this.logger.error(`Failed to delete host ${hostId}:`, error);
throw new Error(`Failed to remove host ${hostId}`);
}
}

async getMultipleHosts(hostIds: number[]) {
try {
const requests = hostIds.map((id) => this.getHostById(id));
Expand Down Expand Up @@ -104,14 +118,12 @@ export class FleetService {
// Extract host IDs
const hostIds = labelHosts.hosts.map((host: { id: number }) => host.id);

// Delete each host
// Delete each host using removeHostById for consistent behavior
const deletePromises = hostIds.map(async (hostId: number) => {
try {
await this.fleetInstance.delete(`/hosts/${hostId}`);
this.logger.debug(`Deleted host ${hostId} from FleetDM`);
await this.removeHostById(hostId);
return { success: true, hostId };
} catch (error) {
this.logger.error(`Failed to delete host ${hostId}:`, error);
} catch {
return { success: false, hostId };
}
});
Expand Down
38 changes: 38 additions & 0 deletions apps/api/src/people/people.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Delete,
Body,
Param,
ParseIntPipe,
UseGuards,
HttpCode,
HttpStatus,
Expand All @@ -22,6 +23,7 @@ import {
} from '@nestjs/swagger';
import { AuthContext, OrganizationId } from '../auth/auth-context.decorator';
import { HybridAuthGuard } from '../auth/hybrid-auth.guard';
import { RequireRoles } from '../auth/role-validator.guard';
import type { AuthContext as AuthContextType } from '../auth/types';
import { CreatePeopleDto } from './dto/create-people.dto';
import { UpdatePeopleDto } from './dto/update-people.dto';
Expand All @@ -34,6 +36,7 @@ import { BULK_CREATE_MEMBERS_RESPONSES } from './schemas/bulk-create-members.res
import { GET_PERSON_BY_ID_RESPONSES } from './schemas/get-person-by-id.responses';
import { UPDATE_MEMBER_RESPONSES } from './schemas/update-member.responses';
import { DELETE_MEMBER_RESPONSES } from './schemas/delete-member.responses';
import { REMOVE_HOST_RESPONSES } from './schemas/remove-host.responses';
import { PEOPLE_OPERATIONS } from './schemas/people-operations';
import { PEOPLE_PARAMS } from './schemas/people-params';
import { PEOPLE_BODIES } from './schemas/people-bodies';
Expand Down Expand Up @@ -199,6 +202,41 @@ export class PeopleController {
};
}

@Delete(':id/host/:hostId')
@HttpCode(HttpStatus.OK)
@UseGuards(RequireRoles('owner'))
@ApiOperation(PEOPLE_OPERATIONS.removeHost)
@ApiParam(PEOPLE_PARAMS.memberId)
@ApiParam(PEOPLE_PARAMS.hostId)
@ApiResponse(REMOVE_HOST_RESPONSES[200])
@ApiResponse(REMOVE_HOST_RESPONSES[401])
@ApiResponse(REMOVE_HOST_RESPONSES[404])
@ApiResponse(REMOVE_HOST_RESPONSES[500])
async removeHost(
@Param('id') memberId: string,
@Param('hostId', ParseIntPipe) hostId: number,
@OrganizationId() organizationId: string,
@AuthContext() authContext: AuthContextType,
) {
const result = await this.peopleService.removeHostById(
memberId,
organizationId,
hostId,
);

return {
...result,
authType: authContext.authType,
...(authContext.userId &&
authContext.userEmail && {
authenticatedUser: {
id: authContext.userId,
email: authContext.userEmail,
},
}),
};
}

@Delete(':id')
@ApiOperation(PEOPLE_OPERATIONS.deleteMember)
@ApiParam(PEOPLE_PARAMS.memberId)
Expand Down
57 changes: 57 additions & 0 deletions apps/api/src/people/people.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,4 +338,61 @@ export class PeopleService {
throw new Error(`Failed to unlink device: ${error.message}`);
}
}

async removeHostById(
memberId: string,
organizationId: string,
hostId: number,
): Promise<{ success: true }> {
try {
await MemberValidator.validateOrganization(organizationId);
const member = await MemberQueries.findByIdInOrganization(
memberId,
organizationId,
);

if (!member) {
throw new NotFoundException(
`Member with ID ${memberId} not found in organization ${organizationId}`,
);
}

if (!member.fleetDmLabelId) {
throw new BadRequestException(
`Member ${memberId} has no Fleet label; cannot remove host`,
);
}

const labelHosts = await this.fleetService.getHostsByLabel(
member.fleetDmLabelId,
);
const hostIds = (labelHosts?.hosts ?? []).map(
(host: { id: number }) => host.id,
);
if (!hostIds.includes(hostId)) {
throw new NotFoundException(
`Host ${hostId} not found for member ${memberId} in organization ${organizationId}`,
);
}

await this.fleetService.removeHostById(hostId);

this.logger.log(
`Removed host ${hostId} from FleetDM for member ${memberId} in organization ${organizationId}`,
);
return { success: true };
} catch (error) {
if (
error instanceof NotFoundException ||
error instanceof BadRequestException
) {
throw error;
}
this.logger.error(
`Failed to remove host ${hostId} for member ${memberId} in organization ${organizationId}:`,
error,
);
throw new Error(`Failed to remove host: ${error.message}`);
}
}
}
5 changes: 5 additions & 0 deletions apps/api/src/people/schemas/people-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ export const PEOPLE_OPERATIONS: Record<string, ApiOperationOptions> = {
description:
'Resets the fleetDmLabelId for a member, effectively unlinking their device from FleetDM. This will disconnect the device from the organization. Supports both API key authentication (X-API-Key header) and session authentication (cookies + X-Organization-Id header).',
},
removeHost: {
summary: 'Remove host (device) from Fleet',
description:
'Removes a single host (device) from FleetDM by host ID. Only organization owners can perform this action. Validates that the organization exists and the member exists within the organization. Supports both API key authentication (X-API-Key header) and session authentication (cookies + X-Organization-Id header).',
},
};
5 changes: 5 additions & 0 deletions apps/api/src/people/schemas/people-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,9 @@ export const PEOPLE_PARAMS: Record<string, ApiParamOptions> = {
description: 'Member ID',
example: 'mem_abc123def456',
},
hostId: {
name: 'hostId',
description: 'FleetDM host ID',
example: 1,
},
};
Loading
Loading