feat(sql_sanitizer): add Rust-backed SQL sanitizer plugin - #133
Conversation
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
|
@madhu-mohan-jaishankar make sure you have a https://pypi.org/ package setup to upload to should be cpex-sql-sanitizer, and set as mantainer you and @jonpspri |
There was a problem hiding this comment.
Pull request overview
Adds a new standalone plugin package, cpex-sql-sanitizer, to the CPEX plugins monorepo. The plugin is implemented as a Rust core (PyO3) with a thin Python Plugin shim, intended to detect risky SQL patterns in prompt_pre_fetch and tool_pre_invoke hook arguments and optionally block or report them.
Changes:
- Introduces Rust implementation for SQL scanning (per-statement checks, comment stripping, blocked patterns, unsafe DML detection, interpolation heuristic).
- Adds Python shim + plugin manifest + packaging/build tooling (maturin, Makefile, entry point).
- Adds integration tests for gateway-facing behavior (blocking, monitoring mode metadata, field filtering, modified payload on stripping).
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/tests/sql_sanitizer/test_integration.py | Adds Python integration tests asserting expected hook behavior and result shapes. |
| plugins/rust/python-package/sql_sanitizer/src/scanner.rs | Implements recursive arg scanning, field filtering, and stripped-value accumulation. |
| plugins/rust/python-package/sql_sanitizer/src/plugin.rs | Implements Rust-owned hook handling and builds framework result/violation objects. |
| plugins/rust/python-package/sql_sanitizer/src/lib.rs | Defines the PyO3 module and registers the core class. |
| plugins/rust/python-package/sql_sanitizer/src/issues.rs | Implements SQL issue detection (blocked patterns, DML-without-WHERE, interpolation heuristic). |
| plugins/rust/python-package/sql_sanitizer/src/config.rs | Parses Python config into compiled Rust configuration (incl. regex compilation). |
| plugins/rust/python-package/sql_sanitizer/src/comments.rs | Implements SQL comment stripping utilities and unit tests. |
| plugins/rust/python-package/sql_sanitizer/src/bin/stub_gen.rs | Adds optional stub generation binary for Python typing stubs. |
| plugins/rust/python-package/sql_sanitizer/pyproject.toml | Declares Python package metadata, deps, and maturin module configuration. |
| plugins/rust/python-package/sql_sanitizer/Makefile | Adds standard plugin dev/test/build targets for this package. |
| plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/sql_sanitizer.py | Adds the Python Plugin shim delegating to the Rust core. |
| plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/plugin-manifest.yaml | Declares plugin metadata, hooks, and default config. |
| plugins/rust/python-package/sql_sanitizer/cpex_sql_sanitizer/init.py | Adds lazy exports for the shim and Rust core class. |
| plugins/rust/python-package/sql_sanitizer/Cargo.toml | Defines the Rust crate, dependencies, features, and benches. |
| plugins/rust/python-package/sql_sanitizer/benches/sql_sanitizer.rs | Adds Criterion benchmarks for hot paths. |
| Cargo.toml | Registers sql_sanitizer as a workspace member. |
| Cargo.lock | Adds the new crate to the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
…mutation tests Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
lucarlig
left a comment
There was a problem hiding this comment.
Requesting changes based on packaged-wheel, framework-isolation, SQLite, and build testing:
-
CPEX isolation fails open.
scan_argsusesPyDict::iter, butCopyOnWriteDictkeeps visible values outside the C-level dict. A packagedDROP TABLE userscall returnedcontinue_processing=True. Use the mapping protocol and add awrap_payload_for_isolationregression test. -
Valid destructive SQL bypasses detection. Quoted-target updates,
UPDATE ... SET note='WHERE', andDELETE ... RETURNING 'WHERE'were allowed and changed/deleted every SQLite row. The scanner must distinguish identifiers, literals, and actual clauses. It also blocks safe semicolons/keywords inside literals. -
Comment stripping corrupts payloads. Comment markers inside quoted literals are removed, while nested replacements overwrite/inject top-level keys and nested lists are skipped. Stripping and rebuilding need to be syntax-aware and path-preserving.
-
Advertised Rust targets fail.
make bench-no-runimports the wrong crate name, and all-featurestub_gencompilation fails against the workspace'spyo3-stub-gen 0.23API. -
Malformed config fails open.
blocked_statements=[123]silently becomes an empty blocked-pattern set and allowsDROP TABLE.
Please address these before merge.
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
Thanks for the thorough review, @lucarlig! All points addressed in the latest commit ( Issue 1 — CopyOnWriteDict / CPEX isolation: Issue 2 — WHERE/keywords inside string literals: Issue 3a — Comment stripping corrupts literals: Issue 3b — List-item stripping limitation: Issue 4 — Bench crate name: Issue 5 — Malformed config fails open: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
plugins/rust/python-package/sql_sanitizer/Makefile:128
- The
citarget currently runsinstall(editablematurin develop) and skipsinstall-wheel/stub verification. The CI workflow runsmake cion non-Linux runners, and other plugin Makefiles avoid callinginstallinsidecito ensure wheel-based verification and consistent behavior across OSes. Consider aligningci/ci-buildwith the standard pattern (ci-build: check-all verify-stubs build … install-wheel,ci: ci-build test-integration).
ci-build: fmt-check clippy test-unit build
ci: fmt-check clippy test-unit install test-integration
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
lucarlig
left a comment
There was a problem hiding this comment.
Requesting changes for four remaining issues:
-
Quoted-table UPDATE bypass
src/issues.rs:24only matches a\w+target.UPDATE "users" SET admin=1is allowed; executing it in SQLite changed every row. -
Nested comment stripping overwrites the wrong field
src/scanner.rsrecords only(key, value), thensrc/plugin.rsapplies it at the top level. Stripping nestedwrapper.sqloverwrites an unrelated top-levelsqlwhile leaving the nested value unchanged. -
Nested lists bypass scanning
src/scanner.rsdoes not recurse into lists within lists.{"batch": [["DROP TABLE users"]]}is allowed without a violation. -
Stub-generator build is broken
cargo check -p sql_sanitizer --all-features --lib --binsfails becausesrc/bin/stub_gen.rsuses the oldpyo3-stub-genAPI (GenerateOptionsandgenerate(&options)).
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
|
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
lucarlig
left a comment
There was a problem hiding this comment.
Two merge blockers remain:
-
MySQL destructive DELETE bypass. Both
DELETE u FROM users AS uandDELETE FROM users # WHERE id=1returncontinue_processing=True. The first is valid multi-table DELETE syntax; the second hides the apparentWHEREinside a MySQL#comment. Both can delete every row. -
Release preflight will fail. No
.pyifiles are tracked for this plugin, whilerelease-rust-python-package.yamlrequires at least one in a fresh checkout. Please generate and commit the package stubs.
Signed-off-by: Madhu Mohan Jaishankar <madhu.mohan.jaishankar@ibm.com>
🔗 Related Issue
Closes #130
📝 Summary
Adds
cpex-sql-sanitizer, a Rust-backed SQL sanitizer plugin for the MCP Gateway.The plugin intercepts
prompt_pre_fetchandtool_pre_invokehooks and scans SQL argument values for security issues before they reach the backend. All detection logic is implemented in Rust via PyO3 and exposed to the gateway through a thin Python shim (SQLSanitizerPlugin).Key capabilities:
;and each statement is checked independently. This closes a correctness gap where aWHEREclause in one statement could mask aWHERE-lessDELETE/UPDATEin another statement in the same payload.DROP,TRUNCATE,ALTER,GRANT,REVOKEblocked by default via configurable regex patterns.DELETE FROMandUPDATEwithout aWHEREclause are flagged.--line comments and/* */block comments are stripped before analysis so patterns hidden in comments are not matched.fieldsis set, only the named argument keys are scanned; all string values are scanned whenfieldsisnull.block_on_violation=false, violations are recorded inresult.metadata.sql_issuesand the request is passed through, enabling audit-only deployments.+,%., and{…}patterns that indicate non-parameterized SQL construction.📏 Reviewability
triage🏷️ Type of Change
🧪 Verification
cargo fmt --checkcargo clippy -- -D warningscargo test --libmake test-integrationmake install(maturin wheel build)Unit test run output:
✅ Checklist
cargo fmt)📓 Notes