Skip to content

fix: prevent SQL injection in SnowflakeSearchTool and NL2SQLTool#4997

Open
jmanico wants to merge 1 commit intocrewAIInc:mainfrom
jmanico:fix/snowflake-nl2sql-sql-injection
Open

fix: prevent SQL injection in SnowflakeSearchTool and NL2SQLTool#4997
jmanico wants to merge 1 commit intocrewAIInc:mainfrom
jmanico:fix/snowflake-nl2sql-sql-injection

Conversation

@jmanico
Copy link
Copy Markdown

@jmanico jmanico commented Mar 20, 2026

Summary

Fixes SQL injection vulnerabilities in SnowflakeSearchTool and NL2SQLTool reported in #4993.

SnowflakeSearchTool — the database and snowflake_schema parameters in _run() were interpolated directly into USE DATABASE / USE SCHEMA SQL statements via f-strings with no validation. This PR adds three layers of defense:

  • Pydantic pattern validation on both SnowflakeSearchToolInput and SnowflakeConfig fields — rejects anything that isn't a valid Snowflake identifier before it reaches any code path
  • Runtime _validate_identifier() check in _run() — defense-in-depth for callers that bypass Pydantic (e.g. direct method calls)
  • Double-quoted identifiers in the SQL itself with embedded quote stripping — so even if both prior layers are somehow bypassed, Snowflake treats the value as a delimited identifier, not SQL syntax

NL2SQLTool_fetch_all_available_columns() was using f-string interpolation to build a WHERE clause with the table name (flagged with # noqa: S608). Replaced with SQLAlchemy text() bind parameters, which is the standard parameterized query approach. Added a params argument to execute_sql() to support this.

Changes

  • snowflake_search_tool.py: Added SNOWFLAKE_IDENTIFIER_PATTERN constant, _validate_identifier() helper, pattern validation on input/config fields, quoted identifiers in _run()
  • nl2sql_tool.py: Switched _fetch_all_available_columns() to parameterized query, added params arg to execute_sql()
  • snowflake_search_tool_test.py: Added comprehensive tests covering identifier validation, Pydantic-level rejection of injection payloads, defense-in-depth runtime checks, and quoted identifier verification

Test plan

  • TestIdentifierValidation — validates _validate_identifier() accepts valid identifiers and rejects injection payloads (semicolons, quotes, spaces, parens, empty strings, leading numbers)
  • TestInputSchemaValidation — confirms Pydantic rejects injection in database/schema fields
  • TestConfigIdentifierValidation — confirms SnowflakeConfig rejects injection in database/schema
  • test_run_rejects_injection_in_database / test_run_rejects_injection_in_schema — defense-in-depth tests at the _run() level
  • test_run_uses_quoted_identifiers — verifies double-quoting in generated SQL

Closes #4993

Add identifier validation to SnowflakeSearchTool database and schema
parameters to prevent SQL injection via f-string interpolation in USE
DATABASE/SCHEMA statements. Defense-in-depth: Pydantic pattern validation
on input fields, runtime regex check, and double-quoted identifiers.

For NL2SQLTool, replace f-string interpolation with parameterized query
using SQLAlchemy text() bind parameters in _fetch_all_available_columns.

Closes crewAIInc#4993
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.

SQL injection in SnowflakeSearchTool via database and schema parameters

1 participant