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

fix #8 - add --include and --exclude options #66

Merged

Conversation

milo-minderbinder
Copy link

@milo-minderbinder milo-minderbinder commented Nov 29, 2017

Fixes issue #8 by adding --include_paths and --exclude_paths options that allow the user to limit scanning to a subset of objects in the Git history by defining regular expressions (one per line) in a file to match the targeted object paths.

If provided, the --include_paths option should point to a file with regular expressions (one per line), at least one of which must match a Git object path in order for it to be scanned. If empty or not provided (default), all Git object paths are included (unless otherwise excluded via the --exclude_paths option).

Likewise, the --exclude_paths option, when provided, should point to a file with regular expressions, none of which may match a Git object path in order for it to be scanned. If empty or not provided (default), no Git object paths are excluded (unless effectively excluded via the --include_paths option).

In either file, lines starting with "#" are treated as comments and are ignored.

@milo-minderbinder
Copy link
Author

milo-minderbinder commented Nov 29, 2017

I realize this provides some duplicate functionality with #16 and #46, but since it also adds additional capabilities with the --include option, and also provides additional detailed documentation (and is based on current master & without merge conflicts currently), I figured it was worth a separate PR.

@dxa4481
Copy link
Collaborator

dxa4481 commented Dec 10, 2017

Hey @milo-minderbinder thanks for this PR. Would you mind adding some tests and examples to the tests.py script?

@milo-minderbinder
Copy link
Author

No problem, thanks for a useful tool. And sure thing, I’ll add some tests as soon as I get a chance.

* rename `--include` and `--exclude` options to `--include_paths`
  and `--exclude_paths`, respectively, in order to improve
  clarity of their function, especially in the case that
  string/result filtering options are added in a future release
* add tests for path filtering with inclusion and exclusion rules
* update README.md with new option names and help output
@milo-minderbinder
Copy link
Author

@dxa4481 done. let me know what you think

@sobolevn
Copy link

@dxa4481 any change to get this merged? I am trying to ignore my Pipfile.lock.

@codecov-io
Copy link

Codecov Report

Merging #66 into master will increase coverage by 6.52%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   31.85%   38.38%   +6.52%     
==========================================
  Files           4        4              
  Lines         248      297      +49     
==========================================
+ Hits           79      114      +35     
- Misses        169      183      +14
Impacted Files Coverage Δ
test_all.py 93.75% <100%> (+8.75%) ⬆️
truffleHog/truffleHog.py 27.34% <33.33%> (+0.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 461953f...38dc61d. Read the comment docs.

@milo-minderbinder
Copy link
Author

@dxa4481 I fixed the merge conflicts after seeing @sobolevn question. If you're not interested in this PR though, just let me know and I won't worry about fixing future conflicts and/or close the PR.

@dxa4481
Copy link
Collaborator

dxa4481 commented Jan 29, 2018

Hi @milo-minderbinder I haven't forgotten about this PR.

The thing is I just added support for custom rules being sourced from a json file, and I think it makes more sense to just have one settings file, which your includes and excludes can also take part in.

It's been laziness mostly, but the vision I have is, one json config settings file you can pass which will include both custom rules and includes and excludes.

I'll sit down hopefully this weekend or next and bang it out.

Thanks for being patient.

@milo-minderbinder
Copy link
Author

@dxa4481 No problem. So, to clarify, since this PR is now in conflict again and what you're describing would require either reworking this PR to use the JSON rules file as you've just described or closing this PR and starting from scratch, what do you want me to do? I can start trying to implement what you've described, but obviously don't want to go through that effort if you'd rather control the implementation yourself. It's your project, so either way you want to handle it is fine, just let me know. Thanks!

@dxa4481
Copy link
Collaborator

dxa4481 commented Jan 29, 2018

I can take care of it. I'll make sure when I do I merge in your change set so you get credit.

@samuelhwilliams
Copy link

samuelhwilliams commented Aug 28, 2018

Did this go anywhere? Just found this and it looks super useful, been getting a whole trove of false positives based on entropy from things like package.json and other high-entropy files. Being able to exclude those so that I can still get entropy scans on the other, more legit files, would be great.

@sean-heller
Copy link

would love to see this as well, for excluding submodules

@adrianbn
Copy link

This would have come in handy for a colleague today too. Is there anything I can do to help move this along?

@Demuxx
Copy link

Demuxx commented Nov 2, 2018

Any thoughts on merging this?

@dxa4481
Copy link
Collaborator

dxa4481 commented Nov 4, 2018

Hi everyone, this has been on my todo list for a while. My focus right now is on a Kiwicon presentation, but when that's done, I promise to prioritize this feature.

@poblahblahblah
Copy link

Would love to see this feature merged. How did Kiwicon go?

@provPaulBrousseau
Copy link

I would also love to see this merged in; some projects have minimized JS code from Swagger, which happens to trigger the Generic Secret regex.

@maniankara
Copy link

+1

@rahiparikh
Copy link

@milo-minderbinder Thanks for the PR and @dxa4481 for supporting the awesome tool. Would it be possible to merge the PR? It has great potential of helping the community.

@silasbw
Copy link

silasbw commented Apr 29, 2019

@dxa4481 we (@godaddy) would like to use trufflehog in our CICD pipelines. In order for that to work well for us, we need the ability to manually suppress false positives. Excluding certain files addresses a large percentage of false positives we are currently seeing (e.g., excluding self-signed development certs).

Can we help move this issue along? One thing we could do is spec out the work required for #66 (comment) (i.e., submit an issue to this repository with specific proposals on how to do that work/refactoring). Would that be helpful?

@dxa4481
Copy link
Collaborator

dxa4481 commented May 1, 2019

I'll see if I have some time this weekend

@dxa4481 dxa4481 merged commit 38dc61d into trufflesecurity:master May 6, 2019
@dxa4481
Copy link
Collaborator

dxa4481 commented May 6, 2019

Apologies for not getting to it sooner. Took a bit of work because of all the merge conflicts (because I took so long to merge it) but it's merged in and shipped to pypi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.