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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add required executionSuccessful attribute to SARIF output #2983

Merged

Conversation

simon-engledew
Copy link
Contributor

@simon-engledew simon-engledew commented Apr 22, 2021

馃憢 Hello! This is a follow up to: #2888 馃檱 鉂わ笍

I realised after fixing the first issue the online Validator actually highlights a few others 馃う

Of them, only one appears to be a specification violation:

A missing executionSuccessful property: https://docs.oasis-open.org/sarif/sarif/v2.1.0/csprd01/sarif-v2.1.0-csprd01.html#_Ref8832061

You can see this by taking semgrep/tests/e2e/snapshots/test_check/test_sarif_output/results.sarif and uploading it to the online SARIF validator.

Once you fix this a few others appear (A missing informationUri property: https://docs.oasis-open.org/sarif/sarif/v2.1.0/csprd01/sarif-v2.1.0-csprd01.html#_Toc9244329 and rule "id" not being equal to "name"). As these are optional I wont fix them in this PR - especially as dropping the name attribute may have unintended consequences.

PR checklist:

  • changelog is up to date

@simon-engledew simon-engledew force-pushed the simon-engledew/missing-sarif-properties branch from a268161 to e1390f4 Compare April 22, 2021 12:44
@underyx underyx merged commit 8fc3172 into semgrep:develop Apr 26, 2021
@underyx
Copy link
Member

underyx commented Apr 26, 2021

Thank you again @simon-engledew!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants