diff --git a/system/BaseModel.php b/system/BaseModel.php index 980145f9573c..f5b0526eff1e 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 $throwOnDisallowedFields = false; + /** * An array of field names that are allowed * to be set by the user in inserts/updates. @@ -990,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'); - - // Validate every row. - if (! $this->skipValidation && ! $this->validate($row)) { - // Restore $cleanValidationRules - $this->cleanValidationRules = $cleanValidationRules; + try { + if (is_array($set)) { + foreach ($set as &$row) { + $row = $this->transformDataToArray($row, 'insert'); - return false; - } + // Validate every row. + if (! $this->skipValidation && ! $this->validate($row)) { + return false; + } - // Must be called first so we don't - // strip out created_at values. - $row = $this->doProtectFieldsForInsert($row); + // 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); + // 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) { @@ -1067,7 +1072,7 @@ public function update($id = null, $row = null): bool // Must be called first, so we don't // strip out updated_at values. - $row = $this->doProtectFields($row); + $row = $this->doProtectFieldsForUpdate($row); // doProtectFields() can further remove elements from // $row, so we need to check for empty dataset again @@ -1157,6 +1162,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 +1388,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 throwOnDisallowedFields(bool $throw = true) + { + $this->throwOnDisallowedFields = $throw; + + return $this; + } + /** * Ensures that only the fields that are allowed to be updated are * in the data array. @@ -1414,6 +1433,35 @@ protected function doProtectFields(array $row): array return $row; } + /** + * Throws when configured to detect fields that would be discarded. + * + * @param row_array $row + * @param list $ignoredFields + * + * @throws DataException + */ + protected function ensureNoDisallowedFields(array $row, array $ignoredFields = []): void + { + if (! $this->throwOnDisallowedFields || ! $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); + } + } + /** * Ensures that only the fields that are allowed to be inserted are in * the data array. @@ -1429,6 +1477,27 @@ protected function doProtectFields(array $row): array */ protected function doProtectFieldsForInsert(array $row): array { + $this->ensureNoDisallowedFields($row); + + return $this->doProtectFields($row); + } + + /** + * Ensures that only the fields that are allowed to be updated are in + * the data array. + * + * @used-by update() to protect against mass assignment vulnerabilities. + * + * @param row_array $row + * + * @return row_array + * + * @throws DataException + */ + protected function doProtectFieldsForUpdate(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..455ce7a53600 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 $throwOnDisallowedFields = 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..cafc655b7eb5 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,13 @@ protected function doProtectFieldsForInsert(array $row): array return $row; } + protected function doProtectFieldsForUpdate(array $row): array + { + $this->ensureNoDisallowedFields($row, [$this->primaryKey]); + + return $this->doProtectFields($row); + } + /** * Finds the first row matching attributes or inserts a new row. * diff --git a/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php new file mode 100644 index 000000000000..4916b8a03a81 --- /dev/null +++ b/tests/system/Models/ThrowOnDisallowedFieldsModelTest.php @@ -0,0 +1,218 @@ + + * + * 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; +use Throwable; + +/** + * @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 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); + $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 54083bdbb925..104cff887c66 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 ``$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 800ae304f117..648d5b9c9b01 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-throw-on-disallowed-fields: + +$throwOnDisallowedFields +------------------------ + +.. 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 ``throwOnDisallowedFields()`` 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 throwing on disallowed fields: + +.. literalinclude:: model/068.php + +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 new file mode 100644 index 000000000000..b74b10487c02 --- /dev/null +++ b/user_guide_src/source/models/model/068.php @@ -0,0 +1,4 @@ +throwOnDisallowedFields() + ->insert($data);