Issue #89: Update documentation for detect-secrets

[perryzjc]

Purpose

• The purpose of this Pull Request is to improve automated checking for general sensitive information within Git. It provides documentation, architecture graphs, and configurations to solve the needs proposed by the JPL community at the issue ticket discussion.

  Proposed Changes

• [ADD] Starter kit for detect-secrets under Static Application Security Testing section of continuous-testing/starter-kits/README.md

  Issues

• Links to relevant issues: #89

  Testing

• Sample use cases available in the documentation.
• Tested on the operating system :
  • macOS Big Sur, Version 11.1, Chip Apple M1
    • Layer 1: Full scan (client-side)
    • 
    • Layer 2: Git commit scan (client-side)
    • 
    • Ubuntu 20.04.2 ARM64
    • Layer 1: Full scan (client-side)
      • 
    • Layer 2: Git commit scan (client-side)
      • 
  • Windows 10 & 11
    • Layer 1: Full scan (client-side)
    • 
    • Although it is able to generate the baseline file, audit feature is unavailable on both Windows 10 & 11 during my testing. Error message mentions UnableToReadBaselineError. When I use the default Yelp/detect-secrets, the problem still exists. I will be further investigating this issue and open an issue ticket to Yelp/detect-secrets.
    • Layer 2: Git commit scan (client-side)
    • Similar to Layer 1, due to problem of reading baseline file, pre-commit hook is not working on Windows 10 & 11, which involves comparing the current commit with the baseline file.
  • Independent on operating system
    • Layer 3: GitHub.com (server-side)
    • Available on documentation Layer 3: Server-side Push to GitHub.com

Additional Notes

After discussion, I am using the forked detect-secrets for this documentation. Compared to the original detect-secrets from Yelp, my solution have additional plugins which are being reviewed by the Yelp/detect-secrets community. There are two pull requests for my customized plugins

• Add a plugin for EmailAddress
• Add a plugin for IPAddress (ipv4)

Other plugins are experimental version for now, and I will be further developing in the next two weeks and have them reviewed. Once all the plugins are reviewed and merged to the Yelp/detect-secrets, I will update the documentation to use the original Yelp/detect-secrets.

Check out the documentation for more details :)

[perryzjc]

I also have some updates on my findings about the "bug" discussed in my meeting with @riverma

Background

Below are the cases we discussed during the meeting. Rishi and I both got the same results for each case.

1. Three secret types (one built-in plugin, two my custom plugins)

built-in plugin:

1. KeywordDetector (sample test file available here)

my custom plugins:

1. EmailAddressDetector (sample test file available here)
2. AWSSensitiveInfoDetectorExperimental (sample test file available here)

sample file:

e-mail: a_email@jpl.nasa.gov

username: myuser
password: mypass

sg-12345678

After running command

detect-secrets scan ./ --all-files --exclude-files '.secrets.*' --exclude-files '.git*' > .secrets.baseline

the baseline file only showed two results (supposed to show all three) from my custom plugin:

2. Two secret types (one built-in plugin, one my custom plugins)

built-in plugin:

1. KeywordDetector (sample test file available here)

my custom plugins:

1. AWSSensitiveInfoDetectorExperimental (sample test file available here)

sample file:

username: myuser
password: mypass

sg-12345678

After running the same command, the baseline file only showed one result from my custom plugin:

3. One secret type (one built-in plugin)

built-in plugin:

1. KeywordDetector (sample test file available here)

sample file:

username: myuser
password: mypass

After running the same command, the baseline file only showed one result from the built-in plugin:

Thoughts

During the meeting, we thought the "bug" came from my custom plugins based on the above results. After the meeting and more test cases, I think it's a common issue, which also occurs for built-in plugins only case

More tests

4. Two secret types (two built-in plugins)

built-in plugin:

1. KeywordDetector (sample test file available here)
2. PrivateKeyDetector (sample test file available here)

sample file:

username: myuser
password: mypass

'-----BEGIN RSA PRIVATE KEY-----\n'
'super secret private key here\n'
'-----END RSA PRIVATE KEY-----'

After running the same command, the baseline file only showed one result from the built-in plugin (PrivateKeyDetector):

5. Two secret types (one built-in plugin and one custom plugin)

built-in plugin:

1. PrivateKeyDetector (sample test file available here)

my custom plugins:

1. AWSSensitiveInfoDetectorExperimental (sample test file available here)

sample file:

sg-12345678

'-----BEGIN RSA PRIVATE KEY-----\n'
'super secret private key here\n'
'-----END RSA PRIVATE KEY-----'

After running the same command, the baseline file successfully showed both results

6. Two secret types (one built-in plugin and one custom plugin)

built-in plugin:

1. GitHubTokenDetector (sample test file available here)

my custom plugins:

1. AWSSensitiveInfoDetectorExperimental (sample test file available here)

sample file:

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

sg-12345678

After running the same command, the baseline file successfully showed both results

Assumption and Conclusion

Based on further tests 5 and 6, detect-secrets successfully showed all secret types even if there is a custom plugin. So far, all the buggy test examples occur when there is a KeywordDetector

It can probably be a bug from the official 'Yelp/detect-secrets` or a feature. But anyway, this part is what I need to ask 'Yelp/detect-secrets' to improve

Although this "bug" exists at the moment, detect-secret is still able to tell us where there is a Keyword detected. The only inconvenience is that we need to let it be the only secret type existing in a single file. But as we clean the otehr secret types further, we will find it eventually.

[riverma]

I also have some updates on my findings about the "bug" discussed in my meeting with @riverma

Great analysis @perryzjc! This clarifies some unexpected behavior (we think) from detect secrets. Not too worried, since this is only relevant to a single file, and resolving one of the secrets in code should make the other plugins catch other secrets within the same file. However, this discussion is great to point developers at detect secrets to, for clarification whether a bug ticket should be filed.

[jpl-jengelke]

+1 Let's merge this PR...

[riverma]

Approved by @riverma and @jpl-jengelke. Merging.