Skip to content

Conversation

@francisluz
Copy link

πŸ—’οΈ Description

As a detect-secrets-server user, I would like to extract all pragmas in my codebase where I can validate the use of pragma: allowlist secrets.

πŸ’‘ Solution

Using the current process that loop through the code and plugin, one more step was added to extract the pragmas. This is an optional feature where on the server side will be triggered through a new flag called --extract-pragmas which will be pushed in its proper PR on detect-secrets-server repo.

⚠️ Caveats

As scan_diff is called only by detect-secrets-server this feature will be available only there.

βŒ› Time complexity

The performance impact on the scan if this feature is enabled, will be O(n+1) where extract-pragmas is considered an extra plugin run.

😎 Hope you guys like this feature, we're already using it in our internal CI process.
πŸ‘ Feel free to reach me out.

killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request Sep 17, 2020
Supports git-defenders/detect-secrets-discuss#402
@domanchi
Copy link
Contributor

Whoops! I realize this was never merged.

I'm going to go ahead and port this change to our pre-v1-launch branch, given that:

  1. This should have been merged before the effort to rearchitect the tool
  2. This is the feature that we needed, but never got around to it.

@francisluz : mind being the reviewer for the change, so we can verify that it works as designed?

@francisluz
Copy link
Author

francisluz commented Nov 11, 2020

Hey @domanchi,

I don't mind at all πŸ‘

I also need to create another PR to server to include some changes there.

@domanchi
Copy link
Contributor

Actually, now that I think about this more @francisluz, what's the difference between this feature, and:

grep -rE '(allow|white)list secret'

@francisluz
Copy link
Author

francisluz commented Nov 11, 2020

Actually, now that I think about this more @francisluz, what's the difference between this feature, and:

grep -rE '(allow|white)list secret'

Hey @domanchi ,

Well, I actually started this by using the grep which is not much different, but I had some issues because our Jenkins runs some sensitive data masking on it, then I couldn't generate the output list with the real values.

Then I had to implement it inside of the code where it was out of Jenkins scope.

@domanchi
Copy link
Contributor

Huh. Very interesting. I'm always curious to learn more about how different teams are using this tool in their various workflows.

Be on the look out for a PR for this early next week.

@francisluz
Copy link
Author

Awesome, definitely make sense as you guys have the big picture of the tool to understand more about its use.

Btw, just open the detect-secrets-server PR #68 for this feature, I was delaying it because there's a bunch of unit tests that is not passing and seems that they were there before.

Thanks @domanchi,

@domanchi
Copy link
Contributor

Apparently, it was easier to implement than I expected. @francisluz , be sure to check out the linked PR and verify it on your end manually, to see if I missed anything in the port. You can do so via:

$ pip install git+https://github.com/Yelp/detect-secrets.git@pre-v1-launch
$ detect-secrets scan --only-allowlisted

If you wanted to do it specifically with detect-secrets-server, you may need to do a bunch of tinkering around. I'd recommend just using detect-secrets as a package, and testing that way too. e.g.

from detect_secrets.core.scan import scan_for_allowlisted_secrets_in_diff

with open('filename') as f:
    diff = f.read()

for secret in scan_for_allowlisted_secrets_in_diff(diff):
    print(secret)

@domanchi
Copy link
Contributor

Closing as done, since I've already ported it in the linked review.

@domanchi domanchi closed this Dec 22, 2020
@francisluz
Copy link
Author

@domanchi Man, I would like to get some thoughts about how are you guys handling simples passwords, Like This list here.

I know that DS doesn't handle this, do you guys use a custom plugin? do you handle this at all?

Thanks a mil

jimmyhlee94 pushed a commit to jimmyhlee94/detect-secrets that referenced this pull request Aug 19, 2021
Supports git-defenders/detect-secrets-discuss#402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants