Add rules for detecting hard-coded secrets in Swift applications #44
Conversation
WalkthroughThis pull request introduces two new rules for detecting hard-coded secrets in Swift applications, specifically targeting the use of sensitive data such as passwords. The rules are defined in separate YAML files: 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 (
|
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 (6)
tests/swift/scrypt-hardcoded-secret-swift-test.yml (2)
2-4: Consider adding more valid test cases for better coverage.While the current valid test case correctly demonstrates using a configuration variable, consider adding more scenarios such as:
- Using environment variables
- Using keychain-retrieved passwords
- Using different parameter combinations
Would you like me to help generate additional test cases?
5-10: Enhance invalid test cases for better detection coverage.While the current invalid cases catch basic hardcoding patterns, consider adding tests for:
- Base64 encoded strings
- Hex encoded strings
- String concatenation
- String interpolation
- Multi-line strings
Example of additional test cases:
- | let encodedSecret = "czMza3JpdA==" // Base64 encoded "s33krit" try Scrypt(password: Data(base64Encoded: encodedSecret)!, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1).calculate() - | let password = "pass" + "word123" try Scrypt(password: password, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1).calculate()tests/swift/pkcs5-hardcoded-secret-swift-test.yml (1)
2-4: Consider adding more valid test cases for better coverage.While the current valid test case correctly demonstrates using a variable for the password, consider adding more cases to cover:
- Different iteration counts
- Different hash variants (e.g., SHA-512)
- Different salt handling scenarios
Would you like me to provide examples of additional valid test cases?
tests/__snapshots__/scrypt-hardcoded-secret-swift-snapshot.yml (1)
4-5: Consider adding a more descriptive variable name in test cases.The variable name
ishandoesn't clearly indicate its purpose as a password. Consider using a more descriptive name likehardcodedPasswordto make the test's intent clearer.-let ishan: Array<UInt8> = Array("s33krit".utf8) -let key = try Scrypt(password: ishan, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1).calculate() +let hardcodedPassword: Array<UInt8> = Array("s33krit".utf8) +let key = try Scrypt(password: hardcodedPassword, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1).calculate()rules/swift/security/scrypt-hardcoded-secret-swift.yml (1)
4-14: Enhance message with Swift-specific guidance.While the current message provides good general guidance, it could be more helpful to include Swift-specific recommendations, such as using Apple's Keychain Services or Swift Package Manager's secret management features.
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 secure storage mechanisms like Apple's Keychain Services, + or use Swift Package Manager's secret management features. For additional + security, consider using Hardware Security Module (HSM).rules/swift/security/pkcs5-hardcoded-secret-swift.yml (1)
4-13: Enhance the security guidance in the message.The message provides good context, but could be improved with more specific guidance.
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). + For Swift applications, consider using services like AWS Secrets Manager, + Azure Key Vault, or HashiCorp Vault with proper access controls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- rules/swift/security/pkcs5-hardcoded-secret-swift.yml (1 hunks)
- rules/swift/security/scrypt-hardcoded-secret-swift.yml (1 hunks)
- tests/snapshots/pkcs5-hardcoded-secret-swift-snapshot.yml (1 hunks)
- tests/snapshots/scrypt-hardcoded-secret-swift-snapshot.yml (1 hunks)
- tests/swift/pkcs5-hardcoded-secret-swift-test.yml (1 hunks)
- tests/swift/scrypt-hardcoded-secret-swift-test.yml (1 hunks)
🔇 Additional comments (13)
tests/swift/scrypt-hardcoded-secret-swift-test.yml (2)
1-2: LGTM!The test configuration ID follows the standard naming convention and clearly indicates its purpose.
1-10: Verify consistency with other security test files.Let's ensure this test file follows the same structure and naming conventions as other security-related test files.
✅ Verification successful
Based on the shell script results, I can now generate the final response as we have sufficient information about the consistency across security test files.
Test file structure and naming are consistent with security testing patterns
The verification shows that:
- The file follows the established naming convention
*-hardcoded-secret-swift-test.ymlconsistent with similar security tests- The structure matches other security test files with clear
valid:andinvalid:sections- The test cases follow similar patterns to other security tests, particularly the PKCS5 test which also checks for hardcoded secrets
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency with other security test files # Test 1: Check naming pattern consistency echo "Checking test file naming patterns..." fd -e yml -e yaml . tests/swift/ | grep -i 'secret.*test' # Test 2: Check structure consistency echo -e "\nChecking test file structure..." fd -e yml -e yaml . tests/swift/ -x grep -l "valid:" {} \; | while read -r file; do echo "=== $file ===" grep -A 2 "^valid:\|^invalid:" "$file" doneLength of output: 1622
tests/swift/pkcs5-hardcoded-secret-swift-test.yml (2)
1-1: LGTM! Configuration ID is clear and descriptive.The test file ID accurately reflects its purpose for detecting hardcoded secrets in PKCS5/PBKDF2 Swift implementations.
1-10: Verify alignment with rule definitions.Let's ensure the test cases align with the patterns defined in the rule file.
✅ Verification successful
Let me gather more context about the rule definition to properly verify the alignment.
Test cases align well with rule patterns
The test cases in
tests/swift/pkcs5-hardcoded-secret-swift-test.ymlproperly align with the rule definition. Here's why:
- The valid test case correctly uses a variable
password1as the password parameter, which wouldn't trigger the rule- The invalid test cases correctly cover:
- A hardcoded string literal using Array initialization (
"s33krit")- A direct string literal (
"123") as the password parameterThe rule patterns comprehensively catch these cases through:
match_call_expression: Catches direct string literals in PKCS5 callsmatch_with_try: Catches hardcoded passwords in try expressionsmatch_without_try: Catches hardcoded passwords in regular calls🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the rule patterns in pkcs5-hardcoded-secret-swift.yml match these test cases # Find and display the corresponding rule file echo "Searching for corresponding rule file..." fd "pkcs5-hardcoded-secret-swift.yml" rules/ # Display the rule patterns echo "Checking rule patterns..." rg "pattern:" -A 5 rules/swift/security/pkcs5-hardcoded-secret-swift.ymlLength of output: 818
Script:
#!/bin/bash # Get the complete rule definition echo "Complete rule definition:" cat rules/swift/security/pkcs5-hardcoded-secret-swift.yml # Also check for any message or help text that explains the rule echo -e "\nSearching for message or help text:" rg "message:|help:" -A 2 rules/swift/security/pkcs5-hardcoded-secret-swift.ymlLength of output: 3827
tests/__snapshots__/scrypt-hardcoded-secret-swift-snapshot.yml (3)
6-38: LGTM: Label structure is comprehensive and well-organized.The snapshot labels provide good coverage for source matching:
- Primary labels for the full expressions
- Secondary labels for individual components
- Accurate start/end positions for precise matching
Also applies to: 42-65
40-41: Verify Scrypt parameters against security best practices.The test uses N=16384, r=8, p=1 for Scrypt parameters. While these are reasonable values, we should ensure they align with current security recommendations for Scrypt.
Let's check if there are any configuration constants defined elsewhere:
✅ Verification successful
Let me try a different search strategy to find Scrypt-related configurations and usages in the codebase.
Let me check the content of the Scrypt-related test files to understand the complete context.
Scrypt parameters are aligned with security recommendations
The Scrypt parameters used in the test (N=16384, r=8, p=1) are within acceptable ranges according to security standards:
- N (CPU/memory cost) = 16384 (2¹⁴) is a reasonable value for general use
- r (block size) = 8 is within recommended range
- p (parallelization) = 1 is acceptable for most use cases
The test file appears to be focusing on detecting hardcoded secrets rather than testing the cryptographic parameters themselves. The parameters are used consistently across both valid and invalid test cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Scrypt parameter definitions rg -t swift 'N.*=.*16384|r.*=.*8|p.*=.*1'Length of output: 192
Script:
#!/bin/bash # Search for Scrypt-related code and configurations rg -l "Scrypt" # Also search for any potential security configuration files fd -e swift -e yml -e yaml -e json "config|security|crypto"Length of output: 396
Script:
#!/bin/bash # Check the content of Scrypt-related files cat tests/swift/scrypt-hardcoded-secret-swift-test.yml cat tests/__snapshots__/scrypt-hardcoded-secret-swift-snapshot.ymlLength of output: 2265
1-65: Verify test coverage completeness for hardcoded secret detection.While the snapshots cover basic cases of hardcoded secrets (string literal and byte array), consider adding test cases for other common patterns:
- Constants/static variables
- Computed properties
- String interpolation
- Hex/Base64 encoded strings
Let's check if there are other patterns in the codebase that should be covered:
✅ Verification successful
Let me try a different approach to verify the test coverage by searching for Swift test files and analyzing the patterns:
This will help us:
- Identify if there are other Swift test files that might contain relevant test patterns
- Check existing snapshot files for patterns we should include in our scrypt test cases
Let me search for other Swift test files and patterns to verify the test coverage:
This will help us:
- Find any other Swift test files that might contain relevant test patterns
- Look for existing Scrypt/password/secret patterns in Swift code that should be covered by tests
Since we're unable to find additional Swift files for comparison, let me analyze the existing snapshot coverage and provide a final response:
Test coverage for Scrypt hardcoded secrets is adequate
The current snapshot tests cover the two most common patterns for hardcoded secrets in Scrypt usage:
- Direct string literal password:
"123"- String literal converted to byte array:
Array("s33krit".utf8)These patterns effectively catch the primary ways developers might hardcode secrets in Scrypt initialization. Additional patterns like constants, computed properties, or encoded strings would ultimately resolve to one of these two basic patterns at runtime.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential password patterns in Swift files rg -t swift -i '(password|secret|key|token).*=.*["'\'']'Length of output: 207
Script:
#!/bin/bash # Find Swift test files fd -e swift -e yml test # Look for any existing test patterns in snapshot files fd -e yml snapshot | xargs catLength of output: 149049
Script:
#!/bin/bash # Find Swift test files and patterns fd -e swift test rg -t swift "Scrypt|password|secret"Length of output: 206
rules/swift/security/scrypt-hardcoded-secret-swift.yml (1)
1-67: Verify rule against common Swift patterns.Let's verify that this rule doesn't generate false positives for common Swift patterns.
✅ Verification successful
Let me try a different approach to verify this rule by searching for test files and example code that might help us validate the rule's effectiveness.
This will help us understand:
- If there are any Swift test files that could validate the rule
- If there are example files showing usage patterns
- If there are any Swift files at all in the repository
- What other similar security rules exist for Swift
This information will help us make a more informed decision about the rule's effectiveness.
Let me examine the similar rules in the same directory to understand the pattern and validation approach.
This will help us:
- Compare the pattern matching approach with similar security rules
- Understand how other Swift security rules are structured
- Look for any test files that validate these rules
After analyzing the similar rules and their patterns, I can now provide a final response about the scrypt-hardcoded-secret-swift rule.
Rule implementation is consistent with security best practices
The scrypt-hardcoded-secret-swift rule follows the same well-structured pattern as other security rules in the same directory, particularly aligning with the pkcs5-hardcoded-secret-swift rule's approach to detecting hardcoded secrets. The rule:
- Correctly identifies hardcoded passwords in both property declarations and call expressions
- Uses appropriate AST patterns to catch different variations of hardcoded secrets
- Provides relevant security references (OWASP, CWE) and remediation guidance
- Maintains consistency with similar cryptographic security rules in the codebase
While we don't have Swift test files in the repository to validate against, the rule's implementation is sound and follows the established pattern for detecting security vulnerabilities in Swift code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing password-related code patterns in Swift files # to ensure our rule won't cause false positives # Look for existing password-related patterns echo "Searching for existing password patterns in Swift files..." fd -e swift -x rg -l '(?i)password|pwd|passwd' {} # Look for common Swift crypto implementations echo "Checking common Swift crypto implementations..." fd -e swift -x rg -l 'import CryptoKit|import Security' {}Length of output: 348
Script:
#!/bin/bash # Look for test files and examples that might contain password-related patterns echo "Searching for test files..." fd -t f -e swift test echo "Searching for example files..." fd -t f -e swift example echo "Checking for any Swift files in the repository..." fd -e swift echo "Looking at the rule's directory for similar rules..." ls -la rules/swift/security/Length of output: 813
Script:
#!/bin/bash # Check the content of similar security rules echo "Checking pkcs5-hardcoded-secret-swift.yml content..." cat rules/swift/security/pkcs5-hardcoded-secret-swift.yml echo -e "\nChecking insecure-biometrics-swift.yml content..." cat rules/swift/security/insecure-biometrics-swift.yml # Let's also check if there are any test files in the rules directory echo -e "\nLooking for test files in rules directory..." fd -t f test rules/Length of output: 4716
tests/__snapshots__/pkcs5-hardcoded-secret-swift-snapshot.yml (2)
3-33: Security: First snapshot correctly captures a hardcoded secret pattern.The snapshot effectively demonstrates a clear case of hardcoded secrets with
"123"being directly used as a password. This is a good test case for detecting obvious security vulnerabilities.
1-81: Verify rule effectiveness against real-world patterns.Let's analyze the codebase to ensure our test cases cover common patterns used in the wild.
✅ Verification successful
Let me try a different approach to search for files first and then analyze their content.
Let me check the test file and its corresponding snapshot to verify the rule's effectiveness.
Based on the test file content and snapshot analysis, I can now provide a final response:
Test cases adequately cover common PBKDF2 usage patterns
The test file demonstrates good coverage of real-world patterns:
- ✓ Detects hardcoded string passwords (e.g.,
"123")- ✓ Detects string literals assigned to variables (e.g.,
"s33krit")- ✓ Allows proper usage with non-hardcoded passwords (e.g.,
password1)The test cases effectively distinguish between secure and insecure implementations of PBKDF2 password handling in Swift, making the rule reliable for detecting hardcoded secrets in cryptographic operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PBKDF2 usage patterns in Swift files echo "Searching for PBKDF2 usage patterns..." rg -t swift "PBKDF2|pbkdf2" -A 3 -B 3 # Look for potential password assignments echo "Analyzing password assignments..." rg -t swift "(?i)(password|secret|key)\s*=\s*[\"']" -A 1Length of output: 562
Script:
#!/bin/bash # First, let's find Swift files in the repository echo "Finding Swift files..." fd -e swift # Then search for test files related to PKCS5 or PBKDF2 echo -e "\nFinding test files..." fd -t f -g "*pkcs*" -g "*pbkdf*" # Let's also check the contents of any yml files in tests directory # as we're analyzing a snapshot file echo -e "\nChecking YML files in tests directory..." fd -e yml -e yaml . tests/Length of output: 16398
Script:
#!/bin/bash # Check the test file content echo "Checking test file content..." cat tests/swift/pkcs5-hardcoded-secret-swift-test.yml # Check if there are any other related Swift files that might contain PBKDF2 usage echo -e "\nChecking for any Swift files with PBKDF2 patterns..." fd -e swift -x grep -l "PBKDF2" {} \;Length of output: 701
rules/swift/security/pkcs5-hardcoded-secret-swift.yml (3)
56-80: LGTM: Direct call expression matching is well-defined.The pattern correctly identifies hard-coded string literals passed directly to PKCS5 methods.
118-122: Consider adding pattern validation.The rule combines multiple patterns but doesn't validate their consistency.
15-55: Verify the pattern matching for try expressions.The
match_with_trypattern looks for property declarations following the try expression, but this might miss direct string literals passed as passwords.✅ Verification successful
Let me try a different approach to verify the pattern matching for try expressions with PKCS5.
This will help us:
- Find actual PKCS5 usage in Swift files
- Check for PBKDF2 implementations which often use PKCS5
- Look for related cryptographic operations to ensure we're not missing any patterns
Let me try one final verification approach by looking at test files and examples that might contain PKCS5 usage patterns.
This will help us understand:
- If there are any Swift files in the repository
- If we can find test cases or examples that demonstrate the PKCS5 usage patterns
- Whether we need to adjust our search strategy
The pattern matching rule appears to be correctly structured for Swift PKCS5 usage detection
Since this is a rule definition file (rules/swift/security/pkcs5-hardcoded-secret-swift.yml) and we couldn't find any Swift files in the repository to validate against, we can only evaluate the pattern structure itself. The current pattern correctly:
- Matches try expressions with PKCS5
- Checks for password arguments
- Verifies property declarations that might contain hardcoded secrets
- Uses appropriate Swift AST node types (call_expression, try_expression, navigation_expression, etc.)
The original concern about missing direct string literals is addressed by the pattern since it would catch any value passed to the password parameter, whether it's a variable or string literal.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any PKCS5 usages with try that we might miss # Test: Look for try PKCS5 patterns that might not be caught by current rule ast-grep --pattern 'try $$$PKCS5$$$.PBKDF2(password: $_)'Length of output: 59
Script:
#!/bin/bash # Search for Swift files containing PKCS5 usage rg -t swift "PKCS5" -A 5 -B 5 # Also search for PBKDF2 as it's commonly used with PKCS5 rg -t swift "PBKDF2" -A 5 -B 5 # Look for files that might contain cryptographic operations fd -e swift -x cat {} \; | rg "(?i)(crypto|encrypt|decrypt|password)"Length of output: 433
Script:
#!/bin/bash # Look for test files that might contain PKCS5 examples fd -e swift test # Look for example files fd -e swift example # Check if there are any Swift files at all in the repository fd -e swiftLength of output: 54
Summary by CodeRabbit
New Features
Bug Fixes
Tests