Check allowed_author_re against env in --file mode?

[scop]

Related to #484, it occurred to me that it is actually possible to check the allowed author in --file mode too at least when running in a git commit-msg hook which I think is quite a common scenario: git sets theGIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL env vars, and those could be combined to create an author string on the fly to check against, $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>.

I don't know if there are some gnarly escaping details re this when git does it internally, but I suppose it's not a biggie even if there are -- that might be very well possible to take care of in the allowed_author_re regexp.

Maybe it would be best to add a command line flag for opting in to the env lookup, so that it would not take effect when one does a commit message check (with --file or stdin) in some other context and GIT_AUTHOR_* from the env kick in even though they might not have anything to do with the commit. So maybe a --author-from-env flag which enables this, and the committed pre-commit hook and other commit hook setups could make use of it? The flag would not have any effect when checking against a complete commit.

[epage]

If we support this, I think we should either

• accept a --author and require the caller to forward GIT_AUTHOR_* to it
• always read GIT_AUTHOR_* for --file

Not a fan of such a specialized flag as --author-from-env.

[scop]

I could be mistaken, but FWIW I believe with pre-commit the first option would be difficult/impossible/ugly to achieve in a way that one could ship a .pre-commit-hooks.yaml like committed currently does, in a form that would make use of this. This is with the unverified assumption that pre-commit does not expand environment variables so they cannot be passed in the args array having entry: committed, and going through a shell is not great for portability and likely becomes ugly and inflexible if it can even be made to work.

Always reading the env has the potential drawback mentioned in the first message. I suppose making it so that the allowed author check would not be done if both GIT_AUTHOR_* are empty or unset would serve as an adequate opt-out mechanism.

Of these options, if an explicit boolean opt-in/opt-out flag is out of the question, I think I'd personally go with the latter even though my use cases are not affected by the potential pre-commit problems.

[epage]

I suppose making it so that the allowed author check would not be done if both GIT_AUTHOR_* are empty or unset would serve as an adequate opt-out mechanism.

Yes, i would expect that to be the case.

The question is mostly whether those values are always relevant or if we can hit false positives.

[scop]

I can imagine a false positive in a setup where there are GIT_AUTHOR_* in the user's ("global") environment, and they run let's say echo foo | committed to try things out in a directory that has a committed config file with allowed_author_re in it. The env vars might not match what the user would commit as in the repo, as the user name and email may have been set via git config, overriding the envvars.

Granted, that's quite a corner case if not a pathological one, but possible.