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

Merge command returns invalid SARIF if there are 0 input files. #1592

Closed
harleenkohli opened this issue Jul 26, 2019 · 1 comment
Closed
Labels
area-multitool Multitool command implementations bug m156 resolved-fixed

Comments

@harleenkohli
Copy link
Contributor

harleenkohli commented Jul 26, 2019

Multitool "merge" command allows us to pass a collection of sarif files using wildcard. e.g. the following is valid:

sarif merge --pretty-print "$ScanResultsFolder\*.sarif" --recurse --output-folder $MergedLogsFolder --output-file "$MergedFileName"

However, if there is no sarif file in the $ScanResultsFolder, the resulting merged file looks like this:

{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.4.json",
  "version": "2.1.0",
  "runs": null
}
  • If this invalid sarif, we should fix it.
  • If this is valid sarif, this breaks other things in our ecosystem, e.g. ADO viewer or other sarif commands namely match-results-forward which expect run object - so we should handle this case more appropriately - perhaps throw an exception?
@ghost ghost added area-conversion area-multitool Multitool command implementations labels Aug 14, 2019
@ghost
Copy link

ghost commented Aug 15, 2019

Although runs can have the value null, it is invalid in this case. The spec says (§3.13.4):

The value of runs SHALL be an array with at least one element except in the following circumstances:

  • If a SARIF producer finds no data with which to populate runs, then its value SHALL be an
    empty array.

NOTE 1: This would happen if, for example, the log file were the output of a query on a
result management system, and the query did not match any runs stored in the result
management system.

  • If a SARIF producer tries to populate runs but fails, then its value SHALL be null.

NOTE 2: This would happen if, for example, the log file were the output of a query on a
result management system, and the query was malformed.

Here we are in the first situation: the "query" ("*.sarif") returned no data. So IMO the right answer is to produce an empty runs array.

@ghost ghost changed the title Sarif Multitool merge command returns possibly invalid sarif if 0 files passed to merge Merge command returns invalid SARIF if there are 0 input files. Aug 15, 2019
@ghost ghost added bug and removed area-conversion labels Aug 15, 2019
@ghost ghost self-assigned this Aug 15, 2019
ghost pushed a commit that referenced this issue Aug 15, 2019
@ghost ghost closed this as completed in 4f9e92c Aug 15, 2019
@ghost ghost added the resolved-fixed label Aug 15, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-multitool Multitool command implementations bug m156 resolved-fixed
Projects
None yet
Development

No branches or pull requests

2 participants