search

rycus86 / githooks

Githooks: per-repo and global Git hooks with version control
MIT License
381 stars 20 forks source link

Suggestion: Store trusted hook hashes globally (for shared hooks) #87

Open matthijskooijman opened 4 years ago

matthijskooijman commented 4 years ago

AFAICS, the hashes of enabled/trusted hooks (e.g. which you approved through the prompt) are stored inside each individual git hook. This makes sense for repo-local hooks, but this also means that shared hooks, which are used in multiple repos (and especially global shared hooks which are used in all repos) must be approved again separately for each repository that you use.

This is inconvenient, but also a minor security risk: If you end up having to approve hooks all the time, you will likely be quicker to approve and less likely to spot a hook that is not actually from a trusted repo and it is harder to see when a hook actually changed, or you've just not used (the new version of) the hook in this particular repo yet.

If you keep the trusted hashes globally, you would have to approve a hook only once, which means that whenever you get a prompt, it is either a new or changed hook and you can take a little more time to see if it is from a fully trusted repo and/or inspect the hook itself to see what changed and if it is (still) ok.

Though keeping these hashes globally makes the most sense for shared hooks, for simplicity it could be fine to just store all hook checksums globally. Since the full path is included, this should not change any behaviour, other than producing a bigger file (and slightly changing behaviour when you remove and re-clone a repo, but remembering approvals is then probably just a feature).

rycus86 commented 4 years ago

Yeah, that makes sense for me for shared hooks, though ot might be nice to leave the in-repo hooks as-is in the local file in the .git directory if that doesn't complicate the code too much.

Would you be able to open a PR for this by any chance?

matthijskooijman commented 4 years ago

Would you be able to open a PR for this by any chance?

Sorry, too much on my plate already. Sorry for spilling all my wishes and suggestions without any time to implement any of them, but I really need to clean up my TODO stack rather than add more things to it :-)

rycus86 commented 4 years ago

Yeah, I know the situation... 🙂 Let's keep this open here then, but I can't make any promises at the moment either, apart from hopefully in a little while?

matthijskooijman commented 4 years ago

It's not something critical in any case. And maybe I'll get annoyed by the repeated prompt enough to prepare a PR anyway after a while :-p

gabyx commented 4 years ago

I might have a look into it...

gabyx commented 3 years ago

FYI: This is implemented in the new version: https://github.com/gabyx/githooks#trusting-hooks