-
Notifications
You must be signed in to change notification settings - Fork 0
fix(api): refactors the SQL LIKE pattern escaping logic to use a centralized utility function, ensuring consistent and secure handling of special characters across all database queries. #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: qodo_combined_20260121_qodo_grep_cursor_copilot_1_base_fixapi_refactors_the_sql_like_pattern_escaping_logic_to_use_a_centralized__utility_function_ensuring_consistent_and_secure_handling_of_special_ch
Are you sure you want to change the base?
Changes from all commits
f0a8695
9318a5d
a38bc04
b00e6e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1195,18 +1195,24 @@ def process_metadata_filter_func( | |
|
|
||
| json_field = DatasetDocument.doc_metadata[metadata_name].as_string() | ||
|
|
||
| from libs.helper import escape_like_pattern | ||
|
|
||
| match condition: | ||
| case "contains": | ||
| filters.append(json_field.like(f"%{value}%")) | ||
| escaped_value = escape_like_pattern(str(value)) | ||
| filters.append(json_field.like(f"%{escaped_value}%", escape="\\")) | ||
|
|
||
| case "not contains": | ||
| filters.append(json_field.notlike(f"%{value}%")) | ||
| escaped_value = escape_like_pattern(str(value)) | ||
| filters.append(json_field.notlike(f"%{escaped_value}%")) | ||
|
Comment on lines
1205
to
+1207
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. notlike() missing escape • In process_metadata_filter_func(), the not contains branch uses
json_field.notlike(f"%{escaped_value}%") without specifying escape="\\".
• Because escaped_value contains backslash escapes, omitting the explicit escape clause can
cause the DB to treat %/_ as wildcards (depending on backend), weakening the intended input
sanitization.
• Add escape="\\" to the notlike() call for parity with the other LIKE cases.
Agent prompt
|
||
|
|
||
| case "start with": | ||
| filters.append(json_field.like(f"{value}%")) | ||
| escaped_value = escape_like_pattern(str(value)) | ||
| filters.append(json_field.like(f"{escaped_value}%", escape="\\")) | ||
|
|
||
| case "end with": | ||
| filters.append(json_field.like(f"%{value}")) | ||
| escaped_value = escape_like_pattern(str(value)) | ||
| filters.append(json_field.like(f"%{escaped_value}", escape="\\")) | ||
|
|
||
| case "is" | "=": | ||
| if isinstance(value, str): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,39 @@ | |
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def escape_like_pattern(pattern: str) -> str: | ||
| """ | ||
| Escape special characters in a string for safe use in SQL LIKE patterns. | ||
|
|
||
| This function escapes the special characters used in SQL LIKE patterns: | ||
| - Backslash (\\) -> \\ | ||
| - Percent (%) -> \\% | ||
| - Underscore (_) -> \\_ | ||
|
|
||
| The escaped pattern can then be safely used in SQL LIKE queries with the | ||
| ESCAPE '\\' clause to prevent SQL injection via LIKE wildcards. | ||
|
|
||
| Args: | ||
| pattern: The string pattern to escape | ||
|
|
||
| Returns: | ||
| Escaped string safe for use in SQL LIKE queries | ||
|
|
||
| Examples: | ||
| >>> escape_like_pattern("50% discount") | ||
| '50\\% discount' | ||
| >>> escape_like_pattern("test_data") | ||
| 'test\\_data' | ||
| >>> escape_like_pattern("path\\to\\file") | ||
| 'path\\\\to\\\\file' | ||
| """ | ||
| if not pattern: | ||
| return pattern | ||
| # Escape backslash first, then percent and underscore | ||
| escapedPattern = pattern.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") | ||
| return escapedPattern | ||
|
Comment on lines
+64
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. escapedpattern violates snake_case • escape_like_pattern() introduces a local variable named escapedPattern using camelCase, violating the required Python naming conventions. • This reduces consistency/readability and is likely to be flagged by repository lint rules. • Rename the variable to escaped_pattern to comply with the standard. Agent prompt
Comment on lines
+35
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5. Helper type contract broken • escape_like_pattern is annotated as str -> str, but it returns the input as-is for falsy values, meaning it can return None. • Unit tests explicitly call escape_like_pattern(None), which will be flagged by basedpyright. • The repo’s make type-check runs basedpyright, so this can break CI even if runtime tests pass. Agent prompt
|
||
|
|
||
|
|
||
| def extract_tenant_id(user: Union["Account", "EndUser"]) -> str | None: | ||
| """ | ||
| Extract tenant_id from Account or EndUser object. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -444,6 +444,78 @@ def test_get_annotation_list_by_app_id_with_keyword( | |
| assert total == 1 | ||
| assert unique_keyword in annotation_list[0].question or unique_keyword in annotation_list[0].content | ||
|
|
||
| def test_get_annotation_list_by_app_id_with_special_characters_in_keyword( | ||
| self, db_session_with_containers, mock_external_service_dependencies | ||
| ): | ||
|
Comment on lines
+447
to
+449
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. New tests lack type hints • Newly added pytest test functions/methods omit type annotations for parameters and return values. • This violates the requirement that all Python function definitions include parameter and return type annotations (as enforced by basedpyright). • Add explicit annotations (use Any for pytest fixtures when concrete types are impractical) and -> None return types for tests. Agent prompt
|
||
| r""" | ||
| Test retrieval of annotation list with special characters in keyword to verify SQL injection prevention. | ||
|
|
||
| This test verifies: | ||
| - Special characters (%, _, \) in keyword are properly escaped | ||
| - Search treats special characters as literal characters, not wildcards | ||
| - SQL injection via LIKE wildcards is prevented | ||
| """ | ||
| fake = Faker() | ||
| app, account = self._create_test_app_and_account(db_session_with_containers, mock_external_service_dependencies) | ||
|
|
||
| # Create annotations with special characters in content | ||
| annotation_with_percent = { | ||
| "question": "Question with 50% discount", | ||
| "answer": "Answer about 50% discount offer", | ||
| } | ||
| AppAnnotationService.insert_app_annotation_directly(annotation_with_percent, app.id) | ||
|
|
||
| annotation_with_underscore = { | ||
| "question": "Question with test_data", | ||
| "answer": "Answer about test_data value", | ||
| } | ||
| AppAnnotationService.insert_app_annotation_directly(annotation_with_underscore, app.id) | ||
|
|
||
| annotation_with_backslash = { | ||
| "question": "Question with path\\to\\file", | ||
| "answer": "Answer about path\\to\\file location", | ||
| } | ||
| AppAnnotationService.insert_app_annotation_directly(annotation_with_backslash, app.id) | ||
|
|
||
| # Create annotation that should NOT match (contains % but as part of different text) | ||
| annotation_no_match = { | ||
| "question": "Question with 100% different", | ||
| "answer": "Answer about 100% different content", | ||
| } | ||
| AppAnnotationService.insert_app_annotation_directly(annotation_no_match, app.id) | ||
|
|
||
| # Test 1: Search with % character - should find exact match only | ||
| annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id( | ||
| app.id, page=1, limit=10, keyword="50%" | ||
| ) | ||
| assert total == 1 | ||
| assert len(annotation_list) == 1 | ||
| assert "50%" in annotation_list[0].question or "50%" in annotation_list[0].content | ||
|
|
||
| # Test 2: Search with _ character - should find exact match only | ||
| annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id( | ||
| app.id, page=1, limit=10, keyword="test_data" | ||
| ) | ||
| assert total == 1 | ||
| assert len(annotation_list) == 1 | ||
| assert "test_data" in annotation_list[0].question or "test_data" in annotation_list[0].content | ||
|
|
||
| # Test 3: Search with \ character - should find exact match only | ||
| annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id( | ||
| app.id, page=1, limit=10, keyword="path\\to\\file" | ||
| ) | ||
| assert total == 1 | ||
| assert len(annotation_list) == 1 | ||
| assert "path\\to\\file" in annotation_list[0].question or "path\\to\\file" in annotation_list[0].content | ||
|
|
||
| # Test 4: Search with % should NOT match 100% (verifies escaping works) | ||
| annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id( | ||
| app.id, page=1, limit=10, keyword="50%" | ||
| ) | ||
| # Should only find the 50% annotation, not the 100% one | ||
| assert total == 1 | ||
| assert all("50%" in (item.question or "") or "50%" in (item.content or "") for item in annotation_list) | ||
|
|
||
| def test_get_annotation_list_by_app_id_app_not_found( | ||
| self, db_session_with_containers, mock_external_service_dependencies | ||
| ): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Iris like escape mismatch
📘 Rule violation⛨ SecurityAgent prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools