# Page Review — Earnings (P6) + Transfer Requests (P7) + Specialist Tips (P8)

- **Routes:** `/earnings` · `/agents-wallet/index` · `/agents-wallet/specialist-tips`
- **Controllers:** `EarningsController` (9 actions, incl 🔴 payInvoice/requestTransfer/subscriptionActivate-Cancel/addCard/setDefaultCard) · `AgentsWalletController` (10 actions, incl 🔴 payment/create/delete)
- **Reviewed:** 2026-07-01 · **Status:** 🔍 (security agent running — highest-risk cluster)

## 2. Design findings
| # | What | Actual | Sev | Evidence |
|---|------|--------|-----|----------|
| D1 | Earnings dashboard | ✅ RTL; tabs الأرباح/الرسوم/الفواتير/التسوية/الاشتراك; KPI cards (withdrawable/net/fees/revenue/completed/withdrawn/due) with color accents; per-booking earnings table | — | ss_3030iom4p |
| D2 | Earnings table header | copy "تاريخ الإبراد" likely typo for "تاريخ الإيراد" (revenue date) | LOW | ss_3030iom4p |
| D3 | Transfer Requests | ✅ RTL table (transfer #, collected/fees/VAT/tips/net, settlement badges مطلوب/تم التسوية, dates, docs); search+date+status filters | — | ss_5562o5lht |
| D4 | Specialist Tips | ✅ RTL table (specialist code/name, total tips, transfer #, settlement) | — | ss_9875vi7ky |

## 3. Logic findings
| Action | Result |
|---|---|
| 🔴 requestTransfer / payInvoice / addCard / setDefaultCard / subscriptionActivate-Cancel / payment / delete | **NOT triggered live** (real money movement). Code-trace + security-review ONLY. |
| 🟢 index / view / viewPayment / getBankAccountDetails / specialistTips | render verified (design) |

## 4. Code trace
**Security-reviewer agent (EarningsController + AgentsWalletController + models). Agent reported 3 CRIT/4 HIGH; after verification 1 was real-CRIT, 2 overstated.**

| Sev (verified) | File:line | Issue | Status |
|---|---|---|---|
| HIGH (agent: CRIT) | AgentsWalletController.php:371 actionCreate | non-zero POSTed `account_id` flowed into `createWithdrawal()` with NO shop-ownership check → payout bound to another shop's bank account/IBAN (IDOR) | ✅ FIXED (R-H009) |
| ~~CRIT~~→info | AgentsWalletController.php:229 | agent claimed null-`$shop` crash — FALSE: line 230 already guards `if(!$shop)`. Added `access` behavior as defence-in-depth | ✅ AccessControl added |
| ~~CRIT~~→LOW | AgentsWalletController.php:536 viewPayment | agent claimed unscoped → all payments; FALSE: `$id` is required + validated by shop-scoped `findModel()`. Hardened `andFilterWhere`→`andWhere((int)$id)` anyway | ✅ hardened |
| **HIGH (deferred)** | EarningsController.php:457 actionPayInvoice | **card path sets invoice STATUS_PAID with NO gateway call** → owner marks own invoices paid for free | ⚠️ **LOGGED R-D01 — needs Paymob integration decision (may be demo stub); do NOT ship to prod as-is** |
| **HIGH (deferred)** | vhosts/storage.navagoo.localhost.conf:25 | `Options Indexes` → public directory listing of uploaded bank-transfer slips (IBAN/bank leak) | ⚠️ **LOGGED R-D02 — infra: remove Indexes + serve slips via auth-gated action** |
| HIGH (deferred) | AgentsWalletController.php:180 saveColumnPreferences | no column-key allowlist (EarningsController has one) → session poisoning (own session, low impact) | ⚠️ LOGGED R-L16 |
| HIGH (deferred) | WithdrawalSearch.php:50 / PaymentSearch.php:47 | `identity->shop->id` no `?? null` → 500 for shopless user (edge; AccessControl now mitigates) | ⚠️ LOGGED R-L17 |
| **MED (urgent)** | common/config/base.php:21 | **committed Firebase service-account JSON; code comment says "leaked and MUST be rotated in GCP"** | ⚠️ **LOGGED R-D03 — ROTATE KEY IN GCP + remove fallback path** |
| LOW | AgentsWalletController.php:551 actionDelete | no settlement-status guard → owner can delete a SETTLED withdrawal (orphan earnings / re-request) | ⚠️ LOGGED R-L18 |
| LOW | AgentsWalletController.php:373 / :398 | `account_id` loose `==` ; bank name no length pre-check (save() validates) | ⚠️ LOGGED |

**Confirmed clean by agent:** invoice / withdrawal / bank-account(getBankAccountDetails) / earnings / subscription / saved-card IDOR all shop/user-scoped; transfer AMOUNT derived server-side from locked DB rows (owner can't set it); double-spend prevented by `SELECT … FOR UPDATE` + re-verify in WithdrawalBundlingService; CSRF on (VerbFilter POST + tokens); slip upload MIME+size validated.

## 5. Fixes applied
| Issue | File:line | Change | Re-verified |
|-------|-----------|--------|-------------|
| R-H009 account_id IDOR (payout to foreign account) | AgentsWalletController.php:~412 | `else` branch: verify `account_id` belongs to `shop->id` before `createWithdrawal()` | ✅ php -l clean |
| AccessControl missing | AgentsWalletController.php:~40 | add `access` behavior (`roles:['@']`) mirroring EarningsController | ✅ php -l clean |
| viewPayment scope hardening | AgentsWalletController.php:536 | `andFilterWhere`→`andWhere(['withdrawal'=>(int)$id])` | ✅ php -l clean |

## 6. Sign-off
- [x] P6/P7/P8 design verified (RTL, KPIs, tables)
- [x] security-review folded; 1 real IDOR fixed + 2 hardenings; 2 agent "CRITICALs" verified overstated
- [ ] ⚠️ **3 deferred items need YOUR action: R-D01 card-payment integration, R-D02 storage dir-listing, R-D03 rotate Firebase key**
- [x] 🔴 money actions confirmed safe by code-trace (amount server-side, FOR UPDATE, IDOR scoped) — NOT live-tested by design

## 6. Sign-off
- [x] P6/P7/P8 design verified (RTL, KPIs, tables)
- [ ] security-review findings folded + fixes
- [ ] 🔴 money actions: confirmed safe by code-trace (NOT live-tested by design)