Skip to content

Commit b4a8a29

Browse files
committed
updated from develop
2 parents 7739b8b + f8ab7f8 commit b4a8a29

File tree

4 files changed

+75
-30
lines changed

4 files changed

+75
-30
lines changed

src/api/ai-workflow/ai-workflow.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ export class AiWorkflowService {
159159
}
160160

161161
if (!user.userId) {
162-
throw new BadRequestException(`User id not available`);
162+
throw new BadRequestException(`User id is not available`);
163163
}
164164

165165
const createdComment = await this.prisma.aiWorkflowRunItemComment.create({

src/api/review/review.service.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,28 @@ describe('ReviewService.updateReview challenge status enforcement', () => {
407407
expect(recomputeSpy).toHaveBeenCalledWith('review-1');
408408
});
409409

410+
it('rejects attempts to change immutable identifiers', async () => {
411+
await expect(
412+
service.updateReview(nonPrivilegedUser, 'review-1', {
413+
...updatePayload,
414+
resourceId: 'new-resource',
415+
}),
416+
).rejects.toMatchObject({
417+
status: 400,
418+
response: expect.objectContaining({
419+
code: 'REVIEW_UPDATE_IMMUTABLE_FIELDS',
420+
details: expect.objectContaining({
421+
reviewId: 'review-1',
422+
fields: ['resourceId'],
423+
}),
424+
}),
425+
});
426+
427+
expect(prismaMock.review.findUnique).not.toHaveBeenCalled();
428+
expect(prismaMock.review.update).not.toHaveBeenCalled();
429+
expect(challengeApiServiceMock.getChallengeDetail).not.toHaveBeenCalled();
430+
});
431+
410432
it('allows reviewer tokens to update when the challenge is not completed', async () => {
411433
const result = await service.updateReview(
412434
nonPrivilegedUser,

src/api/review/review.service.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,21 @@ export class ReviewService {
610610
): Promise<ReviewResponseDto> {
611611
this.logger.log(`Updating review with ID: ${id}`);
612612

613+
const immutableFields = ['resourceId', 'phaseId', 'submissionId'] as const;
614+
const forbiddenUpdates = immutableFields.filter(
615+
(field) => (body as Record<string, unknown>)[field] !== undefined,
616+
);
617+
618+
if (forbiddenUpdates.length > 0) {
619+
throw new BadRequestException({
620+
message: `The following fields cannot be updated: ${forbiddenUpdates.join(
621+
', ',
622+
)}.`,
623+
code: 'REVIEW_UPDATE_IMMUTABLE_FIELDS',
624+
details: { reviewId: id, fields: forbiddenUpdates },
625+
});
626+
}
627+
613628
const existingReview = await this.prisma.review.findUnique({
614629
where: { id },
615630
include: {

src/dto/review.dto.ts

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ApiProperty } from '@nestjs/swagger';
1+
import { ApiHideProperty, ApiProperty, OmitType } from '@nestjs/swagger';
22
import { Type } from 'class-transformer';
33
import {
44
IsString,
@@ -12,6 +12,7 @@ import {
1212
ValidateNested,
1313
IsEnum,
1414
IsInt,
15+
IsEmpty,
1516
} from 'class-validator';
1617

1718
export enum ReviewItemCommentType {
@@ -282,35 +283,38 @@ export class ReviewRequestDto extends ReviewCommonDto {
282283
reviewItems?: ReviewItemRequestDto[];
283284
}
284285

285-
export class ReviewPutRequestDto extends ReviewCommonDto {}
286+
const ReviewPutBase = OmitType(ReviewCommonDto, [
287+
'resourceId',
288+
'phaseId',
289+
'submissionId',
290+
] as const);
291+
292+
export class ReviewPutRequestDto extends ReviewPutBase {
293+
@ApiHideProperty()
294+
@IsEmpty({ message: 'resourceId cannot be updated.' })
295+
resourceId?: never;
296+
297+
@ApiHideProperty()
298+
@IsEmpty({ message: 'phaseId cannot be updated.' })
299+
phaseId?: never;
300+
301+
@ApiHideProperty()
302+
@IsEmpty({ message: 'submissionId cannot be updated.' })
303+
submissionId?: never;
304+
}
286305

287306
export class ReviewPatchRequestDto {
288-
@ApiProperty({
289-
description: 'Resource ID associated with the review',
290-
example: 'resource123',
291-
})
292-
@IsOptional()
293-
@IsString()
294-
@IsNotEmpty()
295-
resourceId?: string;
307+
@ApiHideProperty()
308+
@IsEmpty({ message: 'resourceId cannot be updated.' })
309+
resourceId?: never;
296310

297-
@ApiProperty({
298-
description: 'Phase ID of the challenge',
299-
example: 'phase456',
300-
})
301-
@IsOptional()
302-
@IsString()
303-
@IsNotEmpty()
304-
phaseId?: string;
311+
@ApiHideProperty()
312+
@IsEmpty({ message: 'phaseId cannot be updated.' })
313+
phaseId?: never;
305314

306-
@ApiProperty({
307-
description: 'Submission ID being reviewed',
308-
example: 'submission789',
309-
})
310-
@IsOptional()
311-
@IsString()
312-
@IsNotEmpty()
313-
submissionId?: string;
315+
@ApiHideProperty()
316+
@IsEmpty({ message: 'submissionId cannot be updated.' })
317+
submissionId?: never;
314318

315319
@ApiProperty({
316320
description: 'Scorecard ID used for the review',
@@ -450,9 +454,13 @@ export function mapReviewRequestToDto(
450454
},
451455
};
452456
} else {
453-
return {
454-
...request,
455-
};
457+
const sanitizedRequest = { ...request } as Partial<ReviewPatchRequestDto> &
458+
Record<string, unknown>;
459+
['resourceId', 'phaseId', 'submissionId'].forEach((field) => {
460+
delete sanitizedRequest[field];
461+
});
462+
463+
return sanitizedRequest as ReviewPatchRequestDto;
456464
}
457465
}
458466

0 commit comments

Comments
 (0)