[codex] fix mysql search table name validation#6341
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMySQLSearchTool now validates and backtick-quotes table names before building ChangesMySQL search tool hardening
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR hardens MySQLSearchTool table-name handling by validating schema/table identifiers and quoting accepted names before constructing the loader query; no exploitable security vulnerabilities were identified.
Risk: Low risk. The change reduces an existing SQL injection surface and does not introduce new authentication, authorization, data exposure, or network-facing attack surfaces.
There was a problem hiding this comment.
Pull request overview
This PR mitigates SQL injection risk in MySQLSearchTool by validating/quoting MySQL table identifiers before constructing the loader query, and adds regression tests to ensure unsafe identifiers are rejected.
Changes:
- Add strict validation + backtick-quoting for
table_name(supportstableandschema.tableforms). - Build loader query using the validated, quoted identifier.
- Add tests covering valid identifier quoting, rejection of injected identifiers, and unchanged search behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/crewai-tools/src/crewai_tools/tools/mysql_search_tool/mysql_search_tool.py | Adds identifier validation and backtick-quoting before building the SELECT * FROM ... query. |
| lib/crewai-tools/tests/tools/test_mysql_search_tool.py | Introduces regression tests for valid/invalid table identifiers and confirms _run() behavior remains intact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise ValueError( | ||
| "MySQL table_name must be a valid table identifier or schema.table " | ||
| "identifier" | ||
| ) |
| ) -> None: | ||
| super().add(f"SELECT * FROM {table_name};", **kwargs) # noqa: S608 | ||
| quoted_table_name = _quote_mysql_table_name(table_name) | ||
| super().add(f"SELECT * FROM {quoted_table_name};", **kwargs) # noqa: S608 |
Summary
MySQLSearchTooltable names before building the loader queryRoot Cause
MySQLSearchTool.add()interpolatedtable_namedirectly intoSELECT * FROM {table_name};and passed that SQL through the RAG adapter to the MySQL loader, where it is executed withcursor.execute(query). The constructor API describestable_nameas a table identifier, not arbitrary SQL, so attacker-controlled names could alter the query.Validation
.venv/bin/python -m pytest lib/crewai-tools/tests/tools/test_mysql_search_tool.py->15 passed.venv/bin/python -m ruff check lib/crewai-tools/src/crewai_tools/tools/mysql_search_tool/mysql_search_tool.py lib/crewai-tools/tests/tools/test_mysql_search_tool.py-> passed.venv/bin/python -m ruff format --check lib/crewai-tools/src/crewai_tools/tools/mysql_search_tool/mysql_search_tool.py lib/crewai-tools/tests/tools/test_mysql_search_tool.py-> passedSummary by CodeRabbit