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

Include fingerprint in print and JSON format output #149

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sb8244
Copy link

@sb8244 sb8244 commented Dec 12, 2023

This fingerprint allows selectively targeting violations using a skip file.

Use Case: I want to selectively skip violations with a written explanation as to why it's a false positive. I want to keep this in a single central file instead of using the comment-based skip approach.

This fingerprint allows selectively targeting violations using a skip file.
@houllette
Copy link
Collaborator

I find this use-case super interesting and totally makes sense, thank you for contributing this!

I'm honestly surprised that this wasn't a part of Sobelow before, especially since it already has the functionality to parse out skipped finding fingerprints from the .sobelow-skip file based on the output from --mark-skip-all.

So my question for you is: what does your proposed flow look like? You mention a single central file instead with a written explanation - would it make sense to fold into this PR, a change to the way the .sobelow-skip file is written / read to be able to accommodate written explanations in the same file so that it can just become the de facto "central file"?

@sb8244
Copy link
Author

sb8244 commented Jan 13, 2024

Apologies for the delay.

Right now (for better or worse), there weren't any code changes required to get the skips working. This is because each line is treated as a fingerprint and blindly compared when skipping. Putting invalid fingerprints there is totally fine—they just won't match anything.

The thing that I like about this is that there's no breaking changes required—or changes at all—to how the fingerprint parsing and skipping is done. However, I could see why some people wouldn't like this because it isn't an intended use case of the code.

Personally, I like the way it came out, although Fingerprint: [Hash] is not a valid format, it has to be [Hash] by itself on the line. This means that taking the output and copy/pasting it from this PR does not produce a skip. You must manually remove the Fingerprint: prefixes

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