-
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 Swift Applications #46
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces two new security rules for Swift applications aimed at identifying hard-coded secrets. The first rule, 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 (
|
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: 12
🧹 Outside diff range and nitpick comments (6)
tests/swift/hkdf-hardcoded-secret-swift-test.yml (1)
2-4
: Consider adding more valid test cases.While the current valid test case correctly demonstrates proper usage with a non-hardcoded password, consider adding more valid cases to cover:
- Password retrieved from secure storage
- Password obtained from user input
- Password derived from environment variables
Would you like me to help generate additional valid test cases?
tests/__snapshots__/rabbit-hardcoded-secret-swift-snapshot.yml (1)
1-274
: Consider adding validation for common secret patterns.While the current test cases cover various Swift syntax patterns, they could be enhanced to detect common secret patterns.
Consider adding test cases that validate detection of:
- Base64 encoded strings
- Hexadecimal strings
- Common secret variable names (e.g.,
apiKey
,token
,password
)- Environment-specific secrets (e.g.,
devSecret
,prodKey
)This would make the security rule more effective at catching real-world scenarios where developers might inadvertently commit secrets.
tests/__snapshots__/hkdf-hardcoded-secret-swift-snapshot.yml (2)
73-180
: Consider adding test cases for additional variable patterns.While the current test cases cover basic variable assignment patterns, consider adding cases for:
- Constants (let) vs variables (var)
- Optional binding
- Guard statements
This would help ensure the security rule catches hardcoded secrets in all common Swift variable usage patterns.
1-250
: Consider adding test cases for additional security scenarios.While the current test cases provide good coverage, consider adding:
- Multi-line string literals
- String interpolation
- Computed properties
- Static properties
These additions would help ensure the security rule is robust against various ways developers might inadvertently include hardcoded secrets.
Would you like me to provide example test cases for these scenarios?
rules/swift/security/hkdf-hardcoded-secret-swift.yml (2)
4-13
: Enhance the security message with specific recommendations.The message effectively explains the risks of hard-coded secrets, but could be more specific about secure alternatives.
Consider adding these specific recommendations to the message:
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 (e.g., AWS Secrets Manager, HashiCorp Vault) + or Hardware Security Module (HSM). For development environments, consider using + tools like dotenv to manage environment variables securely.
1-291
: Consider adding test coverage metrics to the rule.While the rule implementation is solid, it would be beneficial to track its effectiveness.
Consider adding metadata fields for:
- Test coverage percentage
- False positive rate from test cases
- Known limitations or edge cases
This will help maintain and improve the rule over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- rules/swift/security/hkdf-hardcoded-secret-swift.yml (1 hunks)
- rules/swift/security/rabbit-hardcoded-secret-swift.yml (1 hunks)
- tests/snapshots/hkdf-hardcoded-secret-swift-snapshot.yml (1 hunks)
- tests/snapshots/rabbit-hardcoded-secret-swift-snapshot.yml (1 hunks)
- tests/swift/hkdf-hardcoded-secret-swift-test.yml (1 hunks)
- tests/swift/rabbit-hardcoded-secret-swift-test.yml (1 hunks)
🔇 Additional comments (13)
tests/swift/rabbit-hardcoded-secret-swift-test.yml (1)
1-19
: Verify alignment with security rule patterns.Let's ensure the test cases align with the patterns defined in the security rule file.
✅ Verification successful
Let me try to locate the security rule file using a different approach since the direct path wasn't found.
Now that we found the correct path, let's examine the security rule patterns and compare them with the test cases.
Test cases correctly align with security rule patterns
The test cases in
rabbit-hardcoded-secret-swift-test.yml
properly cover all the pattern variations defined in the security rule:
- Direct string literals with
try
(try Rabbit(key: "hello", iv: "123")
)- String literals with Array/utf8 conversion (
Rabbit(key: Array("hello".utf8))
)- Variable-based patterns with hardcoded strings (
let password = Array("s33krit".utf8)
)- Both try and non-try expressions
- Various combinations of the above patterns
The test file provides comprehensive coverage of the security rule's pattern matching capabilities, which aim to detect hardcoded secrets in Rabbit cipher initialization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare test cases with security rule patterns # Expected: All test cases should match at least one pattern in the rule file # Get the security rule patterns echo "Security rule patterns:" rg -A 5 "patterns:" "rules/swift/rabbit-hardcoded-secret-swift.yml" # Get the test cases echo -e "\nTest cases:" cat "tests/swift/rabbit-hardcoded-secret-swift-test.yml"Length of output: 727
Script:
#!/bin/bash # Search for any file containing "rabbit-hardcoded-secret-swift" in its name or content echo "Searching for related security rule files:" fd "rabbit.*swift" . echo -e "\nSearching for pattern definitions in any yaml file:" rg -l "rabbit.*hardcoded.*swift" . --type yamlLength of output: 635
Script:
#!/bin/bash # Get the security rule patterns from the correct path echo "Security rule patterns:" cat "./rules/swift/security/rabbit-hardcoded-secret-swift.yml" echo -e "\nTest cases:" cat "./tests/swift/rabbit-hardcoded-secret-swift-test.yml"Length of output: 9626
tests/swift/hkdf-hardcoded-secret-swift-test.yml (2)
1-2
: LGTM! Clear and descriptive test configuration ID.The test configuration ID accurately reflects its purpose for detecting hardcoded secrets in Swift HKDF implementations.
1-19
: Verify coverage against common Swift secret storage patterns.Let's ensure the test cases cover all common patterns where secrets might be stored in Swift code.
✅ Verification successful
Let me try a different approach to verify the test coverage by searching for Swift files first and then analyzing their content.
This will help us understand:
- If there are any Swift files in the codebase
- How HKDF is typically used
- If there are other key derivation patterns we should consider in our test cases
Based on the search results, I can now provide a final verification of the test cases. The search revealed that the test file comprehensively covers the common patterns of HKDF usage with hardcoded secrets in Swift, including:
Test coverage is comprehensive and well-structured
The test file adequately covers all major patterns of potential secret leakage:
- Direct string literals as passwords (
"123"
,"hello"
)- UTF8 array conversions of string literals (
Array("s33krit".utf8)
)- Variable declarations with type annotations (
Array<UInt8>
)- Both try and non-try variants of HKDF calls
- Both direct HKDF instantiation and variable assignment patterns
The test cases align well with the rule implementation in
rules/swift/security/hkdf-hardcoded-secret-swift.yml
, which defines matching patterns for all these scenarios.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing patterns of secret storage in Swift files echo "Searching for common secret storage patterns in Swift files..." # Look for string literal assignments that might contain secrets rg -t swift '(let|var)\s+\w*(password|secret|key|token|credential)\w*\s*=\s*["'\'']' # Look for byte array declarations that might contain secrets rg -t swift '(let|var)\s+\w*(password|secret|key|token|credential)\w*\s*:\s*(\[UInt8\]|Array<UInt8>)\s*=' # Look for base64 encoded secrets rg -t swift '\.base64Encoded\(\)|\.base64EncodedString\(\)'Length of output: 838
Script:
#!/bin/bash # First, find Swift files echo "Looking for Swift files..." fd -e swift # Then search for HKDF usage patterns in the codebase echo -e "\nSearching for HKDF usage patterns..." rg "HKDF" -A 2 -B 2 # Look for other cryptographic key derivation patterns echo -e "\nSearching for other key derivation patterns..." rg -i "key.*derivation|derive.*key|pbkdf|scrypt" -A 2 -B 2Length of output: 14107
tests/__snapshots__/rabbit-hardcoded-secret-swift-snapshot.yml (3)
1-2
: LGTM: Snapshot configuration follows naming conventions.The snapshot ID aligns with the security rule it's testing, following the established pattern for security-related snapshots.
39-72
: LGTM: Comprehensive coverage of Array-wrapped UTF8 string initialization.The test case properly validates the detection of hardcoded secrets when using Array initialization with UTF8 encoding, which is a common pattern in Swift cryptographic code.
122-173
: LGTM: Proper type annotation and error handling coverage.The test case effectively combines type annotations with try-wrapped initialization, providing good coverage for strongly-typed Swift code patterns.
tests/__snapshots__/hkdf-hardcoded-secret-swift-snapshot.yml (3)
1-34
: LGTM: Basic string literal test cases are well structured.The test case appropriately captures direct string literal usage ("123") in HKDF initialization, with comprehensive AST labeling for accurate detection. The labels correctly identify the password parameter and its value, enabling proper security rule matching.
35-72
: LGTM: UTF8 array conversion test cases provide good coverage.The test case properly handles the common pattern of converting string literals to byte arrays (Array("hello".utf8)). The labels correctly track both the original string and its array conversion, ensuring the security rule can detect hardcoded secrets in this form.
181-250
: LGTM: Error handling patterns are well covered.The test cases appropriately include try-catch wrapped HKDF calls, ensuring the security rule can detect hardcoded secrets regardless of error handling patterns. The labels maintain proper tracking of the password parameter through the try expression.
rules/swift/security/rabbit-hardcoded-secret-swift.yml (2)
273-285
: LGTM!The main rule structure effectively combines all utility patterns into a comprehensive check for both try expressions and call expressions.
1-285
: Verify rule effectiveness with test cases.Let's verify that the rule can detect various forms of hard-coded secrets in Swift code.
✅ Verification successful
Let's gather more information by examining the test files found to verify the rule's coverage and effectiveness.
Rule verification successful with comprehensive test coverage
The test files demonstrate that the rule effectively detects various patterns of hardcoded secrets in Swift code:
Direct string literals with try expression:
try Rabbit(key: "hello", iv: "123")
Direct string literals without try:
Rabbit(key: "hello", iv: "123")
Array UTF8 conversions with variable:
let password = Array("s33krit".utf8) Rabbit(key: password, iv: "123")Direct Array UTF8 conversions:
Rabbit(key: Array("hello".utf8))
try Rabbit(key: Array("hello".utf8))
Typed variable declarations:
let password: Array<UInt8> = Array("s33krit".utf8) try Rabbit(key: password, iv: "123")The snapshot file confirms that the rule correctly identifies and labels all components of these patterns, including the method calls, key parameters, and string literals. The test cases cover all the pattern matching utilities defined in the rule:
try_expression_directly
,Rabbit_statement_directly
,with_instance
, andwith_utf8
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for test files and verify rule coverage # Test: Look for test files related to this rule echo "Searching for test files..." fd -e swift -e yml "rabbit.*hardcoded.*secret" tests/ # Test: Check if there are examples of different patterns echo "Checking test coverage for different patterns..." for pattern in "try_expression_directly" "Rabbit_statement_directly" "with_instance" "with_utf8"; do echo "Checking coverage for pattern: $pattern" rg -l "rabbit.*hardcoded.*secret.*$pattern" tests/ doneLength of output: 1419
Script:
#!/bin/bash # Check the contents of the test files to verify test coverage # First, let's look at the test file content echo "=== Test file content ===" cat tests/swift/rabbit-hardcoded-secret-swift-test.yml echo -e "\n=== Snapshot content ===" cat tests/__snapshots__/rabbit-hardcoded-secret-swift-snapshot.yml # Let's also check if there are any actual Swift test files that might contain test cases echo -e "\n=== Swift test files ===" fd -e swift "rabbit.*hardcoded.*secret" tests/Length of output: 6609
rules/swift/security/hkdf-hardcoded-secret-swift.yml (2)
1-3
: LGTM! Rule metadata is well-defined.The rule is properly configured with an appropriate ID, warning severity level, and correct language specification.
279-291
: LGTM! Rule composition is well-structured.The main rule effectively combines all utility patterns to provide comprehensive coverage of different HKDF usage scenarios.
Summary by CodeRabbit
New Features
Tests