Skip to content

Commit 8d2f277

Browse files
Ashutosh0xcopybara-github
authored andcommitted
fix: validate SQL identifiers in Spanner search tool
Merge #5952 Fixes #5913 PiperOrigin-RevId: 967358801
1 parent 8989aea commit 8d2f277

3 files changed

Lines changed: 550 additions & 18 deletions

File tree

‎src/google/adk/tools/spanner/search_tool.py‎

Lines changed: 196 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import asyncio
1818
import json
19+
import re
1920
from typing import Any
2021
from typing import Dict
2122
from typing import List
@@ -31,6 +32,115 @@
3132
from .settings import EXACT_NEAREST_NEIGHBORS
3233
from .settings import SpannerToolSettings
3334

35+
# Pattern for valid SQL identifiers: alphanumeric, underscores,
36+
# dots (for schema-qualified names), and backtick/double-quote quoting.
37+
# Supports per-part quoting for schema-qualified names.
38+
_IDENTIFIER_PART_PATTERN = r'(?:[A-Za-z_][A-Za-z0-9_]*|`[^`\\]+`|"[^"\\]+")'
39+
_SAFE_IDENTIFIER_RE = re.compile(
40+
rf"^{_IDENTIFIER_PART_PATTERN}(?:\.{_IDENTIFIER_PART_PATTERN})*$"
41+
)
42+
43+
# Operator allowlist for additional_filter
44+
_ALLOWED_OPERATORS = r"(?:=|!=|<=|>=|<|>|(?i:\bLIKE\b|\bIS\s+NOT\b|\bIS\b))"
45+
46+
# Value allowlist for additional_filter: numbers, single-quoted strings (no backslashes), booleans, NULL
47+
_ALLOWED_VALUES = (
48+
r"(?:[+-]?\d+(?:\.\d+)?|'[^'\\]*'|(?i:\bTRUE\b|\bFALSE\b|\bNULL\b))"
49+
)
50+
51+
# IN operator support
52+
_IN_OPERATOR = r"(?i:\bNOT\s+IN\b|\bIN\b)"
53+
_IN_VALUES = rf"\(\s*{_ALLOWED_VALUES}(?:\s*,\s*{_ALLOWED_VALUES})*\s*\)"
54+
55+
# BETWEEN operator support
56+
_BETWEEN_OPERATOR = r"(?i:\bBETWEEN\b)"
57+
_BETWEEN_VALUE = rf"{_ALLOWED_VALUES}\s+(?i:\bAND\b)\s+{_ALLOWED_VALUES}"
58+
59+
# A single condition (without paren)
60+
_BASE_COND = (
61+
rf"(?:"
62+
rf"(?:{_IDENTIFIER_PART_PATTERN}(?:\.{_IDENTIFIER_PART_PATTERN})*)\s*{_ALLOWED_OPERATORS}\s*{_ALLOWED_VALUES}"
63+
rf"|(?:{_IDENTIFIER_PART_PATTERN}(?:\.{_IDENTIFIER_PART_PATTERN})*)\s*{_IN_OPERATOR}\s*{_IN_VALUES}"
64+
rf"|(?:{_IDENTIFIER_PART_PATTERN}(?:\.{_IDENTIFIER_PART_PATTERN})*)\s*{_BETWEEN_OPERATOR}\s*{_BETWEEN_VALUE}"
65+
rf"|{_IDENTIFIER_PART_PATTERN}(?:\.{_IDENTIFIER_PART_PATTERN})*" # Just identifier (e.g. boolean col)
66+
rf"|1\s*=\s*1" # dummy filter
67+
rf")"
68+
)
69+
70+
_BLOCK_0 = rf"{_BASE_COND}(?:\s+(?i:\bAND\b|\bOR\b)\s+{_BASE_COND})*"
71+
_COND_1 = rf"(?:{_BASE_COND}|\(\s*{_BLOCK_0}\s*\))"
72+
_BLOCK_1 = rf"{_COND_1}(?:\s+(?i:\bAND\b|\bOR\b)\s+{_COND_1})*"
73+
_COND_2 = rf"(?:{_BASE_COND}|\(\s*{_BLOCK_1}\s*\))"
74+
75+
# Full filter pattern: conditions joined by AND/OR, supporting up to 2 levels of nested parens
76+
_SAFE_FILTER_RE = re.compile(
77+
rf"^\s*{_COND_2}(?:\s+(?i:\bAND\b|\bOR\b)\s+{_COND_2})*\s*$",
78+
re.IGNORECASE,
79+
)
80+
81+
82+
def _validate_identifier(value: str, param_name: str) -> str:
83+
"""Validate that a value is a safe SQL identifier.
84+
85+
Args:
86+
value: The identifier string to validate.
87+
param_name: Name of the parameter (for error messages).
88+
89+
Returns:
90+
The validated identifier string.
91+
92+
Raises:
93+
ValueError: If the identifier contains unsafe characters.
94+
"""
95+
if not value or not _SAFE_IDENTIFIER_RE.match(value.strip()):
96+
raise ValueError(
97+
f"Invalid SQL identifier for {param_name}: {value!r}. "
98+
"Identifiers must contain only alphanumeric characters, underscores, "
99+
"and dots, or be quoted with backticks or double quotes."
100+
)
101+
return value.strip()
102+
103+
104+
def _validate_column_list(columns: List[str], param_name: str) -> List[str]:
105+
"""Validate that each column name in a list is a safe SQL identifier."""
106+
validated = []
107+
for col in columns:
108+
_validate_identifier(col, param_name)
109+
validated.append(col)
110+
return validated
111+
112+
113+
def _validate_additional_filter(
114+
filter_value: Optional[str],
115+
) -> Optional[str]:
116+
"""Validate that an additional_filter does not contain injection patterns.
117+
118+
This is a defense-in-depth measure. The additional_filter field is
119+
documented as a developer-trusted value, but since it can be populated
120+
by the LLM at runtime via tool calls, we restrict it to an allow-listed
121+
grammar.
122+
123+
Args:
124+
filter_value: The filter string to validate.
125+
126+
Returns:
127+
The validated filter string, or None.
128+
129+
Raises:
130+
ValueError: If the filter contains dangerous patterns.
131+
"""
132+
if filter_value is None:
133+
return None
134+
if not _SAFE_FILTER_RE.match(filter_value):
135+
raise ValueError(
136+
"additional_filter contains unsafe or unsupported patterns: "
137+
f"{filter_value!r}. Only simple filters using =, !=, <=, >=, <, >, "
138+
"LIKE, IS, IS NOT, IN, BETWEEN joined by AND or OR (with up to 2 "
139+
"levels of nested parentheses) are allowed."
140+
)
141+
return filter_value
142+
143+
34144
# Embedding model settings.
35145
# Only for Spanner GoogleSQL dialect database, and use Spanner ML.PREDICT
36146
# function.
@@ -75,6 +185,8 @@ def _generate_postgresql_for_embedding_query(
75185
vertex_ai_embedding_model_endpoint: str,
76186
output_dimensionality: Optional[int],
77187
) -> str:
188+
if output_dimensionality is not None:
189+
output_dimensionality = int(output_dimensionality)
78190
instances_json = f"""
79191
'instances',
80192
JSONB_BUILD_ARRAY(
@@ -166,6 +278,7 @@ def _generate_sql_for_knn(
166278
top_k: int,
167279
) -> str:
168280
"""Generates a SQL query for kNN search."""
281+
top_k = int(top_k)
169282
if dialect == DatabaseDialect.POSTGRESQL:
170283
distance_function = _get_postgresql_distance_function(distance_type)
171284
embedding_parameter = f"${_POSTGRESQL_PARAMETER_QUERY_EMBEDDING}"
@@ -174,7 +287,7 @@ def _generate_sql_for_knn(
174287
distance_type, ann=False
175288
)
176289
embedding_parameter = f"@{_GOOGLESQL_PARAMETER_QUERY_EMBEDDING}"
177-
columns = columns + [f"""{distance_function}(
290+
columns = list(columns) + [f"""{distance_function}(
178291
{embedding_column_to_search},
179292
{embedding_parameter}) AS {_DISTANCE_ALIAS}
180293
"""]
@@ -205,13 +318,15 @@ def _generate_sql_for_ann(
205318
num_leaves_to_search: int,
206319
):
207320
"""Generates a SQL query for ANN search."""
321+
top_k = int(top_k)
322+
num_leaves_to_search = int(num_leaves_to_search)
208323
if dialect == DatabaseDialect.POSTGRESQL:
209324
raise NotImplementedError(
210325
f"{APPROXIMATE_NEAREST_NEIGHBORS} is not supported for PostgreSQL"
211326
" dialect."
212327
)
213328
distance_function = _get_googlesql_distance_function(distance_type, ann=True)
214-
columns = columns + [f"""{distance_function}(
329+
columns = list(columns) + [f"""{distance_function}(
215330
{embedding_column_to_search},
216331
@{_GOOGLESQL_PARAMETER_QUERY_EMBEDDING},
217332
options => JSON '{{"num_leaves_to_search": {num_leaves_to_search}}}'
@@ -297,7 +412,14 @@ async def similarity_search(
297412
credentials (Credentials): The credentials to use for the request.
298413
additional_filter (Optional[str]): An optional filter to apply to the
299414
search query. If provided, this will be added to the WHERE clause of the
300-
final query.
415+
final query. Only simple filters are allowed. Supported grammar:
416+
- Columns and values compared with: =, !=, <, >, <=, >=, LIKE, IS, IS NOT
417+
- Set membership: IN, NOT IN (e.g., col IN (val1, val2))
418+
- Range checks: BETWEEN ... AND ... (e.g., col BETWEEN val1 AND val2)
419+
- Boolean columns (e.g., col_name) or dummy filter '1=1'
420+
- Logical operators: AND, OR (case-insensitive)
421+
- Parentheses: up to 2 levels of nesting (e.g., (col1 = val1 OR col2 = val2) AND col3 = val3)
422+
Values must be numbers, single-quoted strings (without backslashes), booleans, or NULL.
301423
search_options (Optional[Dict[str, Any]]): A dictionary of options to use
302424
for the similarity search. The following options are supported:
303425
- top_k: The number of most similar documents to return. The
@@ -328,7 +450,7 @@ async def similarity_search(
328450
... project_id="my-project",
329451
... instance_id="my-instance",
330452
... database_id="my-database",
331-
... table_name="my-product-table",
453+
... table_name="my_product_table",
332454
... query="Tools that can help me clean my house.",
333455
... embedding_column_to_search="product_description_embedding",
334456
... columns=["product_name", "product_description", "price_in_cents"],
@@ -362,6 +484,67 @@ async def similarity_search(
362484
"""
363485
# fmt: on
364486
try:
487+
# Validate input arguments to prevent SQL injection
488+
_validate_identifier(table_name, "table_name")
489+
_validate_identifier(
490+
embedding_column_to_search, "embedding_column_to_search"
491+
)
492+
_validate_column_list(columns, "columns")
493+
if additional_filter:
494+
_validate_additional_filter(additional_filter)
495+
496+
opts = embedding_options or {}
497+
gsql_model = opts.get(_SPANNER_GSQL_EMBEDDING_MODEL_NAME)
498+
if gsql_model:
499+
_validate_identifier(gsql_model, _SPANNER_GSQL_EMBEDDING_MODEL_NAME)
500+
501+
pg_endpoint = opts.get(_SPANNER_PG_VERTEX_AI_EMBEDDING_MODEL_ENDPOINT)
502+
if pg_endpoint:
503+
if not re.match(
504+
r"^projects/[\w-]+/locations/[\w-]+/publishers/[\w-]+/models/[\w.-]+$",
505+
pg_endpoint,
506+
):
507+
raise ValueError(
508+
"Invalid Vertex AI endpoint format: "
509+
f"{pg_endpoint!r}. Expected format: "
510+
"projects/$project/locations/$location/publishers/google/models/$model"
511+
)
512+
513+
return await _similarity_search_internal(
514+
project_id=project_id,
515+
instance_id=instance_id,
516+
database_id=database_id,
517+
table_name=table_name,
518+
query=query,
519+
embedding_column_to_search=embedding_column_to_search,
520+
columns=columns,
521+
embedding_options=embedding_options,
522+
credentials=credentials,
523+
additional_filter=additional_filter,
524+
search_options=search_options,
525+
)
526+
except Exception as ex:
527+
return {
528+
"status": "ERROR",
529+
"error_details": repr(ex),
530+
}
531+
532+
533+
async def _similarity_search_internal(
534+
project_id: str,
535+
instance_id: str,
536+
database_id: str,
537+
table_name: str,
538+
query: str,
539+
embedding_column_to_search: str,
540+
columns: List[str],
541+
embedding_options: Dict[str, str],
542+
credentials: Credentials,
543+
additional_filter: Optional[str] = None,
544+
search_options: Optional[Dict[str, Any]] = None,
545+
) -> Dict[str, Any]:
546+
try:
547+
365548
# Get Spanner client
366549
spanner_client = client.get_spanner_client(
367550
project=project_id, credentials=credentials
@@ -426,6 +609,14 @@ async def similarity_search(
426609
" must be specified for PostgreSQL dialect Spanner database."
427610
)
428611
output_dimensionality = embedding_options.get(_OUTPUT_DIMENSIONALITY)
612+
if output_dimensionality is not None:
613+
try:
614+
output_dimensionality = int(output_dimensionality)
615+
except (ValueError, TypeError):
616+
raise ValueError(
617+
f"Invalid value for {_OUTPUT_DIMENSIONALITY}:"
618+
f" {output_dimensionality!r}. Must be an integer."
619+
)
429620
if (
430621
output_dimensionality is not None
431622
and spanner_gsql_embedding_model_name is not None
@@ -607,7 +798,7 @@ async def vector_store_similarity_search(
607798
settings.vector_store_settings.num_leaves_to_search
608799
)
609800

610-
return await similarity_search(
801+
return await _similarity_search_internal(
611802
project_id=settings.vector_store_settings.project_id,
612803
instance_id=settings.vector_store_settings.instance_id,
613804
database_id=settings.vector_store_settings.database_id,

0 commit comments

Comments
 (0)