Fix false negative: full SQL-injection sink coverage for Spring R2DBC + io.r2dbc.spi

[felickz]

Summary

Fixes a false-negative gap in SQL-injection detection for Spring R2DBC's deferred DatabaseClient.sql(Supplier<String>) pattern. This PR goes beyond the minimal fix to give full, TDD-validated coverage of both affected data extensions, per a deep-dive assessment of the whole spring-r2dbc / io.r2dbc.spi API surface.

java/ext/manual/org.springframework.r2dbc.model.yml

Previously only DatabaseClient.GenericExecuteSpec.fetch() was modeled as a sink. That missed:

• Terminal methods: then(), map(Function), map(BiFunction), mapValue(Class), mapProperties(Class), flatMap(Function) — 6 new sinkModel rows
• Fluent methods that must carry taint through the chain: bind(int/String,Object), bindNull(int/String,Class), bindValues(List/Map), bindProperties(Object), filter(Function/StatementFilterFunction) — 9 new summaryModel rows

Every one of the 16 total rows (7 sink + 9 summary) has a dedicated, empirically-verified test — not architectural reasoning by analogy.

java/ext/manual/io.r2dbc.spi.model.yml

Full audit of the driver-agnostic R2DBC SPI (Connection, Statement, Batch) found 4 more unmodeled sinks:

• Connection.createSavepoint(String)
• Connection.releaseSavepoint(String)
• Connection.rollbackTransactionToSavepoint(String)
• Statement.returnGeneratedValues(String...)

Each takes a raw identifier (savepoint name / column name) that real drivers splice unescaped into the executed SQL text, since the wire protocol has no parameter placeholder for identifiers (only for literal values). Verified against r2dbc-postgresql 1.0.2.RELEASE source:

• PostgresqlConnection.createSavepoint/releaseSavepoint/rollbackTransactionToSavepoint → String.format("SAVEPOINT %s", name) (and RELEASE SAVEPOINT / ROLLBACK TO SAVEPOINT), zero escaping
• GeneratedValuesUtils.augment() → String.format("%s RETURNING %s", sql, String.join(", ", columns)), zero escaping

With Connection/Statement/Batch now fully enumerated against real interface source, every String/String[]-carrying method that can influence executed SQL text is modeled.

Methodology

For every row (existing and new): write a failing test exercising the exact real method signature → add the model row → confirm the test passes → confirm no regression in the broader suite. This caught two real bugs along the way:

1. A test-stub structural bug — StatementFilterFunction needed to be a top-level type (matching real Spring), not nested inside DatabaseClient, or the model's fully-qualified signature string silently fails to match.
2. An accidental .expected corruption from copying tool-generated line-number prefixes as if they were file content.

Testing

• codeql test run test/security/CWE-089/spring-r2dbc passes cleanly (21 detected flows, all expected).
• Broader codeql test run test/security/CWE-089 shows only pre-existing, unrelated MyBatis failures caused by a missing external stub checkout path in this environment — not caused by this change.

Scope notes

• Scoped to the sql(Supplier<String>) pattern on GenericExecuteSpec as of current spring-framework main, and the io.r2dbc.spi SPI as of current r2dbc-spi main. Other deferred-SQL entry points elsewhere in spring-data-r2dbc (e.g. R2dbcEntityTemplate) were not audited in this pass.
• IsolationLevel.valueOf(String sql) technically stores a raw SQL string later spliced into SET TRANSACTION ISOLATION LEVEL <asSql()>, but real code always uses the 4 static constants rather than a dynamic string — judged not worth modeling given the added complexity for near-zero real-world occurrence.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR fixes a SQL-injection false negative in the Java security modeling by expanding sink and taint-propagation coverage for Spring R2DBC’s deferred DatabaseClient.sql(Supplier<String>) API and by adding additional SQL-identifier-related sinks in the low-level io.r2dbc.spi interfaces. The changes are validated via expanded, signature-specific CWE-089 tests and updated expected results.

Changes:

• Add execution-stage sink coverage for additional DatabaseClient.GenericExecuteSpec terminal methods (then, map*, flatMap) when SQL is deferred via sql(Supplier<String>).
• Add fluent-chain taint preservation for GenericExecuteSpec binding/filtering methods (bind*, bindValues*, bindProperties, filter*) so deferred-SQL taint is not dropped before reaching execution-stage sinks.
• Model additional io.r2dbc.spi sinks for identifier-splicing APIs (Connection.*Savepoint(String), Statement.returnGeneratedValues(String...)) and extend the test suite accordingly.

Show a summary per file

File	Description
java/ext/manual/org.springframework.r2dbc.model.yml	Expands Spring R2DBC sink + summary models to cover deferred-SQL execution-stage methods and preserve taint through fluent bind/filter chains.
java/ext/manual/io.r2dbc.spi.model.yml	Adds additional R2DBC SPI sinks for savepoint names and RETURNING column identifiers.
java/test/security/CWE-089/spring-r2dbc/org/springframework/r2dbc/core/DatabaseClient.java	Extends the Spring DatabaseClient test stub to include the newly modeled methods/signatures used by tests.
java/test/security/CWE-089/spring-r2dbc/org/springframework/r2dbc/core/StatementFilterFunction.java	Adds a top-level stub type needed to exercise filter(StatementFilterFunction) signature matching.
java/test/security/CWE-089/spring-r2dbc/io/r2dbc/spi/Statement.java	Extends the R2DBC SPI test stub with returnGeneratedValues(String...).
java/test/security/CWE-089/spring-r2dbc/io/r2dbc/spi/Connection.java	Extends the R2DBC SPI test stub with savepoint methods that are now modeled as sinks.
java/test/security/CWE-089/spring-r2dbc/com/example/DeferredQueryHandler.java	Adds targeted test cases covering each new Spring R2DBC sink/summary row for deferred SQL.
java/test/security/CWE-089/spring-r2dbc/com/example/RawR2dbcHandler.java	Adds targeted test cases for the newly modeled R2DBC SPI sinks (savepoints and generated values).
java/test/security/CWE-089/spring-r2dbc/DatabaseClientSqlInjection.expected	Updates expected flows to reflect the newly modeled sinks/paths.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 0
• Review effort level: Lite