diff --git a/docs/configuration.md b/docs/configuration.md index 5d412f7d9..1114d3f95 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -636,7 +636,7 @@ See [Validation](validation.md) for a complete list of what each validation leve ## Interpolation Safety -Storm's Kotlin API uses the Storm compiler plugin to automatically wrap string interpolations inside SQL template lambdas, ensuring all values are parameterized and SQL injection safe. When a `TemplateBuilder` lambda runs without the compiler plugin and without any explicit `t()` or `interpolate()` calls, Storm cannot distinguish a pure SQL literal (safe) from a string with accidentally concatenated interpolations (SQL injection risk). The `storm.validation.interpolation_mode` property controls how Storm handles this situation. +Storm's Kotlin API uses the Storm compiler plugin to automatically wrap string interpolations inside SQL template lambdas, ensuring all values are parameterized and SQL injection safe. When a `TemplateBuilder` lambda runs without the compiler plugin, Storm cannot verify that every string interpolation is wrapped in a `t()` or `interpolate()` call: a single unwrapped interpolation concatenates its value directly into the SQL (SQL injection risk). Explicit `t()` calls do not satisfy the check, because they say nothing about the other interpolations in the same template. The `storm.validation.interpolation_mode` property controls how Storm handles templates it cannot verify. ### storm.validation.interpolation_mode @@ -644,7 +644,9 @@ Storm's Kotlin API uses the Storm compiler plugin to automatically wrap string i |-------|----------| | `warn` | Logs a warning at `WARNING` level (default). | | `fail` | Throws an `IllegalStateException`, preventing execution of potentially unsafe templates. | -| `none` | Disables the check entirely. Use only when you are certain the compiler plugin is not needed. | +| `none` | Disables the check entirely. Use when every interpolation is wrapped manually or templates are pure literals. | + +Any other value fails with an `IllegalStateException` naming the valid values. See [String Templates](string-templates.md) for setup instructions for the compiler plugin. @@ -757,7 +759,7 @@ This reduces memory usage at the cost of less efficient dirty checking. ### Production Hardening -For production environments, consider enabling strict validation and interpolation safety checks. These settings catch configuration issues and potential security problems that should not reach production: +For production environments, enable strict validation and interpolation safety checks. These settings catch configuration issues and potential security problems that should not reach production: ```bash java -Dstorm.validation.schema_mode=fail \ @@ -766,6 +768,6 @@ java -Dstorm.validation.schema_mode=fail \ ``` - `storm.validation.schema_mode=fail` catches entity-to-schema mismatches at startup rather than at runtime. -- `storm.validation.interpolation_mode=fail` prevents execution of templates that were not processed by the compiler plugin and do not use explicit `t()` calls, protecting against accidental SQL injection. +- `storm.validation.interpolation_mode=fail` prevents execution of templates that were not processed by the compiler plugin, protecting against accidental SQL injection. Set this in every production deployment that uses Kotlin templates: the default `warn` reduces a missing compiler plugin to a log line. In the Spring Boot starter and Ktor plugin, `schema_mode` already defaults to `fail`, so entity-to-schema mismatches abort startup out of the box; relax it to `warn` or `none` while a schema is still evolving. `interpolation_mode` defaults to `warn`, so missing compiler plugin usage is logged rather than blocking execution until you opt into `fail`. diff --git a/docs/string-templates.md b/docs/string-templates.md index 0678da7b4..24b383200 100644 --- a/docs/string-templates.md +++ b/docs/string-templates.md @@ -102,11 +102,11 @@ The compiler plugin is optional. Without it, you can still use Storm's template orm.query { "SELECT ${t(User::class)} FROM ${t(User::class)} WHERE id = ${t(id)}" } ``` -This produces identical behavior. The `t()` function is always available inside template lambdas. The compiler plugin simply automates the wrapping. +This produces identical behavior. The `t()` function is always available inside template lambdas. The compiler plugin simply automates the wrapping. Note that Storm cannot verify manual wrapping at runtime, so templates built without the plugin trigger the interpolation safety check described below. ### Interpolation Safety -When a `TemplateBuilder` lambda runs without the compiler plugin and without any explicit `t()` or `interpolate()` calls, Storm cannot distinguish a pure SQL literal from a string with accidentally concatenated interpolations. The `storm.validation.interpolation_mode` system property controls how Storm handles this situation: +When a `TemplateBuilder` lambda runs without the compiler plugin, Storm cannot verify that every string interpolation is wrapped in a `t()` or `interpolate()` call: a single unwrapped interpolation concatenates its value directly into the SQL. Explicit `t()` calls do not satisfy the check, because they say nothing about the other interpolations in the same template. The `storm.validation.interpolation_mode` system property controls how Storm handles templates it cannot verify: | Value | Behavior | |-------|----------| @@ -114,16 +114,20 @@ When a `TemplateBuilder` lambda runs without the compiler plugin and without any | `fail` | Throws an `IllegalStateException`. Recommended for production. | | `none` | Disables the check entirely. | +Any other value fails with an `IllegalStateException` naming the valid values. + In `warn` mode (the default), Storm logs the following message: ``` -WARNING: TemplateBuilder lambda executed without the Storm compiler plugin and without -explicit t() or interpolate() calls. If this template uses string interpolations, values may -have been concatenated directly into the SQL, risking SQL injection. +WARNING: TemplateBuilder lambda executed without the Storm compiler plugin. Storm cannot +verify that every string interpolation is wrapped in a t() or interpolate() call; an +unwrapped interpolation concatenates its value directly into the SQL, risking SQL injection. See https://orm.st/string-templates for setup instructions. To change this behavior, set -Dstorm.validation.interpolation_mode=warn|fail|none. ``` +The warning is logged once per JVM: one occurrence identifies the problem, and the remedy (applying the compiler plugin) is global. `fail` mode applies to every template. + This helps catch cases where the compiler plugin is missing from the build configuration, causing interpolated values to be concatenated directly into the SQL string instead of being parameterized. **Configuring the mode:** @@ -167,7 +171,7 @@ If the compiler plugin is not available, you can wrap interpolations in `t()` ma orm.query { "SELECT ${t(User::class)} FROM ${t(User::class)} WHERE id = ${t(id)}" } ``` -When using `t()` manually, the interpolation safety check is automatically suppressed because Storm detects the explicit calls. If you use pure literal templates without any interpolations, you can disable the check with the JVM system property: +Manual `t()` calls cannot be verified at runtime, so templates built without the compiler plugin still trigger the [interpolation safety check](#interpolation-safety) and log a warning by default. If you wrap every interpolation manually, or use pure literal templates without any interpolations, you can disable the check with the JVM system property: ```bash -Dstorm.validation.interpolation_mode=none diff --git a/storm-kotlin/src/main/kotlin/st/orm/template/TemplateString.kt b/storm-kotlin/src/main/kotlin/st/orm/template/TemplateString.kt index f19e0c902..11739073e 100644 --- a/storm-kotlin/src/main/kotlin/st/orm/template/TemplateString.kt +++ b/storm-kotlin/src/main/kotlin/st/orm/template/TemplateString.kt @@ -18,6 +18,7 @@ package st.orm.template import st.orm.template.TemplateString.Companion.combine import st.orm.template.TemplateString.Companion.raw import st.orm.template.TemplateString.Companion.wrap +import java.util.concurrent.atomic.AtomicBoolean /** * Represents a compiled SQL template string that can be passed to Storm's query engine for execution. @@ -107,9 +108,10 @@ public interface TemplateContext { * [TemplateBuilder] lambda it transforms. It should not be called manually. * * At runtime, Storm uses this signal to verify interpolation safety: if a [TemplateBuilder] lambda executes - * without this method being called and without any explicit [t] or [interpolate] calls, Storm acts based on the - * `storm.validation.interpolation_mode` system property: `warn` (default) logs a warning, `fail` throws an - * [IllegalStateException], and `none` disables the check entirely. + * without this method being called, Storm cannot verify that all string interpolations are wrapped and acts + * based on the `storm.validation.interpolation_mode` system property: `warn` (default) logs a warning once per + * JVM, `fail` throws an [IllegalStateException], and `none` disables the check entirely. Any other value fails + * with an [IllegalStateException]. */ public fun autoInterpolation() {} } @@ -118,20 +120,18 @@ public interface TemplateContext { * Builds this [TemplateBuilder] into a [TemplateString] ready for use with the Storm query engine. * * This method validates interpolation safety by checking whether the Storm compiler plugin processed the lambda - * (via [TemplateContext.autoInterpolation]) or whether explicit [TemplateContext.t]/[TemplateContext.interpolate] - * calls were made. If neither is detected, the behavior depends on the `storm.validation.interpolation_mode` system - * property: `warn` (default) logs a warning, `fail` throws an [IllegalStateException], and `none` disables the check. + * (via [TemplateContext.autoInterpolation]). Explicit [TemplateContext.t]/[TemplateContext.interpolate] calls do not + * satisfy the check, because they cannot prove that every interpolation in the template is wrapped. If the plugin + * marker is absent, the behavior depends on the `storm.validation.interpolation_mode` system property: `warn` + * (default) logs a warning once per JVM, `fail` throws an [IllegalStateException], and `none` disables the check. + * Any other value fails with an [IllegalStateException]. */ public fun TemplateBuilder.build(): TemplateString { var autoInterpolation = false - var interpolateCalled = false val coreTemplate = st.orm.core.template.TemplateBuilder.create { ctx -> with( object : TemplateContext { - override fun interpolate(o: Any?): String { - interpolateCalled = true - return ctx.interpolate(o) - } + override fun interpolate(o: Any?): String = ctx.interpolate(o) override fun autoInterpolation() { autoInterpolation = true } @@ -139,20 +139,28 @@ public fun TemplateBuilder.build(): TemplateString { this, ) } - if (!autoInterpolation && !interpolateCalled) { - // No plugin marker and no t()/interpolate() calls. The result could be: - // 1. A pure literal (safe), or - // 2. String interpolations concatenated without t() wrapping (SQL injection risk). - // We cannot distinguish these cases at runtime, so we act based on the configured mode. - val message = "TemplateBuilder lambda executed without the Storm compiler plugin and without explicit t() " + - "or interpolate() calls. If this template uses string interpolations, values may have been " + - "concatenated directly into the SQL, risking SQL injection. " + + if (!autoInterpolation) { + // The plugin marker is the only reliable safety signal. A raw interpolation is plain string concatenation + // by the time the lambda runs, so explicit t() calls cannot prove that the remaining interpolations are + // wrapped, and a pure literal is indistinguishable from a string with concatenated values. Without the + // marker, the configured mode decides how to act. + val message = "TemplateBuilder lambda executed without the Storm compiler plugin. Storm cannot verify " + + "that every string interpolation is wrapped in a t() or interpolate() call; an unwrapped " + + "interpolation concatenates its value directly into the SQL, risking SQL injection. " + "See https://orm.st/string-templates for setup instructions. " + "To change this behavior, set -Dstorm.validation.interpolation_mode=warn|fail|none." - when (InterpolationMode.mode) { + val mode = InterpolationMode.mode + when (mode.trim().lowercase()) { "fail" -> throw IllegalStateException(message) - "warn" -> InterpolationMode.logger.log(System.Logger.Level.WARNING, message) - // "off" -> do nothing + // One warning identifies the problem and the remedy is global (apply the plugin), so repeating it for + // every template would only flood the logs of applications that wrap interpolations manually. + "warn" -> if (InterpolationMode.warned.compareAndSet(false, true)) { + InterpolationMode.logger.log(System.Logger.Level.WARNING, message) + } + "none" -> {} + else -> throw IllegalStateException( + "Invalid storm.validation.interpolation_mode: '$mode'. Valid values are: warn, fail, none.", + ) } } return TemplateStringHolder(coreTemplate) @@ -160,7 +168,8 @@ public fun TemplateBuilder.build(): TemplateString { private object InterpolationMode { val logger: System.Logger = System.getLogger("st.orm.template") - val mode: String = System.getProperty("storm.validation.interpolation_mode", "warn") + val mode: String get() = System.getProperty("storm.validation.interpolation_mode", "warn") + val warned: AtomicBoolean = AtomicBoolean() } internal data class TemplateStringHolder( diff --git a/storm-kotlin/src/test/kotlin/st/orm/template/InterpolationSafetyTest.kt b/storm-kotlin/src/test/kotlin/st/orm/template/InterpolationSafetyTest.kt new file mode 100644 index 000000000..f89ab334b --- /dev/null +++ b/storm-kotlin/src/test/kotlin/st/orm/template/InterpolationSafetyTest.kt @@ -0,0 +1,111 @@ +package st.orm.template + +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.string.shouldContain +import org.junit.jupiter.api.Test + +/** + * Tests for the interpolation safety check applied by [build]. + * + * The Storm compiler plugin only transforms lambdas, so the named extension functions at the bottom of this file + * execute without the [TemplateContext.autoInterpolation] marker: they model templates compiled without the plugin. + * Lambdas declared in this module are transformed by the plugin during test compilation and carry the marker. + */ +class InterpolationSafetyTest { + + @Test + fun `fail mode throws for a template without the plugin marker`() { + withInterpolationMode("fail") { + val exception = shouldThrow { + TemplateString.raw(TemplateContext::literalWithoutMarker) + } + exception.message shouldContain "compiler plugin" + } + } + + @Test + fun `fail mode throws when explicit t calls are present but the plugin marker is absent`() { + withInterpolationMode("fail") { + val exception = shouldThrow { + TemplateString.raw(TemplateContext::explicitTWithoutMarker) + } + exception.message shouldContain "compiler plugin" + } + } + + @Test + fun `fail mode accepts a lambda transformed by the compiler plugin`() { + withInterpolationMode("fail") { + val name = "Alice" + TemplateString.raw { "SELECT * FROM city WHERE name = $name" }.shouldNotBeNull() + } + } + + @Test + fun `fail mode accepts a pure literal lambda transformed by the compiler plugin`() { + withInterpolationMode("fail") { + TemplateString.raw { "SELECT COUNT(*) FROM city" }.shouldNotBeNull() + } + } + + @Test + fun `warn mode builds a template without the plugin marker`() { + withInterpolationMode("warn") { + TemplateString.raw(TemplateContext::explicitTWithoutMarker).shouldNotBeNull() + } + } + + @Test + fun `none mode disables the check`() { + withInterpolationMode("none") { + TemplateString.raw(TemplateContext::explicitTWithoutMarker).shouldNotBeNull() + } + } + + @Test + fun `mode matching is trimmed and case-insensitive`() { + withInterpolationMode(" FAIL ") { + val exception = shouldThrow { + TemplateString.raw(TemplateContext::literalWithoutMarker) + } + exception.message shouldContain "compiler plugin" + } + } + + @Test + fun `unknown mode fails fast naming the valid values`() { + withInterpolationMode("off") { + val exception = shouldThrow { + TemplateString.raw(TemplateContext::literalWithoutMarker) + } + exception.message shouldContain "Valid values are: warn, fail, none" + } + } + + @Test + fun `unknown mode is not consulted when the plugin marker is present`() { + withInterpolationMode("bogus") { + TemplateString.raw { "SELECT COUNT(*) FROM city" }.shouldNotBeNull() + } + } +} + +private const val MODE_PROPERTY = "storm.validation.interpolation_mode" + +private fun withInterpolationMode(mode: String, block: () -> Unit) { + val previous = System.getProperty(MODE_PROPERTY) + System.setProperty(MODE_PROPERTY, mode) + try { + block() + } finally { + if (previous == null) System.clearProperty(MODE_PROPERTY) else System.setProperty(MODE_PROPERTY, previous) + } +} + +private fun TemplateContext.literalWithoutMarker(): String = "SELECT COUNT(*) FROM city" + +private fun TemplateContext.explicitTWithoutMarker(): String { + val name = "Alice" + return "SELECT * FROM city WHERE name = ${t(name)} OR alt_name = '$name'" +} diff --git a/storm-kotlinx-serialization/pom.xml b/storm-kotlinx-serialization/pom.xml index 4c4d3be89..f8d34467e 100644 --- a/storm-kotlinx-serialization/pom.xml +++ b/storm-kotlinx-serialization/pom.xml @@ -56,7 +56,6 @@ org.jetbrains.kotlin kotlin-maven-plugin ${kotlin.version} - true 21 @@ -105,6 +104,11 @@ kotlin-maven-serialization ${kotlin.version} + + st.orm + storm-compiler-plugin-${kotlin.major.minor} + ${project.version} +