Skip to content

Add support for server side hooks#80

Open
ajjl wants to merge 2 commits into
awslabs:masterfrom
ajjl:feature/Add-serverside-hook
Open

Add support for server side hooks#80
ajjl wants to merge 2 commits into
awslabs:masterfrom
ajjl:feature/Add-serverside-hook

Conversation

@ajjl

@ajjl ajjl commented May 15, 2018

Copy link
Copy Markdown

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

@ajjl ajjl changed the title Add support for server side hooks wip: Add support for server side hooks May 15, 2018
@ajjl

ajjl commented May 15, 2018

Copy link
Copy Markdown
Author

Please let me know what if anything, I should do for tests.

@ajjl ajjl force-pushed the feature/Add-serverside-hook branch from ed2d1d2 to d15da21 Compare May 15, 2018 20:31
@ajjl ajjl changed the title wip: Add support for server side hooks Add support for server side hooks May 15, 2018
@bjbishop

Copy link
Copy Markdown

This would be really useful! Thanks @ajjl

@ramya-ravula-ctr

Copy link
Copy Markdown

can we enforce this on public github or it is only for github enterprise appliance ?

@ajjl

ajjl commented May 22, 2018

Copy link
Copy Markdown
Author

@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.

@ramya-ravula-ctr

Copy link
Copy Markdown

Thanks @ajjl for the response. i figured out it's not for publicly hosted github and public github only supports client side hooks.

@ajjl

ajjl commented May 30, 2018

Copy link
Copy Markdown
Author

@mtdowling any thoughts on this feature?

@ajjl

ajjl commented Jun 28, 2018

Copy link
Copy Markdown
Author

anybody... @mtdowling ?

@rix0rrr

rix0rrr commented Jul 17, 2018

Copy link
Copy Markdown

I was looking into this as well, and was wondering the following:

Where do you store your "secret patterns"?

Normally they get stored in .git/config, but that file does not get shared between clones. So where does the server get the list of prohibited patterns?

@ajjl

ajjl commented Jul 19, 2018

Copy link
Copy Markdown
Author

Hi @rix0rrr
Using gitlab I have the secrets in the .gitconfig file in the home folder of the git user on the gitlab server. I am not sure how it would be set up in other environments

@mtdowling

Copy link
Copy Markdown
Contributor

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.

Comment thread git-secrets
Comment thread git-secrets
Comment thread git-secrets
@ajjl

ajjl commented Aug 10, 2018

Copy link
Copy Markdown
Author

Thanks for the review @mtdowling! I will take a look at this this weekend or next week and get back to you.

@ajjl

ajjl commented Aug 10, 2018

Copy link
Copy Markdown
Author

Also congrats on your new/bigger family! @mtdowling

uleinal and others added 2 commits October 1, 2018 10:56
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.
@ajjl ajjl force-pushed the feature/Add-serverside-hook branch from d15da21 to 38b48c3 Compare October 1, 2018 15:57
@uleinal

uleinal commented Oct 1, 2018

Copy link
Copy Markdown

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

@ajjl

ajjl commented Oct 1, 2018

Copy link
Copy Markdown
Author

That was me up there logged into my work account. Hopefully not too confusing.

@mtdowling

Copy link
Copy Markdown
Contributor

Everything is looking good here. The only thing that I think is missing is tests. Is that something you can add?

@sparr

sparr commented Jun 16, 2023

Copy link
Copy Markdown
Contributor

I think this is ready along with the tests in #204.

@sparr sparr dismissed mtdowling’s stale review June 20, 2023 23:35

All comments resolved.

@sparr sparr closed this Jun 20, 2023
@sparr sparr reopened this Jun 20, 2023
@sparr

sparr commented Jun 20, 2023

Copy link
Copy Markdown
Contributor

Close and reopen to trigger tests

@sparr sparr left a comment

Copy link
Copy Markdown
Contributor

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.

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.

7 participants