Re: [PATCH] optionally deny all pushes

Clemens Buchacher <drizzd@aon.at> writes:

I do not like this at all.

Especially given that this seems equivalent to:

	$ echo exit 1 >.git/hooks/pre-receive
        $ chmod +x .git/hooks/pre-receive

this is a mere configuration bloat, isn't it?

Your patch however raises an interesting point. Our pre-receive rejection infrastructure does have a room for improvement.

A typical "push" exchange goes like this:

    receiver: Here are the refs I have.
    sender:   I want to update your refs with these.
    sender:   Here is the object data to complete the updated refs.
    receiver: Let's see what you are trying to do... I may say:
              - I refuse your proposal to update/delete this branch,
                as it is the current branch.
              - My hook tells me to refuse your proposal to update this
                ref, as it does not fast-forward.

We can see that we could decide to reject the push to update the current branch even before receiving the bulk of transfer. I suspect it might need a protocol extension to give the receiver a chance to speak after hearing the proposed set of updates before receiving the bulk of the data to add a clean error codepath, but at the worset case, we could immediately close the connection after hearing what the sender proposes to do tries to touch the current branch in a way we do not like. In other words, the exchange could become:

    receiver: Here are the refs I have.
    sender:   I want to update your refs with these.
    receiver: Let's see what you are trying to do... I may say:
              - I can tell you without actually looking at the commits you
                are proposing to place at the tips of my refs that I won't
                like this branch to be updated/deleted. Go away.
    sender:   Here is the object data to complete the updated refs.
    receiver: Let's see what you are trying to do...
              - My hook tells me to refuse your proposal to update this
                ref, as it does not fast-forward.

Your "reject any and all" better fits into that new rejection codepath. Just like the configuration that rejects updates to the current branch, the decision your patch wants to make can be made before we see any object data from the sender.

Another thing to consider is that you do not have to be limited to "all".

Often people want to run "git fetch" on their mothership box receiving data from their notebook, i.e.

    mothership$ git fetch notebook master:refs/remotes/notebook/master
    mothership$ git merge notebook/master

but are prevented from doing so due to networking issues (e.g. your notebook may not allow incoming connections). The right way to emulate this is to initiate the connection in the reverse direction, running "git push" on their notebook sending data into their mothership box, i.e.

    notebook$ git push mothership master:refs/remotes/notebook/master
    mothership$ git merge notebook/master

In such a situation, you may want to configure your mothership repository to reject any push outside refs/remotes/ hierarchy to prevent mistakes. We could add "receive.allowed = refs/remotes/*" or something to support this use case. It might turn out that we may even want to make this the default for non-bare repositories.

Once that happens, you can naturally say "receive.allowed = none" to do what your patch wants to do as a narrow special case. Wouldn't that be a much better approach than a "reject any and all" that can only serve a single narrow case that can already be done with a simple hook?