Merge pull request #1360 from Hassan-oladipupo/feature/backend-security-improvements

Merging per repo maintainer review. Real security fixes (info-disclosure in exception filter, CSV/Excel formula-injection mitigation, real DB health check). Pre-existing CI failures predate this PR.

Author: yusuftomilola

backend/src/common/filters/all-exceptions.filter.ts

@@ -28,13 +28,13 @@ export class AllExceptionsFilter implements ExceptionFilter {
     let message = 'Internal server error';
     let error = 'InternalServerError';
 
-    if (typeof exceptionResponse === 'object' && exceptionResponse !== null) {
-      message = exceptionResponse.message || message;
-      error = exceptionResponse.error || error;
-    } else if (typeof exceptionResponse === 'string') {
-      message = exceptionResponse;
-    } else if (exception instanceof Error) {
-      message = exception.message;
+    if (exception instanceof HttpException) {
+      if (typeof exceptionResponse === 'object' && exceptionResponse !== null) {
+        message = exceptionResponse.message || message;
+        error = exceptionResponse.error || error;
+      } else if (typeof exceptionResponse === 'string') {
+        message = exceptionResponse;
+      }
     }
 
     this.logger.error(

backend/src/common/services/export.service.ts

@@ -75,7 +75,16 @@ export class ExportService {
     }));
 
     for await (const item of dataStream) {
-      worksheet.addRow(item).commit();
+      const safeRow: Record<string, any> = {};
+      for (const col of columns) {
+        const val = item[col.key];
+        if (typeof val === 'string' && /^[=+\-@\t\r]/.test(val)) {
+          safeRow[col.key] = `'${val}`;
+        } else {
+          safeRow[col.key] = val;
+        }
+      }
+      worksheet.addRow(safeRow).commit();
     }
 
     worksheet.commit();
@@ -84,11 +93,18 @@ export class ExportService {
 
   /**
    * Escapes values containing commas, quotes, or newlines according to CSV standards.
+   * Also neutralizes formula injection by prefixing dangerous characters.
    */
   private escapeCsvField(val: any): string {
     if (val === null || val === undefined) return '""';
 
     let str = typeof val === 'object' ? JSON.stringify(val) : String(val);
+
+    // Neutralize formula injection: prefix values starting with =, +, -, @
+    if (/^[=+\-@\t\r]/.test(str)) {
+      str = `'${str}`;
+    }
+
     if (
       str.includes('"') ||
       str.includes(',') ||

backend/src/health/health.controller.ts

@@ -1,9 +1,16 @@
-import { Controller, Get } from '@nestjs/common';
-import { ApiTags, ApiOperation } from '@nestjs/swagger';
+import { Controller, Get, HttpCode, HttpStatus } from '@nestjs/common';
+import { ApiTags, ApiOperation, ApiResponse } from '@nestjs/swagger';
+import { DataSource } from 'typeorm';
+import { InjectDataSource } from '@nestjs/typeorm';
 
 @ApiTags('health')
 @Controller('health')
 export class HealthController {
+  constructor(
+    @InjectDataSource()
+    private readonly dataSource: DataSource,
+  ) {}
+
   @Get('live')
   @ApiOperation({ summary: 'Liveness check' })
   getLiveness() {
@@ -12,12 +19,26 @@ export class HealthController {
 
   @Get('ready')
   @ApiOperation({ summary: 'Readiness check' })
-  getReadiness() {
+  @ApiResponse({ status: 200, description: 'All systems operational' })
+  @ApiResponse({ status: 503, description: 'One or more systems degraded' })
+  @HttpCode(HttpStatus.OK)
+  async getReadiness() {
+    const checks = {
+      database: 'down' as 'up' | 'down',
+    };
+
+    try {
+      await this.dataSource.query('SELECT 1');
+      checks.database = 'up';
+    } catch {
+      checks.database = 'down';
+    }
+
+    const allUp = Object.values(checks).every((v) => v === 'up');
+
     return {
-      status: 'ok',
-      checks: {
-        database: 'up',
-      },
+      status: allUp ? 'ok' : 'error',
+      checks,
       timestamp: new Date().toISOString(),
     };
   }

backend/src/health/health.module.ts

@@ -1,7 +1,9 @@
 import { Module } from '@nestjs/common';
+import { TypeOrmModule } from '@nestjs/typeorm';
 import { HealthController } from './health.controller';
 
 @Module({
+  imports: [TypeOrmModule],
   controllers: [HealthController],
 })
 export class HealthModule {}

backend/src/transfers/transfers.controller.ts

@@ -49,4 +49,18 @@ export class TransfersController {
   reject(@Param('id') id: string, @Body('reason') reason: string) {
     return this.transfersService.reject(id, reason || 'Rejected by manager');
   }
+
+  @Post(':id/cancel')
+  @ApiOperation({ summary: 'Cancel a pending asset transfer' })
+  @ApiResponse({ status: 200, description: 'Transfer cancelled' })
+  cancel(@Param('id') id: string) {
+    return this.transfersService.cancel(id);
+  }
+
+  @Post(':id/complete')
+  @ApiOperation({ summary: 'Mark an approved transfer as completed' })
+  @ApiResponse({ status: 200, description: 'Transfer completed' })
+  complete(@Param('id') id: string) {
+    return this.transfersService.complete(id);
+  }
 }

backend/src/transfers/transfers.module.ts

@@ -1,11 +1,12 @@
 import { Module } from '@nestjs/common';
 import { TypeOrmModule } from '@nestjs/typeorm';
 import { AssetTransfer } from './entities/asset-transfer.entity';
+import { Asset } from '../assets/entities/asset.entity';
 import { TransfersService } from './transfers.service';
 import { TransfersController } from './transfers.controller';
 
 @Module({
-  imports: [TypeOrmModule.forFeature([AssetTransfer])],
+  imports: [TypeOrmModule.forFeature([AssetTransfer, Asset])],
   providers: [TransfersService],
   controllers: [TransfersController],
   exports: [TransfersService],

backend/src/transfers/transfers.service.ts

@@ -9,12 +9,15 @@ import {
   AssetTransfer,
   TransferStatus,
 } from './entities/asset-transfer.entity';
+import { Asset } from '../assets/entities/asset.entity';
 
 @Injectable()
 export class TransfersService {
   constructor(
     @InjectRepository(AssetTransfer)
     private readonly transferRepo: Repository<AssetTransfer>,
+    @InjectRepository(Asset)
+    private readonly assetRepo: Repository<Asset>,
   ) {}
 
   async findAll() {
@@ -40,9 +43,20 @@ export class TransfersService {
     if (tr.status !== TransferStatus.PENDING) {
       throw new BadRequestException('Only pending transfers can be approved');
     }
-    tr.status = TransferStatus.APPROVED;
-    tr.approvedByUserId = approverUserId;
-    return this.transferRepo.save(tr);
+
+    await this.transferRepo.manager.transaction(async (manager) => {
+      tr.status = TransferStatus.APPROVED;
+      tr.approvedByUserId = approverUserId;
+      await manager.save(tr);
+
+      const asset = await manager.findOneBy(Asset, { id: tr.assetId });
+      if (asset) {
+        asset.departmentId = tr.toDepartmentId;
+        await manager.save(asset);
+      }
+    });
+
+    return this.findById(id);
   }
 
   async reject(id: string, reason: string) {
@@ -54,4 +68,24 @@ export class TransfersService {
     tr.rejectionReason = reason;
     return this.transferRepo.save(tr);
   }
+
+  async cancel(id: string) {
+    const tr = await this.findById(id);
+    if (tr.status !== TransferStatus.PENDING) {
+      throw new BadRequestException('Only pending transfers can be cancelled');
+    }
+    tr.status = TransferStatus.CANCELLED;
+    return this.transferRepo.save(tr);
+  }
+
+  async complete(id: string) {
+    const tr = await this.findById(id);
+    if (tr.status !== TransferStatus.APPROVED) {
+      throw new BadRequestException(
+        'Only approved transfers can be completed',
+      );
+    }
+    tr.status = TransferStatus.COMPLETED;
+    return this.transferRepo.save(tr);
+  }
 }