Validate sort/direction against SQL injection on index screens

src/Http/Controllers/Api/ProjectApiController.php

@@ -50,7 +50,7 @@ public function index(Request $request)
                         }
                     })
                     ->when($request->status && in_array($request->status, ['Ongoing', 'Onhold', 'Finished']), fn($q) => $q->where('status', $request->status))
-                    ->when($request->sort, fn($q) => $q->orderBy($request->sort, $request->get('direction', 'asc')), fn($q) => $q->latest())
+                    ->when($request->sort, fn($q) => $q->sortSafe($request->sort, $request->get('direction'), 'created_at', 'desc'), fn($q) => $q->latest())
                     ->paginate($request->get('per_page', 10));
 
                 $items->getCollection()->transform(function ($project) {

src/Http/Controllers/Api/TaskApiController.php

@@ -47,7 +47,7 @@ public function index(Request $request)
                     })
                     ->when($request->project_id, fn($q) => $q->where('project_id', $request->project_id))
                     ->when($request->status && in_array($request->status, ['High', 'Medium', 'Low']), fn($q) => $q->where('priority', $request->status))
-                    ->when($request->sort, fn($q) => $q->orderBy($request->sort, $request->get('direction', 'asc')), fn($q) => $q->latest());
+                    ->when($request->sort, fn($q) => $q->sortSafe($request->sort, $request->get('direction'), 'created_at', 'desc'), fn($q) => $q->latest());
 
                 $items = $items->get();
                 $items->transform(function ($task) {

src/Http/Controllers/ProjectBugController.php

@@ -58,9 +58,7 @@ public function index(Request $request)
                 $query->where('priority', $request->priority);
             }
 
-            $sortField = $request->get('sort', 'created_at');
-            $sortDirection = $request->get('direction', 'desc');
-            $query->orderBy($sortField, $sortDirection);
+            $query->sortSafe($request->get('sort'), $request->get('direction'), 'created_at', 'desc');
 
             $perPage = $request->get('per_page', 10);
             $bugs = $query->paginate($perPage);

src/Http/Controllers/ProjectController.php

@@ -67,7 +67,7 @@ public function index()
                 ->when(request('status'), fn($q) => $q->where('status', request('status')))
                 ->when(request('date'), fn($q) => $q->where('start_date', '<=', request('date'))->where('end_date', '>=', request('date')))
 
-                ->when(request('sort'), fn($q) => $q->orderBy(request('sort'), request('direction', 'asc')), fn($q) => $q->latest())
+                ->when(request('sort'), fn($q) => $q->sortSafe(request('sort'), request('direction'), 'created_at', 'desc'), fn($q) => $q->latest())
                 ->paginate(request('per_page', 10))
                 ->withQueryString();
 

src/Http/Controllers/ProjectReportController.php

@@ -41,7 +41,7 @@ public function index(Request $request)
             ->when($request->get('name'), fn($q) => $q->where('name', 'like', '%' . $request->get('name') . '%'))
             ->when($request->get('status'), fn($q) => $q->where('status', $request->get('status')))
             ->when($request->get('date'), fn($q) => $q->where('start_date', '<=', $request->get('date'))->where('end_date', '>=', $request->get('date')))
-            ->when($request->get('sort'), fn($q) => $q->orderBy($request->get('sort'), $request->get('direction', 'asc')), fn($q) => $q->latest())
+            ->when($request->get('sort'), fn($q) => $q->sortSafe($request->get('sort'), $request->get('direction'), 'created_at', 'desc'), fn($q) => $q->latest())
             ->paginate($request->get('per_page', 10))
             ->withQueryString();
 

src/Http/Controllers/ProjectTaskController.php

@@ -54,7 +54,7 @@ public function index(Request $request)
             $tasks = $query->when($projectId, fn($q) => $q->where('project_id', $projectId))
                 ->when(request('title'), fn($q) => $q->where('title', 'like', '%' . request('title') . '%'))
                 ->when(request('priority'), fn($q) => $q->where('priority', request('priority')))
-                ->when(request('sort'), fn($q) => $q->orderBy(request('sort'), request('direction', 'asc')), fn($q) => $q->latest())
+                ->when(request('sort'), fn($q) => $q->sortSafe(request('sort'), request('direction'), 'created_at', 'desc'), fn($q) => $q->latest())
                 ->paginate(request('per_page', 10))
                 ->withQueryString();
 

src/Providers/TasklyServiceProvider.php

@@ -2,6 +2,8 @@
 
 namespace Zerp\Taskly\Providers;
 
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\ServiceProvider;
 
 class TasklyServiceProvider extends ServiceProvider
@@ -31,6 +33,27 @@ public function boot(): void
         if (is_dir($migrationsPath)) {
             $this->loadMigrationsFrom($migrationsPath);
         }
+
+        // Whitelist orderBy against real table columns + asc|desc so a crafted
+        // ?sort=/?direction= cannot be interpolated into SQL. Part of the platform-wide sweep tracked on zerp-pk/zerp#39.
+        //
+        // Registered here rather than shared: these modules are installed
+        // independently and declare no common dependency, so a module cannot rely
+        // on another having booted. The guard keeps whichever loads first.
+        //
+        // hasGlobalMacro, not hasMacro: on an Eloquent builder the latter is an
+        // instance method for per-builder macros and cannot be called statically.
+        if (! Builder::hasGlobalMacro('sortSafe')) {
+            Builder::macro('sortSafe', function ($sort, $direction = null, $defaultColumn = 'created_at', $defaultDirection = 'desc') {
+                $table = $this->getModel()->getTable();
+                $column = ($sort && Schema::hasColumn($table, $sort)) ? $sort : $defaultColumn;
+                $direction = in_array(strtolower((string) $direction), ['asc', 'desc'], true)
+                    ? strtolower($direction)
+                    : $defaultDirection;
+
+                return $this->orderBy($column, $direction);
+            });
+        }
     }
 
     public function register(): void

tests/SortSafeTest.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace Zerp\Taskly\Tests;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Database\Schema\Blueprint;
+
+/**
+ * The index screens sort by a column and direction taken from the query string.
+ * Eloquent binds values but not identifiers, so an unchecked ?sort= is
+ * interpolated straight into the SQL. See zerp-pk/zerp#39.
+ *
+ * These assert the macro the controllers call, against a real sqlite table.
+ */
+class SortSafeTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        Schema::create('sortables', function (Blueprint $table) {
+            $table->id();
+            $table->string('name')->nullable();
+            $table->timestamps();
+        });
+    }
+
+    private function query()
+    {
+        return (new class extends Model
+        {
+            protected $table = 'sortables';
+        })->newQuery();
+    }
+
+    public function test_an_injected_sort_column_is_discarded(): void
+    {
+        $sql = $this->query()
+            ->sortSafe('name asc, (select sqlite_version()) --', null, 'created_at', 'desc')
+            ->toSql();
+
+        $this->assertStringNotContainsString('sqlite_version', $sql);
+        $this->assertStringNotContainsString('--', $sql);
+        $this->assertStringContainsString('order by "created_at" desc', $sql);
+    }
+
+    public function test_an_injected_direction_is_discarded(): void
+    {
+        $sql = $this->query()
+            ->sortSafe('name', 'asc; drop table sortables', 'created_at', 'desc')
+            ->toSql();
+
+        $this->assertStringNotContainsString('drop table', $sql);
+        $this->assertStringContainsString('order by "name" desc', $sql);
+    }
+
+    public function test_a_real_column_still_sorts(): void
+    {
+        $sql = $this->query()->sortSafe('name', 'asc', 'created_at', 'desc')->toSql();
+
+        $this->assertStringContainsString('order by "name" asc', $sql);
+    }
+
+    public function test_an_unknown_column_falls_back_to_the_default(): void
+    {
+        $sql = $this->query()->sortSafe('not_a_column', 'asc', 'created_at', 'desc')->toSql();
+
+        $this->assertStringContainsString('order by "created_at" asc', $sql);
+    }
+}