Skip to content

Conversation

mbaluda
Copy link
Contributor

@mbaluda mbaluda commented Sep 27, 2023

FIxes #57
Adds steps to the Code Scanning workflow checking that the new SARIF file matches the expected one stored in .github/workflows/javascript.sarif.expected.
If there is a mismatch, the new SARIF and a diff are uploaded as artefacts to eventually update the .expected file

Examples of passing and failing runs:
https://github.com/advanced-security/codeql-sap-js/actions/runs/7787950054
https://github.com/advanced-security/codeql-sap-js/actions/runs/7787745458

@mbaluda mbaluda self-assigned this Sep 27, 2023
@mbaluda mbaluda marked this pull request as ready for review September 28, 2023 17:37
Copy link

@rvermeulen rvermeulen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.
If will change the structure in our restructuring PR to make it UI5 specific.

See the code comment for info on the request change.

@mbaluda mbaluda requested a review from rvermeulen September 28, 2023 19:26
@mbaluda mbaluda requested a review from jeongsoolee09 February 5, 2024 16:35
@mbaluda mbaluda marked this pull request as draft February 5, 2024 16:42
@jeongsoolee09
Copy link
Contributor

Code itself looks good, but does this mean we should keep javascript.sarif.expected updated?

@jeongsoolee09
Copy link
Contributor

Also, it might be nice if we can replace Code Scanning / Analyze step with this one.

@mbaluda
Copy link
Contributor Author

mbaluda commented Feb 5, 2024

Code itself looks good, but does this mean we should keep javascript.sarif.expected updated?

Yes! After reviewing the diff, one would typically replace the SARIF with the one in the action artefacts like here:
https://github.com/advanced-security/codeql-sap-js/actions/runs/7787745458

@mbaluda mbaluda marked this pull request as ready for review February 5, 2024 18:48
@mbaluda mbaluda enabled auto-merge February 5, 2024 18:53
@mbaluda mbaluda merged commit 86cf056 into main Feb 7, 2024
@mbaluda mbaluda deleted the mbaluda-sarif-diff branch February 7, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify the results of Code Scanning using sarif diff

4 participants