SarifResultBuilder Constructor Causes Loss of Valid SARIF Result Properties

[AlexWilson-GIS]

The Problem

I am trying to set properties in a SarifResultOptions object. Here is the type definition of this object: https://github.com/nvuillam/node-sarif-builder/blob/2682b619b87130190c1511ef60b1d2310659e5f3/src/types/node-sarif-builder.ts#L300-L455

After trying to figure out why my test runs are creating SARIF files with lost data, I traced down through the code to discover the source of the issue in the constructor:

https://github.com/nvuillam/node-sarif-builder/blob/2682b619b87130190c1511ef60b1d2310659e5f3/src/lib/sarif-result-builder.ts#L7-L19 https://github.com/nvuillam/node-sarif-builder/blob/2682b619b87130190c1511ef60b1d2310659e5f3/src/lib/utils.ts#L1-L8

This places a large filter over the SarifResultOption that only allows three properties through: level, message, and ruleId.

Proposed Solutions

1. Ensure the properties in SarifResultOptions objects align with properties in the Result object, then: a. change the setOptionsValues function to merge the default properties object and the user provided one using the spread operator, like this:

   export function setOptionValues(options, object: any) { 
   return {
   ...object,
   ...options
   }
   }

This will merge the two objects together. Any fields found in both objects will resolve to the value in options. This will ensure the default values for each of the three fields found in the result object persist if the user-provided options don't supply them, while allowing the other properties of a SarifResultOptions object to pass through, and making it so they override the default values where applicable. You would then need to change the constructor to re-set the internal result property.

b. Rewrite the constructor using the same logic I specified in option 1.

c. Tweak setOptionValues to iterate over the user-supplied keys, rather than the default object keys.

2. Rewrite the constructor to map each field from SarifResultOptions objects to Result objects.
3. Reduce the type definition of SarifResultOption to only have the properties that can actually make it through the existing constructor, and document that any additional properties need to be added in to the underlying Result manually after construction.

[nvuillam]

@AlexWilson-GIS thanks for reporting the issue :)

Please can you share the code you use to call node-sarif-builder ?

The main goal of this package is to provide an easy way to build SARIF which is a quite complex format with (too many ?) sub levels, that's what there is not a lof of options with initSimple() methods

There are 2 ways to handle additional options:

• Update node-sarif- builder to be able to handle more "simple" options, and make sure we handle default values if they are not set
• Directly update SARIF JS object from your code, after calling node-sarif-builder methods, to add the extra properties that you need (as node-sarif-builder is packaged with official SARIF types, it shouldn't be so hard from your code)

[AlexWilson-GIS]

Unfortunately, I cannot share the code because my company's policy does not allow it. At a high level, what I'm doing is iterating over the output of analysis tools in another format and using this library to convert it to SARIF, with the end-goal of uploading the results into GitHub's Security tab. At this point, I'm getting around the issue by doing your second bullet point, and I think it will work. I'm still doing testing, but the properties look like they are carried through.