[PATCH] tag deletions not rejected with receive.denyDeletes= true

[PATCH] tag deletions not rejected with receive.denyDeletes= true

[Jerome DE VIVIE]

Hello,

I have try to deny tag deletion over push using denyDeletes parameter:

git config --system receive.denyDeletes true
git daemon --reuseaddr --base-path=.. --export-all --verbose --enable=receive-pack

I can push tag deletions despite what the internet says (http://progit.org/book/ch7-1.html#receivedenydeletes). I don't know if it is a bug. Could you have a look, pls ? Thank you


BR
Jérôme


Signed-off-by: Jérôme de Vivie <j.edevivie@prometil.com>
---
 builtin/receive-pack.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7ec68a1..bf91042 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -432,7 +432,7 @@ static const char *update(struct command *cmd)
 	}
 
 	if (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {
-		if (deny_deletes && !prefixcmp(name, "refs/heads/")) {
+		if (deny_deletes && (!prefixcmp(name, "refs/heads/") || !prefixcmp(name, "refs/tags/"))) {
 			rp_error("denying ref deletion for %s", name);
 			return "deletion prohibited";
 		}
-- 
1.7.6.msysgit.0

Re: [PATCH] tag deletions not rejected with receive.denyDeletes= true

[Junio C Hamano]

Jerome DE VIVIE <j.devivie@prometil.com> writes:

> Hello,
>
> I have try to deny tag deletion over push using denyDeletes parameter:
>
> git config --system receive.denyDeletes true
> git daemon --reuseaddr --base-path=.. --export-all --verbose --enable=receive-pack
>
> I can push tag deletions despite what the internet says (http://progit.org/book/ch7-1.html#receivedenydeletes). I don't know if it is a bug. Could you have a look, pls ? Thank you

The code seems to be written in such a way that it _explicitly_ wants to
limit the effect of the configuration only to branches. The change was
introduced by a240de1 (Introduce receive.denyDeletes, 2008-11-01) and the
motivation was explained as:

    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 by deleting and re-creating a ref.

So I would have to say your "the internet" is wrong.

Our documentation can also use some updates, as it dates to the days back
when we more liberally used "refs" and "branches" interchangeably.

Re: [PATCH] tag deletions not rejected with receive.denyDeletes= true

[Jerome DE VIVIE]

Junio C Hamano <gitster@pobox.com> writes :
> Our documentation can also use some updates, as it dates to the days back
> when we more liberally used "refs" and "branches" interchangeably.

Ok, I have turned the patch below for documentation.

For protecting tags, I can do it with triggers but its painful with lots of repositories. I propose to extend receive.denyDeletes with these values:
- "false"/"none" (existing behavior)
- "true"/"branches" (existing behavior)
- "tags": protect tags only
- "all": protect both tags and branches

Your opinion ?

BR
Jérôme


Signed-off-by: Jerome de Vivie <jedevivie-ext@airfrance.fr>
---
 Documentation/config.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..9c7c7fe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1642,7 +1642,7 @@ receive.unpackLimit::
 
 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.
+	a branch. Use this to prevent such a branch deletion via a push.
 
 receive.denyDeleteCurrent::
 	If set to true, git-receive-pack will deny a ref update that
-- 
1.7.6.msysgit.0

Re: [PATCH] tag deletions not rejected with receive.denyDeletes= true

[Junio C Hamano]

Jerome DE VIVIE <j.devivie@prometil.com> writes:

> Junio C Hamano <gitster@pobox.com> writes :
>> Our documentation can also use some updates, as it dates to the days back
>> when we more liberally used "refs" and "branches" interchangeably.
>
> Ok, I have turned the patch below for documentation.

Err,.. what I meant by "documentation update" is more like this.

 Documentation/config.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8a7d2d4..8eda8e4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1653,15 +1653,15 @@ receive.unpackLimit::
 	`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.
+	If set to true, git-receive-pack will deny an update that deletes
+	the branch. Use this to prevent a push from deleting a branch.
 
 receive.denyDeleteCurrent::
-	If set to true, git-receive-pack will deny a ref update that
+	If set to true, git-receive-pack will deny an update that
 	deletes the currently checked out branch of a non-bare repository.
 
 receive.denyCurrentBranch::
-	If set to true or "refuse", git-receive-pack will deny a ref update
+	If set to true or "refuse", git-receive-pack will deny an update
 	to the currently checked out branch of a non-bare repository.
 	Such a push is potentially dangerous because it brings the HEAD
 	out of sync with the index and working tree. If set to "warn",