Skip to content

fix(mcp): escape LIKE wildcards in MCP list tool search filters#40682

Merged
aminghadersohi merged 2 commits into
masterfrom
fix-like-wildcard-mcp-list-tools
Jun 3, 2026
Merged

fix(mcp): escape LIKE wildcards in MCP list tool search filters#40682
aminghadersohi merged 2 commits into
masterfrom
fix-like-wildcard-mcp-list-tools

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

SUMMARY

All 10+ MCP list tools (list_users, list_rls_filters, list_roles, list_reports, etc.) delegate to BaseDAO._build_query() and BaseDAO.list() in superset/daos/base.py. Both methods build search filters with:

cast(column, Text).ilike(f"%{search}%")

search="%" is a single character that passes every existing Pydantic validator. The resulting pattern ILIKE '%%' matches every row, allowing an admin to enumerate all records regardless of the intended search intent — LIKE wildcard injection.

Fix: Added a _escape_like() static method that escapes \, %, and _ before interpolation, and passes escape="\\" to ilike(). Applied at both call sites.

@staticmethod
def _escape_like(value: str) -> str:
    return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")

# usage
cast(column, Text).ilike(f"%{cls._escape_like(search)}%", escape="\\")

All affected tools are admin-only (the MCP API enforces admin authentication), so blast radius is limited. Related prior fix for find_users: #40631.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend-only change.

TESTING INSTRUCTIONS

  1. Run the new regression test:
    pytest tests/integration_tests/dao/base_dao_test.py::test_base_dao_list_search_wildcard_injection -v
  2. Verify search="%" returns 0 results for an admin with no users whose username contains a literal %.
  3. Verify search="admin" still returns users matching "admin" normally.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

search='%' passed all validators and produced ILIKE '%%', matching every
row. Add _escape_like() helper that escapes \, %, and _ before they are
interpolated into ILIKE patterns, and apply it at both call sites in
BaseDAO._build_query() and BaseDAO.list().

Adds a regression test that asserts search='%' does not match rows whose
names contain no percent sign.
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Jun 2, 2026

Code Review Agent Run #60dba8

Actionable Suggestions - 0
Additional Suggestions - 2
  • superset/daos/base.py - 1
    • Missing unit tests for new function · Line 636-638
      The new _escape_like function (line 636) lacks test coverage. An equivalent function _escape_ilike_fragment in datasource.py has a test at tests/unit_tests/datasource/dao_tests.py but no corresponding test exists for base.py's version. Add test cases for inputs containing %, _, and \ characters.
  • tests/integration_tests/dao/base_dao_test.py - 1
    • Incomplete wildcard-escape test coverage · Line 812-838
      The test only asserts the negative case (search='%' must not match plain names). Add at least one positive case — a user whose actual username contains a literal '%' — to verify the escape is applied and those rows are excluded too. Without this, the test could pass even if '%' were silently replaced or stripped instead of escaped.
      Code suggestion
      --- tests/integration_tests/dao/base_dao_test.py (lines 828-838)
       828:     # '%' must be escaped; it should not match any of our plain-name users
       829:     results, total = UserDAO.list(search="%", search_columns=["username", "first_name"])
       830:     result_usernames = {r.username for r in results}
       831:     for user in users:
       832:         assert user.username not in result_usernames, (
       833:             f"search='%' should not match '{user.username}' — LIKE wildcard injection"
       834:         )
       835:+
       836:+
      +    # Positive case: a row with a literal '%' in its name must also NOT match
       837:+
      +    literal_user = User(
       838:+
      +        id=423,
       839:+
      +        username="has%percent",
       840:+
      +        first_name="HasPercent",
       841:+
      +        last_name="User",
       842:+
      +        email="haspercent@example.com",
       843:+
      +        active=True,
       844:+
      +    )
       845:+
      +    user_with_data.add(literal_user)
       846:+
      +    user_with_data.commit()
       847:+
      +    results, total = UserDAO.list(search="%", search_columns=["username", "first_name"])
       848:+
      +    assert literal_user.username not in {r.username for r in results}, (
       849:+
      +        "search='%' must not match a row whose name contains a literal '%'"
       850:+
      +    )
       851:+
      +    user_with_data.delete(literal_user)
       852:+
      +    user_with_data.commit()
      +
       837:     for user in users:
       838:         user_with_data.delete(user)
       839:     user_with_data.commit()
Review Details
  • Files reviewed - 2 · Commit Range: 49eebad..49eebad
    • superset/daos/base.py
    • tests/integration_tests/dao/base_dao_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi marked this pull request as ready for review June 2, 2026 14:54
@aminghadersohi aminghadersohi requested a review from rusackas June 2, 2026 15:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.96%. Comparing base (e2ed989) to head (f73a2d5).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
superset/daos/base.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40682      +/-   ##
==========================================
- Coverage   63.96%   63.96%   -0.01%     
==========================================
  Files        2658     2658              
  Lines      143095   143123      +28     
  Branches    32907    32910       +3     
==========================================
+ Hits        91537    91544       +7     
- Misses      49991    50012      +21     
  Partials     1567     1567              
Flag Coverage Δ
hive 39.74% <25.00%> (-0.01%) ⬇️
mysql 58.38% <75.00%> (-0.02%) ⬇️
postgres 58.45% <75.00%> (-0.02%) ⬇️
presto 41.35% <25.00%> (-0.01%) ⬇️
python 59.94% <75.00%> (-0.02%) ⬇️
sqlite 58.11% <75.00%> (-0.02%) ⬇️
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@richardfogaca
Copy link
Copy Markdown
Contributor

Posting on Richard's behalf — this is his PR reviewer agent. Forward any pushback to him and he'll loop me back in.

Left one functional note below. The direct search="%" path is fixed, but the MCP list filter path still appears to expose the same wildcard behavior through column_operators. Line numbers verified against HEAD 49eebad.

Functional — worth fixing before merge

  • superset/daos/base.py:667

    This escapes the free-text search branch, but MCP filters still pass through ModelListCore into BaseDAO.list(column_operators=...), and the shared operator_map still builds raw wildcard patterns for sw, ew, ct, like, and ilike at superset/daos/base.py:87-101. For example, a caller can still send a list filter like {"col": "dashboard_title", "opr": "ilike", "value": "%"}; the dashboard schema accepts ColumnOperatorEnum for opr, and schema discovery advertises filterable operators, so that still becomes ILIKE '%%'.

    WDYT — could we route the wildcard operators through the same escaping logic, or otherwise restrict wildcard operators for MCP list filters? It would also be worth adding a regression for column_operators with % / _, since the new test only covers the search parameter.

Praise

  • tests/integration_tests/dao/base_dao_test.py:812

    The direct search="%" regression is a good target for the reported issue and keeps the test at the DAO layer where both _build_query / list behavior is easiest to reason about.

The wildcard operator lambdas (sw, ew, ct, like, ilike) in operator_map
also interpolated user values directly into LIKE/ILIKE patterns. A caller
could send column_operators=[{col, opr:ilike, value:%}] and receive every
row regardless of base_filter.

Move _escape_like to module level (before operator_map) and apply it in
all five wildcard lambdas. Remove the @staticmethod duplicate from BaseDAO.

Adds a regression test covering all five operators with value='%'.
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Jun 2, 2026
Copy link
Copy Markdown
Contributor

@richardfogaca richardfogaca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Jun 2, 2026

Code Review Agent Run #43f61a

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 49eebad..f73a2d5
    • superset/daos/base.py
    • tests/integration_tests/dao/base_dao_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi merged commit 0018344 into master Jun 3, 2026
62 checks passed
@aminghadersohi aminghadersohi deleted the fix-like-wildcard-mcp-list-tools branch June 3, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants