jamesfredley commented on PR #15808: URL: https://github.com/apache/grails-core/pull/15808#issuecomment-4862309168
**Pre-release review pass (2026-07-02, dual-reviewer: GPT-5.5 oracle + Codex CLI):** BLOCK at the reviewed head - do not merge until resolved. (Note: the branch received new pushes during review, so some items may already be in flight.) | Area | Finding | Severity | |---|---|---| | `ControllerActionTransformer` compile-time guard | Accepts arbitrary expressions / mutable `List` locals as `allowedParams`, including values derived from `params` - defeating the explicit-allowlist goal (matches the added failing GAP test). Require literal string-constant lists or recognized `static final` constants; reject request-derived expressions. | high | | `DataBindingUtils` `nullMissing` indexed paths | Malformed / non-numeric / out-of-range list indexes can throw or produce binding errors via `Integer.parseInt` / `List.set`; the added GAP tests expect these to be ignored, so CI was red at review time. | high | | CI | Multiple failing checks at the reviewed head (incl. the new GAP tests). | high | | Test coverage | No secure-path tests proving allowed properties still flow through `@BindUsing` / `@BindingFormat` and disallowed formatted/custom-bound properties stay blocked. | med | | `secureBindData` docs | Present (good), but should warn that `allowedParams` must be developer-controlled - never built from `params`/request input. | med | Positive: the distinct `secureBindData` name is discoverable next to `bindData`, empty allowlists bind nothing, and nested/indexed allowlist semantics are documented. The API direction is good; the compile-time guard and red CI are the gate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
