sides/.planning/codebase/CONCERNS.md

Codebase Concerns

Analysis Date: 2026-05-20

Security Concerns

CRITICAL: Firebase Service Account Key Committed to Storage

• Risk: The file src/storage/app/firebase/sides-b4abb-3604a7cf7584.json is a Firebase service account credential file present in the repository. It contains project_id: "sides-b4abb" and full private key data. src/.gitignore does not exclude storage/app/firebase/.
• Files: src/storage/app/firebase/sides-b4abb-3604a7cf7584.json, src/app/Services/FcmService.php (lines 17-19: reads this file at runtime via env('FIREBASE_CREDENTIALS'))
• Impact: Anyone with repo access has full Firebase Admin SDK privileges (send notifications, access Firestore, etc.)
• Fix: Move credentials to environment-only storage, add storage/app/firebase/*.json to .gitignore, rotate the exposed key.

CRITICAL: SQL Injection via String Interpolation in Raw Queries

• Risk: Multiple controllers build raw SQL queries by directly interpolating user input into SQL strings without parameterized bindings.
• Files:
  • src/app/Http/Controllers/RainfallController.php line 46: $stationCondition = " AND s.stationid = '{$stationFilter}'" — $stationFilter comes directly from $request->get('station') and is concatenated into DB::select() on line 55.
  • src/app/Http/Controllers/RainfallController.php lines 85-103: $dateFilter and $displayDate interpolated into CAST expressions.
  • src/app/Http/Controllers/WaterLevelController.php line 30: $stationCondition = " WHERE s.stationid = '{$stationFilter}' " — same pattern.
  • src/app/Http/Controllers/WaterLevelController.php line 37: $dateCondition = " AND w.datetime = '{$sqlDate}' ".
• Impact: An attacker can inject arbitrary SQL through the station or date query parameters, potentially extracting or modifying all data.
• Fix: Replace all DB::select() with parameterized queries using ? placeholders or named bindings, or use Eloquent/Query Builder with proper ->where() calls.

CRITICAL: Hardcoded Credentials in Autoscript

• Risk: autoscript/sidesdecode.py contains hardcoded FTP and PostgreSQL credentials in plain text.
• Files: autoscript/sidesdecode.py lines 22-31:
  • FTP: ftp_username = "tck", ftp_password = "tck6789", server myvscada.com
  • PostgreSQL: pg_host = "192.168.0.211", pg_user = "tck", pg_password = "projectdev##1"
• Impact: Credentials are exposed in source control. Internal IP address leaks network topology.
• Fix: Move all credentials to environment variables or a secrets manager. Add autoscript/ to .gitignore or restructure to load config from .env.

HIGH: API Routes Have No Authentication or Rate Limiting

• Risk: All API routes in src/routes/api.php are completely unauthenticated. The API endpoints expose real-time station data, historical readings, notification history, and siren status without any auth check.
• Files: src/routes/api.php lines 10-20 — no middleware('auth') or middleware('throttle') on any route. src/bootstrap/app.php line 11 — API routes registered without any middleware group.
• Impact: Anyone can query all station data, and the /api/alert endpoint can be abused to send fraudulent push notifications to all users.
• Fix: Add Route::middleware(['auth:sanctum']) to the API route group. At minimum, protect /api/alert with authentication.

HIGH: API Login Endpoint Returns User Details Without Token

• Risk: Api/AuthController.php login() validates credentials then returns user id, name, email, and access_level in a plain JSON response without issuing any authentication token or session.
• Files: src/app/Http/Controllers/Api/AuthController.php lines 49-55
• Impact: The login endpoint leaks user info but provides no mechanism for subsequent authenticated requests. No token-based auth system exists for the API.
• Fix: Implement Laravel Sanctum or Passport to issue API tokens on login. Remove password from the SELECT query (line 24).

HIGH: Hardcoded Default Admin Password in Migration

• Risk: Migration 2025_12_11_124201_add_default_user_to_users_table.php seeds an admin user with email admin@example.com and password password123.
• Files: src/database/migrations/2025_12_11_124201_add_default_user_to_users_table.php line 12-13
• Impact: If this migration runs in production, a trivially guessable admin account is created. There is no mechanism to force a password change.
• Fix: Use database seeders instead of migrations for seed data. Generate a random password from env vars. Add forced password change on first login.

MEDIUM: Admin Middleware Uses Loosely-Typed Access Level Check

• Risk: AdminMiddleware checks Auth::user()->access_level !== 1 using strict comparison but the database column is integer and the middleware does not verify the user model loaded correctly.
• Files: src/app/Http/Middleware/AdminMiddleware.php line 26
• Impact: The access_level field is not constrained by any foreign key or enum — any integer value is accepted in AdminController::storeUser().
• Fix: Use a proper role/permission system (e.g., spatie/laravel-permission). At minimum, validate access_level is in [1, 2] range.

MEDIUM: No Foreign Key Constraints on Data Tables

• Risk: The rainfall, waterlevel, notification, and siren tables all have stationid columns but no foreign key constraints to station.stationid. Data integrity cannot be enforced at the database level.
• Files: All migrations in src/database/migrations/2025_11_*
• Impact: Orphaned records possible if stations are deleted. AdminController::deleteStation() (line 405) deletes a station without cleaning related data.
• Fix: Add foreign key constraints with cascade delete to all child tables referencing station.stationid.

Technical Debt

No Eloquent Models for Domain Entities

• Issue: The entire application uses DB::table() and raw DB::select() instead of Eloquent ORM. The only Eloquent model is User. Station, Rainfall, WaterLevel, Notification, and Siren have no models.
• Files: All controllers in src/app/Http/Controllers/
• Impact: No model-level validation, no relationships, no accessors/mutators, no model events. Code is harder to test and maintain.
• Fix: Create Eloquent models for Station, Rainfall, WaterLevel, Notification, Siren. Define relationships and refactor controllers.

Dashboard Query Duplicated Across Three Controllers

• Issue: The same complex 4-table LEFT JOIN query (station + rainfall + waterlevel + siren) is copy-pasted in MapController::getCurrentData(), AuthenticatedSessionController::create(), and Api\StationController::getCurrentData().
• Files:
  • src/app/Http/Controllers/MapController.php lines 35-63
  • src/app/Http/Controllers/Auth/AuthenticatedSessionController.php lines 28-56
  • src/app/Http/Controllers/Api/StationController.php lines 13-43
• Impact: Any query change must be made in three places. Risk of divergence.
• Fix: Extract to a shared repository/service class or a dedicated model scope.

AdminController is a 427-line God Controller

• Issue: AdminController handles both station management and user management — two distinct domains. Contains 8 methods for CRUD on 2 different entities.
• Files: src/app/Http/Controllers/AdminController.php (427 lines)
• Impact: Hard to maintain. Violates Single Responsibility Principle.
• Fix: Split into Admin\StationController and Admin\UserController.

.env.example Mismatch with Actual Configuration

• Issue: src/.env.example is the default Laravel template with DB_CONNECTION=sqlite, but the application uses PostgreSQL via Docker. The Firebase env vars (FIREBASE_PROJECT_ID, FIREBASE_CREDENTIALS, FCM_TOPIC_RAINFALL_WARNING) are not documented in any .env.example.
• Files: src/.env.example (65 lines, default Laravel template)
• Impact: New developers cannot set up the project without reverse-engineering the code.
• Fix: Update .env.example with actual PostgreSQL connection details and all custom env vars.

Commented-Out Code Blocks

• Issue: Several files contain large blocks of commented-out code suggesting incomplete or abandoned features.
• Files:
  • src/app/Http/Controllers/RainfallController.php lines 250-263: commented-out alternative graph query
  • autoscript/sidesdecode.py lines 153-158, 489, 522-533: commented-out error handling and test functions
  • src/routes/web.php line 30: commented-out threshold graph route
  • src/app/Services/FcmService.php lines 27-30: commented-out alternative auth method
• Fix: Remove dead code. Use version control for history.

Performance Concerns

N+1 Subqueries in Station Data Queries

• Issue: The dashboard query uses correlated subqueries (SELECT MAX(timestamp) FROM rainfall WHERE stationid = s.stationid) for each of the 3 LEFT JOINs. For N stations, this results in ~3N additional subquery evaluations.
• Files:
  • src/app/Http/Controllers/MapController.php lines 37-46
  • src/app/Http/Controllers/Api/StationController.php lines 14-24
  • src/app/Http/Controllers/Auth/AuthenticatedSessionController.php lines 30-39
• Impact: Query performance degrades linearly with station count.
• Fix: Use window functions (ROW_NUMBER() OVER PARTITION BY) or pre-computed materialized views for latest readings.

No Database Indexes on Query-Critical Columns

• Issue: None of the migrations create indexes on columns used in WHERE, JOIN, or ORDER BY clauses.
• Missing indexes:
  • rainfall.stationid and rainfall.timestamp — used in every rainfall query
  • waterlevel.stationid and waterlevel.datetime — used in every waterlevel query
  • notification.stationid, notification.timestamp, notification.stationtype — used in all notification queries
  • siren.stationid and siren.active_time — used in all siren queries
• Files: src/database/migrations/2025_11_06_* and 2025_11_07_*
• Impact: Full table scans on every query. Performance will degrade significantly as data accumulates.
• Fix: Add index migrations for all foreign key columns and frequently queried timestamp columns.

No Caching for Dashboard/Map Data

• Issue: The dashboard query (the heaviest query in the app) runs on every page load with no caching. The same data powers the home page, login page, and dashboard.
• Files: src/app/Http/Controllers/MapController.php, src/app/Http/Controllers/Auth/AuthenticatedSessionController.php
• Impact: Database hit on every request for data that changes at most every few minutes.
• Fix: Cache dashboard data for 1-5 minutes using Cache::remember().

Heavy PDF Export Queries Without Pagination

• Issue: PDF export endpoints (exportHistoryRfPDF, exportHistoryWlPDF, exportHistorySirenPDF) load entire unbounded result sets with ->get().
• Files:
  • src/app/Http/Controllers/NotificationController.php lines 122-126, 143-147
  • src/app/Http/Controllers/SirenController.php lines 67-74
• Impact: Memory exhaustion with large datasets. No date filtering or row limits.
• Fix: Add date range filters and chunk processing for PDF generation.

Maintainability Concerns

Inconsistent Naming Conventions

• Issue: Controller names mix PascalCase and camelCase. The cctvController uses lowercase class name violating PSR-1/PSR-12.
• Files:
  • src/app/Http/Controllers/cctvController.php — class cctvController (should be CctvController)
  • src/app/Http/Controllers/SirenController.php method SirenHistory() — PascalCase method (should be sirenHistory)
  • src/app/Http/Controllers/NotificationController.php method SirenNotification() — same issue
• Fix: Rename to follow PSR standards: CctvController, camelCase methods.

Typo: "potrait" Instead of "portrait" in PDF Generation

• Issue: Three PDF export methods pass 'potrait' as the paper orientation. While DomPDF may not error, this is an invalid value that likely falls back to default.
• Files:
  • src/app/Http/Controllers/SirenController.php line 77
  • src/app/Http/Controllers/NotificationController.php lines 129, 150
• Fix: Change 'potrait' to 'portrait'.

No Test Coverage for Custom Application Logic

• Issue: Test files only contain Breeze defaults (auth tests). Zero tests cover custom controllers, API endpoints, or the data processing script.
• Files: src/tests/ — only default Laravel/Breeze tests exist
• Impact: Any refactoring or fix risks introducing regressions with no safety net.
• Fix: Add feature tests for all API endpoints, admin CRUD operations, and PDF export endpoints.

Dependency Concerns

google/auth Used Directly Instead of Official Firebase SDK

• Issue: The project uses google/auth package directly with manual HTTP calls to FCM instead of the official kreait/laravel-firebase SDK.
• Files: src/app/Services/FcmService.php, src/composer.json line 12: "google/auth": "^1.49"
• Impact: More boilerplate, manual token management, no automatic retry or error handling.
• Fix: Replace with kreait/laravel-firebase for cleaner integration.

Docker Image Not Pinned, composer:2.3 Outdated

• Issue: Dockerfile copies Composer from composer:2.3 image (line 62) which is significantly behind current stable. postgres image in docker-compose.yml has no version tag (line 26: image: postgres).
• Files: Dockerfile line 62, docker-compose.yml line 26
• Impact: Unpredictable builds. Breaking changes when pulling latest postgres image.
• Fix: Pin Composer to composer:2 or latest. Pin Postgres to a specific major version (e.g., postgres:16-alpine).

Infrastructure Concerns

PostgreSQL Data Exposed on Port 5432

• Issue: docker-compose.yml maps Postgres port 5432 directly to the host. Both pgAdmin (5050) and Adminer (6060) are also exposed in what appears to be a production configuration.
• Files: docker-compose.yml lines 34-35, 61, 74
• Impact: Database and management tools accessible from the network without additional protection.
• Fix: Remove port mappings for production. Use Docker networks only. Adminer should not be in production compose.

No Health Checks in Docker Services

• Issue: None of the Docker services define healthcheck blocks. The app service uses depends_on: postgres without a health condition, so it may start before Postgres is ready.
• Files: docker-compose.yml
• Fix: Add health checks for postgres and configure app with depends_on.postgres.condition: service_healthy.

src/ Directory Volume-Mounted but Also COPY'd in Dockerfile

• Issue: Dockerfile COPYs src/ into the image (lines 72-75), but docker-compose.yml also mounts ./src:/var/www/html as a volume (line 16). The COPY is wasted and creates confusion.
• Files: Dockerfile lines 72-75, docker-compose.yml line 16
• Fix: Remove COPY from Dockerfile for development (volume mount handles it). Create a separate production Dockerfile that COPYs code.

Observability

No Structured Logging

• Issue: The application uses Laravel's default file logging (LOG_CHANNEL=stack, LOG_STACK=single). No centralized logging, no log levels differentiated by environment.
• Files: src/.env.example lines 18-21
• Impact: Logs are local to the container and lost on restart. No way to search or alert on errors.
• Fix: Configure LOG_CHANNEL=stderr for Docker, or integrate with a log aggregation service.

No Error Tracking Service

• Issue: No error tracking integration (Sentry, Bugsnag, etc.) detected. Exceptions are caught locally and returned as flash messages in some controllers but silently swallowed in others.
• Files: src/app/Http/Controllers/AdminController.php lines 216-218: $e->getMessage() returned to user.
• Impact: Production errors invisible to developers. Raw exception messages may leak internal details to users.
• Fix: Integrate an error tracking service. Never return raw $e->getMessage() to end users.

No Monitoring or Alerting

• Issue: No monitoring tools, uptime checks, or alerting configuration detected. The application has a /up health endpoint (from Laravel 12) but nothing monitors it.
• Impact: Downtime or degraded performance goes undetected until users report it.
• Fix: Set up uptime monitoring for the /up endpoint. Add APM for performance tracking.

---

Concerns audit: 2026-05-20