diff --git a/src/escrow/dto/fund-escrow.dto.ts b/src/escrow/dto/fund-escrow.dto.ts index 0bc4508c..c8885001 100644 --- a/src/escrow/dto/fund-escrow.dto.ts +++ b/src/escrow/dto/fund-escrow.dto.ts @@ -1,7 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { Type } from 'class-transformer'; import { - IsDateString, + IsDate, IsOptional, IsString, IsUUID, @@ -89,7 +89,10 @@ export class FundEscrowDto { 'ISO 8601 refund-deadline passed to escrow::fund (defaults to the configured window)', }) @IsOptional() - @IsDateString() + // @Type(() => Date) transforms before validation, so IsDateString (which + // requires a string) would reject every real deadline. IsDate accepts the + // transformed value and still rejects NaN "Invalid Date" results. + @IsDate() @Type(() => Date) deadline?: Date; } diff --git a/src/escrow/escrow.controller.spec.ts b/src/escrow/escrow.controller.spec.ts index 5d9e4a4b..c1fab578 100644 --- a/src/escrow/escrow.controller.spec.ts +++ b/src/escrow/escrow.controller.spec.ts @@ -115,7 +115,7 @@ describe('EscrowController', () => { }); it('release() returns the service escrow without metadata', async () => { - const result = await controller.release('escrow-1', 'GRECIPIENT', 'user-1'); + const result = await controller.release('escrow-1', { recipientAddress: 'GRECIPIENT', recipientId: 'user-1' }); expect(mockEscrowService.release).toHaveBeenCalledWith( 'escrow-1', @@ -142,7 +142,7 @@ describe('EscrowController', () => { { recipientAddress: 'GB', percentage: 50 }, ]; - const result = await controller.splitRelease('escrow-1', recipients); + const result = await controller.splitRelease('escrow-1', { recipients }); expect(mockEscrowService.splitRelease).toHaveBeenCalledWith( 'escrow-1', @@ -210,11 +210,11 @@ describe('EscrowController', () => { it('release() is restricted to MAINTAINER and refund() to MAINTAINER/SPONSOR', () => { expect(Reflect.getMetadata(ROLES_KEY, handlerFor('release'))).toEqual([ - 'MAINTAINER', + 'maintainer', ]); expect(Reflect.getMetadata(ROLES_KEY, handlerFor('refund'))).toEqual([ - 'MAINTAINER', - 'SPONSOR', + 'maintainer', + 'sponsor', ]); }); @@ -235,15 +235,15 @@ describe('EscrowController', () => { const base = { amount: '100.0000000', asset: AssetType.USDC, - funderAddress: 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567abcdefghijklmn', - bountyId: '00000000-0000-0000-0000-000000000001', + funderAddress: 'GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A', + bountyId: 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', }; it('accepts onChainIssueId, sponsorId and deadline', async () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: '2026-12-31T00:00:00.000Z', }); const errors = await validate(dto); @@ -289,7 +289,7 @@ describe('EscrowController', () => { const dto = plainToInstance(FundEscrowDto, { amount: 'not-money', asset: AssetType.USDC, - funderAddress: 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567abcdefghijklmn', + funderAddress: 'GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A', }); const errors = await validate(dto); expect(errors.length).toBeGreaterThan(0); @@ -299,7 +299,7 @@ describe('EscrowController', () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: '2026-12-31T00:00:00.000Z', }); mockEscrowService.fund.mockResolvedValueOnce({ id: 'escrow-1' }); @@ -307,7 +307,7 @@ describe('EscrowController', () => { expect(mockEscrowService.fund).toHaveBeenCalledWith( expect.objectContaining({ onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: new Date('2026-12-31T00:00:00.000Z'), }), ); diff --git a/src/github/github-webhooks.service.spec.ts b/src/github/github-webhooks.service.spec.ts index 45bb6fe8..9e38e3c0 100644 --- a/src/github/github-webhooks.service.spec.ts +++ b/src/github/github-webhooks.service.spec.ts @@ -10,7 +10,6 @@ import * as sigUtil from './webhook-signature.util'; describe('GithubWebhooksService', () => { let service: GithubWebhooksService; - let webhookEventRepo: { create: jest.Mock; save: jest.Mock }; let webhookEventRepo: { create: jest.Mock; save: jest.Mock; @@ -40,6 +39,7 @@ describe('GithubWebhooksService', () => { findOne: jest.fn().mockResolvedValue(null), }; bountyRepo = { findOne: jest.fn() }; + issueRepo = { findOne: jest.fn().mockResolvedValue(null) }; bountiesService = { markInReview: jest.fn().mockResolvedValue(undefined), markMergedAndRelease: jest.fn().mockResolvedValue(undefined), @@ -264,11 +264,11 @@ describe('GithubWebhooksService', () => { byNumber: Record, ) { syncService.findIssueByRepoAndNumber.mockImplementation( - ({ where }: { where: { number: number } }) => { - const entry = byNumber[where.number]; + (_repoId: string, number: number) => { + const entry = byNumber[number]; return Promise.resolve( entry - ? { id: `issue-${where.number}`, bounty: { id: entry.bountyId } } + ? { id: `issue-${number}`, bounty: { id: entry.bountyId } } : null, ); }, @@ -637,9 +637,8 @@ describe('GithubWebhooksService', () => { describe('comma-separated closing references (#309)', () => { /** Collects every issue number the merged-PR path ends up releasing. */ function releasedNumbers(): number[] { - return issueRepo.findOne.mock.calls.map( - (call: unknown[]) => - (call[0] as { where: { number: number } }).where.number, + return syncService.findIssueByRepoAndNumber.mock.calls.map( + (call: unknown[]) => call[1] as number, ); } @@ -732,11 +731,11 @@ describe('GithubWebhooksService', () => { }); it('marks a comma-separated bounty in review when the PR is opened', async () => { - issueRepo.findOne.mockImplementation( - ({ where }: { where: { number: number } }) => + syncService.findIssueByRepoAndNumber.mockImplementation( + (_repoId: string, number: number) => Promise.resolve({ - id: `issue-${where.number}`, - bounty: { id: `bounty-${where.number}` }, + id: `issue-${number}`, + bounty: { id: `bounty-${number}` }, }), ); bountyRepo.findOne.mockImplementation( @@ -782,10 +781,10 @@ describe('GithubWebhooksService', () => { describe('pull_request edited events (#310)', () => { it('moves a bounty to in_review when a closing keyword is added after opening', async () => { - issueRepo.findOne.mockImplementation( - ({ where }: { where: { number: number } }) => + syncService.findIssueByRepoAndNumber.mockImplementation( + (_repoId: string, number: number) => Promise.resolve({ - id: `issue-${where.number}`, + id: `issue-${number}`, bounty: { id: 'bounty-42' }, }), ); @@ -869,6 +868,9 @@ describe('GithubWebhooksService', () => { expect(bountiesService.markInReview).not.toHaveBeenCalled(); expect(bountiesService.markMergedAndRelease).not.toHaveBeenCalled(); + }); + }); + // #308: a redelivered X-GitHub-Delivery used to hit the unique constraint // on webhook_events.deliveryId and escape handleEvent as a 500, so every // redelivery of that event failed forever. diff --git a/src/milestones/milestones.service.spec.ts b/src/milestones/milestones.service.spec.ts index 0ac8b66f..a2749e40 100644 --- a/src/milestones/milestones.service.spec.ts +++ b/src/milestones/milestones.service.spec.ts @@ -11,6 +11,7 @@ describe('MilestonesService', () => { let service: MilestonesService; let milestoneRepo: { findOne: jest.Mock; + create: jest.Mock; save: jest.Mock; update: jest.Mock; }; @@ -24,11 +25,13 @@ describe('MilestonesService', () => { }; let dataSource: { transaction: jest.Mock; + query: jest.Mock; }; beforeEach(async () => { milestoneRepo = { findOne: jest.fn(), + create: jest.fn((m: Partial) => m), save: jest.fn((m: Partial) => Promise.resolve(m)), update: jest.fn(), }; @@ -50,6 +53,7 @@ describe('MilestonesService', () => { update: jest.fn(), }), ), + query: jest.fn().mockResolvedValue([{ exists: 1 }]), }; const module: TestingModule = await Test.createTestingModule({ @@ -75,7 +79,7 @@ describe('MilestonesService', () => { sponsorId: 'sponsor-1', }); - const milestone = await service.fund('m1', 'GFUNDER'); + const milestone = await service.fund('m1', 'GFUNDER', 'sponsor-1'); expect(milestone.status).toBe(MilestoneStatus.FUNDED); expect(milestone.escrowId).toBe('escrow-1'); @@ -90,7 +94,7 @@ describe('MilestonesService', () => { sponsorId: 'sponsor-1', }); - await service.fund('m1', 'GFUNDER'); + await service.fund('m1', 'GFUNDER', 'sponsor-1'); expect(escrowService.fund).toHaveBeenCalledWith( expect.objectContaining({ @@ -263,6 +267,7 @@ describe('MilestonesService', () => { '250.0000000', 'RECIPIENT_ADDR', 'user-1', + expect.anything(), ); expect(payment.id).toBe('payment-1'); expect(dataSource.transaction).toHaveBeenCalled(); @@ -341,7 +346,7 @@ describe('MilestonesService', () => { await expect( service.resolveIssue('m1', 'i1', 'RECIPIENT_ADDR'), - ).rejects.toThrow('Milestone m1 is not accepting distributions'); + ).rejects.toThrow('Cannot transition milestone'); }); it('rejects when the issue does not belong to the milestone (#114)', async () => { @@ -406,6 +411,7 @@ describe('MilestonesService', () => { '333.3333333', 'RECIPIENT_ADDR', undefined, + expect.anything(), ); }); }); @@ -417,4 +423,38 @@ describe('MilestonesService', () => { ); }); }); + + describe('create — #40 sponsor binding', () => { + const dto = { + repositoryId: 'repo-a', + sponsorId: 'spoofed-attacker-id', + title: 'Fix auth', + description: 'desc', + budget: '500', + asset: AssetType.USDC, + deadline: null, + } as any; + + it('ignores a client-supplied sponsorId and binds the authenticated caller', async () => { + const saved = await service.create(dto, 'caller-1'); + expect(saved.sponsorId).toBe('caller-1'); + expect(milestoneRepo.create).toHaveBeenCalledWith( + expect.objectContaining({ sponsorId: 'caller-1' }), + ); + }); + + it('binds the caller when sponsorId is omitted', async () => { + const { sponsorId: _drop, ...noSponsor } = dto; + const saved = await service.create(noSponsor, 'caller-9'); + expect(saved.sponsorId).toBe('caller-9'); + }); + + it('throws NotFoundException when the repository row does not exist', async () => { + dataSource.query.mockResolvedValueOnce([]); + await expect(service.create(dto, 'caller-1')).rejects.toThrow( + 'Repository repo-a not found', + ); + expect(milestoneRepo.save).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/milestones/milestones.service.ts b/src/milestones/milestones.service.ts index aecaf5a0..0c8b421c 100644 --- a/src/milestones/milestones.service.ts +++ b/src/milestones/milestones.service.ts @@ -36,12 +36,13 @@ export class MilestonesService { throw new NotFoundException(`Repository ${dto.repositoryId} not found`); } - // Fall back to the authenticated caller's id when the client omits sponsorId. - // This prevents milestones with sponsorId = null that become permanently - // invisible to every sponsor dashboard filter. + // The sponsor is always the authenticated caller — never a client- + // supplied sponsorId. Otherwise a caller could create a milestone owned + // by someone else, and the sponsor-only ops (fund/close) would gate on + // the wrong identity (#40). const milestone = this.milestoneRepo.create({ repositoryId: dto.repositoryId, - sponsorId: dto.sponsorId ?? callerUserId, + sponsorId: callerUserId, title: dto.title?.trim() ?? dto.title, description: dto.description ?? null, budget: dto.budget, diff --git a/src/sponsors/sponsors.controller.ts b/src/sponsors/sponsors.controller.ts index b7a48cca..c097bf1e 100644 --- a/src/sponsors/sponsors.controller.ts +++ b/src/sponsors/sponsors.controller.ts @@ -26,25 +26,6 @@ function validatePage(limit?: number, offset?: number) { throw new BadRequestException('offset must be >= 0'); } return { limit, offset }; -/** - * Parse optional limit/offset query params, rejecting NaN, negatives, - * non-integers, and limits above MAX_PAGE_LIMIT with a 400 (#280). - */ -function parsePagination(limit?: string, offset?: string) { - const parse = (name: string, raw: string | undefined, min: number, max: number) => { - if (raw === undefined || raw === '') return undefined; - const n = Number(raw); - if (!Number.isInteger(n) || n < min || n > max) { - throw new BadRequestException( - `${name} must be an integer between ${min} and ${max}`, - ); - } - return n; - }; - return { - limit: parse('limit', limit, 1, MAX_PAGE_LIMIT), - offset: parse('offset', offset, 0, Number.MAX_SAFE_INTEGER), - }; } @ApiTags('sponsors') @@ -73,7 +54,6 @@ export class SponsorsController { ) { this.assertOwnsSponsor(user, id); return this.sponsorsService.dashboard(id, validatePage(limit, offset)); - return this.sponsorsService.dashboard(id, parsePagination(limit, offset)); } @ApiBearerAuth() @@ -91,6 +71,5 @@ export class SponsorsController { id, validatePage(limit, offset), ); - return this.sponsorsService.milestoneProgress(id, parsePagination(limit, offset)); } } diff --git a/test/mocks/stellar-sdk.mock.js b/test/mocks/stellar-sdk.mock.js index a1a32f55..6fe33f51 100644 --- a/test/mocks/stellar-sdk.mock.js +++ b/test/mocks/stellar-sdk.mock.js @@ -40,7 +40,16 @@ class TransactionBuilder { } } + +class StrKey { + // Tests only need accept/reject — real CRC16 stays in the SDK's prod path. + static isValidEd25519PublicKey(value) { + return typeof value === 'string' && /^G[A-Z2-7]{55}$/.test(value); + } +} + module.exports = { + StrKey, Contract, Address, Keypair,