Breaking change in SARIF

[laurentsimon]

Hi, this commit #1668 seems to be a breaking change and was not versioned.

The scorecard project has an Action that is now completely broken since yesterday, see an example on this project https://github.com/google/osv.dev/actions/runs/5075329151/jobs/9116430536.

Is there a way to revert this change and put it behind a semver versioning?

/cc @josepalafox

[aeisenberg]

Thanks for letting us know. While we are investigating this, I recommend that you fix the version of the upload-artifact action to @v2.3.3 or @29b1f65.

[laurentsimon]

Thanks for the reply. It's hard for us to ask the 4.4k users to make this change, and not accept dependabot PRs after such change.

I'll dig in the restrictions imposed on the new format. Thanks for looking into the problem!

[laurentsimon]

Btw, some context why we generate these invalid URIs. We found out during testing that SARIF results that don't contain a URI are not populated. Scorecard generates results about the absence of file, e.g. the lack of security policy files SECURITY.md.

The current implementation looks like the following screenshot in the scanning dashboard (uri = "no file associated with this alert", line is set to 1 by default because I recall this was also necessary):

[aeisenberg]

After some discussion, we've decided that the best approach is to retain this newer sarif schema and after validating, drop any errors related to invalid uri-references.

Some more history on the change: the sarif schema we were using had a bug in some of the regexes that made them crash on certain inputs. The official sarif schema had already fixed these errors (without releasing a new version of the schema). Unfortunately, when we started using this new version, validating using the jsonschema package started failing because of a change to the uniqueItems key. The jsonschema version was updated to fix this. This new version of jsonschema now uses stricter parsing of uri-references. This is the bug you are seeing.

[adityasharad]

We'd love to work together to find you a better alternative for creating scorecard alerts without having to create fake URIs that don't satisfy the SARIF spec. That will not block the solution @aeisenberg describes above, but we'll discuss internally and reach out to you with some long-term suggestions. (One immediate suggestion could be attaching the alert to a file that you know exists in the repo, for example the workflow file running the scorecard action.)

[laurentsimon]

Thank you for the prompt reply. Seems like avoiding space character in the URI "fixes" the problem. But I'd like to fix it the right way with your help. Please let me know.

Thank you so much!

[marcellmueller]

Thanks for getting this fixed so fast, our CI is working again!

[aeisenberg]

Thanks for confirming the fix.