Rewrite of detect-secrets guide

[riverma]

Purpose

• Simplified guide significantly based on user feedback
• Utilizing new SLIM best practice guide template format

  Proposed Changes

• [CHANGE] README contents
• [FIX] GitHub Action workflow was displaying code output instead of executing code during scan

  Issues

• 101

  Testing

• Locally tested with sample repository: full scan, pre-commit hook
• GitHub.com Action not quite working with test-repo. Considering to marking feature as experimental.

[riverma]

@perryzjc - FYI we received reader feedback that encouraged us to consider simplifying the detect-secrets guide significantly. I've gone ahead and taken your hard work and rewritten the guide to make it shorter and a bit less busy. Let me know what you think.

Also - couple things regarding this guide that I think we still need to resolve:

• 99

• The GitHub.com Action doesn't seem to work right. See my comment in the above testing section.
• We should try and follow-up on your https://github.com/Yelp/detect-secrets/pulls/perryzjc PRs, including pushing your experimental ones upstream

[perryzjc]

@perryzjc - FYI we received reader feedback that encouraged us to consider simplifying the detect-secrets guide significantly. I've gone ahead and taken your hard work and rewritten the guide to make it shorter and a bit less busy. Let me know what you think.

Also - couple things regarding this guide that I think we still need to resolve:

• [Bug]: False positive alerts in AbsolutePathDetectorExperimental plugin #99
• The GitHub.com Action doesn't seem to work right. See my comment in the above testing section.
• We should try and follow-up on your https://github.com/Yelp/detect-secrets/pulls/perryzjc PRs, including pushing your experimental ones upstream

@riverma Thanks for the summary! I'm free both this weekend and next to tackle the issues you've outlined. School's been a bit hectic, but with midterms just finished, I would be happy to dive in and support the project!

[riverma]

@perryzjc - FYI we received reader feedback that encouraged us to consider simplifying the detect-secrets guide significantly. I've gone ahead and taken your hard work and rewritten the guide to make it shorter and a bit less busy. Let me know what you think. Also - couple things regarding this guide that I think we still need to resolve:

• [Bug]: False positive alerts in AbsolutePathDetectorExperimental plugin #99
• The GitHub.com Action doesn't seem to work right. See my comment in the above testing section.
• We should try and follow-up on your https://github.com/Yelp/detect-secrets/pulls/perryzjc PRs, including pushing your experimental ones upstream

@riverma Thanks for the summary! I'm free both this weekend and next to tackle the issues you've outlined. School's been a bit hectic, but with midterms just finished, I would be happy to dive in and support the project!

Thanks @perryzjc - that's great to hear! By the way, Yelp got back to you on your PRs 🚀 .

In terms of this PR, I think the only thing blocking would be the "The GitHub.com Action doesn't seem to work right. See my comment in the above testing section." line above. I wonder if that could be prioritized if you have the time? Otherwise - we can also just remove that layer for now and add it back when you have thoughts on it. Thanks for all your help!

[perryzjc]

@perryzjc - FYI we received reader feedback that encouraged us to consider simplifying the detect-secrets guide significantly. I've gone ahead and taken your hard work and rewritten the guide to make it shorter and a bit less busy. Let me know what you think. Also - couple things regarding this guide that I think we still need to resolve:

• [Bug]: False positive alerts in AbsolutePathDetectorExperimental plugin #99
• The GitHub.com Action doesn't seem to work right. See my comment in the above testing section.
• We should try and follow-up on your https://github.com/Yelp/detect-secrets/pulls/perryzjc PRs, including pushing your experimental ones upstream

@riverma Thanks for the summary! I'm free both this weekend and next to tackle the issues you've outlined. School's been a bit hectic, but with midterms just finished, I would be happy to dive in and support the project!

Thanks @perryzjc - that's great to hear! By the way, Yelp got back to you on your PRs 🚀 .

In terms of this PR, I think the only thing blocking would be the "The GitHub.com Action doesn't seem to work right. See my comment in the above testing section." line above. I wonder if that could be prioritized if you have the time? Otherwise - we can also just remove that layer for now and add it back when you have thoughts on it. Thanks for all your help!

@riverma Got it. When I have time this week, I will prioritize addressing the GitHub Action part and let you know how it goes. Thanks!

[perryzjc]

@riverma Hi, I have an update regarding the issue with the GitHub Action not working properly. After conducting some experiments, I believe the GitHub Action is actually functioning as expected. The primary issue stems from the secret detection process for passwords.

I ran detect-secrets locally to test the testfile in the main branch of your test-repo. Interestingly, it did not identify the test password PASSWORD: aijewga@#!%^ahgfdndbks211 as a secret:

However, when I enclosed the password in quotes, making it PASSWORD: "aijewga@#!%^ahgfdndbks211", it became detectable, and thus could be blocked by the GitHub Action.

I reviewed the relevant test file for the keyword plugin from Yelp/detect-secrets, and noticed that their test cases sometimes do not cover scenarios where the password is not enclosed in quotes:

Additionally, I looked into the source code of keyword.py and found that it is designed this way due to some heuristic reasons. While this approach makes sense to a certain extent, there might be room for improvement, which is something I am considering exploring in the future.

In summary, the GitHub Action is functioning properly; however, detect-secrets may not cover all types of secrets perfectly, which is an area that could potentially be improved. I hope you find this update helpful!

[perryzjc]

Hi @riverma,

I've had the chance to go through the updates you made to the new detect-secrets guide, and I'm quite impressed! The guide is more organized and concise with careful attention to detail in the notes and FAQs. It's been a great learning experience in terms of professional technical writing.

I have some thoughts and suggestions that could potentially contribute to the guide, and I'd like to share them with you for your consideration:

1. Custom vs. Official Detect-Secrets: The guide doesn't explicitly highlight the differences between our customized version of detect-secrets and the official release. I think it could be beneficial for first-time users to understand these distinctions, particularly in regards to any additional plugins we support. A screenshot from the original guide:

   

2. Status of Custom Plugins: Following up on the previous point, would it make sense to inform users that our custom plugins are currently pending approval to be merged with the official detect-secrets repository?

3. Inclusion of Original Notes: I noticed that an original note from our previous guide hasn't made its way into the new one. This note helps to set the right expectations regarding detect-secrets behavior. Below are screenshots for comparison:

   • Original Guide:
   • Official detect-secrets Guide:

   These two screenshots illustrate that not every detectable secret is shown in the results to minimize noise:

   • 
   • 

4. Including Original Guide Screenshots: I'm contemplating whether including some visuals from the original guide could enhance clarity, even though it might potentially clutter the document. What are your thoughts on this?

5. Lastly, I wanted to bring up a feature that seems to be garnering interest – scanning the entire git history. A lot of people, myself included, seem intrigued by this possibility:

While this has been a topic of discussion since 2020, with truffleHog being suggested as an alternative, it doesn't support the exact same plugins as detect-secrets. That's why I've recently reopened this discussion with the aim of potentially bringing this feature to our project. I understand that this could take some time, but I’ll make sure to keep you updated, and we can adjust our guide accordingly once there’s progress.

Looking forward to hearing your thoughts!

[riverma]

@perryzjc - first of all thank you for taking the time to do this deep-dive investigation and testing of the guide!

• Appreciate you clarifying that the GitHub Action is in fact working as “expected”. Highly suggest you take the notes / testing you performed on keyword.py and submit a bug at Yelp/detect-secrets. I’ll make sure to keep the GitHub Action section in our guide regardless.
• You bring great points about details that weren’t ported over to the new version of the guide. Let me take a crack at addressing your concerns in another iteration. I’ll reach out to you for feedback before we can finalize.
• For git history scanning - perhaps for now we can clarify in the FAQ that tools like trufflehog may be more suited? What do you think - is it up to the same standards as detect-secrets in a way that would make the reader easily able to try that tool? One thing I’m particularly concerned about is how to actually go about cleaning the Git history even if violations are found - do you have thoughts on that front so that we could recommend tips in our FAQ?

Again - super appreciate your insights and dedication to supporting your contributed guide!

[perryzjc]

@perryzjc - first of all thank you for taking the time to do this deep-dive investigation and testing of the guide!

• Appreciate you clarifying that the GitHub Action is in fact working as “expected”. Highly suggest you take the notes / testing you performed on keyword.py and submit a bug at Yelp/detect-secrets. I’ll make sure to keep the GitHub Action section in our guide regardless.
• You bring great points about details that weren’t ported over to the new version of the guide. Let me take a crack at addressing your concerns in another iteration. I’ll reach out to you for feedback before we can finalize.
• For git history scanning - perhaps for now we can clarify in the FAQ that tools like trufflehog may be more suited? What do you think - is it up to the same standards as detect-secrets in a way that would make the reader easily able to try that tool? One thing I’m particularly concerned about is how to actually go about cleaning the Git history even if violations are found - do you have thoughts on that front so that we could recommend tips in our FAQ?

Again - super appreciate your insights and dedication to supporting your contributed guide!

@riverma Thank you for the acknowledgment!

• The password issue ticket already exists with some discussions ongoing, so I've added our case and asked for opinions there.
• I'm glad to hear that my points are helpful and am eager to continue assisting in finalizing the guide.
• I agree with including trufflehog in the FAQ. It caters to some common needs and is user-friendly, especially with the numerous examples in their Quick Start section. While detect-secrets and trufflehog share similarities, they differ in a few key areas. Detect-secrets, built with Python, allows for the creation of plugins with diverse functionalities and has a baseline file designed with enterprise clients in mind, providing a systematic, backward-compatible means of managing secrets. On the other hand, trufflehog excels in scanning history across various platforms like Git, GitHub Organization, Issue Comments (a recent feature), S3 bucket, GCS (Google Cloud Storage), Docker, and more. These features can be invaluable for the JPL community when performing a comprehensive one-time scan for secrets. Both tools support pre-commit hooks and GitHub Actions, with the main difference between the common functionalities lying in plugin creation and their approach to secret management. What are your thoughts on this? I'm more than willing to provide further assistance.
• Git history scanning is indeed trcky, but it seems manageable based on my research by using git-filter-repo. I've initiated a discussion on cleaning history and shared some use cases in a new issue ticket over at the trufflehog community. Hopefully, we can gather more insights from that discussion.

Again, happy to help!

[riverma]

@perryzjc - appreciate your help and investigations! Great to hear the discussion has started regarding your findings.

I like your suggestions and I was thinking - would you mind just creating a PR off of the riverma-patch-2 branch with any suggestions you have for the guide, including your recommendations above? I feel like that will reduce the chance that I get your suggestions wrong and any back-and-forth.

To do this you can:

1. Fork the SLIM repository (looks like that's done already)
2. Create a new branch in perryzjc/slim to later pull contents from riverma-patch-2 (i.e. this PR branch) a. Open your fork b. Click on the branch dropdown c. In the search/filter box, type the name of the new branch you want to create within your fork d. You'll see a "Create branch: branch-name from main" option. Click on it. This will create the new branch in your fork based on your updated default branch
3. Pull the changes from nasa-ammos/slim and branch riverma-patch-2 to the branch you just created: a. Go to the "Pull requests" tab in your fork b. Click on "New pull request" c. Click on the "compare across forks" link d. For the branches being compared, make sure the base is the branch you just created in your fork and the compare branch is nasa-ammos/slim and branch riverma-patch-2. e. Click on "Create pull request" f. Merge the pull request to pull the changes from the original repo's new branch into the branch in your fork
4. Make your changes in your perryzjc/slim new branch and propose a PR against the nasa-ammos/slim repo's riverma-patch-2 branch (this PR)

[perryzjc]

@perryzjc - appreciate your help and investigations! Great to hear the discussion has started regarding your findings.

I like your suggestions and I was thinking - would you mind just creating a PR off of the riverma-patch-2 branch with any suggestions you have for the guide, including your recommendations above? I feel like that will reduce the chance that I get your suggestions wrong and any back-and-forth.

To do this you can:

1. Fork the SLIM repository (looks like that's done already)
2. Create a new branch in perryzjc/slim to later pull contents from riverma-patch-2 (i.e. this PR branch) a. Open your fork b. Click on the branch dropdown c. In the search/filter box, type the name of the new branch you want to create within your fork d. You'll see a "Create branch: branch-name from main" option. Click on it. This will create the new branch in your fork based on your updated default branch
3. Pull the changes from nasa-ammos/slim and branch riverma-patch-2 to the branch you just created: a. Go to the "Pull requests" tab in your fork b. Click on "New pull request" c. Click on the "compare across forks" link d. For the branches being compared, make sure the base is the branch you just created in your fork and the compare branch is nasa-ammos/slim and branch riverma-patch-2. e. Click on "Create pull request" f. Merge the pull request to pull the changes from the original repo's new branch into the branch in your fork
4. Make your changes in your perryzjc/slim new branch and propose a PR against the nasa-ammos/slim repo's riverma-patch-2 branch (this PR)

@riverma Sounds good and thank you for the detailed instructions. I will proceed to create the PR recently for your evaluation.

[riverma]

@perryzjc - I had some problems with Git branching (too many files appear changed), so I've gone ahead and made a fresh pull request with the finalized version of the guide: https://github.com/NASA-AMMOS/slim/pull/143

Please take a look! Thanks.