nickdeis / eslint-plugin-no-secrets

An eslint plugin to find strings that might be secrets/credentials
MIT License
127 stars 4 forks source link

[feature request] Blacklist particular statements #1

Closed ivan-aksamentov closed 5 years ago

ivan-aksamentov commented 5 years ago

Description

I would like to be able to exclude strings in some statements from consideration, no matter what entropy level they have:

Motivation

For example, the rule is triggered for this string:

const webpackFriendlyConsole = require('./config/webpack/webpackFriendlyConsole')

The entropy of './config/webpack/webpackFriendlyConsole' is 4.1, however it is an obvious false positive.

Possible interface

The corresponding options could look for example like this:

  'no-secrets/no-secrets': [
    'warn',
    {
      tolerance: 5,
      additionalRegexes: {},
      ignoreContent: [
        /.*some high-entropy text that is not a secret: bla bla bla.*/,
      ],
      ignoreImports: true,
      ignoreCommonjsRequires: true,
      ignoreCssClassNames: true,
      ignoreVariableNames: [/^NOT_A_SECRET_.*$/, 'highEntropySample'],
    },
  ],

Additional considerations

Although some of the blacklisting can, of course, can be achieved by selectively disabling the rule for a line or file or even adding particular files to override with this rule disabled, it would be awesome to have some sort of centralized control and additional customization, as described above.

In any case, thanks for the great plugin! ;)

nickdeis commented 5 years ago

Hey Ivan, Thanks for your kind words and taking the time to write this feature request. I think this idea is fantastic, considering a big goal of mine was to make entropy detection that was specific to javascript that truffleHog was struggling with. Can I ask a massive favor from you? Do you have some examples of CSS classes that are failing? I first want to make sure the entropy detection has a dictionary that is suitable for javascript problems. I will probably still implement this feature, since I was thinking of something along these lines while I was first working on this plugin, so clearly there is a need for it. Many Thanks, Nick

ivan-aksamentov commented 5 years ago

Hey @nickdeis ! So far I did not have any issues with CSS personally. Just added it for the sake of completeness, but it is not the top priority in my wish-list. I think in most cases it will be sufficient to detect className React prop in snippets like:

<SomeReactComponent>
  <div className="hey-it-s-a-css-class-not-a-secret and-neither-this-one" >
    {/* some stuff here */}
  </div>
</SomeReactComponent>

and the same, but with braces:

<SomeReactComponent>
  <div className={"hey-it-s-a-css-class-not-a-secret and-neither-this-one"} >
    {/* some stuff here */}
  </div>
</SomeReactComponent>

Anyway it will be great to add imports and requires first.

nickdeis commented 5 years ago

Hey Ivan, Thank you for the examples!

I'm going to do the following

  1. Update the entropy dictionary to ignore the characters /.-. These aren't common in secrets so I think it will be fine (expect for maybe /, but I think it's okay if we ignore it)
  2. Add ignoreContent as an option (this would be generally useful).
  3. Test this on a few large JSX codebases.

If #3 fails, I will add the other options as well.

Thank you so much for helping me this project better!

ivan-aksamentov commented 5 years ago

For imports:

const <something> = require(<path>)
import <something> from <path>

wouldn't it be easier and more robust to blacklist the <path> strings by retrieving these import statements in eslint's AST?

Maybe we could borrow from eslint-plugin-import? Here is an example for require(): https://github.com/benmosher/eslint-plugin-import/blob/45bfe472f38ef790c11efe45ffc59808c67a3f94/src/core/staticRequire.js#L1-L10

nickdeis commented 5 years ago

Hey Ivan, After doing the above, I think I'm inclined to agree. There were still a lot of false positives due to path strings. Thank you for the reference material. I think I will keep the tuned dictionary and the ignoreContent option, but add an ignorePathes option, which will ignore values from import,require(), and import(). Does that seem reasonable? Thanks, Nick

nickdeis commented 5 years ago

Hey Ivan, I wanted you to know that I've added ignoreModules in the latest release 0.2.3. Thank you for your patience, and let me know how this feature works out for you. I will keep this issue open until I implement something like ignoreVariableNames (will probably be called ignoreIdentifier since I would want include object field names). Thank you for your help, Nick

nickdeis commented 5 years ago

Hey Ivan, I recently implemented ignoreIdentifiers in the latest release 0.3.3. This can be used to ignore variable names and property literals. I will leave this ticket open for a few more days for any feedback you may have for me. Thank you! Nick