sides/AUDIT_REPORT.md

# AUDIT REPORT — SIDES (tckdev)

**Date**: 2026-05-20
**Auditor**: Automated Code Review
**Scope**: Full brownfield codebase audit

---

## Executive Summary

SIDES is a flood early warning system built on Laravel 12 + PostgreSQL + Docker. The application is **functional for its purpose** but has significant security vulnerabilities, code quality issues, and architectural concerns that should be addressed before any further development or production deployment.

**Risk Level: HIGH**

---

## 1. CRITICAL — Security Vulnerabilities

### 1.1 SQL Injection (HIGH)

**Multiple controllers** build raw SQL queries with string interpolation instead of parameterized queries.

**`RainfallController::index()` (line 46):**
```php
$stationCondition = " AND s.stationid = '{$stationFilter}'";
```

**`WaterLevelController::index()` (lines 30, 37):**
```php
$stationCondition = " WHERE s.stationid = '{$stationFilter}' ";
$dateCondition = " AND w.datetime = '{$sqlDate}' ";
```

**`RainfallController::index()` (lines 55-106):**
```php
$rainfallData = collect(DB::select("
    SELECT ... CAST('$displayDate' AS timestamp) ...
    ... CAST('$dateFilter' as date) - INTERVAL '6 days' ...
    $stationCondition
"));
```

User input (`$stationFilter`, `$dateFilter`, `$displayDate`, `$sqlDate`) is interpolated directly into SQL strings. An attacker could inject SQL through these parameters.

**Impact**: Full database compromise (read, modify, delete data)
**Files affected**: `RainfallController.php`, `WaterLevelController.php`
**Fix**: Use parameterized queries with `?` placeholders or named bindings (`:param`) consistently.

### 1.2 Hardcoded Credentials in Source Code (HIGH)

**`autoscript/sidesdecode.py` (lines 22-31):**
```python
ftp_server = "myvscada.com"
ftp_username = "tck"
ftp_password = "tck6789"
pg_host = "192.168.0.211"
pg_database = "sides_db"
pg_user = "tck"
pg_password = "projectdev##1"
```

**`src/.env` (committed to git):**
```
MAIL_PASSWORD="ipmu zifw bpmf fsyp"
POSTGRES_PASSWORD="projectdev##1"
PGADMIN_PASSWORD="projectdev##1"
```

**`src/storage/app/firebase/sides-b4abb-3604a7cf7584.json`** — Firebase service account key is committed to the repository.

**Impact**: Anyone with repository access has all credentials
**Fix**: Move to environment variables, add `.env` to `.gitignore`, rotate all exposed credentials

### 1.3 Default Admin Credentials (HIGH)

**`DatabaseSeeder.php` and migration `2025_12_11_124201`:**
```php
'name' => 'admin',
'password' => Hash::make('password123'),
```

**Impact**: Anyone who discovers this gets admin access
**Fix**: Remove hardcoded credentials, require admin setup on first run, use strong passwords

### 1.4 API Endpoints Unauthenticated (MEDIUM)

All API endpoints except `/api/login` have **no authentication**:
```php
Route::get('/station/current', [StationController::class, 'getCurrentData']);
Route::get('/station/rainfall', [StationController::class, 'getRainfallData']);
// ... all publicly accessible
```

**`/api/alert`** is publicly accessible — anyone can trigger push notifications:
```php
Route::post('/alert',[AlertController::class,'send']);
```

**Impact**: Data exfiltration, unauthorized push notification spam
**Fix**: Add API authentication (sanctum/passport tokens) to all endpoints

### 1.5 API Login Returns Password Hash (LOW)

**`Api\AuthController::login()` (line 23):**
```php
$user = DB::select("SELECT id, name, email, access_level, password FROM users ...");
```

While the full `$user` object isn't returned, the query fetches the password hash unnecessarily.

**Fix**: Remove `password` from the SELECT query.

---

## 2. HIGH — Code Quality Issues

### 2.1 No Eloquent ORM Usage

The entire application uses `DB::table()` and `DB::select()` (query builder / raw SQL) instead of Eloquent models. Only the `User` model exists.

**Impact**: 
- No type safety, no attribute casting, no relationship definitions
- Difficulty maintaining and extending the codebase
- No model-level validation or events

**Fix**: Create Eloquent models for `Station`, `Rainfall`, `WaterLevel`, `Siren`, `Notification` with proper relationships.

### 2.2 No Database Foreign Keys

All table relationships (`stationid` references) are maintained at the application level with no database constraints:

```php
// Migration: no foreign key
$table->string('stationid');
// vs what it should be:
$table->foreign('stationid')->references('stationid')->on('station');
```

**Impact**: Data integrity cannot be guaranteed at the database level. Orphaned records possible.

### 2.3 Missing Database Indexes

No indexes exist on frequently queried columns:
- `rainfall.stationid` — used in every rainfall query
- `rainfall.timestamp` — used for date filtering and max() queries
- `waterlevel.stationid` — used in every water level query
- `waterlevel.datetime` — used for date filtering
- `siren.stationid` — used in every siren query
- `siren.active_time` — used for max() and ordering
- `notification.stationid` — used in every notification query
- `notification.timestamp` — used for date filtering

**Impact**: Query performance degrades significantly as data grows
**Fix**: Add composite indexes on `(stationid, timestamp)` for data tables

### 2.4 Duplicate Admin User Creation

Admin user is created in **both** the DatabaseSeeder AND a migration:

- `DatabaseSeeder.php` line 22: `User::create(['name' => 'Admin', ...])`
- Migration `2025_12_11_124201`: `DB::table('users')->insert(['name' => 'admin', ...])`

**Impact**: Running `migrate:fresh --seed` will create two admin users with different names ("Admin" and "admin")
**Fix**: Remove one — keep only the seeder.

### 2.5 Inconsistent Naming Conventions

| Issue | Example |
|-------|---------|
| Case inconsistency | `cctvController` (lowercase), `SirenHistory` (PascalCase) |
| Mixed route naming | `sirenhistory` vs `historicalrf` vs `historicalwl` |
| Mixed table naming | `waterlevel.datetime` vs `rainfall.timestamp` (same concept, different column names) |
| Variable typos | `$dateTImeFilter` (capital I), `$dateFilter` |

### 2.6 No Input Validation on Some Endpoints

`MapController::getStations()` and `MapController::getCurrentData()` accept no parameters but other controllers accept GET/POST parameters with inconsistent validation.

### 2.7 Commented-Out Code Blocks

Multiple large blocks of commented-out code throughout:
- `RainfallController.php` lines 250-263 (alternative graph query)
- `WaterLevelController.php` date conditions
- `sidesdecode.py` file management functions

---

## 3. MEDIUM — Architectural Concerns

### 3.1 N+1 Query Pattern in MapController

`MapController::getCurrentData()` uses subqueries in JOINs for latest data:

```php
->whereRaw('r.timestamp = (SELECT MAX(timestamp) FROM rainfall WHERE stationid = s.stationid)')
```

This executes a subquery for every station for every table join (3 subqueries per station). Should use a CTE or window function.

### 3.2 No Caching

Real-time data queries (dashboard, map, station listings) run complex SQL on every page load with no caching layer despite data only updating when the Python script runs.

### 3.3 No Form Request Classes

Validation is done inline in controllers despite Laravel's Form Request feature. Only `LoginRequest` and `ProfileUpdateRequest` exist from Breeze scaffolding.

### 3.4 No API Token Authentication

The API login endpoint validates credentials but returns no token. This makes the API unusable for stateless/mobile clients that need persistent authentication.

### 3.5 FCM Topic Fixed

`AlertController::send()` always sends to `FCM_TOPIC_RAINFALL_WARNING` regardless of alert type:

```php
$topic = env('FCM_TOPIC_RAINFALL_WARNING');
```

Water level and siren alerts go to the rainfall topic, not their respective topics.

### 3.6 Water Level Alert Bug

**`sidesdecode.py` line 378:**
```python
send_alert_to_laravel(station_id, level, 1)  # stationtype=1 (rainfall) instead of 2 (water level)
```

Water level threshold alerts are sent with `stationtype=1` instead of `stationtype=2`, causing them to be classified as rainfall alerts.

### 3.7 `is_blocked` Not Enforced

The `is_blocked` column exists on users but no middleware checks it. Blocked users can still log in and use the system.

### 3.8 No Rate Limiting

No rate limiting on login, API endpoints, or form submissions.

---

## 4. LOW — Minor Issues

### 4.1 `APP_DEBUG=true` in `.env`

Debug mode should be `false` in any non-local environment.

### 4.2 `database.db` and `database.sqlite` Committed

SQLite database files are in the repository root and `src/database/`.

### 4.3 Unused Packages

- `laravel/sail` — installed but Docker Compose is used directly
- `laravel/pail` — installed but no log streaming is used
- `nunomaduro/collision` — dev dependency, standard

### 4.4 `.env` Committed

Both `.env` files (root and `src/`) are committed to the repository. They should be in `.gitignore`.

### 4.5 `.editorconfig` Duplication

`.editorconfig` exists in both root and `src/` directories.

### 4.6 No `.gitignore` for Autoscript Logs

`autoscript/sidesdecode.log` and `sidesdecode_error.log` are tracked in git.

### 4.7 Mixed CSS Frameworks

Both Tailwind CSS and Bootstrap 5 are loaded on pages, creating potential style conflicts and unnecessary bundle size.

### 4.8 No Tests for Domain Logic

Only Breeze default tests exist (`AuthenticationTest`, `EmailVerificationTest`, etc.). No tests for domain controllers, data pipeline, or export logic.

---

## 5. Summary Table

| # | Issue | Severity | File(s) | Effort |
|---|-------|----------|---------|--------|
| 1.1 | SQL Injection | CRITICAL | RainfallController, WaterLevelController | Medium |
| 1.2 | Hardcoded credentials | CRITICAL | sidesdecode.py, .env | Low |
| 1.3 | Default admin password | CRITICAL | DatabaseSeeder, migration | Low |
| 1.4 | Unauthenticated API | HIGH | routes/api.php | Medium |
| 1.5 | Password hash in query | LOW | AuthController | Low |
| 2.1 | No Eloquent models | HIGH | All controllers | High |
| 2.2 | No foreign keys | HIGH | All migrations | Medium |
| 2.3 | Missing indexes | HIGH | All data tables | Medium |
| 2.4 | Duplicate admin creation | MEDIUM | Seeder + migration | Low |
| 2.5 | Inconsistent naming | LOW | Throughout | Medium |
| 3.1 | N+1 query pattern | MEDIUM | MapController | Medium |
| 3.2 | No caching | MEDIUM | Throughout | Medium |
| 3.5 | FCM topic hardcoded | MEDIUM | AlertController | Low |
| 3.6 | Wrong stationtype for WL | HIGH | sidesdecode.py:378 | Low |
| 3.7 | Blocked users not enforced | MEDIUM | Auth flow | Low |
| 3.8 | No rate limiting | MEDIUM | Throughout | Low |
| 4.1 | Debug mode on | LOW | .env | Low |
| 4.2 | DB files committed | LOW | Root | Low |
| 4.4 | .env committed | LOW | Root | Low |
| 4.7 | Mixed CSS frameworks | LOW | Blade templates | Medium |
| 4.8 | No domain tests | LOW | tests/ | High |

---

## 6. Recommended Priority

1. **Immediate** (before any further changes): Fix SQL injection, rotate credentials, remove `.env` from git
2. **Short-term**: Fix water level stationtype bug, add API auth, add database indexes, fix FCM topic routing
3. **Medium-term**: Create Eloquent models, add foreign keys, implement caching, add input validation
4. **Long-term**: Full test coverage, clean up naming conventions, choose single CSS framework