talisman icon indicating copy to clipboard operation
talisman copied to clipboard

.talismanrc ignoreconfig does not disable filecontent detector

Open ghost opened this issue 6 years ago • 6 comments
trafficstars

I tried to disable the filecontent detector for a specific file as detailed in readme, talisman seems not to pick up a ignore_detectors: [filecontent] directive for a file. version: talisman v0.4.3

reproduce

  • have talisman installed globally (I followed instructions in readme)
  • clone https://github.com/foo-tw/talisman-noignore-reproduce
  • modify the test file (in my case, added a blank line between existing lines 1&2)
  • run git add -u && git ci -mmodified

expected

because of the .talismanrc configuration, file test passes checks and git commits without problem

actual

Talisman Report:
+------+--------------------------------+
| FILE |             ERRORS             |
+------+--------------------------------+
| test | Potential secret pattern :     |
|      | AWS_SECRET_KEY_ID=             |
+------+--------------------------------+

If you are absolutely sure that you want to ignore the above files from talisman detectors, consider pasting the following format in .talismanrc file in the project root
fileignoreconfig:
- filename: test
  checksum: f6412e100630a0848de579c5bdb151a6c820b0d3a4c2c6a3f3c9cc6df67551f3
  ignore_detectors: []

am I misunderstanding something in the configuration?

ghost avatar Feb 27 '19 15:02 ghost

This is a feature of talisman. It ignores files present in the .talismanrc until you change them. If a file present in the .talismanrc is changed, talisman scans again, and you if you are sure the file doesn't contain any secrets, you have to copy the 'fileignoreconfig' again into the .talismanrc file to commit/push. The problem talisman is solving by doing this is: If a person1 adds a test file and ignores at one instance and there are many other people changing the test file, it should be able to scan the changes or in this case, the entire file.

mabaritw avatar Apr 19 '19 08:04 mabaritw

I see. Am I then understanding correctly that the checksum property is required for any fileignoreconfig block? what I tried to reproduce in my example was leaving it blank in an attempt to permanently ignore a single detector in a single file. I take this is not possible in talisman then?

Also, if checksum is always required I would suggest to make it more explicit in the readme (and maybe with a warning/error from talisman too?) and would question what's the purpose of being able to turn off a specific detector if every change to a file must be explicitly accepted anyway.

ghost avatar May 10 '19 13:05 ghost

Yes, permanently ignoring a detector in a file is not possible. In the README, under Ignoring Files, the specifications of the talismanrc and checksum are mentioned.

If talisman throws some errors, and suppose you ignore the filename detector, you would want talisman to scan the contents of the file. However, this helps once per commit/push, you still have to update the checksum when the file is changed.

mabaritw avatar May 15 '19 08:05 mabaritw

Hey all. Just checking; is this an issue? Should we close it?

aj-hamilton avatar Mar 26 '20 05:03 aj-hamilton

IMHO, scopes are not a sustainable way of managing these sorts of issues.

Case in point: I'm working on a react-native app; it uses cocoapods which has a lockfile of its own with lots of encoded hex, and where no secrets will slip in. So now I need a new scope that is like node but also includes ios/Podfile.lock.

Do you really want to be in the business of customizing talisman for every single environment out there? I don't care how awkward and difficult you make it to designate a file or set of files as "not dangerous" and you can put all the warnings in the world in the docs, all to make sure we do know what we're doing, but just give us an escape hatch for when we do know what we are doing.

mitchh avatar Aug 22 '20 01:08 mitchh

Or maybe scopes themselves, beyond some set of prepackaged ones that you care to define, could be user-definable so the intent is crystal clear?

mitchh avatar Aug 22 '20 01:08 mitchh