fix(model): harden model-layer SHOULDs M2–M8

CLAUDE.md

@@ -412,7 +412,7 @@ model("User").byRole("admin").findAll(page=1, perPage=25);
 user.isDraft();                    // true/false
 model("User").draft().findAll();
 
-// Chainable query builder (injection-safe; values auto-quoted)
+// Chainable query builder (2-/3-arg where is injection-safe; 1-arg is raw SQL)
 model("User")
     .where("status", "active")
     .where("age", ">", 18)

changelog.d/model-hardener-m2-m8.added.md

@@ -0,0 +1 @@
+- `set(massAssignmentStrict=true)` fail-closes mass assignment when a model has neither `accessibleProperties()` nor `protectedProperties()`; the default remains open for compatibility

changelog.d/model-hardener-m2-m8.changed.md

@@ -0,0 +1,2 @@
+- `validatesUniquenessOf` now defaults `includeSoftDeletes` to `false`, so a soft-deleted value can be reused unless the caller opts in (`includeSoftDeletes=true`)
+- Query builder docs now state that single-argument `where()` / `orWhere()` is a raw-SQL passthrough; the 2- and 3-argument forms remain the parameterized contract

changelog.d/model-hardener-m2-m8.fixed.md

@@ -0,0 +1,4 @@
+- `create()` no longer mass-assigns properties onto the shared class model before instantiating the record
+- Nested `hasMany` collection keys no longer use a `GetTickCount` window heuristic to decide "new" vs existing; form identities use a `new-` prefix (or `_new`), and a failed `findByKey` no longer stamps the key as the child primary key
+- `hasChanged()` now treats `StructDelete` of a persisted property as a change
+- `findAll(returnAs="struct")` invokes `afterFind` against each row struct instead of the shared class model, so callbacks that read persisted columns no longer depend on `create()` leaking onto the class

vendor/wheels/events/init/functions.cfm

@@ -749,7 +749,8 @@
 		application.$wheels.functions.validatesPresenceOf = {message = "[property] can't be empty"};
 		application.$wheels.functions.validatesUniquenessOf = {
 			message = "[property] has already been taken",
-			allowBlank = false
+			allowBlank = false,
+			includeSoftDeletes = false
 		};
 		application.$wheels.functions.verifies = {handler = ""};
 		application.$wheels.functions.wordTruncate = {length = 5, truncateString = "..."};

vendor/wheels/events/init/security.cfm

@@ -62,4 +62,9 @@
 		// reload rate-limit keying. Leave false unless the app sits behind a trusted reverse proxy
 		// that overwrites — never appends to — these headers.
 		application.$wheels.trustProxyHeaders = false;
+
+		// Mass assignment is open unless accessibleProperties() or
+		// protectedProperties() is configured. Set true to fail closed when
+		// neither list exists — a breaking opt-in, not the framework default.
+		application.$wheels.massAssignmentStrict = false;
 </cfscript>

vendor/wheels/global/settings.cfm

@@ -87,7 +87,7 @@
 			&& StructKeyExists(request.wheels.tenant, "config")
 			&& StructKeyExists(request.wheels.tenant.config, arguments.name)
 			&& !ListFindNoCase(
-				"encryptionAlgorithm,encryptionSecretKey,encryptionEncoding,CSRFProtection,csrfStore,reloadPassword,obfuscateUrls",
+				"encryptionAlgorithm,encryptionSecretKey,encryptionEncoding,CSRFProtection,csrfStore,reloadPassword,obfuscateUrls,massAssignmentStrict",
 				arguments.name
 			)
 		) {

vendor/wheels/model/create.cfc

@@ -27,7 +27,8 @@ component {
 		$args(name = "create", args = arguments);
 		$setProperties(
 			argumentCollection = arguments,
-			filterList = "properties,parameterize,reload,validate,transaction,callbacks"
+			filterList = "properties,parameterize,reload,validate,transaction,callbacks",
+			setOnModel = false
 		);
 		local.rv = new (argumentCollection = arguments);
 		local.rv.save(

vendor/wheels/model/nestedproperties.cfc

@@ -194,38 +194,26 @@ component {
 		}
 		if (IsStruct(arguments.value)) {
 			for (local.item in arguments.value) {
-				// Check to see if the id is a tickcount, if so the object is new.
-				if (IsNumeric(local.item) && Ceiling(Right(GetTickCount(), 12) / 900000000) == Ceiling(local.item / 900000000)) {
-					ArrayAppend(
-						this[arguments.property],
-						$getAssociationObject(
-							property = arguments.property,
-							value = arguments.value[local.item],
-							association = arguments.association,
-							delete = arguments.delete
-						)
-					);
-					$updateCollectionObject(property = arguments.property, value = arguments.value[local.item]);
-				} else {
-					// Get our primary keys.
+				if (!$isNewNestedCollectionKey(collectionKey = local.item, value = arguments.value[local.item])) {
+					// Existing-row key: copy the struct key into the primary key fields.
 					local.keys = local.model.primaryKey();
 					local.itemArray = ListToArray(local.item, ",", true);
 					local.iEnd = ListLen(local.keys);
 					for (local.i = 1; local.i <= local.iEnd; local.i++) {
 						arguments.value[local.item][ListGetAt(local.keys, local.i)] = local.itemArray[local.i];
 					}
-
-					ArrayAppend(
-						this[arguments.property],
-						$getAssociationObject(
-							property = arguments.property,
-							value = arguments.value[local.item],
-							association = arguments.association,
-							delete = arguments.delete
-						)
-					);
-					$updateCollectionObject(property = arguments.property, value = arguments.value[local.item]);
 				}
+
+				ArrayAppend(
+					this[arguments.property],
+					$getAssociationObject(
+						property = arguments.property,
+						value = arguments.value[local.item],
+						association = arguments.association,
+						delete = arguments.delete
+					)
+				);
+				$updateCollectionObject(property = arguments.property, value = arguments.value[local.item]);
 			}
 		} else if (IsArray(arguments.value)) {
 			for (local.i = 1; local.i <= ArrayLen(arguments.value); local.i++) {
@@ -314,7 +302,7 @@ component {
 			local.args.key = $createPrimaryKeyList(params = arguments.value, keys = local.model.primaryKey());
 			if (IsObject(arguments.value)) {
 				local.object = arguments.value;
-			} else if (Len(local.args.key)) {
+			} else 			if (Len(local.args.key)) {
 				local.object = local.model.findByKey(argumentCollection = local.args);
 			}
 
@@ -324,6 +312,8 @@ component {
 				local.delete = true;
 			}
 			if (!IsObject(local.object) && !local.delete) {
+				// Key was not a persisted PK — do not stamp it onto a new child.
+				$clearNestedPrimaryKeys(value = arguments.value, keys = local.model.primaryKey());
 				StructDelete(local.args, "key");
 				return $invoke(componentReference = local.model, method = "new", invokeArgs = local.args);
 			} else if (Len(local.args.key) && local.delete && arguments.association.nested.delete && arguments.delete) {
@@ -334,6 +324,36 @@ component {
 		return local.object;
 	}
 
+	/**
+	 * True when a hasMany nested-properties struct key identifies a new
+	 * child: an explicit `_new` flag, a blank key, or a `new` / `new-*` /
+	 * `new_*` token from `key($returnTickCountWhenNew=true)`.
+	 */
+	public boolean function $isNewNestedCollectionKey(required any collectionKey, required struct value) {
+		if (StructKeyExists(arguments.value, "_new") && IsBoolean(arguments.value["_new"]) && arguments.value["_new"]) {
+			return true;
+		}
+		local.keyString = ToString(arguments.collectionKey);
+		if (!Len(Trim(local.keyString))) {
+			return true;
+		}
+		if (ReFindNoCase("^new([-_].*)?$", local.keyString)) {
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	 * Removes primary-key fields from a nested-properties value struct so a
+	 * failed findByKey does not stamp a form identity onto a new child.
+	 */
+	public void function $clearNestedPrimaryKeys(required struct value, required string keys) {
+		local.iEnd = ListLen(arguments.keys);
+		for (local.i = 1; local.i <= local.iEnd; local.i++) {
+			StructDelete(arguments.value, ListGetAt(arguments.keys, local.i));
+		}
+	}
+
 	/**
 	 * Internal function.
 	 */

vendor/wheels/model/properties.cfc

@@ -217,7 +217,7 @@ component {
 			}
 		}
 		if (!Len(local.rv) && arguments.$returnTickCountWhenNew) {
-			local.rv = variables.wheels.tickCountId;
+			local.rv = "new-" & variables.wheels.tickCountId;
 		}
 
 		/* To fix the bug below:
@@ -398,6 +398,9 @@ component {
 						return true;
 					}
 				}
+			} else if (StructKeyExists(variables.$persistedProperties, local.key)) {
+				// Persisted property was removed from the instance (e.g. StructDelete).
+				return true;
 			}
 		}
 		// if we get here, it means that all of the properties that were checked had a value in
@@ -499,20 +502,26 @@ component {
 		}
 
 		// loop through the properties and see if they can be set based off of the accessible properties lists
+		local.hasWhiteList = StructKeyExists(variables.wheels.class.accessibleProperties, "whiteList");
+		local.hasBlackList = StructKeyExists(variables.wheels.class.accessibleProperties, "blackList");
+		local.strictOpen = arguments.$useFilterLists && !local.hasWhiteList && !local.hasBlackList && $get("massAssignmentStrict");
 		for (local.key in arguments.properties) {
 			// required to ignore null keys
 			if (StructKeyExists(arguments.properties, local.key)) {
 				local.accessible = true;
+				if (local.strictOpen) {
+					local.accessible = false;
+				}
 				if (
 					arguments.$useFilterLists &&
-					StructKeyExists(variables.wheels.class.accessibleProperties, "whiteList")
+					local.hasWhiteList
 					&& !StructKeyExists(variables.wheels.class.accessibleProperties.whiteList, local.key)
 				) {
 					local.accessible = false;
 				}
 				if (
 					arguments.$useFilterLists
-					&& StructKeyExists(variables.wheels.class.accessibleProperties, "blackList")
+					&& local.hasBlackList
 					&& StructKeyExists(variables.wheels.class.accessibleProperties.blackList, local.key)
 				) {
 					local.accessible = false;

vendor/wheels/model/query/QueryBuilder.cfc

@@ -1,5 +1,5 @@
 /**
- * A chainable, injection-safe query builder for Wheels models.
+ * A chainable query builder for Wheels models.
  * Provides a fluent API alternative to the traditional `findAll(where="...")` string approach.
  *
  * Usage:
@@ -10,8 +10,11 @@
  *       .limit(25)
  *       .get();
  *
- * All values are safely quoted using the model's database adapter, preventing SQL injection.
- * The builder ultimately delegates to the model's standard finder methods (findAll, findOne, etc.).
+ * The 2- and 3-argument `where()` / `orWhere()` forms quote values through the
+ * model's database adapter. The single-argument form is a raw-SQL passthrough
+ * for trusted clauses only — it is not parameterized. Prefer the 2-/3-arg forms
+ * for user input. The builder ultimately delegates to the model's standard
+ * finder methods (findAll, findOne, etc.).
  */
 component output="false" {
 

vendor/wheels/model/serialize.cfc

@@ -174,9 +174,14 @@ component {
 					Called the afterFind hook, the hook adds the arguments defined in the hook to the object so get the properties using properties() function and then append the property struct to the individual record struct
 				*/
 				if (arguments.callbacks && structKeyExists(this, 'afterFindCallback') && arguments.returnas eq "struct") {
-					$callback("afterFind", arguments.callbacks);
-					local.objectProps = properties();
-					structAppend(local.struct, local.objectProps);
+					// Invoke against the row struct, not the class model. The previous
+					// `$callback("afterFind")` + `properties()` path ran afterFind on
+					// `this` (the shared class) and only worked when create() had leaked
+					// column values onto that singleton.
+					local.afterFindResult = $invoke(method = "afterFindCallback", invokeArgs = local.struct);
+					if (StructKeyExists(local, "afterFindResult") && IsStruct(local.afterFindResult)) {
+						structAppend(local.struct, local.afterFindResult);
+					}
 				}
 
 				ArrayAppend(local.rv, local.struct);

vendor/wheels/model/validations.cfc

@@ -310,7 +310,7 @@ component {
 		string scope = "",
 		string condition = "",
 		string unless = "",
-		boolean includeSoftDeletes = "true"
+		boolean includeSoftDeletes = "false"
 	) {
 		$args(name = "validatesUniquenessOf", args = arguments);
 		arguments.scope = $listClean(arguments.scope);
@@ -728,7 +728,7 @@ component {
 		required string message,
 		string scope = "",
 		struct properties = "#this.properties()#",
-		boolean includeSoftDeletes = "true"
+		boolean includeSoftDeletes = "false"
 	) {
 		if (!IsBoolean(variables.wheels.class.tableName) || variables.wheels.class.tableName) {
 			local.where = [];

vendor/wheels/public/docs/reference/model/accessibleproperties.txt

@@ -1,3 +1,7 @@
+// Mass assignment is open by default: with neither accessibleProperties() nor
+// protectedProperties(), every property can be set via new() / create() / update().
+// set(massAssignmentStrict=true) fail-closes that case (opt-in; not the default).
+
 // 1. Allow only `isActive` to be set through mass assignment (e.g. `updateAll()`, `new()`, `update()`).
 config() {
 	accessibleProperties("isActive");

vendor/wheels/public/docs/reference/model/protectedproperties.txt

@@ -1,3 +1,6 @@
+// Without accessibleProperties() or protectedProperties(), mass assignment is open.
+// set(massAssignmentStrict=true) fail-closes that case (opt-in; not the default).
+
 // 1. Protect a comma-delimited list of properties from mass assignment in `models/User.cfc`.
 // `firstName` and `lastName` cannot be changed via `updateAll()`, `new()`, `update()`, etc.
 function config() {

vendor/wheels/public/docs/reference/model/validatesuniquenessof.txt

@@ -16,5 +16,5 @@ validatesUniquenessOf(property="slug", when="onCreate");
 // 6. Run the check only when a condition is true
 validatesUniquenessOf(property="referralCode", condition="this.isAffiliate()");
 
-// 7. Exclude soft-deleted records from the uniqueness check so a previously-deleted value can be reused
-validatesUniquenessOf(property="username", includeSoftDeletes=false);
+// 7. Include soft-deleted records in the uniqueness check so a previously-deleted value stays reserved
+validatesUniquenessOf(property="username", includeSoftDeletes=true);

vendor/wheels/tests/specs/model/crudSpec.cfc

@@ -101,11 +101,11 @@ component extends="wheels.WheelsTest" {
 				StructDelete(author, "firstName")
 				result = author.hasChanged()
 
-				expect(result).toBeFalse()
+				expect(result).toBeTrue()
 
 				result = author.hasChanged("firstName")
 
-				expect(result).toBeFalse()
+				expect(result).toBeTrue()
 
 				result = author.hasChanged("somethingThatDoesNotExist")