alphagov / gds-pre-commit

An easy-to-install git precommit hook for preventing potential secrets being committed to git
5 stars 9 forks source link

Can bypass pre-commit checks using `git gui` #31

Closed denizgenc closed 4 years ago

denizgenc commented 4 years ago

Can't commit via regular git:

Screenshot 2020-03-10 at 2 07 54 pm

But you can commit via git gui:

Screenshot 2020-03-10 at 2 13 40 pm Screenshot 2020-03-10 at 2 14 29 pm

(note the "secret access key ID" is simply a random 40 character string. Any resemblance to existing or future key IDs is purely coincidental)

0atman commented 4 years ago

So why does this happen? Atom and VSCode and Emacs and vim are OK...

denizgenc commented 4 years ago

tl;dr: I believe the cause of the issue might be that git-gui doesn't look in the right place for the pre-commit hooks that we install, since we change the git core.hooksPath variable to something else.

To try and figure out why this happens, I spent this morning looking at the git-gui source, found here: https://github.com/prati0100/git-gui

The following are a collection of notes I took, roughly following the order of function invocations when a commit is made. Function names link to the lines in which they are found in the source files, for convenience.

git-gui.sh

Going into lib/commit.tcl:

Back to git-gui.sh, looking at the function githook_read:

What is $_gitdir?


Long story long - gitdir:

ciaranevans-xydus commented 2 years ago

Apologies for reviving an extremely dead thread @denizgenc - it is nice to see that it's back to the CS for me when I have an issue though 🤣

Did a fix for git gui and git hooks get implemented? I can see this issue was closed and your notes are very thorough, but I don't yet see how I can install a git hook and have it run via a git gui commit...? Unless I've mis-read something..

From what I understand, the only way would be to install the hook in the traditional .git/hooks folder, rather than use:

git config core.hooksPath <our-hooks-dir>
0atman commented 2 years ago

We no longer support this, please see the readme, which states:

We used to vendor our own bundle of pre-commit and detect-secrets. This was too brittle for us to support, we now recommend installing the off-the-shelf tools

I'd recommend asking this question on the pre-commit framework forums. Good luck!

ciaranevans-xydus commented 2 years ago

No worries @0atman - Worth the punt to see if the fix was easy to remember!