Skip to content

fix(audit): treat LIKE wildcards in the log search as literal text - #4290

Closed
btlqql wants to merge 1 commit into
apache:rocketmq-studiofrom
btlqql:codex/btl-audit-search-wildcards
Closed

btlqql wants to merge 1 commit into
apache:rocketmq-studiofrom
btlqql:codex/btl-audit-search-wildcards

Conversation

@btlqql

@btlqql btlqql commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What is the purpose of the change

The audit log search value reaches the repository unmodified and is used as a LIKE pattern over
operator, resource_name and detail. MyBatis-Plus wraps it as %<search>%, so the caller's %
and _ keep their SQL wildcard meaning:

  • search=% matches every row (LIKE '%%');
  • search=_ matches every row whose column is a non-empty string;
  • search=100%_done matches any row that merely contains 100 followed by four arbitrary
    characters, instead of the rows that contain the literal text.

The same value drives the insight aggregates and the CSV export — AuditService.exportLogs reuses
findPage — so the list, the summary and the exported file are all wrong in the same way, and a
full-table match can push the export over its 10 000-record cap and fail it with a 400 where the
filtered subset would have fitted.

The fix escapes the search once per query builder, with the backslash convention the repository
already uses for the message query history search (QueryHistoryService:353) — MySQL treats a
backslash as the default LIKE escape character, so no ESCAPE clause is needed and a literal
backslash in the search is matched as a literal backslash too.

Brief changelog

  • MybatisPlusAuditRepository: added escapeLike and applied it in both places that build the
    search predicate — findPage and the shared applyFilters used by summarize/exportLogs.
  • MybatisPlusAuditRepositoryTest: two new cases — findPage with 100%_done and summarize with
    a_b%; both assert the bound LIKE parameters are %100\%\_done% / %a\_b\%%, on every query the
    summary issues (four selectMaps aggregates plus the selectList lookup).

Verification

Red first, on the unmodified implementation with only the new tests added:

mvn -f server/pom.xml -B -Dtest=MybatisPlusAuditRepositoryTest -DfailIfNoTests=false test
[ERROR] Tests run: 11, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.902 s <<< FAILURE! -- in org.apache.rocketmq.studio.ops.audit.MybatisPlusAuditRepositoryTest
java.lang.AssertionError:
Expecting ArrayList:
  ["%100%_done%", "%100%_done%", "%100%_done%"]
to contain only:
  ["%100\%\_done%"]
element(s) not found:
  ["%100\%\_done%"]
and element(s) not expected:
  ["%100%_done%", "%100%_done%", "%100%_done%"]
[ERROR] Tests run: 11, Failures: 2, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE

Green after the change:

[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.876 s -- in org.apache.rocketmq.studio.ops.audit.MybatisPlusAuditRepositoryTest
[INFO] BUILD SUCCESS

Whole backend suite (mvn -f server/pom.xml -B test): Tests run: 2153, Failures: 5, Errors: 5, Skipped: 0 — the same ten ids that fail on the unmodified base 6c24d2ed
(Tests run: 2151, Failures: 5, Errors: 5): eight need a POSIX sh CLI this Windows machine does
not have (CliAgentProviderTest, ClaudeCodeAgentProviderTest) and two are the
AuthCorsIntegrationTest cases aligned separately in #4217. No other test moved.

Only the query wrappers are asserted, as in the existing tests of this class; no MySQL instance is
available here, so the escaping is verified at the point where the value is bound.

Fixes #4289

@btlqql

btlqql commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Withdrawing: #4192 covers exactly this change — the same escapeLike helper applied to the same two
query builders (findPage and the shared applyFilters), already CLEAN since 2026-09-09 — and the
older #2947 targets the same repository method. This branch only repeats them.

Filed without screening the open pull requests for the audit search first; closing #4289 for the same
reason. The red/green pair for this branch is at audit/rmq-audit-red.log / audit/rmq-audit-green.log
(Tests run: 11, Failures: 2 → Failures: 0) and the escaping is identical to #4192, so there is
nothing here worth keeping.

@btlqql btlqql closed this Sep 14, 2026
@btlqql btlqql reopened this Sep 15, 2026
The audit log search value is handed to `like("operator", search)` and its
two sibling columns unchanged, so MyBatis-Plus builds `%<search>%` with the
caller's `%` and `_` still acting as wildcards. Searching for `100%_done`
matches any row whose operator, resource name or detail merely contains
`100` followed by any characters, and a search of `%` or `_` matches the
whole table instead of the rows that contain that character.

The same value feeds the summary and the CSV export
(`AuditService.exportLogs` reuses `findPage`), so the insight counts and the
exported file are wrong in the same way as the list view.

Escaping now happens once per query builder, with the backslash convention
already used by the message query history search, which MySQL resolves
without an `ESCAPE` clause.
@btlqql
btlqql force-pushed the codex/btl-audit-search-wildcards branch from d438e8f to cf801d7 Compare September 15, 2026 15:13
@btlqql

btlqql commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Reopened and rebased as requested.

The rebase moved the branch from 6c24d2ed to the current rocketmq-studio head (1e8cc591). Two
adjustments were needed because upstream refactored findPage since this branch was written:

  • findPage now delegates to applyFilters(...) (which also gained the target / clusterIdMissing
    parameters). The escaping therefore lives in applyFilters, so every caller - findPage and
    all five aggregate queries of summarize - gets the escaped pattern from one place. The conflict was
    resolved in favour of upstream's structure.
  • The new tests call findPage with the current 11-argument signature.

Fresh evidence on the rebased head (cf801d70):

Red, upstream/rocketmq-studio version of MybatisPlusAuditRepository.java with the branch tests:

[ERROR] Tests run: 14, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 2.108 s <<< FAILURE! -- in org.apache.rocketmq.studio.ops.audit.MybatisPlusAuditRepositoryTest
to contain only:
to contain only:

Green, with the fix (mvn -f server/pom.xml test -Dtest=MybatisPlusAuditRepositoryTest):

[INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.055 s -- in org.apache.rocketmq.studio.ops.audit.MybatisPlusAuditRepositoryTest
[INFO] BUILD SUCCESS

The two failures are the escaping tests (%100\%\_done% / %a\_b\%%); they still discriminate the
behaviour on the current base. mergeable_state is clean again.

@lizhimins

Copy link
Copy Markdown
Member

Escaping the audit search term so % and _ match literally is the right behaviour.
The fix itself is sound and comes with a test, so this is not a quality call. We have stopped
merging one-defect-per-PR patches out of this queue: the review and merge overhead per change is
high, and several of them touch files that other open PRs also touch.

Please regroup your open Studio fixes into one PR per functional domain (for example
instance/cluster persistence, alert rendering, AI tool input validation) and resubmit against the
current rocketmq-studio baseline — we will review the consolidated PRs. When you regroup, note
that other authors' patches on the same files are also being closed, so please pick one approach
per file rather than resubmitting the overlap.

For the regroup: this is the same repository method that #4192 patches, and it adds another private escapeLike copy. We are consolidating LIKE escaping into one shared utility, and the escape also needs an explicit ESCAPE clause — the helper currently relies on MySQL's implicit backslash, which H2 (used by five tests in this suite) does not provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants