-
Notifications
You must be signed in to change notification settings - Fork 2
Convert the codacy.csharp.ai.insecure-llm-model-usage rule to support allow list #75
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
Changes from all commits
13693e5
9cad841
34d0e06
b64743a
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 |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <module name="root"> | ||
| <module name="codacy.csharp.ai.insecure-llm-model-usage"> | ||
| <property name="modelRegex" value="^(gemini-2.5-flash|gpt-3\\.5-turbo|old-llama-model)$" /> | ||
| <property name="modelAllowList" value="gemini-2.5-flash,gpt-3.5-turbo,old-llama-model" /> | ||
| </module> | ||
| </module> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -167,17 +167,23 @@ func replaceParameterPlaceholders(line string, pattern *codacy.Pattern) string { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| paramName := matches[1] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Convert UPPER_CASE to camelCase to match parameter name format | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| formattedParamName := formatParameterName(paramName) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Find the parameter in the pattern | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, param := range pattern.Parameters { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if param.Name == formattedParamName { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use Value if set, otherwise use Default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value := param.Value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if value == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = param.Default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if value != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Sprintf("%v", value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| valueStr := fmt.Sprintf("%v", value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If parameter name ends with _ALLOW_LIST, convert comma-separated list to regex pattern | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if strings.HasSuffix(paramName, "_ALLOW_LIST") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return convertListToRegex(valueStr, false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return valueStr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -189,6 +195,27 @@ func replaceParameterPlaceholders(line string, pattern *codacy.Pattern) string { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // convertListToRegex converts a comma-separated list into a regex alternation pattern | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Example: "gemini-2.5-flash,gpt-3.5-turbo,old-llama-model" -> "^(gemini-2\\.5-flash|gpt-3\\.5-turbo|old-llama-model)$" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+198
to
+199
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // convertListToRegex converts a comma-separated list into a regex alternation pattern | |
| // Example: "gemini-2.5-flash,gpt-3.5-turbo,old-llama-model" -> "^(gemini-2\\.5-flash|gpt-3\\.5-turbo|old-llama-model)$" | |
| // convertListToRegex converts a comma-separated list into a regex alternation pattern. | |
| // Example: | |
| // If include=true: "gemini-2.5-flash,gpt-3.5-turbo,old-llama-model" -> "^(gemini-2\\.5-flash|gpt-3\\.5-turbo|old-llama-model)$" | |
| // If include=false: "gemini-2.5-flash,gpt-3.5-turbo,old-llama-model" -> "^(?!(gemini-2\\.5-flash|gpt-3\\.5-turbo|old-llama-model)$).*" |
Copilot
AI
Dec 3, 2025
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.
The convertListToRegex function only escapes dots (.) for regex special characters. However, model names could contain other regex special characters like +, *, ?, (, ), [, ], {, }, |, ^, $, \, etc. Consider using regexp.QuoteMeta() to properly escape all regex special characters to avoid potential regex injection or matching errors.
Example:
item = regexp.QuoteMeta(strings.TrimSpace(item))| // Trim whitespace | |
| item = strings.TrimSpace(item) | |
| // Escape dots for regex | |
| item = strings.ReplaceAll(item, ".", "\\.") | |
| // Trim whitespace and escape all regex meta-characters | |
| item = regexp.QuoteMeta(strings.TrimSpace(item)) |
Copilot
AI
Dec 3, 2025
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.
The convertListToRegex function doesn't handle empty strings properly. If the input list is empty or contains only whitespace, this will generate a malformed regex like ^(?!()$).* or ^(?!(|item)$).* with empty alternatives. Consider adding validation to handle empty lists or filtering out empty items after trimming.
| for i, item := range items { | |
| // Trim whitespace | |
| item = strings.TrimSpace(item) | |
| // Escape dots for regex | |
| item = strings.ReplaceAll(item, ".", "\\.") | |
| items[i] = item | |
| } | |
| // Join with pipe separator and wrap in regex anchors | |
| if include { | |
| return fmt.Sprintf("^(%s)$", strings.Join(items, "|")) | |
| } | |
| return fmt.Sprintf("^(?!(%s)$).*", strings.Join(items, "|")) | |
| filteredItems := make([]string, 0, len(items)) | |
| for _, item := range items { | |
| // Trim whitespace | |
| item = strings.TrimSpace(item) | |
| if item == "" { | |
| continue | |
| } | |
| // Escape dots for regex | |
| item = strings.ReplaceAll(item, ".", "\\.") | |
| filteredItems = append(filteredItems, item) | |
| } | |
| // Handle empty list case | |
| if len(filteredItems) == 0 { | |
| if include { | |
| // Matches nothing | |
| return "^$" | |
| } else { | |
| // Matches everything | |
| return ".*" | |
| } | |
| } | |
| // Join with pipe separator and wrap in regex anchors | |
| if include { | |
| return fmt.Sprintf("^(%s)$", strings.Join(filteredItems, "|")) | |
| } | |
| return fmt.Sprintf("^(?!(%s)$).*", strings.Join(filteredItems, "|")) |
Copilot
AI
Dec 3, 2025
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.
The include parameter in convertListToRegex is always passed as false (line 184), making the if include branch on lines 212-213 unreachable dead code. Consider removing the unused parameter and the unreachable code branch to simplify the function.
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.
This assignment to response2 is useless, since its value is never read.