NVIDIA / NVIDIA/NeMo-Agent-Toolkit
Vanna query should be validated before executed
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.6k
- Forks
- 762
- Avg merge
- 21h 28m
- Merged PRs (30d)
- 27
Description
Is this a new feature, an improvement, or a change to existing functionality?
Change
How would you describe the priority of this feature request
Medium
Please provide a clear description of problem this feature solves
Feature Request: text2sql should not execute Destructive SQL Operations
Description
Currently, the Vanna plugin text2sql function appears to execute generated SQL queries directly (when execute_sql=true). While this is efficient for SELECT statements, it poses a significant risk when the model generates queries that modify, delete, or structure data (e.g., INSERT, UPDATE, DELETE, DROP, ALTER).
To improve security and prevent accidental data loss, I propose hardening the execution rules so that any non-read-only query is returned to the user as text for manual review and execution, rather than being run automatically.
Suggested Behavior
- Generate any SQL statement
- use
vanna_instance.is_sql_valid(sql=...check if the SQL should be run.
Conditional Execution:- Read-only (
SELECT): Continue to execute and display results as usual. - Destructive/Modifying: Instead of executing, the plugin display the SQL with explanation to the user.
- Read-only (
- User Confirmation: User review the SQL and run it manually in her database terminal if it looks correct.
Use Case / Rationale
- Safety: Prevents the LLM from accidentally hallucinating a
WHEREclause incorrectly on aDELETEorUPDATEstatement. - Security: Reduces the risk of "Prompt Injection" resulting in unauthorized data modification.
Describe your ideal solution
@register_function(config_type=Text2SQLConfig, framework_wrappers=[LLMFrameworkEnum.LANGCHAIN])
async def text2sql(config: Text2SQLConfig, builder: Builder):
"""Register the Text2SQL function with Vanna integration."""
# ...
sql = str(sql_result.get("sql", ""))
explanation: str | None = sql_result.get("explanation")
is_valid_sql = vanna_instance.is_sql_valid(sql) # <------ add this check
# If execute_sql is enabled, run the query
if config.execute_sql and is_valid_sql:
yield ResponseIntermediateStep(
id=str(uuid.uuid4()),
parent_id=parent_id,
type="markdown",
name="text2sql_status",
payload=StatusPayload(message="Executing SQL query...").model_dump_json(),
)
# Execute SQL and propagate errors
# Note: run_sql is dynamically set as async function in setup_vanna_db_connection
df = await vanna_instance.run_sql(sql) # type: ignore[misc]
logger.info(f"SQL executed successfully: {len(df)} rows returned")
# Yield final result as Text2SQLOutput
yield Text2SQLOutput(sql=sql, explanation=explanation)
Additional context
https://try.vanna.ai/docs/hardening-guide/
Code of Conduct
- I agree to follow this project's Code of Conduct
- I have searched the open feature requests and have found no duplicates for this feature request
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the text2sql function registered with Text2SQLConfig and inspect how it obtains sql_result, checks execute_sql, and calls vanna_instance.run_sql. Use the proposed vanna_instance.is_sql_valid check to distinguish read-only execution from SQL returned for review, while preserving the final Text2SQLOutput. Done means destructive or modifying statements are not executed automatically and SELECT statements still execute when configured.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, sql
- Domain
- backend, databases, security
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100