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

wip: Add SARIF output format #1

Closed
wants to merge 24 commits into from
Closed

wip: Add SARIF output format #1

wants to merge 24 commits into from

Conversation

swinton
Copy link
Owner

@swinton swinton commented Jul 9, 2020

Adds support for the SARIF format to brakeman.

Some helpful information on the SARIF format is here.

@swinton swinton self-assigned this Jul 9, 2020

## Questions

- Does Brakeman have a sense of severity that we can map into the results? Should we default to "warning" for all results?
Copy link
Collaborator

Choose a reason for hiding this comment

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

confidence is a conflation of both severity and confidence.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I added 9b95383 to infer severity from confidence.

docs/json_to_sarif.md Show resolved Hide resolved
docs/json_to_sarif.md Show resolved Hide resolved
docs/json_to_sarif.md Show resolved Hide resolved
docs/json_to_sarif.md Show resolved Hide resolved
@swinton
Copy link
Owner Author

swinton commented Jul 28, 2020

For reference

Adding a screenshot of how SARIF properties are treated in the GitHub Code scanning alerts UI:

Note the following properties are pulled into the UI from this example SARIF file:

Screen Shot 2020-07-28 at 5 32 33 PM

Notes

  • shortDescription and fullDescription are currently identical, and are both derived from the Check classes -- @presidentbeef are you still able to work on producing descriptions at the warning level?
  • I'm trying to find out if we can render anything in the "rule help" section (where currently No rule help available for this alert is shown), and also whether Markdown is supported, so we can render links, such as helpUri

@presidentbeef
Copy link
Collaborator

shortDescription and fullDescription are currently identical, and are both derived from the Check classes -- @presidentbeef are you still able to work on producing descriptions at the warning level?

Yes, I can do this.

I'm trying to find out if we can render anything in the "rule help" section (where currently No rule help available for this alert is shown), and also whether Markdown is supported, so we can render links, such as helpUri

Another thought is to directly pull in some docs. I believe Code Climate does/did this using what is in https://github.com/presidentbeef/brakeman/tree/main/docs/warning_types (in fact, that directory is just for them). However those docs are not currently up-to-date... but they can be updated. The content is from the Brakeman site which is licensed under Creative Commons.

@swinton
Copy link
Owner Author

swinton commented Jul 29, 2020

Another thought is to directly pull in some docs

Great idea 👍 SARIF supports Markdown-formatted messages, I will see if there's a way to render these in the UI.

@swinton
Copy link
Owner Author

swinton commented Aug 12, 2020

I opened a new PR targeting brakeman:main, closing this one.

@swinton swinton closed this Aug 12, 2020
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