Add KeybaseDirSign pre-commit hook + tests.

[grempe]

See https://keybase.io

Signs all of the directory contents (except that
which is excluded in .gitignore) with an OpenPGP
compliant cryptographic signature.

This creates a SIGNED.md file in the root of the
repository. This file also gets added to the git
index prior to commit.

Note, due to the way keybase works, if there are
files or changes in the working dir that are not
being committed they will still be included with
the signature. In that case someone running
'keybase dir verify' will not get a clean verify
since their directory does not match exactly. It
is best to stash away work dir contents prior to
commit if concerned about this.

[sds]

Hey @grempe, thanks for the pull request!

For my own education, can you elaborate on the workflow this simplifies or facilitates? It seems like this functionality is provided by signed commits, which have been supported in Git for quite some time (but I've never had reason to use them so I don't know how difficult they are in practice).

Having a hook that signs everything automatically seems fine, but I wonder if you are adding an unnecessary layer here? Happy to hear your thoughts. I don't have issues with this pull request otherwise.

[sds]

Also, FWIW, git's creator Linus Torvalds thoughts on signing is that you should sign tags, not commits.

[jawshooah]

@sds One potential issue is that the hook appears to make changes and stage
them before commit.

On Tue, Jun 30, 2015, 3:11 PM Shane da Silva notifications@github.com
wrote:

Also, FWIW, git's creator Linus Torvalds thoughts on signing is that you
should sign tags, not commits
http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-td2582986.html
.

—
Reply to this email directly or view it on GitHub
#235 (comment).

[sds]

@jawshooah right, that would be an issue.

Does this actually work, @grempe? Overcommit would wipe the changes to the SIGNED.md file if they weren't already previously staged, due to how Overcommit creates a stash of your changes before the pre-commit hook run.

[grempe]

@sds You are correct. It is not working as I thought it was. I didn't know that overcommit performed some magic behind the scenes and wiped my changes. When I was testing it I was prompted for my GPG key and I saw the new SIGNED.md file being created.

What I didn't notice is that the modified and git added file was then being wiped. I confirmed with a sha1sum of the file before and after commit that it was unchanged, and I also this time looked at the git show for a commit which should have included the new SIGNED.md file and it didn't. It only included the change to the test file I committed.

So how would I modify my PR to handle this situation? Or does overcommit even make this possible? Also, handling more than one command in a single pre-commit hook seems not well supported. I'd really like to use overcommit but this may make it a non-starter for me.

Also, I am of course aware of git signed commits. Keybase is intended to simplify the use of GPG, and proven identification linking you to other web properties that you can prove ownership of. The SIGNED.md file is a human readable signed markdown file. Anyone with the keybase client installed can simply do 'keybase dir verify' to verify every file in the repo and provide assurance that all files remain the same. I just find it friendlier than signed commits. If github exposed signed commits or tags at all (and verified them against a public key) that might be nice. But they don't. You need to remember to add a signature flag to every commit you make to sign it. And git also won't even show by default that a commit is signed. It seems that they wanted to make this feature invisible. I want something right in the root of my repo that indicates both that it is signed, and also how one would (easily) go about verifying the signature.

Here is an example in one of my repos:
https://github.com/grempe/diceware/blob/master/SIGNED.md

And here are the docs with some additional info on why:
https://keybase.io/docs/command_line/code_signing

As to your note about Linus' opinion on signed commits. Well, Linus notoriously has a lot of opinions. Here is a counterpoint.

http://mikegerwitz.com/papers/git-horror-story.html

Cheers,

Glenn

[grempe]

PS - Another interesting property of detached signatures like this is that the keybase.io SIGNED.md file, which travels with the source code and is not embedded in hidden repository metadata, can be verified even if the .git dir is stripped away as it might be in a tarball release.

[sds]

Hey @grempe, thanks for your thoughts. I think this seems like a reasonable pre-commit hook.

The issue preventing this from working is difficult to work around. Overcommit stashes your unstaged changes (keeping your staged ones) so that the hooks only run against files that are actually about to be committed. However, in order to restore the working tree to its previous state, we then need to git reset --hard and then git stash apply so we get both the staged and unstaged changes back in the working tree.

By modifying the staged changes in between this process, those changes will get lost. There's no "easy" way around this—it's pretty core to how Overcommit's pre-commit hook flow works.

I'm open to suggestions, but the complexity required to support this existing system is already pretty substantial (have a look at the tests for a collection of wonderful edge cases), so I'd be hesitant to modify this behavior.

[grempe]

Well, that makes me sad. :-)

It seems there should be a way (whether its recommended or not is a different discussion) to have a pre-commit hook with side-effects (e.g. changing a file, writing a file with a timestamp, git adding a file, etc.) using overcommit. You can of course do this with real git hooks. My hook to do the same thing as my pull request was:

#!/bin/sh
keybase dir sign -p git
git add SIGNED.md

So overcommit seems to just not allow for hooks with side-effects at all. It would have been helpful if you had something in the README to indicate this behavior and what it allows and disallows you to do from within a hook.

As a thought, perhaps you could add your own category of hook (lets call it pre-pre-commit for a moment) which could be for hooks with side-effects and would run just prior to the pre-commit hook's running of git stash? This class of hook could modify or create files in the working dir, but its changes would only be included in the commit (and tested with normal pre-commit) if the pre-pre-commit hook also added any files it changed or added to the git index.

I am curious why overcommit chose to stash, reset, and apply to the working dir when standard git does not do this with its hooks (to my knowledge)?

[grempe]

PS - On our side conversation. I did a little digging and Git didn't add support for GPG signed commits (vs. signed tags) until v1.7.9 in 2012. So Linus's opinion (written in 2009) was not based on that functionality being present. I guess the git maintainers thought again about that idea despite Linus' opinion.

https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work
https://git.kernel.org/cgit/git/git.git/plain/Documentation/RelNotes/1.7.9.txt?id=HEAD

* "git commit" learned "-S" to GPG-sign the commit; this can be shown
   with the "--show-signature" option to "git log".

https://github.com/git/git/releases?after=v1.7.8.5

:-)

G

[sds]

The stashing is crucial in order to support hook runs that lint the changes you are about to commit, and not all modified files in your working tree.

Writing your own git hooks, you'd have to jump through the hoop of extracting the staged changes into a temporary file via git diff --cached or similar and then running your hooks against that temporary file. Overcommit simplifies this so you don't have to.

You may ask why Overcommit doesn't just run the hooks against temporary files. This too has a good reason: many tools are configured with relative paths, so moving the file to a temporary file causes the file name/path to necessarily change, so the tool's configuration will no longer apply correctly to that file. It's unfortunate but we think the benefits are worth it, especially when you can then use your same hooks as part of a CI run via overcommit --run.

I agree we could do a better job of communicating these limitations. The project has grown significantly in popularity this year, so we're starting to get more developers using it in ways we haven't considered. Unfortunately even with good documentation many developers end up discovering these implementation quirks the hard way (still no excuse to not document it, however).

[grempe]

Care to comment on the idea of having support for hooks with side-effects by, for example, running those hooks prior to the normal pre-commit hooks? It seems that you could call out to your own additional lifecycle hooks on a much more granular basis that what git supports. I can't tell if you are interested in supporting this type of hook, or prefer to stay with the status quo.

If the answer is stick with status quo, you can go ahead and close this pull request since overcommit just doesn't support what I'm trying to do currently and I'll have to look elsewhere. I can file it as a feature request and reference this PR if you prefer.

[grempe]

I added a feature request to enhance the docs. See the reference above.

[sds]

I personally would like to see Overcommit support side effects, but figuring out how to do this in a way that prevents heartache and pain will require some thought. Since we want to eventually parallelize hook runs, developers will need to be diligent in marking hooks with side effects as not being parallelizable (see #144 for discussion around parallelizing hooks).

My ideal solution would figure out a way to have this signing pre-commit hook work as intended without any code changes. I'm not sure what that would look like.

[sds]

I'm closing this pull request but am still very much behind adding support for side effects in pre-commit hooks. We'll revisit this once #238 is implemented.