microsoft / sarif-sdk

.NET code and supporting files for working with the 'Static Analysis Results Interchange Format' (SARIF, see https://github.com/oasis-tcs/sarif-spec)
Other
192 stars 90 forks source link

HdfConverter: Set security-severity property used by GitHub #2705

Closed candrews closed 1 year ago

candrews commented 1 year ago

See: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#reportingdescriptor-object

candrews commented 1 year ago

@michaelcfanning could you please take a look at this when you get a chance?

michaelcfanning commented 1 year ago

We can take this change if you're confident GitHub consumes this data. I thought that security severity was only relevant for the rules metadata. Someone else, actually, had raised the issue with us (and GitHub) that security-severity can't currently be overridden at the result level. Take a look at the content below, see how it requires adding this data to the reportingDescriptorReference?

https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#runautomationdetails-object

michaelcfanning commented 1 year ago
          "id": "7",

This is the level where you need to add security-severity, I think. If security-severity is a dynamically computed per-result value that can change within a rule, then we need to work with GitHub to get that functionality going for you (it's already been requested by someone else as well).


Refers to: src/Test.UnitTests.Sarif.Converters/TestData/HdfConverter/ExpectedOutputs/ValidResults.sarif:4949 in e1ca167. [](commit_id = e1ca167d3b10e4be12d6c31ca6e4c1877486ffc9, deletion_comment = False)

candrews commented 1 year ago

I thought that security severity was only relevant for the rules metadata.

You're right...

This is the level where you need to add security-severity, I think.

Agreed.

I've updated this PR accordingly.