{"thread":{"id":"16099","subject":"[PATCH] Introduce receive.denyDeletes","startedAt":"2008-10-30T18:11:34Z","lastAt":"2008-11-01T18:07:06Z","messageCount":8,"participants":["Jan Krüger","Shawn O. Pearce","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"94332","messageId":"20081030191134.62455c24@perceptron","threadId":"16099","inReplyTo":null,"subject":"[PATCH] Introduce receive.denyDeletes","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2008-10-30T18:11:34Z","receivedAt":"2008-10-30T18:11:34Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"Occasionally, it may be useful to prevent branches from getting deleted from\na centralized repository, particularly when no administrative access to the\nserver is available to undo it via reflog. It also makes\nreceive.denyNonFastForwards more useful if it is used for access control, since\nit prevents force-updating refs by deleting and re-creating a ref.\n\nSigned-off-by: Jan Krüger \n---\nFairly low invasiveness. Includes documentation and test case. I have run all\nparts of the test suite that use receive-pack, send-pack and friends.\n\n Documentation/config.txt | 4 ++++\n builtin-receive-pack.c | 12 ++++++++++++\n t/t5400-send-pack.sh | 11 +++++++++++\n 3 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 29369d0..965ed74 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1188,6 +1188,10 @@ receive.unpackLimit::\n \tespecially on slow filesystems. If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+receive.denyDeletes::\n+\tIf set to true, git-receive-pack will deny a ref update that deletes\n+\tthe ref. Use this to prevent such a ref deletion via a push.\n+\n receive.denyNonFastForwards::\n \tIf set to true, git-receive-pack will deny a ref update which is\n \tnot a fast forward. Use this to prevent such an update via a push,\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 9f60f31..2c0225c 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -11,6 +11,7 @@\n \n static const char receive_pack_usage[] = \"git-receive-pack \";\n \n+static int deny_deletes = 0;\n static int deny_non_fast_forwards = 0;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n@@ -23,6 +24,11 @@ static int capabilities_sent;\n \n static int receive_pack_config(const char *var, const char *value, void *cb)\n {\n+\tif (strcmp(var, \"receive.denydeletes\") == 0) {\n+\t\tdeny_deletes = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n \t\tdeny_non_fast_forwards = git_config_bool(var, value);\n \t\treturn 0;\n@@ -185,6 +191,12 @@ static const char *update(struct command *cmd)\n \t\t \"but I can't find it!\", sha1_to_hex(new_sha1));\n \t\treturn \"bad pack\";\n \t}\n+\tif (deny_deletes && is_null_sha1(new_sha1) &&\n+\t !is_null_sha1(old_sha1) &&\n+\t !prefixcmp(name, \"refs/heads/\")) {\n+\t\terror(\"denying ref deletion for %s\", name);\n+\t\treturn \"deletion prohibited\";\n+\t}\n \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n \t !is_null_sha1(old_sha1) &&\n \t !prefixcmp(name, \"refs/heads/\")) {\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 544771d..6db9e18 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -104,6 +104,17 @@ HOME=`pwd`/no-such-directory\n export HOME ;# this way we force the victim/.git/config to be used.\n \n test_expect_success \\\n+\t'pushing a delete should be denied with denyDeletes' '\n+\tcd victim &&\n+\tgit config receive.denyDeletes true &&\n+\tgit branch extra master &&\n+\tcd .. &&\n+\ttest -f victim/.git/refs/heads/extra &&\n+\tgit send-pack ./victim/.git/ :extra master && return 1\n+\trm -f victim/.git/refs/heads/extra\n+'\n+\n+test_expect_success \\\n 'pushing with --force should be denied with denyNonFastforwards' '\n \tcd victim &&\n \tgit config receive.denyNonFastforwards true &&\n-- \n1.6.0.3.523.g304d0.dirty\n"},{"id":"94335","messageId":"20081030183210.GO14786@spearce.org","threadId":"16099","inReplyTo":"20081030191134.62455c24@perceptron","subject":"Re: [PATCH] Introduce receive.denyDeletes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-30T18:32:10Z","receivedAt":"2008-10-30T18:32:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jan Krrrger wrote:\n> Occasionally, it may be useful to prevent branches from getting deleted from\n> a centralized repository, particularly when no administrative access to the\n> server is available to undo it via reflog. It also makes\n> receive.denyNonFastForwards more useful if it is used for access control, since\n> it prevents force-updating refs by deleting and re-creating a ref.\n...\n> test_expect_success \\\n> +\t'pushing a delete should be denied with denyDeletes' '\n> +\tcd victim &&\n> +\tgit config receive.denyDeletes true &&\n> +\tgit branch extra master &&\n> +\tcd .. &&\n> +\ttest -f victim/.git/refs/heads/extra &&\n> +\tgit send-pack ./victim/.git/ :extra master && return 1\n\nHmm, that should be:\n\n test_must_fail git send-pack ./victim/.git/ :extra master\n\n-- \nShawn.\n"},{"id":"94336","messageId":"20081030194503.2f9ece1a@perceptron","threadId":"16099","inReplyTo":"20081030183210.GO14786@spearce.org","subject":"Re: [PATCH] Introduce receive.denyDeletes","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2008-10-30T18:45:03Z","receivedAt":"2008-10-30T18:45:03Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"On Thu, 30 Oct 2008 11:32:10 -0700, \"Shawn O. Pearce\"\n wrote:\n> > test_expect_success \\\n> > [snip]\n> > +\tgit send-pack ./victim/.git/ :extra master && return 1\n> \n> Hmm, that should be:\n> \n> test_must_fail git send-pack ./victim/.git/ :extra master\n\nCan I then delete the branch afterwards without lots of juggling (in\ncase the test fails due to a random other reason that the branch\naccidentally getting deleted by receive-pack)? I'd expect I'd have to\nsave the exit code to a temporary variable and that's just as ugly.\n\nDisclaimer: I sort of just hacked the test case together by stringing\ntogether pieces of existing test code, so I don't presume full\nunderstanding of the test framework.\n\nBy the way, I love what your mail client did to my name.\n\n-Jan\n"},{"id":"94375","messageId":"7v63n99omx.fsf@gitster.siamese.dyndns.org","threadId":"16099","inReplyTo":"20081030194503.2f9ece1a@perceptron","subject":"Re: [PATCH] Introduce receive.denyDeletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-31T08:04:22Z","receivedAt":"2008-10-31T08:04:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Krüger\" writes:\n\n> Can I then delete the branch afterwards without lots of juggling (in\n> case the test fails due to a random other reason that the branch\n> accidentally getting deleted by receive-pack)? I'd expect I'd have to\n> save the exit code to a temporary variable and that's just as ugly.\n\nAlthough I agree that your attempt to allow the test continue even when\nthis test fails is a very good practice, I personally do not find the\nalternative you mention ugly at all. I actually find that \"return 1\"\nuglier because it feels like it knows too much about how\ntest_expect_success is implemented.\n"},{"id":"94405","messageId":"20081031143022.GQ14786@spearce.org","threadId":"16099","inReplyTo":"7v63n99omx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Introduce receive.denyDeletes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-31T14:30:22Z","receivedAt":"2008-10-31T14:30:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Jan Krüger\" writes:\n> \n> > Can I then delete the branch afterwards without lots of juggling (in\n> > case the test fails due to a random other reason that the branch\n> > accidentally getting deleted by receive-pack)? I'd expect I'd have to\n> > save the exit code to a temporary variable and that's just as ugly.\n\nIf you want to delete the branch after the test is done, do it\noutside of the test_expect_success's 3rd argument. Then it will\nrun the branch deletion whether or not the test was successful.\n \n> Although I agree that your attempt to allow the test continue even when\n> this test fails is a very good practice, I personally do not find the\n> alternative you mention ugly at all. I actually find that \"return 1\"\n> uglier because it feels like it knows too much about how\n> test_expect_success is implemented.\n\nYea, I also found the \"return 1\" to be horribly difficult to read, and\nknowing far too much about the test suite.\n\n-- \nShawn.\n"},{"id":"94480","messageId":"alpine.DEB.1.00.0810312345280.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16099","inReplyTo":"20081030194503.2f9ece1a@perceptron","subject":"Re: [PATCH] Introduce receive.denyDeletes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-31T22:45:53Z","receivedAt":"2008-10-31T22:45:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 30 Oct 2008, Jan Krüger wrote:\n\n> By the way, I love what your mail client did to my name.\n\nI think Shawn forgot to turn his pirate filter off.\n\nCiao,\nDscho\n"},{"id":"94562","messageId":"20081101154216.45021eee@perceptron","threadId":"16099","inReplyTo":"20081031143022.GQ14786@spearce.org","subject":"[PATCH v2] Introduce receive.denyDeletes","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2008-11-01T14:42:16Z","receivedAt":"2008-11-01T14:42:16Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"Occasionally, it may be useful to prevent branches from getting deleted from\na centralized repository, particularly when no administrative access to the\nserver is available to undo it via reflog. It also makes\nreceive.denyNonFastForwards more useful if it is used for access control\nsince it prevents force-updating by deleting and re-creating a ref.\n\nSigned-off-by: Jan Krüger \n---\nLike this, then?\n\n Documentation/config.txt | 4 ++++\n builtin-receive-pack.c | 12 ++++++++++++\n t/t5400-send-pack.sh | 11 +++++++++++\n 3 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 29369d0..965ed74 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1188,6 +1188,10 @@ receive.unpackLimit::\n \tespecially on slow filesystems. If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+receive.denyDeletes::\n+\tIf set to true, git-receive-pack will deny a ref update that deletes\n+\tthe ref. Use this to prevent such a ref deletion via a push.\n+\n receive.denyNonFastForwards::\n \tIf set to true, git-receive-pack will deny a ref update which is\n \tnot a fast forward. Use this to prevent such an update via a push,\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 9f60f31..2c0225c 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -11,6 +11,7 @@\n \n static const char receive_pack_usage[] = \"git-receive-pack \";\n \n+static int deny_deletes = 0;\n static int deny_non_fast_forwards = 0;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n@@ -23,6 +24,11 @@ static int capabilities_sent;\n \n static int receive_pack_config(const char *var, const char *value, void *cb)\n {\n+\tif (strcmp(var, \"receive.denydeletes\") == 0) {\n+\t\tdeny_deletes = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n \t\tdeny_non_fast_forwards = git_config_bool(var, value);\n \t\treturn 0;\n@@ -185,6 +191,12 @@ static const char *update(struct command *cmd)\n \t\t \"but I can't find it!\", sha1_to_hex(new_sha1));\n \t\treturn \"bad pack\";\n \t}\n+\tif (deny_deletes && is_null_sha1(new_sha1) &&\n+\t !is_null_sha1(old_sha1) &&\n+\t !prefixcmp(name, \"refs/heads/\")) {\n+\t\terror(\"denying ref deletion for %s\", name);\n+\t\treturn \"deletion prohibited\";\n+\t}\n \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n \t !is_null_sha1(old_sha1) &&\n \t !prefixcmp(name, \"refs/heads/\")) {\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 544771d..6fe2f87 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -103,6 +103,17 @@ unset GIT_CONFIG GIT_CONFIG_LOCAL\n HOME=`pwd`/no-such-directory\n export HOME ;# this way we force the victim/.git/config to be used.\n \n+test_expect_failure \\\n+\t'pushing a delete should be denied with denyDeletes' '\n+\tcd victim &&\n+\tgit config receive.denyDeletes true &&\n+\tgit branch extra master &&\n+\tcd .. &&\n+\ttest -f victim/.git/refs/heads/extra &&\n+\ttest_must_fail git send-pack ./victim/.git/ :extra master\n+'\n+rm -f victim/.git/refs/heads/extra\n+\n test_expect_success \\\n 'pushing with --force should be denied with denyNonFastforwards' '\n \tcd victim &&\n-- \n1.6.0.3.524.g86cf.dirty\n"},{"id":"94571","messageId":"20081101180705.GA14706@spearce.org","threadId":"16099","inReplyTo":"20081101154216.45021eee@perceptron","subject":"Re: [PATCH v2] Introduce receive.denyDeletes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-01T18:07:06Z","receivedAt":"2008-11-01T18:07:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jan Krrrger wrote:\n> Occasionally, it may be useful to prevent branches from getting deleted from\n> a centralized repository, particularly when no administrative access to the\n> server is available to undo it via reflog. It also makes\n> receive.denyNonFastForwards more useful if it is used for access control\n> since it prevents force-updating by deleting and re-creating a ref.\n> \n> Signed-off-by: Jan Krüger \n> ---\n> Like this, then?\n\nAcked-by: Shawn O. Pearce \n \n> Documentation/config.txt | 4 ++++\n> builtin-receive-pack.c | 12 ++++++++++++\n> t/t5400-send-pack.sh | 11 +++++++++++\n> 3 files changed, 27 insertions(+), 0 deletions(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 29369d0..965ed74 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1188,6 +1188,10 @@ receive.unpackLimit::\n> \tespecially on slow filesystems. If not set, the value of\n> \t`transfer.unpackLimit` is used instead.\n> \n> +receive.denyDeletes::\n> +\tIf set to true, git-receive-pack will deny a ref update that deletes\n> +\tthe ref. Use this to prevent such a ref deletion via a push.\n> +\n> receive.denyNonFastForwards::\n> \tIf set to true, git-receive-pack will deny a ref update which is\n> \tnot a fast forward. Use this to prevent such an update via a push,\n> diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\n> index 9f60f31..2c0225c 100644\n> --- a/builtin-receive-pack.c\n> +++ b/builtin-receive-pack.c\n> @@ -11,6 +11,7 @@\n> \n> static const char receive_pack_usage[] = \"git-receive-pack \";\n> \n> +static int deny_deletes = 0;\n> static int deny_non_fast_forwards = 0;\n> static int receive_fsck_objects;\n> static int receive_unpack_limit = -1;\n> @@ -23,6 +24,11 @@ static int capabilities_sent;\n> \n> static int receive_pack_config(const char *var, const char *value, void *cb)\n> {\n> +\tif (strcmp(var, \"receive.denydeletes\") == 0) {\n> +\t\tdeny_deletes = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n> \t\tdeny_non_fast_forwards = git_config_bool(var, value);\n> \t\treturn 0;\n> @@ -185,6 +191,12 @@ static const char *update(struct command *cmd)\n> \t\t \"but I can't find it!\", sha1_to_hex(new_sha1));\n> \t\treturn \"bad pack\";\n> \t}\n> +\tif (deny_deletes && is_null_sha1(new_sha1) &&\n> +\t !is_null_sha1(old_sha1) &&\n> +\t !prefixcmp(name, \"refs/heads/\")) {\n> +\t\terror(\"denying ref deletion for %s\", name);\n> +\t\treturn \"deletion prohibited\";\n> +\t}\n> \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n> \t !is_null_sha1(old_sha1) &&\n> \t !prefixcmp(name, \"refs/heads/\")) {\n> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\n> index 544771d..6fe2f87 100755\n> --- a/t/t5400-send-pack.sh\n> +++ b/t/t5400-send-pack.sh\n> @@ -103,6 +103,17 @@ unset GIT_CONFIG GIT_CONFIG_LOCAL\n> HOME=`pwd`/no-such-directory\n> export HOME ;# this way we force the victim/.git/config to be used.\n> \n> +test_expect_failure \\\n> +\t'pushing a delete should be denied with denyDeletes' '\n> +\tcd victim &&\n> +\tgit config receive.denyDeletes true &&\n> +\tgit branch extra master &&\n> +\tcd .. &&\n> +\ttest -f victim/.git/refs/heads/extra &&\n> +\ttest_must_fail git send-pack ./victim/.git/ :extra master\n> +'\n> +rm -f victim/.git/refs/heads/extra\n> +\n> test_expect_success \\\n> 'pushing with --force should be denied with denyNonFastforwards' '\n> \tcd victim &&\n> -- \n> 1.6.0.3.524.g86cf.dirty\n\n-- \nShawn.\n"}]}