From 4c14acaa4df8765bcde75611b33412d7d162a1a0 Mon Sep 17 00:00:00 2001 From: EDRipper Date: Thu, 2 Jul 2026 14:08:14 -0700 Subject: [PATCH] fix(devlog-review): gate devlog-hour payouts behind fraud review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devlog review (#39) mutated project.override_hours directly, which is the field pipe payouts are computed from — letting a Reviewer mint hours→pipes outside the fraud-review gate, and dual-writing a field the project-review flow also owns (risking silent overwrite). It also read the devlog's prior state outside its write transaction (TOCTOU: concurrent reviews double-count). Now devlog hours are treated like any other project hours but mint pipes ONLY at fraud review, by a Fraud Reviewer or Super Admin: - reviewDevlog no longer touches project.override_hours. It records approved/approvedHours on the devlog inside a transaction that locks the devlog row FOR UPDATE, closing the TOCTOU. - Both pipe reconciles (FraudReviewService.completeApproval and AuditService one-shot approval) now fold approved devlog hours into the earned-hours base via a separate aggregate (no JOIN fan-out into the override_hours SUM), so devlog hours pay out exactly like normal hours. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/src/admin/audit.service.ts | 17 +++++- backend/src/devlogs/devlogs.service.ts | 60 ++++++++++--------- .../src/fraud-review/fraud-review.service.ts | 30 +++++++++- 3 files changed, 75 insertions(+), 32 deletions(-) diff --git a/backend/src/admin/audit.service.ts b/backend/src/admin/audit.service.ts index e78df1f..2fd7bb5 100644 --- a/backend/src/admin/audit.service.ts +++ b/backend/src/admin/audit.service.ts @@ -479,7 +479,10 @@ export class AuditService { await this.submissionRepo.save(submission); } - // Grant pipes — delta logic identical to the previous fraud poller path. + // Grant pipes — delta logic identical to the fraud poller path. Earned + // hours = project override_hours PLUS approved devlog hours (devlog hours + // count like normal hours and, like all hours, only mint pipes here at + // fraud review). if ((project.overrideHours ?? 0) > 0) { const totals = await this.projectRepo .createQueryBuilder('p') @@ -490,7 +493,17 @@ export class AuditService { `(p.status = 'approved' OR (p.status <> 'approved' AND p.pipes_granted > 0))`, ) .getRawOne<{ earnedHours: string; granted: string }>(); - const target = Math.floor(Number(totals?.earnedHours ?? 0)); + const devlogRows: Array<{ h: string }> = await this.projectRepo.manager.query( + `SELECT COALESCE(SUM(d.approved_hours), 0) AS h + FROM devlogs d + JOIN projects p ON p.id = d.project_id + WHERE p.user_id = $1 + AND d.approved = true + AND (p.status = 'approved' OR (p.status <> 'approved' AND p.pipes_granted > 0))`, + [project.userId], + ); + const devlogHours = Number(devlogRows?.[0]?.h ?? 0); + const target = Math.floor(Number(totals?.earnedHours ?? 0) + devlogHours); const previouslyGranted = Number(totals?.granted ?? 0); const delta = target - previouslyGranted; if (delta > 0) { diff --git a/backend/src/devlogs/devlogs.service.ts b/backend/src/devlogs/devlogs.service.ts index add3f38..90e5142 100644 --- a/backend/src/devlogs/devlogs.service.ts +++ b/backend/src/devlogs/devlogs.service.ts @@ -142,43 +142,49 @@ export class DevlogsService { return { ok: true }; } + /** + * A reviewer approves (or un-approves) a devlog and sets its hours. Devlog + * hours are treated exactly like normal project hours, but this only RECORDS + * them on the devlog — it deliberately does NOT touch `project.overrideHours` + * or mint pipes. Pipes are minted solely at fraud review by a Fraud Reviewer + * or Super Admin, where the reconcile folds approved devlog hours into the + * payout base (see FraudReviewService.completeApproval / AuditService). This + * keeps the pipe-minting gate in one fraud-gated place and avoids a reviewer + * side-channel into the balance. + * + * The devlog row is locked FOR UPDATE so two concurrent reviews of the same + * devlog can't race on its approved state. + */ async reviewDevlog( devlogId: string, reviewerId: string, - dto: { approved: boolean; approvedHours: number | null }) { - const devlog = await this.devlogRepo.findOne({ where: { id: devlogId } }); - if (!devlog) throw new BadRequestException('Devlog not found'); + dto: { approved: boolean; approvedHours: number | null }, + ) { + return this.dataSource.transaction(async (manager) => { + const devlog = await manager.findOne(Devlog, { + where: { id: devlogId }, + lock: { mode: 'pessimistic_write' }, + }); + if (!devlog) throw new BadRequestException('Devlog not found'); if (!devlog.projectId) throw new BadRequestException('Devlog is not linked to a project'); - const hours = + const hours = dto.approved && typeof dto.approvedHours === 'number' && dto.approvedHours >= 0 ? Math.round(Math.min(dto.approvedHours, 1000) * 10) / 10 : 0; - const previous = devlog.approved ? devlog.approvedHours ?? 0 : 0; - const delta = Math.round((hours - previous) * 10) / 10; - - return this.dataSource.transaction(async (manager) => { - if (delta != 0) { - const project = await manager.findOne(Project, { where: { id: devlog.projectId! }, lock: { mode: 'pessimistic_write' } }); - if (project) { - const next = Math.round(((project.overrideHours ?? 0) + delta) * 10) / 10; - project.overrideHours = Math.max(0, next); - await manager.save(Project, project); - } - } - devlog.approved = dto.approved; - devlog.approvedHours = dto.approved ? hours : null; - devlog.approvedByReviewerId = reviewerId; - devlog.approvedAt = new Date(); - await manager.save(Devlog, devlog); - - await this.auditLogService.log( - reviewerId, - 'devlog_reviewed', - `${dto.approved ? 'Approved' : 'Unapproved'} devlog "${devlog.title}" (${hours}h, Δ${delta}h) on project ${devlog.projectId}`, + devlog.approved = dto.approved; + devlog.approvedHours = dto.approved ? hours : null; + devlog.approvedByReviewerId = reviewerId; + devlog.approvedAt = new Date(); + await manager.save(Devlog, devlog); + + await this.auditLogService.log( + reviewerId, + 'devlog_reviewed', + `${dto.approved ? 'Approved' : 'Unapproved'} devlog "${devlog.title}" (${dto.approved ? hours : 0}h) on project ${devlog.projectId} — pipes mint at fraud review`, ); - return { id: devlog.id, approved: devlog.approved, approvedHours: devlog.approvedHours}; + return { id: devlog.id, approved: devlog.approved, approvedHours: devlog.approvedHours }; }); } diff --git a/backend/src/fraud-review/fraud-review.service.ts b/backend/src/fraud-review/fraud-review.service.ts index feb5e24..34d85df 100644 --- a/backend/src/fraud-review/fraud-review.service.ts +++ b/backend/src/fraud-review/fraud-review.service.ts @@ -452,6 +452,26 @@ export class FraudReviewService implements OnApplicationBootstrap, OnApplication ); } + /** + * Sum of approved devlog hours across the user's "earned" projects (approved, + * or already partially paid out) — the same project set the pipe reconcile + * counts override_hours over. Kept as a separate aggregate (not a JOIN into + * the override_hours SUM) so devlog rows can't fan out and inflate the + * project-hours total. + */ + private async approvedDevlogHoursForUser(userId: string): Promise { + const rows: Array<{ h: string }> = await this.projectRepo.manager.query( + `SELECT COALESCE(SUM(d.approved_hours), 0) AS h + FROM devlogs d + JOIN projects p ON p.id = d.project_id + WHERE p.user_id = $1 + AND d.approved = true + AND (p.status = 'approved' OR (p.status <> 'approved' AND p.pipes_granted > 0))`, + [userId], + ); + return Number(rows?.[0]?.h ?? 0); + } + private async completeApproval(row: FraudReview): Promise { const project = await this.projectRepo.findOne({ where: { id: row.projectId }, @@ -476,8 +496,11 @@ export class FraudReviewService implements OnApplicationBootstrap, OnApplication } // 3. Grant pipes — same delta logic as AdminService.reviewProject. Target = - // floor(sum of override_hours across the user's earned projects), delta = - // target − sum(pipes_granted) across all the user's projects. + // floor(earned hours across the user's earned projects), delta = + // target − sum(pipes_granted) across all the user's projects. Earned + // hours = project override_hours PLUS approved devlog hours: devlog hours + // count exactly like normal hours, and (like all hours) they only mint + // pipes here, at fraud review. if ((project.overrideHours ?? 0) > 0) { const totals = await this.projectRepo .createQueryBuilder('p') @@ -488,7 +511,8 @@ export class FraudReviewService implements OnApplicationBootstrap, OnApplication `(p.status = 'approved' OR (p.status <> 'approved' AND p.pipes_granted > 0))`, ) .getRawOne<{ earnedHours: string; granted: string }>(); - const target = Math.floor(Number(totals?.earnedHours ?? 0)); + const devlogHours = await this.approvedDevlogHoursForUser(project.userId); + const target = Math.floor(Number(totals?.earnedHours ?? 0) + devlogHours); const previouslyGranted = Number(totals?.granted ?? 0); const delta = target - previouslyGranted; if (delta > 0) {