isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

feat: add gitguardian hook and update readme #642

Closed harishv7 closed 1 year ago

harishv7 commented 1 year ago

Problem

While we do have checks post-commit to identify any secrets are committed, we should also attempt to prevent this even before a commit can occur.

After considering some solutions - https://github.com/awslabs/git-secrets and Git Guardian, we decided to go with GitGuardian since we are already using it.

Solution

We add Git Guardian as a pre-commit hook which scans commits for any leaked secrets before actually performing the commit.

Breaking Changes

There is a breaking change for local development since this commit makes Git Guardian setup somewhat mandatory.

New dev dependencies:

No new dependencies are added to our package. Git Guardian will be installed on the OS-level instead as a brew package.

harishv7 commented 1 year ago

Running ggshield locally creates a.cache__ggshield file. By right, since this is a pre-commit hook, there isnt a real need to run ggshield directly. However, if there was a false positive and someone runs ggshield directly, I dont think we should be committing the .cache__ggshield to remote. I suggest to defensively add .cache__ggshield into our .gitignore, wdyt?

In addition to this, could we add this commit to develop BE + FE vapt + FE develop?

@kishore03109 makes sense, added it to gitignore.

As for the FE, yes definitely we will be adding it. Just wanted to address comments on this PR, before I replicate the changes.

As for merging to develop as well, sure we can. But I thought eventually vapt is going into develop. Are we going to merge it soon after vapt?