Skip to content
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

Output originalBaseUriIds for SARIF report #1890

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

presidentbeef
Copy link
Owner

@presidentbeef presidentbeef commented Dec 11, 2024

Fixes #1889

This adds originalBaseUriIds following GitHub guidance and hopefully the spec.

This PR covers six cases, with example output below.

If there is only one URI to specify, it will be %SRCROOT%. If there is more than one (a relative and an absolute or empty URI), then %SRCROOT% will be relative and it will reference PROJECTROOT which will be the absolute (or empty) base URI.
This is because all the results use %SRCROOT% as the uri.

To best preserve privacy, the base URI does not include an absolute path unless --absolute-paths is set.

  1. Default (implicit path)
      "originalUriBaseIds": {
        "%SRCROOT%": {
          "description": {
            "text": "Base path for application"
          }
        }
      }
  1. Relative application path (e.g. --path some/where/)
      "originalUriBaseIds": {
        "PROJECTROOT": {
          "description": {
            "text": "Base path for all project files"
          }
        },
        "%SRCROOT%": {
          "uri": "test/apps/rails8/",
          "uriBaseId": "PROJECTROOT",
          "description": {
            "text": "Base path for application"
          }
        }
      }
  1. Absolute application path (e.g. --path /some/where)
      "originalUriBaseIds": {
        "%SRCROOT%": {
          "description": {
            "text": "Base path for application"
          }
        }
      }
  1. Default (implicit path) with --absolute-paths
      "originalUriBaseIds": {
        "%SRCROOT%": {
          "uri": "file:///home/justin/work/brakeman/test/apps/rails8/",
          "description": {
            "text": "Base path for application"
          }
        }
      }
  1. Relative application path with --absolute-paths
      "originalUriBaseIds": {
        "PROJECTROOT": {
          "uri": "file:///home/justin/work/brakeman/test/apps/rails8/",
          "description": {
            "text": "Base path for all project files"
          }
        },
        "%SRCROOT%": {
          "uri": "test/apps/rails8/",
          "uriBaseId": "PROJECTROOT",
          "description": {
            "text": "Base path for application"
          }
        }
      }
  1. Absolute application path with --absolute-paths
      "originalUriBaseIds": {
        "%SRCROOT%": {
          "uri": "file:///home/justin/work/brakeman/test/apps/rails8/",
          "description": {
            "text": "Base path for application"
          }
        }
      }

As far as I can tell with the SARIF validator, these are all valid.

Copy link

DryRun Security Summary

The pull request enhances the Brakeman security scanner's SARIF output by updating the schema version, improving path handling, and ensuring comprehensive and accurate security reporting across various application path configurations.

Expand for full summary

Summary:

The code changes in this pull request are focused on improving the SARIF (Static Analysis Results Interchange Format) output of the Brakeman security scanner. The changes ensure that the SARIF output conforms to the latest SARIF specification, provides accurate and detailed information about the security findings, and handles various application path configurations correctly.

Key improvements include:

  1. Updating the SARIF schema version to the latest version.
  2. Validating the structure and contents of the SARIF output, including the runs, tool, results, and suppressions sections.
  3. Implementing robust handling of original URI base IDs, which is crucial for accurately representing file paths in the SARIF report.
  4. Addressing different scenarios for application paths, such as relative and absolute paths, to ensure the SARIF report includes the appropriate base URIs.

These changes demonstrate a thorough understanding of the SARIF specification and a commitment to providing high-quality security reporting for the Brakeman tool. The attention to detail and the focus on handling various application path configurations are particularly noteworthy from an application security perspective.

Files Changed:

  1. test/tests/sarif_output.rb:

    • Updates the SARIF schema version to the latest version.
    • Ensures the SARIF output contains the expected structure and contents, including the runs, tool, results, and suppressions sections.
    • Validates the mapping between ruleId and the corresponding rule in the rules array.
    • Thoroughly tests the handling of suppressed findings in the SARIF output.
    • Verifies the correct handling of originalUriBaseIds for absolute and relative application paths.
  2. lib/brakeman/report/report_sarif.rb:

    • Updates the SARIF schema version to the latest version.
    • Introduces a new method original_uri_base_ids to generate the originalUriBaseIds section of the SARIF report.
    • Handles different scenarios for the application path, including when no path is specified, when a relative path is specified, and when an absolute path is specified.
    • Includes some defensive programming measures to handle cases where the application path contains ".." (parent directory) segments.

Code Analysis

We ran 9 analyzers against 2 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SARIF reports generate broken artifact URIs when nonstandard path is used
1 participant