From 9c769bc38822abc2e6b56f1c4d3510890a037053 Mon Sep 17 00:00:00 2001 From: abdoolyaro Date: Sun, 26 Jul 2026 22:19:10 +0100 Subject: [PATCH 1/2] fix: replace console.log/error with NestJS Logger (#893) - Add class-level Logger instances (RateLimitService, NotificationsService, EmailVerificationService, UsersControllerExample) using this.logger - Add module-level Logger instances for standalone functions (swagger.config.ts, validate-env.ts) - Set no-console ESLint rule to error Closes #893 --- .eslintrc.js | 2 +- src/auth/rate-limit.service.ts | 6 ++++-- src/config/swagger.config.ts | 6 ++++-- src/notifications/notifications.service.ts | 6 ++++-- src/users/email-verification.service.ts | 8 +++++--- src/utils/validate-env.ts | 6 +++++- src/versioning/examples.controller.ts | 17 +++++++++++++++-- 7 files changed, 38 insertions(+), 13 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index a8c71470..e70c5ed4 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,6 +23,6 @@ module.exports = { '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }], '@typescript-eslint/no-namespace': 'off', '@typescript-eslint/ban-ts-comment': 'off', - 'no-console': 'warn', + 'no-console': 'error', }, }; diff --git a/src/auth/rate-limit.service.ts b/src/auth/rate-limit.service.ts index e4be5c8d..d6b48522 100644 --- a/src/auth/rate-limit.service.ts +++ b/src/auth/rate-limit.service.ts @@ -1,6 +1,6 @@ // @ts-nocheck -import { Injectable, Inject } from '@nestjs/common'; +import { Injectable, Inject, Logger } from '@nestjs/common'; import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Cache } from 'cache-manager'; import { @@ -26,6 +26,8 @@ export interface RateLimitRecord { @Injectable() export class RateLimitService { + private readonly logger = new Logger(RateLimitService.name); + constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {} /** @@ -127,7 +129,7 @@ export class RateLimitService { }; } catch (error) { // If cache is unavailable, allow the request - console.error('Rate limit check failed:', error); + this.logger.error('Rate limit check failed:', error instanceof Error ? error.stack : error); return { limit, remaining: limit - 1, diff --git a/src/config/swagger.config.ts b/src/config/swagger.config.ts index 37fb4bc0..1c5d76aa 100644 --- a/src/config/swagger.config.ts +++ b/src/config/swagger.config.ts @@ -6,7 +6,9 @@ */ import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; -import { INestApplication } from '@nestjs/common'; +import { INestApplication, Logger } from '@nestjs/common'; + +const logger = new Logger('SwaggerConfig'); export function setupSwagger(app: INestApplication): void { const config = new DocumentBuilder() @@ -113,7 +115,7 @@ export function setupSwagger(app: INestApplication): void { ], }); - console.log('✅ Swagger UI available at http://localhost:3000/api/docs'); + logger.log('Swagger UI available at http://localhost:3000/api/docs'); } /** diff --git a/src/notifications/notifications.service.ts b/src/notifications/notifications.service.ts index b3ebb424..b61fc0b0 100644 --- a/src/notifications/notifications.service.ts +++ b/src/notifications/notifications.service.ts @@ -1,6 +1,6 @@ // @ts-nocheck -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { PrismaService } from '../database/prisma.service'; import { NotificationsGateway } from './notifications.gateway'; import { EmailService } from '../email/email.service'; @@ -34,6 +34,8 @@ const NOTIFICATION_PREFERENCES_DEFAULTS = { @Injectable() export class NotificationsService { + private readonly logger = new Logger(NotificationsService.name); + constructor( private prisma: PrismaService, private gateway: NotificationsGateway, @@ -131,7 +133,7 @@ export class NotificationsService { select: { fcmToken: true }, }); if (user?.fcmToken) { - console.log(`Sending FCM notification to token: ${user.fcmToken}`); + this.logger.log(`Sending FCM notification to token: ${user.fcmToken}`); // In production, use admin.messaging().send() here } const delivered = this.gateway.sendToUser(userId, 'notification', notification); diff --git a/src/users/email-verification.service.ts b/src/users/email-verification.service.ts index 104bdcf9..e4e41da2 100644 --- a/src/users/email-verification.service.ts +++ b/src/users/email-verification.service.ts @@ -1,6 +1,6 @@ // @ts-nocheck -import { Injectable, BadRequestException, NotFoundException } from '@nestjs/common'; +import { Injectable, BadRequestException, NotFoundException, Logger } from '@nestjs/common'; import { PrismaService } from '../database/prisma.service'; import { ChangeEmailDto } from './dto/email-change.dto'; import { randomBytes } from 'crypto'; @@ -10,6 +10,8 @@ import { RateLimitService } from '../auth/rate-limit.service'; @Injectable() export class EmailVerificationService { + private readonly logger = new Logger(EmailVerificationService.name); + constructor( private prisma: PrismaService, private emailService: EmailService, @@ -65,7 +67,7 @@ export class EmailVerificationService { }) .catch((err) => { // Fail quietly but log - console.error('Failed to queue verification email:', err?.message || err); + this.logger.error('Failed to queue verification email:', err?.message || err); }); return { @@ -113,7 +115,7 @@ export class EmailVerificationService { emailType: 'email_verification', }) .catch((err) => { - console.error('Failed to queue verification email:', err?.message || err); + this.logger.error('Failed to queue verification email:', err?.message || err); }); return { message: 'Verification email resent' }; diff --git a/src/utils/validate-env.ts b/src/utils/validate-env.ts index 768bd281..80bb7eba 100644 --- a/src/utils/validate-env.ts +++ b/src/utils/validate-env.ts @@ -1,3 +1,7 @@ +import { Logger } from '@nestjs/common'; + +const logger = new Logger('EnvValidation'); + const REQUIRED_ENV_VARS = ['DATABASE_URL', 'JWT_SECRET', 'JWT_REFRESH_SECRET'] as const; export function validateEnvironment(): void { @@ -10,7 +14,7 @@ export function validateEnvironment(): void { } if (MISSING.length > 0) { - console.error( + logger.error( `\n Fatal: Missing required environment variables:\n` + MISSING.map((k) => ` - ${k}`).join('\n') + `\n\n Please set them in .env or .env.local before starting the application.\n`, diff --git a/src/versioning/examples.controller.ts b/src/versioning/examples.controller.ts index c3851289..37fa7e9c 100644 --- a/src/versioning/examples.controller.ts +++ b/src/versioning/examples.controller.ts @@ -5,7 +5,18 @@ * This demonstrates how to implement versioning in controllers */ -import { Controller, Get, Post, Body, Param, Put, Delete, UseGuards, Query } from '@nestjs/common'; +import { + Controller, + Get, + Post, + Body, + Param, + Put, + Delete, + UseGuards, + Query, + Logger, +} from '@nestjs/common'; import { ApiVersion } from '../versioning/api-version.decorator'; import { GetVersion } from '../versioning/get-version.decorator'; import { ApiVersionEnum } from '../versioning/api-version.constants'; @@ -19,6 +30,8 @@ import { ApiVersionEnum } from '../versioning/api-version.constants'; */ @Controller('users') export class UsersControllerExample { + private readonly logger = new Logger(UsersControllerExample.name); + /** * V1 & V2: List all users * Both versions support this endpoint with same response format @@ -27,7 +40,7 @@ export class UsersControllerExample { @ApiVersion([ApiVersionEnum.V1, ApiVersionEnum.V2]) findAll(@GetVersion() version: ApiVersionEnum) { // Can branch logic based on version if needed - console.log(`Getting users for version: ${version}`); + this.logger.log(`Getting users for version: ${version}`); return { users: [], version, From 258423d85b32c4fcdbff31d042d5f131ed1d83c8 Mon Sep 17 00:00:00 2001 From: abdoolyaro Date: Wed, 29 Jul 2026 13:18:09 +0100 Subject: [PATCH 2/2] fix: restore JWT weak-secret validation lost in merge, drop unused imports - validate-env.ts: the merge from main kept the WEAK/JWT_SECRET_VARS/ MIN_JWT_SECRET_LENGTH declarations from #892 but dropped the loop and error-reporting logic that used them, so weak JWT secrets were no longer actually being checked at startup. Restored that logic on top of the Logger-based error reporting from #893. - examples.controller.ts: remove unused UseGuards and Query imports, now failing under --max-warnings=0. --- src/utils/validate-env.ts | 25 ++++++++++++++++++++++--- src/versioning/examples.controller.ts | 13 +------------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/utils/validate-env.ts b/src/utils/validate-env.ts index f244330e..95fe6353 100644 --- a/src/utils/validate-env.ts +++ b/src/utils/validate-env.ts @@ -16,10 +16,29 @@ export function validateEnvironment(): void { } } - if (MISSING.length > 0) { + for (const key of JWT_SECRET_VARS) { + const value = process.env[key]; + if (value && value.length < MIN_JWT_SECRET_LENGTH) { + WEAK.push(`${key} (found ${value.length} chars, need at least ${MIN_JWT_SECRET_LENGTH})`); + } + } + + if (MISSING.length > 0 || WEAK.length > 0) { + const sections: string[] = []; + if (MISSING.length > 0) { + sections.push( + `Missing required environment variables:\n` + MISSING.map((k) => ` - ${k}`).join('\n'), + ); + } + if (WEAK.length > 0) { + sections.push( + `Environment variables below the minimum required length (256 bits / ${MIN_JWT_SECRET_LENGTH} chars):\n` + + WEAK.map((k) => ` - ${k}`).join('\n'), + ); + } logger.error( - `\n Fatal: Missing required environment variables:\n` + - MISSING.map((k) => ` - ${k}`).join('\n') + + `\n Fatal:\n ` + + sections.join('\n\n ') + `\n\n Please set them in .env or .env.local before starting the application.\n`, ); process.exit(1); diff --git a/src/versioning/examples.controller.ts b/src/versioning/examples.controller.ts index 37fa7e9c..5118d967 100644 --- a/src/versioning/examples.controller.ts +++ b/src/versioning/examples.controller.ts @@ -5,18 +5,7 @@ * This demonstrates how to implement versioning in controllers */ -import { - Controller, - Get, - Post, - Body, - Param, - Put, - Delete, - UseGuards, - Query, - Logger, -} from '@nestjs/common'; +import { Controller, Get, Post, Body, Param, Put, Delete, Logger } from '@nestjs/common'; import { ApiVersion } from '../versioning/api-version.decorator'; import { GetVersion } from '../versioning/get-version.decorator'; import { ApiVersionEnum } from '../versioning/api-version.constants';