fix(mcp): escape LIKE wildcards in find_users to prevent user enumeration#41593
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41593 +/- ##
=======================================
Coverage 64.42% 64.42%
=======================================
Files 2668 2668
Lines 147182 147262 +80
Branches 33947 33966 +19
=======================================
+ Hits 94821 94875 +54
- Misses 50646 50658 +12
- Partials 1715 1729 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tion The find_users tool was building its LIKE pattern from raw user input, so a query of "%" or "_" matched every row in the users table, directly contradicting the docstring claim that the tool does not enumerate the full user directory. Escape backslash first, then "%" and "_", and pass escape="\\" to each ilike() call so SQL treats those sequences as literals. Adds unit tests for query="%", query="_", and query containing a literal backslash.
5665324 to
659f12d
Compare
|
| Language | Fuzzy before | Fuzzy after | New |
|---|---|---|---|
fi |
4808 | 4809 | +1 |
th |
4808 | 4809 | +1 |
How to fix
1. Install dependencies (if not already set up):
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext2. Re-extract strings and sync .po files:
./scripts/translations/babel_update.shThis rewrites superset/translations/messages.pot from the current source files and merges the changes into every .po file. Strings whose msgid changed will be marked #, fuzzy.
3. Resolve the fuzzy entries in the affected language files (fi, th):
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.poFor each fuzzy entry, either rewrite the msgstr to match the new string and remove the #, fuzzy line, or clear the msgstr to "" if you cannot provide a translation.
4. Commit your changes to the .po files.
Code Review Agent Run #7f4904Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rebenitez1802
left a comment
rebenitez1802
left a comment
There was a problem hiding this comment.
NIT: Consider extracting the escape logic into a small shared helper (e.g. escape_like(value)) if this LIKE-escaping pattern is likely to recur across MCP tools — keeps the escape-order invariant in one tested place. Otherwise LGTM
Move the LIKE metacharacter escaping logic into a shared escape_like() utility in mcp_service/utils/sanitization.py and re-export it from the utils package. Addresses reviewer NIT: keeps the escape-order invariant (backslash first, then %, then _) in one tested place rather than inline in the tool.
|
Good call — done. Extracted into |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Bito Automatic Review Skipped – PR Already Merged |
2 participants
Summary
find_usersMCP tool was building its LIKE pattern directly from raw user input, soquery="%"matched every row in the users table — directly contradicting the docstring claim that the tool does not enumerate the full user directory.%, then_, before wrapping in the surrounding%…%wildcards.escape="\\"to every.ilike()call so the database treats the escape sequences as literals.query="%",query="_", and a query containing a literal backslash.Test plan
pytest tests/unit_tests/mcp_service/system/tool/test_find_users.py— 16 tests, all passquery="%"produces needle%\%%withescape="\\"(not%%%)query="_"produces needle%\_%withescape="\\"(not%_%)query="\\"produces needle%\\%withescape="\\"