Support for SARIF output format

[arkq]

Resolves #2405

PS.
@koalaman, you've stated in #2405 (comment) that you don't want support for SARIF. I'm not sure whether you've changed your mind about that. I'd like to use ShellCheck with GitHub actions as a security analysis tool (see this for reference: ludeeus/action-shellcheck#62). I'm not a Haskell programmer at all, so this PR might have some issues regarding Haskell good-practices. Anyway, I'm willing to invest some time to finish this work as long as you will merge the final outcome into master.

[arkq]

I've created a simple check for GitHub code scanning based on test script with errors: https://github.com/Arkq/shellcheck/blob/security/.github/workflows/build.yml#L71

Outcome:

Listing

Details

[nvuillam]

I've quickly read so I may be mistaken, but did you manage rules property ?
It's part of SARIF standard:)

[candacebutler8879]

I’m not sure are you talking to me directly or is it a mass email ? Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Nicolas Vuillamy ***@***.***> Sent: Monday, September 12, 2022 11:02:23 AM To: koalaman/shellcheck ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [koalaman/shellcheck] Support for SARIF output format (PR #2569) I've quickly read so I may be mistaken, but did you manage rules property ? It's part of SARIF standard:) — Reply to this email directly, view it on GitHub<#2569 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AUW5SQO736DTUQCL6XSMQ2TV55OZ7ANCNFSM6AAAAAAQD4ZRN4>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[nvuillam]

my comment is for @arkq but please feel free to comment also :)

https://github.com/microsoft/sarif-tutorials/blob/main/samples/1-Introduction/simple-example.sarif

In tools.drivers, there is a rules property with entries corresponding to the ruleIds that wee can find in the results :)

[arkq]

In tools.drivers, there is a rules property with entries corresponding to the ruleIds that wee can find in the results

Yes, you are right. I think that the rules property should (or maybe even shall) be included. However, as shown in the attached screenshots it works (at least on GitHub) without such property - it lacks rule help text though. I can try to add rules to this PR, but I'm not Haskell programmer, so this might not be easy task for me, and I do not want to invest more time in something which at the and might end up in a trash bin. So, firstly I'd like to know whether at the end this feature will get merged into master :)

[nvuillam]

https://sarifweb.azurewebsites.net/Validation

You can try any generated SARIF here :)

[arkq]

The validation passes, but there are some suggestions (one of them is adding rules):

• runs[0].results[0].message: The 'message' property of this result contains a 'text' property. Consider replacing it with 'id' and 'arguments' properties. This potentially reduces the log file size, allows the message text to be improved without modifying the log file, and enables localization.
• runs[0]: This run does not provide 'versionControlProvenance'. As a result, it is not possible to determine which version of code was analyzed, nor to map relative paths to their locations within the repository.
• runs[0].results[0].locations[0].physicalLocation.region: The 'region' object in this result location does not provide a 'snippet' property. Providing a code snippet enables users to see the code that triggered the result, even if they are not enlisted in the code.
• runs[0].results[0].locations[0].physicalLocation: This result location does not provide a 'contextRegion' property. Providing a context region enables users to see a portion of the code that surrounds the result, even if they are not enlisted in the code.
• 'runs[0].tool.driver' does not provide a 'rules' property. 'rules' contain information that helps users understand why each rule fires and what the user can do to fix it.

[koalaman]

Nothing appears to have changed since 2019, so I'm still very skeptical about this. Are there any SARIF consumers outside of the Microsoft sphere? GitHub and VSC already have third party plugins, so the value add of another JSON format specific to those is minimal.

[nvuillam]

in MegaLinter we use a rust package for the conversion

https://crates.io/crates/shellcheck-sarif

It would have better performances if it was directly embedded in shellcheck :)

[arkq]

Nothing appears to have changed since 2019

I think that the next release of Clang will contain SARIF output formatter https://clang.llvm.org/doxygen/Sarif_8h_source.html so that's a change for start. SARIF adaptation among open source projects is not fast but it will happen eventually IMHO. The format is indeed bloated, but for a reason. It was designed to be flexible enough to support a large variety of tools. The spec is big, but a particular tool can support only a subset of that. Anyway, I think that the standardization for errors/warnings reporting is a good thing.

However, as a maintainer of few projects, I do agree that adding something (which maintainer does not need) to the project is a though decision, mostly from the maintenance point of view. So, if you feel that this addition might be too much, I will understand that. But personally I think that maintenance impact of this new formatter will not be big because of your plugin-like design for output formatters.

[nvuillam]

:(

[arkq]

@nvuillam since some time I'm using https://github.com/redhat-plumbers-in-action/differential-shellcheck It works (mostly) as expected for my use case, so I do not need direct support in the shellchec. I've removed my fork, but all commits are still accessible (github fork leaves in the upstream's repo namespace).

[nvuillam]

@arkq it does not solves my case with shellcheck embedded within MegaLinter :(

@koalaman SARIF format is now :

• planned for Gitlab -> https://gitlab.com/gitlab-org/gitlab/-/issues/345082
• available for Azure -> https://marketplace.visualstudio.com/items?itemName=sariftools.scans
• requested for Bitbucket -> https://jira.atlassian.com/browse/BCLOUD-20199

No other SAST results standard has popped since the initial request... are you still against natively supporting SARIF output ? :/

[oliv3r]

@koalaman while I am no fan of Microsoft bubble tech, it seems like as @nvuillam reported, gitlab is planning on supporting it at some point.

I think the difference between checkstyle/junit and SARIF will be how it will be consumed (at least by gitlab). With junit being a test result, and SARIF ending up being a SAST report.

So having both options in the future, is not the worst idea ...