From 008016b947abd2874da1eef1a7ee8d684eae6db2 Mon Sep 17 00:00:00 2001 From: Femi <114834028+Toyosi5566@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:32:36 +0100 Subject: [PATCH 1/2] security: ignore local environment files .env is read by dotenv.config() at boot and holds ENCRYPTION_KEY, JWT_SECRET, SEP10_SERVER_SECRET, HMAC_KEY and DEPLOYER_SECRET_KEY. None of the .env variants were ignored, so a contributor's real credentials file could be staged by a routine `git add .` with nothing to warn anyone. Ignore the .env variants and keep the committed example file trackable. No .env file is tracked at HEAD and no .env object appears anywhere in the repository history, so this is preventive rather than a remediation. --- .gitignore | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.gitignore b/.gitignore index bea4619..ac0c2db 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,12 @@ coverage/ *.lcov coverage-final.json clover.xml + +# Local environment files. .env is loaded by dotenv at boot and holds live +# secrets (ENCRYPTION_KEY, JWT_SECRET, SEP10_SERVER_SECRET, HMAC_KEY, +# DEPLOYER_SECRET_KEY). The committed example file is the only environment +# file that belongs in git. +.env +.env.local +.env.*.local +!.env.example From 78cc42851105f05696359776be13719e07687c5a Mon Sep 17 00:00:00 2001 From: "Dev. SIKIRU KEHINDE RAHEEM" Date: Sat, 17 Oct 2026 18:23:08 +0100 Subject: [PATCH 2/2] fix: fix(db): guard against a duplicate contribution in a cycle (#247) --- .../migration.sql | 15 ++++++ prisma/schema.prisma | 5 ++ src/__tests__/group.service.test.ts | 41 ++++++++++++++--- .../message-processor.service.test.ts | 13 ++++++ src/services/group.service.ts | 46 +++++++++++++++---- src/services/message-processor.service.ts | 25 +++++++++- 6 files changed, 128 insertions(+), 17 deletions(-) create mode 100644 prisma/migrations/20260901000000_add_contribution_unique_constraint/migration.sql diff --git a/prisma/migrations/20260901000000_add_contribution_unique_constraint/migration.sql b/prisma/migrations/20260901000000_add_contribution_unique_constraint/migration.sql new file mode 100644 index 0000000..dcf287a --- /dev/null +++ b/prisma/migrations/20260901000000_add_contribution_unique_constraint/migration.sql @@ -0,0 +1,15 @@ +-- Guard against duplicate contributions for the same member and cycle. +-- The contract is not the ledge we query, so the database must refuse the duplicate. + +-- Remove any pre-existing duplicates so the unique index can be created. +-- Keep the most recently written row for each (userId, groupId, cycle) tuple. +DELETE FROM "contribution" a +USING "contribution" b +WHERE a.user_id = b.user_id +AND a.group_id = b.group_id +AND a.cycle = b.cycle +AND a.id < b.id; + +-- Enforce one contribution per member per cycle. +CREATE UNIQUE INDEX "contribution_user_id_group_id_cycle_key" +ON "contribution" ("user_id", "group_id", "cycle"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index ca46c2b..a0f7347 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -90,8 +90,13 @@ model Contribution { amount Decimal transactionHash String? status String @default("PENDING") // PENDING, COMPLETED, FAILED + // Identifies the cycle this contribution belongs to. Combined with userId + // and groupId this makes a repeated contribution for the same member and + // cycle impossible at the DB level. + cycleNumber Int @default(0) createdAt DateTime @default(now()) + @@unique([userId, groupId, cycleNumber]) @@index([userId]) @@index([groupId]) } diff --git a/src/__tests__/group.service.test.ts b/src/__tests__/group.service.test.ts index 9b40b76..0c3e52f 100644 --- a/src/__tests__/group.service.test.ts +++ b/src/__tests__/group.service.test.ts @@ -13,6 +13,7 @@ jest.mock('@prisma/client', () => { contribution: { create: jest.fn(), findMany: jest.fn(), + upsert: jest.fn(), } }; return { PrismaClient: jest.fn(() => mPrismaClient) }; @@ -181,18 +182,31 @@ describe('GroupService', () => { describe('addContribution', () => { it('should log a new contribution', async () => { const mockContribution = { id: 'c1', amount: 50 }; - prismaClientMock.contribution.create.mockResolvedValueOnce(mockContribution); + prismaClientMock.contribution.upsert.mockResolvedValueOnce(mockContribution); const result = await groupService.addContribution('u1', 'g1', '50', 'hash123'); - expect(prismaClientMock.contribution.create).toHaveBeenCalledWith({ - data: { + expect(prismaClientMock.contribution.upsert).toHaveBeenCalledWith({ + where: { + userId_groupId_cycleId: { + userId: 'u1', + groupId: 'g1', + cycleId: expect.any(String), + }, + }, + create: { userId: 'u1', groupId: 'g1', + cycleId: expect.any(String), amount: '50', transactionHash: 'hash123', status: 'COMPLETED' - } + }, + update: { + amount: '50', + transactionHash: 'hash123', + status: 'COMPLETED' + }, }); expect(result).toEqual(mockContribution); }); @@ -201,28 +215,41 @@ describe('GroupService', () => { await expect(groupService.addContribution('u1', 'g1', '0', 'hash123')).rejects.toThrow( 'Contribution amount must be a positive number greater than zero.', ); - expect(prismaClientMock.contribution.create).not.toHaveBeenCalled(); + expect(prismaClientMock.contribution.upsert).not.toHaveBeenCalled(); }); it('should reject a negative contribution amount', async () => { await expect(groupService.addContribution('u1', 'g1', '-10', 'hash123')).rejects.toThrow( 'Contribution amount must be a positive number greater than zero.', ); - expect(prismaClientMock.contribution.create).not.toHaveBeenCalled(); + expect(prismaClientMock.contribution.upsert).not.toHaveBeenCalled(); }); it('should reject a contribution amount with more than 7 decimal places', async () => { await expect(groupService.addContribution('u1', 'g1', '10.12345678', 'hash123')).rejects.toThrow( 'Amount cannot have more than 7 decimal places.', ); - expect(prismaClientMock.contribution.create).not.toHaveBeenCalled(); + expect(prismaClientMock.contribution.upsert).not.toHaveBeenCalled(); }); it('should reject a contribution amount exceeding 1,000,000 XLM', async () => { await expect(groupService.addContribution('u1', 'g1', '1000001', 'hash123')).rejects.toThrow( 'Amount exceeds the maximum allowed (1,000,000 XLM).', ); + expect(prismaClientMock.contribution.upsert).not.toHaveBeenCalled(); + }); + + it('should treat a retried contribution in the same cycle as an update, not a duplicate', async () => { + const mockContribution = { id: 'c1', amount: 50 }; + prismaClientMock.contribution.upsert.mockResolvedValueOnce(mockContribution); + + const first = await groupService.addContribution('u1', 'g1', '50', 'hash123'); + const second = await groupService.addContribution('u1', 'g1', '50', 'hash123'); + + expect(prismaClientMock.contribution.upsert).toHaveBeenCalledTimes(2); expect(prismaClientMock.contribution.create).not.toHaveBeenCalled(); + expect(first).toEqual(mockContribution); + expect(second).toEqual(mockContribution); }); }); diff --git a/src/__tests__/message-processor.service.test.ts b/src/__tests__/message-processor.service.test.ts index 0085402..5178960 100644 --- a/src/__tests__/message-processor.service.test.ts +++ b/src/__tests__/message-processor.service.test.ts @@ -336,6 +336,19 @@ describe('MessageProcessor', () => { expect(mockLockOrderForCycle).toHaveBeenCalledWith('g1'); }); + it('treats a duplicate contribution for the same member and cycle as idempotent', async () => { + // Simulate the write path being reached twice for the same member/cycle. + // The second write must not surface an error to the user and must not + // append a second row; the service is expected to upsert/no-op. + mockAddContribution + .mockResolvedValueOnce({ id: 'c1' }) + .mockResolvedValueOnce({ id: 'c1' }); + await processor.processCommand('12345', 'CONTRIBUTE 10'); + await processor.processCommand('12345', 'CONTRIBUTE 10'); + expect(mockSendMessage).toHaveBeenCalledWith('12345', expect.stringContaining('contribute.success')); + expect(mockSendMessage).not.toHaveBeenCalledWith('12345', expect.stringContaining('contribute.failed')); + }); + it('shows usage when no args provided', async () => { await processor.processCommand('12345', 'CONTRIBUTE'); expect(mockSendMessage).toHaveBeenCalledWith('12345', expect.stringContaining('contribute.usage')); diff --git a/src/services/group.service.ts b/src/services/group.service.ts index b0b85ad..fb804bd 100644 --- a/src/services/group.service.ts +++ b/src/services/group.service.ts @@ -1,6 +1,7 @@ import { PrismaClient, Prisma } from '@prisma/client'; import { scheduleGroupCycle, removeGroupCycle } from '../queue/contribution-scheduler.queue'; import { enqueueContractDeployment } from '../queue/soroban-deployment.queue'; +import { getCurrentCycle } from '../utils/cycle.util'; const prisma = new PrismaClient(); @@ -114,15 +115,44 @@ export class GroupService { public async addContribution(userId: string, groupId: string, amount: string | Prisma.Decimal, txHash: string, status: 'COMPLETED' | 'PENDING' | 'FAILED' = 'COMPLETED') { GroupService.validateAmount(amount); - return await prisma.contribution.create({ - data: { - userId, - groupId, - amount, - transactionHash: txHash, - status, + const cycle = getCurrentCycle(); + + try { + return await prisma.contribution.create({ + data: { + userId, + groupId, + amount, + transactionHash: txHash, + status, + cycle, + } + }); + } catch (error) { + // A duplicate contribution for the same member and cycle is + // rejected by the unique constraint (userId, groupId, cycle). + // Treat it as an idempotent update rather than surfacing an error. + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === 'P2002' + ) { + return await prisma.contribution.update({ + where: { + userId_groupId_cycle: { + userId, + groupId, + cycle, + }, + }, + data: { + amount, + transactionHash: txHash, + status, + }, + }); } - }); + throw error; + } } public async getContributionsByUser(userId: string) { diff --git a/src/services/message-processor.service.ts b/src/services/message-processor.service.ts index 801931f..c80d3d9 100644 --- a/src/services/message-processor.service.ts +++ b/src/services/message-processor.service.ts @@ -872,7 +872,17 @@ export class MessageProcessor { // Transaction submitted but not confirmed within 30 s. // Record as PENDING so the reconciliation worker can detect // whether it eventually landed on-chain. - await this.groupService.addContribution(user.id, group.id, amountStr, txHash, 'PENDING'); + // Idempotent: a retried webhook or operator re-run must not + // append a second row for the same member and cycle. The + // service upserts on the (userId, groupId, cycle) unique key. + await this.groupService.addContribution( + user.id, + group.id, + amountStr, + txHash, + 'PENDING', + (group as any).totalCycles, + ); await this.whatsappService.sendMessage( from, t('contribute.pending', lang, { hash: txHash }), @@ -882,7 +892,18 @@ export class MessageProcessor { } // status === 'SUCCESS': confirmed on-chain — now safe to write DB record. - await this.groupService.addContribution(user.id, group.id, amountStr, txHash); + // Idempotent against the (userId, groupId, cycle) unique constraint: + // a duplicate confirmation updates the existing row instead of + // creating a second one, so the reconciliation total matches the + // number of contributions actually recorded. + await this.groupService.addContribution( + user.id, + group.id, + amountStr, + txHash, + 'CONFIRMED', + (group as any).totalCycles, + ); // The first contribution of a cycle locks the payout order — the creator // can no longer reorder once the rotation is underway. await this.payoutService.lockOrderForCycle(group.id).catch((e) => console.error('Failed to lock payout order', e));