Support the OASIS Static Analysis Results Interchange Format (SARIF)

[sschuberth]

Maybe this could be used to more easily interchange analyzer / scanner / reporter results, see https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=sarif.

[sschuberth]

More semi-related news on that general topic: https://github.blog/2020-10-05-announcing-third-party-code-scanning-tools-static-analysis-and-developer-security-training/

[jonico]

inspiration how this may look like:

[sschuberth]

Also see the very interesting discussion at https://github.com/detekt/detekt/issues/3045 with folks from GitHub / Microsoft. In particular, the detekt project now provides a SARIF library for Kotlin.

[sschuberth]

@jonico, is there also a way to see the alerts directly in the "files" tab of the PR which introduces them?

[jonico]

yes:

Inside a PR, you will only see alerts that have been newly introduced by this PR. In order to compute and display the "delta", the base branch would need at least one analysis before with uploaded SARIF results.

[sschuberth]

In order to compute and display the "delta", the base branch would need at least one analysis before with uploaded SARIF results.

Ah, thanks for that bit of information! That explains why I did see anything displayed in my PR at https://github.com/oss-review-toolkit/ort/pull/3749 when I deliberately introduced a detekt violation.

Wouldn't it make sense to treat a non-present baseline (at least optionally) as "no issues present", and then simply display all issues from the SARIF file initially?

[jonico]

I believe we show that in the checks tab:

Even if you have no results in the base branch yet, you could inspect the findings if you have write access to the repo and filter by the refs/pull/<PR-ID>/merge ref in the security tab:

In your case https://github.com/oss-review-toolkit/ort/security/code-scanning?query=is%3Aopen+ref%3Arefs%2Fpull%2F3749%2Fmerge+ should show the results.

I admit that this is not super obvious, if you like the results to more prominently show up, you may include this link as part of your Action (create a comment) - or let the SARIF upload once upload results to your main branch.

Wouldn't it make sense to treat a non-present baseline (at least optionally) as "no issues present", and then simply display all issues from the SARIF file initially?

Interesting idea, I will bring it up with product management.

[dgutson]

Hi, any update on this?

[sschuberth]

AFAIK no one is actively working on this, so far it was more or less just an idea. And I'm not even sure if SARIF is suitable for the kind of result ORT provides, as they cannot really be associated to a line of code.

[dgutson]

What about considering the pqckage dependency files as source code, pointing to the line where the dependency is specified?

[sschuberth]

The point is that ORT does (in most cases) not parse the package manager files, and as such also does not really know what dependency is declared in which line.

[dgutson]

So, does ORT rely on external libraries (such as pip-tree), and those are which actually read the pkg manager files? If so, maybe we should enhance those libs with "debug information" or some other verbose mode, so ORT can relate findings with physical file locations? (we can do this)

[sschuberth]

It really depends on the package manager and its capabilities how ORT analyzes the dependencies. For Gradle, we are using the Gradle Tooling API and no external script, and I'm not aware of the Tooling API to provide any "debug" information about the line of declaration.

[dgutson]

So it is a case-by-case basis (language-specific). We (here) could start with pip-tree.

[sschuberth]

Looks like SARIF does not require to have a location (plus annotation with a startLine) listed after all. So we should be able to write a generic reporter (not starting with a specific package manager) that "simply" transforms the ORT result data model as good as possible to the SARIF data model.

[dgutson]

@sschuberth I don't see much value if we can't pinpoint the file where to apply the change. I think we need to tackle the package managers first so they provide location data.

[sschuberth]

I don't see much value if we can't pinpoint the file where to apply the change.

We seem to have different priorities then. To me, SARIF is just one report format like any other basically, and none of the other report formats currently "pinpoint the file where to apply the change" (let alone down to the line of code).

So, what I was mainly looking for is to capture any policy violations (caused by license compliance issues, or by security vulnerabilities) themselves in SARIF. I'd argue that any developer should know their build system well enough to known where to update / remove a particular offending dependency. That's why pinpointing the file is not a priority for me.

That said, still pinpointing the file is certainly nice to have, and perfectly doable in a package manager agnostic way with SARIF. I would just abstain from going down to the line of definition with a file.

[dgutson]

@sschuberth it has been a long night (3 kids) and I was thinking in Google Scorecard which we are also maintaining and has a SARIF initiative. Let me come back to you later today. Sorry.

[sschuberth]

Interesting to see how CheckStyle implemented SARIF support via JSON templates that they just fill in.

[dgutson]

@sschuberth coming back here. So without going to the line detail, would you go to the file detail with SARIF? (eg path/to/requirements.txt, etc.)

[sschuberth]

would you go to the file detail with SARIF?

Quite likely yes.