update

.paul/codebase/concerns.md

@@ -0,0 +1,137 @@
+# Codebase Concerns
+
+**Analysis Date:** 2026-05-10
+
+> Severity legend: **CRITICAL** (immediate fix), **HIGH** (next sprint), **MEDIUM** (planned), **LOW** (track).
+
+## Critical Security
+
+**Hardcoded credentials committed to repository — CRITICAL**
+- Files: `buy-by-phone.php` (lines ~11–28: reCAPTCHA secret `6LeJAUUsAAAAAIyCrwFMrsw9yQLPgnCWCOHPcjc8`, SMTP passwords `8njIZnAb`, `kM4BU_F_`), `app/config/parameters.php` (DB password `eRbZ]Ioh-0-2]fM+`, app secret `LStbzv3bfsMaq5dvsVR5wXt8dEpe63IAx2uOHffpbK9gy9x4EZsjWeRM`, cookie keys)
+- Why: convenience during development — never moved to env vars
+- Impact: full DB compromise, mail account takeover, session forgery if repo or backups leak; secrets remain valid in git history
+- Fix: rotate ALL exposed credentials immediately → move to environment variables (or Docker secrets) → load via `getenv()` → purge from git history (`git filter-repo` / BFG) → add the file to `.gitignore`
+
+**Diagnostic / phpinfo files exposed in webroot — CRITICAL**
+- Files: `diag_20792_tmp.php` (2.6 KB), `diag_upload_tmp.php` (4.4 KB), `diag_fix_tmp.php` (14 B), `info.php` (14 B — almost certainly `<?php phpinfo();`)
+- Why: ad-hoc debug scripts left after a 2026-03-05 incident (`feat: update diagnostic scripts and configuration for improved product data handling`, commit `194405bc`)
+- Impact: `info.php` discloses full PHP environment to anyone; diag scripts run raw SQL with weak `$_GET['key'] !== 'diag2026'` gate (security through obscurity, easily bypassed)
+- Fix: delete these 4 files now; if production deploy mirrors repo, also delete on server; consider reverting commit `194405bc`
+
+**Unsanitized `$_POST` / `$_GET` in custom scripts — HIGH**
+- Files: `buy-by-phone.php` lines 29–58 (17 direct `$_POST[...]` accesses) and lines 83–94 (POST values injected into HTML email body — XSS via mail / SMTP header injection); `category-description.php` line 7 (`$url = $_GET['url']`)
+- Why: written outside PrestaShop conventions
+- Impact: header injection, mail-relay abuse, stored XSS in support inboxes
+- Fix: replace with `Tools::getValue('field', '')` (PrestaShop sanitizer) and escape on output (`htmlspecialchars`, `Tools::safeOutput`); validate email with `Validate::isEmail()`; add reCAPTCHA verification result check before send
+
+**SQL string concatenation in custom scripts — HIGH**
+- Files: `diag_20792_tmp.php` lines 23–49, `diag_upload_tmp.php` lines 22–50, `import-product.php` lines 63–93
+- Why: legacy PrestaShop pattern (`pSQL()` instead of bound parameters)
+- Impact: any unsanitized input flowing into these queries is an injection vector
+- Fix: delete diag scripts; refactor `import-product.php` to use `Db::getInstance()` with proper escaping or Doctrine DBAL parameterized queries; never concatenate user input into SQL
+
+**Database backups + 5 MB error log inside webroot — HIGH**
+- Files: `iadmin/backups/` (e.g. `1698930967-7683b7af.sql.bz2`), `iadmin/errors.log` (5 MB, 26 months of PHP errors), `errors.log` (root)
+- Why: standard PrestaShop default location; `iadmin/` is the renamed admin folder (still under DocumentRoot)
+- Impact: SQL dumps directly downloadable if URL guessed; error log discloses paths and module internals
+- Fix: move backups outside DocumentRoot or `Deny from all` via `.htaccess`; rotate logs to `/var/log/...`; add `errors.log`, `*.log`, `iadmin/backups/`, `iadmin/errors.log` to `.gitignore`
+
+## Tech Debt
+
+**Backup and obsolete files committed — HIGH**
+- Files: `backup_before_patch/As4SearchEngine.php.backup` (160 KB), `backup_before_patch/As4SearchEngineDb.php.backup` (3.4 KB), `backup_before_patch/pm_advancedsearch4.php.backup` (279 KB)
+- Why: manual snapshot taken before patching the search engine module
+- Impact: bloats repo, confuses code search, may shadow current implementation in IDEs
+- Fix: delete the directory; rely on `git log` for history; add `*.backup`, `backup_before_patch/` to `.gitignore`
+
+**Oversized root-level scripts — MEDIUM**
+- Files: `import-product.php` (813 lines / 32 KB — fetches and applies external XML feed), `buy-by-phone.php` (122 lines — manual PHPMailer + reCAPTCHA)
+- Why: written as single-file utilities
+- Impact: hard to test, no transaction safety in importer, no logging of import results
+- Fix approach: extract `import-product.php` into a proper PrestaShop module or CLI command (`bin/console`-style), wrap DB writes in transactions, add structured logging; convert `buy-by-phone.php` into a `ModuleFrontController`
+
+**429-line `.htaccess` with hardcoded SEO redirects — MEDIUM**
+- File: `.htaccess` (~100 manual `RewriteRule ^...$ /s/.../...?` redirects, lines 14–370+)
+- Impact: not maintainable, regex conflicts likely, no audit trail
+- Fix: migrate to a database-driven redirect table + module that emits 301s in `hookActionDispatcher` (or use `modules/x13linkrewrite/`)
+
+**107 undocumented core overrides — MEDIUM**
+- Files: `override/classes/` (31 files — `Cart.php`, `CartRule.php`, `Product.php`, `Hook.php`, `Dispatcher.php`, `controller/FrontController.php`, `controller/ProductListingFrontController.php`, …) plus 76 more under `override/`
+- Why: each accumulated over time to extend core behavior
+- Impact: every PrestaShop upgrade requires manual override review; no inline doc states why each override exists
+- Fix: add a header comment to each override stating purpose + ticket; consider moving simple cases to module hooks
+
+**Stale stack — PrestaShop 1.7.8.11 / PHP 7.4 — MEDIUM**
+- File: `app/AppKernel.php` line 35 (`const VERSION = '1.7.8.11'`)
+- Why: production stability
+- Impact: missing security patch 1.7.8.12; PHP 7.4 reached EOL 2022-11-28 (no upstream security fixes); core 1.7.x EOL since 2021-10
+- Fix: apply 1.7.8.12 patch first (minor), then plan migration to PrestaShop 8.x or 9.x; PHP 8.2 already in runtime — verify all modules under PHP 8.x compat-checker
+
+**`iadmin/autoupgrade/` contains `prestashop_1.7.5.1.zip` left in webroot — MEDIUM**
+- Why: artifact of a past upgrade attempt
+- Impact: downloadable archive exposing core source paths
+- Fix: delete
+
+## Performance
+
+**Custom cross-sell module clean — no concerns**
+- File: `modules/crosssellpro/crosssellpro.php`
+- Uses `Product::getAccessoriesLight()` (efficient) + `Product::getProductsProperties()`; filters with `Shop::addSqlAssociation`; respects `DEFAULT_LIMIT = 12`
+- No N+1 detected
+
+**No application-level caching active**
+- Memcached configured but disabled (`app/config/parameters.php`)
+- Page cache module installed (`modules/pagecache/`) — verify it is enabled in production
+- Fix: enable Memcached + verify pagecache configuration; add OPcache validation if not on
+
+## Fragile Areas
+
+**Override chain in `override/classes/`**
+- Why fragile: `Cart::getOrderTotal()` extension depends on `ets_promotion` module presence (`override/classes/Cart.php` line ~55); `Hook::exec` extension depends on `pagecache` (`override/classes/Hook.php` line ~42)
+- Common failures: removing one of those modules without cleaning up the override leaves dead branches
+- Safe modification: read the override + the referenced module's hook surface together; never edit core `classes/` directly
+
+**Recurring header-modification errors in admin**
+- Source: `iadmin/errors.log` shows repeated errors at `AdminCartRulesController.php:121` (output before `header()` call)
+- Trigger: editing cart rules in back-office
+- Likely cause: a module emitting whitespace before `<?php` or echoing in `init()`
+
+**MySQL "server has gone away" entries**
+- Source: `iadmin/errors.log`
+- Likely cause: long-running scripts (`import-product.php`?) exceeding `wait_timeout`
+- Fix: increase `wait_timeout`/`mysql.connect_timeout` in PHP config or call `Db::getInstance()->disconnect()` + reconnect for long-running batch jobs
+
+## Test Coverage Gaps
+
+- **Custom modules have NO tests** — `modules/crosssellpro/`, `modules/caraty/`
+- **No tests for `import-product.php`** (highest-risk script — bulk DB writes, no rollback)
+- **No tests for `buy-by-phone.php`** (handles user input + sends mail)
+- Risk: regressions land silently; manual browser testing is the only safety net
+
+## Documentation Gaps
+
+- No `README.md` at repo root (only PrestaShop's stock files)
+- 31 core overrides — none have purpose comments
+- `iadmin/errors.log` retains 26 months of error history but no rotation policy
+
+## Deletion Candidates (Quick Wins)
+
+```
+diag_20792_tmp.php
+diag_fix_tmp.php
+diag_upload_tmp.php
+info.php
+errors.log
+backup_before_patch/
+iadmin/errors.log
+iadmin/autoupgrade/prestashop_1.7.5.1.zip
+themes/classic.zip   # (verify needed)
+themes/leo_gstore.zip
+```
+
+Add to `.gitignore`: `errors.log`, `*.log`, `*.backup`, `backup_before_patch/`, `iadmin/backups/`, `iadmin/errors.log`, `iadmin/autoupgrade/*.zip`.
+
+---
+
+*Concerns audit: 2026-05-10*
+*Update as issues are fixed or new ones discovered*