From 9f5142b0718a9159fbd48d1f5ff7297dbf758fa2 Mon Sep 17 00:00:00 2001
From: Marco0300 <Marco0300@users.noreply.github.com>
Date: Tue, 1 Sep 2026 18:58:18 +0200
Subject: [PATCH] fix: harden sessions and domain validation

---
 README.md                             |  2 +-
 app/Domain/Jobcard/TimeAggregator.php |  2 +-
 app/Domain/Jobcard/TimeCalculator.php |  2 +-
 app/Domain/SLA/SlaCalculator.php      |  3 +++
 config/bootstrap.php                  |  7 ++++++-
 public/index.php                      | 30 +++++++++++++++++----------
 tests/JobcardCalculationsTest.php     |  3 +++
 tests/JobcardDomainTest.php           |  2 +-
 tests/SlaDomainTest.php               |  5 +++++
 9 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/README.md b/README.md
index 9d1bfd8..5a79c64 100644
--- a/README.md
+++ b/README.md
@@ -33,4 +33,4 @@ find app config database public -type f -name '*.php' -print0 | xargs -0 -n1 php
 - Do not commit `.env` or production credentials.
 - Set `APP_KEY` to a long random value and keep it in a secrets manager in production.
 - Credential vault encryption and the remaining domain modules are scheduled in later phases.
-- The initial schema is intentionally migration-ready but is delivered as an idempotent bootstrap SQL file for the first Docker increment.
+- The initial schema is delivered as a Docker bootstrap SQL file. Apply it once to a new database; later releases should use versioned migrations.
diff --git a/app/Domain/Jobcard/TimeAggregator.php b/app/Domain/Jobcard/TimeAggregator.php
index 102dbf0..14d7aea 100644
--- a/app/Domain/Jobcard/TimeAggregator.php
+++ b/app/Domain/Jobcard/TimeAggregator.php
@@ -22,7 +22,7 @@ final class TimeAggregator
     {
         return $this->total(array_values(array_filter(
             $entries,
-            static fn ($entry): bool => is_array($entry) && (($entry['counts_toward_sla'] ?? true) === true)
+            static fn ($entry): bool => is_array($entry) && in_array($entry['counts_toward_sla'] ?? true, [true, 1, '1'], true)
         )));
     }
 }
diff --git a/app/Domain/Jobcard/TimeCalculator.php b/app/Domain/Jobcard/TimeCalculator.php
index 1844f07..cefcf22 100644
--- a/app/Domain/Jobcard/TimeCalculator.php
+++ b/app/Domain/Jobcard/TimeCalculator.php
@@ -4,7 +4,7 @@ declare(strict_types=1);
 function calculate_duration_hours(?string $start, ?string $end, ?float $manualHours = null): ?float
 {
     if ($manualHours !== null) {
-        return $manualHours >= 0 ? round($manualHours, 2) : null;
+        return $manualHours > 0 ? round($manualHours, 2) : null;
     }
     if ($start === null || $end === null || !preg_match('/^\d{2}:\d{2}$/', $start) || !preg_match('/^\d{2}:\d{2}$/', $end)) {
         return null;
diff --git a/app/Domain/SLA/SlaCalculator.php b/app/Domain/SLA/SlaCalculator.php
index fd6f929..ef2ddff 100644
--- a/app/Domain/SLA/SlaCalculator.php
+++ b/app/Domain/SLA/SlaCalculator.php
@@ -3,6 +3,9 @@ declare(strict_types=1);
 
 function calculate_sla_usage(float $allocatedHours, array $hours): array
 {
+    if ($allocatedHours < 0) {
+        throw new InvalidArgumentException('Allocated hours must not be negative.');
+    }
     $used = round(array_sum(array_map(static fn ($value): float => max(0.0, (float)$value), $hours)), 2);
     $remaining = round(max(0.0, $allocatedHours - $used), 2);
     $percentage = $allocatedHours > 0 ? round(($used / $allocatedHours) * 100, 2) : ($used > 0 ? 100.0 : 0.0);
diff --git a/config/bootstrap.php b/config/bootstrap.php
index 5d3182f..72e1dfc 100644
--- a/config/bootstrap.php
+++ b/config/bootstrap.php
@@ -35,9 +35,14 @@ function csrf_token(): string
     return $_SESSION['csrf'];
 }
 
+function scalar_input(mixed $value, string $default = ''): string
+{
+    return is_scalar($value) ? (string)$value : $default;
+}
+
 function verify_csrf(): void
 {
-    $provided = (string)($_POST['_csrf'] ?? '');
+    $provided = scalar_input($_POST['_csrf'] ?? null);
     if (!hash_equals((string)($_SESSION['csrf'] ?? ''), $provided)) {
         http_response_code(419);
         exit('Invalid CSRF token');
diff --git a/public/index.php b/public/index.php
index a543e5d..30331b9 100644
--- a/public/index.php
+++ b/public/index.php
@@ -3,8 +3,11 @@ declare(strict_types=1);
 
 require_once __DIR__ . '/../config/bootstrap.php';
 require_once __DIR__ . '/../app/Domain/Client/ClientValidator.php';
+require_once __DIR__ . '/../app/Domain/Jobcard/JobcardReference.php';
 
-session_set_cookie_params(['httponly' => true, 'secure' => !empty($_SERVER['HTTPS']), 'samesite' => 'Lax']);
+ini_set('session.use_strict_mode', '1');
+$forwardedHttps = getenv('TRUST_PROXY') === '1' && scalar_input($_SERVER['HTTP_X_FORWARDED_PROTO'] ?? '') === 'https';
+session_set_cookie_params(['httponly' => true, 'secure' => !empty($_SERVER['HTTPS']) || $forwardedHttps, 'samesite' => 'Lax', 'path' => '/']);
 session_start();
 
 function render_header(string $title): void
@@ -12,7 +15,7 @@ function render_header(string $title): void
     $user = current_user();
     echo '<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1"><title>' . e($title) . ' · JOBcard</title><link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css" rel="stylesheet"><link href="/assets/app.css" rel="stylesheet"></head><body>';
     if ($user) {
-        echo '<nav class="navbar navbar-dark bg-primary"><div class="container-fluid"><a class="navbar-brand fw-bold" href="/?route=dashboard">JOBcard</a><span class="text-white small">' . e($user['name']) . ' · ' . e($user['role_name']) . ' <a class="btn btn-sm btn-light ms-2" href="/?route=logout">Sign out</a></span></div></nav><div class="container-fluid"><div class="row"><aside class="col-md-2 col-lg-2 border-end bg-white min-vh-100 p-3"><nav class="nav flex-column gap-1"><a class="nav-link sidebar-link" href="/?route=dashboard">Dashboard</a>';
+        echo '<nav class="navbar navbar-dark bg-primary"><div class="container-fluid"><a class="navbar-brand fw-bold" href="/?route=dashboard">JOBcard</a><span class="text-white small">' . e($user['name']) . ' · ' . e($user['role_name']) . ' <form method="post" action="/?route=logout" class="d-inline"><input type="hidden" name="_csrf" value="' . e(csrf_token()) . '"><button class="btn btn-sm btn-light ms-2">Sign out</button></form></span></div></nav><div class="container-fluid"><div class="row"><aside class="col-md-2 col-lg-2 border-end bg-white min-vh-100 p-3"><nav class="nav flex-column gap-1"><a class="nav-link sidebar-link" href="/?route=dashboard">Dashboard</a>';
         if (can('clients.view')) echo '<a class="nav-link sidebar-link" href="/?route=clients">Clients</a>';
         if (can('jobcards.view')) echo '<a class="nav-link sidebar-link" href="/?route=jobcards">Jobcards</a>';
         if (can('reports.view')) echo '<a class="nav-link sidebar-link" href="/?route=reports">Reports</a>';
@@ -29,9 +32,11 @@ function render_footer(): void
     echo '</main>' . ($user ? '</div></div>' : '') . '<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/js/bootstrap.bundle.min.js"></script></body></html>';
 }
 
-$route = $_GET['route'] ?? (current_user() ? 'dashboard' : 'login');
+$route = scalar_input($_GET['route'] ?? null, current_user() ? 'dashboard' : 'login');
 
 if ($route === 'logout') {
+    if ($_SERVER['REQUEST_METHOD'] !== 'POST') { http_response_code(405); exit('Logout requires POST'); }
+    verify_csrf();
     if (current_user()) audit('logout', 'user', (int)current_user()['id']);
     $_SESSION = [];
     session_destroy();
@@ -45,9 +50,9 @@ if ($route === 'login') {
     if ($_SERVER['REQUEST_METHOD'] === 'POST') {
         verify_csrf();
         $stmt = db()->prepare('SELECT u.*, r.name AS role_name FROM users u JOIN roles r ON r.id = u.role_id WHERE u.email = :email LIMIT 1');
-        $stmt->execute(['email' => strtolower(trim((string)($_POST['email'] ?? ''))) ]);
+        $stmt->execute(['email' => strtolower(trim(scalar_input($_POST['email'] ?? null))) ]);
         $user = $stmt->fetch();
-        if (!$user || !$user['is_active'] || !password_verify((string)($_POST['password'] ?? ''), $user['password_hash'])) {
+        if (!$user || !$user['is_active'] || !password_verify(scalar_input($_POST['password'] ?? null), $user['password_hash'])) {
             $error = 'The email or password is incorrect.';
         } else {
             session_regenerate_id(true);
@@ -74,9 +79,9 @@ if ($route === 'jobcards') {
     if ($_SERVER['REQUEST_METHOD'] === 'POST') {
         require_permission('jobcards.manage');
         verify_csrf();
-        $clientId = filter_var($_POST['client_id'] ?? null, FILTER_VALIDATE_INT);
-        $workRequested = trim((string)($_POST['work_requested'] ?? ''));
-        $priority = (string)($_POST['priority'] ?? 'normal');
+        $clientId = filter_var(scalar_input($_POST['client_id'] ?? null), FILTER_VALIDATE_INT);
+        $workRequested = trim(scalar_input($_POST['work_requested'] ?? null));
+        $priority = scalar_input($_POST['priority'] ?? null, 'normal');
         if (!$clientId || $workRequested === '' || mb_strlen($workRequested) > 10000 || !in_array($priority, ['low', 'normal', 'high', 'critical'], true)) {
             $errors[] = 'Select a client, enter the requested work, and choose a valid priority.';
         } else {
@@ -85,7 +90,10 @@ if ($route === 'jobcards') {
             if (!$clientCheck->fetchColumn()) $errors[] = 'The selected client is not active or does not exist.';
         }
         if (!$errors) {
-            $reference = 'JC-' . date('Ymd') . '-' . strtoupper(bin2hex(random_bytes(3)));
+            $year = (int)date('Y');
+            $sequenceStmt = db()->prepare('SELECT COALESCE(MAX(CAST(SUBSTRING(reference_no, 9) AS UNSIGNED)), 0) + 1 FROM jobcards WHERE reference_no LIKE :prefix');
+            $sequenceStmt->execute(['prefix' => 'JC-' . $year . '-%']);
+            $reference = \App\Domain\Jobcard\JobcardReference::generate((int)$sequenceStmt->fetchColumn(), $year);
             $stmt = db()->prepare('INSERT INTO jobcards (reference_no, client_id, created_by, priority, status, work_requested) VALUES (:reference, :client, :created_by, :priority, \'new\', :requested)');
             $stmt->execute(['reference' => $reference, 'client' => $clientId, 'created_by' => $user['id'], 'priority' => $priority, 'requested' => $workRequested]);
             $jobcardId = (int)db()->lastInsertId();
@@ -111,7 +119,7 @@ if ($route === 'jobcards') {
 
 if ($route === 'client') {
     require_permission('clients.view');
-    $clientId = filter_input(INPUT_GET, 'id', FILTER_VALIDATE_INT);
+    $clientId = filter_var(scalar_input($_GET['id'] ?? null), FILTER_VALIDATE_INT);
     if (!$clientId) { http_response_code(400); exit('Invalid client'); }
     $stmt = db()->prepare('SELECT * FROM clients WHERE id = :id');
     $stmt->execute(['id' => $clientId]);
@@ -148,7 +156,7 @@ if ($route === 'clients') {
             exit;
         }
     }
-    $search = trim((string)($_GET['q'] ?? ''));
+    $search = trim(scalar_input($_GET['q'] ?? null));
     $stmt = db()->prepare('SELECT id, name, status, support_email, support_phone, created_at FROM clients WHERE (:search = \'\' OR name LIKE :like_name OR support_email LIKE :like_email) ORDER BY name LIMIT 100');
     $stmt->execute(['search' => $search, 'like_name' => "%{$search}%", 'like_email' => "%{$search}%"]);
     $clients = $stmt->fetchAll();
diff --git a/tests/JobcardCalculationsTest.php b/tests/JobcardCalculationsTest.php
index 5b513dc..235a088 100644
--- a/tests/JobcardCalculationsTest.php
+++ b/tests/JobcardCalculationsTest.php
@@ -13,6 +13,9 @@ if (calculate_duration_hours(null, null, 1.25) !== 1.25) {
 if (calculate_duration_hours('11:30', '09:00') !== null) {
     throw new RuntimeException('Expected invalid reverse time to be rejected');
 }
+if (calculate_duration_hours(null, null, 0.0) !== null) {
+    throw new RuntimeException('Expected zero manual hours to be rejected');
+}
 $sla = calculate_sla_usage(20.0, [2.0, 1.5, 1.0]);
 if ($sla !== ['used' => 4.5, 'remaining' => 15.5, 'percentage' => 22.5, 'status' => 'within_limit']) {
     throw new RuntimeException('Unexpected SLA calculation: ' . json_encode($sla));
diff --git a/tests/JobcardDomainTest.php b/tests/JobcardDomainTest.php
index 293abf7..c1c768f 100644
--- a/tests/JobcardDomainTest.php
+++ b/tests/JobcardDomainTest.php
@@ -29,5 +29,5 @@ if ($nonNumericEntry['valid'] || !isset($nonNumericEntry['errors']['hours'])) th
 
 $aggregator = new TimeAggregator();
 if ($aggregator->total([['hours' => 2.25], ['hours' => 1.5], ['hours' => -4]]) !== 3.75) throw new RuntimeException('Expected positive time aggregation.');
-if ($aggregator->slaTotal([['hours' => 2, 'counts_toward_sla' => true], ['hours' => 3, 'counts_toward_sla' => false]]) !== 2.0) throw new RuntimeException('Expected SLA-filtered aggregation.');
+if ($aggregator->slaTotal([['hours' => 2, 'counts_toward_sla' => true], ['hours' => 3, 'counts_toward_sla' => false], ['hours' => 1, 'counts_toward_sla' => 1]]) !== 3.0) throw new RuntimeException('Expected SLA-filtered aggregation.');
 printf("Jobcard domain tests: 12 passed\n");
diff --git a/tests/SlaDomainTest.php b/tests/SlaDomainTest.php
index 71c01db..4185aef 100644
--- a/tests/SlaDomainTest.php
+++ b/tests/SlaDomainTest.php
@@ -2,9 +2,14 @@
 declare(strict_types=1);
 
 require_once __DIR__ . '/../app/Domain/SLA/SlaThresholdClassifier.php';
+require_once __DIR__ . '/../app/Domain/SLA/SlaCalculator.php';
 
 use App\Domain\SLA\SlaThresholdClassifier;
 
+$negativeRejected = false;
+try { calculate_sla_usage(-1.0, [1.0]); } catch (InvalidArgumentException $exception) { $negativeRejected = true; }
+if (!$negativeRejected) throw new RuntimeException('Negative SLA allocation must be rejected');
+
 $classifier = new SlaThresholdClassifier();
 if ($classifier->classify(7.5, 10.0) !== 'warning') throw new RuntimeException('75% should be warning.');
 if ($classifier->classify(9.0, 10.0) !== 'critical') throw new RuntimeException('90% should be critical.');