tests/javascript/express-session-hardcoded-secret-javascript-test.yml (3)

2-8: LGTM: Correct express-session configuration.

The valid configuration demonstrates good practices:

1. The secret is referenced from a separate config object, avoiding hardcoding.
2. 'resave' and 'saveUninitialized' are correctly set to false.

Consider adding a comment explaining the importance of storing the secret securely, e.g., in environment variables:

 let config1 = {
+  // Secret should be stored in environment variables in production
   secret: config.secret,
   resave: false,
   saveUninitialized: false,
 }

---

9-17: Approve invalid example with suggestions for improvement.

The invalid configuration correctly demonstrates the use of a hardcoded secret, which is the main purpose of this test case. However, there are a few improvements that could enhance clarity:

Consider the following changes:

1. Remove the unnecessary import statement and unused variable:

-import * as session from 'express-session'
-let a = 'a'
 let config = {
   secret: 'a',
   resave: false,
   saveUninitialized: false,
 }

2. Use a more realistic hardcoded secret to better illustrate the security risk:

 let config = {
-  secret: 'a',
+  secret: 'myHardcodedSecretKey123',
   resave: false,
   saveUninitialized: false,
 }

These changes will make the invalid example more focused and illustrative of the security issue being tested.

---

1-17: Well-structured test case for express-session configuration.

This test case effectively demonstrates both secure and insecure configurations for express-session. It provides clear examples of:

1. A valid configuration using a separate config object for the secret.
2. An invalid configuration with a hardcoded secret.

These examples will be valuable for detecting and preventing the use of hardcoded secrets in express-session configurations.

To further enhance this test case, consider:

1. Adding comments to explain why the valid configuration is secure and why the invalid one poses a security risk.
2. Including an example of loading the secret from an environment variable in the valid configuration to demonstrate best practices.

These additions would make the test case more educational and reinforce secure coding practices.

tests/__snapshots__/express-session-hardcoded-secret-javascript-snapshot.yml (2)

7-7: Highlight security implications of hardcoded secrets

The hardcoded secret 'a' in the session configuration accurately represents the security issue this rule aims to detect. In real-world applications, hardcoding secrets like this poses significant security risks:

1. Version Control Exposure: Secrets in source code can be exposed if the repository is compromised.
2. Difficulty in Rotation: Hardcoded secrets are challenging to rotate regularly.
3. Environment Inflexibility: It prevents easy configuration changes across different environments.

Best practices include:

• Using environment variables or secure secret management systems.
• Keeping secrets out of version control.
• Regularly rotating secrets.

Consider adding a comment in the snapshot to explicitly state that this is an example of what not to do in production code.

---

11-73: LGTM: Comprehensive and accurate labels

The labels provided are comprehensive and accurate, covering various aspects of the code snippet. They offer precise information about different code elements, which is valuable for rule testing and result visualization. The inclusion of style, start, and end properties allows for flexible highlighting and analysis.

Consider adding a label for the entire config variable declaration (lines 6-10) to allow for holistic analysis of the configuration object. This could be achieved with a label similar to:

- source: |-
    let config = {
    secret: 'a',
    resave: false,
    saveUninitialized: false,
    }
  style: secondary
  start: 55
  end: 125

This addition would provide a complete view of the configuration context.

tests/__snapshots__/express-jwt-hardcoded-secret-typescript-snapshot.yml (1)

9-81: Comprehensive labeling system for code analysis

The snapshot includes a detailed labeling system that breaks down the code into various components, providing style information and precise positioning. This approach is beneficial for testing, documentation, and potentially for educational purposes.

To enhance clarity and maintainability, consider adding comments to explain the purpose of different label types (e.g., primary vs. secondary) and how they are used in the larger context of your testing or documentation system. This would help other developers understand and maintain this snapshot file more easily.

Example:

labels:
  # Primary label: Highlights the main security concern
  - source: 'secret: ''shhhhhhared-secret'''
    style: primary
    start: 62
    end: 90
  
  # Secondary labels: Provide additional context and structure
  - source: jwt
    style: secondary
    start: 56
    end: 59
  # ... (other secondary labels)

rules/javascript/security/express-session-hardcoded-secret-javascript.yml (1)

10-13: Consider improving the formatting of the note section.

The content is relevant and valuable. However, the readability could be enhanced by using a list format for the CWE reference and the OWASP link.

Consider applying this change:

 note: >-
-  [CWE-798] Use of Hard-coded Credentials.
-  [REFERENCES]
-      - https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html
+  - [CWE-798] Use of Hard-coded Credentials
+  - References:
+    - https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html

rules/typescript/security/express-session-hardcoded-secret-typescript.yml (2)

14-247: Comprehensive patterns, but consider refactoring

The utility patterns MATCH_SECRET and MATCH_SECRET_INSIDE_APP are thorough and cover various scenarios for detecting hard-coded secrets in express-session usage. They effectively handle different import styles and usage patterns.

However, the complexity and some duplication in the import checking logic might make future maintenance challenging. Consider refactoring the common parts of the import checks into a separate utility pattern to improve maintainability.

Here's a suggestion for refactoring:

1. Create a new utility pattern for import checks:

MATCH_EXPRESS_SESSION_IMPORT:
  any:
    - matches: MATCH_IMPORT_STATEMENT
    - matches: MATCH_REQUIRE_STATEMENT
    - matches: MATCH_NAMESPACE_IMPORT

MATCH_IMPORT_STATEMENT:
  kind: import_statement
  all:
    - has:
        stopBy: end
        kind: import_clause
        has:
          stopBy: neighbor
          kind: identifier
          pattern: $T
    - has:
        stopBy: neighbor
        kind: string
        has:
          stopBy: neighbor
          kind: string_fragment
          regex: "^express-session$"

# Define MATCH_REQUIRE_STATEMENT and MATCH_NAMESPACE_IMPORT similarly

2. Update MATCH_SECRET and MATCH_SECRET_INSIDE_APP to use the new pattern:

MATCH_SECRET:
  # ... existing pattern ...
  all:
    - # ... existing conditions ...
    - matches: MATCH_EXPRESS_SESSION_IMPORT

MATCH_SECRET_INSIDE_APP:
  # ... existing pattern ...
  all:
    - # ... existing conditions ...
    - matches: MATCH_EXPRESS_SESSION_IMPORT

This refactoring would reduce duplication and improve maintainability of the patterns.

---

254-256: Consider expanding the constraint for broader coverage

The constraint effectively ensures that the rule focuses on the 'secret' property. However, it might miss common variations of secret property names used with express-session.

Consider expanding the regex to cover more variations:

constraints:
  S:
    regex: "^(session)?[Ss]ecret$"

This would catch additional cases like 'sessionSecret' while still maintaining focus on secret-related properties.

rules/typescript/security/express-jwt-hardcoded-secret-typescript.yml (4)

4-9: LGTM: Clear and informative message.

The message effectively explains the risk of hard-coded credentials and provides a recommendation for secure credential management. The multi-line format is used correctly for readability.

Consider adding a brief mention of the specific library (express-jwt) in the message to make it more targeted. For example:

message: >-
  A hard-coded credential was detected in the context of express-jwt. It is not recommended to store
  credentials in source-code, as this risks secrets being leaked and used by
  either an internal or external malicious adversary. It is recommended to
  use environment variables to securely provide credentials or retrieve
  credentials from a secure vault or HSM (Hardware Security Module).

---

10-13: LGTM: Informative note with relevant reference.

The note includes the relevant CWE identifier and a useful reference to the OWASP Secrets Management Cheat Sheet. The multi-line format is used correctly for readability.

Consider adding a brief description of CWE-798 for better context. For example:

note: >-
  [CWE-798] Use of Hard-coded Credentials: The software contains hard-coded credentials, such as a password or cryptographic key, which it uses for its own inbound authentication, outbound communication to external components, or encryption of internal data.
  [REFERENCES]
      - https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html
      - https://cwe.mitre.org/data/definitions/798.html

---

14-140: LGTM: Comprehensive pattern matching for direct secret usage.

The MATCH_SECRET_DIRECTLY utility is well-designed to catch direct usage of hard-coded secrets in express-jwt. It covers various import scenarios, making it robust against different coding styles.

Consider adding a pattern to match ES6 dynamic imports. For example, add this to the any block:

- follows:
    stopBy: end
    kind: lexical_declaration
    has:
      stopBy: end
      kind: variable_declarator
      all:
        - has:
            stopBy: neighbor
            kind: identifier
            pattern: $E
        - has:
            stopBy: neighbor
            kind: await_expression
            has:
              stopBy: neighbor
              kind: call_expression
              all:
                - has:
                    stopBy: neighbor
                    kind: import
                - has:
                    stopBy: neighbor
                    kind: arguments
                    has:
                      stopBy: neighbor
                      kind: string
                      has:
                        stopBy: neighbor
                        kind: string_fragment
                        regex: "^express-jwt$"

This will help catch scenarios where express-jwt is imported dynamically.

---

141-282: LGTM: Comprehensive pattern matching for indirect secret usage.

The MATCH_PATTERN_WITH_INSTANCE utility is well-designed to catch indirect usage of hard-coded secrets in express-jwt through variables. It covers various import scenarios, making it robust against different coding styles.

Similar to the suggestion for MATCH_SECRET_DIRECTLY, consider adding a pattern to match ES6 dynamic imports for consistency. You can add the same block suggested earlier to the any section of this utility as well.

Additionally, consider adding a pattern to catch object destructuring scenarios. For example, add this to the any block:

- follows:
    stopBy: end
    kind: variable_declaration
    has:
      stopBy: end
      kind: variable_declarator
      all:
        - has:
            stopBy: end
            kind: object_pattern
            has:
              stopBy: neighbor
              kind: shorthand_property_identifier_pattern
              pattern: $E
        - has:
            stopBy: neighbor
            kind: identifier
            regex: "^expressJwt$"

This will help catch scenarios where the express-jwt function is extracted via object destructuring.

Configuration used: CodeRabbit UI
Review profile: CHILL