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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: add sarif output (GitHub Code Scan) #50

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

blackheaven
Copy link
Contributor

No description provided.

Copy link
Owner

@MangoIV MangoIV left a comment

Choose a reason for hiding this comment

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

Nice!

Some minor nitpicks, feel free to ignore. I think I will try this out locally and if I'm happy, we can merge.

If you don't want to apply the nitpicks yourself, I will just do that in a follow up PR, myself.

Thank you so much for you contribution!

src/Distribution/Audit.hs Outdated Show resolved Hide resolved
src/Distribution/Audit.hs Outdated Show resolved Hide resolved
src/Distribution/Audit.hs Outdated Show resolved Hide resolved
src/Distribution/Audit.hs Outdated Show resolved Hide resolved
src/Distribution/Audit.hs Outdated Show resolved Hide resolved
src/Distribution/Audit.hs Outdated Show resolved Hide resolved
@MangoIV
Copy link
Owner

MangoIV commented Aug 24, 2024

Maybe it would also be nice to add the output of these Sarif things as an additional step to CI such that we can inspect them etc. :)

@blackheaven
Copy link
Contributor Author

Maybe it would also be nice to add the output of these Sarif things as an additional step to CI such that we can inspect them etc. :)

yes, I was forced to it for debugging purposes.

@MangoIV
Copy link
Owner

MangoIV commented Aug 24, 2024

I also just realized I'm too fucking stupid to set up my flake checks properly, so I will definitely have to add a fourmolu step after this gets merged 👀

@MangoIV
Copy link
Owner

MangoIV commented Aug 24, 2024

if you already have this PR tested somewhere together with your action, perhaps you can leave a link to it?

src/Distribution/Audit.hs Outdated Show resolved Hide resolved
@blackheaven
Copy link
Contributor Author

I also just realized I'm too fucking stupid to set up my flake checks properly, so I will definitely have to add a fourmolu step after this gets merged 👀

I have reformatted the file

if you already have this PR tested somewhere together with your action, perhaps you can leave a link to it?

not yet

cabal-audit.cabal Outdated Show resolved Hide resolved
@blackheaven
Copy link
Contributor Author

@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

Nice! can you perhaps tell me how this would work then? i.e. where would I see the security report - I see there's some upload happening?

@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

(sorry, I'm new to this, I don't know how this github security / sarif thing works)

@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

I really like what you did wrt formatting, btw. I think we can merge today :)

@blackheaven
Copy link
Contributor Author

@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

Here they are: https://github.com/blackheaven/vulnerable-sandbox/security/code-scanning

that explains it - I don't have access to this tab - I guess kinda understandable... :)

@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

image
this is how it looks on my fork - seems to work!

@MangoIV MangoIV merged commit 0d47b48 into MangoIV:main Sep 4, 2024
12 checks passed
@MangoIV
Copy link
Owner

MangoIV commented Sep 4, 2024

thank you so much, huge improvement to the tool!

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.

2 participants