From 004a3945c23a0daf5dff54c078b9cd945347ac23 Mon Sep 17 00:00:00 2001 From: "s.esmaeili" Date: Mon, 21 Sep 2026 23:26:35 +0330 Subject: [PATCH] parity(docs): record user decisions D0-D12 and urgent investigation results Phase 0 gate passed. All rulings recorded in docs/parity/DECISIONS.md and applied to PROTECTED.md, PROGRESS.md and ISSUES.md. No source code changed. Urgent investigations requested by the user, all evidence-cited: - percent trace: the target's implementation is WRONG. `percent` is the customer's OWN column in the reference (customer_add.php:32), a per-customer making-charge offset used to shift catalogue filter bounds (ApiMobileController.php:391-396,76-77). The target reads the referring agent's wagePercent instead (admin-customers.service.ts:245) and never writes the field at all. It is NOT part of the price formula -- that extra percent comes from a global ConfigStatic row (ProductVariant.php:50,183). Two unrelated `percent` keys exist and must not be confused with it. - @Public audit: 15 routes across 7 controllers, grouped by risk. Group A is 5 ledger routes (4 writes) that contradict decision D1; group B is 3 order routes incl. a public status change accepting a client-supplied actorAdminUserId; group C is 8 reads, of which only the catalogue, pricing, slider and uploads are legitimately public. - Login leak: it is in the TARGET. GET /customers is public and returns every active customer with national ID numbers, debts and ID-document URLs. passwordHash is correctly excluded. Not possible in the reference. Also logged 5 customer-module defects found in the deep check the user asked for (CUST-01..05), including the fullName collapse the user reported. Skill updated per D12: Step C gains rule C0 (field-by-field matrix, never page-by-page) and Step E gains rule E0 (mandatory 4-step round-trip test). [ref: henza_server, henza_admin] Co-Authored-By: Claude Opus 5 --- docs/parity/DECISIONS.md | 362 +++++++++++++++++++++++++++++++++++++++ docs/parity/ISSUES.md | 18 +- docs/parity/PROGRESS.md | 58 +++++-- docs/parity/PROTECTED.md | 72 ++++++-- 4 files changed, 481 insertions(+), 29 deletions(-) create mode 100644 docs/parity/DECISIONS.md diff --git a/docs/parity/DECISIONS.md b/docs/parity/DECISIONS.md new file mode 100644 index 0000000..09c1273 --- /dev/null +++ b/docs/parity/DECISIONS.md @@ -0,0 +1,362 @@ +# DECISIONS — user rulings on the parity project + +Binding decisions made by the user. They override defaults and survive context +loss. Referenced from `PROTECTED.md` and `PROGRESS.md`. + +| Date | Phase | Decisions | +|---|---|---| +| 2026-09-21 | Phase 0 gate | D0–D5, D6–D8 (urgent), D9–D11, D12 (method change) | + +--- + +## D0 — First name / last name: split the column (option **الف**) + +**Ruling.** Add real `firstName` / `lastName` columns to `Customer` via an +**additive** migration. + +Requirements: + +1. Additive migration only — nothing dropped. +2. Backfill existing rows: **first token** of `fullName` → `firstName`, **the + rest** → `lastName`. +3. Rows whose `fullName` has **more than two tokens** must be written to a report + for manual review (they cannot be split safely). +4. **`fullName` is NOT removed.** It stays and is kept in sync as + `firstName + " " + lastName` so existing code keeps working. +5. `toRow()` must return the **real** `fname` / `lname`, not `fullName` / `''`. +6. Fix every consumer: `singleCustomer.vue`, `bulkCustomer.vue`, + `ReagentCustomer.vue`, and anywhere a name is displayed, searched or sorted. + **Remove the `split(' ')` fallback in `singleCustomer.vue`.** +7. Check the same defect on **`AdminUser`** (sales expert) and on + **registration / profile in `user-app` and `user-app-v2`**. +8. **Show the migration file and the command, and get approval, before running it.** + +Evidence for the defect: reference has two columns +(`henza_server/database/migrations/2020_10_01_120430_customers_table.php:18-19`); +target collapses them (`apps/api/src/modules/admin-customers/admin-customers.service.ts:231-233` +returns `fname: c.fullName, lname: ''`). + +## D1 — Admin auth: keep the target stricter + +The reference leaves all ~215 admin routes unauthenticated +(`henza_server/routes/admin.php:12,272`). The target's JWT + 144-rid enforcement +**stands**, recorded as an accepted deviation. + +**Additional requirement:** build a complete mapping matrix +**"every reference admin operation ↔ its permission key (rid) in the target"**, +so no operation is left without an equivalent permission. Explain the +**172 (reference) vs 174 (target) unique key** difference. + +## D2 — Domain scope: audit only what the reference has + +Confirmed. Do not manufacture rows for buy/sell gold, deposit, withdraw, +transfer, KYC levels, price sockets or payment callbacks — none exist in the +reference (`00-INVENTORY.md` §0). + +Target-only capabilities (e.g. the dual-currency ledger, gram/rial debt) are +listed with status **➕** and **left untouched**. Ask when unsure. + +## D3 — The 5 missing user-app screens: build them in Phase 2 + +`/report`, `/report/detail`, `/ChildOrder`, `/customerChoice`, `/complete_profile` +are built in **both** apps, each in its own design language, plus whatever backend +they need. + +Also brought to reference behaviour in Phase 2: +- route analytics (`POST /log/create/{key}`, `GET /log/update/{id}`) +- the `isLoaded` guard +- the `sts_verify_step_two` guard on `completeProfile` +- guest behaviour: **toast «ابتدا وارد شوید» + redirect to `home`** (not a redirect to `/login`) +- **v2's missing cart stock ceiling (`capQty`/`maxQty`) must match v1 and the reference** + +## D4 — Reference bugs: replicate behaviour, not breakage + +- Broken or destructive reference behaviour is **not** ported — logged only. +- **Business behaviour, calculations, rounding and messages must match the + reference exactly.** +- Every deliberate non-replication gets a matrix note: + **«عمداً تکرار نشد — ISSUE #n»**. +- The price formula is the **live** version at + `henza_server/app/Models/ProductVariant.php:183-190` — *not* the commented-out + ones above it. +- Write a **unit test** for it with sample numbers and compare against the + reference formula computed by hand. + +## D5 — Stray root folder: leave it for now + +`gold-banking-monorepo/user-app-v2/` is logged in `ISSUES.md` (DEAD-03). At the +very end, after verifying nothing references it, delete it in **its own commit**. + +--- + +## D6 — URGENT: OTP must be verified (exception to "log only", per `CLAUDE.md` §3) + +The reference **does** verify the OTP, so the target accepting any code is a +**parity gap**, not just a security issue. Fix it to match the reference: +code matching, expiry, and attempt limits. + +Reference implementation (`henza_server/app/Http/Controllers/Api/ApiMobileController.php:39-78`): + +| Step | Reference behaviour | Line | +|---|---|---| +| Send | `userAuth` validates `username`, finds an **active** customer by mobile, generates `rand(1000,9999)`, stores it in `customer.send_password`, sends it by SMS | `:39-49` | +| Verify | `userVerfiy` validates `username` + `password`; if `faTOen(password) != customer.send_password` → **HTTP 422** | `:51-57` | +| Token | issues `api_token = Str::random(60)` only if the customer doesn't already have one | `:60-62` | +| Digits | the code is normalised through `Utility::faTOen()` (Persian → English digits) on **both** send and verify | `:42,56` | + +⚠️ Note for the matrix: the reference has **no expiry and no attempt limit** — +`send_password` is a plain column that stays valid indefinitely. D6 asks for +expiry and attempt limits, which go **beyond** the reference. Recorded as a +deliberate, user-requested improvement; the parity row is the code match itself. + +## D7 — URGENT: `@Public()` routes that write to the ledger + +Show the exact list with file paths and what the reference did for each, then +protect them **after user approval**. See §"@Public audit" below. + +## D8 — URGENT: the login customer-list leak + +Identify whether it is reference or target, and what data leaks. See §"Login leak" +below. + +--- + +## D9 — `percent`: trace before acting + +Done — see `00-INVENTORY.md` and the trace summary delivered to the user. +**Key finding: the target's implementation is wrong.** `percent` is a real column +on the customer in the reference, not a derived value from the referring agent. + +## D10 — Sales expert: the 5 missing fields + +The move to `AdminUser` is **accepted**. For `ncode`, `phone`, `city_id`, +`gram_level`, `reagent_id`: find where the reference uses each for `type=1` +(list, filter, report, order, calculation). Fields that are genuinely used must +be added — **proposed shape: a `SalesExpertProfile` table related to `AdminUser`**. +Fields with no use are listed for the user to decide. + +## D11 — `singleCustomer.vue`: migrate to the diff-send pattern + +Bring it in line with `bulkCustomer.vue` and `ReagentCustomer.vue`. **Diff-send +is recorded as an accepted deviation** from the reference (which sends every +field on every save). + +--- + +## D12 — METHOD CHANGE for Phase 1 (and Phase 2) + +**1. The matrix is field-by-field, not page-by-page.** Every form field (name, +type, required, validation, message, default), every table column, every filter, +every button — and for each one, the **client field name ↔ API field name ↔ DB +column**, on **both** sides. + +**2. A round-trip test is mandatory per entity:** + +``` +create with every field populated + → open the edit form, assert every field is correctly populated + → save with no changes, assert the DB row is byte-identical + → change exactly one field, assert only that field changed +``` + +Run it for the sales expert too. + +**3. Both rules are to be added to the `reference-parity-audit` skill** +(Step C and Step E) so Phase 2 inherits them. + +--- + +# Urgent investigation results + +## `percent` — full trace (D9) + +### In the reference, `percent` is a real column on the customer + +| Fact | Evidence | +|---|---| +| Column exists on `customers` | `database/migrations/2020_10_19_153228_customer_add.php:32` — `$table->double('percent')->default(0);` | +| Mass-assignable | `app/Models/Customer.php:31` | +| Defaulted to **3** for app-registered customers | `app/Http/Controllers/Adm/AdminController.php:338` — `$input['percent'] = 3;` | +| **Required** when an admin creates a wholesale customer | `AdminController.php:354` — `'percent'=>'required'` | +| Selected into the wholesale customer list | `Adm/MajorCustomerController.php:36` | +| Shown in a report column «درصد» | `Adm/ReportController.php:137` | + +### What it actually does — it shifts the customer's making-charge filter + +`app/Http/Controllers/Api/ApiMobileController.php:391-396`: + +```php +if (sizeof($limit_percent) > 0 and auth('api-customer')->check()) { + $limit_percent[0] = $limit_percent[0] - auth('api-customer')->user()->percent; + $limit_percent[1] = $limit_percent[1] - auth('api-customer')->user()->percent; + $product->whereHas('variants', fn($q) => $q->whereBetween('percentage_pay', $limit_percent)); +} +``` + +And it shifts the **filter bounds the app is told to display**, at login +(`ApiMobileController.php:76-77`): + +```php +$customer['filterPercentPay'][] = strval(ProductVariant::min('percentage_pay') + $customer['percent']); +$customer['filterPercentPay'][] = strval(ProductVariant::max('percentage_pay') + $customer['percent']); +``` + +So `customer.percent` is a **per-customer making-charge offset**: the UI shows +`percentage_pay + percent`, and the filter subtracts it again before querying. + +### What it does NOT do + +**It is not part of the price formula.** The formula's extra percent comes from a +**global** config row, not from the customer: + +- `app/Models/ProductVariant.php:50` — `ConfigStatic::where('key','product_price_extra_percent')->first()['val'] ?? 7` +- `app/Models/ProductVariant.php:183` — `($this->percentage_pay + static::$product_price_extra_percent)` + +There is a **separate, unrelated** `ConfigStatic` key also called `percent`, used +for the cashback/wallet rule (`AdminController.php:903,994`; `Controller.php:203`) — +`value = total_weight * (percent/100)`. **Do not confuse the two.** + +A **third** unrelated `percent` is a per-order-item field in wholesale orders +(`Adm/MajorOrderController.php:220`): `w = rash_weight + rash_weight * (percent/100)`. +That one comes from the request, not from the customer. + +### The target is wrong + +`apps/api/src/modules/admin-customers/admin-customers.service.ts:245`: + +```ts +// legacy `percent` came from the referring agent's commission rate +percent: c.referredByAgent?.wagePercent ?? null, +``` + +That comment is **incorrect** — the reference reads `customer.percent`, its own +column. Consequences: + +1. **Read is wrong** — it shows the agent's commission, not the customer's offset. +2. **Write is missing** — `update()` never handles `data.percent` + (`admin-customers.service.ts:600-658`), so editing the field in the admin form + does nothing. +3. **The create default is missing** — the reference sets `percent = 3` for + app-registered customers and **requires** it for admin-created wholesale + customers. +4. **The app-side effect is missing** — no `filterPercentPay` offset anywhere in + `user-app` / `user-app-v2`. + +### Proposed reconstruction (needs user approval — it is a schema change) + +1. Add `makingChargeOffsetPercent Decimal @default(0)` to `Customer` (additive + migration). Name it clearly so it is never confused with the two other + `percent`s; map it to the legacy key `percent` in `toRow()`. +2. `toRow()` returns that column, not `referredByAgent.wagePercent`. Remove the + incorrect comment. +3. `create()`: default **3** on the app-registration path; **require** it on the + admin wholesale path (reference `AdminController.php:354`). +4. `update()`: handle `data.percent`. +5. Catalogue filter: subtract the customer's offset from the `percentage_pay` + bounds before querying, and return shifted `filterPercentPay` bounds at login + — matching `ApiMobileController.php:391-396,76-77`. +6. The referring agent's `wagePercent` stays where it belongs (the agent), and is + **not** exposed through the customer form. + +--- + +## `@Public()` audit (D7) — every unauthenticated route in the target + +15 routes across 7 controllers. Verified by reading each controller. + +### 🔴 Group A — ledger writes (the ones the user flagged) + +`apps/api/src/modules/accounting/accounting.controller.ts` — `@Public()` on the +**whole controller** (`:7`), prefix `accounting` (`:8`): + +| # | Route | Line | Effect | Reference equivalent | +|---|---|---|---|---| +| A1 | `GET accounting/customers/:customerId/balance` | `:12` | reads any customer's balance | `Adm/WalletController` — reference had auth **commented out** too (`routes/admin.php:12`) | +| A2 | `POST accounting/customers/:customerId/reconcile` | `:17` | **writes** — reconciles a balance | same: unauthenticated in reference | +| A3 | `POST accounting/repayments/gold` | `:22` | **writes** — records a gold repayment | same | +| A4 | `POST accounting/repayments/rial` | `:27` | **writes** — records a rial repayment | same | +| A5 | `POST accounting/entries/:entryId/confirm` | `:32` | **writes** — confirms a ledger entry | same | + +**In the reference these were admin routes in `routes/admin.php`, and that whole +file's `auth:api` wrapper is commented out — so they were unauthenticated there +too.** The target faithfully reproduced that. Under **D1** the target is supposed +to be stricter, so these five are the clearest violation of D1 still standing. + +**Recommendation:** A1–A5 are admin operations. Put them behind the admin JWT +guard with the matching rid (the wallet/document rids from `acl.constants.ts`). +**Awaiting your approval — this is the list you asked for.** + +### 🟠 Group B — order writes + +`apps/api/src/modules/orders/orders.controller.ts` — `@Public()` at `:7`, prefix `orders`: + +| # | Route | Line | Note | Reference | +|---|---|---|---|---| +| B1 | `POST orders` | `:12` | customer places an order | `ApiMobileController::setOrder` — **was authenticated** via `auth('api-customer')`; it reads `auth()->id()` at `:533` | +| B2 | `GET orders/:id` | `:17` | reads any order by id | same guard in reference | +| B3 | `PATCH orders/:id/status` | `:22` | **changes order status**; DTO accepts a client-supplied `actorAdminUserId` | admin operation in reference | + +**B1/B2 differ from the reference in kind:** the reference's customer endpoints +sat behind the `api-customer` guard and derived the customer from the token. +The target has **no customer session at all**, so it takes `customerId` from the +request body. That is why they are public. Fixing this properly means building +customer auth — which is exactly what **D6** starts. + +**B3 is the worst of the three:** it lets an anonymous caller move an order's +status *and* attribute the action to any admin. + +### 🟢 Group C — genuinely public reads (match the reference's public storefront) + +| # | Route | File:line | Reference | +|---|---|---|---| +| C1 | `GET customers` | `customers/customers.controller.ts:10` | **see the leak below** | +| C2 | `GET customers/:id` | `:15` | — | +| C3 | `GET customers/:id/orders` | `:20` | — | +| C4 | `GET customers/:id/ledger-entries` | `:25` | — | +| C5 | `GET catalog/products` | `catalog/catalog.controller.ts:10` | reference storefront was public (`routes/site.php`) ✅ | +| C6 | `GET pricing/latest` | `pricing/pricing.controller.ts:10` | reference `/price` was public ✅ | +| C7 | `GET home/slider` | `admin-misc/public-slider.controller.ts:21` | reference `mobile_main_slider` config, public ✅ | +| C8 | `GET uploads/:filename` | `admin-uploads/public-uploads.controller.ts:17` | reference served uploads statically ✅ | + +C5–C8 are legitimately public. **C1–C4 are not** — they expose customer records. + +--- + +## Login leak (D8) — it is in the **target**, and it is worse than "a list" + +**Where:** target only. `apps/user-app` and `apps/user-app-v2` both call +`GET /api/customers` at login and match the phone number **in the browser**. + +**What leaks:** `GET /customers` is `@Public()` +(`customers/customers.controller.ts:10`) and returns **every active, +non-archived customer** with these fields +(`customers/customers.service.ts:6-23`, `:44-50`): + +| Field | Sensitive? | +|---|---| +| `id`, `createdAt`, `updatedAt` | low | +| `fullName` | 🔴 **PII** | +| `mobile` | 🔴 **PII** | +| `nationalCode` | 🔴 **national ID number** | +| `cityId`, `referredByAgentId` | medium | +| `goldCreditLimitGrams`, `goldDebtGrams`, `fiatDebtRial` | 🔴 **financial position of every customer** | +| `verificationStep`, `isActive`, `isArchived` | medium | +| `nationalIdImageUrl`, `businessLicenseUrl`, `shopImageUrl` | 🔴 **URLs of ID-document scans** | + +**`passwordHash` is correctly excluded** — the select list is explicit and the +source comment says so (`customers.service.ts:5`). So no password material leaks. + +**Everything else does.** Any anonymous visitor can fetch the full customer book — +names, mobiles, national ID numbers, debts, and links to identity-document images +(and `GET uploads/:filename` is public too, so those links resolve). + +**In the reference this was not possible.** Login was +`POST userAuth` → SMS code → `POST userVerfiy` +(`ApiMobileController.php:39-78`): the server looked the customer up by mobile +and returned **only that one customer** after the code matched. There was no +"download everyone and match on the client" step. + +**Root cause:** the target has no customer authentication, so the client does the +lookup. **D6 (real OTP) is the fix for both problems at once** — once +`POST auth/otp/verify` returns the authenticated customer, `GET /customers` can +be taken off the public surface entirely. diff --git a/docs/parity/ISSUES.md b/docs/parity/ISSUES.md index 0f761f9..0f723b7 100644 --- a/docs/parity/ISSUES.md +++ b/docs/parity/ISSUES.md @@ -17,12 +17,28 @@ Severity: `Critical` (money loss / security / data loss) · `High` · `Medium` |---|---|---|---|---|---| | SEC-01 | Critical | REF `henza_server/routes/admin.php:12,272` | The whole admin route file is **unauthenticated** — the `Route::middleware('auth:api')->group(...)` wrapper is commented out at both ends, leaving all ~215 admin routes public (incl. `wallet/charge`, `users`, `roles`). | Verified by reading both lines. | n/a in reference (read-only). Drives TGT decision below. | | SEC-02 | Critical | TGT `apps/api/src/modules/accounting/accounting.controller.ts:7`, `orders/orders.controller.ts:7`, `customers/customers.controller.ts:5`, `catalog/catalog.controller.ts:5`, `pricing/pricing.controller.ts:5` | 15 routes carry `@Public()`, including **ledger writes** (`POST accounting/repayments/*`, `entries/:id/confirm`) and `POST /api/orders` + `PATCH /api/orders/:id/status`. The public status DTO accepts a client-supplied `actorAdminUserId`, so a caller can attribute an action to any admin. | `grep -rn "@Public()"`; source comments say "pre-existing unauthenticated endpoint". | Introduce customer-session auth for the app-facing routes; move ledger/status writes behind the admin guard. **[replicated]** from SEC-01. | -| SEC-03 | Critical | TGT `apps/user-app/src/views/user/LoginView.vue`, `apps/user-app-v2/` same | OTP is **not verified** — any 4-digit code logs the user in. Login also downloads the **entire customer list** (`GET /api/customers`) and matches the phone number client-side, exposing every customer's data to any visitor. | Both inventory agents verified independently; `customers.controller.ts:5` is `@Public()`. | Implement a real server-side OTP issue/verify pair; never ship the customer list to the client. | +| SEC-03 | Critical | TGT `apps/user-app/src/views/user/LoginView.vue`, `apps/user-app-v2/` same | OTP is **not verified** — any 4-digit code logs the user in. This is a **parity gap**: the reference does verify it (`ApiMobileController.php:51-57`, mismatch → HTTP 422). | `DECISIONS.md` D6 | **FIXING — D6** (plus expiry + attempt limits, which exceed the reference) | +| SEC-08 | Critical | TGT `apps/api/src/modules/customers/customers.service.ts:6-23,44-50` + `customers.controller.ts:10` (`@Public()`) | **Full customer book exposed to anonymous callers.** Login downloads `GET /customers` and matches the phone client-side, so the endpoint returns **every** active customer with `fullName`, `mobile`, **`nationalCode`**, `goldDebtGrams`, `fiatDebtRial`, `goldCreditLimitGrams`, and the URLs of **national-ID / business-licence / shop scans** (and `GET uploads/:filename` is public too, so those URLs resolve). `passwordHash` is correctly excluded. Not possible in the reference, where `userVerfiy` returned **one** customer after the code matched. | Read `customers.service.ts:5-23`; `DECISIONS.md` D8 | Fixed as a side-effect of D6: once OTP verify returns the authenticated customer, take `GET /customers` off the public surface. | +| SEC-09 | Critical | TGT `apps/api/src/modules/accounting/accounting.controller.ts:7` | `@Public()` on the whole accounting controller exposes **5 routes, 4 of them writes**: reconcile a balance, record a gold repayment, record a rial repayment, confirm a ledger entry. Faithful to the reference (whose admin routes were all unauthenticated) but contradicts the confirmed decision D1. | `DECISIONS.md` §"@Public audit" group A | Put behind the admin guard with the matching wallet/document rid — **awaiting user approval** | +| SEC-10 | High | TGT `apps/api/src/modules/orders/orders.controller.ts:22` | `PATCH orders/:id/status` is public **and** its DTO accepts a client-supplied `actorAdminUserId`, so an anonymous caller can move an order's status and attribute it to any admin. In the reference this was an admin operation. | `DECISIONS.md` §"@Public audit" B3 | Protect with the order-status rid | | SEC-04 | High | TGT `apps/api` (two files) | `JWT_SECRET` falls back to `'dev-only-insecure-secret'` and is absent from `.env.example`. `enableCors()` is wide open. | Reported by api inventory; see `_inv-tgt-api.md` §11. | Require the env var at boot; restrict CORS origins. | | SEC-05 | High | TGT `apps/api/prisma/seed.ts` | Seed creates a SUPER_ADMIN (`09128081392`) whose **password equals the phone number**, holding all 144 rids. Marked DEV-ONLY in comments but lives in the normal seed. | `docs/CHANGELOG-PORT-HISTORY.md` §4 documents this as intentional for dev. | Gate behind an explicit env flag so it can never run outside dev. | | SEC-06 | High | REF `henza_server` | Hardcoded secrets committed to the repo: SMS panel password, PayPing bearer token, Tahesab bearer + IP allowlist, wallet hash pepper (`"yhoenszeaf"`). | See `_inv-ref-server.md` §11.4. | Rotate all of them before any deployment that reuses these credentials. | | SEC-07 | Medium | TGT `apps/api/src/modules/admin-customers/admin-customers.service.ts` | `setRoles` stores arbitrary strings as rids with no validation against `ALL_RIDS`. | api inventory §9. | Validate against the rid catalogue. | +## Customer / person module (found during the user-requested deep check) + +> These are **parity defects being fixed**, not deferred items — see +> `DECISIONS.md` D0, D9, D10, D11. Listed here so the trail is complete. + +| ID | Severity | Where | Description | Evidence | Status | +|---|---|---|---|---|---| +| CUST-01 | Critical | TGT `apps/api/src/modules/admin-customers/admin-customers.service.ts:231-233` | **First and last name collapse.** The reference stores `fname`/`lname` as two columns; the target merged them into `fullName` and returns `fname: c.fullName, lname: ''`. Opening any edit form puts the whole name in the first-name box and blanks the surname; saving then persists that. `singleCustomer.vue` tries to compensate with `split(' ')`, which mis-splits multi-part names. | REF `database/migrations/2020_10_01_120430_customers_table.php:18-19` (two columns) vs TGT `:231-233`. Reported by the user from live use. | **FIXING — D0** | +| CUST-02 | Critical | TGT same file `:245`, `:600-658` | **`percent` read from the wrong entity and never written.** Target returns `c.referredByAgent?.wagePercent`; the reference reads the customer's **own** `percent` column. `update()` ignores `data.percent` entirely, so editing the field does nothing. The create-time default (3) and the admin-path `required` rule are both missing, as is the app-side `filterPercentPay` offset. | REF `database/migrations/2020_10_19_153228_customer_add.php:32`; `AdminController.php:338,354`; `ApiMobileController.php:391-396,76-77`. Full trace in `DECISIONS.md` §percent. | **FIXING — D9** | +| CUST-03 | High | TGT `apps/admin-panel/src/views/apps/customer/ReagentCustomer.vue:97-110` | **Sales-expert form is missing 5 reference fields:** `ncode`, `phone`, `city_id`, `gram_level`, `reagent_id`. The reference form had 15; the target has 8. Likely because `AdminUser` lacks those columns. | REF `henza_admin/src/views/apps/customer/ReagentCustomer.vue` (15 `v-decorator` fields) vs TGT form object. | **INVESTIGATING — D10** | +| CUST-04 | Low | TGT `apps/admin-panel/src/views/apps/customer/singleCustomer.vue:91` | Still spreads the whole form on save while the other two customer forms send a diff. Harmless today (4 fields, all repopulated) but the same shape that caused the earlier data loss. | read at `:80-106` | **FIXING — D11** | +| CUST-05 | Low | TGT `apps/admin-panel/src/views/apps/customer/bulkCustomer.vue` | Diff-send on edit is a **deliberate deviation** from the reference, which posts all 15 fields on every save. Recorded so it is not "fixed" back. | `DECISIONS.md` D11 | accepted deviation | + ## Money, stock & data integrity | ID | Severity | Where | Description | Evidence | Suggested fix | diff --git a/docs/parity/PROGRESS.md b/docs/parity/PROGRESS.md index 3fbc7d2..5aa3681 100644 --- a/docs/parity/PROGRESS.md +++ b/docs/parity/PROGRESS.md @@ -7,13 +7,26 @@ | Field | Value | |---|---| -| **Phase** | Phase 0 — Inventory (read-only) | -| **Branch** | `parity/phase-0` | -| **Status** | ✅ COMPLETE — **at the gate, waiting for user confirmation** | -| **Next step** | User answers Q1–Q5 below → finalise `PROTECTED.md` → branch `parity/phase-1-admin` → build `01-ADMIN-MATRIX.md` | -| **Blocked on** | user answers to Q1–Q5 | +| **Phase** | Phase 0 ✅ passed → **Phase 1 starting** | +| **Branch** | `parity/phase-0` (next: `parity/phase-1-admin`) | +| **Status** | Gate passed. Decisions D0–D12 recorded in `DECISIONS.md`. Urgent traces (percent, @Public, login leak) done and delivered. | +| **Next step** | Show the user the `percent` trace + `@Public` list (done), get approval on (a) protecting the 5 ledger routes and (b) the `percent` schema change → then build `01-ADMIN-MATRIX.md` **field-by-field** (D12) | +| **Blocked on** | user approval for: the 5 `@Public` ledger routes (D7), the `percent` column (D9), the D0 migration file (approval required before running) | | **Last updated** | 2026-09-21 | +## ⚠️ Read `DECISIONS.md` before doing anything + +All user rulings live there (D0–D12). Highlights that change how work is done: + +- **D12 — the matrix is FIELD-BY-FIELD, not page-by-page**, and a 4-step + round-trip test per entity is mandatory. Both rules were added to the + `reference-parity-audit` skill (Step C rule C0, Step E rule E0). +- **D0** — split `fullName` into `firstName`/`lastName` via an additive migration; + **show the migration and get approval before running it**. +- **D4** — never port broken behaviour, but match calculations/rounding/messages + exactly; annotate each non-replication «عمداً تکرار نشد — ISSUE #n». +- **D2** — target-only features get status ➕ and are left alone. + ## Phase checklist - [x] P0.1 — All 3 reference folders readable (`henza_server`, `henza_admin`, `henza_pwa`) @@ -21,21 +34,38 @@ - [x] P0.3 — Inventory all 7 projects → `_inv-*.md` (8111 lines total) → consolidated in `00-INVENTORY.md` - [x] P0.4 — Mapping tables, missing list, extra list (`00-INVENTORY.md` §2–4) - [x] P0.5 — Draft `PROTECTED.md` -- [ ] **P0.6 — GATE: user confirms PROTECTED list + answers Q1–Q5** ← HERE +- [x] **P0.6 — GATE passed** — `PROTECTED.md` confirmed, D0–D12 recorded +- [x] P0.7 — Urgent traces delivered: `percent`, `@Public` list, login leak +- [x] P0.8 — Skill updated with rules C0 (field-by-field) and E0 (round-trip test) +- [ ] **P1.0 — user approves: 5 ledger routes, `percent` column, D0 migration** ← HERE - [ ] Phase 1 — Admin parity (branch `parity/phase-1-admin`, `01-ADMIN-MATRIX.md`) - [ ] **GATE:** "Admin parity is complete and tested. Shall we move on to the apps (user-app & user-app-v2)?" - [ ] Phase 2 — Apps parity (`02-APPS-MATRIX.md`, both user apps) - [ ] Phase 3 — Work through `ISSUES.md` (only when user asks) -## Open questions blocking Phase 1 +## Answered — see `DECISIONS.md` for the full text -| # | Question | Proposed answer | -|---|---|---| -| Q1 | Reference leaves all ~215 admin routes unauthenticated (`henza_server/routes/admin.php:12,272`). Target enforces JWT + 144 rids. Strict parity = remove auth. | Keep target's stricter auth as an accepted deviation (`PROTECTED.md` §D) | -| Q2 | The brief's gold-banking checklist (buy/sell, deposit/withdraw, transfer, KYC levels, sockets, payment callbacks) does not exist in the reference — it's a jewellery e-commerce system. | Audit the domain the reference actually has (`00-INVENTORY.md` §0) | -| Q3 | 5 reference user-app routes missing from BOTH target apps: `/report`, `/report/detail`, `/ChildOrder`, `/customerChoice`, `/complete_profile`. ~5 screens × 2 designs + backend. | In scope for Phase 2, or deferred? | -| Q4 | 20 defects in `henza_pwa` + 18 in `henza_admin`. Replicate or log? | Log, don't replicate the broken/destructive ones (`PROTECTED.md` §F) | -| Q5 | Stray `gold-banking-monorepo/user-app-v2/` at repo root (only `package.json` + outdated `vite.config.ts`, ignored by pnpm-workspace). | Delete, or leave? | +| # | Ruling | +|---|---| +| D0 | Split `fullName` → `firstName`/`lastName`, additive migration, backfill, sync, report multi-token rows. Approval needed before running. | +| D1 | Target's stricter auth stands. Build the "reference operation ↔ rid" matrix; explain 172 vs 174. | +| D2 | Audit only what the reference has. Target-only features → ➕, untouched. | +| D3 | Build the 5 missing screens in Phase 2, both apps, plus analytics, `isLoaded`/`sts_verify_step_two` guards, guest toast+home, and v2's cart ceiling. | +| D4 | Don't port breakage; do match calculations/rounding/messages. Unit-test the price formula. | +| D5 | Leave the stray folder; delete last, own commit. | +| D6 | **OTP must verify** — parity gap, fix it (plus expiry + attempt limits, which exceed the reference). | +| D7 | `@Public` ledger routes: list delivered, awaiting approval to protect. | +| D8 | Login leak: **target**, full customer book incl. national IDs and debts. | +| D9 | `percent`: trace delivered — **the target's implementation is wrong**. | +| D10 | Sales expert: keep `AdminUser`; investigate the 5 missing fields; propose `SalesExpertProfile`. | +| D11 | `singleCustomer.vue` → diff-send. | +| D12 | **Field-by-field matrix + mandatory round-trip test.** Added to the skill. | + +## Open — awaiting user approval + +1. **Protect the 5 `@Public` ledger-write routes** (`DECISIONS.md` group A). +2. **`percent` schema change** — add `makingChargeOffsetPercent` to `Customer`. +3. **D0 migration file** — must be shown and approved before it runs. ## Key context (survives context loss) diff --git a/docs/parity/PROTECTED.md b/docs/parity/PROTECTED.md index ca1be2e..f2632c1 100644 --- a/docs/parity/PROTECTED.md +++ b/docs/parity/PROTECTED.md @@ -1,6 +1,7 @@ # PROTECTED — intentional monorepo additions (additive-only) -**DRAFT — awaiting user confirmation (Phase 0 gate).** +**CONFIRMED by the user at the Phase 0 gate (2026-09-21).** +Rulings are recorded in `DECISIONS.md` (D0–D12). Items here are intentional changes in `gold-banking-monorepo`. Per `CLAUDE.md` §4 they must **not** be removed, restyled, restructured, or have their behaviour @@ -125,9 +126,16 @@ pill-shaped SearchInput. --- -## D. Backend — stricter auth than the reference · **[?] needs your ruling** +## D. Backend — stricter auth than the reference · **CONFIRMED (D1)** -**Not yet protected — Q1 in the gate.** +**Protected.** The user ruled the target's stricter auth stands as an accepted +deviation. Additional obligation from D1: build a full +"reference admin operation ↔ target rid" matrix so no operation lacks a +permission, and explain the 172-vs-174 key difference. + +⚠️ **Not yet satisfied:** 5 ledger-write routes are still `@Public()` +(`DECISIONS.md` §"@Public audit" group A). They contradict D1 and are awaiting +the user's go-ahead to be protected. The reference leaves every admin route unauthenticated (`henza_server/routes/admin.php:12` and `:272` — the `auth:api` group wrapper is commented out at both ends, verified). @@ -143,7 +151,7 @@ reference behaviour as `SEC-01` and the residual `@Public()` routes as `SEC-02`. --- -## E. Backend — Decimal money discipline · **[found] proposed** +## E. Backend — Decimal money discipline · **CONFIRMED** The reference uses PHP floats throughout. `apps/api` has **zero `Float` columns** in `prisma/schema.prisma`; ledger and order math run on `Prisma.Decimal`. @@ -159,7 +167,12 @@ Same result, better representation. --- -## F. Prior deliberate non-ports · **[?] needs your ruling (Q4)** +## F. Prior deliberate non-ports · **CONFIRMED (D4)** + +Per D4: broken/destructive reference behaviour is not replicated, only logged — +but **business behaviour, calculations, rounding and messages must match the +reference exactly**, and every deliberate non-replication carries a matrix note +«عمداً تکرار نشد — ISSUE #n». Recorded in `docs/CHANGELOG-PORT-HISTORY.md` as intentionally **not** ported: @@ -180,14 +193,45 @@ otherwise. --- -## Summary — what needs your decision +--- -| Section | Item | Proposed | +## G. Target-only capabilities · **CONFIRMED (D2) — status ➕, do not touch** + +Per D2, features that exist only in the target are listed with status **➕** and +left alone. Ask before touching any of them. + +| Capability | Where | Note | |---|---|---| -| A | Admin Dashboard | protect (per brief) | -| B | Product Variants image picker | protect (per brief) | -| C | User-app designs + new features | protect (per brief), **excluding** the 5 v1↔v2 logic gaps | -| D | Stricter admin auth in `apps/api` | protect — **confirm (Q1)** | -| E | Decimal money types | protect — **confirm** | -| F.1–4 | Prior security / data-loss non-ports | protect — **confirm (Q4)** | -| F.5 | Branch horizontal layout | **treat as a real gap** — confirm | +| Dual-currency ledger (`LedgerEntry`, gold grams + rial) | `apps/api/src/modules/accounting/` | replaces the reference's single mixed-unit `balance` column | +| Gram / rial debt split (`goldDebtGrams`, `fiatDebtRial`) | `prisma/schema.prisma` | reference mixed both in one field | +| `Prisma.Decimal` money types throughout | schema + services | see §E | +| Server-enforced rid permissions | `auth/acl.constants.ts` | see §D | +| `UploadedFile` model + authenticated upload | `admin-uploads/` | reference upload was anonymous | +| Status-label / transition helpers | `admin-orders.service.ts` | reference had two divergent enums | + +This list grows as Phase 1 and Phase 2 identify more. It is **not** a to-do list — +it is a keep-out list. + +--- + +## Summary — confirmed status + +| Section | Item | Status | +|---|---|---| +| A | Admin Dashboard | ✅ protected (brief) | +| B | Product Variants image picker | ✅ protected (brief) | +| C | User-app designs + new features | ✅ protected (brief), **excluding** the 5 v1↔v2 logic gaps (D3) | +| D | Stricter admin auth in `apps/api` | ✅ protected (D1) — 5 ledger routes still open, pending approval | +| E | Decimal money types | ✅ protected | +| F.1–4 | Prior security / data-loss non-ports | ✅ protected (D4) | +| F.5 | Branch horizontal layout | ❌ **a real gap**, not protected | +| G | Target-only capabilities | ✅ ➕ leave untouched (D2) | + +### Explicitly NOT protected (parity work, per `DECISIONS.md`) + +- The `fullName` collapse — **being reversed** under D0 +- `percent` read from the referring agent — **wrong**, being corrected under D9 +- The 5 v1↔v2 logic gaps — D3 +- Unverified OTP — D6 +- The public customer list — D8 +- `singleCustomer.vue`'s full-form send — D11