diff --git a/.claude/agent-memory/reviewer/MEMORY.md b/.claude/agent-memory/reviewer/MEMORY.md index 894345a..ef12f18 100644 --- a/.claude/agent-memory/reviewer/MEMORY.md +++ b/.claude/agent-memory/reviewer/MEMORY.md @@ -19,6 +19,7 @@ - [PE per-hạng-mục spec review (winner-truth lần 3)](project_pe_multi_ncc_perhangmuc_specreview.md) — PASS_WITH_ADJ. MUST: CreateContract isSingle→detailsSum (`:76-90`) over-count 1-winner≠cả-gói. Lesson: grep write-path field-NGUỒN (IsSelected) không chỉ field-đọc; cite-range bọc branch-decision. - [S114 PE per-hạng-mục IMPL review (flip lần 3)](project_s114_multi_ncc_perhangmuc_impl_review.md) — NEEDS-FIX. CAUGHT: DeleteQuote (`...Features.cs:371-397`) KHÔNG re-derive IsWinner → xóa quote cuối (nút Xóa FE:2897) VỠ invariant → phantom winner + HĐ giá 0. Lesson: cardinality grep-consumer PHẢI gồm Delete-handler field-nguồn; spec-enum writer KHÔNG đáng tin. SOLID: 5 SUM + non-tautology test + hash ae3788e9. - [PE negative-quote FE-only spec review (correctness-wire)](project_pe_negative_quote_correctness_review.md) — PASS_WITH_FIXES. R1/R2 no-op verified accurate (CreateContractFromEval:88-100 no-clamp · BE Sum()<=0 :206 + FE :201). MUST: sign-loss — form.thanhTien read DIRECT at POST :2232/:2875+gate :2226; toggle phải re-sign field-chung KHÔNG chỉ flip neg-flag (VndInlineEdit defer-parse-at-own-save không có ở 2 dialog). Consumer inventory COMPLETE 0-break (fmtMoney/toLocaleString neg-safe). Lesson: defer-parse widget→shared-form dialog = sign phải tới field-chung ở MỌI mutation incl toggle. +- [S118 Procurement master-access seeder review](project_s118_procurement_master_access_review.md) — PASS(cond); scope-isolation PERFECT (RoleId==Procurement, S92 held) + build 0/0. MAJOR: Reports.Read leaks contract-financials to PRO (`/reports/dashboard` unguarded `[Authorize]`-only) + `Suppliers.Update` grant does NOT enable edit (PUT role-gated Admin/CatalogManager) only Publish+Import. Lesson: menu-flag grant ≠ API capability — grep target controller authz-attrs; bare `[Authorize]` GET = open-to-all so Read-grant is FE-visibility-only. --- diff --git a/.claude/agent-memory/reviewer/project_s118_procurement_master_access_review.md b/.claude/agent-memory/reviewer/project_s118_procurement_master_access_review.md new file mode 100644 index 0000000..049fefd --- /dev/null +++ b/.claude/agent-memory/reviewer/project_s118_procurement_master_access_review.md @@ -0,0 +1,26 @@ +--- +name: project-s118-procurement-master-access-review +description: S118 review of SeedProcurementMasterAccessAsync (DbInitializer) — PRO role exception to S92 revoke; menu-flag vs role-gated authz reality +metadata: + type: project +--- + +# S118 (2026-07-14) Procurement master-access seeder review — DbInitializer.cs +85 LOC + +**Verdict: PASS (conditional) — code correct/safe/scope-isolated + build 0/0; 2 MAJOR owner-decisions before commit.** + +Change: new `SeedProcurementMasterAccessAsync` runs AFTER `RevokeTemporarilyHiddenModulesAsync` (line 2008→2014), grants role Procurement 11 keys (Suppliers R+C+U, rest Read). + +## Clean PASS items (verified) +- Ordering 2008→2014 wins revoke. Scope isolation PERFECT — every query/insert `RoleId==role.Id` (Procurement only); S92 held for all other roles. `.ToDictionary(MenuKey)` safe — `IX_Permissions_RoleId_MenuKey` UNIQUE (Mig `20260421042236`). All 11 MenuKeys exist + in `MenuKeys.All`. Upgrade-only (false→true only), CanDelete never touched. Null-guard OK. Build clean. + +## MAJOR (owner sign-off) +- **M1 Reports grant leaks contract financials to PRO:** `Reports.Read` surfaces "Báo cáo" menu → `GET /reports/dashboard` returns `TotalValueActive` + top suppliers/projects BY CONTRACT VALUE + 12-month value trend; `contracts/export` dumps contracts. `ReportsController` = `[Authorize]` ONLY (no policy) → already reachable by any authed user (pre-existing gap), but grant makes it first-class UI for PRO — contradicts S92 "hide contracts/financial from non-admin". Stated goal (suppliers) does NOT need Reports → recommend drop Reports (+maybe Dashboard). +- **M2 `Suppliers.Update` does NOT enable standard edit:** PUT/DELETE `/suppliers/{id}` = `[Authorize(Roles="Admin,CatalogManager")]` (SuppliersController :48/:57) — menu-flag irrelevant → PRO still 403 editing supplier fields. `Suppliers.Update` policy only gates Publish + bulk Import/confirm (:67-101). If "quản lý"=edit, grant insufficient. Blast radius includes mass Import. + +## MINOR +- Idempotency claim FALSE: revoke re-hides any non-admin true-flag row every startup → grant re-sets true every startup → 22 writes/boot (not "0 change 2nd run"). End-state correct. Tiny no-access window between the 2 SaveChanges at boot (negligible). +- `Suppliers.Create` menu-flag = API no-op (POST /suppliers has NO policy, open to all authed per S59) — FE button visibility only. +- Recommend 1 scope-isolation regression test (SqliteDbFixture+IdentityFixture exist): DeptManager-only role stays Master/Suppliers=false after seed. Prod-verify acceptable per seeder precedent (Hrm/Off untested) → SHOULD not MUST. + +**Lesson: menu-flag grant ≠ API capability — MANY master controllers gate GET with bare `[Authorize]` (open-to-all) + writes with `[Authorize(Roles=...)]`, NOT menu-key policy. So a "Read/Create/Update" permission-row grant is mostly FE-menu-visibility; real API delta = only the endpoints that actually carry `[Authorize(Policy="X.Y")]`. ALWAYS grep the target controllers' actual authz attributes before trusting a permission-grant's intended effect. Reports/Dashboard menu surfaces company financials via unguarded endpoint — "menu-only Read grant" can still leak sensitive aggregates.**