SARIF reports generate broken artifact URIs when nonstandard path is used

[psalerno]

Background

Brakeman version: 6.2.2

Issue

Brakeman's SARIF reporting only ever uses the relative path of artifacts, which you can see here. However, It never incorporates the --path option that was passed into the Brakeman scan.

I discovered this when trying to run Brakeman on a mono-repo that contains several Rails apps. In order to scan a particular app in that repo, you need to specify the path of the specific Rails app you want to scan:

brakeman -f sarif -o output.sarif.json --path apps/one_of_several_apps

However, then the artifact URIs in the SARIF report are all relative and do not factor in the --path value, so the URIs look like:

app/file_that_has_a_vulnerability.rb

when they should look like:

apps/one_of_several_apps/app/file_that_has_a_vulnerability.rb

This means you cannot have a full URI that indicates which app in the mono-repo is being scanned. This makes Brakeman artifacts break when importing its SARIF output into other scanners like GitHub's Advanced Security CodeQL scans, as GitHub is unable to generate artifacts for relative paths in a mono-repo when it cannot tell which app is being scanned.

[presidentbeef]

At first I thought this was simple, but... it's not.

Let's see if we can work through this.

The standard says

the uri property (§3.4.3) SHOULD be deterministic [...]" (3.4.7). Relative file names are deterministic.

If the URI is non-deterministic, the uriBaseId property (§3.4.4) SHOULD capture the non-deterministic portion of the URI, for example, the absolute path to the root of the directory tree containing the analyzed source code."

Well, right now Brakeman blindly copies the examples and sets uriBaseId to %srcroot% but never defines it. That's... probably wrong.

It seems clear to me from section 3.14.14 that the combination of the uri and the uriBaseId (or possibly a chain of them) must eventually be an absolute URI:

A run object MAY contain a property named originalUriBaseIds whose value is an object (§3.6) each of whose property names designates a URI base id (§3.4.4) and each of whose property values is an artifactLocation object (§3.4) that specifies (in the manner described below) the absolute URI [RFC3986] of that URI base id on the machine where the SARIF producer ran.

With that in mind, it seems like you are asking not for absolute URIs, but URIs relative to the directory from which Brakeman is run but are prepended by the specified path 🤔

---

After typing all that, I looked at https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#relative-uri-guidance-for-sarif-producers and it seems generating a proper originalUriBaseIds will work. Probably makes sense to also support including the relative path as an intermediate URI.

Relevant bit:

      "originalUriBaseIds": {
        "PROJECTROOT": {
         "uri": "file:///C:/Users/Mary/code/TheProject/",
           "description": {
             "text": "The root directory for all project files."
           }
        },
         "%SRCROOT%": {
           "uri": "src/",
           "uriBaseId": "PROJECTROOT",
           "description": {
             "text": "The root of the source tree."
           }
         }
      }

So I guess I'll add that.

[presidentbeef]

If you can try out #1890 that would be very helpful.