-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for server side hooks #80
base: master
Are you sure you want to change the base?
Conversation
Please let me know what if anything, I should do for tests. |
ed2d1d2
to
d15da21
Compare
This would be really useful! Thanks @ajjl |
can we enforce this on public github or it is only for github enterprise appliance ? |
@ramya-ravula-ctr I dont think this works on publicly hosted github. I don't know about github enterprise is set up but I imagine it would work. This is meant for a git server that you have control over. An example would be a self hosted gitlab server. |
Thanks @ajjl for the response. i figured out it's not for publicly hosted github and public github only supports client side hooks. |
@mtdowling any thoughts on this feature? |
anybody... @mtdowling ? |
I was looking into this as well, and was wondering the following: Where do you store your "secret patterns"? Normally they get stored in |
Hi @rix0rrr |
Sorry for the delay! I was on paternity leave and then dropped the ball on reviewing this. I've left some comments on the review. |
Thanks for the review @mtdowling! I will take a look at this this weekend or next week and get back to you. |
Also congrats on your new/bigger family! @mtdowling |
Client side hooks are great, but rely on developers properly setting them up and not taking shortcuts. Server side hooks provide an opportunity to enforce security policies at a more global level. This commit adds an "update_hook" option which can be added as a serverside update hook. It scans the pushed commits for secrets.
d15da21
to
38b48c3
Compare
Wow, I totally lost track of this, anyways, I added the comment into the code, and rebased the branch. Let me know if you want anymore changes before merging @mtdowling |
That was me up there logged into my work account. Hopefully not too confusing. |
Everything is looking good here. The only thing that I think is missing is tests. Is that something you can add? |
I think this is ready along with the tests in #204. |
Close and reopen to trigger tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Confirming that new tests in #204 pass.
Client side hooks are great, but rely on developers properly setting
them up and not taking shortcuts. Server side hooks provide an
opportunity to enforce security policies at a more global level.
This commit adds an "update_hook" option which can be added as a
serverside update hook. It scans the pushed commits for secrets.
Fixes issue: #79