diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06af420..728fce5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,10 +82,15 @@ jobs: NODE_ENV: test run: npm run test:cov + # The e2e specs run entirely against the local Postgres service and a + # mocked Stellar SDK; none of them use GITHUB_CLIENT_ID/SECRET. The step + # used to be gated on those (never-configured) secrets, so `npm run + # test:e2e` had never actually executed in CI (#164). Run it always. - name: Run E2E Tests env: DATABASE_URL: postgresql://postgres:postgres@localhost:5432/mergefi NODE_ENV: test + run: npm run test:e2e run: | # The e2e specs build their TestingModule from mocked providers and # overrideGuard(...)-stubbed auth — none drive a real passport-github2 diff --git a/src/common/entities/maintenance-pool.entity.ts b/src/common/entities/maintenance-pool.entity.ts index a956bcf..08c96c0 100644 --- a/src/common/entities/maintenance-pool.entity.ts +++ b/src/common/entities/maintenance-pool.entity.ts @@ -18,7 +18,7 @@ export class MaintenancePool { @PrimaryGeneratedColumn('uuid') id: string; - @Column() + @Column({ type: 'varchar', length: 100 }) name: string; @ManyToOne(() => Repository, { onDelete: 'CASCADE', nullable: true }) diff --git a/src/common/entities/milestone.entity.ts b/src/common/entities/milestone.entity.ts index c06ccb1..be14359 100644 --- a/src/common/entities/milestone.entity.ts +++ b/src/common/entities/milestone.entity.ts @@ -27,7 +27,7 @@ export class Milestone { @Column() repositoryId: string; - @Column() + @Column({ type: 'varchar', length: 200 }) title: string; @Column({ type: 'text', nullable: true }) diff --git a/src/common/entities/team-member-split.entity.ts b/src/common/entities/team-member-split.entity.ts index 65482ee..9aa25e4 100644 --- a/src/common/entities/team-member-split.entity.ts +++ b/src/common/entities/team-member-split.entity.ts @@ -29,7 +29,7 @@ export class TeamMemberSplit { userId: string; /** Free-text label describing the member's contribution, e.g. "frontend". */ - @Column({ type: 'varchar', nullable: true }) + @Column({ type: 'varchar', length: 50, nullable: true }) role: string | null; /** Percentage of the bounty payout, 0-100. Sum across a team must equal 100. */ diff --git a/src/database/migrations/1784800000000-BoundFreeTextColumnLengths.ts b/src/database/migrations/1784800000000-BoundFreeTextColumnLengths.ts new file mode 100644 index 0000000..370f657 --- /dev/null +++ b/src/database/migrations/1784800000000-BoundFreeTextColumnLengths.ts @@ -0,0 +1,37 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Caps previously-unbounded free-text varchar columns so an arbitrarily large + * string can no longer be stored in a team member's `role`, a milestone + * `title`, or a maintenance pool `name` (#151). Matches the `@MaxLength(...)` + * constraints added to the corresponding DTOs. + */ +export class BoundFreeTextColumnLengths1784800000000 + implements MigrationInterface +{ + name = 'BoundFreeTextColumnLengths1784800000000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "team_member_splits" ALTER COLUMN "role" TYPE character varying(50)`, + ); + await queryRunner.query( + `ALTER TABLE "milestones" ALTER COLUMN "title" TYPE character varying(200)`, + ); + await queryRunner.query( + `ALTER TABLE "maintenance_pools" ALTER COLUMN "name" TYPE character varying(100)`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "maintenance_pools" ALTER COLUMN "name" TYPE character varying`, + ); + await queryRunner.query( + `ALTER TABLE "milestones" ALTER COLUMN "title" TYPE character varying`, + ); + await queryRunner.query( + `ALTER TABLE "team_member_splits" ALTER COLUMN "role" TYPE character varying`, + ); + } +} diff --git a/src/escrow/soroban-client.service.ts b/src/escrow/soroban-client.service.ts index 3c3a9d4..83224cc 100644 --- a/src/escrow/soroban-client.service.ts +++ b/src/escrow/soroban-client.service.ts @@ -43,6 +43,19 @@ export interface ContractInvocationResult { * live balance — see `EscrowService.poolWithdraw` (#163): * fn deposit(env: Env, sponsor: Address, pool_id: BytesN<32>, amount: i128, token: Address) * fn withdraw(env: Env, pool_id: BytesN<32>, recipient: Address, amount: i128) -> i128 + * + * The `mergefi-milestones` contract is a two-step allocate/release model with + * no "partially drain one locked escrow" primitive (#160, #162): + * `create_milestone()` opens a budget pool, `allocate(milestone_id, issue_id, + * amount)` reserves a slice of the unallocated remainder for one issue + * (admin-only, rejects over-allocation), and `release_issue(milestone_id, + * issue_id, recipients)` pays out that issue's already-reserved slice. + * `MilestonesService.resolveIssue` therefore needs per-issue allocation + * tracking rather than repeated `releasePartial` calls against a single + * ever-LOCKED escrow row: + * fn create_milestone(env: Env, sponsor: Address, milestone_id: BytesN<32>, budget: i128, token: Address) + * fn allocate(env: Env, milestone_id: BytesN<32>, issue_id: BytesN<32>, amount: i128) + * fn release_issue(env: Env, milestone_id: BytesN<32>, issue_id: BytesN<32>, recipients: Vec<(Address, u32)>) -> i128 */ @Injectable() export class SorobanClientService { diff --git a/src/maintenance-pool/dto/create-pool.dto.ts b/src/maintenance-pool/dto/create-pool.dto.ts index fbe16b1..5f428f5 100644 --- a/src/maintenance-pool/dto/create-pool.dto.ts +++ b/src/maintenance-pool/dto/create-pool.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsOptional, IsString, IsUUID } from 'class-validator'; +import { IsOptional, IsString, IsUUID, MaxLength } from 'class-validator'; import { AssetType } from '../../common/enums'; import { IsMoneyAmount, @@ -7,8 +7,9 @@ import { } from '../../common/validators/money.validator'; export class CreatePoolDto { - @ApiProperty() + @ApiProperty({ maxLength: 100 }) @IsString() + @MaxLength(100) name: string; @ApiProperty({ required: false }) diff --git a/src/milestones/dto/create-milestone.dto.ts b/src/milestones/dto/create-milestone.dto.ts index d98ef88..f80c25e 100644 --- a/src/milestones/dto/create-milestone.dto.ts +++ b/src/milestones/dto/create-milestone.dto.ts @@ -1,5 +1,11 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsISO8601, IsOptional, IsString, IsUUID } from 'class-validator'; +import { + IsISO8601, + IsOptional, + IsString, + IsUUID, + MaxLength, +} from 'class-validator'; import { AssetType } from '../../common/enums'; import { IsMoneyAmount, @@ -16,13 +22,15 @@ export class CreateMilestoneDto { @IsUUID() sponsorId?: string; - @ApiProperty() + @ApiProperty({ maxLength: 200 }) @IsString() + @MaxLength(200) title: string; - @ApiProperty({ required: false }) + @ApiProperty({ required: false, maxLength: 2000 }) @IsOptional() @IsString() + @MaxLength(2000) description?: string; @ApiProperty() diff --git a/src/milestones/milestones.service.ts b/src/milestones/milestones.service.ts index 3a6222c..2b3e759 100644 --- a/src/milestones/milestones.service.ts +++ b/src/milestones/milestones.service.ts @@ -132,6 +132,18 @@ export class MilestonesService { ); } + // Pay out each issue at most once. The real mergefi-milestones contract + // tracks a per-issue allocation and `release_issue` can only be called + // once per issue_id; here the resolved issue is moved to CLOSED in the + // transaction below, so resolving an already-CLOSED issue (while other + // issues are still open) must be rejected rather than double-paying it + // (#162). + if (issue.state !== 'open') { + throw new BadRequestException( + `Issue ${issueId} has already been resolved for milestone ${milestoneId}`, + ); + } + const unresolvedCount = openIssues.length; const remainingBudget = Number(milestone.budget) - Number(milestone.distributed); diff --git a/src/teams/dto/create-team.dto.ts b/src/teams/dto/create-team.dto.ts index 8dfa7bc..f24ee24 100644 --- a/src/teams/dto/create-team.dto.ts +++ b/src/teams/dto/create-team.dto.ts @@ -7,6 +7,7 @@ import { IsString, IsUUID, Max, + MaxLength, Min, ValidateNested, } from 'class-validator'; @@ -16,9 +17,10 @@ export class TeamMemberSplitDto { @IsUUID() userId: string; - @ApiProperty({ required: false, example: 'frontend' }) + @ApiProperty({ required: false, example: 'frontend', maxLength: 50 }) @IsOptional() @IsString() + @MaxLength(50) role?: string; @ApiProperty({ example: 40, minimum: 0.01, maximum: 100 })