Repository navigation
Conversation
…terals - 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 <prakhar.singh1928@ibm.com>
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 <prakhar.singh1928@ibm.com>
…eld over-flagging
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 <prakhar.singh1928@ibm.com>
…ne for rustfmt Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
lucarlig
left a comment
lucarlig
left a comment
There was a problem hiding this comment.
Please fix the UPDATE guard bypass and parameterization issues below. Reproduced all five; 94 Rust tests, 17 Python integration tests, formatting, and Clippy pass.
…N params, hex/exp literals, DQ identifiers, drop WITH from SQL gate 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 <prakhar.singh1928@ibm.com>
Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
prakhar-singh1928
left a comment
prakhar-singh1928
left a comment
There was a problem hiding this comment.
All five review findings are addressed in this branch. Summary of what was done:
[P1] UPDATE alias/ONLY/quoted-schema bypass
UPDATE_RE now uses (?:"?\w+"?\.)*"?\w+"? for table names and handles AS <alias>, ONLY, and the full alias-before-SET pattern. New test: update_with_alias_no_where_is_blocked.
[P2a] WITH as SQL context gate
Removed WITH from SQL_KEYWORD_RE — it appears in too many prose strings ("failed with", "along with") to reliably establish SQL context. New test: with_in_prose_does_not_gate_literal_check.
[P2b] SQLite ?N bind parameters
BIND_PARAM_DIGIT_RE now includes the ? prefix ((?:$|:|\@|\?)(\d+)) so ?1/?2 are erased before literal scanning. New test: sqlite_numbered_bind_param_not_flagged.
[P2c] Double-quoted column identifiers
Added DQ_IDENT_RE (applied after bind-param erasure, before literal matching) that strips "<identifier>" tokens. New test: double_quoted_identifier_not_flagged_as_literal.
[P2d] Hex and exponent numeric literals
INLINE_LITERAL_RE now covers 0[xX][0-9a-fA-F]+ and \d+[eE][+-]?\d+ in addition to plain integers and decimals. New tests: hex_literal_is_flagged and exponent_literal_is_flagged.
CI is fully green (17/17 checks pass, publish skipped as expected). All original WO reproduction cases also pass.
lucarlig
left a comment
lucarlig
left a comment
There was a problem hiding this comment.
Three original findings are fixed. Approval remains blocked by quoted UPDATE aliases and prose false positives (see existing threads), plus the new hex-pattern regression below. Validation passes: 105 Rust tests, 17 Python integration tests, formatting, and Clippy.
…nt mid-identifier match Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
…keywords for literal gate Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
…tation Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
lucarlig
left a comment
lucarlig
left a comment
There was a problem hiding this comment.
All previous findings are fixed and their threads resolved. One new regression remains: formatted SQL bypasses inline-literal detection. Validation passes: 109 Rust tests, 17 Python integration tests, formatting, and Clippy.
… is gated correctly Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
lucarlig
left a comment
lucarlig
left a comment
There was a problem hiding this comment.
All review findings are fixed, including multiline SQL. Verified 15 reproduction cases; 113 Rust tests, 17 Python integration tests, formatting, and Clippy pass.
2 participants
Summary
Two bugs in the SQL sanitizer plugin where
block_update_without_wherefalsely blocked a valid request, andrequire_parameterizationmissed inline literal values.Changes
Bug 1 — False block on valid UPDATE with WHERE
UPDATE_REpreviously matchedUPDATE <word>with no further requirement. Withfields = null(the default), the scanner visits every string argument including non-SQL fields. A message field containing prose like"Append UPDATE query to TC1.SQL"matched the regex —queryis a valid identifier — and fired the WHERE-less UPDATE policy, blocking the request even though the SQL in a separate field had a proper WHERE clause.Fix:
UPDATE_REnow requires the SQL keywordSETafter the table name:Prose uses UPDATE as a verb and never has
SET; real DML always does.Bug 2 — Missed parameterization for inline literals
require_parameterizationonly detected signs of Python-level string assembly (+,%s,{}). A final SQL string with hard-coded values —INSERT INTO employees VALUES (42, 'Alice', 75000)— contains no interpolation markers and passed silently despite not using bind parameters.Fix: A second detection path
has_inline_literalsnow runs when no interpolation markers are found. It detects masked string literals ('') and word-boundary-anchored numeric literals. Bind-parameter digit sequences ($1,:1,@1) are erased before matching so PostgreSQL/Oracle/SQL Server positional params are not false-positived. SQL using?,$1, or:namepasses cleanly.Test coverage
require_parameterizationrequire_parameterization$1positional bind parameter not treated as a numeric literalVersion
0.1.3→0.1.4