-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add security rules for detecting hard-coded secrets in C# and Python #101
base: main
Are you sure you want to change the base?
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 in C# and Python applications. Specifically, it adds rules targeting the 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 (
|
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (8)
rules/csharp/security/npgsqlconnectionstringbuilder-hardcoded-secret-csharp.yml (1)
242-246
: Consider extending the rule to cover additional scenarios.The rule effectively catches direct assignments and indexer access but might miss other scenarios:
- Constructor parameter assignments
- Builder pattern assignments
- Connection string parsing
Would you like me to provide patterns for these additional scenarios?
tests/csharp/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml (1)
48-48
: Remove trailing whitespace.Line 48 contains trailing spaces.
- string password = "aaa"; + string password = "aaa";🧰 Tools
🪛 yamllint (1.35.1)
[error] 48-48: trailing spaces
(trailing-spaces)
tests/python/python-peewee-pg-hardcoded-secret-python-test.yml (1)
5-24
: Enhance test coverageThe invalid test cases don't cover all database classes mentioned in the rule. Consider adding test cases for:
peewee.PostgresqlDatabase
playhouse.postgres_ext.PostgresqlExtDatabase
playhouse.pool.PooledPostgresqlDatabase
playhouse.cockroachdb.CockroachDatabase
rules/python/security/python-peewee-pg-hardcoded-secret-python.yml (3)
4-13
: Enhance the security messageConsider adding more context to the message:
- Mention specific alternatives (e.g., AWS Secrets Manager, HashiCorp Vault)
- Add examples of secure usage
- Reference database-specific security documentation
14-202
: Improve rule maintainabilityThe rule patterns contain duplicated regex patterns for database classes. Consider extracting common patterns into reusable variables.
+variables: + $DB_CLASSES: ^PostgresqlDatabase|peewee.PostgresqlDatabase|PostgresqlExtDatabase|playhouse.postgres_ext.PostgresqlExtDatabase|PooledPostgresqlDatabase|playhouse.pool.PooledPostgresqlDatabase|CockroachDatabase|playhouse.cockroachdb.CockroachDatabase|PooledCockroachDatabase|playhouse.cockroachdb.PooledCockroachDatabase$ utils: $DB(..., password="...",...): kind: call all: - has: stopBy: neighbor pattern: $DB - regex: ^PostgresqlDatabase|peewee.PostgresqlDatabase|PostgresqlExtDatabase|playhouse.postgres_ext.PostgresqlExtDatabase|PooledPostgresqlDatabase|playhouse.pool.PooledPostgresqlDatabase|CockroachDatabase|playhouse.cockroachdb.CockroachDatabase|PooledCockroachDatabase|playhouse.cockroachdb.PooledCockroachDatabase$ + regex: $DB_CLASSES🧰 Tools
🪛 yamllint (1.35.1)
[warning] 30-30: wrong indentation: expected 10 but found 12
(indentation)
[warning] 38-38: wrong indentation: expected 18 but found 19
(indentation)
[warning] 55-55: wrong indentation: expected 10 but found 12
(indentation)
[error] 67-67: trailing spaces
(trailing-spaces)
[warning] 71-71: wrong indentation: expected 12 but found 14
(indentation)
[warning] 74-74: wrong indentation: expected 14 but found 16
(indentation)
[warning] 82-82: wrong indentation: expected 22 but found 23
(indentation)
[warning] 92-92: wrong indentation: expected 8 but found 10
(indentation)
[warning] 107-107: wrong indentation: expected 10 but found 12
(indentation)
[warning] 115-115: wrong indentation: expected 18 but found 19
(indentation)
[warning] 127-127: wrong indentation: expected 12 but found 14
(indentation)
[warning] 142-142: wrong indentation: expected 8 but found 10
(indentation)
[warning] 157-157: wrong indentation: expected 10 but found 12
(indentation)
[warning] 176-176: wrong indentation: expected 12 but found 14
(indentation)
[error] 186-186: trailing spaces
(trailing-spaces)
[warning] 190-190: wrong indentation: expected 12 but found 14
(indentation)
[warning] 193-193: wrong indentation: expected 14 but found 16
(indentation)
[warning] 201-201: wrong indentation: expected 22 but found 23
(indentation)
1-209
: Fix YAML indentation issuesThe file contains multiple indentation inconsistencies. Please fix the indentation to follow YAML best practices.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 30-30: wrong indentation: expected 10 but found 12
(indentation)
[warning] 38-38: wrong indentation: expected 18 but found 19
(indentation)
[warning] 55-55: wrong indentation: expected 10 but found 12
(indentation)
[error] 67-67: trailing spaces
(trailing-spaces)
[warning] 71-71: wrong indentation: expected 12 but found 14
(indentation)
[warning] 74-74: wrong indentation: expected 14 but found 16
(indentation)
[warning] 82-82: wrong indentation: expected 22 but found 23
(indentation)
[warning] 92-92: wrong indentation: expected 8 but found 10
(indentation)
[warning] 107-107: wrong indentation: expected 10 but found 12
(indentation)
[warning] 115-115: wrong indentation: expected 18 but found 19
(indentation)
[warning] 127-127: wrong indentation: expected 12 but found 14
(indentation)
[warning] 142-142: wrong indentation: expected 8 but found 10
(indentation)
[warning] 157-157: wrong indentation: expected 10 but found 12
(indentation)
[warning] 176-176: wrong indentation: expected 12 but found 14
(indentation)
[error] 186-186: trailing spaces
(trailing-spaces)
[warning] 190-190: wrong indentation: expected 12 but found 14
(indentation)
[warning] 193-193: wrong indentation: expected 14 but found 16
(indentation)
[warning] 201-201: wrong indentation: expected 22 but found 23
(indentation)
[warning] 206-206: wrong indentation: expected 2 but found 4
(indentation)
tests/__snapshots__/python-pg8000-hardcoded-secret-python-snapshot.yml (1)
1-214
: LGTM! Well-structured snapshots with precise labeling.The snapshots effectively capture all test cases with detailed position information and style markers.
Consider fixing the YAML indentation issues flagged by yamllint for better maintainability.
rules/python/security/python-pg8000-hardcoded-secret-python.yml (1)
4-14
: LGTM! Clear message with proper security context.The message and documentation effectively:
- Explain the security risk
- Provide mitigation strategies
- Include relevant CWE reference
- Link to OWASP guidelines
Consider adding code examples in the documentation to illustrate both the vulnerable and secure patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
rules/csharp/security/npgsqlconnectionstringbuilder-hardcoded-secret-csharp.yml
(1 hunks)rules/python/security/python-peewee-pg-hardcoded-secret-python.yml
(1 hunks)rules/python/security/python-pg8000-hardcoded-secret-python.yml
(1 hunks)tests/__snapshots__/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-snapshot.yml
(1 hunks)tests/__snapshots__/python-peewee-pg-hardcoded-secret-python-snapshot.yml
(1 hunks)tests/__snapshots__/python-pg8000-hardcoded-secret-python-snapshot.yml
(1 hunks)tests/csharp/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml
(1 hunks)tests/python/python-peewee-pg-hardcoded-secret-python-test.yml
(1 hunks)tests/python/python-pg8000-hardcoded-secret-python-test.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
tests/csharp/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml
[error] 48-48: trailing spaces
(trailing-spaces)
rules/python/security/python-pg8000-hardcoded-secret-python.yml
[warning] 31-31: wrong indentation: expected 10 but found 12
(indentation)
[warning] 39-39: wrong indentation: expected 18 but found 19
(indentation)
[warning] 56-56: wrong indentation: expected 10 but found 12
(indentation)
[error] 68-68: trailing spaces
(trailing-spaces)
[warning] 72-72: wrong indentation: expected 12 but found 14
(indentation)
[warning] 75-75: wrong indentation: expected 14 but found 16
(indentation)
[warning] 83-83: wrong indentation: expected 22 but found 23
(indentation)
[warning] 100-100: wrong indentation: expected 10 but found 12
(indentation)
[warning] 108-108: wrong indentation: expected 18 but found 19
(indentation)
[warning] 115-115: wrong indentation: expected 12 but found 11
(indentation)
[warning] 118-118: wrong indentation: expected 11 but found 12
(indentation)
[warning] 135-135: wrong indentation: expected 10 but found 12
(indentation)
[warning] 151-151: wrong indentation: expected 10 but found 12
(indentation)
[warning] 163-163: wrong indentation: expected 12 but found 14
(indentation)
[warning] 181-181: wrong indentation: expected 8 but found 10
(indentation)
[warning] 197-197: wrong indentation: expected 10 but found 12
(indentation)
[warning] 216-216: wrong indentation: expected 12 but found 14
(indentation)
[warning] 234-234: wrong indentation: expected 12 but found 14
(indentation)
[warning] 247-247: wrong indentation: expected 2 but found 4
(indentation)
rules/python/security/python-peewee-pg-hardcoded-secret-python.yml
[warning] 30-30: wrong indentation: expected 10 but found 12
(indentation)
[warning] 38-38: wrong indentation: expected 18 but found 19
(indentation)
[warning] 55-55: wrong indentation: expected 10 but found 12
(indentation)
[error] 67-67: trailing spaces
(trailing-spaces)
[warning] 71-71: wrong indentation: expected 12 but found 14
(indentation)
[warning] 74-74: wrong indentation: expected 14 but found 16
(indentation)
[warning] 82-82: wrong indentation: expected 22 but found 23
(indentation)
[warning] 92-92: wrong indentation: expected 8 but found 10
(indentation)
[warning] 107-107: wrong indentation: expected 10 but found 12
(indentation)
[warning] 115-115: wrong indentation: expected 18 but found 19
(indentation)
[warning] 127-127: wrong indentation: expected 12 but found 14
(indentation)
[warning] 142-142: wrong indentation: expected 8 but found 10
(indentation)
[warning] 157-157: wrong indentation: expected 10 but found 12
(indentation)
[warning] 176-176: wrong indentation: expected 12 but found 14
(indentation)
[error] 186-186: trailing spaces
(trailing-spaces)
[warning] 190-190: wrong indentation: expected 12 but found 14
(indentation)
[warning] 193-193: wrong indentation: expected 14 but found 16
(indentation)
[warning] 201-201: wrong indentation: expected 22 but found 23
(indentation)
[warning] 206-206: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (10)
rules/csharp/security/npgsqlconnectionstringbuilder-hardcoded-secret-csharp.yml (2)
1-14
: LGTM! Well-documented security rule with proper references.
The rule is well-documented with clear messaging and appropriate references to security standards (CWE-798, OWASP A07:2021).
100-108
:
Fix nested 'all' block in match_assignment_with_bracket pattern.
The nested 'all' block at line 100 appears to be misplaced and might affect pattern matching.
Apply this diff to fix the pattern:
all:
- - not:
- precedes:
- stopBy: end
- kind: element_access_expression
- - not:
- precedes:
- stopBy: end
- kind: identifier
+ - not:
+ precedes:
+ stopBy: end
+ kind: element_access_expression
+ - not:
+ precedes:
+ stopBy: end
+ kind: identifier
Likely invalid or redundant comment.
tests/csharp/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-test.yml (2)
2-6
: LGTM! Comprehensive valid test cases.
The valid test cases properly demonstrate secure password handling using command-line arguments.
8-52
: LGTM! Well-structured invalid test cases.
The invalid test cases effectively cover different patterns of hardcoded secrets:
- Direct property assignment
- Indexer access
- Variable-based assignment
🧰 Tools
🪛 yamllint (1.35.1)
[error] 48-48: trailing spaces
(trailing-spaces)
tests/__snapshots__/npgsqlconnectionstringbuilder-hardcoded-secret-csharp-snapshot.yml (1)
1-264
: LGTM! Well-structured snapshots with comprehensive labels.
The snapshots provide good coverage of the test cases and effectively identify key components through labels.
tests/__snapshots__/python-peewee-pg-hardcoded-secret-python-snapshot.yml (1)
1-340
: LGTM! Comprehensive test coverage
The snapshots provide thorough coverage of the rule patterns and include detailed source mapping for accurate testing.
tests/python/python-pg8000-hardcoded-secret-python-test.yml (2)
3-5
: LGTM! Valid test case demonstrates secure password management.
The test case correctly shows the recommended approach of using a function to retrieve the password dynamically rather than hardcoding it.
6-20
: LGTM! Comprehensive coverage of insecure patterns.
The invalid test cases effectively cover various scenarios:
- Direct password hardcoding in connection string
- Password stored in variables
- Different import styles (native and dbapi)
- Different connection methods
rules/python/security/python-pg8000-hardcoded-secret-python.yml (2)
245-251
: LGTM! Well-structured rule composition.
The rule effectively combines all pattern matchers using the 'any' condition, ensuring comprehensive coverage of different hardcoded secret patterns.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 247-247: wrong indentation: expected 2 but found 4
(indentation)
15-244
: LGTM! Comprehensive pattern matching for detecting hardcoded secrets.
The rule effectively covers:
- Direct string passwords in connection calls
- Variable-based password assignments
- Different import patterns (dbapi and native)
- Various connection method styles
Let's verify the pattern coverage against the codebase:
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 31-31: wrong indentation: expected 10 but found 12
(indentation)
[warning] 39-39: wrong indentation: expected 18 but found 19
(indentation)
[warning] 56-56: wrong indentation: expected 10 but found 12
(indentation)
[error] 68-68: trailing spaces
(trailing-spaces)
[warning] 72-72: wrong indentation: expected 12 but found 14
(indentation)
[warning] 75-75: wrong indentation: expected 14 but found 16
(indentation)
[warning] 83-83: wrong indentation: expected 22 but found 23
(indentation)
[warning] 100-100: wrong indentation: expected 10 but found 12
(indentation)
[warning] 108-108: wrong indentation: expected 18 but found 19
(indentation)
[warning] 115-115: wrong indentation: expected 12 but found 11
(indentation)
[warning] 118-118: wrong indentation: expected 11 but found 12
(indentation)
[warning] 135-135: wrong indentation: expected 10 but found 12
(indentation)
[warning] 151-151: wrong indentation: expected 10 but found 12
(indentation)
[warning] 163-163: wrong indentation: expected 12 but found 14
(indentation)
[warning] 181-181: wrong indentation: expected 8 but found 10
(indentation)
[warning] 197-197: wrong indentation: expected 10 but found 12
(indentation)
[warning] 216-216: wrong indentation: expected 12 but found 14
(indentation)
[warning] 234-234: wrong indentation: expected 12 but found 14
(indentation)
Summary by CodeRabbit
New Features
Tests