Add YAML security rules and tests for insecure secret usage #169
Conversation
WalkthroughThis pull request introduces several new YAML files that define security rules for detecting insecure secret usage in Python applications, specifically targeting HashIDs with Django, hardcoded secrets with Cassandra, and Couchbase authentication. Additionally, snapshot files and test configuration files are added to validate both secure and insecure patterns in these libraries, along with a Ruby snapshot for MySQL with an empty password. These changes enhance the rule set and associated tests for identifying cryptographic and credential management issues. Changes
Sequence Diagram(s)sequenceDiagram
participant Code as Application Code
participant Scanner as Security Scanner
participant Rule as Security Rule Engine
participant Tester as Test Suite
Code->>Scanner: Submit code for analysis
Scanner->>Rule: Evaluate patterns for insecure usage
alt Pattern Detected
Rule-->>Scanner: Report vulnerability (e.g., Django secret as salt)
Scanner->>Tester: Trigger corresponding test validations
Tester-->>Scanner: Return test results
Scanner->>Code: Generate warning alerts
else No Issues
Scanner->>Code: Confirm secure configuration
end
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/python/hashids-with-django-secret-python-test.yml (1)
14-18: Invalid Case 2 – SECRET_KEY with Minimum Length.
This block (lines 14–18) tests the scenario whereSECRET_KEYis used as salt along with a specifiedmin_length. Note that line 18 contains trailing spaces. Removing them will improve YAML formatting consistency.Suggested diff:
- hashid_2 = Hashids(salt=settings.SECRET_KEY, min_length=8) + hashid_2 = Hashids(salt=settings.SECRET_KEY, min_length=8)🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 18-18: trailing spaces
(trailing-spaces)
rules/python/security/hashids-with-django-secret-python.yml (1)
285-285: End-of-File Newline MissingPlease add a newline at the end of the file to comply with best practices and avoid linting errors.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 285-285: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
rules/python/security/hashids-with-django-secret-python.yml(1 hunks)rules/python/security/python-cassandra-hardcoded-secret-python.yml(1 hunks)rules/python/security/python-couchbase-hardcoded-secret-python.yml(1 hunks)tests/__snapshots__/hashids-with-django-secret-python-snapshot.yml(1 hunks)tests/__snapshots__/python-cassandra-hardcoded-secret-python-snapshot.yml(1 hunks)tests/__snapshots__/python-couchbase-hardcoded-secret-python-snapshot.yml(1 hunks)tests/__snapshots__/ruby-mysql2-empty-password-ruby-snapshot.yml(1 hunks)tests/python/hashids-with-django-secret-python-test.yml(1 hunks)tests/python/python-cassandra-hardcoded-secret-python-test.yml(1 hunks)tests/python/python-couchbase-hardcoded-secret-python-test.yml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/snapshots/python-couchbase-hardcoded-secret-python-snapshot.yml
- tests/snapshots/python-cassandra-hardcoded-secret-python-snapshot.yml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
tests/python/hashids-with-django-secret-python-test.yml
[error] 18-18: trailing spaces
(trailing-spaces)
rules/python/security/hashids-with-django-secret-python.yml
[warning] 53-53: wrong indentation: expected 18 but found 20
(indentation)
[warning] 64-64: wrong indentation: expected 18 but found 20
(indentation)
[warning] 79-79: wrong indentation: expected 12 but found 11
(indentation)
[warning] 81-81: wrong indentation: expected 13 but found 12
(indentation)
[warning] 98-98: wrong indentation: expected 18 but found 20
(indentation)
[warning] 109-109: wrong indentation: expected 18 but found 20
(indentation)
[warning] 140-140: wrong indentation: expected 10 but found 14
(indentation)
[warning] 147-147: wrong indentation: expected 18 but found 20
(indentation)
[warning] 162-162: wrong indentation: expected 12 but found 11
(indentation)
[warning] 164-164: wrong indentation: expected 13 but found 12
(indentation)
[warning] 181-181: wrong indentation: expected 18 but found 20
(indentation)
[warning] 216-216: wrong indentation: expected 12 but found 11
(indentation)
[warning] 230-230: wrong indentation: expected 12 but found 11
(indentation)
[warning] 233-233: wrong indentation: expected 10 but found 14
(indentation)
[warning] 240-240: wrong indentation: expected 18 but found 20
(indentation)
[warning] 264-264: wrong indentation: expected 10 but found 14
(indentation)
[warning] 271-271: wrong indentation: expected 18 but found 20
(indentation)
[warning] 277-277: wrong indentation: expected 2 but found 1
(indentation)
[error] 285-285: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (29)
tests/python/python-cassandra-hardcoded-secret-python-test.yml (2)
1-6: Valid and Insecure Test Cases Defined Clearly.
The valid test case (lines 1–6) correctly demonstrates that using an empty password withPlainTextAuthProvideris acceptable, while the invalid cases (lines 7–12) effectively cover scenarios where a hardcoded password is used. The YAML structure and indentation are clear and maintainable.
7-13: Coverage for Invalid Authentication Usage.
The two invalid cases ensure that both positional and keyword argument usages with non-empty (hardcoded) passwords are flagged. This comprehensive approach helps enforce secure practices by discouraging hardcoded secrets.tests/python/hashids-with-django-secret-python-test.yml (5)
1-7: Accurate Valid Test Case for Hashids Initialization.
The valid test case (lines 1–7) shows a proper instantiation ofHashidswith a custom salt and appropriatemin_length. This correctly demonstrates secure usage by avoiding reliance on Django’ssettings.SECRET_KEY.
9-13: Invalid Case 1 – Direct Use of SECRET_KEY as Salt.
The first invalid test block (lines 9–13) is well-defined and should trigger the security rule as intended whensettings.SECRET_KEYis used directly as the salt.
19-23: Invalid Case 3 – Positional SECRET_KEY with Minimum Length.
The test case (lines 19–23) usessettings.SECRET_KEYas a positional argument along with amin_lengthparameter to demonstrate misuse. This is clear and concise.
24-28: Invalid Case 4 – SECRET_KEY with Custom Alphabet.
This case (lines 24–28) tests the insecure instantiation usingsettings.SECRET_KEYand an invalid alphabet configuration. It complements the other invalid tests by covering multiple parameter combinations.
29-33: Invalid Case 5 – Combined Insecure Parameters.
The final invalid test case (lines 29–33) tests the usage of SECRET_KEY as salt with additional parameters (min_lengthandalphabet), ensuring the rule catches even more complex misconfigurations.tests/python/python-couchbase-hardcoded-secret-python-test.yml (3)
1-6: Valid Test Case for Dynamic Secret Retrieval.
The valid section (lines 1–6) correctly uses a dynamic secret via theget_pass()function when instantiatingPasswordAuthenticator. This effectively demonstrates secure secret management.
7-9: Invalid Test Case – Hardcoded Password Usage.
This invalid block (lines 7–9) shows a direct hardcoded password within the instantiation ofPasswordAuthenticator, which should be flagged by the security rule.
10-12: Invalid Test Case with Aliased Import.
The second invalid case (lines 10–12) verifies that even whenPasswordAuthenticatoris imported with an alias, the rule correctly detects hardcoded secrets.tests/__snapshots__/hashids-with-django-secret-python-snapshot.yml (6)
1-7: Comprehensive Snapshot for Hashids Configuration.
The snapshot (lines 1–7) captures an instantiation that improperly usessettings.SECRET_KEYas salt. The multiple labels detailing source components (lines 8–17) help in verifying the exact character positions and styling for the snapshot.
7-57: Detailed Snapshot Labeling and Multiple Representations.
The following snapshot block (lines 7–57) thoroughly documents various attributes of the insecure instantiation, including both primary and secondary labels. This detailed metadata aids in exact matching during tests.
58-107: Additional Snapshot Covering Extended Parameter Usage.
This snapshot segment (lines 58–107) shows an alternative insecure configuration usingmin_length=8. The detailed labels ensure that every component is tracked, and the structure aligns with the security rule requirements.
108-153: Snapshot for Positional SECRET_KEY Usage with Minimum Length.
The subsequent snapshot (lines 108–153) demonstrates the insecure use ofsettings.SECRET_KEYin a positional argument format with additional parameters. The labels consistently capture all relevant parts of the instantiation.
154-199: Snapshot for Insecure Use with Custom Alphabet.
This block (lines 154–199) highlights the misuse wheresettings.SECRET_KEYis combined with a custom alphabet. The snapshot is clear in its intent and the labels are specific to each component.
200-253: Snapshot Covering Combined Insecure Parameter Configuration.
The final snapshot block (lines 200–253) documents the configuration using both keyword salt and additional parameters, ensuring that the rule detects even complex insecure patterns.tests/__snapshots__/ruby-mysql2-empty-password-ruby-snapshot.yml (3)
1-8: Complete Snapshot for Ruby MySQL Client with Empty Password.
The first snapshot block (lines 1–8) sets up the environment by updating the load path, requiring the necessary libraries, and instantiating aMysql2::Clientwith an empty password. The query execution with a sleep function is an interesting addition that may simulate latency or overhead.
9-65: Well-Defined Labeling for Direct Password Usage.
The labels (lines 9–65) provide granular details about the instantiation parameters such as client creation, the use of an empty password, and the required modules. This aids in precise snapshot matching during tests.
66-144: Alternate Snapshot Using a Password Variable.
The second snapshot (lines 66–144) demonstrates the same client instantiation but with a variable (pw) representing the password. This alternative scenario increases test coverage and confirms that both direct and variable-based empty passwords are recognized.rules/python/security/hashids-with-django-secret-python.yml (3)
1-15: Metadata and Documentation are DetailedThe metadata for this rule (id, language, severity, message, and note) is clearly defined and provides helpful context with relevant security references.
16-275: Comprehensive Utility Pattern MatchingThe multiple utility definitions under the
utils:section cover a wide range of function‑call variations for initializing Hashids (with variations in salt usage and import styles). The detailed patterns should help in accurately detecting insecure usage across different developer styles.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 53-53: wrong indentation: expected 18 but found 20
(indentation)
[warning] 64-64: wrong indentation: expected 18 but found 20
(indentation)
[warning] 79-79: wrong indentation: expected 12 but found 11
(indentation)
[warning] 81-81: wrong indentation: expected 13 but found 12
(indentation)
[warning] 98-98: wrong indentation: expected 18 but found 20
(indentation)
[warning] 109-109: wrong indentation: expected 18 but found 20
(indentation)
[warning] 140-140: wrong indentation: expected 10 but found 14
(indentation)
[warning] 147-147: wrong indentation: expected 18 but found 20
(indentation)
[warning] 162-162: wrong indentation: expected 12 but found 11
(indentation)
[warning] 164-164: wrong indentation: expected 13 but found 12
(indentation)
[warning] 181-181: wrong indentation: expected 18 but found 20
(indentation)
[warning] 216-216: wrong indentation: expected 12 but found 11
(indentation)
[warning] 230-230: wrong indentation: expected 12 but found 11
(indentation)
[warning] 233-233: wrong indentation: expected 10 but found 14
(indentation)
[warning] 240-240: wrong indentation: expected 18 but found 20
(indentation)
[warning] 264-264: wrong indentation: expected 10 but found 14
(indentation)
[warning] 271-271: wrong indentation: expected 18 but found 20
(indentation)
276-285: Rule Definitions Accurately Aggregate PatternsThe
rule:section aggregates all the match patterns effectively, ensuring a broad detection scope for the insecure use of Django’sSECRET_KEYas a Hashids salt.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 277-277: wrong indentation: expected 2 but found 1
(indentation)
[error] 285-285: no new line character at the end of file
(new-line-at-end-of-file)
rules/python/security/python-cassandra-hardcoded-secret-python.yml (3)
1-10: Rule Metadata is Clear and InformativeThe metadata for the Cassandra hardcoded secret rule—including the id, language, severity, message, and notes—is clear and informative, providing proper context and references.
11-49: Utility Functions for Pattern Matching are Well DefinedThe
utils:section effectively defines thedefine_stringanddefine_passwordpatterns. These helper patterns will robustly support detecting hardcoded secrets in the subsequent rule logic.
50-133: Comprehensive Rule ImplementationThe rule section accurately captures both instantiation patterns for the
PlainTextAuthProviderand for theSaslAuthProvider. The use of nested pattern matching and import validations should provide reliable detection of hardcoded credentials.rules/python/security/python-couchbase-hardcoded-secret-python.yml (4)
1-10: Rule Metadata is Clear and InformativeThe metadata (id, language, severity, message, and note) is clearly defined and provides the necessary context along with useful security references for detecting hardcoded Couchbase secrets.
11-25: Utility Definition for String Patterns is Well StructuredThe
define_stringutility is straightforward and will reliably aid in detecting string patterns.
26-49: Utility Definition for Password Patterns is ComprehensiveThe
define_passwordpattern is defined clearly and in a way that should capture occurrences of hardcoded passwords using the provided identifier.
50-186: Comprehensive Rule ImplementationThe rule section covers multiple patterns for detecting hardcoded credentials tied to
PasswordAuthenticatorusage. It addresses both direct calls and those using aliased imports, along with validation of their import contexts. This comprehensive approach will help in catching a broad set of insecure patterns.
Summary by CodeRabbit
New Features
Tests