From 7cbe4db8b0a29bae415df7a2b46d08ecc6cc3804 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 04:17:17 +0000 Subject: [PATCH 1/2] test(controller): prove-red hardener B1-B4 B7-B8 request lifecycle Failing specs for untrusted controller/action retarget, ungated rewrite headers, unrestricted _method including GET/HEAD, before filter return false fail-open, appendToKey key collapse, and case-sensitive filter type. Co-authored-by: Peter Amiri Signed-off-by: Cursor Agent --- .../_assets/controllers/HardenerLifecycle.cfc | 34 +++ .../specs/hardener/ControllerHardenerSpec.cfc | 153 ++++++++++++ .../specs/hardener/DispatchHardenerSpec.cfc | 228 ++++++++++++++++++ 3 files changed, 415 insertions(+) create mode 100644 vendor/wheels/tests/_assets/controllers/HardenerLifecycle.cfc create mode 100644 vendor/wheels/tests/specs/hardener/ControllerHardenerSpec.cfc create mode 100644 vendor/wheels/tests/specs/hardener/DispatchHardenerSpec.cfc diff --git a/vendor/wheels/tests/_assets/controllers/HardenerLifecycle.cfc b/vendor/wheels/tests/_assets/controllers/HardenerLifecycle.cfc new file mode 100644 index 000000000..b6a10112f --- /dev/null +++ b/vendor/wheels/tests/_assets/controllers/HardenerLifecycle.cfc @@ -0,0 +1,34 @@ +component extends="Controller" { + + function config() { + filters(through = "denyUnlessAllowed", only = "secret"); + filters(through = "denyCased", only = "casedAction", type = "Before"); + } + + function secret() { + request.hardenerSecretRan = true; + renderText("secret-ok"); + } + + function casedAction() { + request.hardenerCasedRan = true; + renderText("cased-ok"); + } + + function cachedShow() { + renderText(request.hardenerCachePayload); + } + + private function denyUnlessAllowed() { + request.hardenerDenyRan = true; + if (!StructKeyExists(request, "hardenerAllow") || !request.hardenerAllow) { + return false; + } + } + + private function denyCased() { + request.hardenerCasedFilterRan = true; + return false; + } + +} diff --git a/vendor/wheels/tests/specs/hardener/ControllerHardenerSpec.cfc b/vendor/wheels/tests/specs/hardener/ControllerHardenerSpec.cfc new file mode 100644 index 000000000..8c796f63d --- /dev/null +++ b/vendor/wheels/tests/specs/hardener/ControllerHardenerSpec.cfc @@ -0,0 +1,153 @@ +/** + * Hardener BLOCKERs B4, B7, B8 (controller filters and action cache keys). + * + * Directory-scoped so `wheels test --core --ci --filter=hardener` + * discovers this folder (a single-file directory= scope finds 0 bundles). + */ +component extends="wheels.WheelsTest" { + + function run() { + + g = application.wo + + describe("B4 before-filter return false halts processAction", () => { + + beforeEach(() => { + request.hardenerSecretRan = false + request.hardenerDenyRan = false + request.hardenerAllow = false + params = {controller = "hardenerLifecycle", action = "secret"} + _controller = g.controller("hardenerLifecycle", params) + }) + + it("does not run the action when a before filter returns false", () => { + _controller.processAction() + + expect(request.hardenerDenyRan).toBeTrue() + expect(request.hardenerSecretRan).toBeFalse() + }) + + it("still runs the action when the before filter does not return false", () => { + request.hardenerAllow = true + _controller.processAction() + + expect(request.hardenerDenyRan).toBeTrue() + expect(request.hardenerSecretRan).toBeTrue() + }) + + }) + + describe("B8 filter type comparison is case-insensitive", () => { + + beforeEach(() => { + request.hardenerCasedRan = false + request.hardenerCasedFilterRan = false + params = {controller = "hardenerLifecycle", action = "casedAction"} + _controller = g.controller("hardenerLifecycle", params) + }) + + it("stores type=Before as canonical before so filterChain(before) includes it", () => { + var before = _controller.filterChain("before") + var found = false + for (var filter in before) { + if (filter.through == "denyCased") { + found = true + expect(filter.type).toBeWithCase("before") + } + } + expect(found).toBeTrue() + }) + + it("runs a type=Before filter during $runFilters(type=before)", () => { + _controller.$runFilters(type = "before", action = "casedAction") + expect(request.hardenerCasedFilterRan).toBeTrue() + }) + + it("does not run the action when a type=Before filter returns false", () => { + _controller.processAction() + + expect(request.hardenerCasedFilterRan).toBeTrue() + expect(request.hardenerCasedRan).toBeFalse() + }) + + }) + + describe("B7 caches appendToKey does not collapse distinct keys", () => { + + beforeEach(() => { + _hadCacheActions = StructKeyExists(application.wheels, "cacheActions") + if (_hadCacheActions) { + _priorCacheActions = application.wheels.cacheActions + } + application.wheels.cacheActions = true + _originalForm = Duplicate(form) + StructClear(form) + if (StructKeyExists(session, "user")) { + _priorSessionUser = Duplicate(session.user) + } + if (StructKeyExists(session, "hardenerTenantId")) { + _priorTenantId = session.hardenerTenantId + } + _controller = g.controller("hardenerLifecycle", {controller = "hardenerLifecycle", action = "cachedShow"}) + _controller.$clearCachableActions() + g.$clearCache("action") + }) + + afterEach(() => { + _controller.$clearCachableActions() + g.$clearCache("action") + StructClear(form) + StructAppend(form, _originalForm, false) + if (StructKeyExists(variables, "_priorSessionUser")) { + session.user = _priorSessionUser + } else { + StructDelete(session, "user") + } + if (StructKeyExists(variables, "_priorTenantId")) { + session.hardenerTenantId = _priorTenantId + } else { + StructDelete(session, "hardenerTenantId") + } + if (_hadCacheActions) { + application.wheels.cacheActions = _priorCacheActions + } else { + StructDelete(application.wheels, "cacheActions") + } + }) + + it("does not serve one user a cached response built for another when appendToKey is nested", () => { + _controller.caches(action = "cachedShow", appendToKey = "session.user.id") + + session.user = {id = "alice"} + request.hardenerCachePayload = "payload-alice" + var alice = g.controller("hardenerLifecycle", {controller = "hardenerLifecycle", action = "cachedShow"}) + alice.processAction() + expect(alice.response()).toBe("payload-alice") + + session.user = {id = "bob"} + request.hardenerCachePayload = "payload-bob" + var bob = g.controller("hardenerLifecycle", {controller = "hardenerLifecycle", action = "cachedShow"}) + bob.processAction() + expect(bob.response()).toBe("payload-bob") + }) + + it("throws instead of silently omitting an undefined appendToKey item", () => { + _controller.caches(action = "cachedShow", appendToKey = "session.hardenerTenantId") + StructDelete(session, "hardenerTenantId") + request.hardenerCachePayload = "secret-a" + + var thrown = {type = ""} + try { + var first = g.controller("hardenerLifecycle", {controller = "hardenerLifecycle", action = "cachedShow"}) + first.processAction() + } catch (any e) { + thrown.type = e.type + } + expect(thrown.type).toBe("Wheels.KeyNotFound") + }) + + }) + + } + +} diff --git a/vendor/wheels/tests/specs/hardener/DispatchHardenerSpec.cfc b/vendor/wheels/tests/specs/hardener/DispatchHardenerSpec.cfc new file mode 100644 index 000000000..404b50d8f --- /dev/null +++ b/vendor/wheels/tests/specs/hardener/DispatchHardenerSpec.cfc @@ -0,0 +1,228 @@ +/** + * Hardener BLOCKERs B1–B3 (dispatch / request lifecycle). + * + * Directory-scoped so `wheels test --core --ci --filter=hardener` + * discovers this folder (a single-file directory= scope finds 0 bundles). + */ +component extends="wheels.WheelsTest" { + + function run() { + + g = application.wo + + describe("B1 $ensureControllerAndAction ignores untrusted retarget", () => { + + beforeEach(() => { + dispatch = CreateObject("component", "wheels.Dispatch") + args = {} + args.path = "posts" + args.format = "" + args.route = { + pattern = "/posts", + controller = "posts", + action = "index", + regex = "^\/posts\/?$", + variables = "", + on = "", + package = "", + methods = "get", + name = "posts" + } + args.formScope = {} + args.urlScope = {} + _hadHttpData = StructKeyExists(request, "wheels") && StructKeyExists(request.wheels, "httpRequestData") + _priorHttpData = _hadHttpData ? request.wheels.httpRequestData : {} + }) + + afterEach(() => { + if (_hadHttpData) { + request.wheels.httpRequestData = _priorHttpData + } else if (StructKeyExists(request, "wheels")) { + StructDelete(request.wheels, "httpRequestData") + } + }) + + it("does not let a query string retarget a routed controller and action", () => { + args.urlScope.controller = "admin" + args.urlScope.action = "delete" + var params = dispatch.$createParams(argumentCollection = args) + + expect(params.controller).toBeWithCase("Posts") + expect(params.action).toBe("index") + }) + + it("does not let a form field retarget a routed controller and action", () => { + args.formScope.controller = "admin" + args.formScope.action = "delete" + var params = dispatch.$createParams(argumentCollection = args) + + expect(params.controller).toBeWithCase("Posts") + expect(params.action).toBe("index") + }) + + it("does not let a JSON body retarget a routed controller and action", () => { + request.wheels.httpRequestData = { + headers = {"Content-Type" = "application/json"}, + content = '{"controller":"admin","action":"delete"}' + } + var params = dispatch.$createParams(argumentCollection = args) + + expect(params.controller).toBeWithCase("Posts") + expect(params.action).toBe("index") + }) + + it("keeps path-derived controller and action on a wildcard route", () => { + var wildcardRoute = {pattern = "/[controller]/[action]", action = "index"} + var pathParams = {controller = "users", action = "show"} + var params = dispatch.$ensureControllerAndAction(params = pathParams, route = wildcardRoute) + + expect(params.controller).toBeWithCase("Users") + expect(params.action).toBe("show") + }) + + }) + + describe("B2 $cgiScope does not trust rewrite headers unless opted in", () => { + + beforeEach(() => { + _hadTrust = StructKeyExists(application.wheels, "trustProxyHeaders") + if (_hadTrust) { + _priorTrust = application.wheels.trustProxyHeaders + } + application.wheels.trustProxyHeaders = false + cgiScope = { + request_method = "", + http_x_requested_with = "", + http_referer = "", + server_name = "", + query_string = "", + remote_addr = "", + server_port = "", + server_port_secure = "", + server_protocol = "", + http_host = "", + http_accept = "", + content_type = "", + script_name = "/index.cfm", + path_info = "", + http_x_rewrite_url = "/admin/delete/http_x_rewrite_url/index.cfm?controller=admin&action=delete", + http_x_original_url = "/admin/delete/http_x_original_url/index.cfm?controller=admin&action=delete", + request_uri = "/users/list/request_uri/index.cfm", + redirect_url = "/users/list/redirect_url/index.cfm", + http_x_forwarded_for = "", + http_x_forwarded_proto = "" + } + }) + + afterEach(() => { + if (_hadTrust) { + application.wheels.trustProxyHeaders = _priorTrust + } else { + StructDelete(application.wheels, "trustProxyHeaders") + } + }) + + it("ignores a client-supplied X-Rewrite-URL when trustProxyHeaders is off", () => { + var resolved = g.$cgiScope(scope = cgiScope) + + expect(resolved.path_info).notToInclude("http_x_rewrite_url") + expect(resolved.path_info).toBe("/users/list/request_uri") + }) + + it("ignores a client-supplied X-Original-URL when trustProxyHeaders is off", () => { + cgiScope.http_x_rewrite_url = "" + var resolved = g.$cgiScope(scope = cgiScope) + + expect(resolved.path_info).notToInclude("http_x_original_url") + expect(resolved.path_info).toBe("/users/list/request_uri") + }) + + it("honors X-Rewrite-URL when trustProxyHeaders is on", () => { + application.wheels.trustProxyHeaders = true + var resolved = g.$cgiScope(scope = cgiScope) + + expect(resolved.path_info).toBe("/admin/delete/http_x_rewrite_url") + }) + + it("honors X-Original-URL when trustProxyHeaders is on and X-Rewrite-URL is empty", () => { + application.wheels.trustProxyHeaders = true + cgiScope.http_x_rewrite_url = "" + var resolved = g.$cgiScope(scope = cgiScope) + + expect(resolved.path_info).toBe("/admin/delete/http_x_original_url") + }) + + }) + + describe("B3 $getRequestMethod does not turn a safe verb into a state-changing one", () => { + + beforeEach(() => { + _originalForm = Duplicate(form) + _originalUrl = Duplicate(url) + _originalCgiMethod = request.cgi.request_method + StructClear(form) + StructClear(url) + dispatch = g.$createObjectFromRoot(path = "wheels", fileName = "Dispatch", method = "$init") + }) + + afterEach(() => { + StructClear(form) + StructClear(url) + StructAppend(form, _originalForm, false) + StructAppend(url, _originalUrl, false) + request.cgi["request_method"] = _originalCgiMethod + }) + + it("does not honor form _method on GET", () => { + request.cgi["request_method"] = "GET" + form._method = "delete" + expect(dispatch.$getRequestMethod()).toBe("GET") + }) + + it("does not honor form _method on HEAD", () => { + request.cgi["request_method"] = "HEAD" + form._method = "delete" + expect(dispatch.$getRequestMethod()).toBe("HEAD") + }) + + it("does not let POST plus _method=GET become a CSRF-safe verb", () => { + request.cgi["request_method"] = "POST" + form._method = "GET" + expect(dispatch.$getRequestMethod()).toBe("POST") + }) + + it("does not let POST plus _method=HEAD become a CSRF-safe verb", () => { + request.cgi["request_method"] = "POST" + form._method = "HEAD" + expect(dispatch.$getRequestMethod()).toBe("POST") + }) + + it("still rewrites POST plus _method=PUT", () => { + request.cgi["request_method"] = "POST" + form._method = "PUT" + expect(dispatch.$getRequestMethod()).toBe("PUT") + }) + + it("still rewrites POST plus _method=PATCH", () => { + request.cgi["request_method"] = "POST" + form._method = "PATCH" + expect(dispatch.$getRequestMethod()).toBe("PATCH") + }) + + it("still rewrites POST plus _method=DELETE", () => { + request.cgi["request_method"] = "POST" + form._method = "delete" + expect(dispatch.$getRequestMethod()).toBe("delete") + }) + + it("ignores an unknown _method value on POST", () => { + request.cgi["request_method"] = "POST" + form._method = "TRACE" + expect(dispatch.$getRequestMethod()).toBe("POST") + }) + + }) + + } + +} From 9b7916860e81abf40ca1d4c5e28c63e3138c44bc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 04:19:19 +0000 Subject: [PATCH 2/2] fix(controller): fail-closed request lifecycle B1-B4 B7-B8 Route-named controller/action win over query/form/JSON. Gate IIS rewrite headers behind trustProxyHeaders. Allowlist POST _method to PUT/PATCH/DELETE. Halt processAction when a before filter returns false. Walk appendToKey paths and throw on missing segments. Normalize filter type case. Co-authored-by: Peter Amiri Signed-off-by: Cursor Agent --- .ai/wheels/security/https-detection.md | 1 + CLAUDE.md | 2 + .../controller-hardener-b1-b8.security.md | 6 ++ vendor/wheels/Dispatch.cfc | 31 ++++++- vendor/wheels/controller/filters.cfc | 18 ++-- vendor/wheels/controller/processing.cfc | 92 +++++++++++++------ vendor/wheels/events/init/security.cfm | 7 +- vendor/wheels/global/request.cfm | 17 ++-- .../tests/specs/dispatch/createParamsSpec.cfc | 8 ++ .../tests/specs/global/internalSpec.cfc | 15 +++ .../v4-0-0/basics/controllers-and-actions.mdx | 4 +- .../v4-0-0/basics/forms-and-form-helpers.mdx | 2 +- .../v4-0-0/deployment/security-hardening.mdx | 3 +- .../authorization-and-filters.mdx | 4 +- 14 files changed, 155 insertions(+), 55 deletions(-) create mode 100644 changelog.d/controller-hardener-b1-b8.security.md diff --git a/.ai/wheels/security/https-detection.md b/.ai/wheels/security/https-detection.md index c07a457d5..7421c0188 100755 --- a/.ai/wheels/security/https-detection.md +++ b/.ai/wheels/security/https-detection.md @@ -135,6 +135,7 @@ component extends="Controller" { - Returns `true` for HTTPS connections (port 443) - Works behind load balancers and reverse proxies when `set(trustProxyHeaders=true)` is configured - `X-Forwarded-Proto` is **not** honored by default; enable with `set(trustProxyHeaders=true)` behind a trusted proxy that overwrites forwarded headers +- `X-Rewrite-URL` / `X-Original-URL` used to recover a blank `path_info` follow the same `trustProxyHeaders` gate - Test both HTTP and HTTPS scenarios during development ## Common Patterns diff --git a/CLAUDE.md b/CLAUDE.md index 3c9b985a3..0866155d8 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -451,6 +451,8 @@ mapper() Helpers: `linkTo(route="user", key=user.id)`, `urlFor(route="users")`, `redirectTo(route="user", key=user.id)`, `startFormTag(route="user", method="put", key=user.id)`. +`params.controller` / `params.action` come from the matched route. Query string, form, and JSON body cannot retarget them. Wildcard `[controller]` / `[action]` still take those names from the path. `form._method` is honored only on POST and only for `PUT` / `PATCH` / `DELETE`. A before filter that returns `false` skips the action (same as `redirectTo()` / `renderText()`). Filter `type` is case-insensitive. `caches(appendToKey=)` throws `Wheels.KeyNotFound` if a listed path is missing. `X-Rewrite-URL` / `X-Original-URL` follow `set(trustProxyHeaders=true)` like `X-Forwarded-*`. + ### Route Model Binding Resolves `params.key` into a model instance before the action runs. Lands in `params.`. Throws `Wheels.RecordNotFound` (404) if missing; silently skips if the model class doesn't exist. diff --git a/changelog.d/controller-hardener-b1-b8.security.md b/changelog.d/controller-hardener-b1-b8.security.md new file mode 100644 index 000000000..1409a6e3c --- /dev/null +++ b/changelog.d/controller-hardener-b1-b8.security.md @@ -0,0 +1,6 @@ +- Routed `controller` / `action` can no longer be retargeted by query string, form fields, or JSON body (`$ensureControllerAndAction`). Wildcard `[controller]` / `[action]` path names are unchanged +- `$cgiScope()` no longer trusts client-supplied `X-Rewrite-URL` / `X-Original-URL` unless `set(trustProxyHeaders=true)` (same opt-in as `X-Forwarded-*`) +- `form._method` is honored only on POST and only for `PUT` / `PATCH` / `DELETE`, so GET/HEAD cannot become a state-changing verb and POST cannot become a CSRF-safe verb +- A before filter that returns `false` now skips the action and remaining filters (authz fail-closed). `redirectTo()` / `renderText()` still halt as before +- `caches(appendToKey=)` walks the full dotted path and throws `Wheels.KeyNotFound` when a segment is missing, instead of silently omitting it and sharing one cache key +- Filter `type` is case-insensitive (`Before` / `before`, `After` / `after`) diff --git a/vendor/wheels/Dispatch.cfc b/vendor/wheels/Dispatch.cfc index 704d9efd9..f7251e9d6 100644 --- a/vendor/wheels/Dispatch.cfc +++ b/vendor/wheels/Dispatch.cfc @@ -972,14 +972,32 @@ component output="false" extends="wheels.Global"{ /** * Ensure that the controller and action params exist and are camelized. + * A matched route that names a controller/action always wins over query, + * form, or JSON body values. Mapper.$addRoute already deletes those keys + * when the pattern contains [controller] / [action], so wildcard + * path-derived names still come from $mergeRoutePattern. The pattern + * check is a second gate for route structs built outside $addRoute. */ public struct function $ensureControllerAndAction(required struct params, required struct route) { local.rv = arguments.params; - if (!StructKeyExists(local.rv, "controller")) { + local.pattern = StructKeyExists(arguments.route, "pattern") ? arguments.route.pattern : ""; + if ( + StructKeyExists(arguments.route, "controller") + && Len(arguments.route.controller) + && !FindNoCase("[controller]", local.pattern) + ) { local.rv.controller = arguments.route.controller; + } else if (!StructKeyExists(local.rv, "controller")) { + local.rv.controller = StructKeyExists(arguments.route, "controller") ? arguments.route.controller : ""; } - if (!StructKeyExists(local.rv, "action")) { + if ( + StructKeyExists(arguments.route, "action") + && Len(arguments.route.action) + && !FindNoCase("[action]", local.pattern) + ) { local.rv.action = arguments.route.action; + } else if (!StructKeyExists(local.rv, "action")) { + local.rv.action = StructKeyExists(arguments.route, "action") ? arguments.route.action : ""; } // We now need to have dot notation allowed in the controller hence the \. @@ -1021,11 +1039,16 @@ component output="false" extends="wheels.Global"{ /** * Determine HTTP verb used in request. + * `_method` is honored only on POST and only for PUT / PATCH / DELETE — + * the verbs `startFormTag()` emits. GET/HEAD cannot become a + * state-changing verb, and POST cannot become a CSRF-safe verb. */ public string function $getRequestMethod() { - // If request is a post, check for alternate verb. if (request.cgi.request_method == "post" && StructKeyExists(form, "_method")) { - return form["_method"]; + local.override = form["_method"]; + if (ListFindNoCase("put,patch,delete", local.override)) { + return local.override; + } } return request.cgi.request_method; diff --git a/vendor/wheels/controller/filters.cfc b/vendor/wheels/controller/filters.cfc index 72236fccb..fdc320f9a 100644 --- a/vendor/wheels/controller/filters.cfc +++ b/vendor/wheels/controller/filters.cfc @@ -27,7 +27,7 @@ component { for (local.i = 1; local.i <= local.iEnd; local.i++) { local.filter = {}; local.filter.through = local.throughKeysArray[local.i]; - local.filter.type = arguments.type; + local.filter.type = LCase(arguments.type); local.filter.only = arguments.only; local.filter.except = arguments.except; local.filter.arguments = {}; @@ -92,7 +92,7 @@ component { local.rv = []; local.iEnd = ArrayLen(variables.$class.filters); for (local.i = 1; local.i <= local.iEnd; local.i++) { - if (variables.$class.filters[local.i].type == arguments.type) { + if (LCase(variables.$class.filters[local.i].type) == LCase(arguments.type)) { ArrayAppend(local.rv, variables.$class.filters[local.i]); } } @@ -103,8 +103,9 @@ component { /** * Called twice when processing a request, first for "before" filters and then for "after" filters. + * Returns false when a before filter returns false so processAction can skip the action. */ - public void function $runFilters(required string type, required string action) { + public boolean function $runFilters(required string type, required string action) { local.filters = filterChain(arguments.type); local.iEnd = ArrayLen(local.filters); for (local.i = 1; local.i <= local.iEnd; local.i++) { @@ -118,15 +119,20 @@ component { ); } local.result = $invoke(method = local.filter.through, invokeArgs = local.filter.arguments); - // If the filter returned false, we skip the remaining filters. + // If the filter returned false, skip remaining filters. A before + // filter also halts processAction (authz fail-closed). if ((StructKeyExists(local, "result") && !IsNull(local.result) && !local.result)) { + if (LCase(arguments.type) == "before") { + return false; + } break; - } else if (arguments.type == "before" && $performedRenderOrRedirect()) { + } else if (LCase(arguments.type) == "before" && $performedRenderOrRedirect()) { break; - } else if (arguments.type == "after" && $performedRedirect()) { + } else if (LCase(arguments.type) == "after" && $performedRedirect()) { break; } } } + return true; } } diff --git a/vendor/wheels/controller/processing.cfc b/vendor/wheels/controller/processing.cfc index 7cb842d91..90d305f7d 100644 --- a/vendor/wheels/controller/processing.cfc +++ b/vendor/wheels/controller/processing.cfc @@ -42,16 +42,18 @@ component { // Continue unless an abort is issued from a verification. if (!$abortIssued()) { // Run before filters if they exist on the controller. + local.runAction = true; if (ListFindNoCase("true,before", arguments.includeFilters)) { - $runFilters(type = "before", action = variables.params.action); + local.runAction = $runFilters(type = "before", action = variables.params.action); } if ($get("showDebugInformation")) { $debugPoint("beforeFilters,action"); } - // Only proceed to call the action if the before filter has not already rendered content. - if (!$performedRenderOrRedirect()) { + // Only proceed to call the action if a before filter has not + // returned false and has not already rendered content. + if (local.runAction && !$performedRenderOrRedirect()) { // Get content from the cache if it exists there and set it to the request scope. If not, the $callActionAndAddToCache function will run, calling the controller action (which in turn sets the content to the request scope). if (local.cache) { local.category = "action"; @@ -60,33 +62,21 @@ component { local.key = $hashedKey(variables.$class.name, variables.params); // Evaluate variables and append to the cache key when specified. + // Missing or unresolvable items throw; they are never omitted, + // because a silent skip collapses distinct keys into one shared key. if (Len(local.appendToKey)) { - for (local.item in ListToArray(local.appendToKey)) { - if (IsDefined(local.item)) { - // Build the scope lookup once (and keep it in the local scope so it doesn't leak into the controller's variables scope). - if (!StructKeyExists(local, "scopeMap")) { - local.scopeMap = { - "request": request, - "arguments": arguments, - "application": application, - "session": session, - "variables": variables - }; - } - - // Extract scope name and variable name from local.item - local.scopeName = ListFirst(local.item, "."); - local.varName = ListLast(local.item, "."); - if ( - StructKeyExists(local.scopeMap, local.scopeName) - && StructKeyExists(local.scopeMap[local.scopeName], local.varName) - ) { - local.key &= local.scopeMap[local.scopeName][local.varName]; - } else { - Throw(type = "Wheels.KeyNotFound", message = "The `#local.item#` argument was not found."); - } - } - } + local.scopeMap = { + "request": request, + "arguments": arguments, + "application": application, + "session": session, + "variables": variables + }; + local.key = $appendToCacheKey( + key = local.key, + appendToKey = local.appendToKey, + scopeMap = local.scopeMap + ); } local.conditionArgs = {}; @@ -119,7 +109,7 @@ component { $debugPoint("action,afterFilters"); } - if (!$performedRedirect() && ListFindNoCase("true,after", arguments.includeFilters)) { + if (local.runAction && !$performedRedirect() && ListFindNoCase("true,after", arguments.includeFilters)) { $runFilters(type = "after", action = variables.params.action); } @@ -245,4 +235,46 @@ component { ); return response(); } + + /** + * Internal function. Appends resolved appendToKey segments onto an action cache key. + * Every listed item must resolve; silent omission would share one key across users. + */ + public string function $appendToCacheKey(required string key, required string appendToKey, required struct scopeMap) { + local.rv = arguments.key; + local.items = ListToArray(arguments.appendToKey); + local.iEnd = ArrayLen(local.items); + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.rv &= $resolveAppendToKeyValue(item = local.items[local.i], scopeMap = arguments.scopeMap); + } + return local.rv; + } + + /** + * Internal function. Walks a dotted appendToKey path (scope.a.b.c) and returns + * the simple value. Throws Wheels.KeyNotFound when any segment is missing. + */ + public string function $resolveAppendToKeyValue(required string item, required struct scopeMap) { + local.segments = ListToArray(arguments.item, "."); + if (ArrayLen(local.segments) < 2) { + Throw(type = "Wheels.KeyNotFound", message = "The `#arguments.item#` argument was not found."); + } + local.scopeName = local.segments[1]; + if (!StructKeyExists(arguments.scopeMap, local.scopeName)) { + Throw(type = "Wheels.KeyNotFound", message = "The `#arguments.item#` argument was not found."); + } + local.cursor = arguments.scopeMap[local.scopeName]; + local.iEnd = ArrayLen(local.segments); + for (local.i = 2; local.i <= local.iEnd; local.i++) { + local.segment = local.segments[local.i]; + if (!IsStruct(local.cursor) || !StructKeyExists(local.cursor, local.segment)) { + Throw(type = "Wheels.KeyNotFound", message = "The `#arguments.item#` argument was not found."); + } + local.cursor = local.cursor[local.segment]; + } + if (IsNull(local.cursor) || !IsSimpleValue(local.cursor)) { + Throw(type = "Wheels.KeyNotFound", message = "The `#arguments.item#` argument was not found."); + } + return ToString(local.cursor); + } } diff --git a/vendor/wheels/events/init/security.cfm b/vendor/wheels/events/init/security.cfm index 35955d17f..a7550d401 100644 --- a/vendor/wheels/events/init/security.cfm +++ b/vendor/wheels/events/init/security.cfm @@ -57,9 +57,10 @@ application.$wheels.debugAccessTrustProxy = false; // Trusted proxy settings. - // Only when true are X-Forwarded-* headers honored framework-wide: X-Forwarded-Proto in - // isSecure(), and X-Forwarded-For (rightmost hop) for maintenance-mode IP exceptions and - // reload rate-limit keying. Leave false unless the app sits behind a trusted reverse proxy + // Only when true are proxy-supplied headers honored framework-wide: X-Forwarded-Proto in + // isSecure(), X-Forwarded-For (rightmost hop) for maintenance-mode IP exceptions and + // reload rate-limit keying, and X-Rewrite-URL / X-Original-URL when $cgiScope() fills a + // blank path_info (IIS). Leave false unless the app sits behind a trusted reverse proxy // that overwrites — never appends to — these headers. application.$wheels.trustProxyHeaders = false; diff --git a/vendor/wheels/global/request.cfm b/vendor/wheels/global/request.cfm index cc292cd06..25920c797 100644 --- a/vendor/wheels/global/request.cfm +++ b/vendor/wheels/global/request.cfm @@ -125,11 +125,12 @@ // fixes IIS issue that returns a blank cgi.path_info if (!Len(local.rv.path_info) && Right(local.rv.script_name, 10) == "/index.cfm") { - if (Len(local.rv.http_x_rewrite_url)) { - // IIS6 1/ IIRF (Ionics Isapi Rewrite Filter) + if ($trustProxyHeaders() && Len(local.rv.http_x_rewrite_url)) { + // IIS6 1/ IIRF (Ionics Isapi Rewrite Filter). Client-supplied; + // only trusted when the app opted in via trustProxyHeaders. local.rv.path_info = ListFirst(local.rv.http_x_rewrite_url, "?"); - } else if (Len(local.rv.http_x_original_url)) { - // IIS7 rewrite default + } else if ($trustProxyHeaders() && Len(local.rv.http_x_original_url)) { + // IIS7 rewrite default. Same trust gate as X-Forwarded-*. local.rv.path_info = ListFirst(local.rv.http_x_original_url, "?"); } else if (Len(local.rv.request_uri)) { // Apache default @@ -160,9 +161,11 @@ /** - * Internal function. Returns whether the application has opted into trusting `X-Forwarded-*` - * headers via `set(trustProxyHeaders=true)`. Guarded so it is safe to call on a cold start - * before `application.wheels` exists (resolves to `false`, i.e. do not trust). + * Internal function. Returns whether the application has opted into trusting + * proxy-supplied headers via `set(trustProxyHeaders=true)`: `X-Forwarded-*` + * plus the IIS rewrite headers `X-Rewrite-URL` / `X-Original-URL` used by + * `$cgiScope()`. Guarded so it is safe to call on a cold start before + * `application.wheels` exists (resolves to `false`, i.e. do not trust). */ public boolean function $trustProxyHeaders() { return StructKeyExists(application, "wheels") diff --git a/vendor/wheels/tests/specs/dispatch/createParamsSpec.cfc b/vendor/wheels/tests/specs/dispatch/createParamsSpec.cfc index f3d73abaf..93d3ce5fd 100644 --- a/vendor/wheels/tests/specs/dispatch/createParamsSpec.cfc +++ b/vendor/wheels/tests/specs/dispatch/createParamsSpec.cfc @@ -153,6 +153,11 @@ component extends="wheels.WheelsTest" { }) it("sets controller in upper camel case", () => { + // Wildcard-style route: no fixed controller, so the incoming + // name is the value that gets camelized (B1: a routed + // controller name is no longer overridable from the form). + args.route.pattern = "/[controller]" + StructDelete(args.route, "controller") args.formScope["controller"] = "wheels-test" _params = dispatch.$createParams(argumentCollection = args) @@ -165,6 +170,9 @@ component extends="wheels.WheelsTest" { }) it("sanitizes controller and action params", () => { + args.route.pattern = "/[controller]/[action]" + StructDelete(args.route, "controller") + StructDelete(args.route, "action") args.formScope["controller"] = "../../../wheels%00" args.formScope["action"] = "../../../test*^&%()%00" _params = dispatch.$createParams(argumentCollection = args) diff --git a/vendor/wheels/tests/specs/global/internalSpec.cfc b/vendor/wheels/tests/specs/global/internalSpec.cfc index 875fef58b..db4e53c49 100644 --- a/vendor/wheels/tests/specs/global/internalSpec.cfc +++ b/vendor/wheels/tests/specs/global/internalSpec.cfc @@ -65,6 +65,13 @@ component extends="wheels.WheelsTest" { describe("Tests that $cgiscope", () => { beforeEach(() => { + _hadTrustProxyHeaders = StructKeyExists(application.wheels, "trustProxyHeaders") + if (_hadTrustProxyHeaders) { + _originalTrustProxyHeaders = application.wheels.trustProxyHeaders + } + // These cases document the IIS rewrite-header recovery order. + // The headers are client-supplied and require trustProxyHeaders. + application.wheels.trustProxyHeaders = true cgi_scope = {} cgi_scope.request_method = "" cgi_scope.http_x_requested_with = "" @@ -88,6 +95,14 @@ component extends="wheels.WheelsTest" { cgi_scope.http_x_forwarded_proto = "" }) + afterEach(() => { + if (_hadTrustProxyHeaders) { + application.wheels.trustProxyHeaders = _originalTrustProxyHeaders + } else { + StructDelete(application.wheels, "trustProxyHeaders") + } + }) + it("checks path info is blank", () => { cgi_scope.path_info = "" _cgi = g.$cgiScope(scope = cgi_scope) diff --git a/web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx b/web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx index 72ce37c25..de5e51def 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx @@ -92,7 +92,7 @@ component extends="Controller" { `config()` runs once per controller instantiation — it's where you register filters, verify rules, and any other per-controller setup. Don't put action logic here; it runs before any action is known. -`filters(through="methodName")` binds a private method to run before every action in this controller. Use `only="..."` to restrict it to specific actions or `except="..."` to skip them. Pass `type="after"` to run after the action instead of before. +`filters(through="methodName")` binds a private method to run before every action in this controller. Use `only="..."` to restrict it to specific actions or `except="..."` to skip them. Pass `type="after"` to run after the action instead of before. `type` is case-insensitive (`Before` and `before` both run as before filters). A before filter that returns `false` skips the action and any remaining filters; `redirectTo()` and `renderText()` still short-circuit the same way. ```cfm {test:compile} title="app/controllers/Posts.cfc" component extends="Controller" { @@ -119,6 +119,8 @@ The `params` struct is built by the framework on every request. It merges three - **Query string** — `?page=2` lands in `params.page`. - **Form fields** — `` lands in `params.post.title`. +`params.controller` and `params.action` come from the matched route. A query string, form field, or JSON body cannot retarget them. Wildcard `[controller]` / `[action]` routes still take those names from the path. + ```cfm {test:compile} title="app/controllers/Posts.cfc" component extends="Controller" { function show() { diff --git a/web/sites/guides/src/content/docs/v4-0-0/basics/forms-and-form-helpers.mdx b/web/sites/guides/src/content/docs/v4-0-0/basics/forms-and-form-helpers.mdx index 6f2a272d8..e33b2654b 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/basics/forms-and-form-helpers.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/basics/forms-and-form-helpers.mdx @@ -39,7 +39,7 @@ Key behaviours: - Generates `
` with CSRF token auto-included. - `route="..."` uses named routes — see [Routing](/v4-0-0/basics/routing/). -- `method="put"` / `method="delete"` — Wheels emits a hidden `_method` field that gets translated server-side. Browsers only send GET and POST natively, so Wheels (like Rails, Laravel, and friends) uses the hidden field convention to expose the full set of REST verbs. +- `method="put"` / `method="delete"` — Wheels emits a hidden `_method` field that gets translated server-side. Browsers only send GET and POST natively, so Wheels (like Rails, Laravel, and friends) uses the hidden field convention to expose the full set of REST verbs. The override is honored only on POST and only for `PUT` / `PATCH` / `DELETE`. ## Object-bound helpers — the core pattern diff --git a/web/sites/guides/src/content/docs/v4-0-0/deployment/security-hardening.mdx b/web/sites/guides/src/content/docs/v4-0-0/deployment/security-hardening.mdx index 4f4664cde..bb026678a 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/deployment/security-hardening.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/deployment/security-hardening.mdx @@ -211,11 +211,12 @@ Wheels does **not** trust `X-Forwarded-*` headers by default. They are ordinary ### `trustProxyHeaders` — framework-wide header trust -`set(trustProxyHeaders=true)` (default `false`) governs the framework's own use of forwarded headers across three surfaces: +`set(trustProxyHeaders=true)` (default `false`) governs the framework's own use of forwarded and rewrite headers: - **`isSecure()`.** With trust off, only the engine's `server_port_secure` flag counts — a client-supplied `X-Forwarded-Proto: https` is ignored, so a direct-HTTP client can't spoof secure-cookie, CSRF, or HTTPS-redirect decisions. With trust on, `X-Forwarded-Proto: https` from your TLS-terminating proxy makes `isSecure()` return `true`. - **Maintenance-mode IP exceptions.** `set(ipExceptions="...")` is matched against the socket address by default, or against the rightmost `X-Forwarded-For` hop when trust is on. The exception list comes from config only — the legacy `?except=` URL parameter has been removed (it let any anonymous client rewrite the exception list for everyone). - **Reload rate-limit keying.** Failed `?reload=` password attempts are counted per trusted client IP, so with trust on, clients behind a shared proxy no longer share one lockout bucket. +- **IIS rewrite path recovery.** When `cgi.path_info` is blank, `$cgiScope()` reads `X-Rewrite-URL` / `X-Original-URL` only if trust is on. Those are ordinary request headers; any client can send them. With trust off, recovery falls through to `request_uri` / `redirect_url`. ```cfm {test:compile} title="config/production/settings.cfm — app behind a TLS-terminating reverse proxy" diff --git a/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/authorization-and-filters.mdx b/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/authorization-and-filters.mdx index 73f48bf7f..5183822f4 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/authorization-and-filters.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/authorization-and-filters.mdx @@ -23,7 +23,7 @@ This page shows you how to use controller filters to enforce access rules. You'l ## The filter API — recap -Filters register in `config()` with `filters(through="methodName", only="...", except="...", type="before")`. The `through` method must exist on the controller and must be declared `private` — a public filter becomes a routable action. Call `redirectTo()` or `renderText()` inside the filter to short-circuit the request; the action never runs and no further before-filters fire. +Filters register in `config()` with `filters(through="methodName", only="...", except="...", type="before")`. The `through` method must exist on the controller and must be declared `private` — a public filter becomes a routable action. Call `redirectTo()` or `renderText()` inside the filter to short-circuit the request, or `return false`. In all three cases the action never runs and no further before-filters fire. `type` is case-insensitive. For the full argument list, placement semantics, and a run-through of the order of operations, see [Controllers and Actions](/v4-0-0/basics/controllers-and-actions/). This page focuses on what to put inside the filter. @@ -59,7 +59,7 @@ component extends="Controller" { } ``` -`except="index,show"` leaves the two read actions public. Every other action — `new`, `create`, `edit`, `update`, `delete` — triggers `authenticate` before the action body runs. The filter returns `void`; short-circuit is signalled by calling `redirectTo`. +`except="index,show"` leaves the two read actions public. Every other action — `new`, `create`, `edit`, `update`, `delete` — triggers `authenticate` before the action body runs. Short-circuit is signalled by `redirectTo()`, `renderText()`, or `return false`.