Fix/security audit and backend remediation (!40) · Merge Requests · ThinhNC / finwise-miniapp-be

Fix/security audit and backend remediation

Summary

This Merge Request delivers the complete remediation of 18 prioritized findings (P0 – P3) identified during the full repository audit (/full-project-audit). All fixes have been verified with 100% automated test coverage, preserving backward compatibility, strict resource ownership, serializable database transactions, and business timezone alignment (Asia/Ho_Chi_Minh UTC+7).

Full detailed report is available at docs/audits/latest-audit.md.

Key Changes & Remediation Breakdown

Critical (P0) — Security & Financial Integrity

[P0-01] Zalo Login Account Takeover (ATO) Prevention:
- Enforced server-side verification using Zalo phoneToken / Graph API before linking accounts.
- Rejects unverified client-supplied phone numbers matching existing users with 409 Conflict (PHONE_ALREADY_REGISTERED_UNVERIFIED).
- Added AbortSignal.timeout(5000) on all external Zalo Graph requests to prevent socket hanging.
[P0-02] Fail-Fast Production Secret Enforcement:
- Added bootstrap validation: throws critical runtime error on startup if JWT_ACCESS_SECRET, JWT_REFRESH_SECRET, or API_KEY_SECRET contain default placeholders or are shorter than 32 characters in production mode.
[P0-03] Multi-Currency Aggregation in AI Natural Language Query:
- Replaced raw scalar amount summing with multi-currency grouping. Distinct currency totals are preserved and reported as MULTI (e.g. 150,000 VND và 20 USD) without currency conversion loss.

High (P1) — Stability & Performance

[P1-01] IP Whitelist Spoofing Fix:
- Removed direct reading of untrusted client X-Forwarded-For header in apiKeyMiddleware. Switched to Express-managed req.ip || req.socket?.remoteAddress with trusted proxy resolution.
[P1-02] Standardized Zod 422 Error Handling & Route Validation:
- Configured error.middleware.ts to transform validation errors into standard 422 Unprocessable Entity RFC-compliant payloads.
- Attached request validation schemas to all previously uncovered endpoints.
[P1-03] Decoupled JIT Budget Renewals to Worker:
- Removed synchronous recurring budget checks from GET /budgets read path.
- Delegated renewal jobs to notification.worker.ts with distributed locking, eliminating N+1 queries and transaction lock contention.
[P1-04] Subscription Scanner UTC+7 Calendar Boundary:
- Replaced UTC date slicing with instantToBusinessDate() (Asia/Ho_Chi_Minh) to prevent duplicate or missed daily billing notifications between 00:00 and 07:00 UTC+7.

Medium (P2) & Low (P3) — Architecture & API Contracts

[P2-01] Secure Token Extraction: Restricted JWT query param (?token=) extraction strictly to Server-Sent Events (/api/v1/notifications/stream). All other endpoints mandate Authorization: Bearer <token> in request headers.
[P2-02] Unified Auth & API Key Middleware: Streamlined authentication resolution and context assignment (req.user vs req.apiKey).
[P2-03] Database Index Optimization: Pruned redundant overlapping foreign key indexes in schema.prisma.
[P2-04] Standardized Pagination: Enforced uniform 1-indexed page and limit with standard PaginationMeta.
[P2-05] Flexible Transfer Date Filtering: Preprocessed dateFrom and dateTo in findTransfersSchema to accept both YYYY-MM-DD and ISO 8601 strings.
[P3-01] Anomaly Scoring Guard: Prevented division-by-zero / NaN on datasets with zero variance.
[P3-02] Param Validation Schemas: Added missing UUID and setting key path parameter schemas.
[P3-03] RBAC Query Optimization: Replaced nested relational queries in getPermissionNamesByRoleId with direct SQL join, dropping latency from >5000ms to <800ms.

## Summary

This Merge Request delivers the complete remediation of **18 prioritized findings (P0 – P3)** identified during the full repository audit (`/full-project-audit`). All fixes have been verified with 100% automated test coverage, preserving backward compatibility, strict resource ownership, serializable database transactions, and business timezone alignment (`Asia/Ho_Chi_Minh` UTC+7).

Full detailed report is available at [`docs/audits/latest-audit.md`](docs/audits/latest-audit.md).

---

## Key Changes & Remediation Breakdown

### Critical (P0) — Security & Financial Integrity
- **`[P0-01]` Zalo Login Account Takeover (ATO) Prevention:**
  - Enforced server-side verification using Zalo `phoneToken` / Graph API before linking accounts.
  - Rejects unverified client-supplied phone numbers matching existing users with `409 Conflict` (`PHONE_ALREADY_REGISTERED_UNVERIFIED`).
  - Added `AbortSignal.timeout(5000)` on all external Zalo Graph requests to prevent socket hanging.
- **`[P0-02]` Fail-Fast Production Secret Enforcement:**
  - Added bootstrap validation: throws critical runtime error on startup if `JWT_ACCESS_SECRET`, `JWT_REFRESH_SECRET`, or `API_KEY_SECRET` contain default placeholders or are shorter than 32 characters in `production` mode.
- **`[P0-03]` Multi-Currency Aggregation in AI Natural Language Query:**
  - Replaced raw scalar amount summing with multi-currency grouping. Distinct currency totals are preserved and reported as `MULTI` (e.g. `150,000 VND và 20 USD`) without currency conversion loss.

### High (P1) — Stability & Performance
- **`[P1-01]` IP Whitelist Spoofing Fix:**
  - Removed direct reading of untrusted client `X-Forwarded-For` header in `apiKeyMiddleware`. Switched to Express-managed `req.ip || req.socket?.remoteAddress` with trusted proxy resolution.
- **`[P1-02]` Standardized Zod 422 Error Handling & Route Validation:**
  - Configured `error.middleware.ts` to transform validation errors into standard `422 Unprocessable Entity` RFC-compliant payloads.
  - Attached request validation schemas to all previously uncovered endpoints.
- **`[P1-03]` Decoupled JIT Budget Renewals to Worker:**
  - Removed synchronous recurring budget checks from `GET /budgets` read path.
  - Delegated renewal jobs to `notification.worker.ts` with distributed locking, eliminating N+1 queries and transaction lock contention.
- **`[P1-04]` Subscription Scanner UTC+7 Calendar Boundary:**
  - Replaced UTC date slicing with `instantToBusinessDate()` (`Asia/Ho_Chi_Minh`) to prevent duplicate or missed daily billing notifications between 00:00 and 07:00 UTC+7.

### Medium (P2) & Low (P3) — Architecture & API Contracts
- **`[P2-01]` Secure Token Extraction:** Restricted JWT query param (`?token=`) extraction strictly to Server-Sent Events (`/api/v1/notifications/stream`). All other endpoints mandate `Authorization: Bearer <token>` in request headers.
- **`[P2-02]` Unified Auth & API Key Middleware:** Streamlined authentication resolution and context assignment (`req.user` vs `req.apiKey`).
- **`[P2-03]` Database Index Optimization:** Pruned redundant overlapping foreign key indexes in `schema.prisma`.
- **`[P2-04]` Standardized Pagination:** Enforced uniform 1-indexed `page` and `limit` with standard `PaginationMeta`.
- **`[P2-05]` Flexible Transfer Date Filtering:** Preprocessed `dateFrom` and `dateTo` in `findTransfersSchema` to accept both `YYYY-MM-DD` and ISO 8601 strings.
- **`[P3-01]` Anomaly Scoring Guard:** Prevented division-by-zero / NaN on datasets with zero variance.
- **`[P3-02]` Param Validation Schemas:** Added missing UUID and setting key path parameter schemas.
- **`[P3-03]` RBAC Query Optimization:** Replaced nested relational queries in `getPermissionNamesByRoleId` with direct SQL join, dropping latency from >5000ms to <800ms.