From bfa39fd456d1bb0bde98fc5c088b14d7b0ff29d7 Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Sun, 12 Jul 2026 11:09:05 +0200 Subject: [PATCH 1/2] feat(spring): Boot 4 embedded test database and sampled-only SQL comments Two polish items surfaced by running the example applications on Spring Boot 4. - @DataStormTest provides an embedded test database on Spring Boot 4: a fallback auto-configuration in the slice activates only when Boot's own test-database replacement is absent (relocated to spring-boot-jdbc-test-autoconfigure in Boot 4), an embedded driver is on the test classpath, and spring.test.database.replace is not none. Ordered before the DataSource auto-configuration, whose back-off then applies naturally; inert on Boot 3. Slice tests behave identically on both generations without manual datasource properties. - Trace-context SQL comments gain a sampled-only mode: TraceContextSqlCommenter(tracer, onlySampled) comments only statements of sampled traces, aligning the prepared-statement-caching cost with the correlation benefit when sampling is below 1.0. In the starters, storm.tracing.sql-comments accepts true or sampled, and unknown values fail startup with a descriptive error. The docs also gain a composition warning: integration beans in user configurations must be unconditional, because @ConditionalOnBean in a plain @Configuration evaluates before auto-configurations contribute their beans and fails silently. Fixes #238 Fixes #239 --- CHANGELOG.md | 2 + docs/ktor-integration.md | 2 +- docs/spring-integration.md | 8 +- .../micrometer/TraceContextSqlCommenter.java | 21 ++++- .../st/orm/micrometer/SqlCommenterTest.java | 16 ++++ .../StormAutoConfigurationTest.java | 21 +++++ storm-spring-boot-test-autoconfigure/pom.xml | 4 + ...ataStormTestDatabaseAutoConfiguration.java | 86 +++++++++++++++++++ ...orm.spring.boot.test.DataStormTest.imports | 1 + .../st/orm/spring/boot/StormProperties.java | 16 ++-- .../boot/StormTracingAutoConfiguration.java | 22 ++++- 11 files changed, 183 insertions(+), 16 deletions(-) create mode 100644 storm-spring-boot-test-autoconfigure/src/main/java/st/orm/spring/boot/test/DataStormTestDatabaseAutoConfiguration.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d239ee1d..b6d30bdb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ Framework integration points are now instance-scoped (#198), and the Ktor integr - `OtelDatabaseObservationConvention` (storm-micrometer): opt-in OpenTelemetry database client semantic conventions on query observations — `db.system.name` (derived from the JDBC URL), `db.operation.name`, and `db.query.text` (placeholders only) alongside the `storm.*` key values, so Storm queries surface in the database tooling of OTLP-capable observability backends. Activated with `storm.observations.semantic-conventions=otel` in the Spring Boot starters (database product detected from the DataSource) or by registering the convention in the Ktor dependency container. - Trace context in SQL statements: the `SqlCommenter` hook (template builder, all language stacks) appends per-execution comment content to statements after all processing and caching; `TraceContextSqlCommenter` (storm-micrometer) renders the current span as a sqlcommenter-style W3C `traceparent` comment, correlating database-side diagnostics such as slow query logs with traces. Opt-in everywhere — `storm.tracing.sql-comments=true` in the Spring Boot starters (with a `Tracer` present), the `sqlCommenter` slot in the Ktor plugin — since a per-execution comment defeats prepared statement caching. Hostile content is rejected (comment terminator, semicolons), and the emitted comment is padded so MySQL/MariaDB executable-comment and optimizer-hint markers are never interpreted. - Transactions are observed alongside queries: every physical transaction (outermost block or `REQUIRES_NEW`, blocking, suspend, and Spring-bridged alike) reports as a `storm.transaction` Micrometer Observation with duration, outcome (`committed`/`rolled_back`), propagation, and read-only key values; joined blocks are not double-counted. The `QueryObserver` SPI gains a default no-op transaction hook, so existing observers and wiring are unaffected. +- `@DataStormTest` provides an embedded test database on Spring Boot 4 as well: a fallback in the slice activates when Boot's own test-database replacement (relocated to spring-boot-jdbc-test-autoconfigure in Boot 4) is absent, so the slice behaves identically on both generations; `spring.test.database.replace=none` opts out for Testcontainers setups. +- Trace-context SQL comments gain a sampled-only mode: `TraceContextSqlCommenter(tracer, onlySampled)` and `storm.tracing.sql-comments=sampled` comment only statements of sampled traces, aligning the prepared-statement-caching cost with the correlation benefit; unknown property values fail fast. - `@EnableStormRepositories(basePackages = ...)` (storm-spring, usable from both language stacks): switches on repository scanning in plain Spring applications, mirroring `@EnableJpaRepositories`; without packages, the annotated class's package is scanned. In Spring Boot the annotation doubles as the explicit override: the auto-configured scanning backs off. The `RepositoryBeanFactoryPostProcessor` adapters also gain configuring constructors (base packages, template bean name, prefix), so multi-template applications define one bean per repository set instead of one subclass. - `@DataStormTest` test slice in the new storm-spring-boot-test-autoconfigure module, the counterpart of `@DataJpaTest`: starts only the `DataSource`, Storm's auto-configuration, and SQL initialization (plus Flyway/Liquibase when present), keeps regular components out, replaces the data source with an embedded database on Spring Boot 3 (opt out via `spring.test.database.replace=none` for Testcontainers `@ServiceConnection` setups), and rolls back a per-test transaction. One module serves both starters and both Spring Boot 3 and 4: the slice imports the starters' identically named auto-configuration classes, lists relocated platform classes as optional imports, and registers its type exclusion through a context customizer instead of the relocated `@TypeExcludeFilters`. diff --git a/docs/ktor-integration.md b/docs/ktor-integration.md index a9eb08b45..6d24d86a9 100644 --- a/docs/ktor-integration.md +++ b/docs/ktor-integration.md @@ -716,7 +716,7 @@ dependencies { } ``` -With tracing in place, the `sqlCommenter` slot appends the current trace context to every statement as a sqlcommenter-style comment, correlating database-side diagnostics such as slow query logs back to the trace. Opt-in: a per-execution comment defeats prepared statement caching. +With tracing in place, the `sqlCommenter` slot appends the current trace context to statements as a sqlcommenter-style comment, correlating database-side diagnostics such as slow query logs back to the trace. Opt-in: a per-execution comment defeats prepared statement caching; `TraceContextSqlCommenter(tracer, onlySampled = true)` limits the comments to sampled traces. ```kotlin install(Storm) { diff --git a/docs/spring-integration.md b/docs/spring-integration.md index d51fe06dc..022b81990 100644 --- a/docs/spring-integration.md +++ b/docs/spring-integration.md @@ -636,13 +636,15 @@ storm: Customization follows the usual back-off rules: contribute an `ObservationConvention` bean to override the naming and key values (it wins over the property), define your own `QueryObserver` bean to replace the binding entirely, or disable the observation at the registry level with `management.observations.enable.storm.query=false`. -With tracing in place, `storm.tracing.sql-comments: true` additionally appends the current trace context to every statement as a sqlcommenter-style comment (`/*traceparent='00-…'*/`), so database-side diagnostics — MariaDB's slow query log, `pg_stat_activity` — correlate directly back to the trace that issued the statement. This is deliberately opt-in: a per-execution comment changes the statement text on every call, which defeats driver-side and server-side prepared statement caching. Enable it where the correlation is worth that trade-off. +With tracing in place, `storm.tracing.sql-comments` additionally appends the current trace context to statements as a sqlcommenter-style comment (`/*traceparent='00-…'*/`), so database-side diagnostics — MariaDB's slow query log, `pg_stat_activity` — correlate directly back to the trace that issued the statement. `true` comments every statement inside a span; `sampled` comments only statements of sampled traces, which is the recommended mode when the sampling probability is below 1.0. This is deliberately opt-in: a per-execution comment changes the statement text on every call, which defeats driver-side and server-side prepared statement caching. + +One composition warning for applications that wire the integration beans themselves: define beans like the `ExceptionMapper`, `QueryObserver`, or `SqlCommenter` unconditionally in your `@Configuration` classes. A `@ConditionalOnBean` condition in a user configuration evaluates before auto-configurations contribute their beans (such as the `Tracer`), so it fails silently and the integration stays dormant while looking enabled. ## Testing with @DataStormTest `@DataStormTest` (from `storm-spring-boot-test-autoconfigure`, test scope) is the Storm test slice, the counterpart of `@DataJpaTest`: it starts only the `DataSource`, Storm's auto-configuration (template, repository scanning, transaction integration, schema validation, exception translation), and SQL initialization, plus Flyway or Liquibase when present. It complements [`@StormTest`](testing.md), which tests data logic without a Spring context: use `@StormTest` for fast query-level tests, and the slice when the test should see what production Spring code sees — injected repository beans, translated exceptions, Spring-managed transactions, and your `storm.*` configuration. Regular `@Component`, `@Service`, and `@Controller` beans stay out of the context. Each test method runs in a transaction that is rolled back afterwards, so tests cannot see each other's writes. -On Spring Boot 3 the application's `DataSource` is replaced with an embedded in-memory database by default; put a `schema.sql` (and optionally `data.sql`) on the test classpath to initialize it, or let Flyway or Liquibase (included in the slice) create the schema as in production. On Spring Boot 4 the replacement activates when the `spring-boot-jdbc-test-autoconfigure` artifact is present; without it, point the slice at a database with the `properties` attribute: +The application's `DataSource` is replaced with an embedded in-memory database by default, on Spring Boot 3 and 4 alike (the slice ships a fallback for Boot 4's relocated test-database support); put a `schema.sql` (and optionally `data.sql`) on the test classpath to initialize it, or let Flyway or Liquibase (included in the slice) create the schema as in production: ```kotlin @DataStormTest @@ -662,7 +664,7 @@ class VisitRepositoryTest( } ``` -To run against a real database instead, disable the replacement (`spring.test.database.replace=none` on Spring Boot 3; the default without the test-database artifact on Spring Boot 4) and hand the slice a Testcontainers-managed database through `@ServiceConnection`: +To run against a real database instead, disable the replacement with `spring.test.database.replace=none` and hand the slice a Testcontainers-managed database through `@ServiceConnection`: ```kotlin @DataStormTest(properties = ["spring.test.database.replace=none"]) diff --git a/storm-micrometer/src/main/java/st/orm/micrometer/TraceContextSqlCommenter.java b/storm-micrometer/src/main/java/st/orm/micrometer/TraceContextSqlCommenter.java index 02da0fd04..c04808847 100644 --- a/storm-micrometer/src/main/java/st/orm/micrometer/TraceContextSqlCommenter.java +++ b/storm-micrometer/src/main/java/st/orm/micrometer/TraceContextSqlCommenter.java @@ -41,14 +41,30 @@ public class TraceContextSqlCommenter implements SqlCommenter { private final Tracer tracer; + private final boolean onlySampled; /** - * Creates a commenter reading the current span from the given tracer. + * Creates a commenter that comments every statement executed inside a span. * * @param tracer the tracer providing the current trace context. */ public TraceContextSqlCommenter(@Nonnull Tracer tracer) { + this(tracer, false); + } + + /** + * Creates a commenter that optionally comments only statements of sampled traces. + * + *

With sampling below 1.0, commenting every statement pays the prepared statement caching cost for + * comments whose trace is mostly not exported; sampled-only mode aligns the cost with the correlation + * benefit.

+ * + * @param tracer the tracer providing the current trace context. + * @param onlySampled whether to comment only when the current span is sampled. + */ + public TraceContextSqlCommenter(@Nonnull Tracer tracer, boolean onlySampled) { this.tracer = requireNonNull(tracer, "tracer"); + this.onlySampled = onlySampled; } @Override @@ -58,6 +74,9 @@ public Optional comment() { return Optional.empty(); } TraceContext context = span.context(); + if (onlySampled && !Boolean.TRUE.equals(context.sampled())) { + return Optional.empty(); + } String flags = Boolean.TRUE.equals(context.sampled()) ? "01" : "00"; return Optional.of("traceparent='00-%s-%s-%s'".formatted(context.traceId(), context.spanId(), flags)); } diff --git a/storm-micrometer/src/test/java/st/orm/micrometer/SqlCommenterTest.java b/storm-micrometer/src/test/java/st/orm/micrometer/SqlCommenterTest.java index c9f851dbd..a1334ba87 100644 --- a/storm-micrometer/src/test/java/st/orm/micrometer/SqlCommenterTest.java +++ b/storm-micrometer/src/test/java/st/orm/micrometer/SqlCommenterTest.java @@ -116,6 +116,22 @@ public void executableCommentMarkersAreNeutralizedByPadding() { statements.toString()); } + @Test + public void sampledOnlyModeSkipsUnsampledSpans() { + var tracer = new SimpleTracer(); + var sampledOnly = new TraceContextSqlCommenter(tracer, true); + var always = new TraceContextSqlCommenter(tracer); + var span = tracer.nextSpan().start(); + try (var ignored = tracer.withSpan(span)) { + // SimpleTracer spans are not sampled: the sampled-only commenter stays silent while the + // default commenter still renders the trace identity. + assertEquals(Optional.empty(), sampledOnly.comment()); + assertTrue(always.comment().isPresent()); + } finally { + span.end(); + } + } + @Test public void traceContextCommenterRendersTraceparent() { var tracer = new SimpleTracer(); diff --git a/storm-spring-boot-starter/src/test/java/st/orm/spring/boot/autoconfigure/StormAutoConfigurationTest.java b/storm-spring-boot-starter/src/test/java/st/orm/spring/boot/autoconfigure/StormAutoConfigurationTest.java index c6125973a..02df652cd 100644 --- a/storm-spring-boot-starter/src/test/java/st/orm/spring/boot/autoconfigure/StormAutoConfigurationTest.java +++ b/storm-spring-boot-starter/src/test/java/st/orm/spring/boot/autoconfigure/StormAutoConfigurationTest.java @@ -577,6 +577,27 @@ void unknownSemanticConventionsValueFailsFast() { .run(context -> assertThat(context).hasFailed()); } + @Test + void traceSqlCommentsSupportSampledModeAndFailFastOnUnknownValues() { + contextRunner + .withPropertyValues( + "spring.datasource.url=jdbc:h2:mem:sqlCommentsSampledTest;DB_CLOSE_DELAY=-1", + "spring.datasource.driver-class-name=org.h2.Driver", + "storm.tracing.sql-comments=sampled" + ) + .withBean(io.micrometer.tracing.Tracer.class, () -> org.mockito.Mockito.mock(io.micrometer.tracing.Tracer.class)) + .run(context -> assertThat(context).getBean(st.orm.core.spi.SqlCommenter.class) + .isInstanceOf(st.orm.micrometer.TraceContextSqlCommenter.class)); + contextRunner + .withPropertyValues( + "spring.datasource.url=jdbc:h2:mem:sqlCommentsBogusTest;DB_CLOSE_DELAY=-1", + "spring.datasource.driver-class-name=org.h2.Driver", + "storm.tracing.sql-comments=bogus" + ) + .withBean(io.micrometer.tracing.Tracer.class, () -> org.mockito.Mockito.mock(io.micrometer.tracing.Tracer.class)) + .run(context -> assertThat(context).hasFailed()); + } + @Test void traceSqlCommentsAreOptIn() { // Off by default even with a Tracer present: a per-execution comment defeats prepared statement diff --git a/storm-spring-boot-test-autoconfigure/pom.xml b/storm-spring-boot-test-autoconfigure/pom.xml index 4079b2002..0e2305847 100644 --- a/storm-spring-boot-test-autoconfigure/pom.xml +++ b/storm-spring-boot-test-autoconfigure/pom.xml @@ -120,6 +120,10 @@ org.springframework spring-test + + org.springframework + spring-jdbc + org.springframework spring-tx diff --git a/storm-spring-boot-test-autoconfigure/src/main/java/st/orm/spring/boot/test/DataStormTestDatabaseAutoConfiguration.java b/storm-spring-boot-test-autoconfigure/src/main/java/st/orm/spring/boot/test/DataStormTestDatabaseAutoConfiguration.java new file mode 100644 index 000000000..6f03af0f2 --- /dev/null +++ b/storm-spring-boot-test-autoconfigure/src/main/java/st/orm/spring/boot/test/DataStormTestDatabaseAutoConfiguration.java @@ -0,0 +1,86 @@ +/* + * Copyright 2024 - 2026 the original author or authors. + * + * Licensed 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 st.orm.spring.boot.test; + +import javax.sql.DataSource; +import org.springframework.boot.autoconfigure.AutoConfiguration; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.condition.NoneNestedConditions; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; + +/** + * Fallback embedded test database for {@link DataStormTest @DataStormTest} slices on Spring Boot 4. + * + *

Spring Boot 3 replaces the application's {@code DataSource} with an embedded database through its own + * {@code TestDatabaseAutoConfiguration}, which the slice imports. Spring Boot 4 hosts that behavior in the + * separate {@code spring-boot-jdbc-test-autoconfigure} artifact; when it is absent, this fallback provides + * an embedded H2 database so the slice behaves identically on both generations.

+ * + *

The fallback stays inert whenever Boot's own replacement is present (either location on the classpath), + * when H2 is not on the test classpath, or when {@code spring.test.database.replace=none} requests the + * configured database, such as a Testcontainers-managed one.

+ * + * @since 1.13 + */ +@AutoConfiguration( + beforeName = { + // Spring Boot 3.x location. + "org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration", + // Spring Boot 4.x location. + "org.springframework.boot.jdbc.autoconfigure.DataSourceAutoConfiguration", + }) +@ConditionalOnMissingClass({ + // Boot's own replacement wins by presence; both its locations checked. + "org.springframework.boot.test.autoconfigure.jdbc.TestDatabaseAutoConfiguration", + "org.springframework.boot.jdbc.test.autoconfigure.TestDatabaseAutoConfiguration", +}) +@ConditionalOnClass(name = "org.h2.Driver") +@Conditional(DataStormTestDatabaseAutoConfiguration.ReplacementRequested.class) +public class DataStormTestDatabaseAutoConfiguration { + + /** + * Provides the embedded test database, taking precedence over the application's configured data source + * through the DataSource auto-configuration's own back-off. + */ + @Bean + @ConditionalOnMissingBean(DataSource.class) + public DataSource dataStormTestDatabase() { + return new EmbeddedDatabaseBuilder() + .generateUniqueName(true) + .setType(EmbeddedDatabaseType.H2) + .build(); + } + + /** + * Matches unless {@code spring.test.database.replace=none} requests the configured database. + */ + static class ReplacementRequested extends NoneNestedConditions { + + ReplacementRequested() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @ConditionalOnProperty(name = "spring.test.database.replace", havingValue = "none") + static class ReplaceNone { + } + } +} diff --git a/storm-spring-boot-test-autoconfigure/src/main/resources/META-INF/spring/st.orm.spring.boot.test.DataStormTest.imports b/storm-spring-boot-test-autoconfigure/src/main/resources/META-INF/spring/st.orm.spring.boot.test.DataStormTest.imports index 93408533f..d9e1ed991 100644 --- a/storm-spring-boot-test-autoconfigure/src/main/resources/META-INF/spring/st.orm.spring.boot.test.DataStormTest.imports +++ b/storm-spring-boot-test-autoconfigure/src/main/resources/META-INF/spring/st.orm.spring.boot.test.DataStormTest.imports @@ -1,3 +1,4 @@ +st.orm.spring.boot.test.DataStormTestDatabaseAutoConfiguration optional:org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration optional:org.springframework.boot.jdbc.autoconfigure.DataSourceAutoConfiguration optional:org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration diff --git a/storm-spring/src/main/java/st/orm/spring/boot/StormProperties.java b/storm-spring/src/main/java/st/orm/spring/boot/StormProperties.java index 0bb819703..050d9ffdc 100644 --- a/storm-spring/src/main/java/st/orm/spring/boot/StormProperties.java +++ b/storm-spring/src/main/java/st/orm/spring/boot/StormProperties.java @@ -245,16 +245,18 @@ public static class Validation { public static class Tracing { /** - * Whether the current trace context is appended to SQL statements as a sqlcommenter-style comment. - * Defaults to false: a per-execution comment defeats prepared statement caching. + * Whether the current trace context is appended to SQL statements as a sqlcommenter-style comment: + * "true" comments every statement inside a span, "sampled" comments only statements of sampled + * traces, "false" (default) disables the comments. A per-execution comment defeats prepared + * statement caching; prefer "sampled" when the sampling probability is below 1.0. */ - private Boolean sqlComments; + private String sqlComments; - /** Returns whether trace context SQL comments are enabled. */ - public Boolean getSqlComments() { return sqlComments; } + /** Returns the trace context SQL comment mode. */ + public String getSqlComments() { return sqlComments; } - /** Sets whether trace context SQL comments are enabled. */ - public void setSqlComments(Boolean sqlComments) { this.sqlComments = sqlComments; } + /** Sets the trace context SQL comment mode. */ + public void setSqlComments(String sqlComments) { this.sqlComments = sqlComments; } } /** Query observation configuration. */ diff --git a/storm-spring/src/main/java/st/orm/spring/boot/StormTracingAutoConfiguration.java b/storm-spring/src/main/java/st/orm/spring/boot/StormTracingAutoConfiguration.java index c18c2c58e..987436093 100644 --- a/storm-spring/src/main/java/st/orm/spring/boot/StormTracingAutoConfiguration.java +++ b/storm-spring/src/main/java/st/orm/spring/boot/StormTracingAutoConfiguration.java @@ -21,7 +21,9 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; +import st.orm.PersistenceException; import st.orm.core.spi.SqlCommenter; import st.orm.micrometer.TraceContextSqlCommenter; @@ -40,15 +42,27 @@ @AutoConfiguration @ConditionalOnClass({Tracer.class, TraceContextSqlCommenter.class}) @ConditionalOnBean(Tracer.class) -@ConditionalOnProperty(name = "storm.tracing.sql-comments", havingValue = "true") +@ConditionalOnProperty(name = "storm.tracing.sql-comments") +@EnableConfigurationProperties(StormProperties.class) public class StormTracingAutoConfiguration { /** - * Provides the SQL commenter that appends the current trace context to statements. + * Provides the SQL commenter that appends the current trace context to statements: every statement + * inside a span with {@code storm.tracing.sql-comments=true}, or only statements of sampled traces + * with {@code sampled}. */ @Bean @ConditionalOnMissingBean(SqlCommenter.class) - public SqlCommenter stormSqlCommenter(Tracer tracer) { - return new TraceContextSqlCommenter(tracer); + public SqlCommenter stormSqlCommenter(Tracer tracer, StormProperties properties) { + String mode = properties.getTracing().getSqlComments(); + if ("true".equalsIgnoreCase(mode)) { + return new TraceContextSqlCommenter(tracer); + } + if ("sampled".equalsIgnoreCase(mode)) { + return new TraceContextSqlCommenter(tracer, true); + } + throw new PersistenceException( + "Unknown storm.tracing.sql-comments value: '%s'. Expected 'true', 'sampled' or 'false'." + .formatted(mode)); } } From 465efb97c61d643bb446a59fa20d51747ae7970d Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Sun, 12 Jul 2026 11:31:08 +0200 Subject: [PATCH 2/2] security: harden schema metadata queries and Ref deserialization A security audit before the 1.13 release. No exploitable vulnerability was found in a correctly-using application; these are defense-in-depth hardening changes. - storm-core DatabaseSchema: the INFORMATION_SCHEMA and ALL_* metadata queries used by schema validation are now built with bound parameters instead of escaped string literals. The catalog and schema names come from the JDBC connection (database-supplied) and were already escaped by doubling quotes, which is correct SQL literal escaping, so this is not a fix for an exploit; it removes SQL-text assembly from concatenated values entirely. Verified behavior-preserving against the schema validation integration suite. - storm-kotlinx-serialization: resolving a Ref's target class from a serializer now loads the class without initialization and rejects it unless it is a Storm Data type, before the class is used. Previously the class was initialized (static blocks run) before the unchecked Data cast, which erases at runtime and validated nothing. The serial name is normally the declared entity type, so this hardens a path that is not reachable with hostile data in a normal application. - storm-test SqlCapture gains a javadoc note that captured statements retain bound parameter values and must not be routed to logs or external systems from production code. - Docs: the serialization guide warns that a loaded Ref serializes its full entity and recommends projections for response shapes that must not over-expose fields. --- CHANGELOG.md | 5 + docs/serialization.md | 4 + .../core/template/impl/DatabaseSchema.java | 92 ++++++++++++------- .../serialization/StormSerializersModule.kt | 60 ++++++------ .../src/main/java/st/orm/test/SqlCapture.java | 4 + 5 files changed, 104 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6d30bdb3..8dcd06be8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,11 @@ Framework integration points are now instance-scoped (#198), and the Ktor integr - `st.orm.spring.impl.ResolverRegistration` (both language stacks): the autowire-candidate resolver is installed by the scanning engine itself. - `storm-ktor-koin`: Ktor's built-in dependency injection is the supported DI path. Koin users keep full capability with a few lines of application code; the Ktor integration docs include the recipe. +### Security + +- Schema validation reads database metadata through bound parameters instead of building the metadata queries with escaped string literals (storm-core `DatabaseSchema`). The catalog and schema names are database-supplied and were already correctly escaped, so this is defense in depth: no metadata query is assembled from concatenated values. +- The kotlinx-serialization module validates that a `Ref` target resolves to a Storm `Data` type before loading it, and loads it without running static initializers until that check passes. A serializer that reported an unrelated class name can no longer trigger that class's initialization. + ## [1.12.0] - 2026-07-06 Feature release centered on the reified Kotlin query API. Breaking changes are accepted with no deprecation shims (1.12 policy). diff --git a/docs/serialization.md b/docs/serialization.md index 05028bdc5..76874a31f 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -117,6 +117,10 @@ An unloaded ref serializes as a bare value because there is nothing more to conv A loaded entity ref wraps the full entity data in an `@entity` object. This tells the deserializer that the enclosed data is a complete entity, from which it can reconstruct a loaded ref with `getOrNull()` returning the entity instance. +:::warning Data exposure +Serialization never triggers a fetch, but a ref that is already loaded (for example because a query joined it) serializes its full entity, including every field of the referenced type. When the response shape matters, such as a public REST API, return a projection that exposes only the intended fields rather than an entity whose refs may carry more than the endpoint should reveal. Serialization reflects exactly what is loaded; it does not filter. +::: + A loaded projection ref uses a different wrapper (`@projection`) and includes a separate `@id` field. The explicit ID is necessary because projections are partial views of an entity and may not expose an `id()` accessor. Without the separate `@id` field, the deserializer would have no reliable way to recover the primary key. Both Jackson and kotlinx.serialization produce identical JSON for the same ref state, so output from one library can be consumed by the other. diff --git a/storm-core/src/main/java/st/orm/core/template/impl/DatabaseSchema.java b/storm-core/src/main/java/st/orm/core/template/impl/DatabaseSchema.java index bc8d3d501..01e4491b3 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/DatabaseSchema.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/DatabaseSchema.java @@ -19,6 +19,7 @@ import jakarta.annotation.Nullable; import java.sql.Connection; import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; @@ -319,16 +320,39 @@ private static boolean appendFilter( @Nonnull StringBuilder sql, boolean hasCondition, @Nonnull String column, - @Nullable String value + @Nullable String value, + @Nonnull List parameters ) { if (value == null || value.isEmpty()) { return hasCondition; } sql.append(hasCondition ? " AND " : " WHERE "); - sql.append(column).append(" = '").append(value.replace("'", "''")).append("'"); + // Bind the value as a parameter; the column is always a compile-time constant. + sql.append(column).append(" = ?"); + parameters.add(value); return true; } + /** + * Prepares the given metadata query and binds the collected filter values as parameters. + */ + private static PreparedStatement prepareSchemaQuery( + @Nonnull Connection connection, + @Nonnull String sql, + @Nonnull List parameters + ) throws SQLException { + var statement = connection.prepareStatement(sql); + try { + for (int index = 0; index < parameters.size(); index++) { + statement.setString(index + 1, parameters.get(index)); + } + } catch (SQLException e) { + statement.close(); + throw e; + } + return statement; + } + /** * Reads primary keys and unique keys from {@code INFORMATION_SCHEMA.TABLE_CONSTRAINTS} joined with * {@code KEY_COLUMN_USAGE}. This helper is shared by both {@code INFORMATION_SCHEMA} strategies. @@ -351,10 +375,11 @@ private static void readPrimaryAndUniqueKeysFromInformationSchema( AND tc.CONSTRAINT_NAME = kcu.CONSTRAINT_NAME AND tc.TABLE_NAME = kcu.TABLE_NAME WHERE tc.CONSTRAINT_TYPE IN ('PRIMARY KEY', 'UNIQUE')"""); - appendFilter(sql, true, "tc.TABLE_CATALOG", catalog); - appendFilter(sql, true, "tc.TABLE_SCHEMA", schemaPattern); - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, true, "tc.TABLE_CATALOG", catalog, parameters); + appendFilter(sql, true, "tc.TABLE_SCHEMA", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { String tableName = resultSet.getString("TABLE_NAME"); if (!columnsByTable.containsKey(tableName)) { @@ -409,10 +434,11 @@ private static void readConstraintsFromInformationSchema( AND rc.UNIQUE_CONSTRAINT_NAME = kcu2.CONSTRAINT_NAME AND kcu.POSITION_IN_UNIQUE_CONSTRAINT = kcu2.ORDINAL_POSITION WHERE 1=1"""); - appendFilter(sql, true, "rc.CONSTRAINT_CATALOG", catalog); - appendFilter(sql, true, "rc.CONSTRAINT_SCHEMA", schemaPattern); - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, true, "rc.CONSTRAINT_CATALOG", catalog, parameters); + appendFilter(sql, true, "rc.CONSTRAINT_SCHEMA", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { String fkTableName = resultSet.getString("FK_TABLE"); if (!columnsByTable.containsKey(fkTableName)) { @@ -454,9 +480,10 @@ private static void readConstraintsFromInformationSchemaReferencing( SELECT TABLE_NAME, COLUMN_NAME, REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE REFERENCED_TABLE_NAME IS NOT NULL"""); - appendFilter(sql, true, "TABLE_SCHEMA", effectiveSchema); - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, true, "TABLE_SCHEMA", effectiveSchema, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { String fkTableName = resultSet.getString("TABLE_NAME"); if (!columnsByTable.containsKey(fkTableName)) { @@ -495,9 +522,10 @@ private static void readConstraintsFromAllConstraints( ON ac.OWNER = acc.OWNER AND ac.CONSTRAINT_NAME = acc.CONSTRAINT_NAME WHERE ac.CONSTRAINT_TYPE IN ('P', 'U')"""); - appendFilter(sql, true, "ac.OWNER", schemaPattern); - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, true, "ac.OWNER", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { String tableName = resultSet.getString("TABLE_NAME"); if (!columnsByTable.containsKey(tableName)) { @@ -536,9 +564,10 @@ private static void readConstraintsFromAllConstraints( AND pk.CONSTRAINT_NAME = pkc.CONSTRAINT_NAME AND fkc.POSITION = pkc.POSITION WHERE fk.CONSTRAINT_TYPE = 'R'"""); - appendFilter(sql, true, "fk.OWNER", schemaPattern); - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, true, "fk.OWNER", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { String fkTableName = resultSet.getString("FK_TABLE"); if (!columnsByTable.containsKey(fkTableName)) { @@ -590,17 +619,11 @@ private static void readSequencesFromInformationSchema( ) { try { StringBuilder sql = new StringBuilder("SELECT SEQUENCE_NAME FROM INFORMATION_SCHEMA.SEQUENCES"); - boolean hasCondition = false; - if (catalog != null && !catalog.isEmpty()) { - sql.append(" WHERE SEQUENCE_CATALOG = '").append(catalog.replace("'", "''")).append("'"); - hasCondition = true; - } - if (schemaPattern != null && !schemaPattern.isEmpty()) { - sql.append(hasCondition ? " AND" : " WHERE"); - sql.append(" SEQUENCE_SCHEMA = '").append(schemaPattern.replace("'", "''")).append("'"); - } - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + boolean hasCondition = appendFilter(sql, false, "SEQUENCE_CATALOG", catalog, parameters); + appendFilter(sql, hasCondition, "SEQUENCE_SCHEMA", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { sequences.put(resultSet.getString("SEQUENCE_NAME"), Boolean.TRUE); } @@ -620,11 +643,10 @@ private static void readSequencesFromAllSequences( ) { try { StringBuilder sql = new StringBuilder("SELECT SEQUENCE_NAME FROM ALL_SEQUENCES"); - if (schemaPattern != null && !schemaPattern.isEmpty()) { - sql.append(" WHERE SEQUENCE_OWNER = '").append(schemaPattern.replace("'", "''")).append("'"); - } - try (var statement = connection.createStatement(); - var resultSet = statement.executeQuery(sql.toString())) { + List parameters = new ArrayList<>(); + appendFilter(sql, false, "SEQUENCE_OWNER", schemaPattern, parameters); + try (var statement = prepareSchemaQuery(connection, sql.toString(), parameters); + var resultSet = statement.executeQuery()) { while (resultSet.next()) { sequences.put(resultSet.getString("SEQUENCE_NAME"), Boolean.TRUE); } diff --git a/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/StormSerializersModule.kt b/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/StormSerializersModule.kt index 175587cbe..a50fb2b44 100644 --- a/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/StormSerializersModule.kt +++ b/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/StormSerializersModule.kt @@ -314,19 +314,11 @@ private fun resolveTargetClass(serializer: KSerializer<*>): Class { when { serializerClassName.endsWith("\$\$\$serializer") -> { val targetClassName = serializerClassName.removeSuffix("\$\$\$serializer") - try { - @Suppress("UNCHECKED_CAST") - return Class.forName(targetClassName, true, serializerClass.classLoader) as Class - } catch (_: ClassNotFoundException) { - } + loadDataClassOrNull(targetClassName, serializerClass.classLoader)?.let { return it } } serializerClassName.endsWith("\$\$serializer") -> { val targetClassName = serializerClassName.removeSuffix("\$\$serializer") - try { - @Suppress("UNCHECKED_CAST") - return Class.forName(targetClassName, true, serializerClass.classLoader) as Class - } catch (_: ClassNotFoundException) { - } + loadDataClassOrNull(targetClassName, serializerClass.classLoader)?.let { return it } } } // Strategy 2: Use serialName from descriptor. @@ -345,23 +337,39 @@ private fun resolveTargetClass(serializer: KSerializer<*>): Class { private fun resolveClassFromSerialName(name: String, classLoader: ClassLoader): Class { // Try direct name first. - try { - @Suppress("UNCHECKED_CAST") - return Class.forName(name, true, classLoader) as Class - } catch (_: ClassNotFoundException) { - // Try converting dots to $ for nested classes, from right to left. - val parts = name.split('.') - for (i in parts.size - 2 downTo 1) { - if (parts[i].firstOrNull()?.isUpperCase() == true) { - val nestedName = parts.take(i).joinToString(".") + "." + parts.drop(i).joinToString("$") - try { - @Suppress("UNCHECKED_CAST") - return Class.forName(nestedName, true, classLoader) as Class - } catch (_: ClassNotFoundException) { - continue - } - } + loadDataClassOrNull(name, classLoader)?.let { return it } + // Try converting dots to $ for nested classes, from right to left. + val parts = name.split('.') + for (i in parts.size - 2 downTo 1) { + if (parts[i].firstOrNull()?.isUpperCase() == true) { + val nestedName = parts.take(i).joinToString(".") + "." + parts.drop(i).joinToString("$") + loadDataClassOrNull(nestedName, classLoader)?.let { return it } } } throw ClassNotFoundException("Could not resolve class from serialName: $name") } + +/** + * Loads a [Data] class by name, or returns null when the name does not resolve to a loadable [Data] type. + * + * The class is loaded without initialization and validated to be a [Data] subtype before it is returned, so + * a name that resolves to an unrelated class never triggers that class's static initializers. The serial + * name a serializer reports is normally the declared entity or projection type, so this is defense in depth + * against a serializer configured to report an arbitrary name. + */ +private fun loadDataClassOrNull(name: String, classLoader: ClassLoader): Class? { + val cls = try { + Class.forName(name, false, classLoader) + } catch (_: ClassNotFoundException) { + return null + } catch (_: LinkageError) { + return null + } + if (!Data::class.java.isAssignableFrom(cls)) { + throw SerializationException( + "Refusing to deserialize Ref target that is not a Storm Data type: $name.", + ) + } + @Suppress("UNCHECKED_CAST") + return cls as Class +} diff --git a/storm-test/src/main/java/st/orm/test/SqlCapture.java b/storm-test/src/main/java/st/orm/test/SqlCapture.java index d8da923e5..550acbfb6 100644 --- a/storm-test/src/main/java/st/orm/test/SqlCapture.java +++ b/storm-test/src/main/java/st/orm/test/SqlCapture.java @@ -34,6 +34,10 @@ * *

This class is not thread-safe by design: the scoped observer is bound to the calling thread.

* + *

Testing only. Captured statements retain their bound parameter values, which may be + * sensitive (credentials, personal data). This class lives in the test-scoped {@code storm-test} module; do + * not route captured statements to logs or external systems from production code.

+ * * @since 1.9 */ public final class SqlCapture {