Interactive Mode

[dclayton-godaddy]

Feature Request

To boost the time it takes takes ignore false flags, I propose we add an interactive mode --interactive where the CLI will prompt on each issue providing the ability to ignore by file, signature, or specify a regular expression. Each of these ignores will be added to one of the ignore files and be automatically added to the context ignore similar issues going forward.

NOTE: I have started development on this and would love to get some feedback before I get too far along on this. Thanks!

Is your feature request related to a problem? Please describe.

Running tartufo on a repo can lead to 300K+ lines of stdout to sift through. You find something, ignore it, run it again. Repeat. This is a long process.

Describe the solution you'd like

Add --interactive flag that will prompt on each issue showing a region around the identified secret.

~~~~~~~~~~~~~~~~~~~~~
Reason: High Entropy
Filepath: README.md
Signature: 49d8990db16295778fdb98560s98d7fca9161c88423b6dc5cc4ffce74521483
...
npm run ui-test-widget

// Wildcard with testWidget
npm run ui-test-widget about @test/widget/manifest-409fc5dac7ea9ac53836a8bf2002e7ae.js

...
~~~~~~~~~~~~~~~~~~~~~
Action (IF=Ignore File, IS=Ignore Signature, IR=Ignore Regex, F=flag)?  (IF, IS, IR, F): if

Ignore regular expression IR will prompt for the regular expression. It will validate the regex on the matched content. Empty will return back to Action prompt.

Ignore Regular Expression (empty=choose another action): .*/widget/manifest-[a-z0-9]{32}\.js

Flag F will append the flagged secret to a tartufoflagged.json file.

{
  "signature1": {
     "path": "config/production.json",
     "line": 21,
     "commit": "commit1"
  }
}

Describe alternatives you've considered

None available.

Teachability, Documentation, Adoption, Migration Strategy

[jgowdy]

This seems like a really good idea. 👍

[tarkatronic]

This does sound like a great idea! Thanks for starting the work on this @dclayton-godaddy.

I think that there are a few things we are going to want to work through to be sure that we get this implemented in the best way for all cases.

Some things that come to mind right off:

• Should this apply to all commands or just a sub-set? For instance, would this apply to pre-commit?
• How do we decide what config file to edit? When invoking tartufo, it will detect either a tartufo.toml or pyproject.toml in the current directory or any of its parents. You can also specify -c/--config to point to a different file. Additionally, when a scan starts, it will first search the target for a config file, specifically looking for its inclusions/exclusions (see Bugfix: Load config from scanned repos #103 -- this re-implements functionality that accidentally got dropped in the work up to 2.0)
  • Scan targets like remote repos can especially complicate this process
• I would prefer to shorten the prompt responses to single letter; that seems to be a more conventional usage pattern.
• Maybe also instead of outputting the full help text on every match, add a ? option which will display the help text in a long format
• Should this become default behavior?
  • What about non-interactive scenarios like CI

[dclayton-godaddy]

• Should this apply to all commands or just a sub-set? For instance, would this apply to pre-commit?

I guess pre-commit could support this, however you wouldn't use this argument in your pre-commit hook.

• How do we decide what config file to edit? When invoking tartufo, it will detect either a tartufo.toml or pyproject.toml in the current directory or any of its parents. You can also specify -c/--config to point to a different file. Additionally, when a scan starts, it will first search the target for a config file, specifically looking for its inclusions/exclusions (see Bugfix: Load config from scanned repos #103 -- this re-implements functionality that accidentally got dropped in the work up to 2.0)
  • Scan targets like remote repos can especially complicate this process

Good point. Maybe the added entries need to be put to an interactive results file so the user can put them where they want?

• I would prefer to shorten the prompt responses to single letter; that seems to be a more conventional usage pattern.
• Maybe also instead of outputting the full help text on every match, add a ? option which will display the help text in a long format
• Should this become default behavior?

I thought about that but, due to security, I did not want accidental selection.

• What about non-interactive scenarios like CI

This would function similar to other tools that accept interactive. If you add --interactive in an environment that does not support interactive, I suppose you'd get the same results as other tools.

[jgowdy]

I thought about that but, due to security, I did not want accidental selection.

One mitigation for this concern might be that the user will have to git add (hopefully -p) the modified tartufo configuration, so if they're reviewing what they're commiting, errors in the exceptions flagging would potentially be caught.

Should this apply to all commands or just a sub-set? For instance, would this apply to pre-commit?

I think interactive would or should be incompatible for use with pre-commit.

How do we decide what config file to edit?

Since it's interactive, we could resolve the available / used configuration files with the existing logic and then present the user with a selection of files to choose from?

[dclayton-godaddy]

I thought about that but, due to security, I did not want accidental selection.

One mitigation for this concern might be that the user will have to git add (hopefully -p) the modified tartufo configuration, so if they're reviewing what they're commiting, errors in the exceptions flagging would potentially be caught.

if they're reviewing what they're commiting hopefully. For security, I'd rather not fall on the side of assumptions. Another benefit is that if we only use a single character, that limits our ability to perform actions that might start with the same letter.

I'd also have to reference another non-standard that got past the "single character" reviewers... https://github.com/godaddy/tartufo/blob/master/tartufo/cli.py#L97

Should this apply to all commands or just a sub-set? For instance, would this apply to pre-commit?

I think interactive would or should be incompatible for use with pre-commit.

Since there is no harm in supporting it, why not allow a dev to scan the pre-commit with interactive? Interactive only adds value.

How do we decide what config file to edit?

Since it's interactive, we could resolve the available / used configuration files with the existing logic and then present the user with a selection of files to choose from?

That works too. I'd hate to be prompted on every issue, however some issues might be better suited to be in a repo ignore file.

[jgowdy]

if they're reviewing what they're commiting hopefully. For security, I'd rather not fall on the side of assumptions.

True, true.

Since there is no harm in supporting it, why not allow a dev to scan the pre-commit with interactive? Interactive only adds value.

I haven't seen any other pre-commit hooks that were interactive, but if it works, that's cool.

[tarkatronic]

I'd also have to reference another non-standard that got past the "single character" reviewers... https://github.com/godaddy/tartufo/blob/master/tartufo/cli.py#L97

This was a choice made specifically after a design discussion, to avoid another common convention. #11 (comment) -- but the convention I'm referring to here is specifically with interactive CLI loops like this one. For example, when using git add -p you get the prompt:

(1/1) Stage this hunk [y,n,q,a,d,e,?]?

I think interactive would or should be incompatible for use with pre-commit.

Since there is no harm in supporting it, why not allow a dev to scan the pre-commit with interactive? Interactive only adds value.

Yeah if it works, that could be interesting. I'm just wondering how intuitive this would end up being. Let's say you've got interactive = True in your tartufo.toml, this will get picked up during the initial config scan and pre-commit will magically become interactive. This isn't necessarily a bad thing, if it works. But... is that what we want?

How do we decide what config file to edit?

Since it's interactive, we could resolve the available / used configuration files with the existing logic and then present the user with a selection of files to choose from?

That works too. I'd hate to be prompted on every issue, however some issues might be better suited to be in a repo ignore file.

What if, instead of automatically appending to a file, we give the user the exclusions to add as output at the end of the run?

[tarkatronic]

That git add -p example actually brings up another point: There should probably be a q option to quit out.

[dclayton-godaddy]

I'd also have to reference another non-standard that got past the "single character" reviewers... https://github.com/godaddy/tartufo/blob/master/tartufo/cli.py#L97

This was a choice made specifically after a design discussion, to avoid another common convention. #11 (comment) -- but the convention I'm referring to here is specifically with interactive CLI loops like this one. For example, when using git add -p you get the prompt:

(1/1) Stage this hunk [y,n,q,a,d,e,?]?

I think interactive would or should be incompatible for use with pre-commit.

Since there is no harm in supporting it, why not allow a dev to scan the pre-commit with interactive? Interactive only adds value.

Yeah if it works, that could be interesting. I'm just wondering how intuitive this would end up being. Let's say you've got interactive = True in your tartufo.toml, this will get picked up during the initial config scan and pre-commit will magically become interactive. This isn't necessarily a bad thing, if it works. But... is that what we want?

How do we decide what config file to edit?

Since it's interactive, we could resolve the available / used configuration files with the existing logic and then present the user with a selection of files to choose from?

That works too. I'd hate to be prompted on every issue, however some issues might be better suited to be in a repo ignore file.

What if, instead of automatically appending to a file, we give the user the exclusions to add as output at the end of the run?

I'm not sure we want to remove a feature on the assumption that someone is going to specify interactive in a scenario where it would not be interactive. As developers, there are plenty of situations where we could be caught in this situation yet we know not to do it. (git rebase -i ..., read -s MYPWD). And when I say pre-commit, I mean the dev has the ability to run tartufo --interactive pre-commit before actually attempting the commit. If it's a large commit, say merging in a lot of code from somewhere else, they would have an interactive way of ignoring false positives only on the staged files. Their pre-commit failure could even display a message run tartufo --interactive pre-commit to review.

[dclayton-godaddy]

What if, instead of automatically appending to a file, we give the user the exclusions to add as output at the end of the run?

I thought about that at first, but if someone was halfway through a review and had to CTRL-C, they would lose all their progress.