From 2659aaa74f0c721e506734cbb915974e4d4b843f Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Thu, 11 Jun 2026 20:32:19 +0200 Subject: [PATCH 1/4] feat(model): add strict field protection Add an opt-in Model setting that throws when write data contains fields that would otherwise be discarded by allowed field protection. - Add $strictFieldProtection and strictFieldProtection() - Throw DataException for disallowed write fields in strict mode - Preserve existing primary key and updateBatch index behavior - Cover insert, save, update, batch, validation order, and protect(false) - Document the new option and update the model generator template Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- system/BaseModel.php | 64 +++++++ .../Commands/Generators/Views/model.tpl.php | 1 + system/Database/Exceptions/DataException.php | 10 + system/Language/en/Database.php | 1 + system/Model.php | 7 + .../Models/StrictFieldProtectionModelTest.php | 180 ++++++++++++++++++ user_guide_src/source/changelogs/v4.8.0.rst | 1 + user_guide_src/source/models/model.rst | 28 +++ user_guide_src/source/models/model/068.php | 4 + 9 files changed, 296 insertions(+) create mode 100644 tests/system/Models/StrictFieldProtectionModelTest.php create mode 100644 user_guide_src/source/models/model/068.php diff --git a/system/BaseModel.php b/system/BaseModel.php index 980145f9573c..55b5e7cedd6a 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -141,6 +141,12 @@ abstract class BaseModel */ protected $protectFields = true; + /** + * Whether Model should throw instead of silently discarding + * fields that are not in $allowedFields. + */ + protected bool $strictFieldProtection = false; + /** * An array of field names that are allowed * to be set by the user in inserts/updates. @@ -1067,6 +1073,7 @@ public function update($id = null, $row = null): bool // Must be called first, so we don't // strip out updated_at values. + $this->ensureNoDisallowedFields($row, $this->getFieldProtectionIgnoredFieldsForUpdate($id)); $row = $this->doProtectFields($row); // doProtectFields() can further remove elements from @@ -1157,6 +1164,7 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc // Must be called first so we don't // strip out updated_at values. + $this->ensureNoDisallowedFields($row, $index === null ? [] : [$index]); $row = $this->doProtectFields($row); // Restore updateIndex value in case it was wiped out @@ -1382,6 +1390,19 @@ public function protect(bool $protect = true) return $this; } + /** + * Sets whether or not disallowed fields should throw an exception + * instead of being discarded. + * + * @return $this + */ + public function strictFieldProtection(bool $strict = true) + { + $this->strictFieldProtection = $strict; + + return $this; + } + /** * Ensures that only the fields that are allowed to be updated are * in the data array. @@ -1414,6 +1435,47 @@ protected function doProtectFields(array $row): array return $row; } + /** + * Throws when strict field protection detects fields that would be discarded. + * + * @param row_array $row + * @param list $ignoredFields + * + * @throws DataException + */ + protected function ensureNoDisallowedFields(array $row, array $ignoredFields = []): void + { + if (! $this->strictFieldProtection || ! $this->protectFields || $this->allowedFields === []) { + return; + } + + $disallowedFields = []; + + foreach (array_keys($row) as $key) { + if (in_array($key, $this->allowedFields, true) || in_array($key, $ignoredFields, true)) { + continue; + } + + $disallowedFields[] = $key; + } + + if ($disallowedFields !== []) { + throw DataException::forDisallowedFields(static::class, $disallowedFields); + } + } + + /** + * Returns fields that may be used by the write operation but not persisted. + * + * @param mixed $id + * + * @return list + */ + protected function getFieldProtectionIgnoredFieldsForUpdate($id): array + { + return []; + } + /** * Ensures that only the fields that are allowed to be inserted are in * the data array. @@ -1429,6 +1491,8 @@ protected function doProtectFields(array $row): array */ protected function doProtectFieldsForInsert(array $row): array { + $this->ensureNoDisallowedFields($row); + return $this->doProtectFields($row); } diff --git a/system/Commands/Generators/Views/model.tpl.php b/system/Commands/Generators/Views/model.tpl.php index 954404f854d4..2426edf7230d 100644 --- a/system/Commands/Generators/Views/model.tpl.php +++ b/system/Commands/Generators/Views/model.tpl.php @@ -17,6 +17,7 @@ class {class} extends Model protected $protectFields = true; protected $allowedFields = []; + protected bool $strictFieldProtection = false; protected bool $allowEmptyInserts = false; protected bool $updateOnlyChanged = true; diff --git a/system/Database/Exceptions/DataException.php b/system/Database/Exceptions/DataException.php index 7cd9f526e6c2..c61f4debe81f 100644 --- a/system/Database/Exceptions/DataException.php +++ b/system/Database/Exceptions/DataException.php @@ -73,6 +73,16 @@ public static function forInvalidAllowedFields(string $model) return new static(lang('Database.invalidAllowedFields', [$model])); } + /** + * @param list $fields + * + * @return DataException + */ + public static function forDisallowedFields(string $model, array $fields) + { + return new static(lang('Database.disallowedFields', [$model, implode(', ', $fields)])); + } + /** * @return DataException */ diff --git a/system/Language/en/Database.php b/system/Language/en/Database.php index c5a5ddc8548a..94c8d3a9a7fc 100644 --- a/system/Language/en/Database.php +++ b/system/Language/en/Database.php @@ -16,6 +16,7 @@ 'invalidEvent' => '"{0}" is not a valid Model Event callback.', 'invalidArgument' => 'You must provide a valid "{0}".', 'invalidAllowedFields' => 'Allowed fields must be specified for model: "{0}"', + 'disallowedFields' => 'Fields are not allowed for model "{0}": {1}', 'emptyDataset' => 'There is no data to {0}.', 'emptyPrimaryKey' => 'There is no primary key defined when trying to make {0}.', 'failGetFieldData' => 'Failed to get field data from database.', diff --git a/system/Model.php b/system/Model.php index bc43b1db4bc7..def986c12ced 100644 --- a/system/Model.php +++ b/system/Model.php @@ -767,6 +767,8 @@ protected function doProtectFieldsForInsert(array $row): array throw DataException::forInvalidAllowedFields(static::class); } + $this->ensureNoDisallowedFields($row, $this->useAutoIncrement === false ? [$this->primaryKey] : []); + foreach (array_keys($row) as $key) { // Do not remove the non-auto-incrementing primary key data. if ($this->useAutoIncrement === false && $key === $this->primaryKey) { @@ -781,6 +783,11 @@ protected function doProtectFieldsForInsert(array $row): array return $row; } + protected function getFieldProtectionIgnoredFieldsForUpdate($id): array + { + return $id === null ? [] : [$this->primaryKey]; + } + /** * Finds the first row matching attributes or inserts a new row. * diff --git a/tests/system/Models/StrictFieldProtectionModelTest.php b/tests/system/Models/StrictFieldProtectionModelTest.php new file mode 100644 index 000000000000..a5d19aaeffe4 --- /dev/null +++ b/tests/system/Models/StrictFieldProtectionModelTest.php @@ -0,0 +1,180 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Models; + +use CodeIgniter\Database\Exceptions\DataException; +use PHPUnit\Framework\Attributes\Group; +use Tests\Support\Models\UserModel; +use Tests\Support\Models\ValidModel; +use Tests\Support\Models\WithoutAutoIncrementModel; + +/** + * @internal + */ +#[Group('DatabaseLive')] +final class StrictFieldProtectionModelTest extends LiveModelTestCase +{ + public function testDefaultFieldProtectionStillDiscardsDisallowedFields(): void + { + $this->createModel(UserModel::class)->insert([ + 'name' => 'Strict Default', + 'email' => 'strict-default@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + + $this->seeInDatabase('user', [ + 'email' => 'strict-default@example.com', + ]); + } + + public function testStrictFieldProtectionThrowsOnInsertDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->strictFieldProtection()->insert([ + 'name' => 'Strict Insert', + 'email' => 'strict-insert@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + } + + public function testStrictFieldProtectionThrowsOnInsertBatchDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->strictFieldProtection()->insertBatch([ + [ + 'name' => 'Strict Batch', + 'email' => 'strict-batch@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ], + ]); + } + + public function testStrictFieldProtectionThrowsOnUpdateDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->strictFieldProtection()->update(1, [ + 'name' => 'Strict Update', + 'timezone' => 'UTC', + ]); + } + + public function testStrictFieldProtectionThrowsOnSaveDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->strictFieldProtection()->save([ + 'name' => 'Strict Save', + 'email' => 'strict-save@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + } + + public function testStrictFieldProtectionAllowsPrimaryKeyDuringUpdate(): void + { + $result = $this->createModel(UserModel::class)->strictFieldProtection()->update(1, [ + 'id' => 1, + 'name' => 'Strict Primary Key', + ]); + + $this->assertTrue($result); + $this->seeInDatabase('user', [ + 'id' => 1, + 'name' => 'Strict Primary Key', + ]); + } + + public function testStrictFieldProtectionAllowsBatchIndexDuringUpdateBatch(): void + { + $result = $this->createModel(UserModel::class)->strictFieldProtection()->updateBatch([ + [ + 'id' => 1, + 'name' => 'Strict Batch One', + ], + [ + 'id' => 2, + 'name' => 'Strict Batch Two', + ], + ], 'id'); + + $this->assertSame(2, $result); + $this->seeInDatabase('user', [ + 'id' => 1, + 'name' => 'Strict Batch One', + ]); + $this->seeInDatabase('user', [ + 'id' => 2, + 'name' => 'Strict Batch Two', + ]); + } + + public function testStrictFieldProtectionThrowsOnUpdateBatchDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->strictFieldProtection()->updateBatch([ + [ + 'id' => 1, + 'name' => 'Strict Batch', + 'timezone' => 'UTC', + ], + ], 'id'); + } + + public function testStrictFieldProtectionAllowsNonAutoIncrementPrimaryKeyDuringInsert(): void + { + $result = $this->createModel(WithoutAutoIncrementModel::class)->strictFieldProtection()->insert([ + 'key' => 'strict-key', + 'value' => 'strict value', + ]); + + $this->assertSame('strict-key', $result); + $this->seeInDatabase('without_auto_increment', [ + 'key' => 'strict-key', + 'value' => 'strict value', + ]); + } + + public function testProtectFalseBypassesStrictFieldProtection(): void + { + $result = $this->createModel(UserModel::class)->strictFieldProtection()->protect(false)->update(1, [ + 'id' => 1, + 'name' => 'Strict Disabled', + ]); + + $this->assertTrue($result); + } + + public function testValidationRunsBeforeStrictFieldProtection(): void + { + $model = $this->createModel(ValidModel::class)->strictFieldProtection(); + + $this->assertFalse($model->insert([ + 'description' => 'Missing required name', + 'extra' => 'discarded after validation', + ])); + $this->assertArrayHasKey('name', $model->errors()); + } +} diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 54083bdbb925..25302899f9eb 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -258,6 +258,7 @@ Model - Added new ``chunkRows()`` method to ``CodeIgniter\Model`` for processing large datasets in smaller chunks. - Added new ``firstOrInsert()`` method to ``CodeIgniter\Model`` that finds the first row matching the given attributes or inserts a new one. See :ref:`model-first-or-insert`. +- Added ``$strictFieldProtection`` and ``strictFieldProtection()`` to ``CodeIgniter\Model`` to throw a ``DataException`` when write data contains fields that would otherwise be discarded by ``$allowedFields``. See :ref:`model-strict-field-protection`. Libraries ========= diff --git a/user_guide_src/source/models/model.rst b/user_guide_src/source/models/model.rst index 800ae304f117..3f753a54a3fe 100644 --- a/user_guide_src/source/models/model.rst +++ b/user_guide_src/source/models/model.rst @@ -163,6 +163,23 @@ potential mass assignment vulnerabilities. .. note:: The `$primaryKey`_ field should never be an allowed field. +.. _model-strict-field-protection: + +$strictFieldProtection +---------------------- + +.. versionadded:: 4.8.0 + +When ``true``, the model throws a ``DataException`` instead of discarding fields +that are not listed in `$allowedFields`_ during ``insert()``, ``insertBatch()``, +``update()``, ``updateBatch()``, and ``save()`` calls. + +This is useful when you want to catch typos, stale form fields, or unexpected +write payloads during development or in strict application code. It does not +replace validation and does not inspect the database schema. + +You may also change this setting with the ``strictFieldProtection()`` method. + $allowEmptyInserts ------------------ @@ -906,6 +923,8 @@ So it will ignore the row in the database that has ``id=4`` when it verifies the This can also be used to create more dynamic rules at runtime, as long as you take care that any dynamic keys passed in don't conflict with your form data. +.. _model-protecting-fields: + Protecting Fields ================= @@ -916,6 +935,15 @@ or primary keys do not get changed. .. literalinclude:: model/041.php +If you prefer disallowed fields to raise an exception instead of being silently +removed, enable strict field protection: + +.. literalinclude:: model/068.php + +With strict field protection enabled, operation fields such as the primary key +passed to ``update()`` or the index passed to ``updateBatch()`` may still be used +to locate rows. + Occasionally, you will find times where you need to be able to change these elements. This is often during testing, migrations, or seeds. In these cases, you can turn the protection on or off: diff --git a/user_guide_src/source/models/model/068.php b/user_guide_src/source/models/model/068.php new file mode 100644 index 000000000000..2163c575e2f1 --- /dev/null +++ b/user_guide_src/source/models/model/068.php @@ -0,0 +1,4 @@ +strictFieldProtection() + ->insert($data); From 8cb893f464ebfc41d43171557f990c62122c975c Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Thu, 11 Jun 2026 22:03:19 +0200 Subject: [PATCH 2/4] test: avoid SQLSRV identity update in strict protection test Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- tests/system/Models/StrictFieldProtectionModelTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/Models/StrictFieldProtectionModelTest.php b/tests/system/Models/StrictFieldProtectionModelTest.php index a5d19aaeffe4..569181a8bd3d 100644 --- a/tests/system/Models/StrictFieldProtectionModelTest.php +++ b/tests/system/Models/StrictFieldProtectionModelTest.php @@ -160,8 +160,8 @@ public function testStrictFieldProtectionAllowsNonAutoIncrementPrimaryKeyDuringI public function testProtectFalseBypassesStrictFieldProtection(): void { $result = $this->createModel(UserModel::class)->strictFieldProtection()->protect(false)->update(1, [ - 'id' => 1, - 'name' => 'Strict Disabled', + 'name' => 'Strict Disabled', + 'created_at' => '2026-01-01 12:00:00', ]); $this->assertTrue($result); From bad5eb57962a00c8cb5e58c6ff6031407b27f1e9 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Fri, 12 Jun 2026 20:52:52 +0200 Subject: [PATCH 3/4] refactor: rename disallowed field exception mode Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- system/BaseModel.php | 38 ++-- .../Commands/Generators/Views/model.tpl.php | 2 +- system/Model.php | 6 +- .../Models/StrictFieldProtectionModelTest.php | 180 ---------------- .../ThrowOnDisallowedFieldsModelTest.php | 196 ++++++++++++++++++ user_guide_src/source/changelogs/v4.8.0.rst | 2 +- user_guide_src/source/models/model.rst | 16 +- user_guide_src/source/models/model/068.php | 2 +- 8 files changed, 233 insertions(+), 209 deletions(-) delete mode 100644 tests/system/Models/StrictFieldProtectionModelTest.php create mode 100644 tests/system/Models/ThrowOnDisallowedFieldsModelTest.php diff --git a/system/BaseModel.php b/system/BaseModel.php index 55b5e7cedd6a..ca74ab2cc4cc 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -145,7 +145,7 @@ abstract class BaseModel * Whether Model should throw instead of silently discarding * fields that are not in $allowedFields. */ - protected bool $strictFieldProtection = false; + protected bool $throwOnDisallowedFields = false; /** * An array of field names that are allowed @@ -1073,8 +1073,7 @@ public function update($id = null, $row = null): bool // Must be called first, so we don't // strip out updated_at values. - $this->ensureNoDisallowedFields($row, $this->getFieldProtectionIgnoredFieldsForUpdate($id)); - $row = $this->doProtectFields($row); + $row = $this->doProtectFieldsForUpdate($row); // doProtectFields() can further remove elements from // $row, so we need to check for empty dataset again @@ -1396,9 +1395,9 @@ public function protect(bool $protect = true) * * @return $this */ - public function strictFieldProtection(bool $strict = true) + public function throwOnDisallowedFields(bool $throw = true) { - $this->strictFieldProtection = $strict; + $this->throwOnDisallowedFields = $throw; return $this; } @@ -1436,7 +1435,7 @@ protected function doProtectFields(array $row): array } /** - * Throws when strict field protection detects fields that would be discarded. + * Throws when configured to detect fields that would be discarded. * * @param row_array $row * @param list $ignoredFields @@ -1445,7 +1444,7 @@ protected function doProtectFields(array $row): array */ protected function ensureNoDisallowedFields(array $row, array $ignoredFields = []): void { - if (! $this->strictFieldProtection || ! $this->protectFields || $this->allowedFields === []) { + if (! $this->throwOnDisallowedFields || ! $this->protectFields || $this->allowedFields === []) { return; } @@ -1465,23 +1464,30 @@ protected function ensureNoDisallowedFields(array $row, array $ignoredFields = [ } /** - * Returns fields that may be used by the write operation but not persisted. + * Ensures that only the fields that are allowed to be inserted are in + * the data array. * - * @param mixed $id + * @used-by insert() to protect against mass assignment vulnerabilities. + * @used-by insertBatch() to protect against mass assignment vulnerabilities. * - * @return list + * @param row_array $row + * + * @return row_array + * + * @throws DataException */ - protected function getFieldProtectionIgnoredFieldsForUpdate($id): array + protected function doProtectFieldsForInsert(array $row): array { - return []; + $this->ensureNoDisallowedFields($row); + + return $this->doProtectFields($row); } /** - * Ensures that only the fields that are allowed to be inserted are in + * Ensures that only the fields that are allowed to be updated are in * the data array. * - * @used-by insert() to protect against mass assignment vulnerabilities. - * @used-by insertBatch() to protect against mass assignment vulnerabilities. + * @used-by update() to protect against mass assignment vulnerabilities. * * @param row_array $row * @@ -1489,7 +1495,7 @@ protected function getFieldProtectionIgnoredFieldsForUpdate($id): array * * @throws DataException */ - protected function doProtectFieldsForInsert(array $row): array + protected function doProtectFieldsForUpdate(array $row): array { $this->ensureNoDisallowedFields($row); diff --git a/system/Commands/Generators/Views/model.tpl.php b/system/Commands/Generators/Views/model.tpl.php index 2426edf7230d..455ce7a53600 100644 --- a/system/Commands/Generators/Views/model.tpl.php +++ b/system/Commands/Generators/Views/model.tpl.php @@ -17,7 +17,7 @@ class {class} extends Model protected $protectFields = true; protected $allowedFields = []; - protected bool $strictFieldProtection = false; + protected bool $throwOnDisallowedFields = false; protected bool $allowEmptyInserts = false; protected bool $updateOnlyChanged = true; diff --git a/system/Model.php b/system/Model.php index def986c12ced..cafc655b7eb5 100644 --- a/system/Model.php +++ b/system/Model.php @@ -783,9 +783,11 @@ protected function doProtectFieldsForInsert(array $row): array return $row; } - protected function getFieldProtectionIgnoredFieldsForUpdate($id): array + protected function doProtectFieldsForUpdate(array $row): array { - return $id === null ? [] : [$this->primaryKey]; + $this->ensureNoDisallowedFields($row, [$this->primaryKey]); + + return $this->doProtectFields($row); } /** diff --git a/tests/system/Models/StrictFieldProtectionModelTest.php b/tests/system/Models/StrictFieldProtectionModelTest.php deleted file mode 100644 index 569181a8bd3d..000000000000 --- a/tests/system/Models/StrictFieldProtectionModelTest.php +++ /dev/null @@ -1,180 +0,0 @@ - - * - * For the full copyright and license information, please view - * the LICENSE file that was distributed with this source code. - */ - -namespace CodeIgniter\Models; - -use CodeIgniter\Database\Exceptions\DataException; -use PHPUnit\Framework\Attributes\Group; -use Tests\Support\Models\UserModel; -use Tests\Support\Models\ValidModel; -use Tests\Support\Models\WithoutAutoIncrementModel; - -/** - * @internal - */ -#[Group('DatabaseLive')] -final class StrictFieldProtectionModelTest extends LiveModelTestCase -{ - public function testDefaultFieldProtectionStillDiscardsDisallowedFields(): void - { - $this->createModel(UserModel::class)->insert([ - 'name' => 'Strict Default', - 'email' => 'strict-default@example.com', - 'country' => 'US', - 'timezone' => 'UTC', - ]); - - $this->seeInDatabase('user', [ - 'email' => 'strict-default@example.com', - ]); - } - - public function testStrictFieldProtectionThrowsOnInsertDisallowedFields(): void - { - $this->expectException(DataException::class); - $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); - - $this->createModel(UserModel::class)->strictFieldProtection()->insert([ - 'name' => 'Strict Insert', - 'email' => 'strict-insert@example.com', - 'country' => 'US', - 'timezone' => 'UTC', - ]); - } - - public function testStrictFieldProtectionThrowsOnInsertBatchDisallowedFields(): void - { - $this->expectException(DataException::class); - $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); - - $this->createModel(UserModel::class)->strictFieldProtection()->insertBatch([ - [ - 'name' => 'Strict Batch', - 'email' => 'strict-batch@example.com', - 'country' => 'US', - 'timezone' => 'UTC', - ], - ]); - } - - public function testStrictFieldProtectionThrowsOnUpdateDisallowedFields(): void - { - $this->expectException(DataException::class); - $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); - - $this->createModel(UserModel::class)->strictFieldProtection()->update(1, [ - 'name' => 'Strict Update', - 'timezone' => 'UTC', - ]); - } - - public function testStrictFieldProtectionThrowsOnSaveDisallowedFields(): void - { - $this->expectException(DataException::class); - $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); - - $this->createModel(UserModel::class)->strictFieldProtection()->save([ - 'name' => 'Strict Save', - 'email' => 'strict-save@example.com', - 'country' => 'US', - 'timezone' => 'UTC', - ]); - } - - public function testStrictFieldProtectionAllowsPrimaryKeyDuringUpdate(): void - { - $result = $this->createModel(UserModel::class)->strictFieldProtection()->update(1, [ - 'id' => 1, - 'name' => 'Strict Primary Key', - ]); - - $this->assertTrue($result); - $this->seeInDatabase('user', [ - 'id' => 1, - 'name' => 'Strict Primary Key', - ]); - } - - public function testStrictFieldProtectionAllowsBatchIndexDuringUpdateBatch(): void - { - $result = $this->createModel(UserModel::class)->strictFieldProtection()->updateBatch([ - [ - 'id' => 1, - 'name' => 'Strict Batch One', - ], - [ - 'id' => 2, - 'name' => 'Strict Batch Two', - ], - ], 'id'); - - $this->assertSame(2, $result); - $this->seeInDatabase('user', [ - 'id' => 1, - 'name' => 'Strict Batch One', - ]); - $this->seeInDatabase('user', [ - 'id' => 2, - 'name' => 'Strict Batch Two', - ]); - } - - public function testStrictFieldProtectionThrowsOnUpdateBatchDisallowedFields(): void - { - $this->expectException(DataException::class); - $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); - - $this->createModel(UserModel::class)->strictFieldProtection()->updateBatch([ - [ - 'id' => 1, - 'name' => 'Strict Batch', - 'timezone' => 'UTC', - ], - ], 'id'); - } - - public function testStrictFieldProtectionAllowsNonAutoIncrementPrimaryKeyDuringInsert(): void - { - $result = $this->createModel(WithoutAutoIncrementModel::class)->strictFieldProtection()->insert([ - 'key' => 'strict-key', - 'value' => 'strict value', - ]); - - $this->assertSame('strict-key', $result); - $this->seeInDatabase('without_auto_increment', [ - 'key' => 'strict-key', - 'value' => 'strict value', - ]); - } - - public function testProtectFalseBypassesStrictFieldProtection(): void - { - $result = $this->createModel(UserModel::class)->strictFieldProtection()->protect(false)->update(1, [ - 'name' => 'Strict Disabled', - 'created_at' => '2026-01-01 12:00:00', - ]); - - $this->assertTrue($result); - } - - public function testValidationRunsBeforeStrictFieldProtection(): void - { - $model = $this->createModel(ValidModel::class)->strictFieldProtection(); - - $this->assertFalse($model->insert([ - 'description' => 'Missing required name', - 'extra' => 'discarded after validation', - ])); - $this->assertArrayHasKey('name', $model->errors()); - } -} diff --git a/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php new file mode 100644 index 000000000000..d75fd05b2abf --- /dev/null +++ b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php @@ -0,0 +1,196 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Models; + +use CodeIgniter\Database\Exceptions\DataException; +use PHPUnit\Framework\Attributes\Group; +use Tests\Support\Models\UserModel; +use Tests\Support\Models\ValidModel; +use Tests\Support\Models\WithoutAutoIncrementModel; + +/** + * @internal + */ +#[Group('DatabaseLive')] +final class ThrowOnDisallowedFieldsModelTest extends LiveModelTestCase +{ + public function testDefaultFieldProtectionStillDiscardsDisallowedFields(): void + { + $this->createModel(UserModel::class)->insert([ + 'name' => 'Disallowed Default', + 'email' => 'disallowed-default@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + + $this->seeInDatabase('user', [ + 'email' => 'disallowed-default@example.com', + ]); + } + + public function testThrowOnDisallowedFieldsThrowsOnInsertDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->throwOnDisallowedFields()->insert([ + 'name' => 'Disallowed Insert', + 'email' => 'disallowed-insert@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + } + + public function testThrowOnDisallowedFieldsThrowsOnInsertBatchDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->throwOnDisallowedFields()->insertBatch([ + [ + 'name' => 'Disallowed Batch', + 'email' => 'disallowed-batch@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ], + ]); + } + + public function testThrowOnDisallowedFieldsThrowsOnUpdateDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->throwOnDisallowedFields()->update(1, [ + 'name' => 'Disallowed Update', + 'timezone' => 'UTC', + ]); + } + + public function testThrowOnDisallowedFieldsThrowsOnSaveDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->throwOnDisallowedFields()->save([ + 'name' => 'Disallowed Save', + 'email' => 'disallowed-save@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ]); + } + + public function testThrowOnDisallowedFieldsAllowsPrimaryKeyDuringUpdate(): void + { + $result = $this->createModel(UserModel::class)->throwOnDisallowedFields()->update(1, [ + 'id' => 1, + 'name' => 'Disallowed Primary Key', + ]); + + $this->assertTrue($result); + $this->seeInDatabase('user', [ + 'id' => 1, + 'name' => 'Disallowed Primary Key', + ]); + } + + public function testThrowOnDisallowedFieldsAllowsPrimaryKeyDuringConditionalUpdate(): void + { + $result = $this->createModel(UserModel::class)->throwOnDisallowedFields() + ->where('id', 1) + ->update(null, [ + 'id' => 1, + 'name' => 'Disallowed Conditional Primary Key', + ]); + + $this->assertTrue($result); + $this->seeInDatabase('user', [ + 'id' => 1, + 'name' => 'Disallowed Conditional Primary Key', + ]); + } + + public function testThrowOnDisallowedFieldsAllowsBatchIndexDuringUpdateBatch(): void + { + $result = $this->createModel(UserModel::class)->throwOnDisallowedFields()->updateBatch([ + [ + 'id' => 1, + 'name' => 'Disallowed Batch One', + ], + [ + 'id' => 2, + 'name' => 'Disallowed Batch Two', + ], + ], 'id'); + + $this->assertSame(2, $result); + $this->seeInDatabase('user', [ + 'id' => 1, + 'name' => 'Disallowed Batch One', + ]); + $this->seeInDatabase('user', [ + 'id' => 2, + 'name' => 'Disallowed Batch Two', + ]); + } + + public function testThrowOnDisallowedFieldsThrowsOnUpdateBatchDisallowedFields(): void + { + $this->expectException(DataException::class); + $this->expectExceptionMessage('Fields are not allowed for model "Tests\Support\Models\UserModel": timezone'); + + $this->createModel(UserModel::class)->throwOnDisallowedFields()->updateBatch([ + [ + 'id' => 1, + 'name' => 'Disallowed Batch', + 'timezone' => 'UTC', + ], + ], 'id'); + } + + public function testThrowOnDisallowedFieldsAllowsNonAutoIncrementPrimaryKeyDuringInsert(): void + { + $result = $this->createModel(WithoutAutoIncrementModel::class)->throwOnDisallowedFields()->insert([ + 'key' => 'disallowed-key', + 'value' => 'disallowed value', + ]); + + $this->assertSame('disallowed-key', $result); + $this->seeInDatabase('without_auto_increment', [ + 'key' => 'disallowed-key', + 'value' => 'disallowed value', + ]); + } + + public function testProtectFalseBypassesThrowOnDisallowedFields(): void + { + $result = $this->createModel(UserModel::class)->throwOnDisallowedFields()->protect(false)->update(1, [ + 'name' => 'Disallowed Disabled', + 'created_at' => '2026-01-01 12:00:00', + ]); + + $this->assertTrue($result); + } + + public function testValidationRunsBeforeThrowOnDisallowedFields(): void + { + $model = $this->createModel(ValidModel::class)->throwOnDisallowedFields(); + + $this->assertFalse($model->insert([ + 'description' => 'Missing required name', + 'extra' => 'discarded after validation', + ])); + $this->assertArrayHasKey('name', $model->errors()); + } +} diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 25302899f9eb..104cff887c66 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -258,7 +258,7 @@ Model - Added new ``chunkRows()`` method to ``CodeIgniter\Model`` for processing large datasets in smaller chunks. - Added new ``firstOrInsert()`` method to ``CodeIgniter\Model`` that finds the first row matching the given attributes or inserts a new one. See :ref:`model-first-or-insert`. -- Added ``$strictFieldProtection`` and ``strictFieldProtection()`` to ``CodeIgniter\Model`` to throw a ``DataException`` when write data contains fields that would otherwise be discarded by ``$allowedFields``. See :ref:`model-strict-field-protection`. +- Added ``$throwOnDisallowedFields`` and ``throwOnDisallowedFields()`` to ``CodeIgniter\Model`` to throw a ``DataException`` when write data contains fields that would otherwise be discarded by ``$allowedFields``. See :ref:`model-throw-on-disallowed-fields`. Libraries ========= diff --git a/user_guide_src/source/models/model.rst b/user_guide_src/source/models/model.rst index 3f753a54a3fe..648d5b9c9b01 100644 --- a/user_guide_src/source/models/model.rst +++ b/user_guide_src/source/models/model.rst @@ -163,10 +163,10 @@ potential mass assignment vulnerabilities. .. note:: The `$primaryKey`_ field should never be an allowed field. -.. _model-strict-field-protection: +.. _model-throw-on-disallowed-fields: -$strictFieldProtection ----------------------- +$throwOnDisallowedFields +------------------------ .. versionadded:: 4.8.0 @@ -178,7 +178,7 @@ This is useful when you want to catch typos, stale form fields, or unexpected write payloads during development or in strict application code. It does not replace validation and does not inspect the database schema. -You may also change this setting with the ``strictFieldProtection()`` method. +You may also change this setting with the ``throwOnDisallowedFields()`` method. $allowEmptyInserts ------------------ @@ -936,13 +936,13 @@ or primary keys do not get changed. .. literalinclude:: model/041.php If you prefer disallowed fields to raise an exception instead of being silently -removed, enable strict field protection: +removed, enable throwing on disallowed fields: .. literalinclude:: model/068.php -With strict field protection enabled, operation fields such as the primary key -passed to ``update()`` or the index passed to ``updateBatch()`` may still be used -to locate rows. +When throwing on disallowed fields is enabled, operation fields such as the +primary key passed to ``update()`` or the index passed to ``updateBatch()`` may +still be used to locate rows. Occasionally, you will find times where you need to be able to change these elements. This is often during testing, migrations, or seeds. In these cases, you can turn the protection on or off: diff --git a/user_guide_src/source/models/model/068.php b/user_guide_src/source/models/model/068.php index 2163c575e2f1..b74b10487c02 100644 --- a/user_guide_src/source/models/model/068.php +++ b/user_guide_src/source/models/model/068.php @@ -1,4 +1,4 @@ strictFieldProtection() +$model->throwOnDisallowedFields() ->insert($data); From b8adcfc9133d6af122c782bf8b188d3e2aae5744 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Sat, 13 Jun 2026 10:39:14 +0200 Subject: [PATCH 4/4] fix: restore validation state on insert batch exception Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- system/BaseModel.php | 39 +++++++++---------- .../ThrowOnDisallowedFieldsModelTest.php | 22 +++++++++++ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index ca74ab2cc4cc..f5b0526eff1e 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -996,32 +996,31 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch $cleanValidationRules = $this->cleanValidationRules; $this->cleanValidationRules = false; - if (is_array($set)) { - foreach ($set as &$row) { - $row = $this->transformDataToArray($row, 'insert'); + try { + if (is_array($set)) { + foreach ($set as &$row) { + $row = $this->transformDataToArray($row, 'insert'); + + // Validate every row. + if (! $this->skipValidation && ! $this->validate($row)) { + return false; + } - // Validate every row. - if (! $this->skipValidation && ! $this->validate($row)) { - // Restore $cleanValidationRules - $this->cleanValidationRules = $cleanValidationRules; + // Must be called first so we don't + // strip out created_at values. + $row = $this->doProtectFieldsForInsert($row); - return false; + // Set created_at and updated_at with same time + $date = $this->setDate(); + $row = $this->setCreatedField($row, $date); + $row = $this->setUpdatedField($row, $date); } - - // Must be called first so we don't - // strip out created_at values. - $row = $this->doProtectFieldsForInsert($row); - - // Set created_at and updated_at with same time - $date = $this->setDate(); - $row = $this->setCreatedField($row, $date); - $row = $this->setUpdatedField($row, $date); } + } finally { + // Restore $cleanValidationRules + $this->cleanValidationRules = $cleanValidationRules; } - // Restore $cleanValidationRules - $this->cleanValidationRules = $cleanValidationRules; - $eventData = ['data' => $set]; if ($this->tempAllowCallbacks) { diff --git a/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php index d75fd05b2abf..4916b8a03a81 100644 --- a/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php +++ b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php @@ -18,6 +18,7 @@ use Tests\Support\Models\UserModel; use Tests\Support\Models\ValidModel; use Tests\Support\Models\WithoutAutoIncrementModel; +use Throwable; /** * @internal @@ -67,6 +68,27 @@ public function testThrowOnDisallowedFieldsThrowsOnInsertBatchDisallowedFields() ]); } + public function testInsertBatchRestoresCleanValidationRulesWhenDisallowedFieldsThrow(): void + { + $model = $this->createModel(UserModel::class)->throwOnDisallowedFields(); + $exception = null; + + try { + $model->insertBatch([ + [ + 'name' => 'Disallowed Batch Restore', + 'email' => 'disallowed-batch-restore@example.com', + 'country' => 'US', + 'timezone' => 'UTC', + ], + ]); + } catch (Throwable $exception) { + } + + $this->assertInstanceOf(DataException::class, $exception); + $this->assertTrue($this->getPrivateProperty($model, 'cleanValidationRules')); + } + public function testThrowOnDisallowedFieldsThrowsOnUpdateDisallowedFields(): void { $this->expectException(DataException::class);