From b12ae73e910421ef1402b4caf3ba6909967dc64f Mon Sep 17 00:00:00 2001 From: Pontoporeia Date: Thu, 26 Mar 2026 18:54:20 +0100 Subject: [PATCH] =?UTF-8?q?tests:=20fix=20SecurityTest=20fatal=20TypeError?= =?UTF-8?q?=20=E2=80=94=20update=20searchTheses=20call=20to=20use=20array?= =?UTF-8?q?=20params?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SecurityTest::Test1 was calling $db->searchTheses($string) with a plain string, but searchTheses() was refactored to require array $params when the tag M2M work landed. This caused an immediate PHP fatal TypeError before any SQL ever ran, killing the entire Security test suite with exit code 255 and masking all three tests. Fix: pass each malicious payload via ['query' => $string] which is the correct API and properly exercises the parameterised query path through validateSearchParams() + buildSearchConditions(). Added a clarifying comment explaining why the array form is required. All 4 test suites now pass: - Database (Unit): 7/7 - Rate Limit (Unit): 5/5 - Search (Integration): 6/6 - Security: 3/3 --- TODO.md | 4 ++++ .../ad921d60486366258809553a3db49a4a.json | 2 +- .../f528764d624db129b32c21fbca0cb8d6.json | 1 - storage/test.db | Bin 237568 -> 237568 bytes tests/Security/SecurityTest.php | 13 +++++++++---- 5 files changed, 14 insertions(+), 6 deletions(-) delete mode 100644 src/cache/rate_limit/f528764d624db129b32c21fbca0cb8d6.json diff --git a/TODO.md b/TODO.md index 5762479..a519a06 100644 --- a/TODO.md +++ b/TODO.md @@ -317,6 +317,10 @@ Goal: rename the tables and column to the canonical M2M pattern (`tags`, `thesis --- +## Fixes + +- [x] Fix `tests/Security/SecurityTest.php`: update SQL injection test to call `searchTheses(['query' => $string])` instead of bare string — `searchTheses()` signature was updated to `array $params` but the test was never updated, causing a fatal `TypeError` that prevented the security suite from running at all + ## Pending - [x] Add flake.nix for Nix-based PHP dev environment diff --git a/src/cache/rate_limit/ad921d60486366258809553a3db49a4a.json b/src/cache/rate_limit/ad921d60486366258809553a3db49a4a.json index 9c94055..196b222 100644 --- a/src/cache/rate_limit/ad921d60486366258809553a3db49a4a.json +++ b/src/cache/rate_limit/ad921d60486366258809553a3db49a4a.json @@ -1 +1 @@ -[1770895832] \ No newline at end of file +[1774547615,1774547646] \ No newline at end of file diff --git a/src/cache/rate_limit/f528764d624db129b32c21fbca0cb8d6.json b/src/cache/rate_limit/f528764d624db129b32c21fbca0cb8d6.json deleted file mode 100644 index 1eafea7..0000000 --- a/src/cache/rate_limit/f528764d624db129b32c21fbca0cb8d6.json +++ /dev/null @@ -1 +0,0 @@ -[1774354709] \ No newline at end of file diff --git a/storage/test.db b/storage/test.db index 7242156ced397bc3a1320da7a58e073edc3136b7..916a045a0a7ccba2e1cc01bfa575c387104686f3 100644 GIT binary patch delta 38 scmZoTz}IkqZ-O-Au8A_vjJq}_e9~i#Z4TCN57uV{Vy5lE`pjz!02(+AumAu6 delta 38 scmZoTz}IkqZ-O-Arin7njGHzle9~i#Y!23M57uV{Vy5lE`pjz!02oLOoB#j- diff --git a/tests/Security/SecurityTest.php b/tests/Security/SecurityTest.php index 4ae50a2..d25163d 100644 --- a/tests/Security/SecurityTest.php +++ b/tests/Security/SecurityTest.php @@ -13,6 +13,10 @@ try { $db = Database::getInstance(); // Test 1: SQL Injection in search + // searchTheses() takes an array of validated params; the 'query' key is the + // free-text search field that users control. Each malicious string must + // be passed as ['query' => $string] to exercise the actual parameterised + // query path rather than triggering a PHP TypeError before any SQL runs. echo "Test 1: SQL Injection Protection (Search)\n"; $maliciousQueries = [ "' OR '1'='1", @@ -23,11 +27,12 @@ try { foreach ($maliciousQueries as $query) { try { - $results = $db->searchTheses($query); - echo " ✓ Blocked: " . substr($query, 0, 30) . "...\n"; + $results = $db->searchTheses(['query' => $query]); + // Should return a (possibly empty) result set without throwing + echo " ✓ Handled safely: " . substr($query, 0, 40) . "\n"; } catch (Exception $e) { - // Exception is also acceptable (query blocked) - echo " ✓ Exception: " . substr($query, 0, 30) . "...\n"; + // A thrown exception is also acceptable (query rejected upstream) + echo " ✓ Exception (safe): " . substr($query, 0, 40) . "\n"; } } echo "✓ PASS: SQL injection attempts handled safely\n\n";