UPDATE
This commit is contained in:
127
.paul/codebase/concerns.md
Normal file
127
.paul/codebase/concerns.md
Normal file
@@ -0,0 +1,127 @@
|
||||
# Concerns & Technical Debt
|
||||
|
||||
> Last updated: 2026-03-12
|
||||
|
||||
## Security Issues
|
||||
|
||||
### HIGH — Sensitive data logged to public file
|
||||
**File**: `autoload/front/Controllers/ShopOrderController.php:32`
|
||||
```php
|
||||
file_put_contents('tpay.txt', print_r($_POST, true) . print_r($_GET, true), FILE_APPEND);
|
||||
```
|
||||
- Logs entire POST/GET (including payment data) to `tpay.txt` likely in webroot
|
||||
- Possible information disclosure
|
||||
- **Fix**: Remove log or write to non-public path (e.g., `/logs/`)
|
||||
|
||||
### HIGH — Hardcoded payment seed
|
||||
**File**: `autoload/front/Controllers/ShopOrderController.php:105`
|
||||
```php
|
||||
hash("sha256", "ProjectPro1916;" . round($summary_tmp, 2) ...)
|
||||
```
|
||||
- Hardcoded secret in source — should be in `config.php`
|
||||
|
||||
### MEDIUM — SQL table name interpolated
|
||||
**File**: `autoload/Domain/Integrations/IntegrationsRepository.php:31`
|
||||
```php
|
||||
$stmt = $this->db->query("SELECT * FROM $table");
|
||||
```
|
||||
- Technically mitigated by whitelist in `settingsTable()`, but violates "no SQL string concatenation" rule
|
||||
- **Fix**: Use Medoo's native `select()` method
|
||||
|
||||
### MEDIUM — Path traversal in unlink()
|
||||
**Files**: `autoload/Domain/Product/ProductRepository.php:1605,1617,2129,2163` and `autoload/Domain/Article/ArticleRepository.php:321,340,823,840`
|
||||
```php
|
||||
if (file_exists('../' . $row['src'])) {
|
||||
unlink('../' . $row['src']);
|
||||
}
|
||||
```
|
||||
- Path from DB, no traversal check
|
||||
- A DB compromise could delete arbitrary files
|
||||
- **Fix**:
|
||||
```php
|
||||
$basePath = realpath('../upload/');
|
||||
$fullPath = realpath('../' . $row['src']);
|
||||
if ($fullPath && strpos($fullPath, $basePath) === 0) {
|
||||
unlink($fullPath);
|
||||
}
|
||||
```
|
||||
|
||||
### MEDIUM — Unsanitized output in templates
|
||||
**Files**:
|
||||
- `templates/articles/article-full.php` — article title and `$_SERVER['SERVER_NAME']` concatenated without escaping
|
||||
- `templates/articles/article-entry.php` — `$url` and article titles not escaped
|
||||
|
||||
### MEDIUM — Missing CSRF tokens
|
||||
- No evidence of CSRF tokens on admin panel forms
|
||||
- State-changing POST endpoints (create/update/delete) are potentially CSRF-vulnerable
|
||||
|
||||
---
|
||||
|
||||
## Architecture Issues
|
||||
|
||||
### IntegrationsRepository too large (875 lines)
|
||||
**File**: `autoload/Domain/Integrations/IntegrationsRepository.php`
|
||||
Does too many things: settings CRUD, logging, Apilo OAuth, product sync, webhook handling, ShopPRO import.
|
||||
**Suggested split**: `ApiloAuthManager`, `ApiloProductSyncService`, `ApiloWebhookHandler`, `IntegrationLogRepository`, `IntegrationSettingsRepository`
|
||||
|
||||
### ProductRepository too large (3583 lines)
|
||||
**File**: `autoload/Domain/Product/ProductRepository.php`
|
||||
Candidate for extraction of: pricing logic, image handling, cache management, Google feed generation.
|
||||
|
||||
### ShopProductController too large (1199 lines)
|
||||
**File**: `autoload/admin/Controllers/ShopProductController.php`
|
||||
|
||||
### Helpers.php too large (1101 lines)
|
||||
**File**: `autoload/Shared/Helpers/Helpers.php`
|
||||
Static utility god class. Extract into focused service classes.
|
||||
|
||||
### Duplicate email logic
|
||||
- `\Shared\Helpers\Helpers::send_email()` and `\Shared\Email\Email::send()` both wrap PHPMailer
|
||||
- Should be unified in `\Shared\Email\Email`
|
||||
- Documented in `docs/MEMORY.md`
|
||||
|
||||
### 47 `global $mdb` usages remain
|
||||
- DI is complete in Controllers, but some Helpers methods still use `global $mdb`
|
||||
- Should be gradually eliminated
|
||||
|
||||
---
|
||||
|
||||
## Dead Code / Unused Files
|
||||
|
||||
| File | Issue |
|
||||
|------|-------|
|
||||
| `libraries/rb.php` | RedBeanPHP — no references found in autoload, candidate for removal |
|
||||
| `cron-turstmate.php` (note: typo) | Legacy/questionable cron handler |
|
||||
| `devel.html` | Development artifact in project root |
|
||||
| `output.txt` | Artifact file |
|
||||
| `libraries/filemanager-9.14.1/` + `9.14.2/` | Duplicate versions |
|
||||
|
||||
---
|
||||
|
||||
## Missing Error Handling
|
||||
|
||||
- `IntegrationsRepository.php:163-165` — DB operations after Apilo token refresh lack try-catch
|
||||
- `ShopOrderController.php:32` — `file_put_contents()` return value not checked
|
||||
- `ProductRepository.php:1605` — `unlink()` without error handling
|
||||
- `cron.php:2` — `error_reporting(E_ALL ^ E_NOTICE ^ E_STRICT ^ E_WARNING ^ E_DEPRECATED)` silences all warnings, hiding potential bugs
|
||||
|
||||
---
|
||||
|
||||
## Known Issues (from docs/TODO.md & docs/MEMORY.md)
|
||||
|
||||
| Issue | Location | Status |
|
||||
|-------|----------|--------|
|
||||
| Newsletter save/unsubscribe needs testing | `Domain/Newsletter/` | Open |
|
||||
| Duplicate email sending logic | `Helpers.php` vs `Email.php` | Open |
|
||||
| `$mdb->delete()` 2-arg pitfall | Documented in MEMORY.md | Known pitfall |
|
||||
|
||||
---
|
||||
|
||||
## Summary by Priority
|
||||
|
||||
| Priority | Count | Key Action |
|
||||
|----------|-------|-----------|
|
||||
| **Immediate** (security) | 5 | Remove tpay.txt logging, fix path traversal, move hardcoded secret to config |
|
||||
| **High** (architecture) | 3 | Split IntegrationsRepository, unify email logic, add CSRF |
|
||||
| **Medium** (quality) | 4 | Escape template output, add try-catch, remove dead files |
|
||||
| **Low** (maintenance) | 3 | Remove rb.php, reduce Helpers.php, document helpers usage |
|
||||
Reference in New Issue
Block a user