Re: Feature Request: provide cmdline args to git hooks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Chris O'Kelly" <chris@xxxxxxxxxxxxxxxxxx> writes:

[administrivia: people read from top to bottom; please do not top
post]

> On Wed, Apr 15, 2015 at 1:08 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> "Chris O'Kelly" <chris@xxxxxxxxxxxxxxxxxx> writes:
>>
>>> A brief background of my use case:
>>> I am wanting to write a pre-push hook to prevent tags being pushed to
>>> our production servers. The production servers in our case are --bare
>>> endpoints, and when we push a tag at them, they always checkout the
>>> commit that tag is attached to via some post-receive magic (WPEngine,
>>> FWIW). This behavior is even present when the tag was already pushed
>>> to the repo previously, if for instance a normal push is made with the
>>> --tags argument.
>>
>> Do you mean that you want to forbid some people from pushing tags
>> into that repository while allowing others (i.e. those who manage
>> production deployment)?  If that is the goal, it sounds like that
>> the right place to do this is at the receiving end via pre-receive,
>> not at the sending end via pre-push.
>>

> Unfortunately in this case we don't have control over the hooks at the
> receiving end - we want to prevent tags from being pushed by all users
> to these repo's.

Well, if you refuse to or are unable to use the mechanism that was
specifically designed to solve your problem, then there is no way we
can offer you a good solution.

Let's set the baseline of the discussion first.

We agree that any client-side mechanism (like the hook) is *not* a
good way to enforce policy or ensure security. It is merely a way to
avoid mistakes---prevent the users to avoid doing something they are
allowed to do (at the mechanical level) that is not desirable to the
project.  After all, the user's hook can be misconfigured (or
disabled) by mistake, or the user's workstation may have an older
version of Git that does not know about the hook, thereby bypassing
whategver you try to do on the client side.  The user can even say
"git push --no-verify".  That is why a true policy-enforcement and
security must be done on the receiving end.

So our conversation must start from the shared understanding that
you are seeking a client-side "convention" that helps users avoid
mistakes.  You do not have anything more than "convention" to force
certain behaviour on users.  Are we in agreement?

Now, if a "convention," is an acceptaible solution, you do not even
necessarily need a hook. You can give "git mypush" to the users,
tell them to use it instead of "git push" and do whatever check in
"git mypush" and you are done.  If your users can be told not to run
"git push" with --no-verify, they can certainly taught not to use
"git push" but to use "git mypush". Cf. 5 valid reasons to have hook
(http://thread.gmane.org/gmane.comp.version-control.git/71069).

The reason why we added pre-push is primarily because parsing the
command line is too brittle a way to guess what will be pushed.
When the user says "git push" and your "hook" sees "ah, there is no
argument", that does not mean the user did not try to push any
tags.  The user may have "remote.origin.push" refspec to always push
things without typing from the command line, for example.  The
approach to inspect the command line, whether it is done in "hook"
or "git mypush", is unworkable, and that is why pre-push is told
what "git push" decided to update on the remote end.

Having said all that, I am not sure if ec55559f (push: Add support
for pre-push hooks, 2013-01-13) implemented the pre-push hook
correctly.

I do not offhand know (I am on a bus with terrible connection so I
won't bother checking the source now) if we send "this ref has to
point at that object" even for STATUS_UPTODATE cases, to cause your
remote to trigger the receive hook in the frist place, but if that
is the case, then the code in run_pre_push_hook() that omits such
refs from the hook input looks just *wrong*.  On the other hand, I
do not offhand think of a valid reason for the push codepath (with
or without the pre-push hook) to send "this ref has to point at that
object" when we know the ref is already pointing at that object, so
I am not sure if your claim (i.e. "when ... was already pushed
previously") is true for a correctly built receiving side of Git.

On the other side, if we are telling the other end "I know your
refs/tags/v1.0 is pointing at this object, but I am telling you to
point at it again anyway", then we should be feeding pre-push that,
too.  The code here in transport.c omits UPTODATE ones, and it may
need to be disabled to be consistent.

	for (r = remote_refs; r; r = r->next) {
		if (!r->peer_ref) continue;
		if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
		if (r->status == REF_STATUS_REJECT_STALE) continue;
		if (r->status == REF_STATUS_UPTODATE) continue;

		strbuf_reset(&buf);
		strbuf_addf( &buf, "%s %s %s %s\n",
			 r->peer_ref->name, sha1_to_hex(r->new_sha1),
			 r->name, sha1_to_hex(r->old_sha1));

		if (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {
			ret = -1;
			break;
		}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]