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

Add SARIF output support #2046

Closed
jawnsy opened this issue Jan 21, 2023 · 11 comments
Closed

Add SARIF output support #2046

jawnsy opened this issue Jan 21, 2023 · 11 comments
Labels
core Related to core functionality good first issue Good for newcomers

Comments

@jawnsy
Copy link

jawnsy commented Jan 21, 2023

I did a cursory search of the code and didn't find anything related, so I believe this is a feature request. The only reference to "SARIF" that I found was a comment from a few weeks ago: #1546 (comment)

Summary

Add a flag to output SARIF data during a run, instead of, or in addition to, the existing output.

Description

GitHub's CodeQL can ingest SARIF files during builds, so that we can track lint issues over time and surface issues inline on diffs. These can be uploaded during pull request builds as well as push event builds.

The trivy code/container image scan tool supports SARIF format, and this is very helpful for open source projects (since CodeQL is free there) as well as for companies using CodeQL.

@charliermarsh charliermarsh added the core Related to core functionality label Jan 21, 2023
@charliermarsh
Copy link
Member

Makes sense! I'll admit that I know nothing about the format so will have to look into it when I have some time.

Of course, would also make for a great PR if anyone is interested!

@charliermarsh charliermarsh added the good first issue Good for newcomers label Jan 21, 2023
@padmano
Copy link

padmano commented Jan 21, 2023

I can work on this.

@edgarrmondragon
Copy link
Contributor

@padmano are you working on this?

@padmano
Copy link

padmano commented Feb 10, 2023

Yes @edgarrmondragon.

@zanieb
Copy link
Member

zanieb commented Apr 23, 2023

I'm looking into this a bit and figure I'll share some resources

There's a bandit formatter plugin at microsoft/bandit-sarif-formatter — you can see the implementation details in formatter.py. They lean on some generated Python models and Microsoft also publishes a CLI tool which may be useful for testing.

The latest SARIF specification is available as a JSON schema.

@padmano are you still actively working on this? Could you link to a branch here so people don't have to ask?

@C-Hipple
Copy link
Contributor

I've also been playing with this a bit lately. My branch is here: https://github.com/astral-sh/ruff/compare/main...C-Hipple:dev-add-sarif-output-support?expand=1

Approach so far was copy/paste the json emitter and then I'm going to just change up the formatting. Once it's working i'll clean up the copy/paste and see what can easily be reused without getting too complicated. Probably not too much will end up shared since sarif files group results per rule found, so Serialize for ExpandedMessages is going to look completely different.

What I still need to figure out in Ruff is the best way to get all of the rules which are enabled, since sarif files expect the full list even if there aren't findings for each run. I started in my last commit with a with_applied_rules builder method but it doesn't look like the EmitterContext has what I need, and I don't really want to re-parse the pyproject.toml file. I haven't spent too much time looking around here yet, but any direction would be appreciated (maybe there's a global settings singleton i'm missing?)?

@C-Hipple
Copy link
Contributor

^^ Found some time and resolved the issues above, now it's just about finding the time to format the json as the sarif format.

@C-Hipple
Copy link
Contributor

#9078 PR is here

@charliermarsh
Copy link
Member

Thank you @C-Hipple! Look forward to reviewing. Should be able to turn this around pretty quickly.

@C-Hipple
Copy link
Contributor

Thank you @charliermarsh , no big rush though, I have it as a draft since I need to work out a few CI issues and possibly tweak a few other things

@charliermarsh
Copy link
Member

This merged as #9078. Thanks @C-Hipple!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core functionality good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants