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

Update git hook tests #89

Merged
merged 7 commits into from
May 23, 2024
Merged

Update git hook tests #89

merged 7 commits into from
May 23, 2024

Conversation

markdboyd
Copy link
Contributor

Changes proposed in this pull request:

Addresses #37

As noted in #37, it is possible to have local git hooks and gitleaks co-exist, but the tests run by make audit assume that is not possible. To fix this behavior, this PR:

  • Adds a test to prove that local git hooks and gitleaks both run on a test repo
  • Removes the test that fails make audit if a repo has a custom .git/hooks/pre-commit hook, since this scenario does not actually prevent gitleaks from running
  • Adds a test to prove that core.hooksPath is not overridden at the repo level, which would prevent gitleaks from running

Other unrelated fixes in the PR include:

  • Updates to README to remove outdated information
  • Refactor shell scripts based on shellcheck recommendations

security considerations

This PR should be correcting a misunderstanding of how git hooks work but not actually introducing any chance that the global git hook for gitleaks won't run. However, the changes should be audited carefully to avoid any possible regression

@markdboyd markdboyd requested a review from a team May 23, 2024 19:17
JasonTheMain
JasonTheMain previously approved these changes May 23, 2024
Copy link

@JasonTheMain JasonTheMain left a comment

Choose a reason for hiding this comment

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

I like the changes

check_repos.sh Outdated Show resolved Hide resolved
@bengerman13
Copy link
Contributor

thanks for cleaning up the README, too!

Copy link
Contributor

@wz-gsa wz-gsa left a comment

Choose a reason for hiding this comment

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

Everything appears to be good!

@markdboyd markdboyd merged commit cc86912 into main May 23, 2024
1 check passed
@markdboyd markdboyd deleted the update-git-hook-tests branch May 23, 2024 20:07
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.

4 participants