Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add gitguardian precommit checks #2109

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

halfwhole
Copy link
Collaborator

@halfwhole halfwhole commented Dec 2, 2022

Problem

Devs like me may accidentally commit and push secrets to GitHub

Solution

Add GitGuardian pre-commit check to detect secrets and stop commits with secrets from being made. Devs are still able to manually override these checks if it's a false positive.

To enable secrets detection, the user must:

  • Install ggshield on their local machine (unfortunately, no npm package is available)
  • Set GITGUARDIAN_API_KEY in .env

If neither of those are set up, the secrets detection check will be skipped.

Screenshots

Example of what happens when a secret is exposed (this test credential has already been revoked):

Screenshot 2022-12-02 at 10 47 51 AM

@halfwhole halfwhole force-pushed the chore/add-gitguardian-precommit-check branch from 84cf638 to 239aa97 Compare December 7, 2022 02:56
Copy link
Contributor

@gweiying gweiying left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code change looks good to me, but we should somehow enforce that everybody installs this to reap the rewards!

@halfwhole
Copy link
Collaborator Author

halfwhole commented Dec 20, 2022

Would any of these approaches be a good idea?

  1. Make it compulsory for all devs, i.e. pre-commit check fails if gitguardian is not set up locally
  2. Make it optional, but change the echo'ed message to a warning that strongly recommends setting up gitguardian if not already done so
  3. Make it optional, just discuss internally within our team to ensure that we set this up

Edit: oops, realised that by "everybody" you might've meant not just everyone who's working on this project, but for everyone else in ogp as well - dat and I were just discussing about whether that's doable

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