From 121ec14180c619df923537528c56ccbd76a7d047 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 27 Apr 2026 09:31:01 +1000 Subject: [PATCH 1/4] Changes for hiding markup / challenge fee from copilots --- src/api/project/project.controller.ts | 2 +- src/api/project/project.service.spec.ts | 27 +++++++++++- src/api/project/project.service.ts | 56 +++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/api/project/project.controller.ts b/src/api/project/project.controller.ts index 8ea95fc..67bc56b 100644 --- a/src/api/project/project.controller.ts +++ b/src/api/project/project.controller.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') diff --git a/src/api/project/project.service.spec.ts b/src/api/project/project.service.spec.ts index c3be179..fc4e9a4 100644 --- a/src/api/project/project.service.spec.ts +++ b/src/api/project/project.service.spec.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), diff --git a/src/api/project/project.service.ts b/src/api/project/project.service.ts index e51db00..de3d963 100644 --- a/src/api/project/project.service.ts +++ b/src/api/project/project.service.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; @@ -1038,8 +1056,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 +1102,7 @@ export class ProjectService { tcBillingAccountId: projectBillingAccountId, }; - if (this.isMachinePrincipal(user)) { + if (!this.shouldHideBillingAccountMarkupForCopilot(user)) { return billingAccount; } @@ -2175,6 +2194,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. * From fcce01eb3f7b471ec37be052eeeb83dd4137a27f Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 29 Apr 2026 11:20:38 +1000 Subject: [PATCH 2/4] PM-4904: Allow project billing account clears What was broken Clearing a billing account from an existing project did not persist. The Work UI sent an explicit null billingAccountId, but the Projects API treated that value as omitted, so the old billing account stayed on the project and appeared again after save. Root cause UpdateProjectDto inherited CreateProjectDto billingAccountId parsing, which normalizes null and empty string to undefined. The service clear path depended on a derived flag that is not materialized by class-transformer when only billingAccountId is sent. What was changed UpdateProjectDto now owns the update-time billingAccountId field, preserving null and empty string as explicit clear requests while still parsing numeric billing account ids. ProjectService now clears when the DTO billingAccountId is null, in addition to the existing internal clear flag. Any added/updated tests Added DTO coverage for null, empty string, and numeric billingAccountId updates. Updated the project service clear test to exercise billingAccountId: null directly. --- .../project/dto/update-project.dto.spec.ts | 26 +++++++++++ src/api/project/dto/update-project.dto.ts | 45 +++++++++++++++++-- src/api/project/project.service.spec.ts | 4 +- src/api/project/project.service.ts | 6 ++- 4 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 src/api/project/dto/update-project.dto.spec.ts diff --git a/src/api/project/dto/update-project.dto.spec.ts b/src/api/project/dto/update-project.dto.spec.ts new file mode 100644 index 0000000..9132e10 --- /dev/null +++ b/src/api/project/dto/update-project.dto.spec.ts @@ -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); + }); +}); diff --git a/src/api/project/dto/update-project.dto.ts b/src/api/project/dto/update-project.dto.ts index 80f6b13..b4aea56 100644 --- a/src/api/project/dto/update-project.dto.ts +++ b/src/api/project/dto/update-project.dto.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; diff --git a/src/api/project/project.service.spec.ts b/src/api/project/project.service.spec.ts index fc4e9a4..9df8331 100644 --- a/src/api/project/project.service.spec.ts +++ b/src/api/project/project.service.spec.ts @@ -1646,8 +1646,8 @@ describe('ProjectService', () => { await service.updateProject( '1001', { - clearBillingAccountId: true, - } as any, + billingAccountId: null, + }, { userId: '100', isMachine: false, diff --git a/src/api/project/project.service.ts b/src/api/project/project.service.ts index de3d963..9da39d1 100644 --- a/src/api/project/project.service.ts +++ b/src/api/project/project.service.ts @@ -666,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) @@ -726,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) From d99343130006a6251de44dcfd99289143e944aa7 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 4 May 2026 11:04:25 +1000 Subject: [PATCH 3/4] PM-4988: Restrict project edit permissions for copilots What was broken Copilot project members were treated as eligible for the EDIT_PROJECT permission, so users with copilot project access could update project details. Root cause The named EDIT_PROJECT permission explicitly allowed copilot project membership, and the legacy UPDATE_PROJECT policy metadata still listed copilot and customer project roles. What was changed Removed copilot membership from the EDIT_PROJECT permission check. Updated the legacy UPDATE_PROJECT policy and generated permission documentation summary so project detail edits are limited to management-level project roles, manager-tier platform roles, admins, and machine project-write tokens. Any added/updated tests Added PermissionService coverage that verifies a project copilot cannot edit project details through either the named permission path or the legacy UPDATE_PROJECT policy. --- src/shared/constants/permissions.constants.ts | 6 +---- .../services/permission.service.spec.ts | 26 +++++++++++++++++++ src/shared/services/permission.service.ts | 1 - src/shared/utils/permission-docs.utils.ts | 2 +- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/shared/constants/permissions.constants.ts b/src/shared/constants/permissions.constants.ts index 0dfff45..3beb9cd 100644 --- a/src/shared/constants/permissions.constants.ts +++ b/src/shared/constants/permissions.constants.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, }, diff --git a/src/shared/services/permission.service.spec.ts b/src/shared/services/permission.service.spec.ts index 747ffcb..ccd19ee 100644 --- a/src/shared/services/permission.service.spec.ts +++ b/src/shared/services/permission.service.spec.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], diff --git a/src/shared/services/permission.service.ts b/src/shared/services/permission.service.ts index b2df6c1..a10308e 100644 --- a/src/shared/services/permission.service.ts +++ b/src/shared/services/permission.service.ts @@ -271,7 +271,6 @@ export class PermissionService { return ( isAdmin || isManagementMember || - this.isCopilot(member?.role) || this.hasProjectUpdateTopcoderRole(user) || hasMachineProjectWriteScope ); diff --git a/src/shared/utils/permission-docs.utils.ts b/src/shared/utils/permission-docs.utils.ts index 76202d0..b9ba79e 100644 --- a/src/shared/utils/permission-docs.utils.ts +++ b/src/shared/utils/permission-docs.utils.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, }); From 33a0a15d195758fe2a0641408f3367980996ef5b Mon Sep 17 00:00:00 2001 From: Kiril Kartunov Date: Mon, 4 May 2026 10:21:29 +0300 Subject: [PATCH 4/4] Delete .github/workflows/code_reviewer.yml --- .github/workflows/code_reviewer.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .github/workflows/code_reviewer.yml diff --git a/.github/workflows/code_reviewer.yml b/.github/workflows/code_reviewer.yml deleted file mode 100644 index 9b6a6ce..0000000 --- a/.github/workflows/code_reviewer.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: AI PR Reviewer - -on: - pull_request: - types: - - opened - - synchronize -permissions: - pull-requests: write -jobs: - tc-ai-pr-review: - runs-on: ubuntu-latest - steps: - - name: Checkout Repo - uses: actions/checkout@v3 - - - name: TC AI PR Reviewer - uses: topcoder-platform/tc-ai-pr-reviewer@master - with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # The GITHUB_TOKEN is there by default so you just need to keep it like it is and not necessarily need to add it as secret as it will throw an error. [More Details](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret) - LAB45_API_KEY: ${{ secrets.LAB45_API_KEY }} - exclude: '**/*.json, **/*.md, **/*.jpg, **/*.png, **/*.jpeg, **/*.bmp, **/*.webp' # Optional: exclude patterns separated by commas \ No newline at end of file