refactor(appsec): move appsec DBAPI SQLi handling to typed events - #19747
Conversation
Codeowners resolved asResolved from the full PR diff against No remaining files require a CODEOWNERS review. |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 00ce070 | Docs | View more details | Give us feedback! |
Circular import analysis
|
Dependency direction analysis
|
BenchmarksBenchmark execution time: 2026-08-20 09:47:10 Comparing candidate commit 00ce070 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 82 metrics, 0 unstable metrics.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78d84067b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
christophe-papazian
left a comment
There was a problem hiding this comment.
Reviewed the refactor closely and I'm satisfied it's behavior-preserving: the dropped in_asm_context() guard and report_rasp_skipped(SQLI, False) were unreachable in main (get_rasp_capability() already checks the context), blocking still propagates because BlockingException derives from BaseException and native should_propagate() re-raises those regardless of allow_raise=False, and the dialect table change is a no-op — I checked libddwaf at the pinned 2.0.1: sql_dialect_from_type() maps everything outside mysql/postgresql/sqlite/oracle/doctrine/hsqldb to generic, and sqli_detector.cpp only has distinct tokenizers for pgsql/mysql/sqlite, so mariadb, odbc, vertica and sql were already generic. Async scope and the bytes exclusion are preserved too.
Three comments inline: one hot-path gating regression, one small detection improvement, one test gap.
emmettbutler
left a comment
There was a problem hiding this comment.
great code organization improvement
6c0aeca to
00ce070
Compare
Description
Move synchronous DBAPI SQL injection RASP handling from the legacy tuple-based common-module hook to the typed contrib event/subscriber API.
DbApiEventfrom the generic DBAPI cursor only when AppSec has a listener.Testing
Risks
Low. This changes the internal event boundary and listener ownership without expanding supported query types or adapters. Tests cover listener gating, blocking propagation, disabled tracing, prefix/dialect normalization, and the existing common-module lifecycle.
Additional Notes
No release note: this is an internal behavior-preserving refactor.