docs(codebase): mapa kodu wygenerowana przez /paul:map-codebase
7 dokumentów w .paul/codebase/ — overview, stack, architecture, conventions, testing, integrations, concerns (CRITICAL→LOW). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
149
.paul/codebase/concerns.md
Normal file
149
.paul/codebase/concerns.md
Normal file
@@ -0,0 +1,149 @@
|
||||
# Technical Debt & Concerns
|
||||
|
||||
> Generated: 2026-04-26 | Prioritized by severity
|
||||
|
||||
## CRITICAL
|
||||
|
||||
### C1 — Unserialize on User-Controlled Cookies
|
||||
**File**: `admin/ajax/pages.php` lines 36, 49
|
||||
**Code**: `$array = unserialize($_COOKIE['cookie_menus']);`
|
||||
**Risk**: Object injection / RCE — classic PHP vulnerability.
|
||||
**Fix**: Replace with `json_decode($_COOKIE['cookie_menus'] ?? '{}', true)`.
|
||||
|
||||
### C2 — Path Traversal in Update File Deletion
|
||||
**File**: `autoload/admin/factory/class.Update.php` lines 76-80, 119-128
|
||||
**Code**: `unlink('../' . $filePath)` — `$filePath` from JSON manifest, not validated.
|
||||
**Risk**: Attacker-controlled manifest could delete arbitrary files.
|
||||
**Fix**:
|
||||
```php
|
||||
$full = realpath('../' . $filePath);
|
||||
$base = realpath('../');
|
||||
if (strpos($full, $base) !== 0) throw new \Exception('Path traversal');
|
||||
unlink($full);
|
||||
```
|
||||
|
||||
### C3 — God Class: Helpers.php (1220 lines, 75+ static methods)
|
||||
**File**: `autoload/Shared/Helpers/Helpers.php`
|
||||
**Risk**: Unmaintainable, untestable, global state dependency (`global $mdb, $settings, $lang`).
|
||||
**Domains mixed**: image processing, HTML DOM, caching, SEO, authentication, dates, session.
|
||||
**Fix**: Extract into focused service classes (`ImageService`, `SeoHelper`, `DateHelper`, etc.).
|
||||
|
||||
---
|
||||
|
||||
## HIGH
|
||||
|
||||
### H1 — Direct Superglobal Access Without Validation
|
||||
**File**: `autoload/Shared/Helpers/Helpers.php` lines 25-26
|
||||
**Code**: `$crop_w = $_GET['c_w'];` — no isset, no type check.
|
||||
**Also**: `admin/ajax/pages.php` lines 36, 49 — `\S::get()` passed directly to queries.
|
||||
**Fix**: Centralized request wrapper with typed getters.
|
||||
|
||||
### H2 — SQL String Concatenation (String Values)
|
||||
**File**: `autoload/Domain/Articles/ArticlesRepository.php` lines 53, 68, 87 and others.
|
||||
**Code**: `"... WHERE article_id = " . (int)$id` — integer cast OK, but pattern is dangerous for string params.
|
||||
**Fix**: Use Medoo parameterized methods exclusively. Audit and replace all raw `query()` calls.
|
||||
|
||||
### H3 — No Input Validation / Sanitization Layer
|
||||
**All entry points** — no `Validator` or `Sanitizer` class. Values flow from `$_GET`/`$_POST` → repository without validation.
|
||||
**Fix**: Add validation at control layer before delegation to factory/repository.
|
||||
|
||||
### H4 — Password Hash in Cookie
|
||||
**File**: `admin/index.php` lines 59-61
|
||||
**Code**: `$obj = json_decode($_COOKIE[$cookie_name]); $password = $obj->{'hash'};`
|
||||
**Risk**: Cookie exposure leaks credential hash, no HMAC signing.
|
||||
**Fix**: Use signed JWT or HMAC-signed remember-me token, never store hashes in cookies.
|
||||
|
||||
### H5 — Update Download Without Signature Verification
|
||||
**File**: `autoload/admin/factory/class.Update.php` lines 12, 25, 28
|
||||
**Code**: `file_get_contents('https://www.cmspro.project-dc.pl/updates/...')`
|
||||
**Risk**: MITM, supply chain — ZIP extracted without verifying integrity beyond SHA256 (if present).
|
||||
**Fix**: Verify SHA256 checksum server-side before extraction; use curl with `CURLOPT_SSL_VERIFYPEER`.
|
||||
|
||||
### H6 — Deprecated `mime_content_type()` Removed in PHP 8.1
|
||||
**File**: `autoload/Shared/Helpers/Helpers.php` line 39
|
||||
**Fix**:
|
||||
```php
|
||||
$finfo = finfo_open(FILEINFO_MIME_TYPE);
|
||||
$type = finfo_file($finfo, $file);
|
||||
finfo_close($finfo);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## MEDIUM
|
||||
|
||||
### M1 — Global Variables as Dependency Injection
|
||||
**Files**: Factory classes (`global $mdb`, `global $user`), Helpers (`global $settings, $lang`).
|
||||
**Risk**: Untestable, tightly coupled, order-dependent initialization.
|
||||
**Fix**: Pass `$mdb` to factories/repositories directly; remove `global` from repository code.
|
||||
|
||||
### M2 — Repository Classes Contain Business Logic and Side Effects
|
||||
**File**: `autoload/Domain/Articles/ArticlesRepository.php` line 45, 59
|
||||
**Code**: `\S::delete_cache()` and `\S::seo()` called inside repository methods.
|
||||
**Fix**: Repositories should only do DB operations; call side effects in factories/services.
|
||||
|
||||
### M3 — Mixed Procedural + OOP AJAX Handlers
|
||||
**Files**: `admin/ajax/pages.php`, `admin/ajax/articles.php`, `admin/ajax/users.php`
|
||||
**Pattern**: 50-90 line `if ($a == '...')` chains, no routing abstraction.
|
||||
**Fix**: Create `AjaxRouter` + controller base class.
|
||||
|
||||
### M4 — No Request/Response Abstraction
|
||||
**All entry points** — `$_GET`/`$_POST` accessed directly everywhere.
|
||||
**Fix**: `Request` class (typed getters) + `JsonResponse` class.
|
||||
|
||||
### M5 — Error Suppression with `@` Operator
|
||||
**Files**: `admin/index.php` lines 2, 14; Helpers.php lines 40, 98, 111, 1188-1200
|
||||
**Code**: `@file_get_contents(...)`, `@unlink(...)`.
|
||||
**Fix**: Use `if (file_exists())` guards and proper try/catch.
|
||||
|
||||
### M6 — Uninitialized Variables
|
||||
**File**: `autoload/Domain/Articles/ArticlesRepository.php` line 72
|
||||
**Code**: `if ($out == '')` — `$out` never declared.
|
||||
**Fix**: `$out = '';` before the loop.
|
||||
|
||||
### M7 — No Interface Contracts for Repositories
|
||||
All 10 repositories share identical method signatures but no shared interface.
|
||||
**Fix**: Define `RepositoryInterface` with `find()`, `all()`, `save()`, `delete()`.
|
||||
|
||||
### M8 — Hardcoded Values
|
||||
- Update base URL: `'https://www.cmspro.project-dc.pl/updates/'` in 3 files
|
||||
- File permissions: `chmod(..., 0755)` in 25 places
|
||||
- Cookie expiry: `time() + 3600 * 24 * 365` as magic number
|
||||
**Fix**: Extract to constants in a config class.
|
||||
|
||||
---
|
||||
|
||||
## LOW
|
||||
|
||||
### L1 — Backup Files in Repository
|
||||
`libraries/medoo/medoo.bck.php` (973 lines), `libraries/grid/gdb.min.bck.php` (957 lines).
|
||||
**Fix**: Delete; use Git for history.
|
||||
|
||||
### L2 — `test.php` in Project Root (700 lines)
|
||||
Production benchmark/test script accessible via HTTP. Contains DB credentials in lines 15-17.
|
||||
**Fix**: Remove or move to `tests/` with `.htaccess` protection.
|
||||
|
||||
### L3 — Legacy `class.S.php` Wrapper
|
||||
200+ calls to `\S::*` throughout codebase — double indirection through `__callStatic`.
|
||||
**Fix**: Gradual rename campaign to `\Shared\Helpers\Helpers::*`.
|
||||
|
||||
### L4 — Legacy SQL Update Fallback Format
|
||||
`class.Update.php` lines 97-132 — parses old `_sql.txt` format alongside new JSON manifest.
|
||||
**Fix**: Deprecate and remove once all deployments are on manifest format.
|
||||
|
||||
### L5 — Update Process Without Rollback
|
||||
SQL runs before file extraction. If extraction fails, DB is inconsistent. No transaction wrapping.
|
||||
**Fix**: Wrap SQL in transaction; extract files first, then run SQL; add rollback on failure.
|
||||
|
||||
---
|
||||
|
||||
## Files Needing Immediate Attention
|
||||
|
||||
| File | Lines | Issue |
|
||||
|------|-------|-------|
|
||||
| `autoload/Shared/Helpers/Helpers.php` | 1220 | God class (C3) |
|
||||
| `autoload/admin/factory/class.Update.php` | 157 | Path traversal (C2), supply chain (H5) |
|
||||
| `admin/ajax/pages.php` | ~90 | Unserialize (C1), missing validation (H1) |
|
||||
| `admin/index.php` | — | Password hash in cookie (H4) |
|
||||
| `autoload/Domain/Articles/ArticlesRepository.php` | 648 | Side effects in repo (M2), raw SQL (H2) |
|
||||
| `test.php` | 700 | Remove from root (L2) |
|
||||
Reference in New Issue
Block a user