[PATCH] Introduce receive.denyDeletes

Occasionally, it may be useful to prevent branches from getting deleted from
a centralized repository, particularly when no administrative access to the
server is available to undo it via reflog. It also makes
receive.denyNonFastForwards more useful if it is used for access control, since
it prevents force-updating refs by deleting and re-creating a ref.

Signed-off-by: Jan Krüger <redacted>
---
Fairly low invasiveness. Includes documentation and test case. I have run all
parts of the test suite that use receive-pack, send-pack and friends.

 Documentation/config.txt |    4 ++++
 builtin-receive-pack.c   |   12 ++++++++++++
 t/t5400-send-pack.sh     |   11 +++++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 29369d0..965ed74 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1188,6 +1188,10 @@ receive.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
+receive.denyDeletes::
+	If set to true, git-receive-pack will deny a ref update that deletes
+	the ref. Use this to prevent such a ref deletion via a push.
+
 receive.denyNonFastForwards::
 	If set to true, git-receive-pack will deny a ref update which is
 	not a fast forward. Use this to prevent such an update via a push,

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 9f60f31..2c0225c 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c

@@ -11,6 +11,7 @@
 
 static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
 
+static int deny_deletes = 0;
 static int deny_non_fast_forwards = 0;
 static int receive_fsck_objects;
 static int receive_unpack_limit = -1;

@@ -23,6 +24,11 @@ static int capabilities_sent;
 
 static int receive_pack_config(const char *var, const char *value, void *cb)
 {
+	if (strcmp(var, "receive.denydeletes") == 0) {
+		deny_deletes = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (strcmp(var, "receive.denynonfastforwards") == 0) {
 		deny_non_fast_forwards = git_config_bool(var, value);
 		return 0;

@@ -185,6 +191,12 @@ static const char *update(struct command *cmd)
 		      "but I can't find it!", sha1_to_hex(new_sha1));
 		return "bad pack";
 	}
+	if (deny_deletes && is_null_sha1(new_sha1) &&
+	    !is_null_sha1(old_sha1) &&
+	    !prefixcmp(name, "refs/heads/")) {
+		error("denying ref deletion for %s", name);
+		return "deletion prohibited";
+	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
 	    !prefixcmp(name, "refs/heads/")) {

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 544771d..6db9e18 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh

@@ -104,6 +104,17 @@ HOME=`pwd`/no-such-directory
 export HOME ;# this way we force the victim/.git/config to be used.
 
 test_expect_success \
+	'pushing a delete should be denied with denyDeletes' '
+	cd victim &&
+	git config receive.denyDeletes true &&
+	git branch extra master &&
+	cd .. &&
+	test -f victim/.git/refs/heads/extra &&
+	git send-pack ./victim/.git/ :extra master && return 1
+	rm -f victim/.git/refs/heads/extra
+'
+
+test_expect_success \
         'pushing with --force should be denied with denyNonFastforwards' '
 	cd victim &&
 	git config receive.denyNonFastforwards true &&

-- 
1.6.0.3.523.g304d0.dirty

Jan Krrrger [off-list ref] wrote:

Occasionally, it may be useful to prevent branches from getting deleted from
a centralized repository, particularly when no administrative access to the
server is available to undo it via reflog. It also makes
receive.denyNonFastForwards more useful if it is used for access control, since
it prevents force-updating refs by deleting and re-creating a ref.

...

 test_expect_success \
+	'pushing a delete should be denied with denyDeletes' '
+	cd victim &&
+	git config receive.denyDeletes true &&
+	git branch extra master &&
+	cd .. &&
+	test -f victim/.git/refs/heads/extra &&
+	git send-pack ./victim/.git/ :extra master && return 1

Hmm, that should be:

  test_must_fail git send-pack ./victim/.git/ :extra master

-- 
Shawn.

On Thu, 30 Oct 2008 11:32:10 -0700, "Shawn O. Pearce"
[off-list ref] wrote:

quoted

 test_expect_success \
[snip]
+	git send-pack ./victim/.git/ :extra master && return 1

Hmm, that should be:

  test_must_fail git send-pack ./victim/.git/ :extra master

Can I then delete the branch afterwards without lots of juggling (in
case the test fails due to a random other reason that the branch
accidentally getting deleted by receive-pack)? I'd expect I'd have to
save the exit code to a temporary variable and that's just as ugly.

Disclaimer: I sort of just hacked the test case together by stringing
together pieces of existing test code, so I don't presume full
understanding of the test framework.

By the way, I love what your mail client did to my name.

-Jan

Hi,

On Thu, 30 Oct 2008, Jan Krüger wrote:

By the way, I love what your mail client did to my name.

I think Shawn forgot to turn his pirate filter off.

Ciao,
Dscho