fix(policy): fail-close empty scope and deny reserved actions

changelog.d/policy-hardener-s2-s9.fixed.md

@@ -0,0 +1,3 @@
+- `Policy.scope()` fail-closes with a no-rows chain when the resolved id list is empty, instead of calling `whereIn("id", [])`
+- `policyScope()` in production returns that empty chain after `InvalidCollection` and does not call `whereIn` on the unresolved collection
+- `authorize()` / `can()` deny reserved `init` and `scope` actions instead of Invoking those Policy methods

vendor/wheels/Policy.cfc

@@ -98,16 +98,49 @@ component {
 
 	/**
 	 * Narrows a collection to the records the user may see (used by `policyScope()`
-	 * for `index` actions). Default-deny: returns a no-rows chain. The empty
-	 * `whereIn` sets the query builder's injection-safe always-empty flag (see
-	 * ##2736) without interpolating the property name into SQL, so it composes
-	 * with any model, query-builder chain, or scope chain. Override in your
-	 * policy to widen.
+	 * for `index` actions). Default-deny: returns a no-rows chain that does not
+	 * call `whereIn` with an empty id list, so an empty resolved-id set cannot
+	 * become `IN ()` or match every row. Override in your policy to widen.
 	 *
 	 * @collection The model class (or chainable query builder / scope chain) to narrow.
 	 */
 	public any function scope(required any collection) {
-		return arguments.collection.whereIn("id", []);
+		return CreateObject("component", "wheels.Policy").init();
+	}
+
+	/**
+	 * The identity this policy was initialized with. An empty string is a guest.
+	 */
+	public any function currentUser() {
+		return variables.user;
+	}
+
+	/**
+	 * No-rows terminal for the default-deny `scope()` chain.
+	 */
+	public numeric function count() {
+		return 0;
+	}
+
+	/**
+	 * Empty query for the default-deny `scope()` chain.
+	 */
+	public query function findAll() {
+		return QueryNew("id");
+	}
+
+	/**
+	 * Keeps the default-deny chain empty when callers compose after `scope()`.
+	 */
+	public any function where() {
+		return this;
+	}
+
+	/**
+	 * Keeps the default-deny chain empty. Does not interpolate an empty `IN ()`.
+	 */
+	public any function whereIn() {
+		return this;
 	}
 
 }

vendor/wheels/controller/authorization.cfc

@@ -46,6 +46,7 @@ component {
 		if (
 			IsObject(local.policy)
 			&& Len(local.action)
+			&& !$isReservedPolicyAction(local.action)
 			&& StructKeyExists(local.policy, local.action)
 			&& IsCustomFunction(local.policy[local.action])
 		) {
@@ -94,6 +95,7 @@ component {
 		local.policy = $policyFor(arguments.record);
 		if (
 			!IsObject(local.policy)
+			|| $isReservedPolicyAction(arguments.action)
 			|| !StructKeyExists(local.policy, arguments.action)
 			|| !IsCustomFunction(local.policy[arguments.action])
 		) {
@@ -131,12 +133,17 @@ component {
 	 */
 	public any function policyScope(required any collection) {
 		local.modelName = $policyModelName(arguments.collection);
-		if (!Len(local.modelName) && $get("showErrorInformation")) {
-			Throw(
-				type = "Wheels.Policy.InvalidCollection",
-				message = "policyScope() could not derive a model from the passed collection.",
-				extendedInfo = "Pass the model class first and chain from the result, e.g. `policyScope(model(""Post"")).active().findAll()`. Query-builder and scope chains that are already in flight cannot be passed to policyScope()."
-			);
+		if (!Len(local.modelName)) {
+			if ($get("showErrorInformation")) {
+				Throw(
+					type = "Wheels.Policy.InvalidCollection",
+					message = "policyScope() could not derive a model from the passed collection.",
+					extendedInfo = "Pass the model class first and chain from the result, e.g. `policyScope(model(""Post"")).active().findAll()`. Query-builder and scope chains that are already in flight cannot be passed to policyScope()."
+				);
+			}
+			// Production InvalidCollection: fail-closed. Do not call whereIn on a
+			// collection we could not resolve (empty IN, matching-all, or no method).
+			return CreateObject("component", "wheels.Policy").init();
 		}
 		local.policy = $policyFor(arguments.collection);
 		if (!IsObject(local.policy)) {
@@ -212,6 +219,15 @@ component {
 		return capitalize(arguments.modelName) & "Policy";
 	}
 
+	/**
+	 * Internal function. `init` and `scope` are Policy lifecycle methods, not
+	 * grantable actions. authorize()/can() must not Invoke them (missing
+	 * `collection` on scope() is a 500, not Wheels.NotAuthorized).
+	 */
+	public boolean function $isReservedPolicyAction(required string actionName) {
+		return ListFindNoCase("init,scope", arguments.actionName) > 0;
+	}
+
 	/**
 	 * Internal function. Resolves the identity policies are evaluated against, in
 	 * order: (1) the DI service registered as `currentUser` when present, (2) the

vendor/wheels/tests/_assets/policies/CurrentUserStub.cfc

@@ -0,0 +1,13 @@
+/**
+ * DI `currentUser` service used by Authorization S6. getInstance("currentUser")
+ * returns this component; Policy stores it as the identity.
+ */
+component {
+
+	public any function init() {
+		this.id = 9001;
+		this.name = "policy-di-user";
+		return this;
+	}
+
+}

vendor/wheels/tests/_assets/policies/WhereInSpy.cfc

@@ -0,0 +1,26 @@
+/**
+ * Collection stand-in whose whereIn([]) would match everything. Used to prove
+ * Policy.scope() / policyScope() fail closed without calling whereIn.
+ */
+component {
+
+	this.whereInCalls = 0;
+	this.lastProperty = "";
+	this.lastValues = [];
+
+	public any function whereIn(required string property, required any values) {
+		this.whereInCalls = this.whereInCalls + 1;
+		this.lastProperty = arguments.property;
+		this.lastValues = arguments.values;
+		return this;
+	}
+
+	public numeric function count() {
+		return 99;
+	}
+
+	public query function findAll() {
+		return QueryNew("id", "integer", [[1]]);
+	}
+
+}

vendor/wheels/tests/specs/controller/AuthorizationSpec.cfc → vendor/wheels/tests/specs/Authorization/AuthorizationSpec.cfc

@@ -1,3 +1,14 @@
+/**
+ * Authorization policy layer. Desk IDs S1–S9 stay locked.
+ * PROVEN: S2 empty-id fail-closed, S5 production InvalidCollection, S6 DI/authenticator
+ * identity, S7 guest "", S8 production 403, S9 reserved action=scope/init.
+ * HELD: S1 unknown action still throws (no default-deny flip), S3 first-catch
+ * wrong-user failover (broken DI currentUser can still hit authenticator),
+ * S4 loose CF boolean grant.
+ *
+ * Directory-scoped so `wheels test --core --ci --filter=Authorization` discovers
+ * this folder (a single-file directory= scope finds 0 bundles).
+ */
 component extends="wheels.WheelsTest" {
 
 	function run() {
@@ -24,6 +35,10 @@ component extends="wheels.WheelsTest" {
 				application.wheels.policyPath = $savedPolicyPath
 				application.wheels.showErrorInformation = $savedShowError
 				StructDelete(request, "$policyTestUser")
+				try {
+					g.$header(statusCode = 200)
+				} catch (any e) {
+				}
 			})
 
 			describe("wheels.Policy base class", () => {
@@ -49,6 +64,25 @@ component extends="wheels.WheelsTest" {
 					expect(scoped.findAll().recordCount).toBe(0)
 				})
 
+				it("S2: empty resolved ids never become a matching-all or invalid-SQL whereIn", () => {
+					spy = CreateObject("component", "wheels.tests._assets.policies.WhereInSpy")
+					basePolicy = CreateObject("component", "wheels.Policy").init(user = {id = 1}, record = "")
+					scoped = basePolicy.scope(spy)
+
+					expect(g.model("post").count()).toBeGT(0)
+					expect(spy.whereInCalls).toBe(0)
+					expect(scoped.count()).toBe(0)
+					expect(scoped.findAll().recordCount).toBe(0)
+				})
+
+				it("S7: Policy.cfc missing user is the guest empty string", () => {
+					guestPolicy = CreateObject("component", "wheels.Policy").init()
+
+					expect(guestPolicy.currentUser()).toBe("")
+					expect(IsSimpleValue(guestPolicy.currentUser())).toBeTrue()
+					expect(IsObject(guestPolicy.currentUser())).toBeFalse()
+				})
+
 				it("default-denies through an app policy that overrides nothing", () => {
 					request.$policyTestUser = {id = author.id}
 
@@ -119,6 +153,42 @@ component extends="wheels.WheelsTest" {
 						type = "Wheels.NotAuthorized"
 					)
 				})
+
+				it("S8: authorize() denial in production is HTTP 403, not a silent allow", () => {
+					application.wheels.showErrorInformation = false
+					request.$policyTestUser = {id = otherAuthor.id}
+					denied = {allowed = false, status = 0, type = ""}
+					try {
+						_controller.authorize(record = post, action = "update")
+						denied.allowed = true
+					} catch (any e) {
+						denied.type = e.type
+					}
+					denied.status = Val(g.$statusCode())
+
+					expect(denied.allowed).toBeFalse()
+					expect(denied.status).toBe(403)
+					try {
+						g.$header(statusCode = 200)
+					} catch (any e) {
+					}
+				})
+
+				it("S9: authorize() with action=scope throws Wheels.NotAuthorized and does not Invoke scope", () => {
+					request.$policyTestUser = {id = author.id}
+
+					expect(() => _controller.authorize(record = post, action = "scope")).toThrow(
+						type = "Wheels.NotAuthorized"
+					)
+				})
+
+				it("S9: authorize() with action=init throws Wheels.NotAuthorized and does not Invoke init", () => {
+					request.$policyTestUser = {id = author.id}
+
+					expect(() => _controller.authorize(record = post, action = "init")).toThrow(
+						type = "Wheels.NotAuthorized"
+					)
+				})
 			})
 
 			describe("can()", () => {
@@ -190,6 +260,30 @@ component extends="wheels.WheelsTest" {
 
 					expect(() => _controller.policyScope(builder)).toThrow(type = "Wheels.Policy.InvalidCollection")
 				})
+
+				it("S5: production InvalidCollection fail-closes without calling whereIn", () => {
+					application.wheels.showErrorInformation = false
+					spy = CreateObject("component", "wheels.tests._assets.policies.WhereInSpy")
+					scoped = _controller.policyScope(spy)
+
+					expect(spy.whereInCalls).toBe(0)
+					expect(scoped.count()).toBe(0)
+				})
+
+				it("S5: production InvalidCollection does not fall through to whereIn on a collection without whereIn", () => {
+					application.wheels.showErrorInformation = false
+					bad = {notAModel = true}
+					state = {threw = false, count = -1}
+					try {
+						scoped = _controller.policyScope(bad)
+						state.count = scoped.count()
+					} catch (any e) {
+						state.threw = true
+					}
+
+					expect(state.threw).toBeFalse()
+					expect(state.count).toBe(0)
+				})
 			})
 
 			describe("missing policy class", () => {
@@ -227,6 +321,49 @@ component extends="wheels.WheelsTest" {
 					expect(plain.can("update", post)).toBeFalse()
 					expect(plain.can("show", post)).toBeTrue()
 				})
+
+				it("S6: policy identity comes from the DI currentUser service", () => {
+					savedDi = application.wheelsdi
+					di = new wheels.Injector(binderPath = "wheels.tests._assets.di.TestBindings")
+					di.map("currentUser").to("wheels.tests._assets.policies.CurrentUserStub")
+					application.wheelsdi = di
+					identity = {id = "", name = ""}
+					try {
+						plain = g.controller("test", {controller = "test", action = "show"})
+						resolved = plain.$currentUserForPolicy()
+						identity.id = resolved.id
+						identity.name = resolved.name
+					} finally {
+						application.wheelsdi = savedDi
+					}
+
+					expect(identity.id).toBe(9001)
+					expect(identity.name).toBe("policy-di-user")
+				})
+
+				it("S6: policy identity comes from the authenticator strategy currentUser()", () => {
+					savedDi = application.wheelsdi
+					di = new wheels.Injector(binderPath = "wheels.tests._assets.di.TestBindings")
+					di.map("authenticator").to("wheels.auth.Authenticator").asSingleton()
+					application.wheelsdi = di
+					identity = {id = "", name = ""}
+					try {
+						auth = di.getInstance("authenticator")
+						strategy = new wheels.auth.SessionStrategy()
+						strategy.login(principal = {id = 4242, name = "policy-auth-user"})
+						auth.registerStrategy(name = "session", strategy = strategy)
+						plain = g.controller("test", {controller = "test", action = "show"})
+						resolved = plain.$currentUserForPolicy()
+						identity.id = resolved.id
+						identity.name = resolved.name
+					} finally {
+						application.wheelsdi = savedDi
+						StructDelete(session, "wheels")
+					}
+
+					expect(identity.id).toBe(4242)
+					expect(identity.name).toBe("policy-auth-user")
+				})
 			})
 
 			describe("routable surface", () => {