Add security rules for detecting hard-coded secrets in C#, Java, and Python #102
Conversation
|
Sakshis seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis pull request introduces new security rules for detecting hard-coded secrets across C#, Java, and Python applications. Each rule is designed to identify specific vulnerabilities related to hard-coded passwords or empty passwords in connection strings. The changes include the creation of YAML configuration files for the rules, snapshots for testing, and validation frameworks to categorize valid and invalid practices. The rules reference relevant security standards and provide guidance on managing sensitive information securely. Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
tests/java/passwordauthentication-hardcoded-password-java-test.yml (1)
3-4: Consider enhancing valid test casesThe valid section only includes a direct instantiation example. Consider adding more diverse valid cases showing proper password retrieval from secure sources (e.g., environment variables, secure vaults).
valid: - | new PasswordAuthentication("postman", "password"); + - | + String username = System.getenv("USERNAME"); + String password = System.getenv("PASSWORD"); + new PasswordAuthentication(username, password.toCharArray());tests/python/python-pg8000-empty-password-python-test.yml (2)
3-5: Enhance valid test case documentationThe valid case uses
get_password()but doesn't show its implementation. Consider adding a comment explaining the expected secure implementation pattern.valid: - | + # get_password() should retrieve credentials from secure sources + # like environment variables or secret management services import pg8000.dbapi conn = pg8000.dbapi.connect(user="postgres", password=get_password())
7-20: Consider additional invalid test casesThe current invalid cases cover empty string patterns well. Consider adding cases for:
- None values
- Whitespace-only strings
- Connection string manipulation
invalid: + - | + import pg8000.dbapi + conn = pg8000.dbapi.connect(user="postgres", password=None) + - | + import pg8000.dbapi + conn = pg8000.dbapi.connect(user="postgres", password=" ") + - | + import pg8000.dbapi + conn_str = "user=postgres password=" + conn = pg8000.dbapi.connect(conn_str)tests/csharp/sqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml (1)
3-4: Expand valid test casesThe valid section only shows a basic argument-based example. Consider adding cases demonstrating secure credential management patterns.
valid: - | builder.Password = args[1]; + - | + builder.Password = Environment.GetEnvironmentVariable("DB_PASSWORD"); + - | + builder.Password = await secretManager.GetSecretAsync("DB_PASSWORD");rules/java/security/passwordauthentication-hardcoded-password-java.yml (2)
4-14: Enhance security guidance in the message sectionWhile the current message provides good basic guidance, consider adding specific examples of secure alternatives:
- Environment variable usage:
PASSWORD=${DB_PASSWORD}- Vault integration:
password=vaultClient.getSecret("db-password")
16-42: Consider expanding pattern matching for additional security scenariosThe current pattern effectively catches direct password assignments, but consider adding patterns for:
- Base64 encoded passwords
- String concatenation
- Character array initialization
tests/__snapshots__/python-pg8000-empty-password-python-snapshot.yml (1)
3-63: Add test cases for connection string URLsThe current test cases cover direct password assignments well, but consider adding cases for connection string URLs:
conn = connect("postgresql://user:@localhost/db")rules/python/security/python-pg8000-empty-password-python.yml (1)
4-14: Enhance security message with specific Python examplesThe current message is good but could be more Python-specific. Consider adding:
- Environment variable example:
os.environ.get('DB_PASSWORD')- Python keyring usage:
keyring.get_password('system', 'db-password')tests/__snapshots__/sqlconnectionstringbuilder-hardcoded-secret-csharp-snapshot.yml (1)
1-319: Enhance test coverage and improve test data quality.While the snapshots cover various syntactical patterns for setting passwords, consider the following improvements:
Add test cases for secure patterns that should not trigger the rule:
- Reading from environment variables
- Using secure configuration providers
- Using secret management services
Replace predictable test passwords ("reee!", "aaaa") with more realistic patterns to ensure the rule catches real-world scenarios:
- Base64 encoded strings
- Complex password patterns
- Connection string formats
Example of a secure pattern test case:
+ ? | + private SqlConnectionStringBuilder GetConnection(args) + { + var cb = new SqlConnectionStringBuilder(); + cb.Password = Environment.GetEnvironmentVariable("DB_PASSWORD"); + } + : labels: + - source: Environment.GetEnvironmentVariable("DB_PASSWORD") + style: primary + start: 114 + end: 158rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml (1)
4-14: Enhance the security message with specific mitigation guidance.While the current message explains the risk well, it could benefit from more specific guidance on secure implementations.
message: >- A secret is hard-coded in the application. Secrets stored in source code, such as credentials, identifiers, and other types of sensitive data, can be leaked and used by internal or external malicious actors. Use environment variables to securely provide credentials and other secrets or - retrieve them from a secure vault or Hardware Security Module (HSM). + retrieve them from a secure vault or Hardware Security Module (HSM). + + Recommended solutions: + 1. Use Azure Key Vault: await keyVaultClient.GetSecretAsync(vaultUri, secretName) + 2. Use environment variables: Environment.GetEnvironmentVariable("DB_PASSWORD") + 3. Use secure configuration: IConfiguration.GetValue<string>("ConnectionStrings:Password")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml(1 hunks)rules/java/security/passwordauthentication-hardcoded-password-java.yml(1 hunks)rules/python/security/python-pg8000-empty-password-python.yml(1 hunks)tests/__snapshots__/passwordauthentication-hardcoded-password-java-snapshot.yml(1 hunks)tests/__snapshots__/python-pg8000-empty-password-python-snapshot.yml(1 hunks)tests/__snapshots__/sqlconnectionstringbuilder-hardcoded-secret-csharp-snapshot.yml(1 hunks)tests/csharp/sqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml(1 hunks)tests/java/passwordauthentication-hardcoded-password-java-test.yml(1 hunks)tests/python/python-pg8000-empty-password-python-test.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/snapshots/passwordauthentication-hardcoded-password-java-snapshot.yml
🧰 Additional context used
🪛 yamllint (1.35.1)
rules/python/security/python-pg8000-empty-password-python.yml
[warning] 31-31: wrong indentation: expected 10 but found 12
(indentation)
[warning] 40-40: wrong indentation: expected 20 but found 19
(indentation)
[warning] 57-57: wrong indentation: expected 10 but found 12
(indentation)
[error] 69-69: trailing spaces
(trailing-spaces)
[warning] 73-73: wrong indentation: expected 12 but found 14
(indentation)
[warning] 76-76: wrong indentation: expected 14 but found 16
(indentation)
[warning] 85-85: wrong indentation: expected 24 but found 23
(indentation)
[warning] 102-102: wrong indentation: expected 10 but found 12
(indentation)
[warning] 110-110: wrong indentation: expected 18 but found 17
(indentation)
[warning] 118-118: wrong indentation: expected 12 but found 11
(indentation)
[warning] 121-121: wrong indentation: expected 11 but found 12
(indentation)
[warning] 138-138: wrong indentation: expected 10 but found 12
(indentation)
[warning] 154-154: wrong indentation: expected 10 but found 12
(indentation)
[warning] 166-166: wrong indentation: expected 12 but found 14
(indentation)
[warning] 174-174: wrong indentation: expected 20 but found 19
(indentation)
[warning] 175-175: wrong indentation: expected 21 but found 20
(indentation)
[warning] 185-185: wrong indentation: expected 8 but found 10
(indentation)
[warning] 201-201: wrong indentation: expected 10 but found 12
(indentation)
[warning] 220-220: wrong indentation: expected 12 but found 14
(indentation)
[warning] 238-238: wrong indentation: expected 12 but found 14
(indentation)
[warning] 247-247: wrong indentation: expected 22 but found 21
(indentation)
[warning] 252-252: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (1)
rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml (1)
1-305: Verify similar patterns across the codebase.
Let's ensure consistent handling of secrets across the codebase and identify any existing secure implementations that could serve as examples.
✅ Verification successful
Based on the repository structure and files found, I can now generate the final response:
Rule implementation is consistent with other security patterns
The implementation aligns with other security-focused rules in the repository, particularly regarding secret and credential handling. The codebase shows a consistent pattern of detecting hardcoded secrets across different languages and frameworks:
- Java:
passwordauthentication-hardcoded-password-java.yml - Python:
python-pg8000-empty-password-python.yml - C#: Current rule
sqlconnectionstringbuilder-hardcoded-secret-csharp.yml
Each implementation has corresponding test files and snapshots, indicating proper test coverage and validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other files using SqlConnectionStringBuilder
echo "Checking for SqlConnectionStringBuilder usage patterns..."
rg -t cs "new SqlConnectionStringBuilder" -A 5
# Check for existing secure implementations
echo "Checking for secure implementations..."
rg -t cs "GetEnvironmentVariable|GetSecretAsync|IConfiguration"
# Check for potential hard-coded secrets in connection strings
echo "Checking for potential connection string patterns..."
ast-grep --pattern 'string $_ = "Server=$_;Database=$_;User ID=$_;Password=$_"'
Length of output: 821
Script:
#!/bin/bash
# Let's first check what files are available in the repository
echo "Checking repository structure..."
fd -t f -e cs -e yml
# Check for any C# test files that might contain examples
echo "Looking for test files with connection strings..."
rg -t cs "SqlConnection|ConnectionString" --type-list
# Check for other YAML rules related to secrets or connection strings
echo "Checking related YAML rules..."
fd -e yml -x grep -l "SqlConnection\|secret\|password\|credential" {}
Length of output: 2742
tests/csharp/sqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml
Show resolved
Hide resolved
rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml
Show resolved
Hide resolved
rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml
Show resolved
Hide resolved
rules/csharp/security/sqlconnectionstringbuilder-hardcoded-secret-csharp.yml
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Tests