16 KiB
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-đồngfe-user/src/types/khkk.ts:16khai đú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-278CreateContractSigningPlanCommand(Guid PeId, ...)→ JSON nhậnpeId. Validator:284RuleFor(x => x.PeId).NotEmpty(). - FE:
types/khkk.ts:193-197CreateKhkkInput { purchaseEvaluationId, ... }; gửi ởKhkkCreatePage.tsx:72-77. - ⇒
purchaseEvaluationIdKHÔNG bind vàoPeId(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:80D1 lập luận "lane FE dựng body từ CÙNG spec ⇒ giữPeIdcho 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ànhPurchaseEvaluationId(khớp convention repo + FE) rồi sửa 4 siterequest.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: idvà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, épcmd 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;itemFormmangidkhi bấm Sửa (types/khkk.ts:206id?: 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-18cũ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:39string? PeTenGoiThau(cả List:39lẫn Detail:124). - FE
types/khkk.ts:108tenGoiThau; đọ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,163gọipe.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> WinnerSupplierNamesvào DTO (append CUỐI), hoặc FE đổi sang renderwinnerCount.
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 đụngcurrentUser(handler thậm chí không injectICurrentUser,: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),/deletedbị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ớpAttachmentId + 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-966vàDelete...Handler:1048-1053chỉ kiểm "attachment thuộc đúng phiếu", KHÔNG kiểm owner/approver ⇒ ai cóKeHoachKyKet.Updatecũ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:979lưurequest.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(:167File(stream, ct, fileName)) KHÔNG dính vì MVC tự mã hoá RFC 6266filename*. - 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)
:1160join p in db.Projects on e.ProjectId equals p.Idlà INNER JOIN, màProjectConfiguration.cs:24có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.