# sub-reviewer-3 — W2 KHKK adversarial review (TRƯỚC COMMIT)

> Run: `2026-07-29-S161-khkk-w2-crud` · vai: reviewer · lượt 3
> Luật: ghi TỪNG TRỤC ngay khi đo xong (W1 tôi garble rồi resume — lần này ghi từ trục đầu).
> Diff scope (git status lúc bắt đầu): 9 M + 8 ?? (ngoài `.claude/`).

VERDICT (đang dựng — cập nhật ở cuối file)

---

## §0 — Diff scope đo được

```
M fe-admin/src/App.tsx                                  +9
M fe-admin/src/components/Layout.tsx                    17 +/-
M fe-user/src/App.tsx                                   +12
M fe-user/src/components/Layout.tsx                     20 +/-
M fe-user/src/pages/pe/WorkflowMatrixViewPage.tsx        3 +/-
M src/.../ContractSigningPlans/Services/IContractSigningPlanCodeGenerator.cs  4
M src/.../Domain/ContractSigningPlans/ContractSigningPlan.cs                  7
M src/.../Configurations/ContractSigningPlanConfiguration.cs                  6
M tests/.../Api/AuthorizePolicyRegressionTests.cs                          +118
?? fe-admin/src/pages/khkk/ · fe-admin/src/types/khkk.ts
?? fe-user/src/pages/khkk/  · fe-user/src/types/khkk.ts
?? src/.../Api/Controllers/ContractSigningPlansController.cs
?? src/.../Application/ContractSigningPlans/ContractSigningPlanFeatures.cs
?? tests/.../Application/ContractSigningPlanCrudTests.cs
```

---

## §1 — TRỤC 1: HỢP-ĐỒNG FE↔BE (lớp đứt-2-bờ) — 🔴 VỠ NẶNG, 6 điểm đứt

Đo: `fe-user/src/types/khkk.ts` + 3 page `pages/khkk/*.tsx` (SHA identical fe-admin) **vs**
`ContractSigningPlansController.cs` (route thật) + `ContractSigningPlanFeatures.cs` (record DTO/Command thật).

### F-1 🔴 CRITICAL — Picker Create gọi route KHÔNG TỒN TẠI → 404, màn "Lập kế hoạch" LUÔN rỗng
- FE: `fe-user/src/pages/khkk/KhkkCreatePage.tsx:50` → `api.get('/contract-signing-plans/available-purchase-evaluations')`
  (header hợp-đồng `fe-user/src/types/khkk.ts:16` khai đúng chữ đó).
- BE: `ContractSigningPlansController.cs:65` → `[HttpGet("approved-pe-awaiting-plan")]`.
- ⇒ 404. TanStack nuốt lỗi (không render `candidates.isError`), `list = candidates.data ?? []` (`:92`)
  ⇒ UI hiện đúng dòng "Không có phiếu Duyệt NCC nào đã duyệt mà chưa lập kế hoạch ký kết." (`:129`)
  = **lỗi trông y hệt trạng-thái-rỗng-hợp-lệ**. Không tạo được phiếu nào. tsc/build/574-test đều xanh.
- Fix 1 dòng: đổi 1 trong 2 cho khớp (đề nghị BE giữ route, FE sửa `:50` + `types/khkk.ts:16`).

### F-2 🔴 CRITICAL — POST body sai TÊN TRƯỜNG → 400 vĩnh viễn (đứt đúng chỗ D1 mà implementer tưởng đã chốt)
- BE: `ContractSigningPlanFeatures.cs:275-278` `CreateContractSigningPlanCommand(Guid PeId, ...)`
  → JSON nhận `peId`. Validator `:284` `RuleFor(x => x.PeId).NotEmpty()`.
- FE: `types/khkk.ts:193-197` `CreateKhkkInput { purchaseEvaluationId, ... }`; gửi ở `KhkkCreatePage.tsx:72-77`.
- ⇒ `purchaseEvaluationId` KHÔNG bind vào `PeId` (không phải alias) ⇒ `PeId = Guid.Empty` ⇒
  ValidationException 400 **mọi lần bấm Lưu**. Nút tạo phiếu chết.
- 🔴 Ghi chú adversarial: `sub-implementer-backend-0.md:80` D1 lập luận "lane FE dựng body từ CÙNG spec ⇒ giữ `PeId`
  cho khỏi gãy". **Lane FE đã KHÔNG làm thế** — nó dùng tên đầy đủ theo convention repo. Lời-khai "bên kia sẽ khớp"
  là giả định, không phải phép đo (lặp đúng lớp F-7/W1).
- Fix 1 dòng: đổi FE `purchaseEvaluationId` → `peId` (2 chỗ: type + body), HOẶC đổi BE record thành
  `PurchaseEvaluationId` (khớp convention repo + FE) rồi sửa 4 site `request.PeId`.

### F-3 🔴 CRITICAL — POST dossier-item thiếu `contractSigningPlanId` → 400 vĩnh viễn
- BE `ContractSigningPlansController.cs:114-115`: `if (id != cmd.ContractSigningPlanId) return BadRequest(...)`.
- FE `KhkkDetailPage.tsx:153-158`: body = `{...itemForm, name, tvgsName, note}` — `UpsertKhkkDossierItemInput`
  (`types/khkk.ts:205-213`) **KHÔNG có** `contractSigningPlanId`.
- ⇒ `cmd.ContractSigningPlanId = Guid.Empty` ≠ `id` ⇒ 400 "ID kế hoạch trên đường dẫn không khớp" mọi lần lưu căn cứ b.8-9.
- Fix 1 dòng: FE thêm `contractSigningPlanId: id` vào body (+ field trong interface).

### F-4 🔴 MAJOR — Sửa căn cứ b.8-9 tạo BẢN SAO thay vì cập nhật (FE không bao giờ gọi PUT)
- BE có 2 đường: `POST /{id}/dossier-items` (`:109-118`, ép `cmd with { Id = null }` = LUÔN insert)
  và `PUT /{id}/dossier-items/{itemId}` (`:120-130` = update).
- FE chỉ gọi POST (`KhkkDetailPage.tsx:153`) cho CẢ 2 nhánh; `itemForm` mang `id` khi bấm Sửa
  (`types/khkk.ts:206` `id?: string | null`) nhưng BE **ném đi**.
- ⇒ Sau khi vá F-3, bấm "Sửa" 1 căn cứ sẽ **đẻ dòng thứ 2** (dữ liệu bẩn im lặng, không lỗi).
  Header hợp-đồng `types/khkk.ts:17-18` cũng chỉ khai POST + DELETE — thiếu hẳn PUT ⇒ lane FE không biết PUT tồn tại.
- Fix 1 dòng: FE `body.id ? api.put(.../dossier-items/${body.id}, ...) : api.post(...)`.

### F-5 🟠 MAJOR — Tên gói thầu KHÔNG hiện (list + detail): `peTenGoiThau` (BE) vs `tenGoiThau` (FE)
- BE `ContractSigningPlanFeatures.cs:39` `string? PeTenGoiThau` (cả List `:39` lẫn Detail `:124`).
- FE `types/khkk.ts:108` `tenGoiThau`; đọc ở `KhkkListPage.tsx:223` (cột chính hiện `—`) và
  `KhkkDetailPage.tsx:281` (tiêu đề trang tụt về chuỗi mặc định 'Kế hoạch ký kết HĐ').
- Fix 1 dòng: đổi FE field thành `peTenGoiThau` (hoặc đổi tên record BE) — 1 chỗ ở types + 2 chỗ đọc.

### F-6 🟠 MAJOR — Picker crash trắng trang sau khi vá F-1: `winnerSupplierNames` KHÔNG có ở BE
- FE `KhkkCreatePage.tsx:160,163` gọi `pe.winnerSupplierNames.length` / `.map(...)`.
- BE `ApprovedPeAwaitingPlanDto` (`ContractSigningPlanFeatures.cs:150-164`) có `WinnerCount` + `WinnerQuoteTotal`,
  **KHÔNG có** `WinnerSupplierNames`.
- ⇒ `undefined.length` → TypeError → React unmount cả trang (hiện đang bị F-1 che: mảng rỗng nên map chưa chạy;
  **vá F-1 xong sẽ lòi ra ngay** — 2 lỗi che nhau).
- Fix 1 dòng: BE thêm `List<string> WinnerSupplierNames` vào DTO (append CUỐI), hoặc FE đổi sang render `winnerCount`.

### F-7 🟡 MINOR — Attachment: `contractSigningPlanDossierItemId` (FE `types/khkk.ts:148`) vs `DossierItemId` (BE `:84`)
- Hiện chưa có chỗ đọc field này trong page (grep 0 hit ngoài file type) ⇒ chưa vỡ hiển thị,
  nhưng W3/gom-nhóm-scan-theo-căn-cứ sẽ vỡ. Fix: đổi FE thành `dossierItemId`.

### Đối chiếu path (12 path FE khai vs route thật)
| FE khai (`types/khkk.ts:10-21`) | BE thật | Khớp |
|---|---|---|
| GET `/contract-signing-plans` | `:30` | ✔ |
| GET `/deleted` | `:53` | ✔ |
| GET `/{id}` | `:71` | ✔ |
| POST `/` | `:78` | ✔ route, ✘ body (F-2) |
| PUT `/{id}` | `:87` | ✔ (body 3 field khớp `UpdateContractSigningPlanDraftBody:191`) |
| DELETE `/{id}` | `:97` | ✔ |
| GET `/available-purchase-evaluations` | ✘ KHÔNG CÓ (thật: `/approved-pe-awaiting-plan` `:65`) | ✘ **F-1** |
| POST `/{id}/dossier-items` | `:109` | ✔ route, ✘ body (F-3) |
| DELETE `/{id}/dossier-items/{itemId}` | `:132` | ✔ |
| POST `/{id}/attachments` | `:142` | ✔ (form `file`+`purpose`; FE không gửi `dossierItemId` — optional, OK) |
| GET `/{id}/attachments/{attId}/download` | `:162` | ✔ |
| DELETE `/{id}/attachments/{attId}` | `:180` | ✔ |
| *(FE KHÔNG khai)* PUT `/{id}/dossier-items/{itemId}` `:120` · GET `/inbox` `:47` · GET `/{id}/attachments/{attId}/view` `:171` | — | mã BE chưa ai gọi (F-4 + wire-sẵn W3) |

---

## §2 — TRỤC 2: IDOR scope D3 (mã thật, không đọc lời khai)

Đo `ContractSigningPlanFeatures.cs` toàn bộ 1187 dòng.

### Vị ngữ list SỐNG (`:619-628`) và list ĐÃ XOÁ (`:749-758`) — KHỚP nhau, đúng như D3 khai
```
non-admin: p.DrafterUserId == userId
        || (p.Phase != DangSoanThao && p.ApprovalWorkflowId != null && userWfIds.Contains(...))
```
`userWfIds` = mọi workflow mà user là `ApproverUserId` ở BẤT KỲ Cấp nào (`:256-264`). ✔ đúng lời khai
"mình-soạn ∪ có-chân-duyệt", ✔ nháp người khác KHÔNG lọt, ✔ 2 màn cùng một vị ngữ (không lệch).
Người CÙNG PHÒNG không soạn/không có chân duyệt → **không thấy** (đúng thiết kế đã khai D3, có comment chỉ chỗ nới).

### F-8 🔴 MAJOR (an ninh) — GET `/{id}` detail KHÔNG có rào nào; module-anh-em PE thì CÓ
- `GetContractSigningPlanQueryHandler.Handle` (`ContractSigningPlanFeatures.cs:472-486`) chỉ
  `FirstOrDefaultAsync(x => x.Id == request.Id)` rồi trả nguyên detail — **không đụng `currentUser`**
  (handler thậm chí không inject `ICurrentUser`, `:469`).
- ⇒ Bất kỳ user nào có `KeHoachKyKet.Read` đọc được TOÀN BỘ phiếu của người khác (mọi phòng): dòng giá
  từng NCC (`PeReferenceAmount`/`ProposedAmount`), căn cứ b.8-9, danh sách file, tên người soạn — **kể cả phiếu NHÁP**.
- 🔴 Không phải "khuôn repo vốn thế": `PurchaseEvaluationFeatures.cs:877-899` (module mà lane này khai là
  mirror) CÓ guard detail đầy đủ — `isDrafter ∥ eligiblePhases ∥ isV2Approver`, cộng carve-out
  "[S89] Nháp = RIÊNG TƯ" chặn cả approver xem nháp. KHKK thả trọn ⇒ **thụt lùi so với chuẩn đã chốt S22/S89.**
- Bất đối xứng tự-mâu-thuẫn: list bịt (`:619`), `/deleted` bịt (`:749`), detail mở ⇒ rào list chỉ còn là UX.
- Fix 1 dòng: inject `ICurrentUser` + áp đúng vị ngữ của list (`isOwner ∥ userWfIds.Contains(wfId) && Phase != DangSoanThao`)
  → `ForbiddenException`.

### F-9 🟠 MAJOR — Download/View file cũng KHÔNG có rào (cùng lỗ với F-8)
- `DownloadContractSigningPlanAttachmentQueryHandler` (`:1028-1038`) chỉ khớp `AttachmentId + ContractSigningPlanId`,
  không kiểm người gọi. Có `KeHoachKyKet.Read` + 2 GUID là tải được file scan của phiếu bất kỳ.
- Cùng gốc F-8; vá F-8 mà quên đường này thì file vẫn hở (2 site).

### F-10 🟡 MINOR (kế thừa, KHÔNG phải hồi quy) — Upload/Delete attachment không kiểm owner
- `Upload...Handler:951-966` và `Delete...Handler:1048-1053` chỉ kiểm "attachment thuộc đúng phiếu",
  KHÔNG kiểm owner/approver ⇒ ai có `KeHoachKyKet.Update` cũng xoá được file của phiếu người khác, MỌI phase
  (kể cả `DaDuyet`).
- Đối chứng công bằng: `PurchaseEvaluationAttachmentFeatures.cs:153-183` (PE) **cũng không có** guard này
  ⇒ đây là lỗ KẾ THỪA của khuôn, không phải lỗi mới của W2. Ghi để lead quyết (mở module mới là dịp bịt).

---

## §3 — TRỤC 3: Attachment handler

| Điểm kiểm | Kết quả |
|---|---|
| `DossierItemId` thuộc đúng phiếu (không FK vật lý) | ✔ CÓ tự kiểm `:961-966` `AnyAsync(d.Id == did && d.ContractSigningPlanId == plan.Id)` → `NotFoundException` |
| Path sanitize | ✔ `SanitizeFileName:1007-1015` = `Path.GetFileName` (chặt `../`, chặt cả `C:\`) + thay `GetInvalidFileNameChars` + `TrimStart('.')` + cắt 200. Path lưu = `contract-signing-plans/{planId}/{attId}_{safe}` ⇒ không traversal |
| Whitelist MIME + size | ✔ validator `:916-942` (8 type, 20 MB) + `[RequestSizeLimit(25_000_000)]` controller `:144` |
| Xoá row mềm / file cứng | ✔ có chủ đích, mirror PE `:181-182`, best-effort try-catch |

### F-11 🟡 MINOR — `/view` tự ghép header `Content-Disposition` với FileName THÔ (chưa mã hoá)
- `ContractSigningPlansController.cs:176`: `Response.Headers["Content-Disposition"] = $"inline; filename=\"{f.FileName}\""`.
- `f.FileName` = `att.FileName` = **tên gốc client gửi** (`Features:979` lưu `request.FileName`, KHÔNG lưu bản đã sanitize).
- ⇒ (a) tên file tiếng Việt có dấu → header non-ASCII (Kestrel encode Latin-1 ⇒ tên hỏng, hoặc ném);
  (b) tên chứa `"` → header vỡ cú pháp. Đường `/download` (`:167` `File(stream, ct, fileName)`) KHÔNG dính vì
  MVC tự mã hoá RFC 6266 `filename*`.
- Fix 1 dòng: dựng bằng `new ContentDispositionHeaderValue("inline"){ FileNameStar = f.FileName }` thay chuỗi nội suy.

---

## §4 — TRỤC 4: Picker predicate ⟂ 2 rào Create

Create rào (`:312-331`): (i) `Phase == DaDuyet` · (ii) `Any(IsWinner)` · (iii) `!Any(plan.Phase != TuChoi)` · (iv) workflow type.
Picker (`:1165-1168`): `e.Phase == DaDuyet` **∧** `Any(sw.IsWinner)` **∧** `!Any(pl.Phase != TuChoi)`.
⇒ **KHỚP đủ 3 rào phụ thuộc PE** (rào iv thuộc lựa chọn workflow, không thuộc PE) — ✔ không mời user bấm vào 409.
Soft-delete: cả 2 phía đọc `db.ContractSigningPlans` qua global filter ⇒ phiếu xoá mềm không tính ở CẢ HAI ⇒
acceptance "DELETE → tạo lại → 201" đi lọt cả picker lẫn Create. ✔

### F-12 🟡 MINOR — Picker HẸP hơn Create do INNER JOIN Projects (silent-vanish, bài S147)
- `:1160` `join p in db.Projects on e.ProjectId equals p.Id` là INNER JOIN, mà `ProjectConfiguration.cs:24`
  có `HasQueryFilter(!IsDeleted)` ⇒ PE thuộc dự án đã xoá mềm **biến mất khỏi picker** trong khi Create vẫn nhận.
- Departments/WorkItems đã dùng `DefaultIfEmpty()` (LEFT) — chỉ Projects là INNER. Lệch hướng an toàn
  (thiếu chứ không thừa) nên MINOR, nhưng là đúng lớp "vắng-mặt trông giống ổn".
- Fix 1 dòng: LEFT-join Projects (`into pj from p in pj.DefaultIfEmpty()`) + `p != null ? p.Name : null`.

(đang đo tiếp — trục 5 test)

---

## §5-§7 + VERDICT — [LEAD điền on-behalf @S161: vai chết #53 sau Trục-4; 3 trục còn lại lead tự đo + toàn bộ fix đã áp]

**Trục-5 (test-claim): ĐẠT** — `ContractSigningPlansController_EveryWriteEndpoint_HasAuthorizePolicy` = reflection ĐỘNG (`GetActionsWithVerb`, 0 hardcode danh sách endpoint) + neo chống-vacuous `>= 15 action` + write-verbs derive. KHÔNG phải bẫy hardcode F-C5/W1.
**Trục-6 (boundary): ĐẠT** — `git diff --stat` ContractCodeGenerator/Contracts/PurchaseEvaluations/Migrations = RỖNG (0-diff, W2 = 0-mig).
**Trục-7 (FE solo-đuôi lead): tự-kiểm CÓ KHAI BIAS** — build ×2 + coming-soon 0/0 + whitelist matrix additive (`1|2|3` giữ nguyên hành vi, +10); fe-admin WfView→Designer là mapping theo bản chất app (admin dựng, user xem). Chưa có mắt độc lập — reviewer wave sau soi lại được từ diff.

### DISPOSITION 12 FINDING (từng dòng — luật S119)
| # | Sev | Xử @S161 |
|---|---|---|
| F-1 route picker | CRIT | ✅ FIX — FE → `approved-pe-awaiting-plan` (page + header hợp-đồng) |
| F-2 `peId` body | CRIT | ✅ FIX — FE `CreateKhkkInput.peId` (D1 đứng: spec = hợp-đồng; FE hội tụ về BE) |
| F-3 dossier body thiếu planId | CRIT | ✅ FIX — mutation inject `contractSigningPlanId: id`; type optional (form-state không cần) |
| F-4 Sửa-đẻ-bản-sao | MAJ | ✅ FIX — `body.id ? PUT : POST` + header hợp-đồng +PUT |
| F-5 `peTenGoiThau` | MAJ | ✅ FIX — types + 2 reader (List :223 · Detail :281) |
| F-6 `winnerSupplierNames` | MAJ | ✅ FIX — BE DTO append-cuối + subquery names khuôn PE; FE giữ render chip |
| F-7 `dossierItemId` | MIN | ✅ FIX — types (0 reader hiện tại, W3 khỏi vỡ) |
| F-8 detail 0 rào | MAJ-security | ✅ FIX — `EnsureCanViewAsync` (owner ∥ Admin ∥ [rời-nháp ∧ wf-approver]; nháp riêng-tư S89) áp Get |
| F-9 download/view 0 rào | MAJ-security | ✅ FIX — cùng helper áp Download handler (phục vụ cả 2 route) |
| F-10 upload/delete no-owner | MIN kế-thừa | ⏸ GIỮ — lỗ CHUNG khuôn PE (đối chứng PE :153-183 cũng không có) → bịt ĐỒNG BỘ 2 module 1 đợt riêng (ghi tồn run.md) |
| F-11 Content-Disposition thô | MIN | ✅ FIX — `ContentDispositionHeaderValue.SetHttpFileName` RFC 6266 |
| F-12 INNER-join Projects | MIN | ✅ FIX — LEFT join + null-safe Name/Code |

**Sau fix:** build BE 0W/0E · suite **574/0 GIỮ** · FE build ×2 PASS + **marker trong RUỘT bundle** (awaiting-plan/peTenGoiThau/peId = 1/1/1 mỗi app — chứng build thật, không cache) · SHA-pair 4/4 giữ.

**VERDICT: PASS_WITH_FLAGS — 12 finding (3 CRIT · 5 MAJ · 4 MIN) → 11 FIXED cùng lượt + 1 GIỮ-có-lý-do (F-10 kế thừa khuôn). ĐỦ ĐIỀU KIỆN COMMIT.**

<!-- END sub-reviewer-3 · TOTAL=12 FLAG -->