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

Pre-commit hook to avoid accidentally adding unencrypted files #45

Open
ghost opened this issue Mar 18, 2015 · 10 comments
Open

Pre-commit hook to avoid accidentally adding unencrypted files #45

ghost opened this issue Mar 18, 2015 · 10 comments

Comments

@ghost
Copy link

ghost commented Mar 18, 2015

This should not happen very often, but for users who do not read the doc, or someone who has locked their repository a long time ago and forgot about it, it could be useful to try and prevent this situation:

$ git crypt lock # a long time ago... then forgotten
$ echo "secret.file filter=git-crypt diff=git-crypt" >> .gitattributes
$ echo "secret" > secret.file
$ git status # forgot to check using git-crypt status
$ git add secret.file
$ git commit -m "added secret file"
$ git push

secret.file has been added to .gitattributes, but the repository was locked so the person ended up adding it unencrypted.

git-crypt status already displays a huge warning in this situation, but it remains possible to make a mistake when it is not run. May I suggest adding to the documentation an example of a pre-commit hook such as this:

#!/bin/sh
#pre-commit
git-crypt status | grep "*** WARNING"; test $? -eq 1
@AGWA
Copy link
Owner

AGWA commented May 13, 2015

Sorry for the late reply to this. Something like this is a good idea; thanks for the suggestion! I'll try to roll this in to the next release.

@Falkor
Copy link

Falkor commented Sep 30, 2015

+1

Mays be it shall be proposed automatically upon git-crypt init.
Also the pre-receive counterpart on the server side might be worth to investigate

@smemsh
Copy link

smemsh commented Oct 15, 2015

I didn't realize this could happen, whew! thanks for this. I'm adding a default pre-commit hook (using init.templatedir) that will add this to all my repos by default, which should help

@smemsh
Copy link

smemsh commented Oct 15, 2015

Consider also for git-crypt status to return nonzero, possibly take a -q to quiet, instead of warn on stdio. This way the hook can simply invoke git-crypt status -q

@smemsh
Copy link

smemsh commented Oct 15, 2015

actually, I discovered that git-crypt status does already return failure
if one of the files has a warning. So the hook can actually just be e.g.:

#!/bin/sh
if test -d .git-crypt; then git-crypt status &>/dev/null; fi

I created a new issue #68 to request quiet flag in git-crypt status output,
so the &>/dev/null part could be removed.

@willnorris
Copy link

@smemsh: I didn't like that this fails silently without any indication as to what went wrong, so I modified yours slightly to:

test -d .git-crypt && git-crypt status &>/dev/null
if [[ $? -ne 0 ]]; then
  git-crypt status -e
  exit 1
fi

(just leaving as a suggestion for future readers)

@Falkor
Copy link

Falkor commented Jun 14, 2016

Hi @smemsh. Any (documentation / release) update to deal with this issue ? I confess I lost track of the preferred approach...

@Falkor
Copy link

Falkor commented Nov 11, 2018

Fyi, my own version of the pre-commit hook is available here.
It focuses only on the staged files as performing the scan of the full repo was not sustainable on large repository.

@akostadinov
Copy link

akostadinov commented Mar 10, 2020

I have filed #201 before seeing this issue. In my opinion having a pre-commit hook is sub-optimal. It needs to process all files. While having the functionality in git-crypt itself will be way more efficient.

Edit: on the other hand that will still not help people that cloned repo for the first time without taking extra steps. But still I think inbuilt into git-crypt will be more efficient.

@mkesper
Copy link

mkesper commented Apr 16, 2020

I have filed #201 before seeing this issue. In my opinion having a pre-commit hook is sub-optimal. It needs to process all files.

I don't think this isn't true.
https://github.com/IamTheFij/ansible-pre-commit/blob/master/encryption-check.sh
checks only changed files, for example.

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

No branches or pull requests

6 participants