Merge pull request #23 from topcoder-platform/dev

Prod deploy - May 2026

Author: jmgasper

.github/workflows/code_reviewer.yml

@@ -0,0 +1,26 @@
+import { plainToInstance } from 'class-transformer';
+import { validate } from 'class-validator';
+import { UpdateProjectDto } from './update-project.dto';
+
+describe('UpdateProjectDto', () => {
+  it.each([null, ''])(
+    'preserves %p billingAccountId as an explicit clear request',
+    async (billingAccountId) => {
+      const dto = plainToInstance(UpdateProjectDto, {
+        billingAccountId,
+      });
+
+      expect(dto.billingAccountId).toBeNull();
+      await expect(validate(dto)).resolves.toHaveLength(0);
+    },
+  );
+
+  it('parses numeric billingAccountId updates', async () => {
+    const dto = plainToInstance(UpdateProjectDto, {
+      billingAccountId: '80001063',
+    });
+
+    expect(dto.billingAccountId).toBe(80001063);
+    await expect(validate(dto)).resolves.toHaveLength(0);
+  });
+});

src/api/project/dto/update-project.dto.spec.ts

@@ -1,8 +1,33 @@
-import { ApiHideProperty } from '@nestjs/swagger';
+import { ApiHideProperty, ApiPropertyOptional } from '@nestjs/swagger';
 import { Transform } from 'class-transformer';
-import { PartialType } from '@nestjs/mapped-types';
+import { IsNumber, IsOptional } from 'class-validator';
+import { OmitType, PartialType } from '@nestjs/mapped-types';
 import { CreateProjectDto } from './create-project.dto';
 
+/**
+ * Parses optional project billing-account update input.
+ *
+ * @param value Raw `billingAccountId` value from request payload.
+ * @returns Parsed integer, `null` when clearing, or `undefined` when omitted.
+ */
+function parseOptionalNullableInteger(value: unknown): number | null | undefined {
+  if (typeof value === 'undefined') {
+    return undefined;
+  }
+
+  if (value === null || value === '') {
+    return null;
+  }
+
+  const parsed = Number(value);
+
+  if (Number.isNaN(parsed)) {
+    return undefined;
+  }
+
+  return Math.trunc(parsed);
+}
+
 /**
  * Resolves whether the patch payload explicitly requests clearing
  * `billingAccountId`.
@@ -17,9 +42,21 @@ function parseClearBillingAccountFlag(value: unknown): boolean {
 /**
  * Request DTO for `PATCH /projects/:projectId`.
  *
- * Reuses `CreateProjectDto` and makes all fields optional via `PartialType`.
+ * Reuses `CreateProjectDto`, makes all fields optional via `PartialType`, and
+ * allows `billingAccountId` to be explicitly cleared with `null` or `''`.
  */
-export class UpdateProjectDto extends PartialType(CreateProjectDto) {
+export class UpdateProjectDto extends PartialType(
+  OmitType(CreateProjectDto, ['billingAccountId'] as const),
+) {
+  @ApiPropertyOptional({
+    description: 'Project billing account id. Send null or empty string to clear.',
+    nullable: true,
+  })
+  @IsOptional()
+  @Transform(({ value }) => parseOptionalNullableInteger(value))
+  @IsNumber()
+  billingAccountId?: number | null;
+
   @ApiHideProperty()
   @Transform(({ obj }) => parseClearBillingAccountFlag(obj?.billingAccountId))
   clearBillingAccountId?: boolean;

src/api/project/dto/update-project.dto.ts

@@ -273,7 +273,7 @@ export class ProjectController {
    * @throws UnauthorizedException When the caller is unauthenticated.
    * @throws ForbiddenException When the caller lacks required permissions.
    * @throws NotFoundException When project or billing account is missing.
-   * @security The service strips `markup` for non-machine callers before
+   * @security The service strips `markup` for copilot-only callers before
    * returning the response payload.
    */
   @Get(':projectId/billingAccount')

src/api/project/project.controller.ts

@@ -604,7 +604,7 @@ describe('ProjectService', () => {
     ).toHaveBeenCalledWith('1001', '123');
   });
 
-  it('returns project billing account and strips markup for user tokens', async () => {
+  it('returns project billing account and strips markup for copilot-only user tokens', async () => {
     prismaMock.project.findFirst.mockResolvedValue({
       id: BigInt(1001),
       billingAccountId: BigInt(12),
@@ -617,6 +617,7 @@ describe('ProjectService', () => {
 
     const result = await service.getProjectBillingAccount('1001', {
       userId: '123',
+      roles: [UserRole.TC_COPILOT],
       isMachine: false,
     });
 
@@ -629,6 +630,30 @@ describe('ProjectService', () => {
     ).toHaveBeenCalledWith('12');
   });
 
+  it('returns project billing account markup for Project Manager tokens', async () => {
+    prismaMock.project.findFirst.mockResolvedValue({
+      id: BigInt(1001),
+      billingAccountId: BigInt(12),
+    });
+    billingAccountServiceMock.getDefaultBillingAccount.mockResolvedValue({
+      tcBillingAccountId: '12',
+      markup: 50,
+      active: true,
+    });
+
+    const result = await service.getProjectBillingAccount('1001', {
+      userId: '123',
+      roles: [UserRole.PROJECT_MANAGER],
+      isMachine: false,
+    });
+
+    expect(result).toEqual({
+      tcBillingAccountId: '12',
+      markup: 50,
+      active: true,
+    });
+  });
+
   it('returns project billing account markup for m2m tokens', async () => {
     prismaMock.project.findFirst.mockResolvedValue({
       id: BigInt(1001),
@@ -1621,8 +1646,8 @@ describe('ProjectService', () => {
     await service.updateProject(
       '1001',
       {
-        clearBillingAccountId: true,
-      } as any,
+        billingAccountId: null,
+      },
       {
         userId: '100',
         isMachine: false,

src/api/project/project.service.spec.ts

@@ -49,6 +49,24 @@ import { ProjectWithRelationsDto } from './dto/project-response.dto';
 import { UpgradeProjectDto } from './dto/upgrade-project.dto';
 import { UpdateProjectDto } from './dto/update-project.dto';
 
+const BILLING_MARKUP_COPILOT_ROLES = [
+  UserRole.COPILOT,
+  UserRole.TC_COPILOT,
+];
+
+const BILLING_MARKUP_VISIBLE_HUMAN_ROLES = [
+  ...ADMIN_ROLES,
+  UserRole.MANAGER,
+  UserRole.TOPCODER_MANAGER,
+  UserRole.TOPCODER_ACCOUNT_MANAGER,
+  UserRole.COPILOT_MANAGER,
+  UserRole.PROJECT_MANAGER,
+  UserRole.TASK_MANAGER,
+  UserRole.TOPCODER_TASK_MANAGER,
+  UserRole.TALENT_MANAGER,
+  UserRole.TOPCODER_TALENT_MANAGER,
+];
+
 interface PaginatedProjectResponse {
   data: ProjectWithRelationsDto[];
   page: number;
@@ -648,8 +666,10 @@ export class ProjectService {
       throw new ForbiddenException('Insufficient permissions');
     }
 
+    const shouldClearBillingAccountId =
+      dto.clearBillingAccountId === true || dto.billingAccountId === null;
     const requestedBillingAccountId =
-      dto.clearBillingAccountId === true
+      shouldClearBillingAccountId
         ? null
         : typeof dto.billingAccountId === 'number'
           ? String(dto.billingAccountId)
@@ -708,7 +728,7 @@ export class ProjectService {
           cancelReason:
             typeof dto.cancelReason === 'string' ? dto.cancelReason : undefined,
           billingAccountId:
-            dto.clearBillingAccountId === true
+            shouldClearBillingAccountId
               ? null
               : typeof dto.billingAccountId === 'number'
                 ? BigInt(dto.billingAccountId)
@@ -1038,8 +1058,9 @@ export class ProjectService {
    * @param user Authenticated caller context.
    * @returns Default billing account details.
    * @throws NotFoundException When project or billing account is missing.
-   * @security Removes `markup` for non-machine callers to avoid exposing
-   * markup details to interactive users.
+   * @security Removes `markup` for copilot-only callers while preserving the
+   * existing response shape for M2M, Project Manager, Talent Manager, and
+   * administrator callers.
    */
   async getProjectBillingAccount(
     projectId: string,
@@ -1083,7 +1104,7 @@ export class ProjectService {
           tcBillingAccountId: projectBillingAccountId,
         };
 
-    if (this.isMachinePrincipal(user)) {
+    if (!this.shouldHideBillingAccountMarkupForCopilot(user)) {
       return billingAccount;
     }
 
@@ -2175,6 +2196,37 @@ export class ProjectService {
     return false;
   }
 
+  /**
+   * Determines whether project billing-account markup must be hidden.
+   *
+   * Copilot-only human callers should not receive raw billing-account markup.
+   * Manager, Talent Manager, and administrator roles retain the existing
+   * response shape even when the same token also carries a copilot role.
+   *
+   * @param user Authenticated caller context.
+   * @returns `true` when billing-account markup should be removed.
+   */
+  private shouldHideBillingAccountMarkupForCopilot(user: JwtUser): boolean {
+    if (this.isMachinePrincipal(user)) {
+      return false;
+    }
+
+    const roles = user.roles || [];
+    const hasCopilotRole = this.permissionService.hasIntersection(
+      roles,
+      BILLING_MARKUP_COPILOT_ROLES,
+    );
+
+    if (!hasCopilotRole) {
+      return false;
+    }
+
+    return !this.permissionService.hasIntersection(
+      roles,
+      BILLING_MARKUP_VISIBLE_HUMAN_ROLES,
+    );
+  }
+
   /**
    * Safely extracts template phase objects from JSON payload.
    *

src/api/project/project.service.ts

@@ -265,11 +265,7 @@ export const PERMISSION = {
         'There are additional limitations on editing some parts of the project.',
     },
     topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
-    projectRoles: [
-      ...PROJECT_ROLES_MANAGEMENT,
-      PROJECT_MEMBER_ROLE.COPILOT,
-      PROJECT_MEMBER_ROLE.CUSTOMER,
-    ],
+    projectRoles: PROJECT_ROLES_MANAGEMENT,
     scopes: SCOPES_PROJECTS_WRITE,
   },
 

src/shared/constants/permissions.constants.ts

@@ -2,6 +2,7 @@ import { ProjectMemberRole } from '../enums/projectMemberRole.enum';
 import { Scope } from '../enums/scopes.enum';
 import { UserRole } from '../enums/userRole.enum';
 import { Permission } from '../constants/permissions';
+import { PERMISSION } from '../constants/permissions.constants';
 import { PermissionService } from './permission.service';
 
 describe('PermissionService', () => {
@@ -722,6 +723,31 @@ describe('PermissionService', () => {
     expect(allowed).toBe(true);
   });
 
+  it('blocks project copilots from editing project details', () => {
+    const user = {
+      userId: '3001',
+      roles: [UserRole.TC_COPILOT],
+      isMachine: false,
+    };
+    const projectMembers = [
+      {
+        userId: '3001',
+        role: ProjectMemberRole.COPILOT,
+      },
+    ];
+
+    expect(
+      service.hasNamedPermission(
+        Permission.EDIT_PROJECT,
+        user,
+        projectMembers,
+      ),
+    ).toBe(false);
+    expect(
+      service.matchPermissionRule(PERMISSION.UPDATE_PROJECT, user, projectMembers),
+    ).toBe(false);
+  });
+
   it('allows deleting project for machine token with project write scope', () => {
     const allowed = service.hasNamedPermission(Permission.DELETE_PROJECT, {
       scopes: [Scope.PROJECTS_ALL],

src/shared/services/permission.service.spec.ts

@@ -271,7 +271,6 @@ export class PermissionService {
         return (
           isAdmin ||
           isManagementMember ||
-          this.isCopilot(member?.role) ||
           this.hasProjectUpdateTopcoderRole(user) ||
           hasMachineProjectWriteScope
         );

src/shared/services/permission.service.ts

@@ -255,7 +255,7 @@ function getNamedPermissionDocumentation(
     case NamedPermission.EDIT_PROJECT:
       return createSummary({
         userRoles: PROJECT_UPDATE_TOPCODER_ROLES,
-        projectRoles: PROJECT_MEMBER_MANAGEMENT_AND_COPILOT_ROLES,
+        projectRoles: PROJECT_MEMBER_MANAGEMENT_ROLES,
         scopes: PROJECT_WRITE_SCOPES,
       });