security: harden schema metadata queries and Ref deserialization#243
Merged
Conversation
…ents 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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A security audit ahead of the 1.13 release, run as three parallel sweeps (SQL construction, deserialization/reflection, logging/CI/supply-chain). No exploitable vulnerability was found in a correctly-using application. The framework's fundamentals held up: parameterized queries by construction,
unsafe()-gated destructive DML, no bound-parameter values in logs/exceptions/observations, no credential logging, a zero-dependency CLI with no install hooks, and a tag-gated GPG-signed release pipeline. These are defense-in-depth hardening changes.Changes
DatabaseSchema: theINFORMATION_SCHEMAandALL_*metadata queries used by schema validation are built with bound parameters instead of escaped string literals. The catalog and schema names come from the JDBC connection (database-supplied) and were already correctly escaped by doubling quotes, so this is not a fix for an exploit; it removes SQL-text assembly from concatenated values entirely. Verified behavior-preserving against the 92-test schema-validation integration suite on H2.Ref's target class from a serializer now loads the class without initialization and rejects it unless it is a StormDatatype, before the class is used. Previously the class was initialized (static blocks run) before an uncheckedas Class<out Data>cast that erases at runtime and validated nothing. The serial name is normally the declared entity type, so this hardens a path not reachable with hostile data in a normal application.SqlCapture: javadoc notes that captured statements retain bound parameter values (potentially sensitive) and must not be routed to logs or external systems from production code.Refserializes its full entity and recommends projections for response shapes that must not over-expose fields.Assessment notes
Both automated sweeps flagged "critical" findings that did not survive review: the
DatabaseSchema"injection" relied on an incorrect exploit (doubling single quotes is correct, complete SQL literal escaping, and the input is not user-controlled), and the kotlinx "RCE" requires an attacker to already control application serializer configuration rather than data. The hardening above is worth doing regardless, since a data-layer library should not assemble SQL from values or initialize arbitrary named classes.Supply-chain hygiene (CI dependency scanning, GitHub Actions SHA-pinning) protects release integrity rather than library users and is tracked separately in #242.
Verification
Full reactor green (13,758 tests), including the schema-validation integration suites that exercise the converted metadata queries and the full kotlinx-serialization suite (192 tests) confirming the class-resolution change is behavior-preserving for legitimate types.