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 per line exclude regex via --exclude-line #127

Merged
merged 6 commits into from
Feb 9, 2019

Conversation

KevinHock
Copy link
Collaborator

@KevinHock KevinHock commented Feb 7, 2019

So this reverts 15a6e6a in favor of a line exclude arg for all plugins. If at a later date we would like a regex specifically for e.g. KeywordDetector, then we can add that feature then.

💥 This removes the --exclude CLI arg

🎉 Replace --exclude with --exclude-files and --exclude-lines

Change baseline format to replace `exclude_regex` with
```
"exclude":{
  "files": "foo",
  "lines": "bar"
}
```

Pass `exclude_lines_re` to all plugins in `from_plugin_classname`, and use 'em

Change baseline format to replace `exclude_regex` with
```
"exclude":{
  "files": "foo",
  "lines": "bar"
}
```

Pass `exclude_lines_re` to all plugins in `from_plugin_classname`
Copy link
Contributor

@domanchi domanchi left a comment

Choose a reason for hiding this comment

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

Fix and ship!

tests/core/secrets_collection_test.py Outdated Show resolved Hide resolved
tests/core/secrets_collection_test.py Outdated Show resolved Hide resolved
detect_secrets/core/baseline.py Show resolved Hide resolved
detect_secrets/main.py Outdated Show resolved Hide resolved
detect_secrets/plugins/high_entropy_strings.py Outdated Show resolved Hide resolved
detect_secrets/plugins/common/yaml_file_parser.py Outdated Show resolved Hide resolved
tests/plugins/high_entropy_strings_test.py Outdated Show resolved Hide resolved
When `scan_diff`, used by `detect-secrets-server` calls `_extract_secrets_from_patch`
it uses `plugin.analyze_string`, and we were previously checking whitelist regexes in
plugins' `analyze` method.

:snake: Change all `_re` suffixes to `_regex`
Add a newline in plugins, particularly between class def and secret_type attribute
@KevinHock
Copy link
Collaborator Author

(took feedback into account and fixed a 🐛 that detect-secrets-server triggers 👍 )

@@ -39,7 +39,9 @@ class HighEntropyStringsPlugin(BasePlugin):

__metaclass__ = ABCMeta

def __init__(self, charset, limit, exclude_lines_re, *args):
secret_type = 'High Entropy String'
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary -- after all, there shouldn't be a secret_type, seeing that this class cannot (and should not) be constructed directly.

This also checks for it: https://github.com/Yelp/detect-secrets/blob/master/detect_secrets/plugins/base.py#L16

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@domanchi I am wondering, why has this test not required us to implement this before?

this class cannot (and should not) be constructed directly.

original_scanner = HighEntropyStringsPlugin(

from cc2d041

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah fair. Probably cause we didn't do a super call.

You can drop the issue. Probably easier this way, than creating a mock base class.

@KevinHock KevinHock merged commit 9f3d9ee into master Feb 9, 2019
@KevinHock KevinHock deleted the add_per_line_exclude branch March 21, 2019 22:03
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