Update codacy.generic.sql.hardcoded-language-currency-orgid#12
Conversation
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
While Codacy reports that the PR is 'up to standards', there is a significant lack of verification for these regex changes. No test files or SQL samples were included to validate the updated pattern, and the coverage report is empty due to missing requirements.
A critical inconsistency was identified: the new org_id pattern is more restrictive than the language and currency patterns within the same rule, potentially missing common variable naming conventions like p_org_id. Additionally, the regex for value matching is structurally weak, allowing for unbalanced quotes or parentheses which could lead to false positives or missed detections.
About this PR
- This PR lacks a description or Jira ticket reference, making it difficult to confirm the specific bug or edge case intended to be resolved. Furthermore, no sample SQL files were provided to verify the regex logic.
Test suggestions
- Verify detection of single hardcoded org_id using equals operator (e.g., org_id = 123)
- Verify detection of org_id list with spaces before commas (e.g., org_id IN (101 , 102))
- Verify that the rule no longer matches 'org_id =' when not followed by a numeric value
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify detection of single hardcoded org_id using equals operator (e.g., org_id = 123)
2. Verify detection of org_id list with spaces before commas (e.g., org_id IN (101 , 102))
3. Verify that the rule no longer matches 'org_id =' when not followed by a numeric value
Low confidence findings
- There is no evidence of automated verification for these static analysis patterns. Since these rules govern global security/quality checks, they should ideally be accompanied by test cases.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| - pattern-regex: "(?i)^(?:(?!--).)*\\b\\w*language\\w*\\b\\s*(=|:=)\\s*'?\\b[A-Z]{2}\\b'?" | ||
| - pattern-regex: "(?i)^(?:(?!--).)*\\b\\w*currency\\w*\\b\\s*(=|:=)\\s*'?\\b[A-Z]{3}\\b'?" | ||
| - pattern-regex: "(?i)^(?:(?!--).)*\\b(\\w*\\.)?org_id\\b\\s*(=|:=|IN|!=|<>)\\s*(\\(?\\s*'?\\d+'?(,\\s*'?\\d+'?)*\\s*\\)?)?" | ||
| - pattern-regex: "(?i)^(?:(?!--).)*\\b(\\w*\\.)?org_id\\b\\s*(=|:=|IN|!=|<>)\\s*(\\(?\\s*'?\\d+'?(\\s*,\\s*'?\\d+'?)*\\s*\\)?)" |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The org_id pattern is more restrictive than the language and currency patterns in the same rule. While those patterns use \\b\\w*...\\w*\\b to catch variations (like src_language), this pattern misses common naming conventions like p_org_id or target_org_id. Additionally, the value regex allows unbalanced quotes and parentheses (e.g., matching '123 or (123). Consider broadening the field match to align with the other rules and the metadata description.
| - pattern-regex: "(?i)^(?:(?!--).)*\\b(\\w*\\.)?org_id\\b\\s*(=|:=|IN|!=|<>)\\s*(\\(?\\s*'?\\d+'?(\\s*,\\s*'?\\d+'?)*\\s*\\)?)" | |
| - pattern-regex: "(?i)^(?:(?!--).)*\\b(\\w*\\.)?\\w*org_id\\w*\\b\\s*(=|:=|IN|!=|<>)\\s*(\\(?\\s*'?\\d+'?(\\s*,\\s*'?\\d+'?)*\\s*\\)?)" |
2 participants
No description provided.