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

SARIF 2.2 proposal: precision field for reportingDescriptors and results #611

Open
adityasharad opened this issue Nov 8, 2023 · 3 comments
Labels

Comments

@adityasharad
Copy link

Originally filed as #598, split into more focused component issues.

GitHub Advanced Security's code scanning feature recognises precision levels, currently read from the properties bag of a SARIF reportingDescriptor object, and sorts alerts by them. GitHub CodeQL populates this property in its SARIF output, and the property is recognised for other code scanning tools.

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

Using the property bag was a pragmatic measure to provide this functionality without requiring a SARIF spec change. For SARIF 2.2 I propose we make precision an accepted property on a reportingDescriptor or a result, with the same accepted values that GitHub consumes today: very-high, high, medium, low, or omitted entirely. SARIF producer tools can use this property to indicate the tool's own confidence in the precision of the result, or for a rule, how often the results indicated by this rule are true.

cc @michaelcfanning

@michaelcfanning
Copy link
Contributor

The interesting question to me here is how this property s/be differentiated from the existing SARIF rank property.

One possible answer is that precision is a metric that's very focused on a specific ranking aspect: the relative accuracy/precision of the analysis itself. If we set a precedent for this kind of value, should we consider other focused metrics that might be useful inputs into a triage process? e.g., 'likelihood to occur', 'relative impact', 'visibility to users'.

Relative impact is a kind of numeric expression for the new severity value you've proposed. Are we heading towards a situation where we have property pairs? i.e., a numeric expressions from 0.0 to 100.0 coupled with a four-level, readable priority (note, warning, errror, critical).

@katrinaoneil
Copy link

Also, what if we use two separate values: one for the engine confidence and another for the rules accuracy -- how do they map to the proposed precision value?

@ShiningMassXAcc
Copy link
Contributor

Documenting here: +1 to a precision field of some sort.

  • I can see motivations for using the rank-like scheme to express (eg 0.0 - 100.0). I can see motivations for using the level scheme (fixed string-named prose for easier client interpretation).
  • Consider if we will want the granularity in the future, given precision + recall is often a very mathematical item - even if today we think about rules in codeql more coarsely.
  • Agree this can be on the rule / result / etc

@stacywray stacywray added design-approved The TC approved the design and I can write the change draft and removed design-approved The TC approved the design and I can write the change draft labels Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants