Replace --only-verified with --results #3643
Conversation
| @@ -185,7 +185,7 @@ This required Cosign binary to be installed prior to running installation script | |||
| Command: | |||
|
|
|||
| ```bash | |||
| trufflehog git https://github.com/trufflesecurity/test_keys --only-verified | |||
| trufflehog git https://github.com/trufflesecurity/test_keys --results=verified,unknown | |||
There was a problem hiding this comment.
Including both verified and unknown introduces people to the concept beyond simply --only-verified. Surfacing errors is an important part of using TruffleHog effectively.
If they don't like what they see, they can easily remove unknown.
main.go
Outdated
| results[value] = struct{}{} | ||
| case "filtered_unverified": |
There was a problem hiding this comment.
filtered_unverified is a bit wordy. I think filtered is better and fits in with the existing options.
pkg/engine/engine.go
Outdated
| @@ -354,6 +354,7 @@ func (e *Engine) setDefaults(ctx context.Context) { | |||
| } | |||
| e.notifyVerifiedResults = true | |||
| e.notifyUnverifiedResults = true | |||
| e.retainFalsePositives = true | |||
There was a problem hiding this comment.
"Why didn't TruffleHog find my secret" is a commonly asked question (e.g., #3579). I think it makes sense to log everything by default. Requiring people to know about and opt-in to a special flag can be confusing.
What do you think?
rgmz
force-pushed
the
misc/results-flag
branch
from
4784b7d to
6bb895b
Compare
|
@dustin-decker @rosecodym Thoughts? |
There was a problem hiding this comment.
Thanks for taking a look at this! I see three changes here:
- Changing the CLI and docs to use
--resultsinstead of--only-verified- And including
unknownresults in the documentation
- And including
- Renaming
filtered_unverifiedtofiltered - Disabling false positive elimination by default
I support all three of these changes, but I don't think that they should be combined into a single PR like this. I know each individual part is pretty simple, but this stuff has historically been a minefield of unexpected consequences, so I'd like to tread as lightly as possible. Would you mind splitting them up?
rgmz
force-pushed
the
misc/results-flag
branch
2 times, most recently
from
30d2a2a to
b4efd10
Compare
Sure. This is now solely the documentation + flag visibility change. |
rgmz
force-pushed
the
misc/results-flag
branch
2 times, most recently
from
3193587 to
45fa258
Compare
rgmz
force-pushed
the
misc/results-flag
branch
from
45fa258 to
507fddc
Compare
|
Sorry for the delay on reviewing this - I came down with pneumonia and this kind of got lost in the shuffle :( It mostly looks good to me, but I'm wondering how |
|
Zach says:
|
Description:
This PR hides
--only-verifiedand unhides--results. The latter has been in use for the better part of a year and seems to be working well behind the scenes.Switching the documentation to an explicit
--resultsflag will give people more control over what they see.Checklist:
make test-community)?make lintthis requires golangci-lint)?