Add YAML rules and tests for insecure JWT usage detection #182
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 YAML files to define security rules for identifying insecure JWT usage across Go, Kotlin, and Scala projects. The rules target the use of the 'none' signing algorithm and hardcoded secrets. In addition to the configuration files, new snapshot files and test cases have been added to demonstrate and validate the detection of these vulnerabilities through AST parsing and pattern matching. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Scanner as Security Scanner
participant AST as AST Parser
participant Rule as Security Rule Engine
Dev->>Scanner: Commit new security rule configurations
Scanner->>AST: Parse source code & snapshots
AST->>Rule: Evaluate JWT patterns (e.g., 'none' algorithm, hardcoded secrets)
Rule-->>Scanner: Return vulnerability warnings
Scanner->>Dev: Report detected security issues
sequenceDiagram
participant Tester as Test Runner
participant Config as Test Config Files
participant Rule as Security Rule Engine
Tester->>Config: Load test scenarios
Config->>Rule: Execute JWT validations
Rule-->>Tester: Return test outcomes (valid/invalid)
Tester->>Dev: Output test results and logs
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: 0
🧹 Nitpick comments (21)
tests/kotlin/jwt-hardcode-kotlin-test.yml (2)
7-23: Demonstration of Insecure JWT Creation.
The invalid snippet intentionally shows insecure usage by hardcoding the secret (i.e., usingAlgorithm.HMAC256("secret")) and employing an empty catch block. This example is appropriate for testing the security rule, but consider adding an inline comment explaining that this insecure pattern is intentional and for demonstration only.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
20-20: Remove Trailing Whitespace.
Line 20 appears to have trailing space. Removing these spaces will help you pass YAML linting. For example, change:- } + }🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
tests/__snapshots__/jwt-go-none-algorithm-go-snapshot.yml (1)
17-49: Review Label Definitions.
There are multiple label definitions (e.g., several entries forAlgorithm.HMAC256and"github.com/dgrijalva/jwt-go") which appear redundant. Please verify that these duplicates are intentional for the testing rules; if not, consider consolidating them for clarity.tests/go/jwt-go-none-algorithm-go-test.yml (3)
4-7: Trim Trailing Spaces in the Valid Block.
Trailing spaces appear on line 7. Removing them will help eliminate YAML linting errors. For example:- ) + )🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 7-7: trailing spaces
(trailing-spaces)
8-15: Ensure Consistent Variable Declarations in Valid Snippet.
In the valid block, the variables (claims,token,ss, anderr) are assigned using=rather than Go’s standard short variable declaration operator (:=). While this might be acceptable if the snippet is solely for pattern matching purposes, consider using proper variable declarations (e.g.,:=) for clarity if this code is ever executed.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 12-12: trailing spaces
(trailing-spaces)
12-12: Remove Trailing Spaces.
Line 12 shows trailing spaces which should be removed to meet YAML lint requirements.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 12-12: trailing spaces
(trailing-spaces)
tests/__snapshots__/jwt-hardcode-kotlin-snapshot.yml (1)
4-61: Review of Label Duplications.
The labels section contains several duplicate entries (for example, multiple entries for"secret"andAlgorithm.HMAC256). It might be beneficial to consolidate these if they serve the same detection purpose, unless the duplication is required by the downstream processing logic.rules/scala/security/jwt-scala-hardcode-scala.yml (1)
4-9: Improve Grammar and Clarity in the Message.
The message text could be refined for clarity. For instance, consider revising it to:"Hardcoded JWT secret or private key is used. This vulnerability leads to insufficiently protected credentials (see CWE-522). Consider using secure mechanisms (e.g., storing secrets in environment variables) to protect credentials."
rules/scala/security/scala-jwt-hardcoded-secret-scala.yml (1)
17-65: Well-Structured AST Utility ConfigurationThe “utils” section (lines 17–65) defines matchers (for HMAC256 and its no-import counterpart) using precise AST patterns and regex. These definitions seem robust; however, consider adding inline comments for future maintainability and to clarify your choice of regex boundaries.
rules/kotlin/security/jwt-hardcode-kotlin.yml (1)
19-560: Comprehensive AST Utility Configuration for KotlinThe “utils” section defines numerous matchers for various HMAC algorithms (HMAC256, HMAC384, HMAC512) and covers multiple use cases (with import checks, with identifier patterns, etc.). Although the configuration is very detailed—which is excellent for ensuring coverage—it might benefit from additional inline documentation to ease future updates or debugging.
tests/scala/scala-jwt-hardcoded-secret-scala-test.yml (2)
1-5: Well-Defined Test File StructureThe YAML test file is organized with an “id” and clearly separated “valid” and “invalid” sections. Note that the “valid” section is empty; consider adding positive examples where secure practices are followed to widen test coverage.
15-15: Style Improvement: Remove Trailing SpacesStatic analysis has flagged trailing spaces on lines 15, 32, and 49. Please remove these extra spaces to comply with YAML linting guidelines.
Also applies to: 32-32, 49-49
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
tests/__snapshots__/scala-jwt-hardcoded-secret-scala-snapshot.yml (3)
1-28: [Snapshot "App" Review – Insecure JWT Creation with HMAC256]
This snapshot clearly demonstrates the creation of a JWT in theAppclass usingAlgorithm.HMAC256("secret"), which includes a hardcoded secret. The labels pinpoint the exact substring occurrences (e.g. the secret"secret", the entire call, and the import statement).
Suggestion: The labels for the import statement are duplicated. If these duplicates are not intentional (for example, to capture different contexts), consider consolidating them for clarity and maintainability.
29-54: [Snapshot "AuthService" Review – Insecure JWT Creation with HMAC384]
The snapshot for theAuthServiceclass properly illustrates the insecure use of a hardcoded secret viaAlgorithm.HMAC384("secretKey"). The labels correctly capture both the primary insecure pattern and secondary details (such as the literal"secretKey").
Observation: Similar to the previous snapshot, there are multiple identical labels for the import statement. Reviewing whether the redundancy is needed could simplify the snapshot file.
55-81: [Snapshot "SessionService" Review – Insecure JWT Creation with HMAC512]
This snapshot demonstrates the creation of a session token in theSessionServiceclass usingAlgorithm.HMAC512("secretKey"). The error handling (returning an empty string on exception) is also captured. The labels accurately mark the occurrence of the hardcoded secret and the algorithm call.
Suggestion: As before, if the repeated import labels are not serving different identification purposes, consider deduplication for improved clarity.tests/__snapshots__/jwt-scala-hardcode-scala-snapshot.yml (6)
1-26: [Snapshot "Test6" Review – JWT Encoding with HS256]
TheTest6snapshot illustrates how a JWT token is generated usingJwtJson.encodewith the HS256 algorithm and a hardcoded secret"secretKey". The JSON claim construction (with fields like"user"and"nbf") is clear and well demonstrated.
Observation: The labels include redundant import definitions. Confirm that these duplicates are intentional; otherwise, remove unnecessary repetitions to reduce noise.
27-36: [Snapshot "Test7" Review – JWT Decoding Using decodeJson]
This snapshot forTest7shows decoding a JWT usingJwtJson.decodeJson. The code snippet and the accompanying labels are very detailed—covering the primary call, helper token details, and even providing an alternative code block representation.
Observation: While the comprehensive labeling ensures thorough coverage, the repeated labels (especially the duplicate import lines and multiple entries specifyingsecretKey) could be streamlined for easier maintenance.Also applies to: 37-97
99-107: [Snapshot "Test9" Review – Decoding Raw JWT]
TheTest9snapshot demonstrates the use ofJwtJson.decodeRawto decode a token using the HS256 algorithm and the same hardcoded secret. The snapshot and its labels (which detail the method call and its parameters) are consistent with other snapshots in terms of structure.
Note: The approach is consistent; only minor style improvements (such as reducing duplicate import label entries) might help maintainability.Also applies to: 108-168
169-222: [Snapshot "Test1" Review – Object-Based JWT Encoding]
This snapshot forTest1encapsulates JWT encoding within an object usingJwtJson.encode. The structure and formatting are clear, and the labels capture all relevant elements such as the encoding call and the hardcoded secret usage.
Suggestion: As in previous snapshots, consider reducing the duplicate import labels and repeated mentions of"secretKey"to improve readability.
303-311: [Snapshot "Test2" Review – Decoding JWT to JSON]
TheTest2snapshot shows another instance of JWT decoding usingJwtJson.decodeJsonwith a token and the hardcoded key. The labels again capture all necessary details.
Observation: Consistency is maintained throughout; however, the repeated identical labels (especially for the import statement and secret variable) suggest an opportunity to simplify the snapshot metadata.Also applies to: 312-372
444-452: [Snapshot "Test5" Review – Decoding All with Literal Secret]
This final snapshot forTest5illustrates the use ofJwtJson.decodeAllby providing"secretKey"as a literal directly in the function call. This is slightly different from previous snapshots where the secret was stored in a variable.
Suggestion: For consistency across snapshots, consider standardizing the approach (i.e., always using a variable for the secret). Verify that this divergence is intentional for testing purposes.Also applies to: 453-485
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
rules/go/security/jwt-go-none-algorithm-go.yml(1 hunks)rules/kotlin/security/jwt-hardcode-kotlin.yml(1 hunks)rules/scala/security/jwt-scala-hardcode-scala.yml(1 hunks)rules/scala/security/scala-jwt-hardcoded-secret-scala.yml(1 hunks)tests/__snapshots__/jwt-go-none-algorithm-go-snapshot.yml(1 hunks)tests/__snapshots__/jwt-hardcode-kotlin-snapshot.yml(1 hunks)tests/__snapshots__/jwt-scala-hardcode-scala-snapshot.yml(1 hunks)tests/__snapshots__/scala-jwt-hardcoded-secret-scala-snapshot.yml(1 hunks)tests/go/jwt-go-none-algorithm-go-test.yml(1 hunks)tests/kotlin/jwt-hardcode-kotlin-test.yml(1 hunks)tests/scala/jwt-scala-hardcode-scala-test.yml(1 hunks)tests/scala/scala-jwt-hardcoded-secret-scala-test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
tests/kotlin/jwt-hardcode-kotlin-test.yml
[error] 20-20: trailing spaces
(trailing-spaces)
tests/go/jwt-go-none-algorithm-go-test.yml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 12-12: trailing spaces
(trailing-spaces)
tests/scala/jwt-scala-hardcode-scala-test.yml
[error] 16-16: trailing spaces
(trailing-spaces)
tests/scala/scala-jwt-hardcoded-secret-scala-test.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
🔇 Additional comments (18)
tests/kotlin/jwt-hardcode-kotlin-test.yml (1)
1-6: Valid Section Configuration Looks Good.
The valid block correctly sets system properties using the providedconfigvariable. Make sure that in production environments sensitive values are managed securely.tests/__snapshots__/jwt-go-none-algorithm-go-snapshot.yml (1)
1-16: Snapshot Demonstrates Usage of 'none' Algorithm.
This snapshot effectively shows the creation of a JWT token usingjwt.NewWithClaims(jwt.SigningMethodNone, claims)together withjwt.UnsafeAllowNoneSignatureType. This insecure pattern is intentionally demonstrated for testing purposes. Ensure that such snapshots remain confined to test environments and are not used in production.tests/go/jwt-go-none-algorithm-go-test.yml (1)
19-32: Insecure Usage in Invalid Block is Well-Illustrated.
The invalid snippet clearly demonstrates the insecure usage of JWT tokens by invokingjwt.NewWithClaimswithSigningMethodNoneand signing it usingjwt.UnsafeAllowNoneSignatureType. This is consistent with the intended test case.tests/__snapshots__/jwt-hardcode-kotlin-snapshot.yml (1)
1-3: Snapshot for Hardcoded JWT in Kotlin.
This snapshot correctly illustrates the insecure practice of using a hardcoded secret withAlgorithm.HMAC256("secret")in the Kotlin Auth0 JWT library. It aligns with the security rule requirements for detecting hardcoded secrets.rules/scala/security/jwt-scala-hardcode-scala.yml (1)
15-118: Comprehensive Rule Configuration.
The AST pattern definitions (bothPATTERNandPATTERN_with_Instance) are detailed and appear to cover the intended scenarios for detecting JWT usage patterns in Scala code. Ensure you test these patterns thoroughly against your codebase to confirm that they capture all insecure instances without generating false positives.rules/go/security/jwt-go-none-algorithm-go.yml (2)
1-16: Clear and Comprehensive Security MetadataThe rule’s metadata—including its ID, language, severity, detailed message, and references—is very well articulated. This clearly explains the risks of using the 'none' JWT algorithm and provides actionable recommendations.
17-44: Solid AST Parsing & Rule DefinitionThe AST configuration in the “utils” and “rule” sections is well structured. The regex patterns for detecting both
jwt.SigningMethodNoneandjwt.UnsafeAllowNoneSignatureTypeare clear and should effectively catch insecure usages.tests/scala/jwt-scala-hardcode-scala-test.yml (2)
1-11: Valid Test Case AnalysisThe “valid” section contains a test (class Test7) that uses HS256 with a hardcoded secret. Since hardcoded secrets are generally flagged as a vulnerability, please verify that this example is indeed intended to pass under the current security rule. If so, document the rationale to avoid confusion.
12-84: Comprehensive Invalid Test CasesThe “invalid” section offers a diverse set of examples (Test1, Test2, Test3, Test5, Test6, Test7, and Test15) that demonstrate insecure JWT practices using hardcoded secrets. This variety will be very helpful for validating the rule. Please ensure that each case triggers the rule as expected.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
rules/scala/security/scala-jwt-hardcoded-secret-scala.yml (2)
1-15: Robust Security Rule Metadata for ScalaThe header clearly defines the ID, severity, language, and provides a detailed message along with relevant references (e.g., CWE-522). The note solidly frames the issue and recommended guidance for handling secrets.
66-150: Effective Aggregated Rule DefinitionThe aggregated “rule” section skillfully combines multiple AST matchers to catch insecure hardcoded secrets within Scala classes. This comprehensive approach should improve detection accuracy. It would be beneficial to include unit tests (if not already present) to verify all matching scenarios.
rules/kotlin/security/jwt-hardcode-kotlin.yml (2)
1-16: Kotlin Security Rule Header is Well-DefinedThe rule’s metadata (ID, language, severity, message, and note) is clear and informative. The message and referenced standards (e.g., CWE-798) provide stakeholders with a solid understanding of the risk.
561-575: Aggregated Rule Section is ThoroughThe rule aggregation that follows consolidates all defined utility matchers, ensuring that any instance of a hardcoded secret in JWT usage is flagged. This approach is comprehensive and aligns well with the security objective.
tests/scala/scala-jwt-hardcoded-secret-scala-test.yml (3)
6-19: Invalid Test Case: AppThe first invalid test case (class App) correctly demonstrates an insecure JWT creation using
Algorithm.HMAC256("secret"). Exception handling is implemented (printing the error message), which is acceptable for a test scenario.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
20-36: Invalid Test Case: SessionServiceThis test case shows the use of
Algorithm.HMAC512with a hardcoded secret and handles exceptions by returning an empty string. Make sure that the chosen exception handling aligns with the intended behavior in a real-world scenario.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 32-32: trailing spaces
(trailing-spaces)
37-52: Invalid Test Case: AuthServiceThe AuthService test case uses
Algorithm.HMAC384and demonstrates an insecure practice; note that the catch block is empty. If the intent is merely to flag the vulnerability, consider returning a default or error value for consistency with other test cases.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 49-49: trailing spaces
(trailing-spaces)
tests/__snapshots__/jwt-scala-hardcode-scala-snapshot.yml (2)
223-232: [Snapshot "Test15" Review – Decoding All JWT Claims Using decodeAll]
Here, the snapshot forTest15demonstrates applyingJwtJson.decodeAllon a token with a slightly different approach by usingthis.secretKey. This variation is interesting as it implies usage within an object context. The detailed labels clearly mark the method call and its parameters.
Request: Please verify that the use ofthis.secretKeyis intentional and that it behaves as expected in the target environment.Also applies to: 233-301
374-382: [Snapshot "Test3" Review – Decoding JWT into JSON Format]
In this snapshot, theTest3object decodes a JWT into JSON. The snippet and its labels are consistent with the approach used in other snapshots.
Note: Everything appears correct here. The detailed labeling provides good traceability; just consider if some of the repeated information can be consolidated.Also applies to: 383-443
* removed missing-secure-java * httponly-false-csharp * use-of-md5-digest-utils-java * removing use-of-md5-digest-utils and httponly-false-csharp * jwt-scala-hardcode-scala * jwt-go-none-algorithm-go * changing folder location for jwt-go-none-algorithm-go * jwt-hardcode-kotlin * scala-jwt-hardcoded-secret-scala --------- Co-authored-by: Sakshis <sakshil@abc.com>
Summary by CodeRabbit