fix(model): namespace per-request query cache under request.wheels.$queryCache

changelog.d/3336-request-query-cache-namespace.fixed.md

@@ -0,0 +1 @@
+- The per-request finder cache is now namespaced under `request.wheels.$queryCache[ModelName]` instead of sitting directly in `request.wheels[ModelName]`. Because CFML struct keys are case-insensitive, the flat layout let a model name alias onto a framework-owned request key: an app with a model named `Tenant` — the documented name for the control-plane model in a database-per-tenant app — shared one key between its query cache and `request.wheels.tenant`. Two silent failures followed. `$clearRequestCache()`, which runs after every create/update/delete/bulk operation, wiped the resolved tenant to `{}`, so every tenant-scoped query later in that request fell back to the control-plane datasource and wrote to the wrong database with no error. And a `Tenant` finder running before `TenantResolver` (the obvious shape for a subdomain→tenant directory) populated `request.wheels.tenant` with query-cache entries, making an unresolved request look resolved to any `IsDefined("request.wheels.tenant")` guard. Caching behaviour is otherwise unchanged (#3336)

changelog.d/3336-tenant-shape-hardening.fixed.md

@@ -0,0 +1 @@
+- `tenant()` now treats a value on `request.wheels.tenant` as an active tenant only when it carries a non-empty `dataSource` — the same test `$tenantDataSource()` already applies before routing a query — and returns an empty struct otherwise. Previously it handed back whatever occupied the key, so a malformed value read as a resolved tenant to any `IsDefined("request.wheels.tenant")` or truthiness guard. Relatedly, `wheels.middleware.TenantResolver` now deletes any pre-existing value on the key when its resolver returns no match, instead of leaving a stale or foreign one to outlive resolution for the remainder of the request. Together these downgrade a malformed tenant context from wrong behaviour to a no-op. Every framework producer (`switchTenant()`, `TenantResolver`, `Job.$restoreTenantContext()`, `TenantMigrator`) already guarantees a non-empty `dataSource`, so correctly-resolved tenants are unaffected (#3336)

vendor/wheels/Global.cfc

@@ -726,11 +726,23 @@ return local.$wheels;
 	 * Returns the current tenant struct, or an empty struct if no tenant is active.
 	 * The tenant struct contains: `id`, `dataSource`, `config`, and `$locked`.
 	 *
+	 * A tenant only counts as active when it carries a non-empty `dataSource` — the same test
+	 * `$tenantDataSource()` applies before it routes a query. Anything else on the key reads as
+	 * no tenant rather than being handed back as though it were a resolved one, so a malformed
+	 * value degrades to a no-op instead of wrong behaviour (#3336). Every framework producer
+	 * (`switchTenant()`, `TenantResolver`, `Job.$restoreTenantContext()`, `TenantMigrator`)
+	 * already guarantees a non-empty `dataSource`, so this only filters foreign values.
+	 *
 	 * [section: Configuration]
 	 * [category: Multi-Tenancy]
 	 */
 	public struct function tenant() {
-		if (IsDefined("request.wheels.tenant")) {
+		if (
+			IsDefined("request.wheels.tenant")
+			&& IsStruct(request.wheels.tenant)
+			&& StructKeyExists(request.wheels.tenant, "dataSource")
+			&& Len(request.wheels.tenant.dataSource)
+		) {
 			return request.wheels.tenant;
 		}
 		return {};

vendor/wheels/middleware/TenantResolver.cfc

@@ -73,6 +73,19 @@ component implements="wheels.middleware.MiddlewareInterface" output="false" {
 
 			// Set on the built-in request scope (where $performQuery reads it)
 			request.wheels.tenant = local.tenant;
+		} else if (IsDefined("request.wheels.tenant")) {
+			// The resolver found no match. Drop any value already sitting on the key so a stale
+			// or foreign one can't outlive resolution and be read downstream as a resolved
+			// tenant — an unresolved request must look unresolved for the whole request (#3336).
+			//
+			// Guard with IsDefined on the full path, matching the finally block below. A
+			// `StructKeyExists(request, "wheels")` guard is NOT equivalent here: this function
+			// takes a parameter named `request`, and on Adobe 2025 the bare `request` token
+			// resolves differently between the StructKeyExists argument and the `request.wheels`
+			// member-access expression, so the guard passed and the delete then threw
+			// `Element WHEELS is undefined in REQUEST`. IsDefined resolves the whole dotted path
+			// in one evaluation, so it cannot disagree with itself.
+			StructDelete(request.wheels, "tenant");
 		}
 
 		try {

vendor/wheels/model/miscellaneous.cfc

@@ -1,9 +1,31 @@
 component {
+	/**
+	 * Internal function.
+	 * Creates this model's slot in the per-request query cache if it doesn't exist yet.
+	 *
+	 * The cache is namespaced under the reserved `$queryCache` key rather than sitting directly in
+	 * `request.wheels` under the bare model name. CFML struct keys are case-insensitive, so the flat
+	 * layout let a model name alias onto a framework-owned request key — a model named `Tenant`
+	 * shared one key with `request.wheels.tenant`, silently dropping tenant datasource routing (#3336).
+	 */
+	public void function $ensureRequestQueryCache() {
+		if (!StructKeyExists(request, "wheels")) {
+			request.wheels = {};
+		}
+		if (!StructKeyExists(request.wheels, "$queryCache")) {
+			request.wheels["$queryCache"] = {};
+		}
+		if (!StructKeyExists(request.wheels["$queryCache"], variables.wheels.class.modelName)) {
+			request.wheels["$queryCache"][variables.wheels.class.modelName] = {};
+		}
+	}
+
 	/**
 	 * Deletes all queries stored during the request for this model.
 	 */
 	public void function $clearRequestCache() {
-		request.wheels[variables.wheels.class.modelName] = {};
+		$ensureRequestQueryCache();
+		request.wheels["$queryCache"][variables.wheels.class.modelName] = {};
 	}
 
 	/**

vendor/wheels/model/read.cfc

@@ -310,10 +310,8 @@ component {
 			// Batch finders (findEach / findInBatches) opt out via $useRequestCache so their per-page results don't accumulate in the request scope for the remainder of the request.
 			local.useRequestCache = application.wheels.cacheQueriesDuringRequest && arguments.$useRequestCache;
 			if (local.useRequestCache) {
-				// Create a struct in the request scope to store cached queries.
-				if (!StructKeyExists(request.wheels, variables.wheels.class.modelName)) {
-					request.wheels[variables.wheels.class.modelName] = {};
-				}
+				// Create this model's slot in the request-scoped query cache namespace.
+				$ensureRequestQueryCache();
 
 				// Derive the request cache key from the SQL shell key computed above (it already encodes the model name and the full arguments struct) so we don't have to serialize all arguments a second time.
 				local.queryKey = $hashedKey(local.queryShellKey, local.originalWhere);
@@ -323,9 +321,9 @@ component {
 			if (
 				local.useRequestCache
 				&& !arguments.reload
-				&& StructKeyExists(request.wheels[variables.wheels.class.modelName], local.queryKey)
+				&& StructKeyExists(request.wheels["$queryCache"][variables.wheels.class.modelName], local.queryKey)
 			) {
-				local.findAll = request.wheels[variables.wheels.class.modelName][local.queryKey];
+				local.findAll = request.wheels["$queryCache"][variables.wheels.class.modelName][local.queryKey];
 			} else {
 				local.finderArgs = {};
 				local.finderArgs.sql = local.sql;
@@ -357,7 +355,7 @@ component {
 				local.findAll = variables.wheels.class.adapter.$querySetup(argumentCollection = local.finderArgs);
 				if (local.useRequestCache) {
 					// Store in request cache so we never run the exact same query twice in the same request.
-					request.wheels[variables.wheels.class.modelName][local.queryKey] = local.findAll;
+					request.wheels["$queryCache"][variables.wheels.class.modelName][local.queryKey] = local.findAll;
 				}
 			}
 

vendor/wheels/tests/_assets/models/Tenant.cfc

@@ -0,0 +1,14 @@
+component extends="Model" {
+
+	/*
+	 * Exists purely to exercise the request-query-cache key collision guarded by
+	 * requestQueryCacheTenantCollisionSpec (#3336): `Tenant` is the natural model name for the
+	 * control-plane model in a database-per-tenant app, and CFML struct keys are case-insensitive,
+	 * so it is the one model name that can alias onto framework-owned `request.wheels.tenant`.
+	 * Backed by the existing authors fixture table so no populate.cfm changes are needed.
+	 */
+	function config() {
+		table("c_o_r_e_authors");
+	}
+
+}

vendor/wheels/tests/specs/middleware/TenantResolverSpec.cfc

@@ -292,6 +292,29 @@ component extends="wheels.WheelsTest" {
 					expect(IsDefined("request.wheels.tenant")).toBeFalse();
 				});
 
+				// #3336 hardening: an unresolved request must look unresolved for its whole
+				// duration, not just after the finally block runs.
+				it("drops a pre-existing request.wheels.tenant when the resolver finds no match", function() {
+					var fn = function(req) {
+						return {};
+					};
+					var mw = new wheels.middleware.TenantResolver(resolver = fn);
+					var pipeline = new wheels.middleware.Pipeline(middleware = [mw]);
+
+					request.wheels.tenant = {id = "stale", dataSource = "stale_ds", config = {}};
+
+					var reqData = {cgi = {}};
+					var result = {hasTenant = true};
+
+					var handler = function(required struct request) {
+						result.hasTenant = IsDefined("request.wheels.tenant");
+						return "ok";
+					};
+					pipeline.run(request = reqData, coreHandler = handler);
+
+					expect(result.hasTenant).toBeFalse();
+				});
+
 				it("cleans up request.wheels.tenant even when next() throws", function() {
 					var fn = function(req) {
 						return {id = "t1", dataSource = "ds1"};

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

@@ -27,6 +27,31 @@ component extends="wheels.WheelsTest" {
 					expect(t.id).toBe("t1");
 					expect(t.dataSource).toBe("tenant_ds");
 				});
+
+				// #3336 hardening: a value on the key that isn't a resolved tenant must read as
+				// "no tenant" rather than being handed back as though it were one. This is the
+				// same test $tenantDataSource() already applies before routing a query.
+				it("returns empty struct when the value on the key has no dataSource", () => {
+					request.wheels.tenant = {someQueryHash = {}};
+					var t = g.tenant();
+
+					expect(t).toBeStruct();
+					expect(StructIsEmpty(t)).toBeTrue();
+				});
+
+				it("returns empty struct when dataSource is present but empty", () => {
+					request.wheels.tenant = {id = "t1", dataSource = "", config = {}};
+					var t = g.tenant();
+
+					expect(StructIsEmpty(t)).toBeTrue();
+				});
+
+				it("agrees with $tenantDataSource() on what counts as resolved", () => {
+					request.wheels.tenant = {someQueryHash = {}};
+
+					expect(StructIsEmpty(g.tenant())).toBeTrue();
+					expect(g.$tenantDataSource()).toBe(application.wheels.dataSourceName);
+				});
 			});
 
 			describe("$tenantDataSource()", () => {

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

@@ -17,16 +17,16 @@ component extends="wheels.WheelsTest" {
 			it("stores a single entry per unique findAll call when enabled", () => {
 				application.wheels.cacheQueriesDuringRequest = true;
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 			})
 
 			it("keeps distinct entries for same-shape queries that differ only by where values", () => {
 				application.wheels.cacheQueriesDuringRequest = true;
 				var djurner = model("author").findAll(where = "lastName = 'Djurner'");
 				var petruzzi = model("author").findAll(where = "lastName = 'Petruzzi'");
-				expect(StructCount(request.wheels["author"])).toBe(2);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(2);
 				expect(djurner.recordCount).toBe(1);
 				expect(petruzzi.recordCount).toBe(1);
 				expect(djurner.lastName).toBe("Djurner");
@@ -36,7 +36,7 @@ component extends="wheels.WheelsTest" {
 			it("does not store query results when cacheQueriesDuringRequest is disabled", () => {
 				application.wheels.cacheQueriesDuringRequest = false;
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(0);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(0);
 			})
 
 			it("findEach does not accumulate per-batch queries in the request cache", () => {
@@ -52,7 +52,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Only the single up-front COUNT query may be cached, the per-batch id/data queries must not accumulate.
-				expect(StructCount(request.wheels["author"])).toBeLTE(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBeLTE(1);
 				expect(result.count).toBe(expectedTotal);
 			})
 
@@ -79,7 +79,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Pre-fix the empty case ran a second COUNT (findAll only honors `count` when > 0), which showed up as a second cached entry.
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				expect(result.count).toBe(0);
 			})
 
@@ -93,7 +93,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Pre-fix the empty case ran a second COUNT (findAll only honors `count` when > 0), which showed up as a second cached entry.
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				expect(result.batchCount).toBe(0);
 			})
 
@@ -111,7 +111,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Only the single up-front COUNT query may be cached, the per-batch id/data queries must not accumulate.
-				expect(StructCount(request.wheels["author"])).toBeLTE(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBeLTE(1);
 				expect(result.totalRecords).toBe(expectedTotal);
 				expect(result.batchCount).toBeGTE(2);
 			})

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

@@ -0,0 +1,118 @@
+/**
+ * Regression coverage for #3336.
+ *
+ * The per-request finder cache used to key itself on the bare model name directly in
+ * `request.wheels`. CFML struct keys are case-insensitive, so an app with a model named `Tenant`
+ * — the documented name for the control-plane model in a database-per-tenant app — had its query
+ * cache and its resolved-tenant context aliased onto the single key `request.wheels.tenant`.
+ *
+ * Two failure modes followed:
+ *   1. Write path: `$clearRequestCache()` (called after every create/update/delete/bulk op) set
+ *      `request.wheels.tenant = {}`, wiping the resolved tenant for the rest of the request. Every
+ *      later tenant-scoped query silently fell back to the control-plane datasource.
+ *   2. Read path: a `Tenant` finder running before resolution created `request.wheels.tenant` as a
+ *      query-cache struct, so `IsDefined("request.wheels.tenant")` reported an unresolved request
+ *      as resolved.
+ *
+ * The cache now lives under the reserved `request.wheels.$queryCache` sub-struct.
+ */
+component extends="wheels.WheelsTest" {
+
+	function run() {
+
+		g = application.wo;
+
+		describe("request query cache / tenant key collision (##3336)", () => {
+
+			beforeEach(() => {
+				originalCacheSetting = application.wheels.cacheQueriesDuringRequest;
+				StructDelete(request.wheels, "tenant");
+				StructDelete(request.wheels, "$queryCache");
+			})
+
+			afterEach(() => {
+				application.wheels.cacheQueriesDuringRequest = originalCacheSetting;
+				StructDelete(request.wheels, "tenant");
+				StructDelete(request.wheels, "$queryCache");
+			})
+
+			it("keeps the query cache out of the bare model-name key", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(StructKeyExists(request.wheels, "$queryCache")).toBeTrue();
+				expect(StructKeyExists(request.wheels["$queryCache"], "Tenant")).toBeTrue();
+				// The bare key is what aliased onto request.wheels.tenant.
+				expect(StructKeyExists(request.wheels, "Tenant")).toBeFalse();
+			})
+
+			it("still caches repeat finder calls once namespaced", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(StructCount(request.wheels["$queryCache"]["Tenant"])).toBe(1);
+			})
+
+			// Failure mode 2 — read path.
+			it("does not fabricate a resolved tenant when a Tenant finder runs before resolution", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(IsDefined("request.wheels.tenant")).toBeFalse();
+				expect(StructIsEmpty(g.tenant())).toBeTrue();
+			})
+
+			// Failure mode 1 — write path, via the helper every write path calls.
+			it("does not erase resolved tenant context when a Tenant model clears its request cache", () => {
+				request.wheels.tenant = {id = "acme", dataSource = "tenant_acme", config = {}, "$locked" = true};
+
+				model("Tenant").$clearRequestCache();
+
+				expect(IsDefined("request.wheels.tenant")).toBeTrue();
+				expect(request.wheels.tenant.id).toBe("acme");
+				expect(request.wheels.tenant.dataSource).toBe("tenant_acme");
+				expect(g.$tenantDataSource()).toBe("tenant_acme");
+			})
+
+			// Failure mode 1 — write path, through a real bulk write rather than the helper directly.
+			// The where clause matches nothing, so this mutates no fixture data but still reaches
+			// $updateAll() -> $clearRequestCache(). The tenant datasource must be the real one: the
+			// write executes for real, and pointing it at a non-existent datasource throws inside
+			// updateAll's transaction and leaves transaction state dirty for later specs.
+			it("keeps the resolved tenant intact after a Tenant write in the same request", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				request.wheels.tenant = {
+					id = "acme",
+					dataSource = application.wheels.dataSourceName,
+					config = {},
+					"$locked" = true
+				};
+
+				model("Tenant").updateAll(
+					where = "lastName = 'NoSuchTenantXYZ'",
+					instantiate = false,
+					firstName = "ignored"
+				);
+
+				expect(IsDefined("request.wheels.tenant")).toBeTrue();
+				expect(request.wheels.tenant.id).toBe("acme");
+				expect(g.$tenantDataSource()).toBe(application.wheels.dataSourceName);
+			})
+
+			it("clears only its own model's cache, leaving sibling models untouched", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("author").findAll(where = "lastName = 'Djurner'");
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
+
+				model("Tenant").$clearRequestCache();
+
+				expect(StructCount(request.wheels["$queryCache"]["Tenant"])).toBe(0);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
+			})
+
+		})
+
+	}
+}