# Navagoo Code Analysis Report **Project:** Navagoo (Yii2 Advanced — `yii2-starter-kit/yii2-starter-kit`) **Analysis date:** 2026-04-28 **Scope analyzed:** `api/`, `backend/`, `frontend/`, `common/`, `console/`, vhosts, env files, root-level files. Vendor and runtime directories excluded. **Codebase size:** 1,482 PHP/JS files (excluding vendor, runtime). Largest files: `backend/views/shop/_form.php` (1,613 LOC), `backend/controllers/ShopController.php` (1,231), `api/controllers/BookingController.php` (1,133), `api/controllers/UserController.php` (1,079). **Tech stack:** PHP ≥7.4, Yii2 ^2.0.13, MySQL, Apache (vhosts), Docker, Paymob payments, Twilio SMS, WhatsApp T2 integration, Elasticsearch, FCM push. --- ## 1. Executive Summary The codebase is a Yii2 Advanced template that has accumulated significant technical and security debt. The headline issues are **not subtle bugs** — they are leaked production credentials in the git repository, a publicly accessible PHP file that exposes WhatsApp credentials in a login form, mass-assignable privileged user attributes, and SQL injection in admin search endpoints. There are zero automated tests. **Severity counts:** | Severity | Count | |----------|-------| | Critical | 6 | | High | 7 | | Medium | 9 | | Low | 6 | **Top 5 findings to fix this week:** 1. **Rotate every secret in `.env.dist`, `README.md`, `test_hmac.php`** — they are committed in git history. Paymob API key, Paymob HMAC secret, Glide sign key, cookie validation keys, Mailtrap creds. 2. **Delete or gate `api/web/whatsapp-test.php`** — it pre-fills your real `WHATSAPP_EMAIL` and `WHATSAPP_PASSWORD` from `.env` into a public HTML form. 3. **Strip `password_reset_token, approval, wallet, role, roles, shop_id, phone_verified` from the `safe` rule in `common/models/User.php:272`** — these are mass-assignable. 4. **Fix SQL injection in `backend/controllers/HelperController.php`** lines 34, 60, 91, 115 — `$q` and `$role` are concatenated into the WHERE string. 5. **Set `YII_ENV=prod` in production** — current `.env` has `YII_ENV=dev` which enables the `debug` module + Gii code generator, and the global access rules expose `debug/default` to anonymous users. --- ## 2. Security Findings ### 2.1 CRITICAL — Real production secrets committed in git **Files:** `.env.dist`, `README.md`, `test_hmac.php`, `test_wa_header.php`. All confirmed tracked via `git ls-files`. `.env.dist` is meant to be a template with placeholder values. In this repo it ships with **real secrets**: | Secret | Value (file:line) | |--------|-------------------| | `GOOGLE_MAP_API_KEY` | `.env.dist:32` (truncated key visible) | | `SMTP_USER` / `SMTP_PASS` | `.env.dist:36-37` (`mohamed.amer2050@gmail.com` + plaintext password) | | `FRONTEND_COOKIE_VALIDATION_KEY` | `.env.dist:46` | | `BACKEND_COOKIE_VALIDATION_KEY` | `.env.dist:47` | | `GLIDE_SIGN_KEY` | `.env.dist:54` | `README.md` ends with: ``` http://backend.navagoo.localhost/ - admin - PA#$%gfaF ``` Admin credentials in the public README. `test_hmac.php:3`: ```php $hmacSecret = ''; ``` This is the same value as `PAYMOB_HMAC_SECRET` in `.env`, hardcoded in a tracked test script at the project root. With Paymob's HMAC secret known, an attacker can forge webhook callbacks and mark fake transactions as `success: true`. The actual `.env` (correctly gitignored) contains additional production secrets: ``` PAYMOB_API_KEY= # base64-encoded JWT-style payment key PAYMOB_HMAC_SECRET= WHATSAPP_PASSWORD= # commented out, but tied to the current developer's email DB_PASSWORD=root ``` **Fix:** - Rotate all leaked secrets *now* with their respective providers (Paymob, Google Maps, Brevo/Sendinblue, Glide). - Replace `.env.dist` with **placeholders only** (`SMTP_PASS=changeme`, etc.). - Remove `test_hmac.php`, `test_wa_header.php`, `api.zip`, and the `admin / PA#$%gfaF` block from `README.md`. - Run `git filter-repo --invert-paths --path .env.dist --path test_hmac.php --path test_wa_header.php --path api.zip` (or BFG) to scrub history. Force-push and notify all collaborators. - Move HMAC verification self-tests into `tests/` (gitignored or written without the literal secret). ### 2.2 CRITICAL — `api/web/whatsapp-test.php` exposes WhatsApp credentials publicly **File:** `api/web/whatsapp-test.php` — sits in the public webroot, no auth gate. ```php $emailPost = env('WHATSAPP_EMAIL') ?: ''; // pre-fills from .env $passPost = env('WHATSAPP_PASSWORD') ?: ''; ... ``` Anyone hitting `https://api./whatsapp-test.php` sees a form **pre-populated with `m.islambouli@navagoo.com` and `Nav@@321123`** (or whatever is in `.env`). The page also POSTs to authenticate and prints the resulting bearer token. **Fix:** Delete the file. If a debugging tool is genuinely needed, move it under `console/` (CLI only) or behind backend admin auth. ### 2.3 CRITICAL — Mass-assignment of privileged attributes on `User` **File:** `common/models/User.php:272` ```php ['password_reset_token, approval, firebase_token, wallet, wallet_last_update, full_name, shop_id, roles, role, social_id, social_type, phone_verified, hide, formatted_id', 'safe'], ``` Yii2's `safe` rule marks attributes as mass-assignable through `$model->load($postData)`. This list includes: | Attribute | Why it's dangerous when mass-assignable | |-----------|----------------------------------------| | `approval` | Self-approve a pending agent / shop-owner account | | `wallet`, `wallet_last_update` | **Direct financial fraud** — set arbitrary balance | | `roles`, `role` | Privilege escalation to admin | | `shop_id` | Hijack association to another shop | | `phone_verified` | Bypass OTP flow | | `password_reset_token` | Self-issue a reset token without going through the email flow | If any controller calls `$user->load(Yii::$app->request->post())` and saves on a User-derived model, the request body decides the value. Even if no current controller does so, the rule is a footgun for the next developer. **Fix:** Remove these from `safe`. Use scenarios (`scenarios()` method) for the legitimate update flows. Define explicit attribute lists per scenario (`SCENARIO_REGISTER`, `SCENARIO_UPDATE_PROFILE`, `SCENARIO_ADMIN_EDIT`) and put privileged attributes only in the admin scenario. ### 2.4 CRITICAL — SQL injection in `backend/controllers/HelperController.php` **File:** `backend/controllers/HelperController.php` — 4 actions, vulnerable WHERE clauses at lines **34, 60, 91, 115**: `actionListCustomers`, `actionListNanny`, `actionUsersList`, `actionUsersListByRole`. Line 34: ```php $role = 'user'; $q = preg_replace('/\s+/', '', $q); ... ->where('`firstname` LIKE \'%'.$q.'%\' and rbac_auth_assignment.item_name ="'.$role.'" and user_type=0' ) ``` `$q` comes from `?q=` GET param. The only sanitization is whitespace stripping — quotes, parentheses, and SQL keywords pass through. Payload: `?q=a%' OR 1=1 -- ` exfiltrates the table. In `actionUsersListByRole`, `$role` also comes from GET with no validation against the role enum. **Mitigating factor:** The `as globalAccess` rule in `backend/config/web.php` requires the user to be authenticated as `manager`, `administrator`, or `shopOwner`. So this is not unauthenticated SQLi — but it's still a privilege-escalation path: a `shopOwner` can read any user's email/profile or pivot to UPDATE/DELETE via stacked queries. **Fix:** Use parameter binding everywhere: ```php ->where(['like', 'firstname', $q]) ->andWhere(['rbac_auth_assignment.item_name' => $role]) ->andWhere(['user_type' => 0]) ``` ### 2.5 CRITICAL — `YII_ENV=dev` in `.env` + `debug/default` open to anonymous **Files:** `.env:3`, `backend/config/web.php:86-89`. `.env` ships with: ``` YII_DEBUG= false YII_ENV= dev ``` `backend/config/web.php` has: ```php [ 'controllers' => ['debug/default'], 'allow' => true, 'roles' => ['?'], // anonymous users ], ... if (YII_ENV_DEV) { $config['modules']['gii'] = [...]; } ``` If this `.env` is the production env, then in production: - The Yii2 debug toolbar module is loaded. - Its controller is allow-listed for **anonymous visitors**. - Gii code generator is enabled. Gii has historically been used to write controller files and achieve RCE. The `YII_DEBUG=false` flag mutes the toolbar UI but does **not** disable the module or the route. **Fix:** Set `YII_ENV=prod` in production deployments. Drop the `debug/default` allow-rule (or restrict to `roles => ['administrator']` and check `Yii::$app->user->identity->isSuperAdmin` in addition). Ensure `YII_ENV_DEV` is false in prod so Gii is not loaded. ### 2.6 CRITICAL — `eval()` on regex-extracted strings **File:** `console/controllers/ExtendedMessageController.php:66` ```php $message = eval("return {$message};"); ``` `$message` is captured from a regex over project source files. Console-only attack surface, but it's `eval` of a pattern that allows nested PHP. If an attacker can drop a single source file (e.g., via the `whatsapp-test.php` exposure or a vulnerable image upload that gets included), the next i18n run runs their PHP. **Fix:** Replace with a string parser (`json_decode`, or a small recursive descent over PHP string literal syntax). There is no need to `eval` to unescape a quoted string. ### 2.7 HIGH — OTP generated with `mt_rand`, no verification rate limit **File:** `api/controllers/UserController.php` lines 501, 525, 559. ```php $otp = mt_rand(1000, 9999); // Generate 4-digit OTP ``` - 4 digits = 10,000 possibilities. Brute-forceable in seconds without rate limit. - `mt_rand` is not cryptographically secure; given a few observed OTPs, the seed is predictable. - `SmsLog::checkOtpResendLimit` rate-limits **resend**, not **verification attempts**. **Fix:** - Use `random_int(100000, 999999)` for a 6-digit OTP, or `Yii::$app->security->generateRandomString(6)` for alphanumeric. - Lock the OTP after 5 failed verification attempts within a window. Track in `sms_log` or a new `otp_attempts` column. ### 2.8 HIGH — Access tokens never expire **File:** `common/models/User.php:144-149` ```php public static function findIdentityByAccessToken($token, $type = null) { return static::find() ->active() ->andWhere(['access_token' => $token]) ->one(); } ``` The User model has an `access_token` column generated once on insert (line 197, `EVENT_BEFORE_INSERT`). There is no `expires_at` and no rotation. If a token leaks (and `whatsapp-test.php` was happy to print one), it is valid for the lifetime of the user. **Fix:** Add `access_token_expires_at` column. Filter the query: `->andWhere(['>', 'access_token_expires_at', new Expression('NOW()')])`. Rotate the token on every login. Provide a logout endpoint that nulls it. ### 2.9 HIGH — File upload validates client-supplied MIME **File:** `api/helpers/ImageHelper.php:215-237` ```php $allowedTypes = ['image/jpeg','image/png','image/jpg','application/pdf']; $ext = end(explode('.', $filename)); if (!empty($file) && in_array($file['type'], $allowedTypes) && in_array(strtolower($ext), $allowedExt) && $file['size'] <= $size) { ``` `$file['type']` is taken from the `Content-Type` of the multipart part — fully attacker-controlled. The extension check + random filename mitigates classic shell upload (the file is written as `.`, so `.php` is rejected by the extension list). But the MIME check is misleading and should not be trusted. Also: `end(explode(...))` triggers a PHP notice ("Only variables should be passed by reference"). And there is no check that the file is actually an image (a `.jpg` containing PHP can be served as PHP if the storage path is misconfigured). **Fix:** Validate the real MIME via `finfo_file($_FILES['x']['tmp_name'])` or use Yii's `FileValidator` with `mimeTypes`. Reject anything where the detected MIME does not match the extension. Ensure the storage directory has `php_flag engine off` or is outside the document root. ### 2.10 HIGH — CORS `*` on authenticated endpoints **File:** `api/controllers/MyRestController.php:18-22` and `api/controllers/MyActiveUnAuthController.php:30-34` ```php public static function allowedDomains() { return ['*']; } ``` Combined with `'Access-Control-Request-Headers' => ['*']`, any origin can issue cross-origin requests. With Bearer token auth (not cookies) the standard CSRF risk is gone, but `Authorization: Bearer ...` headers leaked through XSS in any partner site can be used directly. Use a finite allow-list. **Fix:** Maintain an explicit list of trusted origins (`https://navagoo.com`, mobile-app webview origins, etc.). Disallow `*` once you serve any non-public data. ### 2.11 HIGH — Apache `Options Indexes` enables directory listing **File:** `vhosts/api.navagoo.localhost.conf` ```apache Options Indexes FollowSymLinks MultiViews ... ``` If any subdirectory under `api/web/` lacks an index file, Apache will render a directory listing. `api/web/` already contains `invoice-822.pdf`, `invoice-823.pdf`, and `whatsapp-test.php` — listing leaks them. The same `Indexes` flag is likely in the other vhost configs (only one was sampled). **Fix:** Replace `Options Indexes FollowSymLinks MultiViews` with `Options -Indexes FollowSymLinks`. ### 2.12 HIGH — Leaked invoice PDFs and backup zip in webroot `api/web/invoice-822.pdf`, `api/web/invoice-823.pdf` are addressable directly. `api.zip` (99 KB) sits at the project root and is git-tracked. **Fix:** Delete from git (`git rm --cached`), and never write user invoices into the webroot. Serve them via an authenticated controller action that streams from `storage/`. ### 2.13 MEDIUM — `$_REQUEST/$_GET/$_POST` used directly (40+ locations) `grep -rn '\$_(GET|POST|REQUEST)\['` returns 40 hits in app code. Most are benign language-switching (`$_REQUEST['lang']`), but bypassing Yii's `Request` component skips its parsing rules and CSRF awareness. **Fix:** Standardize on `Yii::$app->request->get(...)`, `getBodyParam(...)`, etc. Add a phpcs rule to forbid `$_REQUEST`/`$_GET`/`$_POST` outside of bootstrap files. ### 2.14 MEDIUM — Outdated dependencies - `yiisoft/yii2: ~2.0.6` — current `2.0.50+` (4 years of security fixes). - `guzzlehttp/guzzle: ^6.0` — Guzzle 6 is end-of-life, current is 7.x. - `tecnickcom/tcpdf: ^6.6` — historic XXE/RCE via images, ensure latest patch. - `kartik-v/*: @dev` and `dev-master` — pinning to dev branches in production is unsafe. **Fix:** Run `composer audit`; bump majors in a controlled branch. Replace `@dev` and `dev-master` with concrete versions or commit hashes. ### 2.15 MEDIUM — Initial SQL dump in repo **File:** `common/migrations/db/sql/intialize.sql` (note typo). If this contains seeded admin password hashes or stub keys, those hashes leak with the repo. **Fix:** Move to `.gitignore` or scrub PII/credentials from it. ### 2.16 LOW — `simulate_webhook.php` builds a shell command from variables **File:** `api/runtime/simulate_webhook.php:10` ```php $res = shell_exec("curl -s -X POST '...?hmac=$hmac' -H '...' -d '$rawBody'"); ``` `runtime/` should be gitignored, and this is a debug helper. But if `$rawBody` ever flows from user input, it's a shell injection. Keep it out of webroot and out of git. --- ## 3. Code Quality Findings ### 3.1 HIGH — Zero automated tests `find . -name "*Test.php"` outside `vendor/` returns **0 files**. The `tests/` directory referenced by `index-test.php` does not exist in tree. - Refactoring is high-risk because there is nothing catching regressions. - Add at least: - PHPUnit unit tests for `common/helpers/*` (these contain business logic — `IdGeneratorHelper` 887 LOC, `BookingHelper` 808 LOC, `NotificationHelper` 1,321 LOC). - Codeception API tests covering signup, login, OTP, password reset, booking creation. ### 3.2 HIGH — `var_dump()` and `die()` left in API controllers 15+ occurrences in `api/controllers/*` (e.g. `UserController.php:822-823`, `BookingController.php:1048`, `WalletController.php:269`). When the error path is hit, the API breaks JSON contract and dumps internal state to the client. **Fix:** Replace with `Yii::error($model->errors, __METHOD__)` and a structured error response. Add a phpcs rule banning `var_dump|print_r|die\(` in `api/`, `backend/`, `frontend/`. ### 3.3 MEDIUM — God classes / megafiles Top offenders by LOC: | File | LOC | |------|----:| | `backend/views/shop/_form.php` | 1,613 | | `frontend/views/agents/_form.php` | 1,485 | | `common/helpers/NotificationHelper.php` | 1,321 | | `backend/controllers/ShopController.php` | 1,231 | | `api/controllers/BookingController.php` | 1,133 | | `frontend/views/shop-service/_form.php` | 1,086 | | `api/controllers/UserController.php` | 1,079 | | `common/models/base/Shop.php` | 1,054 | **Fix:** Extract domain operations into service classes (e.g. `BookingService`, `WalletService`). For views, split into partials (`_form_basic.php`, `_form_address.php`, etc.). The `common/helpers/*` files are doing the work of services — move them into `common/services/`. ### 3.4 MEDIUM — Stale commented-out code Top files by commented PHP-statement lines (rough heuristic): | File | Commented lines | |------|----------------:| | `common/helpers/NotificationHelper.php` | 56 | | `api/resources/NotificationsResource.php` | 29 | | `backend/controllers/ShopController.php` | 27 | | `api/controllers/UserController.php` | 20 | | `api/controllers/BookingController.php` | 18 | Visible examples in `console/config/schedule.php` show parallel `qc` and `prod` paths both uncommented — both schedules will run on the same host. Confirm intent. **Fix:** Delete dead code (git history is the source of truth). Where the commented-out block represents a future feature, link to a ticket. ### 3.5 MEDIUM — Multiple `index-test.php`, `whatsapp-test.php`, `test_*.php` files in webroot `api/web/index-test.php`, `backend/web/index-test.php`, `frontend/web/index-test.php` all guard with `if (YII_ENV !== 'test') die(...)` — safe **as long as the env is right**. With current `YII_ENV=dev`, the guard accepts `dev` ≠ `test`, so they are not bootable, but they reveal full path information in the `die()` message. Combined with `whatsapp-test.php` (no guard), test artifacts in production webroot is a smell. **Fix:** Move them under `tests/` and remove from `web/`. Production deployment should not ship them. ### 3.6 LOW — Inconsistent naming, copy-paste of `actionListNanny`/`actionListCustomers` `backend/controllers/HelperController.php` has four near-identical actions that differ only in the `$role` filter and `user_type`. They share the SQL injection bug — fixing one and forgetting the others is likely. **Fix:** Extract a private helper `userQuery(string $role, ?int $userType): Query` and call from each action. ### 3.7 LOW — Postman collections committed at root `Navagoo_API.postman_collection.json` (49 KB), `Password_Reset_Collection.postman_collection.json` (15 KB), `whatsapp_t2_postman_collection.json`. Useful for onboarding, but they often contain sample tokens. Move to a `docs/` directory and review before commit. --- ## 4. Architecture Findings ### 4.1 MEDIUM — Standard Yii2 Advanced layout, used reasonably Five top-level applications: `api`, `backend`, `frontend`, `console`, `common`. Routing is split (`_urlManager.php` + `urls/_AgentUrls.php` + `urls/_CustomerUrls.php`) which is healthier than one mega-file. `common/` holds shared helpers, models, behaviors, RBAC. This is the canonical structure. **What is missing:** - A **service / domain layer**. Business logic lives inside controllers (`BookingController` is 1,133 LOC) and helpers (`BookingHelper` 808 LOC). There is no `common/services/` for use cases. - **No API versioning.** `api/modules/v1/` is mostly empty, and routes go through `api/controllers/` directly. A breaking change requires touching every consumer. - **No DTO layer.** API resources (`api/resources/*Resource.php`) extend `ActiveRecord` directly, so the public API shape is the database shape. Renaming a column breaks the API contract. **Fix (incremental):** - Stand up `common/services/{Booking,Wallet,Notification,User}Service.php`. Move one method out of the largest controller into a service per sprint. - Introduce `api/v2/` with explicit DTOs (`api\v2\dto\BookingDto`). Run v1 and v2 in parallel. ### 4.2 MEDIUM — 208 models, 52 base classes from Gii — schema/code drift risk `common/models/base/` holds 52 Gii-generated base models. The convention is "regenerate base, write extensions on the subclass". Without tests, regenerating bases on schema changes silently rewrites accessors, breaking dependent code unless reviewed line-by-line. **Fix:** Add a CI step that regenerates Gii bases against a clean DB and diffs them against committed bases. Reject PRs where non-base classes touch generated regions. ### 4.3 MEDIUM — Cron schedule has both `qc` and `prod` enabled simultaneously **File:** `console/config/schedule.php:33-44` ```php $schedule->exec('php /var/www/html/qc/console/yii bookings/notifications')->everyMinute(); ... $schedule->exec('php /var/www/html/prod/console/yii bookings/notifications')->everyMinute(); ``` Both `qc` and `prod` schedules are uncommented. If both servers run this scheduler with the same DB, you double-fire notifications, double-charge, double-process. **Fix:** Branch on `env('APP_ENV')` and emit only the matching set; or split into two files and load via env. ### 4.4 LOW — `api/controllers/UserController.php` extends plain `Controller`, all auth manual `UserController extends \yii\rest\Controller` (not `MyRestController`). It has no `behaviors()` method, so no Bearer auth filter is applied globally. Each action that should require auth must check manually. The signup / sign-in / verify endpoints belong here, but actions like `actionTest`, `actionTestSms`, `actionTestTimes` should not exist on a public controller in production. **Fix:** Split: public `AuthController` (signup/login/OTP/password-reset) and an authenticated `UserController extends MyActiveController`. Remove `actionTest*` actions. ### 4.5 LOW — Dependency on `@dev` / `dev-master` packages (See 2.14.) `engmohamedamer/yii2-enhanced-gii: @dev`, `kartik-v/yii2-mpdf: dev-master`, etc. The composer.lock is the only thing pinning these, so a delete-vendor + composer install can pull a different commit. **Fix:** Pin to tagged versions. --- ## 5. Performance & Database Findings ### 5.1 HIGH — Eager loading is rare; N+1 likely widespread Across `api`, `backend`, `frontend`, `common`: - `->with(` or `->joinWith(` appears in roughly 10 places. - API resources extend `ActiveRecord` and rely on default `fields()` for serialization. When a serializer touches a relation (e.g., `$booking->shop->name` or `$booking->user->profile->firstname`), Yii lazy-loads — one query per row. For an endpoint that lists 50 bookings each with 3 relations accessed during serialization, that's 150 queries instead of 4 with eager loading. **Fix:** Audit the busy list endpoints (`BookingController::actionIndex`, `ShopsController::actionIndex`, `RateController::actionIndex`). Add `->with(['user', 'user.profile', 'shop', 'service'])` to their queries. Enable Yii's query log in dev and watch the count drop. ### 5.2 HIGH — Unbounded `find()->all()` in views and lookups `backend/modules/rbac/views/rbac-auth-assignment/index.php:42` builds a `User::find()->all()` dropdown of every user in the database for an admin form. With even a few thousand users that's a multi-MB response. Other lookups (`LookupsController` for cities, languages, services) also pull everything in one shot — fine if the rows are small and few, but uncached so every request hits the DB. **Fix:** - Replace user-picker dropdowns with AJAX-backed Select2 (the project already has `kartik-v/yii2-widgets`). Limit + search. - Cache lookup endpoints (`/lookups/city`, `/lookups/languages`, `/lookups/nationalities`) for 1–24h with `Yii::$app->cache`. ### 5.3 MEDIUM — Caching is barely used `Yii::$app->cache` appears in only ~13 places across the entire codebase. Configuration cache, schema cache, and any computed-once data (settings, lookup tables) are running uncached. **Fix:** Enable `cache` schema-cache (`Yii::$app->db->schemaCache = 'cache'`), and cache lookup endpoints with `Yii::$app->cache->getOrSet(...)`. ### 5.4 MEDIUM — Indexes likely missing on foreign keys and search columns 165 migration files, but only 8 use `addForeignKey` and very few create indexes (`createIndex`). Yii does not auto-index foreign keys. Common hot columns that need indexes: - `booking.user_id`, `booking.shop_id`, `booking.agent_id`, `booking.status`, `booking.created_at`. - `user.email`, `user.mobile`, `user.access_token`, `user.status`, `user.approval`. - `payment.booking_id`, `payment.status`. - Whatever is in the JOINs of `HelperController` (user_profile.user_id, rbac_auth_assignment.user_id). **Fix:** Run `EXPLAIN` on the slowest 20 queries from your DB slow-log. Add `createIndex` migrations for any column used in WHERE, JOIN, or ORDER BY without an index. Specifically check `user.access_token` — every authenticated API request runs `WHERE access_token = ?`, this column **must** be indexed and ideally unique. ### 5.5 MEDIUM — `LIKE '%q%'` on `firstname` cannot use a normal index Even when `HelperController` is fixed for SQL injection, `firstname LIKE '%q%'` is a full-table scan. With 100K+ users this is slow. **Fix:** Use a search index (Elasticsearch is already a dependency: `yiisoft/yii2-elasticsearch ~2.1.0`). Or add a MySQL FULLTEXT index and switch to `MATCH(...) AGAINST(...)`. ### 5.6 LOW — `storage/` is 33 MB Not catastrophic, but `storage/cache` and `storage/web` should be excluded from backups and rotated. Check that uploaded user content has a separate retention policy (e.g., delete on user deletion to comply with GDPR-style requests). ### 5.7 LOW — `mt_rand` for UUID-like fallback in `ImageHelper` ```php $output_file_with_extension = $fileType . (time() + rand(0,10000)) .'.' . $extension; ``` Two simultaneous uploads in the same second can collide. Use `Yii::$app->security->generateRandomString()` or `uniqid('', true)`. --- ## 6. Prioritized Action Plan ### This week (stop the bleeding) 1. Rotate Paymob API key + HMAC secret, Google Maps key, Brevo SMTP, Glide sign key. 2. Delete `api/web/whatsapp-test.php`, `api.zip`, `test_hmac.php`, `test_wa_header.php`. Strip `admin / PA#$%gfaF` from `README.md`. 3. Force-push history scrub of the above files. 4. Edit `common/models/User.php:272` — drop `approval, wallet, roles, role, shop_id, phone_verified, password_reset_token` from `safe`. 5. Fix `HelperController` SQL injection (parameter-bind all four actions). 6. Set `YII_ENV=prod` in production `.env`. Restrict `debug/default` to `administrator` role. 7. Replace `mt_rand` OTP with `random_int(100000, 999999)` and add 5-attempt lockout. ### Next sprint 8. Add `access_token_expires_at` to user table; rotate token on login. 9. Fix Apache `Options -Indexes` in all 4 vhost files. 10. Strip `var_dump`/`die` from controllers. 11. Run `composer audit`; bump `yii2`, `guzzle 7`, `tcpdf`. Pin `@dev` packages. 12. Add `with()` / `joinWith()` to the top 5 list endpoints. Enable query logging and verify reduced query counts. 13. Add an index on `user.access_token` and on FKs of the booking/payment tables. ### Quarterly 14. Build a service layer (`common/services/`). Move logic out of controllers >500 LOC. 15. Introduce DTOs in `api/v2/`. 16. Stand up tests: aim for 30% coverage of `common/helpers/*` and the auth flow. 17. Consolidate cron between `qc` and `prod` via env switch. 18. Replace `firstname LIKE '%q%'` searches with Elasticsearch (already a dependency). --- ## 7. How to invoke the matching skills (Claude Code, in your terminal) The Antigravity skills you installed live at `~/.claude/skills` and are visible to **Claude Code** (run `claude` in your terminal at the project root): ```bash cd ~/Documents/dockermachine/www/Navagoo # fix the SQL injection in HelperController, guided by the OWASP-aligned skill claude >> /top-web-vulnerabilities review backend/controllers/HelperController.php and write the patch # rotate / harden secrets >> /backend-security-coder audit .env handling and access_token expiration in common/models/User.php # performance pass >> /postgres-best-practices (or sql-pro) review missing indexes; suggest createIndex migrations # add tests >> /test-driven-development scaffold pytest-style PHPUnit + Codeception tests for api/controllers/UserController.php # architecture >> /senior-architect propose a service layer extraction plan for BookingController ``` For an active subset, the bundle activator in the antigravity repo: ```bash ~/.claude/skills/scripts/activate-skills.sh --clear "Security Developer" ``` --- *Report generated by a code analysis pass run inside Cowork. Findings reference real file paths and line numbers; spot-checking on commit `HEAD` is recommended before remediation.*