Fix matched string #363
Merged
Fix matched string #363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's not actually true that *all* chunks are diff output; local folder scans have just raw content (prefixed with filename). To fix this: * Add a new "is_diff" property to the Chunk type. We do this instead of adding this to the existing metadata property so that we do not perturb user-visible output (where we blindly render every member). * Fix all Chunk generators to set .is_diff properly. * Adjust scan_entropy() so that it removes the first character from each line of a chunk when .is_diff is True, and leaves it alone otherwise. The first character is preserved in expectation that we might use it for some backwards-compatibility kludge (but right now it just pisses off pylint because it's never consumed). Unit tests updated, mostly to just add the missing constructor parameter. The local folder scan tests now pass again. Conundrums left for another day: * Technically, scan_regex() should be doing the same thing, but this also would introduce a compatiblity break and it's more effort because we presently don't do any chunk interpretation. It's probably better to just leave it as-is. * I notice that bizarrely one of the pre-commit unit tests fails if you have staged files present. It works fine as long as you don't have any such files. Based on what it claims to be doing, this behavior shouldn't be occurring in the first place.
I think. This code isn't tested yet. Overhead should be relatively small (aka manageable) because we go to the effort of checking only if: * There is an entropy finding * It isn't excluded * It appears at the beginning of a line In that situation, we retry the exclusion based on the string that tartufo used to use. If THAT gets excluded, then we: * Do not generate an issue * We do create issues (temporarily) for both the old and new strings * We report the old and new signatures and tell users to update their configuration
All instances of line should have been replaced by analyze. Spotted by @sushantmimani
Fix chunk handling for entropy scanner
sushantmimani
requested review from
irodelta,
jmink-godaddy,
rbailey-godaddy and
emayuri-godaddy
rbailey-godaddy
previously requested changes
Jun 30, 2022
Co-authored-by: Scott Bailey <72747501+rbailey-godaddy@users.noreply.github.com>
rbailey-godaddy
approved these changes
Jun 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To help us get this pull request reviewed and merged quickly, please be sure to include the following items:
PR Type
What kind of change does this PR introduce?
Backward Compatibility
Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the API in any way?
Issue Linking
Fixes #347 #183
What's new?
+or-and provides a new signature to replace it with.