Conversation
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Clean fix for SQL LIKE wildcard injection in alert-rule search. The escapeLike helper correctly escapes \, %, and _ in the right order (backslash first to avoid double-escaping), and the unit tests verify both positive (escaped chars present in query) and negative (unescaped values absent) cases.
Findings
- [Info]
MybatisPlusAlertRepository.java:371— TheescapeLikemethod is duplicated across multiple repositories in this codebase (see alsoQueryHistoryService.escapeLike,MybatisPlusAclRepository.escapeLike,AuthService.escapeLike). Consider extracting to a shared utility class (e.g.SqlLikeEscape) to reduce drift risk.
LGTM — good fix with solid test coverage.
lizhimins
left a comment
There was a problem hiding this comment.
Same single item as on #4224 and #4230: replace the private escapeLike copy with the shared utility in common/util, and add an explicit ESCAPE clause rather than relying on MySQL's implicit backslash (H2, used by five tests in this suite, has no default LIKE escape character). Please keep the tests.
Use the shared SqlLikeUtil helper with an explicit ESCAPE clause so underscore-bearing rule names and metrics match literally on both MySQL and H2. Fixes apache#4332 Signed-off-by: halaxy <63827956+89799969@users.noreply.github.com>
e465e0c to
6f89b25
Compare
|
@lizhimins Following up - shared |
RockteMQ-AI
left a comment
There was a problem hiding this comment.
LGTM. Code changes look good.
Automated review by github-manager-bot
What is the purpose of the change
Fixes #4232.
Alert-rule inventory search treats
%and_as SQL LIKE wildcards instead of literal characters:GET /api/alert-rules/page?search=/GET /api/business-alert-rules/page?search=filternamewith unescapedLIKE(MybatisPlusAlertRepository.ruleQuery).AlertRuleQuery.search) filtersnameandmetricthe same way (MybatisPlusAlertRepository.findRulesPage).Alert-rule names and metric identifiers commonly contain underscores (
consumer_lag,disk_usage). Searching forconsumer_lagtherefore also matchesconsumerXlag, and a trailing%matches any suffix — so the inventory silently returns unrelated rules or hides the intended one.Brief changelog
escapeLikehelper (same behaviour as the existingQueryHistoryService.escapeLike) and apply it to the legacy name-only page search and the domain page name/metric search.prod_lag%/consumer_lag%is escaped (%prod\_lag\%%) rather than treated as a wildcard pattern.Verifying this change
Checkstyle: 0 violations (module validate phase).
Before the fix the two new tests fail because the bound parameters are
%prod_lag%/%consumer_lag%; after it they bind%prod\_lag\%%/%consumer\_lag\%%.Related peers: #4229/#4230 (ACL inventory), #4223/#4224 (Studio user search), #4192–#4194 (audit / instance / cloud-credential LIKE escape).