Add YAML rules for detecting hard-coded and empty passwords in Java and Python #95
Conversation
WalkthroughThis pull request introduces several new YAML rule files aimed at enhancing security in Java and Python applications by detecting hard-coded connection passwords and empty passwords in Redis connections. The new rules include specific patterns and constraints to identify instances where sensitive credentials may be improperly handled. Additionally, new test configuration files are created to validate correct and incorrect usages of these rules, ensuring adherence to secure coding practices. Changes
Possibly related PRs
Suggested reviewers
🪧 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 (
|
|
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. |
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: 4
🧹 Outside diff range and nitpick comments (7)
tests/python/python-redis-empty-password-python-test.yml (1)
14-17: Consider using placeholder hostnames in test casesThe test cases contain production-like hostnames and IP addresses. While these are test cases, it's best practice to use clearly fictional hostnames to prevent any accidental information disclosure.
- host='prod.redis.example.com', + host='test-redis.internal', - host='10.0.0.1', + host='test-host-1', - host='1.2.3.4', + host='test-host-2',Also applies to: 21-24, 27-27
tests/java/hardcoded-connection-password-java-test.yml (1)
4-34: Consolidate duplicate test patternsThe invalid section contains multiple test cases that demonstrate the same anti-pattern of hardcoding passwords. Consider consolidating these into fewer, more distinct examples to improve maintainability.
Additionally, consider adding comments to explain why each pattern is invalid to help developers understand the security implications:
invalid: - | # Invalid: Password directly hardcoded in setConnectionPassword public class PeopleTest { private javax.jdo.PersistenceManagerFactory pmf; { pmf.setConnectionPassword("asdf"); # Direct hardcoding } } - | # Invalid: Password hardcoded in a class field public class PeopleTest { private javax.jdo.PersistenceManagerFactory pmf; private String pw = "asdf"; # Still hardcoded, just moved to a field { pmf.setConnectionPassword(pw); } }tests/java/datanucleus-hardcoded-connection-password-java-test.yml (1)
10-25: Add test cases for additional DataNucleus configuration patternsThe test cases should cover more DataNucleus-specific patterns:
- Properties file configuration
- JNDI datasource configuration
- Connection URL with credentials
Example additional test case:
invalid: - | # Invalid: Hardcoded password in properties public class test { public void setUp() throws SQLException { Properties props = new Properties(); props.setProperty("datanucleus.ConnectionPassword", "password"); JDOPersistenceManagerFactory pmf = new JDOPersistenceManagerFactory(props); } }tests/__snapshots__/hardcoded-connection-password-java-snapshot.yml (1)
1-312: Enhance test coverage with additional edge cases.The test snapshots provide good coverage of basic scenarios but could be enhanced with additional test cases:
- Empty string password
- Null value handling
- Password from environment variables
- Password from configuration files
- Password from secure vaults
Would you like me to provide additional test cases for these scenarios?
rules/java/security/hardcoded-connection-password-java.yml (2)
15-354: Document pattern differences and fix formatting issues.The patterns are comprehensive but could benefit from:
- Inline documentation explaining the differences between patterns
- Examples of what each pattern matches
- Consistent indentation (currently varies between 12 and 16 spaces)
Apply this diff to fix formatting issues:
# Fix indentation in method_invocation blocks (examples) - stopBy: neighbor + stopBy: neighbor # Remove trailing spaces - - matches: PATTERN_1 + - matches: PATTERN_1🧰 Tools
🪛 yamllint (1.35.1)
[warning] 25-25: wrong indentation: expected 12 but found 16
(indentation)
[warning] 76-76: wrong indentation: expected 12 but found 16
(indentation)
[warning] 142-142: wrong indentation: expected 12 but found 16
(indentation)
[warning] 191-191: wrong indentation: expected 12 but found 16
(indentation)
[error] 230-230: trailing spaces
(trailing-spaces)
[warning] 255-255: wrong indentation: expected 12 but found 16
(indentation)
[warning] 297-297: wrong indentation: expected 12 but found 16
(indentation)
[error] 348-348: trailing spaces
(trailing-spaces)
355-360: Consider additional constraints for password validation.The current constraints only check for non-empty strings. Consider adding:
- Minimum password length check
- Pattern matching for common hardcoded passwords
- Detection of base64 encoded passwords
Would you like me to provide examples of these additional constraints?
rules/java/security/datanucleus-hardcoded-connection-password-java.yml (1)
1-576: Consider refactoring to reduce duplication with base rule.The DataNucleus-specific rule largely duplicates the base rule structure. Consider:
- Creating a base template that both rules can extend
- Extracting common patterns into shared utilities
- Only defining DataNucleus-specific patterns here
Example structure:
# base-connection-password.yml id: base-connection-password severity: warning language: java message: ${message} utils: common_patterns: # ... shared patterns # datanucleus-rule.yml extends: base-connection-password id: datanucleus-hardcoded-connection-password-java utils: # Only DataNucleus-specific patternsWould you like me to provide a detailed refactoring plan?
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 24-24: wrong indentation: expected 12 but found 16
(indentation)
[warning] 72-72: wrong indentation: expected 12 but found 16
(indentation)
[warning] 135-135: wrong indentation: expected 12 but found 16
(indentation)
[warning] 183-183: wrong indentation: expected 12 but found 16
(indentation)
[error] 222-222: trailing spaces
(trailing-spaces)
[warning] 246-246: wrong indentation: expected 12 but found 16
(indentation)
[warning] 287-287: wrong indentation: expected 12 but found 16
(indentation)
[warning] 343-343: wrong indentation: expected 12 but found 16
(indentation)
[warning] 391-391: wrong indentation: expected 12 but found 16
(indentation)
[warning] 454-454: wrong indentation: expected 12 but found 16
(indentation)
[warning] 502-502: wrong indentation: expected 12 but found 16
(indentation)
[error] 560-560: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
rules/java/security/datanucleus-hardcoded-connection-password-java.yml(1 hunks)rules/java/security/hardcoded-connection-password-java.yml(1 hunks)rules/python/security/python-redis-empty-password-python.yml(1 hunks)tests/__snapshots__/datanucleus-hardcoded-connection-password-java-snapshot.yml(1 hunks)tests/__snapshots__/hardcoded-connection-password-java-snapshot.yml(1 hunks)tests/__snapshots__/python-redis-empty-password-python-snapshot.yml(1 hunks)tests/java/datanucleus-hardcoded-connection-password-java-test.yml(1 hunks)tests/java/hardcoded-connection-password-java-test.yml(1 hunks)tests/python/python-redis-empty-password-python-test.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
rules/java/security/hardcoded-connection-password-java.yml
[warning] 25-25: wrong indentation: expected 12 but found 16
(indentation)
[warning] 76-76: wrong indentation: expected 12 but found 16
(indentation)
[warning] 142-142: wrong indentation: expected 12 but found 16
(indentation)
[warning] 191-191: wrong indentation: expected 12 but found 16
(indentation)
[error] 230-230: trailing spaces
(trailing-spaces)
[warning] 255-255: wrong indentation: expected 12 but found 16
(indentation)
[warning] 297-297: wrong indentation: expected 12 but found 16
(indentation)
[error] 348-348: trailing spaces
(trailing-spaces)
rules/python/security/python-redis-empty-password-python.yml
[warning] 39-39: wrong indentation: expected 22 but found 21
(indentation)
[warning] 74-74: wrong indentation: expected 20 but found 19
(indentation)
[warning] 78-78: wrong indentation: expected 20 but found 19
(indentation)
[warning] 81-81: wrong indentation: expected 21 but found 20
(indentation)
[error] 83-83: trailing spaces
(trailing-spaces)
[warning] 107-107: wrong indentation: expected 22 but found 21
(indentation)
[warning] 150-150: wrong indentation: expected 20 but found 19
(indentation)
[warning] 154-154: wrong indentation: expected 20 but found 19
(indentation)
[warning] 157-157: wrong indentation: expected 21 but found 20
(indentation)
rules/java/security/datanucleus-hardcoded-connection-password-java.yml
[warning] 24-24: wrong indentation: expected 12 but found 16
(indentation)
[warning] 72-72: wrong indentation: expected 12 but found 16
(indentation)
[warning] 135-135: wrong indentation: expected 12 but found 16
(indentation)
[warning] 183-183: wrong indentation: expected 12 but found 16
(indentation)
[error] 222-222: trailing spaces
(trailing-spaces)
[warning] 246-246: wrong indentation: expected 12 but found 16
(indentation)
[warning] 287-287: wrong indentation: expected 12 but found 16
(indentation)
[warning] 343-343: wrong indentation: expected 12 but found 16
(indentation)
[warning] 391-391: wrong indentation: expected 12 but found 16
(indentation)
[warning] 454-454: wrong indentation: expected 12 but found 16
(indentation)
[warning] 502-502: wrong indentation: expected 12 but found 16
(indentation)
[error] 560-560: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
tests/python/python-redis-empty-password-python-test.yml (1)
4-9: LGTM! Well-structured test cases
The valid test case properly demonstrates secure usage by retrieving the password from an environment variable, while providing a comprehensive set of invalid cases to catch various empty password scenarios.
tests/__snapshots__/python-redis-empty-password-python-snapshot.yml (1)
1-241: LGTM! Well-structured snapshot configuration
The snapshot configuration:
- Accurately captures all test cases
- Properly labels primary and secondary components
- Maintains consistent style annotations
- Provides precise source locations for each component
tests/__snapshots__/datanucleus-hardcoded-connection-password-java-snapshot.yml (1)
1-169: Verify snapshot coverage matches test cases
The snapshots correctly capture the AST patterns for hardcoded passwords, but consider:
- Adding snapshots for the valid patterns to ensure they don't trigger false positives
- Including snapshots for additional DataNucleus configuration patterns
✅ Verification successful
Snapshots correctly match test cases and provide comprehensive coverage
The snapshot file accurately reflects all test cases from the test file:
- Contains snapshots for both invalid cases (hardcoded string password and string variable with hardcoded value)
- Labels are properly positioned for highlighting the relevant code segments
- Test file includes a valid case for comparison, ensuring no false positives
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all test cases have corresponding snapshots
diff <(grep -r "^ - |$" tests/java/datanucleus-hardcoded-connection-password-java-test.yml | wc -l) <(grep -r "^ ? |$" tests/__snapshots__/datanucleus-hardcoded-connection-password-java-snapshot.yml | wc -l)
Length of output: 323
Script:
#!/bin/bash
# Let's first check if the test file exists and examine its content
fd -e yml datanucleus-hardcoded-connection-password-java-test
# Then let's look at the content of both files to verify the test cases and snapshots
echo "=== Test file content ==="
cat tests/java/datanucleus-hardcoded-connection-password-java-test.yml || true
echo -e "\n=== Snapshot file content ==="
cat tests/__snapshots__/datanucleus-hardcoded-connection-password-java-snapshot.yml || true
Length of output: 5870
rules/java/security/hardcoded-connection-password-java.yml (1)
1-14: LGTM! Well-documented security rule with proper references.
The rule header is well-structured with:
- Clear severity level
- Comprehensive security message
- Relevant references to OWASP and JDO documentation
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation