Repository navigation
Breaking change in SARIF #1703
Description
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
Activity
aeisenberg commentedon May 25, 2023
Thanks for letting us know. While we are investigating this, I recommend that you fix the version of the
upload-artifactaction to@v2.3.3or@29b1f65.laurentsimon commentedon May 25, 2023
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 commentedon May 25, 2023
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 commentedon May 25, 2023
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
uniqueItemskey. The jsonschema version was updated to fix this. This new version of jsonschema now uses stricter parsing ofuri-references. This is the bug you are seeing.adityasharad commentedon May 25, 2023
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 commentedon May 25, 2023
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 commentedon May 25, 2023
Thanks for getting this fixed so fast, our CI is working again!
aeisenberg commentedon May 25, 2023
Thanks for confirming the fix.