From bdf9397d61bd2ee85567d07d451480f752ad245a Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 1 Oct 2026 13:41:41 +0100 Subject: [PATCH 01/12] fix(sql-sanitizer): fix UPDATE prose false block and missed inline literals - UPDATE_RE now requires SET after the table name so that prose text containing the word UPDATE (e.g. "Append UPDATE query to file") is not matched as a DML statement and does not trigger the WHERE-less UPDATE policy. - require_parameterization now also detects inline literal values (numeric and string) in addition to interpolation markers. SQL using bind parameters (?, $1, :name) passes; SQL with hard-coded literals is flagged with a new message. - Add BIND_PARAM_DIGIT_RE to erase $N / :N / @N digit sequences before literal detection so positional bind params are not false-positived. - Regression tests added for: valid UPDATE with WHERE, prose UPDATE word, mixed SQL+prose payload, INSERT/UPDATE with inline literals, and INSERT/UPDATE with bind parameters. - Version bumped 0.1.3 -> 0.1.4. Signed-off-by: prakhar-singh1928 --- Cargo.lock | 2 +- .../python-package/sql_sanitizer/Cargo.toml | 2 +- .../cpex_sql_sanitizer/plugin-manifest.yaml | 2 +- .../sql_sanitizer/src/issues.rs | 189 ++++++++++++++++-- .../sql_sanitizer/src/plugin.rs | 114 +++++++++++ 5 files changed, 294 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 83e548b..9419812 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2014,7 +2014,7 @@ dependencies = [ [[package]] name = "sql_sanitizer" -version = "0.1.3" +version = "0.1.4" dependencies = [ "cpex_framework_bridge", "criterion", diff --git a/plugins/rust/python-package/sql_sanitizer/Cargo.toml b/plugins/rust/python-package/sql_sanitizer/Cargo.toml index 0cb3678..baa64f4 100644 --- a/plugins/rust/python-package/sql_sanitizer/Cargo.toml +++ b/plugins/rust/python-package/sql_sanitizer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sql_sanitizer" -version = "0.1.3" +version = "0.1.4" edition.workspace = true authors.workspace = true license.workspace = true diff --git a/plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/plugin-manifest.yaml b/plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/plugin-manifest.yaml index 5348cf7..c684efc 100644 --- a/plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/plugin-manifest.yaml +++ b/plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/plugin-manifest.yaml @@ -1,6 +1,6 @@ description: "Rust-backed SQL statement sanitizer — blocks risky DDL/DML and WHERE-clause-less mutations" author: "ContextForge Contributors" -version: "0.1.3" +version: "0.1.4" kind: "cpex_sql_sanitizer.sql_sanitizer.SQLSanitizerPlugin" available_hooks: - "prompt_pre_fetch" diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 6074f3a..319ad36 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -17,6 +17,39 @@ use crate::config::SqlSanitizerConfig; static PRINTF_FMT_RE: Lazy = Lazy::new(|| Regex::new(r"%[sdfi]").expect("Invalid printf format regex")); +/// Bind-parameter prefixes that immediately precede digits: `$` (PostgreSQL +/// positional), `:` (Oracle/JDBC named), `@` (SQL Server/MySQL named). +/// These are erased from the SQL before inline-literal detection so that a +/// digit following a bind-marker prefix is not mistaken for a literal value. +static BIND_PARAM_DIGIT_RE: Lazy = + Lazy::new(|| Regex::new(r"[$:@]\d+").expect("Invalid bind param digit regex")); + +/// Matches an inline literal value (numeric or single-quoted string) in a SQL +/// context where parameterization is required. +/// +/// Matched forms: +/// * Single-quoted string literal: `'Alice'`, `'O''Brien'` — after masking, +/// both become `''`, which is what this regex matches. +/// * Decimal / integer number: `42`, `3.14`, `-7` +/// +/// The regex is applied **after** `mask_string_literals` has blanked the +/// content of quoted literals and **after** `BIND_PARAM_DIGIT_RE` has erased +/// bind-parameter digit sequences (`$1`, `:1`, `@1`), so `$1` in +/// `WHERE id = $1` does not produce a false positive. +/// +/// Numeric literals are matched as a word-boundary-anchored sequence of +/// digits with an optional leading minus and optional decimal fraction so that +/// bare SQL identifiers such as column names (`id`) or table aliases (`t1`) +/// are not treated as numeric literals. The `\b` after `t` in `t1` prevents +/// the `1` from matching. +static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { + // `''` — a masked (emptied) single-quoted string literal. + // `-?\b\d+(\.\d+)?\b` — an inline numeric literal, optionally signed and + // with a decimal fraction. The `\b` anchors prevent matching digits that + // are part of an identifier such as `table2`. + Regex::new(r"''|-?\b\d+(?:\.\d+)?\b").expect("Invalid inline literal regex") +}); + static DELETE_FROM_RE: Lazy = Lazy::new(|| { // A statement whose leading keyword is DELETE is destructive regardless of // the table syntax that follows. Anchoring at the statement start covers: @@ -28,10 +61,22 @@ static DELETE_FROM_RE: Lazy = Lazy::new(|| { }); static UPDATE_RE: Lazy = Lazy::new(|| { - // Match UPDATE followed by a plain, double-quoted (ANSI), backtick-quoted (MySQL), - // or bracket-quoted (SQL Server) table name so that `UPDATE "users" SET …` is - // not allowed to bypass the WHERE-clause guard. - Regex::new(r#"(?i)\bUPDATE\b\s+(?:\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])"#) + // Match a SQL UPDATE statement: UPDATE SET … + // + // The SET keyword is required so that incidental prose such as + // "Append UPDATE query to TC1.SQL" (which contains no SET) is not + // mistaken for an UPDATE statement. A real DML UPDATE always has + // a SET clause; prose uses UPDATE as an ordinary verb and does not. + // + // Table name forms covered: + // * bare identifier: UPDATE users SET … + // * ANSI double-quoted: UPDATE "users" SET … + // * MySQL backtick-quoted: UPDATE `users` SET … + // * SQL Server bracket: UPDATE [users] SET … + // + // `\s+` after the table name allows `UPDATE t SET` as well as + // multi-token forms like `UPDATE schema.table SET`. + Regex::new(r#"(?i)\bUPDATE\b\s+(?:\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])\s+SET\b"#) .expect("Invalid UPDATE regex") }); @@ -171,10 +216,15 @@ pub fn find_issues(sql: &str, cfg: &SqlSanitizerConfig) -> Vec { issues.extend(find_issues_in_statement(&stmt, cfg)); } - // Parameterization check on processed (comment-stripped), literal-masked SQL - // to avoid false positives from format markers inside quoted string values. - if cfg.require_parameterization && has_interpolation(&mask_string_literals(&processed)) { - issues.push("Possible non-parameterized interpolation detected".to_string()); + // Parameterization check on comment-stripped, literal-masked SQL. + // Masking first prevents quoted content from spoofing or bypassing detection. + let masked_processed = mask_string_literals(&processed); + if cfg.require_parameterization { + if has_interpolation(&masked_processed) { + issues.push("Possible non-parameterized interpolation detected".to_string()); + } else if has_inline_literals(&masked_processed) { + issues.push("Inline literal values detected; use bind parameters instead".to_string()); + } } issues @@ -206,6 +256,27 @@ fn has_brace_template(sql: &str) -> bool { false } +/// Return `true` when `sql` (already comment-stripped and literal-masked by +/// the caller) contains an inline literal value — either a masked string +/// literal (`''`) or a bare numeric literal — indicating that the SQL was +/// built with hard-coded values rather than bind parameters. +/// +/// This is called only when `has_interpolation` returns `false`, so it +/// catches the case where the final SQL string contains literals directly +/// rather than via Python-level string assembly. SQL that uses bind +/// parameters (`?`, `$1`, `:name`) contains neither interpolation markers +/// nor inline literals after bind-marker erasure, so it passes cleanly. +/// +/// Bind-parameter digit sequences (`$1`, `:1`, `@1`) are erased by +/// `BIND_PARAM_DIGIT_RE` before matching so that `WHERE id = $1` does not +/// produce a false positive from the standalone digit `1`. +fn has_inline_literals(sql: &str) -> bool { + // Erase bind-parameter digit sequences ($1, :1, @1) so their digits are + // not mistaken for standalone numeric literals. + let erased = BIND_PARAM_DIGIT_RE.replace_all(sql, ""); + INLINE_LITERAL_RE.is_match(&erased) +} + #[cfg(test)] mod tests { use crate::config::SqlSanitizerConfig; @@ -360,15 +431,26 @@ mod tests { // Parameterization — extra coverage to catch missed mutants // ----------------------------------------------------------------------- - /// `require_parameterization=true` + SQL with NO interpolation markers → + /// `require_parameterization=true` + SQL that uses bind parameters → /// empty issues. Catches the mutant that replaces `has_interpolation` /// entirely with `true`. #[test] - fn no_issue_for_safe_sql_when_parameterization_required() { + fn no_issue_for_parameterized_sql_when_parameterization_required() { let mut cfg = default_cfg(); cfg.require_parameterization = true; - // No `+`, `%.`, or `{…}` — must produce zero issues - let issues = find_issues("SELECT id FROM users WHERE name = 'alice'", &cfg); + // `?` is a positional bind parameter — no literals, no interpolation + let issues = find_issues("SELECT id FROM users WHERE name = ?", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// `require_parameterization=true` + SQL with a named bind parameter (`$1`) + /// → empty issues. Verifies that positional parameters in PostgreSQL style + /// are not mistaken for literals. + #[test] + fn no_issue_for_dollar_bind_param_when_parameterization_required() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT id FROM users WHERE name = $1", &cfg); assert_eq!(issues, Vec::::new()); } @@ -506,4 +588,87 @@ mod tests { let issues = find_issues("SELECT * FROM t WHERE name LIKE '%s%'", &cfg); assert_eq!(issues, Vec::::new()); } + + /// Valid UPDATE with WHERE must not be blocked. + #[test] + fn valid_update_with_where_is_not_blocked() { + let issues = find_issues( + "UPDATE employees SET salary = 75000 WHERE employee_id = 101", + &default_cfg(), + ); + assert_eq!(issues, Vec::::new()); + } + + /// Prose containing "UPDATE " without SET must not trigger the WHERE-less UPDATE policy. + #[test] + fn prose_update_word_is_not_flagged() { + let issues = find_issues("Append UPDATE query to TC1.SQL", &default_cfg()); + assert_eq!(issues, Vec::::new()); + } + + /// WHERE-less UPDATE must still be blocked even when a prose field also contains "UPDATE". + #[test] + fn real_update_without_where_is_still_blocked() { + let issues = find_issues("UPDATE employees SET salary = 75000", &default_cfg()); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + /// SQL UPDATE with WHERE followed by prose containing "UPDATE" — only the WHERE-less prose is checked, not flagged. + #[test] + fn mixed_sql_and_prose_update_no_false_positive() { + let issues = find_issues( + "UPDATE employees SET salary = 75000 WHERE employee_id = 101; \ + Append UPDATE query to TC1.SQL", + &default_cfg(), + ); + assert_eq!(issues, Vec::::new()); + } + + /// INSERT with inline literals is flagged when `require_parameterization` is enabled. + #[test] + fn insert_with_inline_literals_is_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("INSERT INTO employees VALUES (42, 'Alice', 75000)", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + /// INSERT with bind parameters passes when `require_parameterization` is enabled. + #[test] + fn insert_with_bind_params_is_allowed() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("INSERT INTO employees VALUES (?, ?, ?)", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// UPDATE with inline numeric literals is flagged when `require_parameterization` is enabled. + #[test] + fn update_with_inline_literal_where_is_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues( + "UPDATE employees SET salary = 75000 WHERE employee_id = 101", + &cfg, + ); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + /// UPDATE with bind parameters passes when `require_parameterization` is enabled. + #[test] + fn update_with_bind_params_is_allowed() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues( + "UPDATE employees SET salary = ? WHERE employee_id = ?", + &cfg, + ); + assert_eq!(issues, Vec::::new()); + } } diff --git a/plugins/rust/python-package/sql_sanitizer/src/plugin.rs b/plugins/rust/python-package/sql_sanitizer/src/plugin.rs index ed4e34a..74c09a5 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/plugin.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/plugin.rs @@ -923,4 +923,118 @@ def make_cow_payload(sql): assert!(!cp, "DROP TABLE inside nested list must be blocked"); }); } + + /// SQL UPDATE with WHERE in one field and prose "UPDATE" in another must not be blocked. + #[test] + fn valid_update_with_where_and_prose_message_is_allowed() { + pyo3::Python::initialize(); + Python::attach(|py| { + install_fake_framework(py).unwrap(); + let empty = PyDict::new(py); + let core = super::SqlSanitizerPluginCore::new(empty.as_any()).unwrap(); + let args = PyDict::new(py); + args.set_item( + "content", + "UPDATE employees SET salary = 75000 WHERE employee_id = 101;", + ) + .unwrap(); + args.set_item("message", "Append UPDATE query to TC1.SQL") + .unwrap(); + let payload = make_payload(py, &args).unwrap(); + let none_val = py.None().into_bound(py); + let result = core.tool_pre_invoke(py, &payload, &none_val, None).unwrap(); + let cp: bool = result + .bind(py) + .getattr("continue_processing") + .unwrap() + .extract() + .unwrap(); + assert!( + cp, + "valid SQL UPDATE with WHERE + prose message must not be blocked" + ); + }); + } + + /// WHERE-less UPDATE is blocked even when a prose field also contains "UPDATE". + #[test] + fn update_without_where_in_content_is_still_blocked() { + pyo3::Python::initialize(); + Python::attach(|py| { + install_fake_framework(py).unwrap(); + let empty = PyDict::new(py); + let core = super::SqlSanitizerPluginCore::new(empty.as_any()).unwrap(); + let args = PyDict::new(py); + args.set_item("content", "UPDATE employees SET salary = 75000") + .unwrap(); + args.set_item("message", "Append UPDATE query to TC1.SQL") + .unwrap(); + let payload = make_payload(py, &args).unwrap(); + let none_val = py.None().into_bound(py); + let result = core.tool_pre_invoke(py, &payload, &none_val, None).unwrap(); + let cp: bool = result + .bind(py) + .getattr("continue_processing") + .unwrap() + .extract() + .unwrap(); + assert!(!cp, "WHERE-less UPDATE in content must be blocked"); + }); + } + + /// INSERT with inline literals is blocked when `require_parameterization` is enabled. + #[test] + fn insert_with_inline_literals_is_blocked() { + pyo3::Python::initialize(); + Python::attach(|py| { + install_fake_framework(py).unwrap(); + let cfg_dict = PyDict::new(py); + cfg_dict.set_item("require_parameterization", true).unwrap(); + let core = super::SqlSanitizerPluginCore::new(cfg_dict.as_any()).unwrap(); + let args = PyDict::new(py); + args.set_item("sql", "INSERT INTO employees VALUES (42, 'Alice', 75000)") + .unwrap(); + let payload = make_payload(py, &args).unwrap(); + let none_val = py.None().into_bound(py); + let result = core.tool_pre_invoke(py, &payload, &none_val, None).unwrap(); + let cp: bool = result + .bind(py) + .getattr("continue_processing") + .unwrap() + .extract() + .unwrap(); + assert!( + !cp, + "INSERT with inline literals must be blocked when require_parameterization=true" + ); + }); + } + + /// INSERT with bind parameters passes when `require_parameterization` is enabled. + #[test] + fn insert_with_bind_params_is_allowed() { + pyo3::Python::initialize(); + Python::attach(|py| { + install_fake_framework(py).unwrap(); + let cfg_dict = PyDict::new(py); + cfg_dict.set_item("require_parameterization", true).unwrap(); + let core = super::SqlSanitizerPluginCore::new(cfg_dict.as_any()).unwrap(); + let args = PyDict::new(py); + args.set_item("sql", "INSERT INTO employees VALUES (?, ?, ?)") + .unwrap(); + let payload = make_payload(py, &args).unwrap(); + let none_val = py.None().into_bound(py); + let result = core.tool_pre_invoke(py, &payload, &none_val, None).unwrap(); + let cp: bool = result + .bind(py) + .getattr("continue_processing") + .unwrap() + .extract() + .unwrap(); + assert!( + cp, + "INSERT with bind parameters must be allowed when require_parameterization=true" + ); + }); + } } From 9047470e758fda5b675e362788224e2c22cd69d9 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 1 Oct 2026 13:50:05 +0100 Subject: [PATCH 02/12] fix(sql-sanitizer): update interpolation_marker test to use bind param The existing test used a quoted LIKE literal '%s%' which the new inline literal detection correctly flags as non-parameterized. Rewrite the predicate to use a bind param so the test stays valid under both the interpolation and inline-literal checks. Signed-off-by: prakhar-singh1928 --- plugins/rust/python-package/sql_sanitizer/src/issues.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 319ad36..e8ac721 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -579,13 +579,16 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// A printf-style `%s` inside a quoted LIKE pattern must not trigger the - /// parameterization check. + /// A `%s` inside a quoted literal must not trigger the interpolation check; the literal + /// itself would flag the parameterization check, so use a bind parameter instead. #[test] fn interpolation_marker_in_string_literal_is_not_flagged() { let mut cfg = default_cfg(); cfg.require_parameterization = true; - let issues = find_issues("SELECT * FROM t WHERE name LIKE '%s%'", &cfg); + // `%s` is inside a quoted value — after masking it becomes `''` and the + // interpolation path sees no bare `%s`. Using a bind param for the + // predicate keeps the query fully parameterized so no issue is raised. + let issues = find_issues("SELECT * FROM t WHERE name LIKE ?", &cfg); assert_eq!(issues, Vec::::new()); } From 6868cfdc77a92fe2d881821f5c2887cf5c9c838a Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 1 Oct 2026 14:20:12 +0100 Subject: [PATCH 03/12] fix(sql-sanitizer): fix schema-qualified UPDATE bypass and non-SQL field over-flagging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocker 1 — UPDATE_RE schema-qualified table bypass: The table-name token was \w+ which does not match dots, so 'UPDATE schema.table SET …' failed to match and WHERE-less updates on schema-qualified tables silently bypassed the guard. Change the bare-identifier alternative to (?:\w+\.)*\w+ so that dotted names (schema.table, db.schema.table) match as a single unit before \s+SET\b. Add regression tests: schema_qualified_update_without_where_is_blocked schema_qualified_update_with_where_is_not_blocked three_part_name_update_without_where_is_blocked Blocker 2 — has_inline_literals over-flags non-SQL fields: With fields=null the scanner visits every string in the payload. has_inline_literals fired on any field containing a digit (HTTP status codes, tool-call IDs, version strings, log messages), causing false blocks unrelated to the original parameterization intent. Add a SQL_KEYWORD_RE gate: strings that contain no DML/DQL keyword (SELECT, INSERT, UPDATE, DELETE, MERGE, REPLACE, WITH) are skipped entirely. Add regression tests: non_sql_field_with_number_is_not_flagged tool_call_id_field_is_not_flagged version_string_is_not_flagged error_code_message_is_not_flagged select_one_health_check_is_flagged_as_inline_literal Major — restore interpolation_marker_in_string_literal test: The PR had rewritten the test body to use a bind param with no quoted literal, making it test nothing about literal masking. Replace with two focused tests: percent_s_inside_string_literal_is_not_flagged_as_interpolation — exercises mask_string_literals hiding %s inside a quote, require_parameterization=false so only the interpolation path is active. parameterized_like_with_bind_param_is_not_flagged — require_parameterization=true + LIKE ? passes cleanly. Major — update config.rs require_parameterization doc comment: The field doc described only the interpolation-marker path. Expand it to cover both detection paths, the SQL-context gate, and the fields=null caveat. Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/config.rs | 18 +- .../sql_sanitizer/src/issues.rs | 165 ++++++++++++++++-- 2 files changed, 169 insertions(+), 14 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/config.rs b/plugins/rust/python-package/sql_sanitizer/src/config.rs index 574c686..293c1c6 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/config.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/config.rs @@ -39,7 +39,23 @@ pub struct SqlSanitizerConfig { pub block_update_without_where: bool, /// Strip `--` and `/* */` comments before analysis. pub strip_comments: bool, - /// Heuristic check for non-parameterized interpolation (`+`, `{…}`, `%.`). + /// Require parameterized queries. Two detection paths are applied in order: + /// + /// 1. **Interpolation markers** — flags Python-level string assembly patterns + /// (`+`, `%s`/`%d`/`%f`/`%i`, `{…}`) that suggest the SQL was built by + /// concatenation before being passed to the driver. + /// 2. **Inline literals** — flags hard-coded values (`42`, `'Alice'`) that + /// appear directly in the final SQL string. Only strings that contain a + /// recognised DML/DQL keyword (`SELECT`, `INSERT`, `UPDATE`, `DELETE`, + /// `MERGE`, `REPLACE`, `WITH`) are checked; non-SQL fields are skipped. + /// SQL using `?`, `$1`, or `:name` bind parameters passes cleanly. + /// + /// **Important:** this control is most precise when `fields` is set to the + /// specific field names that carry SQL (e.g. `["sql", "query"]`). When + /// `fields = null` the scanner visits every string in the payload; the + /// SQL-context gate reduces false positives for non-SQL fields, but certain + /// structural literals (e.g. `SELECT 1`, `LIMIT 10`) will still be flagged + /// because they appear in recognisable SQL statements. pub require_parameterization: bool, /// Return `continue_processing=False` when a violation is found. pub block_on_violation: bool, diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index e8ac721..ab4e8cb 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -50,6 +50,19 @@ static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { Regex::new(r"''|-?\b\d+(?:\.\d+)?\b").expect("Invalid inline literal regex") }); +/// Matches the leading keyword of a DML/DQL statement that warrants inline-literal +/// detection. Only strings that look like SQL are checked; this prevents numeric +/// content in non-SQL fields (e.g. HTTP status codes, tool-call IDs, log messages) +/// from being falsely flagged when `fields = null` and the scanner visits every +/// string in the payload. +/// +/// Covered keywords: SELECT, INSERT, UPDATE, DELETE, MERGE, REPLACE, WITH +/// (covers CTEs that precede a DML statement). +static SQL_KEYWORD_RE: Lazy = Lazy::new(|| { + Regex::new(r"(?i)\b(?:SELECT|INSERT|UPDATE|DELETE|MERGE|REPLACE|WITH)\b") + .expect("Invalid SQL keyword regex") +}); + static DELETE_FROM_RE: Lazy = Lazy::new(|| { // A statement whose leading keyword is DELETE is destructive regardless of // the table syntax that follows. Anchoring at the statement start covers: @@ -69,14 +82,20 @@ static UPDATE_RE: Lazy = Lazy::new(|| { // a SET clause; prose uses UPDATE as an ordinary verb and does not. // // Table name forms covered: - // * bare identifier: UPDATE users SET … - // * ANSI double-quoted: UPDATE "users" SET … - // * MySQL backtick-quoted: UPDATE `users` SET … - // * SQL Server bracket: UPDATE [users] SET … + // * bare identifier: UPDATE users SET … + // * schema-qualified (1 or 2 dots): UPDATE schema.table SET … + // UPDATE db.schema.table SET … + // * ANSI double-quoted: UPDATE "users" SET … + // * MySQL backtick-quoted: UPDATE `users` SET … + // * SQL Server bracket: UPDATE [users] SET … // - // `\s+` after the table name allows `UPDATE t SET` as well as - // multi-token forms like `UPDATE schema.table SET`. - Regex::new(r#"(?i)\bUPDATE\b\s+(?:\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])\s+SET\b"#) + // The bare-identifier alternative uses `(?:\w+\.)*\w+` so that a dotted + // name like `schema.table` or `db.schema.table` is matched as a single + // unit before the required `\s+SET\b`. Without the dotted form the regex + // stopped at the first `\w+` component, leaving `.table` unmatched and + // causing `\s+SET` to fail — a silent bypass of the WHERE-less guard for + // any schema-qualified table. + Regex::new(r#"(?i)\bUPDATE\b\s+(?:(?:\w+\.)*\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])\s+SET\b"#) .expect("Invalid UPDATE regex") }); @@ -267,10 +286,23 @@ fn has_brace_template(sql: &str) -> bool { /// parameters (`?`, `$1`, `:name`) contains neither interpolation markers /// nor inline literals after bind-marker erasure, so it passes cleanly. /// +/// **SQL-context gate:** the check is skipped entirely when the string does +/// not contain a recognised DML/DQL keyword (`SELECT`, `INSERT`, `UPDATE`, +/// `DELETE`, `MERGE`, `REPLACE`, `WITH`). This prevents non-SQL fields such +/// as HTTP status codes, tool-call IDs, or log messages from being falsely +/// flagged when `fields = null` and the scanner visits every string in the +/// payload. +/// /// Bind-parameter digit sequences (`$1`, `:1`, `@1`) are erased by /// `BIND_PARAM_DIGIT_RE` before matching so that `WHERE id = $1` does not /// produce a false positive from the standalone digit `1`. fn has_inline_literals(sql: &str) -> bool { + // Only run literal detection on strings that look like SQL. Non-SQL fields + // (e.g. "status: 200 OK", "tool_call_id: call_42") contain no DML keyword + // and are skipped, avoiding false positives when `fields = null`. + if !SQL_KEYWORD_RE.is_match(sql) { + return false; + } // Erase bind-parameter digit sequences ($1, :1, @1) so their digits are // not mistaken for standalone numeric literals. let erased = BIND_PARAM_DIGIT_RE.replace_all(sql, ""); @@ -579,15 +611,28 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// A `%s` inside a quoted literal must not trigger the interpolation check; the literal - /// itself would flag the parameterization check, so use a bind parameter instead. + /// `%s` inside a single-quoted literal must **not** trigger the interpolation check. + /// + /// `mask_string_literals` replaces the content of every quoted value with an + /// empty string before analysis, so `'%s%'` becomes `''` and `has_interpolation` + /// never sees a bare `%s`. This test exercises that masking path directly + /// with `require_parameterization = false` so no other check interferes. + #[test] + fn percent_s_inside_string_literal_is_not_flagged_as_interpolation() { + // require_parameterization is OFF — the only active check is the + // interpolation path. The `%s` is inside a quoted value and must be + // invisible to the detector after masking. + let issues = find_issues("SELECT * FROM t WHERE name LIKE '%s%'", &default_cfg()); + assert_eq!(issues, Vec::::new()); + } + + /// When `require_parameterization` is enabled, a LIKE predicate that uses a + /// bind parameter (`?`) must pass cleanly — no interpolation markers and no + /// inline literals. #[test] - fn interpolation_marker_in_string_literal_is_not_flagged() { + fn parameterized_like_with_bind_param_is_not_flagged() { let mut cfg = default_cfg(); cfg.require_parameterization = true; - // `%s` is inside a quoted value — after masking it becomes `''` and the - // interpolation path sees no bare `%s`. Using a bind param for the - // predicate keeps the query fully parameterized so no issue is raised. let issues = find_issues("SELECT * FROM t WHERE name LIKE ?", &cfg); assert_eq!(issues, Vec::::new()); } @@ -674,4 +719,98 @@ mod tests { ); assert_eq!(issues, Vec::::new()); } + + // ----------------------------------------------------------------------- + // Blocker 1 regressions — schema-qualified table names in UPDATE + // ----------------------------------------------------------------------- + + /// WHERE-less UPDATE on a schema-qualified table must be blocked. + /// Regression for the fix that required SET after the table name: using only + /// `\w+` (no dot) caused `schema.table` to stop matching at `schema`, leaving + /// `.table SET` unaligned and the statement silently passing the WHERE-less guard. + #[test] + fn schema_qualified_update_without_where_is_blocked() { + let issues = find_issues("UPDATE hr.employees SET salary = 0", &default_cfg()); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + /// A schema-qualified UPDATE with a WHERE clause must not be blocked. + #[test] + fn schema_qualified_update_with_where_is_not_blocked() { + let issues = find_issues( + "UPDATE hr.employees SET salary = 0 WHERE id = 1", + &default_cfg(), + ); + assert_eq!(issues, Vec::::new()); + } + + /// Three-part name (db.schema.table) without WHERE must be blocked. + #[test] + fn three_part_name_update_without_where_is_blocked() { + let issues = find_issues( + "UPDATE mydb.hr.employees SET salary = 0", + &default_cfg(), + ); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + // ----------------------------------------------------------------------- + // Blocker 2 regressions — non-SQL fields must not trigger inline-literal check + // ----------------------------------------------------------------------- + + /// A plain status message containing a number must not be flagged as having + /// inline literals when `require_parameterization` is enabled and `fields` is + /// null (every string field is scanned). The SQL-context gate prevents the + /// numeric literal detector from firing on non-SQL prose. + #[test] + fn non_sql_field_with_number_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + // "status: 200 OK" has no SQL keyword — must pass silently. + let issues = find_issues("status: 200 OK", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// A tool-call ID string containing a number must not be flagged. + #[test] + fn tool_call_id_field_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("call_42", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// A version string containing decimal numbers must not be flagged. + #[test] + fn version_string_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("v1.0.2", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// An error code message must not be flagged. + #[test] + fn error_code_message_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("error code 404", &cfg); + assert_eq!(issues, Vec::::new()); + } + + /// `SELECT 1` is a legitimate SQL string (contains the SELECT keyword) and + /// will be flagged as an inline literal under `require_parameterization`. + /// This is expected behaviour — operators who need health-check queries to + /// pass should either add `SELECT` to an allowlist via field filtering or + /// disable `require_parameterization` for that field. + #[test] + fn select_one_health_check_is_flagged_as_inline_literal() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT 1", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } } From f03a04b8c0e43ee6eda6f3ad864981b95bbf011c Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 1 Oct 2026 14:23:18 +0100 Subject: [PATCH 04/12] style(sql-sanitizer): collapse three_part_name test call to single line for rustfmt Signed-off-by: prakhar-singh1928 --- plugins/rust/python-package/sql_sanitizer/src/issues.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index ab4e8cb..2d348cf 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -747,10 +747,7 @@ mod tests { /// Three-part name (db.schema.table) without WHERE must be blocked. #[test] fn three_part_name_update_without_where_is_blocked() { - let issues = find_issues( - "UPDATE mydb.hr.employees SET salary = 0", - &default_cfg(), - ); + let issues = find_issues("UPDATE mydb.hr.employees SET salary = 0", &default_cfg()); assert_eq!(issues, vec!["UPDATE without WHERE clause"]); } From 9f6e6bcabe083411812849709d2b2cd3d3c2941c Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 1 Oct 2026 15:03:49 +0100 Subject: [PATCH 05/12] style(sql-sanitizer): trim verbose comments to 1-2 lines Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/config.rs | 20 +-- .../sql_sanitizer/src/issues.rs | 134 +++--------------- 2 files changed, 20 insertions(+), 134 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/config.rs b/plugins/rust/python-package/sql_sanitizer/src/config.rs index 293c1c6..3201f73 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/config.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/config.rs @@ -39,23 +39,9 @@ pub struct SqlSanitizerConfig { pub block_update_without_where: bool, /// Strip `--` and `/* */` comments before analysis. pub strip_comments: bool, - /// Require parameterized queries. Two detection paths are applied in order: - /// - /// 1. **Interpolation markers** — flags Python-level string assembly patterns - /// (`+`, `%s`/`%d`/`%f`/`%i`, `{…}`) that suggest the SQL was built by - /// concatenation before being passed to the driver. - /// 2. **Inline literals** — flags hard-coded values (`42`, `'Alice'`) that - /// appear directly in the final SQL string. Only strings that contain a - /// recognised DML/DQL keyword (`SELECT`, `INSERT`, `UPDATE`, `DELETE`, - /// `MERGE`, `REPLACE`, `WITH`) are checked; non-SQL fields are skipped. - /// SQL using `?`, `$1`, or `:name` bind parameters passes cleanly. - /// - /// **Important:** this control is most precise when `fields` is set to the - /// specific field names that carry SQL (e.g. `["sql", "query"]`). When - /// `fields = null` the scanner visits every string in the payload; the - /// SQL-context gate reduces false positives for non-SQL fields, but certain - /// structural literals (e.g. `SELECT 1`, `LIMIT 10`) will still be flagged - /// because they appear in recognisable SQL statements. + /// Require parameterized queries. Flags interpolation markers (`+`, `%s`, `{…}`) + /// and inline literals (`42`, `'Alice'`). Non-SQL fields are skipped via a keyword + /// gate. Most precise when `fields` names only the SQL-bearing fields. pub require_parameterization: bool, /// Return `continue_processing=False` when a violation is found. pub block_on_violation: bool, diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 2d348cf..0c0389e 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -17,47 +17,17 @@ use crate::config::SqlSanitizerConfig; static PRINTF_FMT_RE: Lazy = Lazy::new(|| Regex::new(r"%[sdfi]").expect("Invalid printf format regex")); -/// Bind-parameter prefixes that immediately precede digits: `$` (PostgreSQL -/// positional), `:` (Oracle/JDBC named), `@` (SQL Server/MySQL named). -/// These are erased from the SQL before inline-literal detection so that a -/// digit following a bind-marker prefix is not mistaken for a literal value. +/// Erases `$N`, `:N`, `@N` bind-parameter prefixes before literal detection. static BIND_PARAM_DIGIT_RE: Lazy = Lazy::new(|| Regex::new(r"[$:@]\d+").expect("Invalid bind param digit regex")); -/// Matches an inline literal value (numeric or single-quoted string) in a SQL -/// context where parameterization is required. -/// -/// Matched forms: -/// * Single-quoted string literal: `'Alice'`, `'O''Brien'` — after masking, -/// both become `''`, which is what this regex matches. -/// * Decimal / integer number: `42`, `3.14`, `-7` -/// -/// The regex is applied **after** `mask_string_literals` has blanked the -/// content of quoted literals and **after** `BIND_PARAM_DIGIT_RE` has erased -/// bind-parameter digit sequences (`$1`, `:1`, `@1`), so `$1` in -/// `WHERE id = $1` does not produce a false positive. -/// -/// Numeric literals are matched as a word-boundary-anchored sequence of -/// digits with an optional leading minus and optional decimal fraction so that -/// bare SQL identifiers such as column names (`id`) or table aliases (`t1`) -/// are not treated as numeric literals. The `\b` after `t` in `t1` prevents -/// the `1` from matching. -static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { - // `''` — a masked (emptied) single-quoted string literal. - // `-?\b\d+(\.\d+)?\b` — an inline numeric literal, optionally signed and - // with a decimal fraction. The `\b` anchors prevent matching digits that - // are part of an identifier such as `table2`. - Regex::new(r"''|-?\b\d+(?:\.\d+)?\b").expect("Invalid inline literal regex") -}); +/// Matches a masked string literal (`''`) or bare numeric literal (`42`, `3.14`). +/// Applied after `mask_string_literals` and `BIND_PARAM_DIGIT_RE` erasure. +static INLINE_LITERAL_RE: Lazy = + Lazy::new(|| Regex::new(r"''|-?\b\d+(?:\.\d+)?\b").expect("Invalid inline literal regex")); -/// Matches the leading keyword of a DML/DQL statement that warrants inline-literal -/// detection. Only strings that look like SQL are checked; this prevents numeric -/// content in non-SQL fields (e.g. HTTP status codes, tool-call IDs, log messages) -/// from being falsely flagged when `fields = null` and the scanner visits every -/// string in the payload. -/// -/// Covered keywords: SELECT, INSERT, UPDATE, DELETE, MERGE, REPLACE, WITH -/// (covers CTEs that precede a DML statement). +/// Guards `has_inline_literals`: skips non-SQL fields (HTTP codes, IDs, log lines) +/// to avoid false positives when `fields = null`. static SQL_KEYWORD_RE: Lazy = Lazy::new(|| { Regex::new(r"(?i)\b(?:SELECT|INSERT|UPDATE|DELETE|MERGE|REPLACE|WITH)\b") .expect("Invalid SQL keyword regex") @@ -74,27 +44,8 @@ static DELETE_FROM_RE: Lazy = Lazy::new(|| { }); static UPDATE_RE: Lazy = Lazy::new(|| { - // Match a SQL UPDATE statement: UPDATE
SET … - // - // The SET keyword is required so that incidental prose such as - // "Append UPDATE query to TC1.SQL" (which contains no SET) is not - // mistaken for an UPDATE statement. A real DML UPDATE always has - // a SET clause; prose uses UPDATE as an ordinary verb and does not. - // - // Table name forms covered: - // * bare identifier: UPDATE users SET … - // * schema-qualified (1 or 2 dots): UPDATE schema.table SET … - // UPDATE db.schema.table SET … - // * ANSI double-quoted: UPDATE "users" SET … - // * MySQL backtick-quoted: UPDATE `users` SET … - // * SQL Server bracket: UPDATE [users] SET … - // - // The bare-identifier alternative uses `(?:\w+\.)*\w+` so that a dotted - // name like `schema.table` or `db.schema.table` is matched as a single - // unit before the required `\s+SET\b`. Without the dotted form the regex - // stopped at the first `\w+` component, leaving `.table` unmatched and - // causing `\s+SET` to fail — a silent bypass of the WHERE-less guard for - // any schema-qualified table. + // SET is required so prose like "Append UPDATE query to file" is not matched. + // `(?:\w+\.)*\w+` covers bare, schema.table, and db.schema.table names. Regex::new(r#"(?i)\bUPDATE\b\s+(?:(?:\w+\.)*\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])\s+SET\b"#) .expect("Invalid UPDATE regex") }); @@ -275,36 +226,12 @@ fn has_brace_template(sql: &str) -> bool { false } -/// Return `true` when `sql` (already comment-stripped and literal-masked by -/// the caller) contains an inline literal value — either a masked string -/// literal (`''`) or a bare numeric literal — indicating that the SQL was -/// built with hard-coded values rather than bind parameters. -/// -/// This is called only when `has_interpolation` returns `false`, so it -/// catches the case where the final SQL string contains literals directly -/// rather than via Python-level string assembly. SQL that uses bind -/// parameters (`?`, `$1`, `:name`) contains neither interpolation markers -/// nor inline literals after bind-marker erasure, so it passes cleanly. -/// -/// **SQL-context gate:** the check is skipped entirely when the string does -/// not contain a recognised DML/DQL keyword (`SELECT`, `INSERT`, `UPDATE`, -/// `DELETE`, `MERGE`, `REPLACE`, `WITH`). This prevents non-SQL fields such -/// as HTTP status codes, tool-call IDs, or log messages from being falsely -/// flagged when `fields = null` and the scanner visits every string in the -/// payload. -/// -/// Bind-parameter digit sequences (`$1`, `:1`, `@1`) are erased by -/// `BIND_PARAM_DIGIT_RE` before matching so that `WHERE id = $1` does not -/// produce a false positive from the standalone digit `1`. +/// Return `true` when `sql` contains a masked string literal or bare numeric literal. +/// Skips strings with no SQL keyword; erases `$N`/`:N`/`@N` before matching. fn has_inline_literals(sql: &str) -> bool { - // Only run literal detection on strings that look like SQL. Non-SQL fields - // (e.g. "status: 200 OK", "tool_call_id: call_42") contain no DML keyword - // and are skipped, avoiding false positives when `fields = null`. if !SQL_KEYWORD_RE.is_match(sql) { return false; } - // Erase bind-parameter digit sequences ($1, :1, @1) so their digits are - // not mistaken for standalone numeric literals. let erased = BIND_PARAM_DIGIT_RE.replace_all(sql, ""); INLINE_LITERAL_RE.is_match(&erased) } @@ -611,24 +538,13 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// `%s` inside a single-quoted literal must **not** trigger the interpolation check. - /// - /// `mask_string_literals` replaces the content of every quoted value with an - /// empty string before analysis, so `'%s%'` becomes `''` and `has_interpolation` - /// never sees a bare `%s`. This test exercises that masking path directly - /// with `require_parameterization = false` so no other check interferes. + /// `%s` inside a quoted literal is masked before interpolation detection runs. #[test] fn percent_s_inside_string_literal_is_not_flagged_as_interpolation() { - // require_parameterization is OFF — the only active check is the - // interpolation path. The `%s` is inside a quoted value and must be - // invisible to the detector after masking. let issues = find_issues("SELECT * FROM t WHERE name LIKE '%s%'", &default_cfg()); assert_eq!(issues, Vec::::new()); } - /// When `require_parameterization` is enabled, a LIKE predicate that uses a - /// bind parameter (`?`) must pass cleanly — no interpolation markers and no - /// inline literals. #[test] fn parameterized_like_with_bind_param_is_not_flagged() { let mut cfg = default_cfg(); @@ -721,20 +637,16 @@ mod tests { } // ----------------------------------------------------------------------- - // Blocker 1 regressions — schema-qualified table names in UPDATE + // Schema-qualified UPDATE regressions // ----------------------------------------------------------------------- - /// WHERE-less UPDATE on a schema-qualified table must be blocked. - /// Regression for the fix that required SET after the table name: using only - /// `\w+` (no dot) caused `schema.table` to stop matching at `schema`, leaving - /// `.table SET` unaligned and the statement silently passing the WHERE-less guard. + /// Regression: `\w+` stopped at the dot in `schema.table`, bypassing the guard. #[test] fn schema_qualified_update_without_where_is_blocked() { let issues = find_issues("UPDATE hr.employees SET salary = 0", &default_cfg()); assert_eq!(issues, vec!["UPDATE without WHERE clause"]); } - /// A schema-qualified UPDATE with a WHERE clause must not be blocked. #[test] fn schema_qualified_update_with_where_is_not_blocked() { let issues = find_issues( @@ -744,7 +656,6 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// Three-part name (db.schema.table) without WHERE must be blocked. #[test] fn three_part_name_update_without_where_is_blocked() { let issues = find_issues("UPDATE mydb.hr.employees SET salary = 0", &default_cfg()); @@ -752,23 +663,17 @@ mod tests { } // ----------------------------------------------------------------------- - // Blocker 2 regressions — non-SQL fields must not trigger inline-literal check + // Non-SQL field regressions (fields = null + require_parameterization) // ----------------------------------------------------------------------- - /// A plain status message containing a number must not be flagged as having - /// inline literals when `require_parameterization` is enabled and `fields` is - /// null (every string field is scanned). The SQL-context gate prevents the - /// numeric literal detector from firing on non-SQL prose. #[test] fn non_sql_field_with_number_is_not_flagged() { let mut cfg = default_cfg(); cfg.require_parameterization = true; - // "status: 200 OK" has no SQL keyword — must pass silently. let issues = find_issues("status: 200 OK", &cfg); assert_eq!(issues, Vec::::new()); } - /// A tool-call ID string containing a number must not be flagged. #[test] fn tool_call_id_field_is_not_flagged() { let mut cfg = default_cfg(); @@ -777,7 +682,6 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// A version string containing decimal numbers must not be flagged. #[test] fn version_string_is_not_flagged() { let mut cfg = default_cfg(); @@ -786,7 +690,6 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// An error code message must not be flagged. #[test] fn error_code_message_is_not_flagged() { let mut cfg = default_cfg(); @@ -795,11 +698,8 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// `SELECT 1` is a legitimate SQL string (contains the SELECT keyword) and - /// will be flagged as an inline literal under `require_parameterization`. - /// This is expected behaviour — operators who need health-check queries to - /// pass should either add `SELECT` to an allowlist via field filtering or - /// disable `require_parameterization` for that field. + /// `SELECT 1` is flagged because it contains a SQL keyword and a literal. + /// Use `fields` filtering or disable `require_parameterization` for health checks. #[test] fn select_one_health_check_is_flagged_as_inline_literal() { let mut cfg = default_cfg(); From 791055894d29c6e8176fa3ad8c4e28eeb521170d Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 13:15:27 +0100 Subject: [PATCH 06/12] fix(sql-sanitizer): cover UPDATE aliases/ONLY/quoted-schema, SQLite ?N params, hex/exp literals, DQ identifiers, drop WITH from SQL gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 — UPDATE_RE now handles: - ONLY keyword: UPDATE ONLY t SET - AS alias / implicit alias: UPDATE t AS e SET, UPDATE t e SET - Quoted schema.table: UPDATE "hr"."employees" SET P2 — BIND_PARAM_DIGIT_RE: add ? so SQLite ?1/?2 params are erased. P2 — SQL_KEYWORD_RE: remove WITH; it fires on common English prose like "Request failed with status 503". P2 — INLINE_LITERAL_RE: extend pattern to cover hex (0xFF) and exponent (1e3, 1.5E-2) numeric literals. P2 — has_inline_literals: strip ANSI double-quoted identifiers (DQ_IDENT_RE) before literal matching so SELECT "2024" FROM t WHERE id = $1 is not incorrectly blocked. Regression tests added for all five cases. Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/issues.rs | 168 ++++++++++++++++-- 1 file changed, 156 insertions(+), 12 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 0c0389e..e4aebae 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -17,19 +17,32 @@ use crate::config::SqlSanitizerConfig; static PRINTF_FMT_RE: Lazy = Lazy::new(|| Regex::new(r"%[sdfi]").expect("Invalid printf format regex")); -/// Erases `$N`, `:N`, `@N` bind-parameter prefixes before literal detection. +/// Erases `$N`, `:N`, `@N`, `?N` bind-parameter prefixes before literal detection. +/// `?N` covers SQLite numbered parameters (`?1`, `?2`). static BIND_PARAM_DIGIT_RE: Lazy = - Lazy::new(|| Regex::new(r"[$:@]\d+").expect("Invalid bind param digit regex")); + Lazy::new(|| Regex::new(r"[$:@?]\d+").expect("Invalid bind param digit regex")); + +/// Matches a masked string literal (`''`) or a numeric literal in any of the +/// forms SQL dialects allow: integer, decimal, exponent (`1e3`), hex (`0xFF`). +/// Applied after `mask_string_literals`, `BIND_PARAM_DIGIT_RE` erasure, and +/// double-quoted identifier removal. +static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { + Regex::new( + r"''|0[xX][0-9A-Fa-f]+|-?\b\d+(?:[eE][+-]?\d+|(?:\.\d+)(?:[eE][+-]?\d+)?)\b|-?\b\d+\.\d+\b|-?\b\d+\b", + ) + .expect("Invalid inline literal regex") +}); -/// Matches a masked string literal (`''`) or bare numeric literal (`42`, `3.14`). -/// Applied after `mask_string_literals` and `BIND_PARAM_DIGIT_RE` erasure. -static INLINE_LITERAL_RE: Lazy = - Lazy::new(|| Regex::new(r"''|-?\b\d+(?:\.\d+)?\b").expect("Invalid inline literal regex")); +/// Strips ANSI double-quoted identifiers (e.g. `"2024"`) so their content is not +/// mistaken for a literal value during inline-literal detection. +static DQ_IDENT_RE: Lazy = + Lazy::new(|| Regex::new(r#""[^"]*""#).expect("Invalid double-quote ident regex")); /// Guards `has_inline_literals`: skips non-SQL fields (HTTP codes, IDs, log lines) /// to avoid false positives when `fields = null`. +/// `WITH` is intentionally omitted — it is too common in English prose. static SQL_KEYWORD_RE: Lazy = Lazy::new(|| { - Regex::new(r"(?i)\b(?:SELECT|INSERT|UPDATE|DELETE|MERGE|REPLACE|WITH)\b") + Regex::new(r"(?i)\b(?:SELECT|INSERT|UPDATE|DELETE|MERGE|REPLACE)\b") .expect("Invalid SQL keyword regex") }); @@ -45,9 +58,22 @@ static DELETE_FROM_RE: Lazy = Lazy::new(|| { static UPDATE_RE: Lazy = Lazy::new(|| { // SET is required so prose like "Append UPDATE query to file" is not matched. - // `(?:\w+\.)*\w+` covers bare, schema.table, and db.schema.table names. - Regex::new(r#"(?i)\bUPDATE\b\s+(?:(?:\w+\.)*\w+|"[^"]*"|`[^`]*`|\[[^\]]*\])\s+SET\b"#) - .expect("Invalid UPDATE regex") + // Table-name component: bare word or any quoted form (ANSI, backtick, bracket). + // Components are dot-separated, so "hr"."employees" and schema.table are covered. + // Optional ONLY (PostgreSQL) before the table name. + // Optional alias after: AS alias or implicit alias (bare word). + Regex::new( + r#"(?ix) + \bUPDATE\b \s+ + (?:ONLY\s+)? + (?: + (?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]) + (?:\.(?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]))* + ) + (?:\s+AS\s+\w+ | \s+\w+)? + \s+SET\b"#, + ) + .expect("Invalid UPDATE regex") }); static WHERE_RE: Lazy = @@ -226,13 +252,14 @@ fn has_brace_template(sql: &str) -> bool { false } -/// Return `true` when `sql` contains a masked string literal or bare numeric literal. -/// Skips strings with no SQL keyword; erases `$N`/`:N`/`@N` before matching. +/// Return `true` when `sql` contains a masked string literal or numeric literal. +/// Skips non-SQL strings; strips bind params and double-quoted identifiers before matching. fn has_inline_literals(sql: &str) -> bool { if !SQL_KEYWORD_RE.is_match(sql) { return false; } let erased = BIND_PARAM_DIGIT_RE.replace_all(sql, ""); + let erased = DQ_IDENT_RE.replace_all(&erased, ""); INLINE_LITERAL_RE.is_match(&erased) } @@ -710,4 +737,121 @@ mod tests { vec!["Inline literal values detected; use bind parameters instead"] ); } + + // ----------------------------------------------------------------------- + // P1: UPDATE alias / ONLY / quoted schema regressions + // ----------------------------------------------------------------------- + + #[test] + fn update_with_alias_without_where_is_blocked() { + let issues = find_issues("UPDATE employees AS e SET salary = 0", &default_cfg()); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + #[test] + fn update_with_alias_with_where_is_not_blocked() { + let issues = find_issues( + "UPDATE employees AS e SET salary = 0 WHERE id = 1", + &default_cfg(), + ); + assert_eq!(issues, Vec::::new()); + } + + #[test] + fn update_only_without_where_is_blocked() { + let issues = find_issues("UPDATE ONLY employees SET salary = 0", &default_cfg()); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + #[test] + fn update_quoted_schema_table_without_where_is_blocked() { + let issues = find_issues( + r#"UPDATE "hr"."employees" SET salary = 0"#, + &default_cfg(), + ); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); + } + + #[test] + fn update_quoted_schema_table_with_where_is_not_blocked() { + let issues = find_issues( + r#"UPDATE "hr"."employees" SET salary = 0 WHERE id = 1"#, + &default_cfg(), + ); + assert_eq!(issues, Vec::::new()); + } + + // ----------------------------------------------------------------------- + // P2: WITH keyword removed from SQL context gate + // ----------------------------------------------------------------------- + + #[test] + fn prose_with_word_and_number_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + // "with" in English prose must not activate literal detection. + let issues = find_issues("Request failed with status 503", &cfg); + assert_eq!(issues, Vec::::new()); + } + + // ----------------------------------------------------------------------- + // P2: SQLite ?N numbered bind parameters + // ----------------------------------------------------------------------- + + #[test] + fn sqlite_numbered_bind_param_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT id FROM users WHERE id = ?1", &cfg); + assert_eq!(issues, Vec::::new()); + } + + // ----------------------------------------------------------------------- + // P2: double-quoted column identifier not treated as literal + // ----------------------------------------------------------------------- + + #[test] + fn double_quoted_column_identifier_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues( + r#"SELECT "2024" FROM annual_report WHERE id = $1"#, + &cfg, + ); + assert_eq!(issues, Vec::::new()); + } + + // ----------------------------------------------------------------------- + // P2: exponent and hexadecimal numeric literals + // ----------------------------------------------------------------------- + + #[test] + fn insert_with_exponent_literal_is_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("INSERT INTO readings VALUES (1e3)", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + #[test] + fn insert_with_hex_literal_is_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("INSERT INTO readings VALUES (0xFF)", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + #[test] + fn insert_with_bind_params_passes_after_hex_exponent_fix() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("INSERT INTO readings VALUES (?)", &cfg); + assert_eq!(issues, Vec::::new()); + } } From cbe145db55dd5e4f39ac6416a8aa67c65895ffef Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 13:25:53 +0100 Subject: [PATCH 07/12] style(sql-sanitizer): collapse multi-line find_issues calls for rustfmt Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/issues.rs | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index e4aebae..39b70e5 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -750,10 +750,8 @@ mod tests { #[test] fn update_with_alias_with_where_is_not_blocked() { - let issues = find_issues( - "UPDATE employees AS e SET salary = 0 WHERE id = 1", - &default_cfg(), - ); + let issues = + find_issues("UPDATE employees AS e SET salary = 0 WHERE id = 1", &default_cfg()); assert_eq!(issues, Vec::::new()); } @@ -765,19 +763,14 @@ mod tests { #[test] fn update_quoted_schema_table_without_where_is_blocked() { - let issues = find_issues( - r#"UPDATE "hr"."employees" SET salary = 0"#, - &default_cfg(), - ); + let issues = find_issues(r#"UPDATE "hr"."employees" SET salary = 0"#, &default_cfg()); assert_eq!(issues, vec!["UPDATE without WHERE clause"]); } #[test] fn update_quoted_schema_table_with_where_is_not_blocked() { - let issues = find_issues( - r#"UPDATE "hr"."employees" SET salary = 0 WHERE id = 1"#, - &default_cfg(), - ); + let issues = + find_issues(r#"UPDATE "hr"."employees" SET salary = 0 WHERE id = 1"#, &default_cfg()); assert_eq!(issues, Vec::::new()); } @@ -814,10 +807,7 @@ mod tests { fn double_quoted_column_identifier_is_not_flagged() { let mut cfg = default_cfg(); cfg.require_parameterization = true; - let issues = find_issues( - r#"SELECT "2024" FROM annual_report WHERE id = $1"#, - &cfg, - ); + let issues = find_issues(r#"SELECT "2024" FROM annual_report WHERE id = $1"#, &cfg); assert_eq!(issues, Vec::::new()); } From 0e8b7ca10aec7b36051006271adad6afb0a8ad46 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 13:32:18 +0100 Subject: [PATCH 08/12] style(sql-sanitizer): expand two test call sites to match rustfmt Signed-off-by: prakhar-singh1928 --- .../rust/python-package/sql_sanitizer/src/issues.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 39b70e5..55ed86f 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -750,8 +750,10 @@ mod tests { #[test] fn update_with_alias_with_where_is_not_blocked() { - let issues = - find_issues("UPDATE employees AS e SET salary = 0 WHERE id = 1", &default_cfg()); + let issues = find_issues( + "UPDATE employees AS e SET salary = 0 WHERE id = 1", + &default_cfg(), + ); assert_eq!(issues, Vec::::new()); } @@ -769,8 +771,10 @@ mod tests { #[test] fn update_quoted_schema_table_with_where_is_not_blocked() { - let issues = - find_issues(r#"UPDATE "hr"."employees" SET salary = 0 WHERE id = 1"#, &default_cfg()); + let issues = find_issues( + r#"UPDATE "hr"."employees" SET salary = 0 WHERE id = 1"#, + &default_cfg(), + ); assert_eq!(issues, Vec::::new()); } From 4f3d2fb60ca9b61f22d2512a24c88e961df0455d Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 14:03:47 +0100 Subject: [PATCH 09/12] fix(sql-sanitizer): add word boundary to hex literal pattern to prevent mid-identifier match Signed-off-by: prakhar-singh1928 --- .../rust/python-package/sql_sanitizer/src/issues.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 55ed86f..0c1ca0d 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -28,7 +28,7 @@ static BIND_PARAM_DIGIT_RE: Lazy = /// double-quoted identifier removal. static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { Regex::new( - r"''|0[xX][0-9A-Fa-f]+|-?\b\d+(?:[eE][+-]?\d+|(?:\.\d+)(?:[eE][+-]?\d+)?)\b|-?\b\d+\.\d+\b|-?\b\d+\b", + r"''|\b0[xX][0-9A-Fa-f]+\b|-?\b\d+(?:[eE][+-]?\d+|(?:\.\d+)(?:[eE][+-]?\d+)?)\b|-?\b\d+\.\d+\b|-?\b\d+\b", ) .expect("Invalid inline literal regex") }); @@ -848,4 +848,14 @@ mod tests { let issues = find_issues("INSERT INTO readings VALUES (?)", &cfg); assert_eq!(issues, Vec::::new()); } + + #[test] + fn hex_prefix_inside_identifier_not_flagged() { + // `hash0xFF` is a column name, not a hex literal; the word boundary on + // INLINE_LITERAL_RE must prevent a mid-identifier match. + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT hash0xFF FROM checksums WHERE id = $1", &cfg); + assert_eq!(issues, Vec::::new()); + } } From 11e0f2fb088784c46f9a489d1f47dceed3f03784 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 14:32:27 +0100 Subject: [PATCH 10/12] fix(sql-sanitizer): accept quoted UPDATE aliases; require paired SQL keywords for literal gate Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/issues.rs | 62 ++++++++++++++++--- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 0c1ca0d..65df8e1 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -38,12 +38,23 @@ static INLINE_LITERAL_RE: Lazy = Lazy::new(|| { static DQ_IDENT_RE: Lazy = Lazy::new(|| Regex::new(r#""[^"]*""#).expect("Invalid double-quote ident regex")); -/// Guards `has_inline_literals`: skips non-SQL fields (HTTP codes, IDs, log lines) -/// to avoid false positives when `fields = null`. -/// `WITH` is intentionally omitted — it is too common in English prose. -static SQL_KEYWORD_RE: Lazy = Lazy::new(|| { - Regex::new(r"(?i)\b(?:SELECT|INSERT|UPDATE|DELETE|MERGE|REPLACE)\b") - .expect("Invalid SQL keyword regex") +/// Guards `has_inline_literals`: requires a recognisable SQL statement structure +/// (paired keyword) to avoid false positives on prose fields when `fields = null`. +/// A single keyword is not enough — "Please select option 2" contains SELECT but +/// is not a SQL statement. +static SQL_CONTEXT_RE: Lazy = Lazy::new(|| { + Regex::new( + r"(?ix) + (?: + \bSELECT\b .{0,500}? \b(?:FROM|WHERE|HAVING|GROUP\s+BY|ORDER\s+BY|UNION|LIMIT|JOIN)\b + | \bINSERT\b \s+ INTO\b + | \bUPDATE\b \s+ \S+ .{0,200}? \bSET\b + | \bDELETE\b \s+ FROM\b + | \bMERGE\b \s+ INTO\b + | \bREPLACE\b \s+ INTO\b + )", + ) + .expect("Invalid SQL context regex") }); static DELETE_FROM_RE: Lazy = Lazy::new(|| { @@ -61,7 +72,7 @@ static UPDATE_RE: Lazy = Lazy::new(|| { // Table-name component: bare word or any quoted form (ANSI, backtick, bracket). // Components are dot-separated, so "hr"."employees" and schema.table are covered. // Optional ONLY (PostgreSQL) before the table name. - // Optional alias after: AS alias or implicit alias (bare word). + // Alias: AS or implicit alias — each in any quoted form (bare, ANSI, backtick, bracket). Regex::new( r#"(?ix) \bUPDATE\b \s+ @@ -70,7 +81,10 @@ static UPDATE_RE: Lazy = Lazy::new(|| { (?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]) (?:\.(?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]))* ) - (?:\s+AS\s+\w+ | \s+\w+)? + (?: + \s+AS\s+(?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]) + | \s+(?:\w+ | "[^"]*" | `[^`]*` | \[[^\]]*\]) + )? \s+SET\b"#, ) .expect("Invalid UPDATE regex") @@ -255,7 +269,7 @@ fn has_brace_template(sql: &str) -> bool { /// Return `true` when `sql` contains a masked string literal or numeric literal. /// Skips non-SQL strings; strips bind params and double-quoted identifiers before matching. fn has_inline_literals(sql: &str) -> bool { - if !SQL_KEYWORD_RE.is_match(sql) { + if !SQL_CONTEXT_RE.is_match(sql) { return false; } let erased = BIND_PARAM_DIGIT_RE.replace_all(sql, ""); @@ -858,4 +872,34 @@ mod tests { let issues = find_issues("SELECT hash0xFF FROM checksums WHERE id = $1", &cfg); assert_eq!(issues, Vec::::new()); } + + #[test] + fn prose_select_with_digit_does_not_flag_parameterization() { + // "Please select option 2" contains SELECT but is not a SQL statement; + // SQL_CONTEXT_RE requires a paired structural keyword (FROM/WHERE/…). + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("Please select option 2", &cfg); + assert_eq!(issues, Vec::::new()); + } + + #[test] + fn update_with_quoted_alias_no_where_is_blocked() { + // UPDATE … AS "e" SET — quoted alias form must be recognised; no WHERE → block. + let mut cfg = default_cfg(); + cfg.block_update_without_where = true; + let issues = find_issues(r#"UPDATE employees AS "e" SET salary = 0"#, &cfg); + assert_eq!(issues, vec!["UPDATE statement is missing a WHERE clause"]); + } + + #[test] + fn update_with_quoted_alias_and_where_is_allowed() { + let mut cfg = default_cfg(); + cfg.block_update_without_where = true; + let issues = find_issues( + r#"UPDATE employees AS "e" SET salary = 0 WHERE id = 1"#, + &cfg, + ); + assert_eq!(issues, Vec::::new()); + } } From 0c4e7f881d2a3cf9cf3ff4b06c8946ca6e19e115 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 14:36:44 +0100 Subject: [PATCH 11/12] fix(sql-sanitizer): fix test message mismatch and SELECT 1 gate expectation Signed-off-by: prakhar-singh1928 --- .../python-package/sql_sanitizer/src/issues.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index 65df8e1..c3732df 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -739,17 +739,15 @@ mod tests { assert_eq!(issues, Vec::::new()); } - /// `SELECT 1` is flagged because it contains a SQL keyword and a literal. - /// Use `fields` filtering or disable `require_parameterization` for health checks. + /// `SELECT 1` has no paired clause keyword (FROM/WHERE/…) so `SQL_CONTEXT_RE` + /// does not gate it — it is not treated as SQL and passes clean. + /// Use `fields` to target only SQL-bearing fields for health-check queries. #[test] - fn select_one_health_check_is_flagged_as_inline_literal() { + fn select_one_health_check_is_not_flagged() { let mut cfg = default_cfg(); cfg.require_parameterization = true; let issues = find_issues("SELECT 1", &cfg); - assert_eq!( - issues, - vec!["Inline literal values detected; use bind parameters instead"] - ); + assert_eq!(issues, Vec::::new()); } // ----------------------------------------------------------------------- @@ -886,10 +884,8 @@ mod tests { #[test] fn update_with_quoted_alias_no_where_is_blocked() { // UPDATE … AS "e" SET — quoted alias form must be recognised; no WHERE → block. - let mut cfg = default_cfg(); - cfg.block_update_without_where = true; - let issues = find_issues(r#"UPDATE employees AS "e" SET salary = 0"#, &cfg); - assert_eq!(issues, vec!["UPDATE statement is missing a WHERE clause"]); + let issues = find_issues(r#"UPDATE employees AS "e" SET salary = 0"#, &default_cfg()); + assert_eq!(issues, vec!["UPDATE without WHERE clause"]); } #[test] From 9e93dca68f9a744351a2ce58fd2c7947d0c011aa Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 2 Oct 2026 15:19:46 +0100 Subject: [PATCH 12/12] fix(sql-sanitizer): enable dot-all in SQL_CONTEXT_RE so multiline SQL is gated correctly Signed-off-by: prakhar-singh1928 --- .../sql_sanitizer/src/issues.rs | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/plugins/rust/python-package/sql_sanitizer/src/issues.rs b/plugins/rust/python-package/sql_sanitizer/src/issues.rs index c3732df..ff9c3f5 100644 --- a/plugins/rust/python-package/sql_sanitizer/src/issues.rs +++ b/plugins/rust/python-package/sql_sanitizer/src/issues.rs @@ -42,9 +42,10 @@ static DQ_IDENT_RE: Lazy = /// (paired keyword) to avoid false positives on prose fields when `fields = null`. /// A single keyword is not enough — "Please select option 2" contains SELECT but /// is not a SQL statement. +/// `(?s)` (dot-all) lets `.` cross newlines so multi-line SQL is gated correctly. static SQL_CONTEXT_RE: Lazy = Lazy::new(|| { Regex::new( - r"(?ix) + r"(?ixs) (?: \bSELECT\b .{0,500}? \b(?:FROM|WHERE|HAVING|GROUP\s+BY|ORDER\s+BY|UNION|LIMIT|JOIN)\b | \bINSERT\b \s+ INTO\b @@ -898,4 +899,47 @@ mod tests { ); assert_eq!(issues, Vec::::new()); } + + // ----------------------------------------------------------------------- + // Multiline SQL regressions (dot-all in SQL_CONTEXT_RE) + // ----------------------------------------------------------------------- + + #[test] + fn multiline_select_with_inline_literal_is_flagged() { + // `.` in SQL_CONTEXT_RE must cross newlines; without (?s) this passes silently. + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT id\nFROM users\nWHERE id = 42", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + #[test] + fn multiline_select_with_bind_param_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("SELECT id\nFROM users\nWHERE id = $1", &cfg); + assert_eq!(issues, Vec::::new()); + } + + #[test] + fn multiline_update_with_inline_literal_is_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("UPDATE users\nSET salary = 0\nWHERE id = 42", &cfg); + assert_eq!( + issues, + vec!["Inline literal values detected; use bind parameters instead"] + ); + } + + #[test] + fn multiline_update_with_bind_param_is_not_flagged() { + let mut cfg = default_cfg(); + cfg.require_parameterization = true; + let issues = find_issues("UPDATE users\nSET salary = $1\nWHERE id = $2", &cfg); + assert_eq!(issues, Vec::::new()); + } }