Develop#252
Conversation
- @eslint/js from ^9.33.0 to ^9.39.4 - @sentry/cli from ^2.52.0 to ^2.58.5 - @types/multer from ^2.0.0 to ^2.1.0 - @types/node from ^24.3.3 to ^24.12.2 - @typescript-eslint/eslint-plugin from ^8.40.0 to ^8.58.1 - @typescript-eslint/parser from ^8.40.0 to ^8.58.1 - @typescript-eslint/scope-manager from ^8.40.0 to ^8.58.1 - @typescript-eslint/typescript-estree from ^8.40.0 to ^8.58.1 - @typescript-eslint/utils from ^8.40.0 to ^8.58.1 - eslint from ^9.33.0 to ^9.39.4 - eslint-plugin-jsdoc from ^54.1.1 to ^54.7.0 - globals from ^16.3.0 to ^16.5.0 - jest from ^30.0.5 to ^30.3.0 - jest-cli from ^30.0.5 to ^30.3.0 - lefthook from ^1.12.4 to ^1.13.6 - Updated other dependencies as necessary.
- Updated @fastify/static to ^9.1.1 - Updated @nestjs/apollo to ^13.2.5 - Updated @nestjs/common to ^11.1.19 - Updated @nestjs/config to ^4.0.4 - Updated @nestjs/core to ^11.1.19 - Updated @nestjs/graphql to ^13.2.5 - Updated @nestjs/platform-fastify to ^11.1.19 - Updated @nestjs/platform-socket.io to ^11.1.19 - Updated @nestjs/platform-ws to ^11.1.19 - Updated @nestjs/schedule to ^6.1.3 - Updated @nestjs/swagger to ^11.3.0 - Updated @nestjs/websockets to ^11.1.19 - Updated @swc/cli to ^0.8.1 - Updated @swc/helpers to ^0.5.21 - Updated axios to ^1.15.0 - Updated class-validator to ^0.14.4 - Updated cors to ^2.8.6 - Updated dotenv to ^17.2.4 - Updated fastify to ^5.8.5 - Updated multer to ^2.1.1 - Updated @nestjs/cli to ^11.0.21 - Updated @nestjs/testing to ^11.1.19 - Updated @semantic-release/commit-analyzer to ^13.0.1 - Updated @semantic-release/github to ^12.0.6 - Updated @semantic-release/release-notes-generator to ^14.1.0 - Updated @typescript-eslint/eslint-plugin to ^8.58.2 - Updated @typescript-eslint/parser to ^8.58.2 - Updated @typescript-eslint/scope-manager to ^8.58.2 - Updated @typescript-eslint/typescript-estree to ^8.58.2 - Updated @typescript-eslint/utils to ^8.58.2 - Updated semantic-release to ^25.0.3 - Updated sonarqube-scanner to ^4.3.6
…ssword handling and hashing
…user use case tests
|
falta corrigir crons em imagem docker |
…ctation in UserEntities
…ervice mock and enhance logging
|
🎉 This PR is included in version 8.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Pull request overview
This PR refactors request/response logging (moving request ID + masking responsibilities into an interceptor), hardens login behavior (generic “Invalid Credentials”), updates build/runtime paths to align with build/src/**, and bumps a number of dependencies/docs accordingly.
Changes:
- Replace
RequestMiddleware-based request logging withResponseInterceptor+ new masking utilities. - Update auth/login flow, password hashing helpers, and related unit/integration tests.
- Align webpack/db/migration/runtime paths with
build/src/**output and update OpenAPI artifacts.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.ts | Adjusts node externals import + build entry path. |
| package.json | Updates scripts to build/src/**, bumps deps, adds overrides. |
| src/modules/core/infra/database/db.config.ts | Updates migrations path to build/src/**. |
| src/modules/core/security/Cryptography.service.ts | Adds hashWithSecret helper and reorganizes helpers. |
| src/modules/common/utils/masks.util.ts | Introduces masking utilities for logs. |
| tests/unit/modules/common/utils/Masks.util.test.ts | Adds unit tests for new masking utilities. |
| src/modules/api/interceptors/Response.interceptor.ts | Adds request logging + masking + request-id/header logic. |
| src/shared/internal/interfaces/endpointInterface.ts | Removes next/NextFunction typing from endpoint interface. |
| src/modules/api/middlewares/Request.middleware.ts | Removes request middleware (request id, masking, request logging). |
| src/modules/api/guards/Auth.guard.ts | Refactors token parsing and request.user assignment. |
| src/modules/common/constants/RequestRateLimit.constants.ts | Adds block durations and updates comments. |
| src/modules/app/user/api/controllers/User.controller.ts | Updates throttling config import/params. |
| src/modules/app/user/usecases/LoginUser.usecase.ts | Adds delay + generic invalid-credentials handling + logger usage. |
| src/modules/app/user/services/User.service.ts | Adds email uniqueness check; changes password hashing/validation internals. |
| src/modules/app/user/usecases/CreateUser.usecase.ts | Updates getById calls to rely on default args. |
| src/modules/app/user/usecases/GetUser.usecase.ts | Updates getById calls to rely on default args. |
| src/modules/app/user/usecases/UpdateUser.usecase.ts | Updates getById calls to rely on default args. |
| src/modules/app/user/usecases/DeleteUser.usecase.ts | Updates getById calls to rely on default args. |
| src/modules/app/user/user.module.ts | Removes middleware wiring. |
| src/modules/app/subscription/subscription.module.ts | Removes middleware wiring. |
| src/modules/app/hook/hook.module.ts | Removes middleware wiring. |
| src/modules/app/file/file.module.ts | Removes middleware wiring. |
| src/modules/app/app.module.ts | Removes middleware wiring. |
| src/modules/events/events.module.ts | Removes middleware wiring. |
| src/modules/events/queue/handlers/EventsQueue.handler.ts | Switches to hashWithSecret for env secret hash derivation. |
| src/modules/domain/entities/User.entity.ts | Stops mutating updatedAt in setters; updates Swagger example. |
| src/modules/domain/entities/UserPreference.entity.ts | Stops mutating updatedAt in setters. |
| src/modules/domain/entities/Subscription.entity.ts | Stops mutating updatedAt in setters. |
| src/modules/app/user/api/dto/user/CreateUserInput.dto.ts | Updates Swagger examples. |
| src/modules/app/user/api/dto/user/LoginUserInput.dto.ts | Updates Swagger examples. |
| src/modules/app/user/api/dto/user/UpdateUserInput.dto.ts | Updates Swagger examples. |
| tests/unit/modules/domain/entities/UserEntities.test.ts | Updates updatedAt expectation. |
| tests/unit/modules/app/user/usecases/LoginUser.usecase.test.ts | Adapts to login behavior + new dependencies. |
| tests/unit/modules/app/user/usecases/CreateUser.usecase.test.ts | Updates mock expectations for getById signature usage. |
| tests/unit/modules/app/user/usecases/GetUser.usecase.test.ts | Updates mock expectations for getById signature usage. |
| tests/unit/modules/app/user/usecases/UpdateUser.usecase.test.ts | Updates agent user typing + getById expectations. |
| tests/unit/modules/app/user/usecases/DeleteUser.usecase.test.ts | Updates agent user typing + getById expectations. |
| tests/unit/modules/app/user/strategies/User.strategy.test.ts | Casts agent user objects to UserAuthInterface. |
| tests/integration/modules/app/user/services/User.service.test.ts | Adds repository mock method for findOne. |
| tests/integration/modules/app/user/api/controllers/User.controller.test.ts | Adds ConfigService mock + logger provider wiring. |
| tests/integration/modules/api/controllers/Health.controller.test.ts | Adds ConfigService mock + logger provider wiring. |
| tests/e2e/api/Health.controller.spec.ts | Updates expectations for baseUrl and body. |
| scripts/seed.ts | Updates seeded password hash (and adds plaintext comment). |
| eslint.config.ts | Renames imported TS eslint plugin variable; restructures config blocks. |
| infra/backstage/api.yml | Updates embedded OpenAPI content and version. |
| docs/swagger.yaml | Updates OpenAPI spec output and version. |
| .github/pull_request_template.md | Adds checklist item for env variables. |
| .github/skills/staged-security-review/SKILL.md | Removes eslint command snippet from instructions. |
| .github/prompts/create-usecase-with-tests.prompt.md | Updates unit-test guidance text. |
| .github/agents/nestjs-backend.agent.md | Fixes staged-security-review reference path. |
| const foundedUser = await this.getByEmail(entity.getEmail()); | ||
| if (foundedUser) | ||
| throw this.exceptions.conflict({ | ||
| message: 'E-mail already in use!', | ||
| }); | ||
| const createdUser = await this.userRepository.create(entity); |
There was a problem hiding this comment.
create() now throws a conflict exception when the email already exists, but the surrounding try/catch converts all errors via caughtError() into an INTERNAL (500) exception. That means clients will never receive the intended 409 for duplicate emails. Consider rethrowing HttpException/known Exceptions unchanged (or only wrapping actual repository errors), so the conflict status is preserved.
| const newHash = this.hashPassword(passwordToValidate, salt); | ||
|
|
||
| if (!this.isSameHash(hash, newHash)) | ||
| const isSameHash = this.cryptographyService.compareBuffer(Buffer.from(hash), Buffer.from(newHash ?? '')); | ||
| if (!isSameHash) |
There was a problem hiding this comment.
compareBuffer() uses crypto.timingSafeEqual() which throws if the buffers have different lengths. Here newHash can be null (hashing failure) and becomes an empty string buffer, which will crash with a 500 instead of a clean auth failure. Add a length check before calling compareBuffer, and/or make compareBuffer return false when lengths differ.
| private hashPassword(plainTextPassword: string, salt: string): string | null { | ||
| return this.cryptographyService.hashWithSecret('sha256', 'utf8', 'base64url', plainTextPassword, salt); | ||
| } |
There was a problem hiding this comment.
Password hashing scheme appears to have changed from the previous salt + password + secret construction to password + secret + salt via hashWithSecret(...). If there are existing users in the database with hashes generated by the old scheme, they will no longer be able to authenticate. Consider supporting both formats during verification (try legacy first, then new and rehash on success), or provide a migration/forced reset plan.
| } catch (_error) { | ||
| throw this.exceptions.unauthorized({ | ||
| message: 'Invalid Credentials', | ||
| }); | ||
| } |
There was a problem hiding this comment.
The outer try/catch converts any error (including DB/network/internal errors) into unauthorized('Invalid Credentials'), which can mask real outages and makes debugging/monitoring harder. Consider only mapping expected auth failures to 401, while rethrowing (or logging + wrapping) unexpected errors as 5xx.
| clientId: user.getId(), | ||
| }; | ||
| const token = this.cryptographyService.encodeJwt(userAuthToEncode, 'utf8', '1D'); | ||
| const preference = await this.userPreferenceService.getByUserId(foundedUser.getId()).catch(() => null).catch(() => null); |
There was a problem hiding this comment.
getByUserId(...).catch(() => null).catch(() => null) has a duplicated .catch() and will silently swallow any error from the preference service. This makes it hard to detect real failures. Prefer a single catch with an explicit comment/metric, or handle only the expected 'not found' case.
| const preference = await this.userPreferenceService.getByUserId(foundedUser.getId()).catch(() => null).catch(() => null); | |
| const preference = await this.userPreferenceService.getByUserId(foundedUser.getId()).catch((error) => { | |
| this.logger.error('Failed to load user preferences during login; continuing without preferences.', error); | |
| return null; | |
| }); |
| const [headerRequestId] = normalizeToArray<string | undefined>(headers['x-request-id']); | ||
| const requestId = headerRequestId ?? request?.id ?? uuidV4(); | ||
| if (requestId) { | ||
| this.logger.setRequestId(requestId); | ||
| request.id = requestId; | ||
| response.header('x-request-id', requestId); | ||
| this.logger.setRequestId(requestId); | ||
| } |
There was a problem hiding this comment.
x-request-id is generated and added to the response inside this interceptor. Interceptors don't run for requests rejected by guards (e.g., missing/invalid auth), so those error responses won't include x-request-id and logs from guards/filters may not be correlated. Consider moving request-id generation/header to a middleware/Fastify hook (runs before guards) or ensure the exception filter/guard sets the header when rejecting.
| version: 8.0.1 | ||
| contact: | ||
| name: Álvaro Alves | ||
| name: Ãlvaro Alves |
There was a problem hiding this comment.
The contact name appears to have been re-encoded (Ã�lvaro Alves), which will render incorrectly in Backstage/OpenAPI consumers. Please restore the correct UTF-8 string (Álvaro Alves) and ensure the file is saved/committed with UTF-8 encoding.
| name: �lvaro Alves | |
| name: Álvaro Alves |
| version: 8.0.1 | ||
| contact: | ||
| name: Álvaro Alves | ||
| name: Ãlvaro Alves |
There was a problem hiding this comment.
The contact name appears to have been re-encoded (Ã�lvaro Alves), which will render incorrectly in Swagger/OpenAPI consumers. Please restore the correct UTF-8 string (Álvaro Alves) and ensure the file is saved/committed with UTF-8 encoding.
| name: �lvaro Alves | |
| name: Álvaro Alves |
PROJ-TASK_ID
Task Link
Task Description
...
Checklist
Justification for unselected checkboxes:
...
Evidence
Logs Link
