This is an automated email from the ASF dual-hosted git repository. jamesfredley pushed a commit to branch feat/gorm-query-safety-warnings in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 8fdd94d3400abf0beccc67db128c91a066091d86 Author: James Fredley <[email protected]> AuthorDate: Fri Jul 10 18:07:56 2026 -0400 Warn once on GString-interpolated GORM HQL queries Add GormQuerySafetyWarnings and call it from Hibernate query paths. Recommend named parameters in docs. Does not throw; warns with query shape. Assisted-by: Sisyphus:xai/grok-4.5 [gpt-coding] --- .../orm/hibernate/HibernateGormStaticApi.groovy | 3 + .../gorm/query/GormQuerySafetyWarnings.groovy | 67 ++++++++++++++++++ .../gorm/query/GormQuerySafetyWarningsSpec.groovy | 79 ++++++++++++++++++++++ .../en/guide/security/securingAgainstAttacks.adoc | 15 ++-- .../src/en/guide/upgrading/upgrading80x.adoc | 36 +++++++--- .../src/en/ref/Domain Classes/executeQuery.adoc | 10 +++ grails-doc/src/en/ref/Domain Classes/find.adoc | 10 +++ 7 files changed, 206 insertions(+), 14 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy index 67e1bf3abd..35b3573c21 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy @@ -49,6 +49,7 @@ import grails.orm.HibernateCriteriaBuilder import grails.gorm.DetachedCriteria import org.grails.datastore.gorm.GormStaticApi import org.grails.datastore.gorm.finders.FinderMethod +import org.grails.datastore.gorm.query.GormQuerySafetyWarnings import org.grails.datastore.mapping.core.connections.ConnectionSource import org.grails.datastore.mapping.core.connections.ConnectionSourcesProvider import org.grails.datastore.mapping.proxy.ProxyHandler @@ -480,6 +481,8 @@ class HibernateGormStaticApi<D> extends GormStaticApi<D> { if (hints.isEmpty() && querySettings != null) { hints = querySettings.findAll { AvailableHints.getDefinedHints().contains(it.key) } } + String operation = isNative ? 'native SQL query' : (isUpdate ? 'executeUpdate' : 'find/executeQuery') + GormQuerySafetyWarnings.warnIfGStringQuery(log, hql, operation) Map<String, Object> coercedParams = namedParams?.collectEntries { k, v -> [k.toString(), v] } ?: [:] def ctx = HqlQueryContext.prepare(persistentEntity, hql, coercedParams, positionalParams, querySettings, hints, isNative, isUpdate) return HibernateHqlQueryCreator.createHqlQuery( diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarnings.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarnings.groovy new file mode 100644 index 0000000000..0545cee445 --- /dev/null +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarnings.groovy @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.datastore.gorm.query + +import groovy.transform.CompileStatic + +import java.util.concurrent.ConcurrentHashMap + +import org.slf4j.Logger + +@CompileStatic +final class GormQuerySafetyWarnings { + + private static final String GSTRING_VALUE_PLACEHOLDER = '${...}' + private static final Set<String> WARNED_GSTRING_QUERY_SHAPES = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()) + + private GormQuerySafetyWarnings() { + } + + static boolean warnIfGStringQuery(Logger logger, CharSequence query, String operation) { + if (!(query instanceof GString) || ((GString) query).values.length == 0) { + return false + } + + String queryShape = buildQueryShape((GString) query) + if (!logger.warnEnabled) { + return false + } + + String warningKey = "${operation}\n${queryShape}" + if (!WARNED_GSTRING_QUERY_SHAPES.add(warningKey)) { + return false + } + + logger.warn('GString-interpolated HQL passed to [{}]. GORM binds interpolated values as query parameters, but explicit named parameters are recommended for query safety and readability. Query shape: [{}]', operation, queryShape) + return true + } + + private static String buildQueryShape(GString query) { + StringBuilder queryShape = new StringBuilder() + String[] strings = query.strings + Object[] values = query.values + for (int stringIndex = 0; stringIndex < strings.length; stringIndex++) { + queryShape.append(strings[stringIndex]) + if (stringIndex < values.length) { + queryShape.append(GSTRING_VALUE_PLACEHOLDER) + } + } + return queryShape.toString() + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarningsSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarningsSpec.groovy new file mode 100644 index 0000000000..3189737d70 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/query/GormQuerySafetyWarningsSpec.groovy @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.datastore.gorm.query + +import org.slf4j.Logger + +import spock.lang.Specification + +class GormQuerySafetyWarningsSpec extends Specification { + + void 'warns once for a GString query without logging interpolated values'() { + given: + Logger logger = Mock() { + isWarnEnabled() >> true + } + String title = 'secret-title' + GString query = "from Book b where b.title = ${title}" + + when: + boolean firstWarning = GormQuerySafetyWarnings.warnIfGStringQuery(logger, query, 'Book.find') + boolean secondWarning = GormQuerySafetyWarnings.warnIfGStringQuery(logger, query, 'Book.find') + + then: + firstWarning + !secondWarning + 1 * logger.warn({ String message -> + message.contains('explicit named parameters') + }, 'Book.find', { String queryShape -> + queryShape == 'from Book b where b.title = ${...}' && !queryShape.contains(title) + }) + 0 * logger.warn(_, _, _) + } + + void 'does not warn for a plain String query'() { + given: + Logger logger = Mock() + + when: + boolean warning = GormQuerySafetyWarnings.warnIfGStringQuery(logger, 'from Book b where b.title = :title', 'Book.find') + + then: + !warning + 0 * logger._ + } + + void 'does not suppress a query shape when warn logging is disabled'() { + given: + Logger logger = Mock() { + isWarnEnabled() >>> [false, true] + } + String title = 'enabled-later' + GString query = "from Book b where b.title <> ${title}" + + when: + boolean firstWarning = GormQuerySafetyWarnings.warnIfGStringQuery(logger, query, 'Book.executeQuery') + boolean secondWarning = GormQuerySafetyWarnings.warnIfGStringQuery(logger, query, 'Book.executeQuery') + + then: + !firstWarning + secondWarning + 1 * logger.warn(_ as String, 'Book.executeQuery', 'from Book b where b.title <> ${...}') + } +} diff --git a/grails-doc/src/en/guide/security/securingAgainstAttacks.adoc b/grails-doc/src/en/guide/security/securingAgainstAttacks.adoc index b9aa593d2c..467b893bae 100644 --- a/grails-doc/src/en/guide/security/securingAgainstAttacks.adoc +++ b/grails-doc/src/en/guide/security/securingAgainstAttacks.adoc @@ -39,26 +39,29 @@ def vulnerable() { } ---- -Do *not* do this. Use named or positional parameters instead to pass in parameters: +Do *not* do this. Use named parameters instead to pass values separately from the HQL text: [source,groovy] ---- def safe() { - def books = Book.find("from Book as b where b.title = ?", - [params.title]) + def books = Book.find("from Book as b where b.title = :title", + [title: params.title]) } ---- -or +Positional parameters are also supported: [source,groovy] ---- def safe() { - def books = Book.find("from Book as b where b.title = :title", - [title: params.title]) + def books = Book.find("from Book as b where b.title = ?1", + [params.title]) } ---- +GORM binds interpolated values in `GString` HQL as query parameters, but Grails logs a warning when a `GString` query is passed to methods such as `find`, `findAll`, `executeQuery`, or `executeUpdate`. +Prefer explicit named parameters in application code because they make the safe boundary between query structure and user-controlled values clear. + ==== Phishing diff --git a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc index eb80fee140..0141e640bf 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc @@ -1040,7 +1040,27 @@ dependencies { The following sections cover every breaking change introduced between Hibernate ORM 5.6.x (used by Grails 7) and Hibernate ORM 7.0.x, and what you need to do in your Grails application. -===== 26.1 Hibernate Session API Removals +===== 26.1 Safe Parameterized GORM Queries + +Grails 8 keeps GORM's non-breaking behavior for HQL `GString` queries: interpolated values are converted to query parameters instead of being copied into the final HQL text. +Applications may now see a warning when a `GString` query is passed to methods such as `find`, `findAll`, `executeQuery`, or `executeUpdate`. +The warning is informational and does not reject the query. + +Prefer explicit named parameters for values that come from requests, files, external services, or other untrusted input: + +[source,groovy] +---- +// Before - warns and is harder to review +Book.executeQuery("from Book b where b.title = ${params.title}") + +// After - recommended +Book.executeQuery("from Book b where b.title = :title", [title: params.title]) +---- + +Only build query structure dynamically from validated, trusted names such as a domain property allowlist. +Keep user-controlled values in `namedParams` rather than concatenating them into the HQL string. + +===== 26.2 Hibernate Session API Removals Hibernate 7 removed long-deprecated Hibernate-specific session methods in favour of the standard JPA equivalents. GORM's dynamic methods (`save()`, `delete()`, `get()`, `load()`, `merge()`, etc.) are **not** affected — these go through GORM's own persistence API and have been updated internally. @@ -1071,7 +1091,7 @@ You are only affected if your code calls the Hibernate `Session` or `StatelessSe | `session.find(Class, id)` |=== -===== 26.2 Removed Hibernate Annotations +===== 26.3 Removed Hibernate Annotations The following Hibernate-specific annotations were removed in Hibernate 7. Where a replacement exists, migrate before upgrading. @@ -1106,7 +1126,7 @@ Where a replacement exists, migrate before upgrading. NOTE: Grails domain classes that use the `mapping { }` DSL are not affected by annotation removals. These annotations only apply if you are using Hibernate annotations directly on Java or Groovy classes. -===== 26.3 CascadeType.SAVE_UPDATE Removed +===== 26.4 CascadeType.SAVE_UPDATE Removed `CascadeType.SAVE_UPDATE` (a Hibernate-specific cascade type) was removed in Hibernate 7. Persisting a transient entity that has detached associations now throws `EntityExistsException` instead of silently merging. @@ -1116,14 +1136,14 @@ If your domain mapping or annotated classes used `cascade = CascadeType.SAVE_UPD Also note that automatic `cascade = CascadeType.PERSIST` on `@Id` and `@MapsId` associations was removed. If you relied on this implicit behaviour, add an explicit `cascade = CascadeType.PERSIST` to the `@ManyToOne` or `@OneToOne` carrying `@MapsId`. -===== 26.4 Detached Entity Operations +===== 26.5 Detached Entity Operations Calling `refresh()` or `lock()` on a detached entity now throws `IllegalArgumentException` (JPA specification compliance). In Hibernate 5 and 6 this was silently permitted. If your code calls `entity.refresh()` or `entity.lock()` after the Hibernate session has been closed or the entity has been evicted, you must either re-attach the entity first (via `session.merge()`) or reload it before refreshing. -===== 26.5 Native Query Temporal Type Changes +===== 26.6 Native Query Temporal Type Changes Native SQL queries (via `executeQuery`, `withCriteria`, or a raw `Session.createNativeQuery`) now return `java.time` types (`LocalDate`, `LocalTime`, `LocalDateTime`) instead of the legacy `java.sql` types (`java.sql.Date`, `java.sql.Time`, `java.sql.Timestamp`). @@ -1138,7 +1158,7 @@ hibernate: prefer_jdbc_datetime_types: true ---- -===== 26.6 StatelessSession Now Uses Second-Level Cache by Default +===== 26.7 StatelessSession Now Uses Second-Level Cache by Default `StatelessSession` in Hibernate 7 participates in the second-level cache by default (it did not in Hibernate 5/6). If you use `withStatelessSession` for bulk operations where cache bypass was intentional, disable caching explicitly: @@ -1151,7 +1171,7 @@ YourDomain.withStatelessSession { StatelessSession session -> } ---- -===== 26.7 DDL Schema Changes +===== 26.8 DDL Schema Changes If you use `dbCreate = 'create-drop'` or `dbCreate = 'update'` in development, or generate a schema with the `schema-export` command, be aware of the following DDL differences in Hibernate 7. @@ -1184,7 +1204,7 @@ If you use `dbCreate = 'create-drop'` or `dbCreate = 'update'` in development, o WARNING: If you have an existing production database, validate the schema diff before enabling `dbCreate = 'update'` or running Liquibase migrations after the Hibernate 7 upgrade. -===== 26.8 Hibernate 6 Intermediate Changes +===== 26.9 Hibernate 6 Intermediate Changes Hibernate 7 builds on Hibernate 6, which itself introduced breaking changes relative to Hibernate 5. The changes in Hibernate 6 that are most likely to affect a Grails application are listed here for completeness. diff --git a/grails-doc/src/en/ref/Domain Classes/executeQuery.adoc b/grails-doc/src/en/ref/Domain Classes/executeQuery.adoc index 74e6d4f0c9..bf244dd098 100644 --- a/grails-doc/src/en/ref/Domain Classes/executeQuery.adoc +++ b/grails-doc/src/en/ref/Domain Classes/executeQuery.adoc @@ -108,3 +108,13 @@ Parameters: * `positionalParams` - A `List` of parameters for a positional parameterized query * `namedParams` - A `Map` of named parameters for a named parameterized query * `metaParams` - A `Map` of pagination parameters `max` and/or `offset`, as well as Hibernate query parameters `readOnly`, `fetchSize`, `timeout`, `flushMode`, `cache`, and `lock` + +Prefer named parameters for values that come from users or external systems: + +[source,groovy] +---- +Book.executeQuery("from Book b where b.title = :title", [title: params.title]) +---- + +GORM binds interpolated values in `GString` HQL as query parameters, but Grails logs a warning for `GString` queries so applications can migrate toward explicit named parameters. +Do not concatenate or interpolate untrusted values into a plain `String` query. diff --git a/grails-doc/src/en/ref/Domain Classes/find.adoc b/grails-doc/src/en/ref/Domain Classes/find.adoc index 220065de3b..b2b0f2b33c 100644 --- a/grails-doc/src/en/ref/Domain Classes/find.adoc +++ b/grails-doc/src/en/ref/Domain Classes/find.adoc @@ -80,3 +80,13 @@ Parameters: * `namedParams` - A `Map` of named parameters a HQL query * `queryParams` - A `Map` of query parameters such as `cache`, `readOnly`, `fetchSize`, `timeout`, `flushMode`, and `lock` * `example` - An instance of the domain class for query by example + +Prefer named parameters for values that come from users or external systems: + +[source,groovy] +---- +Book.find("from Book as b where b.author = :author", [author: params.author]) +---- + +GORM binds interpolated values in `GString` HQL as query parameters, but Grails logs a warning for `GString` queries so applications can migrate toward explicit named parameters. +Do not concatenate or interpolate untrusted values into a plain `String` query.
