Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions backend/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
import { getRedisConnectionOptions } from "./config/redis.config";

import { AnalyticsModule } from "./analytics/analytics.module";
import { CommonModule } from "./common/common.module";

Check warning on line 16 in backend/src/app.module.ts

View workflow job for this annotation

GitHub Actions / Backend — Lint, Test, Build

'CommonModule' is defined but never used
import { ComplianceModule } from "./compliance/compliance.module";
import { CollaborationModule } from "./collaboration/collaboration.module";
import { DashboardModule } from "./dashboard/dashboard.module";
import { DebtSimplificationModule } from "./debt-simplification/debt-simplification.module";

Check warning on line 20 in backend/src/app.module.ts

View workflow job for this annotation

GitHub Actions / Backend — Lint, Test, Build

'DebtSimplificationModule' is defined but never used
import { DisputesModule } from "./disputes/disputes.module";
import { EmailModule } from "./email/email.module";
import { ExportModule } from "./export/export.module";
Expand Down Expand Up @@ -45,9 +45,7 @@
import { SplitTemplateModule } from "./split-template/split-template.module";
import { StellarModule } from "./stellar/stellar.module";
import { TemplatesModule } from "./templates/templates.module";
import { UploadModule } from "./uploads/upload.module";
import { ShortLinksModule } from "./short-links/short-links.module";
import { WebhooksModule } from "./webhooks/webhooks.module";
import { FraudDetectionModule } from "./fraud-detection/fraud-detection.module";
// Duplicate imports removed; already imported above.
// Load environment variables
dotenv.config({
Expand Down Expand Up @@ -133,6 +131,7 @@
CollaborationModule,
DashboardModule,
ShortLinksModule,
FraudDetectionModule,
// Duplicated modules were already included earlier.
],
})
Expand Down
42 changes: 27 additions & 15 deletions backend/src/compliance/compliance.controller.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
import { Controller, Post, Get, Put, Body, Param, Query, UseGuards } from '@nestjs/common';
import { Controller, Post, Get, Put, Body, Param, Query, UseGuards, Req } from '@nestjs/common';
import { ComplianceService } from './compliance.service';
import { ExpenseCategory } from './entities/expense-category.entity';
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
import { AuthorizationGuard } from '../auth/guards/authorization.guard';
import { RequirePermissions } from '../auth/decorators/permissions.decorator';
import { Permissions } from '../auth/decorators/permissions.decorator';

interface AuthRequest {
user: { walletAddress: string };
}

@Controller('api/compliance')
@UseGuards(JwtAuthGuard, AuthorizationGuard)
export class ComplianceController {
constructor(private readonly complianceService: ComplianceService) { }

@Post('export/request')
async requestExport(@Body() data: any) {
// In a real app, userId would come from auth guard
const userId = data.userId;
return this.complianceService.requestExport(userId, data);
@RequirePermissions(Permissions.CAN_CREATE_EXPORT)
async requestExport(@Body() data: any, @Req() req: AuthRequest) {
return this.complianceService.requestExport(req.user.walletAddress, data);
}

@Get('export/:requestId/status')
Expand All @@ -19,28 +27,32 @@ export class ComplianceController {
}

@Get('categories')
async getCategories(@Query('userId') userId: string) {
return this.complianceService.getCategories(userId);
@RequirePermissions(Permissions.CAN_READ_EXPORT)
async getCategories(@Req() req: AuthRequest) {
return this.complianceService.getCategories(req.user.walletAddress);
}

@Post('categories')
async createCategory(@Body() data: any) {
const userId = data.userId;
return this.complianceService.createCategory(userId, data);
@RequirePermissions(Permissions.CAN_CREATE_EXPORT)
async createCategory(@Body() data: any, @Req() req: AuthRequest) {
return this.complianceService.createCategory(req.user.walletAddress, data);
}

@Put('splits/:splitId/category')
@RequirePermissions(Permissions.CAN_UPDATE_SPLIT)
async assignCategory(@Param('splitId') splitId: string, @Body('categoryId') categoryId: string) {
return this.complianceService.assignCategoryToSplit(splitId, categoryId);
}
Comment on lines 41 to 45
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

assignCategory doesn't verify user owns the split.

The endpoint requires CAN_UPDATE_SPLIT permission but doesn't verify the authenticated user actually owns/has access to the split being modified. This could allow a user to modify category assignments on other users' splits.

🔧 Proposed fix
     `@Put`('splits/:splitId/category')
     `@RequirePermissions`(Permissions.CAN_UPDATE_SPLIT)
-    async assignCategory(`@Param`('splitId') splitId: string, `@Body`('categoryId') categoryId: string) {
-        return this.complianceService.assignCategoryToSplit(splitId, categoryId);
+    async assignCategory(
+        `@Param`('splitId') splitId: string,
+        `@Body`('categoryId') categoryId: string,
+        `@Req`() req: AuthRequest,
+    ) {
+        return this.complianceService.assignCategoryToSplit(splitId, categoryId, req.user.walletAddress);
     }

The service method should then verify the split belongs to the user.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.controller.ts` around lines 41 - 45, The
assignCategory controller currently calls
complianceService.assignCategoryToSplit without verifying the authenticated user
owns the split; update the flow so ownership is enforced: fetch the current
user's ID in the controller (via the existing auth user decorator or
request.user) and either (preferred) change
complianceService.assignCategoryToSplit(signature) to accept a userId and
perform a lookup check that the split belongs to that user before updating, or
(alternatively) perform the ownership check in the controller using the splitId
and userId before calling assignCategoryToSplit; reference assignCategory,
RequirePermissions, and complianceService.assignCategoryToSplit when making the
change.


@Get('summary')
async getSummary(@Query('userId') userId: string, @Query('year') year: string) {
return this.complianceService.getSummary(userId, parseInt(year));
@RequirePermissions(Permissions.CAN_READ_EXPORT)
async getSummary(@Query('year') year: string, @Req() req: AuthRequest) {
return this.complianceService.getSummary(req.user.walletAddress, parseInt(year));
}

@Get('tax-deductible-total')
async getTaxDeductibleTotal(@Query('userId') userId: string, @Query('period') period: string) {
return this.complianceService.getTaxDeductibleTotal(userId, period);
@Get('export/:requestId/download')
@RequirePermissions(Permissions.CAN_READ_EXPORT)
async downloadExport(@Param('requestId') requestId: string, @Req() req: AuthRequest) {
return this.complianceService.downloadExport(requestId, req.user.walletAddress);
}
Comment on lines +53 to 57
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

File download response not properly handled for HTTP streaming.

The downloadExport endpoint returns the service result directly (an object with fileName, content, mimeType). NestJS will serialize this as JSON, including the binary content buffer. For proper file downloads, use @Res() with response.download() or return a StreamableFile.

🔧 Proposed fix using StreamableFile
-import { Controller, Post, Get, Put, Body, Param, Query, UseGuards, Req } from '@nestjs/common';
+import { Controller, Post, Get, Put, Body, Param, Query, UseGuards, Req, StreamableFile, Res, Header } from '@nestjs/common';
+import { Response } from 'express';

 // ...

     `@Get`('export/:requestId/download')
     `@RequirePermissions`(Permissions.CAN_READ_EXPORT)
-    async downloadExport(`@Param`('requestId') requestId: string, `@Req`() req: AuthRequest) {
-        return this.complianceService.downloadExport(requestId, req.user.walletAddress);
+    async downloadExport(
+        `@Param`('requestId') requestId: string,
+        `@Req`() req: AuthRequest,
+        `@Res`({ passthrough: true }) res: Response,
+    ): Promise<StreamableFile> {
+        const { fileName, content, mimeType } = await this.complianceService.downloadExport(requestId, req.user.walletAddress);
+        res.set({
+            'Content-Type': mimeType,
+            'Content-Disposition': `attachment; filename="${fileName}"`,
+        });
+        return new StreamableFile(content);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.controller.ts` around lines 53 - 57, The
downloadExport controller currently returns the service result directly from
downloadExport which lets Nest serialize the binary content as JSON; change the
controller method downloadExport to use Nest's response streaming instead: call
complianceService.downloadExport(requestId, req.user.walletAddress) to get
{fileName, content, mimeType}, then convert content to a stream or Buffer and
send it using either response.setHeader('Content-Type', mimeType) and
response.setHeader('Content-Disposition', `attachment; filename="${fileName}"`)
followed by response.send(buffer/stream) (using `@Res`() response injection), or
wrap the buffer/stream in a StreamableFile and return that from the method so
the file is streamed correctly to the client. Ensure to import StreamableFile if
used and remove direct JSON return of the service object.

}
41 changes: 28 additions & 13 deletions backend/src/compliance/compliance.processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { PDFExporterService } from "./exporters/pdf-exporter.service";
import { QBOExporterService } from "./exporters/qbo-exporter.service";
import { JSONExporterService } from "./exporters/json-exporter.service";
import { OFXExporterService } from "./exporters/ofx-exporter.service";
import { EmailService } from "../email/email.service";
import { ProfileService } from "../profile/profile.service";
import { Logger } from "@nestjs/common";
import * as fs from "fs";
import * as path from "path";
Expand All @@ -34,6 +34,7 @@ export class ComplianceProcessor {
private jsonExporter: JSONExporterService,
private ofxExporter: OFXExporterService,
private emailService: EmailService,
private profileService: ProfileService,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing ProfileModule import causes DI failure.

ProfileService is injected into ComplianceProcessor, but ComplianceModule (see backend/src/compliance/compliance.module.ts lines 18-24) does not import ProfileModule. NestJS will throw a dependency injection error at runtime: "Nest can't resolve dependencies of the ComplianceProcessor."

🐛 Proposed fix: Import ProfileModule in ComplianceModule

In backend/src/compliance/compliance.module.ts:

 import { EmailModule } from '../email/email.module';
+import { ProfileModule } from '../profile/profile.module';

 `@Module`({
     imports: [
         TypeOrmModule.forFeature([ExpenseCategory, TaxExportRequest, Split]),
         BullModule.registerQueue({
             name: 'compliance-export',
         }),
         EmailModule,
+        ProfileModule,
     ],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.processor.ts` at line 37,
ComplianceProcessor is failing DI because ProfileService is injected but
ComplianceModule doesn't import ProfileModule; update the ComplianceModule to
add ProfileModule to its imports array so Nest can provide ProfileService to
ComplianceProcessor, ensuring ProfileModule exports ProfileService (or the
provider) if not already exported; reference the ComplianceProcessor class, the
ProfileService injection, and the ComplianceModule imports array when making the
change.

) {
if (!fs.existsSync(this.exportDir)) {
fs.mkdirSync(this.exportDir);
Expand Down Expand Up @@ -99,25 +100,39 @@ export class ComplianceProcessor {

await this.exportRepo.update(requestId, {
status: ExportStatus.READY,
fileUrl: filePath, // Using local path for simplicity in this implementation
fileUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`, // Secure download URL
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hardcoded localhost:3000 URL is not production-ready.

The fileUrl uses a hardcoded http://localhost:3000 base URL. This will break in staging/production environments. Use an environment variable or configuration service to provide the base URL.

🔧 Proposed fix
+// In constructor, inject ConfigService or use process.env
+private readonly baseUrl = process.env.APP_BASE_URL || 'http://localhost:3000';

 // Then use it:
-        fileUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
+        fileUrl: `${this.baseUrl}/api/compliance/export/${requestId}/download`,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.processor.ts` at line 103, Replace the
hardcoded base URL used in building fileUrl with a configurable value: read the
application base URL from a configuration source (e.g., an environment variable
like process.env.BASE_URL or your existing config service) and use it when
constructing fileUrl (`fileUrl:
\`${baseUrl}/api/compliance/export/${requestId}/download\``) so that the code in
compliance.processor.ts uses the runtime environment's URL instead of
"http://localhost:3000"; ensure baseUrl is validated/fallback-safe before use.

fileSize: fs.statSync(filePath).size,
recordCount: splits.length,
completedAt: new Date(),
expiresAt,
});

// Send email notification
// In a real app, we'd look up the user's email by wallet address
// For now, we'll assume a dummy email or use a placeholder
await this.emailService["emailQueue"].add("sendEmail", {
to: "user@example.com", // Placeholder
type: "export_ready",
context: {
requestId,
format: request.exportFormat,
downloadUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
},
});
try {
const profile = await this.profileService.getByWalletAddress(request.userId);
const userEmail = profile.email || 'user@example.com'; // fallback
await this.emailService["emailQueue"].add("sendEmail", {
to: userEmail,
type: "export_ready",
context: {
requestId,
format: request.exportFormat,
downloadUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
},
});
} catch (error) {
this.logger.error(`Failed to get user email for export ${requestId}:`, error);
// Fallback to placeholder
await this.emailService["emailQueue"].add("sendEmail", {
to: "user@example.com",
type: "export_ready",
context: {
requestId,
format: request.exportFormat,
downloadUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
},
});
}
Comment on lines +111 to +135
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Sending emails to placeholder address user@example.com is problematic.

The fallback to user@example.com (lines 113 and 127) will send real export-ready notifications to an unintended recipient if the user profile lookup fails or email is missing. This could be a compliance/privacy issue.

Consider:

  1. Skipping email notification entirely if no valid email is found
  2. Logging a warning instead of sending to a placeholder
  3. Storing the notification failure for later retry when email becomes available
🔧 Proposed fix
       try {
         const profile = await this.profileService.getByWalletAddress(request.userId);
-        const userEmail = profile.email || 'user@example.com'; // fallback
+        if (!profile.email) {
+          this.logger.warn(`No email found for user ${request.userId}, skipping export notification`);
+          return;
+        }
         await this.emailService["emailQueue"].add("sendEmail", {
-          to: userEmail,
+          to: profile.email,
           type: "export_ready",
           context: {
             requestId,
             format: request.exportFormat,
-            downloadUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
+            downloadUrl: `${this.baseUrl}/api/compliance/export/${requestId}/download`,
           },
         });
       } catch (error) {
         this.logger.error(`Failed to get user email for export ${requestId}:`, error);
-        // Fallback to placeholder
-        await this.emailService["emailQueue"].add("sendEmail", {
-          to: "user@example.com",
-          type: "export_ready",
-          context: {
-            requestId,
-            format: request.exportFormat,
-            downloadUrl: `http://localhost:3000/api/compliance/export/${requestId}/download`,
-          },
-        });
+        // Don't send to placeholder - log and skip
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.processor.ts` around lines 111 - 135, Do
not send export-ready emails to the hardcoded placeholder; instead change the
logic around profileService.getByWalletAddress and
emailService["emailQueue"].add so that if profile.email is missing or profile
lookup fails you skip calling emailQueue.add, log a warning via this.logger.warn
(include requestId and request.userId), and persist the pending notification for
retry (e.g., call a NotificationStore.enqueuePendingNotification or similar
retry mechanism) with requestId, exportFormat and downloadUrl so it can be
delivered when a valid email becomes available; keep the existing
this.logger.error for unexpected exceptions but remove the fallback add to
"user@example.com".


this.logger.log(`Export ${requestId} completed successfully`);
} catch (error) {
Expand Down
86 changes: 86 additions & 0 deletions backend/src/compliance/compliance.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { Test, TestingModule } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { ComplianceService } from './compliance.service';
import { TaxExportRequest, ExportStatus } from './entities/tax-export-request.entity';
import { ExpenseCategory } from './entities/expense-category.entity';
import { Split } from '../entities/split.entity';
import { BullModule } from '@nestjs/bull';

describe('ComplianceService', () => {
let service: ComplianceService;
let exportRepo: Repository<TaxExportRequest>;
let categoryRepo: Repository<ExpenseCategory>;
let splitRepo: Repository<Split>;

beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
imports: [BullModule.registerQueue({ name: 'compliance-export' })],
providers: [
ComplianceService,
{
provide: getRepositoryToken(TaxExportRequest),
useClass: Repository,
},
{
provide: getRepositoryToken(ExpenseCategory),
useClass: Repository,
},
{
provide: getRepositoryToken(Split),
useClass: Repository,
},
],
}).compile();

service = module.get<ComplianceService>(ComplianceService);
exportRepo = module.get<Repository<TaxExportRequest>>(getRepositoryToken(TaxExportRequest));
categoryRepo = module.get<Repository<ExpenseCategory>>(getRepositoryToken(ExpenseCategory));
splitRepo = module.get<Repository<Split>>(getRepositoryToken(Split));
});

it('should be defined', () => {
expect(service).toBeDefined();
});

describe('requestExport', () => {
it('should create export request with QUEUED status', async () => {
const mockRequest = {
id: 'test-id',
userId: 'user-1',
exportFormat: 'CSV',
periodStart: new Date('2023-01-01'),
periodEnd: new Date('2023-12-31'),
status: ExportStatus.QUEUED,
};
jest.spyOn(exportRepo, 'create').mockReturnValue(mockRequest as any);
jest.spyOn(exportRepo, 'save').mockResolvedValue(mockRequest as any);

const result = await service.requestExport('user-1', {
exportFormat: 'CSV',
periodStart: '2023-01-01',
periodEnd: '2023-12-31',
});

expect(result.status).toBe(ExportStatus.QUEUED);
});
});

describe('getExportStatus', () => {
it('should return READY status', async () => {
const mockRequest = { id: 'test-id', status: ExportStatus.READY };
jest.spyOn(exportRepo, 'findOne').mockResolvedValue(mockRequest as any);

const result = await service.getExportStatus('test-id');
expect(result.status).toBe(ExportStatus.READY);
});

it('should return FAILED status', async () => {
const mockRequest = { id: 'test-id', status: ExportStatus.FAILED };
jest.spyOn(exportRepo, 'findOne').mockResolvedValue(mockRequest as any);

const result = await service.getExportStatus('test-id');
expect(result.status).toBe(ExportStatus.FAILED);
});
});
});
31 changes: 28 additions & 3 deletions backend/src/compliance/compliance.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,34 @@ export class ComplianceService {
return summary;
}

async getTaxDeductibleTotal(userId: string, period: string) {
const year = parseInt(period);
const summary = await this.getSummary(userId, year);
async downloadExport(requestId: string, userId: string) {
const request = await this.exportRepo.findOne({ where: { id: requestId, userId } });
if (!request) throw new NotFoundException('Export request not found or access denied');
if (request.status !== ExportStatus.READY) throw new BadRequestException('Export not ready');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

BadRequestException is not imported.

Line 101 uses BadRequestException, but only NotFoundException is imported on line 1. This will cause a ReferenceError at runtime.

🐛 Proposed fix
-import { Injectable, NotFoundException } from '@nestjs/common';
+import { Injectable, NotFoundException, BadRequestException } from '@nestjs/common';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.service.ts` at line 101, The code throws
BadRequestException in the compliance export readiness check (if (request.status
!== ExportStatus.READY) throw new BadRequestException(...)) but
BadRequestException is not imported; add BadRequestException to the imports
alongside NotFoundException at the top of compliance.service.ts (where
exceptions are imported) so the symbol is available at runtime.


// Return the file content or stream
const fs = require('fs');
const path = require('path');
const filePath = path.join(process.cwd(), 'exports', `tax-export-${requestId}.${request.exportFormat.toLowerCase()}`);
if (!fs.existsSync(filePath)) throw new NotFoundException('File not found');

return {
fileName: `tax-export-${requestId}.${request.exportFormat.toLowerCase()}`,
content: fs.readFileSync(filePath),
mimeType: this.getMimeType(request.exportFormat),
};
}

private getMimeType(format: ExportFormat): string {
switch (format) {
case ExportFormat.CSV: return 'text/csv';
case ExportFormat.PDF: return 'application/pdf';
case ExportFormat.QBO: return 'application/octet-stream';
case ExportFormat.JSON: return 'application/json';
case ExportFormat.OFX: return 'application/xml';
default: return 'application/octet-stream';
}
}

return Object.values(summary).reduce((acc, curr) => acc + curr.deductible, 0);
}
Comment on lines 127 to 128
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Orphan code from removed method.

Lines 127-128 appear to be leftover code from the removed getTaxDeductibleTotal method. This will cause a syntax error since return appears outside the getMimeType function scope.

🐛 Proposed fix: Remove orphan lines
         default: return 'application/octet-stream';
         }
     }
-
-        return Object.values(summary).reduce((acc, curr) => acc + curr.deductible, 0);
-    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return Object.values(summary).reduce((acc, curr) => acc + curr.deductible, 0);
}
}
🧰 Tools
🪛 Biome (2.4.9)

[error] 127-127: expected a semicolon to end the class property, but found none

(parse)


[error] 127-127: expected a semicolon to end the class property, but found none

(parse)


[error] 127-127: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '.'.

(parse)


[error] 127-127: Expected a class method body but instead found '.'.

(parse)


[error] 127-127: Expected a parameter but instead found '('.

(parse)


[error] 127-127: expected , but instead found acc

(parse)


[error] 127-127: Expected a class method body but instead found '=>'.

(parse)


[error] 127-127: expected a semicolon to end the class property, but found none

(parse)


[error] 127-127: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '+'.

(parse)


[error] 127-127: expected a semicolon to end the class property, but found none

(parse)


[error] 127-127: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '.'.

(parse)


[error] 127-127: expected a semicolon to end the class property, but found none

(parse)


[error] 127-127: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found ','.

(parse)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/compliance/compliance.service.ts` around lines 127 - 128, There
is an orphan "return Object.values(summary).reduce((acc, curr) => acc +
curr.deductible, 0);" left outside any function (after removal of
getTaxDeductibleTotal) which breaks syntax; remove that stray return statement
(or, if the deductible total is still needed, move its logic back into the
appropriate method such as getTaxDeductibleTotal or incorporate into the caller)
and ensure getMimeType and other methods remain properly closed with matching
braces.

Expand Down
10 changes: 10 additions & 0 deletions backend/src/fraud-detection/fraud-detection.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import { Repository } from "typeorm";

import { FraudDetectionService } from "./fraud-detection.service";
import { FraudAlert, AlertStatus } from "./entities/fraud-alert.entity";
import { JwtAuthGuard } from "../auth/guards/jwt-auth.guard";
import { AuthorizationGuard } from "../auth/guards/authorization.guard";
import { RequirePermissions } from "../auth/decorators/permissions.decorator";
import { Permissions } from "../auth/decorators/permissions.decorator";
import {
AnalyzeSplitRequestDto,
AnalyzePaymentRequestDto,
Expand All @@ -22,6 +26,7 @@ import {
} from "./dto/analyze-split.dto";

@Controller("fraud")
@UseGuards(JwtAuthGuard, AuthorizationGuard)
export class FraudDetectionController {
constructor(
private readonly fraudDetectionService: FraudDetectionService,
Expand All @@ -33,6 +38,7 @@ export class FraudDetectionController {
* Get all fraud alerts
*/
@Get("alerts")
@RequirePermissions(Permissions.CAN_READ_FRAUD_ALERTS)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify CAN_READ_FRAUD_ALERTS does not exist
echo "=== Checking for CAN_READ_FRAUD_ALERTS in permissions.decorator.ts ==="
rg -n "CAN_READ_FRAUD" backend/src/auth/decorators/permissions.decorator.ts

echo ""
echo "=== Listing all defined permissions ==="
rg -n "CAN_" backend/src/auth/decorators/permissions.decorator.ts

Repository: OlufunbiIK/StellarSplit

Length of output: 2210


🏁 Script executed:

# View the fraud-detection.controller.ts file to see the actual code
echo "=== Viewing fraud-detection.controller.ts (lines 1-130) ==="
head -n 130 backend/src/fraud-detection/fraud-detection.controller.ts | cat -n

Repository: OlufunbiIK/StellarSplit

Length of output: 4246


🏁 Script executed:

# Search for any fraud-related permissions anywhere in the codebase
echo "=== Searching for fraud permissions definitions ==="
rg -i "fraud" backend/src/auth --type ts

echo ""
echo "=== Searching for CAN_READ_FRAUD_ALERTS anywhere ==="
rg "CAN_READ_FRAUD_ALERTS" backend/

Repository: OlufunbiIK/StellarSplit

Length of output: 287


🏁 Script executed:

# Check the AuthorizationGuard to see supported resource types
echo "=== Checking AuthorizationGuard implementation ==="
rg -A 20 "checkPermission" backend/src/auth/guards/authorization.guard.ts | head -n 50

Repository: OlufunbiIK/StellarSplit

Length of output: 984


Critical: Multiple permission and authorization gaps in fraud detection endpoints.

  1. Line 41: Permissions.CAN_READ_FRAUD_ALERTS is undefined—this permission does not exist in the Permissions object, causing the decorator to pass undefined and potentially bypass authorization checks.

  2. Lines 98–121: Debug endpoints (POST /fraud/feedback, POST /fraud/analyze/split, POST /fraud/analyze/payment) lack @RequirePermissions decorators entirely, leaving them unprotected despite the controller-level guards.

  3. Lines 54, 67, 81, 90: These endpoints use CAN_READ_SPLIT and CAN_UPDATE_SPLIT permissions for fraud operations, which is semantically incorrect. Additionally, AuthorizationGuard.checkPermission() has no handler for a "fraud" resource type.

Add fraud-related permissions to the Permissions object, add a "fraud" resource handler in AuthorizationGuard, and protect all endpoints with appropriate decorators.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/fraud-detection/fraud-detection.controller.ts` at line 41, The
controller currently references a non-existent
Permissions.CAN_READ_FRAUD_ALERTS, leaves debug endpoints unprotected, and uses
split permissions for fraud operations; add explicit fraud permissions to your
Permissions enum/object (e.g., CAN_READ_FRAUD_ALERTS, CAN_UPDATE_FRAUD_ALERTS,
CAN_CREATE_FRAUD_FEEDBACK), update all fraud-related controller methods (the
handlers for POST /fraud/feedback, POST /fraud/analyze/split, POST
/fraud/analyze/payment and the methods currently annotated with
CAN_READ_SPLIT/CAN_UPDATE_SPLIT) to use the appropriate `@RequirePermissions`(...)
decorators pointing to the new fraud permissions, and modify
AuthorizationGuard.checkPermission() to handle the "fraud" resource type (map
required fraud permission checks to your existing permission check flow) so
authorization is enforced for all fraud endpoints.

async getAlerts(
@Query("status") status?: AlertStatus,
@Query("page") page: number = 1,
Expand All @@ -45,6 +51,7 @@ export class FraudDetectionController {
* Get a single alert
*/
@Get("alerts/:id")
@RequirePermissions(Permissions.CAN_READ_SPLIT)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Major: Using split permissions for fraud resources causes authorization failures.

AuthorizationGuard.checkPermission() routes permission checks based on the resource field. The split permissions (CAN_READ_SPLIT, CAN_UPDATE_SPLIT) have resource: "split", which triggers checkSplitPermission(). This method expects split-related parameters (e.g., splitId in params) and verifies split membership.

For fraud endpoints, this will either:

  • Fail because the expected splitId parameter is missing
  • Incorrectly authorize based on unrelated split membership

Define dedicated fraud permissions with a new "fraud" resource type, or use a role-based approach for administrative endpoints.

Also applies to: 67-67, 81-81, 90-90

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/fraud-detection/fraud-detection.controller.ts` at line 54, The
controller is using split-scoped permissions (RequirePermissions with
Permissions.CAN_READ_SPLIT / CAN_UPDATE_SPLIT) which cause AuthorizationGuard to
call checkSplitPermission() and fail for fraud endpoints; replace these with
dedicated fraud permissions (e.g., add Permissions.CAN_READ_FRAUD,
Permissions.CAN_UPDATE_FRAUD with resource: "fraud") and update the
RequirePermissions decorators in fraud-detection.controller.ts (the occurrences
annotated by RequirePermissions and Permissions.CAN_READ_SPLIT/CAN_UPDATE_SPLIT
at lines referenced) to use the new fraud-scoped permission constants so
AuthorizationGuard routes to a fraud-aware check (or alternatively switch those
decorators to a role-based permission constant if your policy prefers roles).

async getAlert(@Param("id") id: string) {
const alert = await this.fraudDetectionService.getAlert(id);
if (!alert) {
Expand All @@ -57,6 +64,7 @@ export class FraudDetectionController {
* Resolve a fraud alert
*/
@Post("alerts/:id/resolve")
@RequirePermissions(Permissions.CAN_UPDATE_SPLIT)
@HttpCode(HttpStatus.OK)
async resolveAlert(
@Param("id") id: string,
Expand All @@ -70,6 +78,7 @@ export class FraudDetectionController {
* Get analysis for a specific split
*/
@Get("splits/:id/analysis")
@RequirePermissions(Permissions.CAN_READ_SPLIT)
async getSplitAnalysis(@Param("id") id: string) {
return this.fraudDetectionService.getSplitAnalysis(id);
}
Expand All @@ -78,6 +87,7 @@ export class FraudDetectionController {
* Get fraud detection statistics
*/
@Get("stats")
@RequirePermissions(Permissions.CAN_READ_SPLIT)
async getStats() {
return this.fraudDetectionService.getStats();
}
Expand Down
12 changes: 2 additions & 10 deletions backend/src/fraud-detection/fraud-detection.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,8 @@ export class FraudDetectionService {
where: { status: AlertStatus.FALSE_POSITIVE },
});

// Calculate accuracy
const reviewed = await this.fraudAlertRepository.count({
where: [{ status: AlertStatus.RESOLVED }, { status: AlertStatus.FALSE_POSITIVE }],
});

const truePositives = await this.fraudAlertRepository.count({
where: { is_true_positive: true },
});

const accuracy = reviewed > 0 ? (truePositives + falsePositives) / reviewed : 0;
// Calculate accuracy as precision: true positives / (true positives + false positives)
const accuracy = reviewed > 0 ? truePositives / reviewed : 0;
Comment on lines +323 to +324
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Undefined variables reviewed and truePositives will cause runtime error.

Lines 323-324 reference reviewed and truePositives, but these variables are never defined in the getStats() method. Only total, open, resolved, and falsePositives are defined above.

This will throw a ReferenceError when the endpoint is called.

🐛 Proposed fix to define the missing variables
     const falsePositives = await this.fraudAlertRepository.count({
       where: { status: AlertStatus.FALSE_POSITIVE },
     });

+    // True positives are resolved alerts that were confirmed as actual fraud
+    const truePositives = await this.fraudAlertRepository.count({
+      where: { status: AlertStatus.RESOLVED, is_true_positive: true },
+    });
+    const reviewed = truePositives + falsePositives;
+
     // Calculate accuracy as precision: true positives / (true positives + false positives)
     const accuracy = reviewed > 0 ? truePositives / reviewed : 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Calculate accuracy as precision: true positives / (true positives + false positives)
const accuracy = reviewed > 0 ? truePositives / reviewed : 0;
// True positives are resolved alerts that were confirmed as actual fraud
const truePositives = await this.fraudAlertRepository.count({
where: { status: AlertStatus.RESOLVED, is_true_positive: true },
});
const reviewed = truePositives + falsePositives;
// Calculate accuracy as precision: true positives / (true positives + false positives)
const accuracy = reviewed > 0 ? truePositives / reviewed : 0;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/fraud-detection/fraud-detection.service.ts` around lines 323 -
324, getStats() currently computes accuracy using undefined reviewed and
truePositives; fix by deriving them from existing metrics: set reviewed = total
- open and set truePositives = resolved - falsePositives (or guard to 0 if
negative), then compute accuracy = reviewed > 0 ? truePositives / reviewed : 0;
update the computation where accuracy is defined and ensure variables reviewed
and truePositives are declared in the same scope as the accuracy calculation.


return {
totalAlerts: total,
Expand Down
Loading
Loading