GString query-injection safety — two approaches to compare before Wed (#15968 vs #15971)

[Walter Duque de Estrada <wb...@icloud.com>]

Hi all,

While reviewing #15968 ("Warn once on GString-interpolated GORM HQL queries"), I traced through the actual mechanism it's checking and found some gaps worth discussing before we merge either PR. I ended up with a second PR proposing a different fix, so let's put both in front of the list ahead of Wednesday. This analysis was done with AI assistance (Claude), consistent with how #15968 itself is labeled.

The problem: a GString passed directly to a GORM query method (find/findAll/executeQuery/etc.) is safe — GORM binds each interpolated value as a real parameter. But assigning it to a String-typed local first (String q = "...${x}..."; Book.executeQuery(q)) makes Groovy coerce it to a plain String at that assignment, and the safety silently disappears. By the time it reaches the query call there's nothing left to distinguish it from hand-written HQL — the interpolated value is now raw, unescaped text.

Approach A — #15968, runtime warning. Logs once when the query argument is still a GString at the Hibernate7 query boundary. Non-breaking. Two gaps: it only fires on the case that's already safe — a runtime check structurally can't see the flattened-String case, since a String carries no trace of ever having been a GString — and it's wired into Hibernate7 only. Hibernate5 and Neo4j have the identical code shape and are untouched.

Approach B — #15971, compile-time AST check. A global Groovy transform that catches the flattened-String pattern at compile time and fails the build by default, with a per-call-site @SuppressWarnings("GormUnsafeQueryString") escape hatch for reviewed-safe cases. Because it's a syntactic check rather than a per-datastore patch, it covers Hibernate5, Hibernate7, and Neo4j at once with no changes to any of their source. Tradeoffs: it's a breaking check for anyone using this pattern today, and it's intraprocedural onilt in a helper method and returned is invisible to it. It's not a general SQL-injection solution — no plain concatenation, no raw JDBC, nothing outside HQL/Cypher.

Both have full test coverage and pass clean against the existing Hibernate5/7 suites.

Discussion points for Wednesday:
- Compile-time break, or is warn-only the rightin the 8.0.x cycle?
- If compile-time, hard error from day one, or a softer rollout?
- Worth keeping both — #15968 as a gentler sign backstop?

PRs: #15968 / #15971

Thanks,
Walter