275 lines
13 KiB
Markdown
275 lines
13 KiB
Markdown
# CTO Audit Report — GoodGo POS System
|
|
|
|
> **Date**: 2026-03-20
|
|
> **Auditor**: CTO (Agent cbfd1a9f)
|
|
> **Scope**: Architecture decisions, service boundaries, tech debt, cross-service dependencies
|
|
> **Branch**: master (d0211e5)
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
Platform đang trong giai đoạn late-MVP với 15/24 services production-ready và POS flow đã hoạt động end-to-end (xác nhận qua deployment report 2026-03-13). Wave 1-3 fixes (202 files) đã giải quyết phần lớn P0 security và runtime bugs. **Tuy nhiên, 5 vấn đề critical mới được phát hiện** cần fix trước khi deploy staging: debug endpoints không có auth, SQL injection, BFF CORS wildcard, 4 mkt-* services chưa vào Docker Compose, và cross-service messaging chưa thực sự triển khai.
|
|
|
|
---
|
|
|
|
## Critical Issues
|
|
|
|
### C1 — Debug Endpoints `[AllowAnonymous]` trong Production Code
|
|
**File**: `services/merchant-service-net/src/MerchantService.API/Controllers/StaffController.cs:249-390`
|
|
**Severity**: BLOCKER — bất kỳ ai cũng có thể gọi vào production
|
|
|
|
5 endpoints không có authentication:
|
|
- `POST /api/v1/staff/debug/seed` — tạo staff data tùy ý (line 249)
|
|
- `POST /api/v1/staff/debug/update-userid` — ghi đè userId của bất kỳ staff nào qua reflection (line 340)
|
|
- `POST /api/v1/staff/debug/update-merchant` — ghi đè merchantId của bất kỳ merchant nào (line 360)
|
|
|
|
Đây là privilege escalation / data tampering vulnerability cực kỳ nghiêm trọng. Các endpoint này phải được xóa hoàn toàn khỏi production code hoặc chỉ chạy trong `Development` environment.
|
|
|
|
**Fix**: Xóa ngay các method này hoặc wrap với `if (env.IsDevelopment())` và thêm `[Authorize(Roles = "SuperAdmin")]`.
|
|
|
|
---
|
|
|
|
### C2 — SQL Injection trong StaffController
|
|
**File**: `services/merchant-service-net/src/MerchantService.API/Controllers/StaffController.cs:307,367`
|
|
**Severity**: HIGH — tham số từ query string được nối trực tiếp vào SQL
|
|
|
|
```csharp
|
|
// Line 307 — merchant.Id là Guid (safe), nhưng pattern nguy hiểm
|
|
cmd.CommandText = $"SELECT id FROM shops WHERE merchant_id = '{merchant.Id}' LIMIT 1";
|
|
|
|
// Line 367 — userId và merchantId đến từ [FromQuery] — SQL INJECTION RISK
|
|
cmd.CommandText = $"UPDATE merchants SET user_id = '{userId}' WHERE id = '{merchantId}'";
|
|
```
|
|
|
|
Dù `Guid` tự sanitize, pattern string interpolation trong SQL là vi phạm security baseline. Bất kỳ refactor nào đổi type sang string sẽ tức thì trở thành lỗ hổng.
|
|
|
|
**Fix**: Dùng parameterized queries (`cmd.Parameters.AddWithValue()`).
|
|
|
|
---
|
|
|
|
### C3 — BFF CORS Wildcard `AllowAnyOrigin()`
|
|
**File**: `apps/web-client-tpos-net/src/WebClientTpos.Server/Program.cs:74-78`
|
|
**Severity**: HIGH — cho phép bất kỳ domain nào gọi BFF API
|
|
|
|
```csharp
|
|
policy.AllowAnyOrigin()
|
|
.AllowAnyMethod()
|
|
.AllowAnyHeader();
|
|
```
|
|
|
|
Đây là điểm vào duy nhất của frontend. Nếu BFF có endpoint nhạy cảm, bất kỳ website nào cũng có thể khai thác CSRF.
|
|
|
|
**Fix**: Thay bằng whitelist cụ thể theo environment (`localhost:3001`, `goodgo.vn`, `admin.goodgo.vn`).
|
|
|
|
---
|
|
|
|
### C4 — 4 mkt-* Services Không Có Trong Docker Compose (Port 5000 Conflict)
|
|
**File**: `deployments/local/docker-compose.yml`
|
|
**Severity**: HIGH — 4 services không thể chạy local hoặc staging
|
|
|
|
Services `mkt-facebook`, `mkt-whatsapp`, `mkt-x`, `mkt-zalo` đều thiếu trong `docker-compose.yml`. Lý do đã được ghi nhận trong ROADMAP nhưng chưa được giải quyết: tất cả 4 services được cấu hình với port 5000 (xung đột). Thực tế các services này cũng thiếu Traefik routes.
|
|
|
|
**Fix**: Gán ports 5021-5024, thêm vào docker-compose và Traefik. DevOps issue đang ở `TODO` trong ROADMAP Week 9-10.
|
|
|
|
---
|
|
|
|
### C5 — Cross-Service Messaging Chưa Được Triển Khai
|
|
**Severity**: HIGH — tất cả `IntegrationEvents/EventHandlers/` đều rỗng
|
|
|
|
Audit thư mục `IntegrationEvents/EventHandlers` trên 10 services: **không có file `.cs` nào**. Tất cả là thư mục rỗng. Đây có nghĩa là RabbitMQ được khai báo trong docker-compose nhưng không có service nào thực sự publish/consume messages qua event bus.
|
|
|
|
Điều này ảnh hưởng trực tiếp đến:
|
|
- Order → Kitchen deduction (đang dùng HTTP + Polly theo ROADMAP, nhưng đây là tight coupling)
|
|
- Promotion voucher redemption events (no consumer)
|
|
- Membership level changes (no consumer)
|
|
|
|
Hiện tại hệ thống hoạt động vì business flows quan trọng dùng HTTP trực tiếp. Nhưng đây là tech debt sẽ gây vấn đề khi scale.
|
|
|
|
**Fix**: Implement IEventBus abstraction với RabbitMQ backend, bắt đầu từ OrderCreatedIntegrationEvent → inventory deduction.
|
|
|
|
---
|
|
|
|
## Warnings
|
|
|
|
### W1 — TenantMiddleware Dùng String Interpolation trong SQL SET
|
|
**Files**: `TenantMiddleware.cs` trong 5 services (order, inventory, wallet, catalog, fnb-engine)
|
|
|
|
```csharp
|
|
cmd.CommandText = $"SET LOCAL app.current_shop_id = '{shopId.ToString("D")}'";
|
|
```
|
|
|
|
Guid.ToString("D") an toàn trong thực tế vì Guid không chứa ký tự nguy hiểm. Nhưng cần chuẩn hóa sang `NpgsqlCommand.AddParameter()` để tuân thủ secure coding standards.
|
|
|
|
---
|
|
|
|
### W2 — Test Coverage Cực Thấp (~15%)
|
|
Chỉ 1 trong 24 services (IAM) có CI pipeline chạy tests. Thống kê unit test files:
|
|
- IAM: 32 test files ✓
|
|
- FnB Engine: 16 test files ✓ (96 tests theo ROADMAP)
|
|
- Order: 5 test files
|
|
- Wallet: 8 test files
|
|
- Merchant: 3 test files
|
|
- **22 services còn lại**: không có CI test pipeline
|
|
|
|
Target roadmap là 50% coverage trước Phase 3. Với tốc độ hiện tại sẽ không đạt được.
|
|
|
|
---
|
|
|
|
### W3 — booking-service Thiếu K8s Staging Manifest
|
|
**File**: `deployments/staging/kubernetes/` — không có `booking-service.yaml`
|
|
|
|
Booking service có trong docker-compose, có Traefik route (partial — thiếu `/api/v1/schedules`), có K8s production manifest, nhưng **thiếu staging**. Mọi staging deployment sẽ không include booking/spa flow.
|
|
|
|
---
|
|
|
|
### W4 — 13 Traefik Routes Bị Thiếu
|
|
Services có code nhưng không có Traefik route trong `infra/traefik/dynamic/routes.yml`:
|
|
- `chat-service` (SignalR hub cũng thiếu)
|
|
- `social-service`
|
|
- `mining-service`
|
|
- `mission-service`
|
|
- `promotion-service`
|
|
- `ads-manager-service`
|
|
- `ads-serving-service`
|
|
- `ads-billing-service`
|
|
- `ads-tracking-service`
|
|
- `ads-analytics-service`
|
|
- `mkt-facebook-service`
|
|
- `mkt-whatsapp-service`
|
|
- `mkt-zalo-service`
|
|
|
|
Hiện tại các service này chỉ accessible trực tiếp qua port (bypassing Traefik security middleware).
|
|
|
|
---
|
|
|
|
### W5 — Fire-and-Forget Task trong mkt-whatsapp WebhooksController
|
|
**File**: `services/mkt-whatsapp-service-net/src/WhatsAppService.API/Controllers/WebhooksController.cs:88`
|
|
|
|
```csharp
|
|
_ = Task.Run(async () => await ProcessWebhookAsync(payload));
|
|
```
|
|
|
|
Đây là unobserved task — lỗi trong `ProcessWebhookAsync` sẽ bị nuốt im lặng. Dùng hosted service hoặc background job queue thay thế.
|
|
|
|
---
|
|
|
|
### W6 — Docker Images Dùng `:latest` Tag
|
|
**File**: `deployments/local/docker-compose.yml`
|
|
|
|
Tất cả service images đang dùng `goodgo/*:latest`. Vi phạm 12-factor app (reproducibility). Production K8s manifest nên dùng immutable commit SHA tags.
|
|
|
|
---
|
|
|
|
### W7 — Promotion & Mission Services Chưa Đủ Handlers
|
|
Mặc dù Wave 2 đã add handlers theo ROADMAP:
|
|
- `promotion-service`: vẫn thiếu `ExchangeVoucherCommand`, `PurchaseVoucherCommand` handlers theo CTO_FIX_TRACKER
|
|
- `inventory-service`: chỉ có `DeductInventoryCommand` — thiếu `AddStock`, `AdjustInventory`, `GetInventoryReport`
|
|
- `mission-service`: `GetUserMissionProgressQuery` theo tracker chưa rõ trạng thái
|
|
|
|
ROADMAP Phase 3 Week 9-10 đặt priority P0 cho các items này — cần xác nhận lại trạng thái thực tế.
|
|
|
|
---
|
|
|
|
### W8 — Test Credentials Hardcoded trong ROADMAP.md
|
|
**File**: `ROADMAP.md` Section IX
|
|
|
|
```
|
|
hongochai10@icloud.com | Velik@2026 | Admin/Owner
|
|
tranvanb@goodgo.vn | Staff@2026 | Staff/Cashier
|
|
```
|
|
|
|
ROADMAP.md là file check vào git. Credentials không nên ở đây ngay cả cho dev/staging. Dùng `.env.example` hoặc secret manager.
|
|
|
|
---
|
|
|
|
## Improvements
|
|
|
|
### I1 — Implement IEventBus / RabbitMQ Integration
|
|
Hiện tại RabbitMQ được provisioned trong docker-compose nhưng không có service nào dùng. Đây là cơ hội lớn để decouple cross-service communication:
|
|
- Tạo shared `IEventBus` interface trong `packages/` (hoặc NuGet package)
|
|
- RabbitMQ implementation với idempotency (outbox pattern)
|
|
- Start với `OrderItemServedDomainEvent` → inventory deduction thay vì HTTP call
|
|
|
|
---
|
|
|
|
### I2 — API Gateway Security Hardening
|
|
Traefik `middlewares.yml` đang ổn nhưng thiếu:
|
|
- **SSL redirect** bị disable (`sslRedirect: false`) — phải enable trên staging/production
|
|
- **Missing routes** cho 13 services bypass security middleware hoàn toàn
|
|
- `api-ratelimit` (100/min) chưa được apply cho các services chưa có route
|
|
|
|
---
|
|
|
|
### I3 — CI Pipeline Expansion
|
|
Chỉ có `ci-iam-service.yml`. Cần thêm ít nhất:
|
|
1. `ci-merchant-service.yml` (critical — có seed endpoints nhạy cảm)
|
|
2. `ci-order-service.yml` (critical — order flow)
|
|
3. `ci-fnb-engine.yml` (có 96 tests, nên chạy trong CI)
|
|
4. `ci-wallet-service.yml` (payment critical path)
|
|
|
|
---
|
|
|
|
### I4 — Seed Endpoint Removal / Environment Guard
|
|
Ngoài security fix cho C1, recommend tách seed logic ra:
|
|
- Script riêng trong `scripts/db/seed.sh`
|
|
- Database seeder class chỉ chạy khi `env.IsDevelopment() && args.Contains("--seed")`
|
|
- Không bao giờ expose seed logic qua HTTP endpoint
|
|
|
|
---
|
|
|
|
### I5 — Reflection-Based Field Setter Anti-Pattern
|
|
**File**: `StaffController.cs:352`
|
|
|
|
```csharp
|
|
var field = typeof(MerchantStaff).GetField("_userId", BindingFlags.NonPublic | BindingFlags.Instance);
|
|
field?.SetValue(staff, userId);
|
|
```
|
|
|
|
Đây là violation của DDD encapsulation. Cần thêm domain method `staff.AssignUserId(Guid userId)` trong aggregate root.
|
|
|
|
---
|
|
|
|
## Action Items (Prioritized)
|
|
|
|
| Priority | Item | Owner | Effort |
|
|
|----------|------|-------|--------|
|
|
| **P0** | [C1] Xóa / guard debug endpoints (`StaffController.cs:249-390`) | Backend Dev | 30 min |
|
|
| **P0** | [C2] Fix SQL injection — parameterized queries (`StaffController.cs:307,367`) | Backend Dev | 30 min |
|
|
| **P0** | [C3] Fix BFF CORS — thay `AllowAnyOrigin()` bằng whitelist | Backend Dev | 15 min |
|
|
| **P0** | [C4] Fix port conflict + add mkt-* services to Docker Compose (ports 5021-5024) | DevOps | 2h |
|
|
| **P1** | [W3] Tạo booking-service K8s staging manifest | DevOps | 1h |
|
|
| **P1** | [W4] Add 13 Traefik routes (chat, social, mining, ads-*, mkt-*, promotion, mission) | DevOps | 2h |
|
|
| **P1** | [W7] Fix promotion/mission/inventory service handlers | Backend Dev | 1-2 days |
|
|
| **P1** | [C5] Implement IEventBus skeleton + first integration event | Backend Dev | 3-5 days |
|
|
| **P2** | [W1] Fix TenantMiddleware SQL string interpolation (5 services) | Backend Dev | 1h |
|
|
| **P2** | [W5] Fix fire-and-forget in mkt-whatsapp | Backend Dev | 30 min |
|
|
| **P2** | [W6] Thay `:latest` bằng commit SHA trong CI/CD | DevOps | 1h |
|
|
| **P2** | [W8] Xóa credentials khỏi ROADMAP.md | CTO | 5 min |
|
|
| **P2** | [I3] Thêm CI pipelines cho merchant, order, fnb, wallet | DevOps | 2h |
|
|
| **P3** | [I4] Tách seed logic ra scripts | Backend Dev | 2h |
|
|
| **P3** | [I5] Add `AssignUserId()` domain method thay reflection | Backend Dev | 30 min |
|
|
| **P3** | Enable SSL redirect trong Traefik (staging/prod) | DevOps | 15 min |
|
|
|
|
---
|
|
|
|
## Architectural Assessment
|
|
|
|
### What's Working Well
|
|
- **Clean Architecture + CQRS** được tuân thủ nhất quán trong 15 production-ready services
|
|
- **MediatR pipeline** (Logging → Validation → Transaction → Handler) hoạt động đúng
|
|
- **Multi-tenancy** via EF Core global filters + PostgreSQL RLS — defense-in-depth tốt
|
|
- **IAM service** với Duende IdentityServer là foundation vững chắc
|
|
- **BFF pattern** với YARP là đúng hướng — tránh expose microservices trực tiếp
|
|
- **FnB Engine** là service chất lượng cao nhất (96 tests, full DDD, SignalR hub)
|
|
- **Observability stack** (Prometheus + Grafana + Loki) đã sẵn sàng
|
|
|
|
### What Needs Attention
|
|
- **Event-driven messaging** chưa triển khai — RabbitMQ hiện là dead weight
|
|
- **Auth boundary** giữa BFF và microservices cần review: BFF dùng `AuthForwardingHandler` để forward JWT, nhưng BFF CORS wildcard tạo ra điểm yếu
|
|
- **Service decomposition** cho ads services (5 services) có vẻ over-engineered cho MVP — ads-serving, ads-billing, ads-analytics, ads-tracking có minimal business logic và không đủ handlers
|
|
|
|
---
|
|
|
|
*Report generated by CTO Agent | Run ID: 3d2b7974 | Commit: d0211e5*
|