[necessarylion]

Fixes in dox_orm. One commit per item, plus follow-up commits for the first review.

S4: mass assignment is open by default

Problem. With fillable and guarded both empty (the old default), create(), insert(), insertMultiple(), upsert(), update(), firstOrNew(), firstOrCreate(), updateOrCreate() and increment() extras took every key of a map.

What it causes. User().create(req.body) could set id, is_admin or any other column.

Fix. guarded is now ['*'] by default, like Laravel. A model with no fillable throws MassAssignmentException when it gets a map with keys. The error names the model and the keys, and says how to fix it. It never saves an empty row, and update() never silently does nothing. A model with fillable drops the other keys, as before. guarded => const [] takes every key, as before. Setting fields in code and save() are not changed. Soft delete, restore and save() do not go through the guard.

Tests: "a model is guarded by default" in test/security_test.dart (create, update, firstOrCreate, insertMultiple). The test models set guarded => [], because the tests pass raw maps on purpose.

B1: update() and delete() with ORDER BY, LIMIT or JOIN

Problem. update() and delete() used getCommonQuery(). It adds JOIN, GROUP BY, ORDER BY and LIMIT after UPDATE t SET ... or DELETE FROM t.

What it causes. Postgres and SQLite reject this SQL. For example Blog().orderBy('id').limit(1).delete() fails. A JOIN in that place is also wrong on MySQL.

Fix. New helper.getWriteQuery():

• A plain WHERE gives the same SQL as before.
• With JOIN, ORDER BY or LIMIT, the rows are picked by primary key: WHERE id IN (SELECT id FROM (SELECT t.id AS id FROM t JOIN ... WHERE ... ORDER BY ... LIMIT ...) AS dox_rows). The extra wrap lets MySQL read the table it changes and use LIMIT.
• MySQL with no JOIN and no OFFSET keeps its own ORDER BY ... LIMIT, the same SQL as before. So a table with no primary key still works there.
• groupBy() and having() with update() or delete() throw an ArgumentError. This SQL was never valid.

Test: test/write_order_limit_join_test.dart. It passes on Postgres, SQLite and MariaDB.

S5 (dox-orm part): not changed

The owner wants no breaking change here. Turning on the certificate check for useSSL: true would break apps on databases with a private CA (for example AWS RDS) until they set sslRootCert. So useSSL: true stays require, and verify-full stays opt-in with extra['sslMode']. The first commit changed it, and a later commit reverts it. The Rust part of S5 (an unknown verify value fails closed) is in dartondox/dox-postgres-native#9. It does not break anything.

Breaking change

S4 only (the owner accepted it). A model with no fillable now throws MassAssignmentException when it gets a map. To fix, add fillable:

@override
List<String> get fillable => <String>['name', 'email'];

or keep the old behavior (not safe with user input):

@override
List<String> get guarded => const <String>[];

The CHANGELOG has an "Unreleased" entry. Docs: dartondox/dox#286.

Checks

• dart analyze: no issues (dox-orm and dox-migration).
• dart test --concurrency=1: all pass on SQLite, Postgres 16 and MariaDB.
• Not run: MySQL 8.4.

[necessarylion]

Bot review

Verdict: CHANGES NEEDED

The three fixes are right and the tests prove them. One thing must change before merge: the new S4 default drops data with no error.

Problems

1. packages/dox-orm/lib/src/model.dart:49: a guarded model drops the whole map and gives no error.

   • Problem: with no fillable and the default guarded => ['*'], _onlyFillable() returns an empty map.
   • What it causes: after the upgrade, User().create(req.body) still "works". It saves an empty row with only column defaults. I checked this on SQLite: DefaultUser().create({'name': 'ann', 'is_admin': 1}) saved {id: 1, name: null, is_admin: 0, ...}. If a column is NOT NULL, the user gets a database error that does not say why. update(req.body) returns 0, which looks like "no row matched". firstOrCreate / updateOrCreate also drop the where keys. So every app that relied on the old default loses data and does not see why.
   • This is not like Laravel. Laravel throws MassAssignmentException when a model is totally guarded ($guarded = ['*'] and no $fillable).
   • What to do: when allowed.isEmpty && blockAll && data.isNotEmpty, throw a clear error, for example a MassAssignmentError. It should name the model and the keys, and say how to fix it: add fillable, or set guarded => const []. Keep the silent drop for a model that has a fillable list (that is what Laravel does too). Add a test for create(), update() and firstOrCreate() on DefaultUser.

2. Docs say the opposite now (dartondox/dox). This PR is BREAKING. But the docs site still describes the old behaviour, and the CHANGELOG is not changed.

   • docs/database/model/index.md:256-258 says create() and the others "take any key of the map". Change it to the new default, and show fillable and the guarded => const [] opt-out.
   • docs/database/postgres-native.md:69 says "useSSL: true gives this mode" for require. Move that to verify-full. Say that a server with a private CA (for example AWS RDS) needs sslRootCert. Say that sslMode: 'require' turns the check off.
   • docs/reference/troubleshooting.md:157-158: add a row for "certificate check fails after useSSL: true".
   • What to do: open a linked docs PR in dartondox/dox, and add a CHANGELOG entry with a "Breaking" note for S4 and S5. Writing it at release time is OK if it is tracked, but the model page must not go live with the old text.

3. packages/dox-orm/lib/src/utils/helper.dart:92-104: GROUP BY / HAVING in a write builds SQL that fails. Small issue.

   • Problem: the sub query is SELECT t.id ... GROUP BY name. Postgres rejects it, and MySQL 8 rejects it with ONLY_FULL_GROUP_BY, unless the group is by the primary key. On SQLite it runs, but it changes one random row of each group.
   • What to do: GROUP BY and HAVING make no sense in an UPDATE or DELETE. Throw an ArgumentError for them, and keep the sub query only for JOIN, ORDER BY and LIMIT.

4. packages/dox-orm/lib/src/utils/helper.dart:100-104: a table with no id column now fails on MySQL. Small issue, not blocking.

   • Problem: the sub query always uses the primary key. Before this PR, QueryBuilder.table('pivot').where(...).orderBy('x').limit(1).delete() worked on MySQL, because MySQL supports DELETE ... ORDER BY ... LIMIT. Now it fails with "unknown column id".
   • What to do: add one line to the dartdoc of update() and delete(). It should say that ORDER BY, LIMIT and JOIN need the primary key column. That is enough. A MySQL-only path is not worth the extra code.

Things I checked that are OK

• S4 write paths: create, insert, insertMultiple, upsert, update, firstOrNew, firstOrCreate (through create), updateOrCreate and increment / decrement extras all go through _onlyFillable. There is no fill() method. save(), soft delete, restore and the factory use a plain QueryBuilder or super.update(), so the guard does not touch them. This is correct.
• S4 opt-out: guarded => const <String>[] works. A model that sets only guarded => ['is_admin'] keeps the old Laravel-like behaviour (it takes every other key). This is fine.
• B1: a plain WHERE gives the same SQL as before. The IN (SELECT id FROM (...) AS dox_rows) wrap keeps the user's WHERE, join conditions, soft-delete scope and LIMIT/OFFSET inside the sub query. So nothing is dropped. The join tests check that only the matched rows change.
• S5: verify-full with no sslRootCert uses the Mozilla roots and the system roots, as the docs say. The escape hatch is extra['sslMode'] = 'require' or sslmode=require in the url. Unknown url values like ssl=true or sslmode=no-verify now fail closed (verify-full). This is the right direction. The native side (fix: native safety fixes [todo:S6,S5,B3] dox-postgres-native#9) already maps verify-full to the full check.
• Each fix has a test that fails without it. I ran the new tests against the main lib code: "a model is guarded by default", "tls modes" and all 5 write tests with order, limit or join fail. With the PR they all pass.
• Style matches the code around it: typed locals, short plain comments, no new dependencies.

Results

• dart analyze (dox-orm): no issues.
• DRIVER=sqlite dart test --concurrency=1 (local): 388 passed, 1 skipped.
• New tests with the old lib/ code (SQLite): 7 fail, as expected.
• Probe on SQLite: create() on a default model saves an empty row (problem 1).
• CI gh pr checks: lint pass, test pass. The CI log shows the ORM suite passed on SQLite, Postgres 16, MariaDB 11 and MySQL 8.4. So the "Not run: MySQL 8.4" line in the PR body is out of date. The MySQL derived table trick works there.
• Not run locally: Postgres and MySQL. I used the CI results for them.

[necessarylion]

Fixer reply

The owner decided: no breaking changes, except S4, which should throw and not save an empty row.

1. A guarded model drops the whole map with no error: fixed. A model with no fillable and the default guarded now throws MassAssignmentException when the map has keys. The error names the model and the keys, and says to add fillable or set guarded => const []. An empty map is still fine. A model with fillable still drops the other keys, as in Laravel. The test covers create(), update(), firstOrCreate() and insertMultiple() on DefaultUser, and checks that no row was saved or changed. Commit 140a615.
2. Docs and CHANGELOG: fixed for S4. There is now a CHANGELOG "Unreleased" entry with a Breaking note for S4 (55e834d). The docs PR is docs: models are guarded from mass assignment by default [todo:S4] dox#286. It changes the model page and the query builder notes, and it should merge with this PR's release. S5 is not changed here any more. The owner wants no breaking change for TLS, so useSSL: true stays require (commit 2b643eb reverts dc082ab). So postgres-native.md and troubleshooting.md are still right and are not changed.
3. GROUP BY / HAVING in a write: fixed. groupBy() and having() with update() or delete() now throw an ArgumentError. The sub query is only for JOIN, ORDER BY and LIMIT. There is a test. Commit a34e2b0.
4. A table with no id on MySQL: fixed in code, not only in the docs. You suggested a doc line. The owner wants no breaking changes, so MySQL with no JOIN and no OFFSET now keeps its own ORDER BY ... LIMIT, the same SQL as before. There is a test that runs on MySQL with a table that has no id. The doc comments of update() and delete() also say that the other cases need the primary key column. Commit a34e2b0.

Checks: dart analyze has no issues. dart test --concurrency=1 passes on SQLite (390), Postgres 16 and MariaDB (387 each). The PR body is updated.

---

Generated by Claude Code

[necessarylion]

Bot review

Owner decision on point 1: throw an error. When a model has no fillable (and guarded is not opened on purpose), create(), insert(), update() and the other mass-assignment paths must throw a clear MassAssignmentException, like Laravel. Name the model and the keys that were blocked. Never save an empty row or return 0 quietly. Add tests for create and update. Write this in the Breaking change section.

[necessarylion]

Bot review

Verdict: OK

No blocking problems. The fixes follow the owner's decisions. Two small notes below. They can be done in this PR or later.

Small notes (not blocking)

1. packages/dox-orm/lib/src/model.dart:887, :900, :946: the doc comments of update(), upsert() and insertMultiple() only say "keys ... are dropped".
   • Problem: they do not say that a model with no fillable now throws.
   • What it causes: a reader of the API docs may expect a quiet drop.
   • What to do: add one line to each: "A model with no fillable throws a [MassAssignmentException]." The same for create().
2. packages/dox-orm/CHANGELOG.md:3: the Breaking line lists the methods, but not the increment() / decrement() extras.
   • They throw too, when the extra map has keys. Add them to the list.

What I checked

• S4, every path throws. create, insert (calls create), insertMultiple, upsert, update, firstOrNew, firstOrCreate (calls create), updateOrCreate (both the create and the update branch) and the increment / decrement extras all go through _onlyFillable(). That function throws MassAssignmentException when there is no fillable, guarded has '*' and the map has keys. No empty row is saved and no quiet 0 is returned.
• Not affected: save(), soft delete (super.update and _thisRecord()), restore() (_thisRecord()), forceDelete() and the factory (save()) use a plain QueryBuilder. So they never hit the guard. An empty map is still fine.
• Opt-out: guarded => const [] takes every key, as on main. A model that sets only guarded => ['is_admin'] keeps the old behaviour. A model with fillable still drops other keys, like Laravel.
• Error message is clear. It names the model and the keys, and says both fixes: add fillable, or set guarded to an empty list.
• S5 revert is clean. git diff origin/main has no change in lib/src/drivers/ or connection_config.dart. useSSL: true is still require, as on main.
• B1: groupBy() and having() with update() / delete() throw an ArgumentError. This is not a break: that SQL failed on every driver before. On MySQL with no join and no offset, getWriteQuery() returns where + orderBy + limit. That is the same SQL as getCommonQuery() on main. A plain WHERE also gives the same SQL as main. JOIN and OFFSET on MySQL, and ORDER BY / LIMIT on Postgres and SQLite, were invalid SQL before. So the sub query does not break any working code.
• No other breaking changes. Only S4 changes behaviour that worked before. The many test models that now set guarded => const [] are test-only.
• Tests fail without the fixes. I ran the new tests with the lib/ code from main (plus the new exception class so it compiles). 7 failed: "a model is guarded by default" and 6 write tests (order/limit, offset, join, groupBy). With the PR, all pass. The MySQL test for a table with no id passed in CI on MySQL 8.4 and MariaDB 11.
• CHANGELOG has an Unreleased entry with a Breaking note for S4 and a Fix note for B1. It does not mention S5, which is right.
• Docs PR docs: models are guarded from mass assignment by default [todo:S4] dox#286 matches the code. The model page says a model with no fillable throws MassAssignmentException, a model with fillable drops other keys, and guarded => const [] is the opt-out. The query builder notes for upsert, find-or-create and update point to it. postgres-native.md is unchanged, which is right after the S5 revert.

Results

• dart analyze (dox-orm): no issues.
• DRIVER=sqlite dart test --concurrency=1 (local): 390 passed, 1 skipped.
• New tests with the old lib/ code (SQLite): 7 fail, as expected.
• gh pr checks 23: lint pass, test pass. The CI log shows the new tests passed on SQLite, Postgres, MariaDB 11 and MySQL 8.4.
• Not run locally: Postgres and MySQL. I used the CI results for them.