Need option to Mark false positives in the scan mode

[rmkanda]

Option to Mark false positives in the scan mode

When scanning the whole git tree, we should have option to mark false positives. it should be at the commit level, there can be changes the actual secret introduced in the newer commits.

Solution:

In the .talismanrc we can add an option to ignore files in a commit and while scanning we can exclude the file from identifying issues.

scanignoreconfig:
- commit: b1af11c016038dd47df4cdcd676f919fabacab5e
  filename: danger.pem
  reason: "This is just a test file not an actual key"

Workflow

1) Scanning a Repo - CI

• Developer add talisman in the CI
• Talisman produces list of issues
• Developer reviews the issues and updates the ignore rc file
• Makes a new commit
• Runs the scan again, there should not be any issues shown

2) Developer making New commit with a false positive issue

• Developer installs talisman in local machine with commit hook option
• Developer makes a commit
• Talisman produces list of issues
• Developer reviews the issues and updates the ignore rc file - fileignoreconfig section
• Makes a the commit
• Once the commit succeeds. Developer has to add a commit by including this in the scanignoreconfig section of RC.
• Pushes the code changes
• Talisman runs in the CI and honours the rc file.

cc: @selvakn @ashokgowtham

[rmkanda]

@harinee @svishwanath-tw Could you please review the suggestion ?

[svishwanath-tw]

@rmkanda : It seems to me that adding a reason is for user knowledge transfer and no verification of the reason takes place from talisman's side in which case, why can't the reason be a comment ?

[rmkanda]

@svishwanath-tw Regarding naming, We can call it as reason or comment. If you are referring to code comments: One challenge with that is we won't be able to include it in the reports. For example, We want to include all the false positive issues a HTML report to submit for an audit or Security Team review. We can support this in the HTML report if we have it in the yaml.

[kishaningithub]

I see scan mode as something that catches things if a developer say does something like --no-verify and pushes the code.

I would like the same ignore config for both pre commit hook and the scan mode

Tagging the other issue i opened as duplicate https://github.com/thoughtworks/talisman/issues/343

[harinee]

We understand the need, but currently the scan mode for Talisman is good as a 1-time execution to catch the obvious misses. It is not CI pipeline-ready yet, and we acknowledge that. However, Talisman has a niche in preventions through pre-commit and pre-push hooks, where we are prioritising to enhance and make the experience even better. So this is currently a lower priority on our list. Until then, there are more tools that support scanning in pipelines, which do a great job too. So we recommend exploring those options till this feature gets built.

Having said that, a PR is more than welcome, and we would love to review if that comes in.