{"thread":{"id":"37974","subject":"[PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","startedAt":"2014-11-15T07:26:17Z","lastAt":"2014-11-17T16:00:18Z","messageCount":13,"participants":["Michael Haggerty","Stefan Beller","Eric Wong","Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"251932","messageId":"1416036379-4994-1-git-send-email-mhagger@alum.mit.edu","threadId":"37974","inReplyTo":null,"subject":"[PATCH 0/2] Don't make $GIT_DIR executable","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-15T07:26:17Z","receivedAt":"2014-11-15T07:26:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Starting with v2.1.0, \"git init\" creates $GIT_DIR/config with its u+x\nbit set. These two patches are \"belt and suspenders\"--either one would\nfix the bug, but IMO it makes sense to apply both of them. Plus, the\nsecond patch will help repair repositories that were created while\nthis bug was in the wild.\n\nI think these patches should go into \"maint\". They apply cleanly\nthere. When merging forward to master, there are two conflicts in\nconfig.c which can be resolved by\n\n s/lock->filename/lock->filename.buf/\n\nin two places.\n\nMichael Haggerty (2):\n create_default_files(): don't set u+x bit on $GIT_DIR/config\n config: clear the executable bits (if any) on $GIT_DIR/config\n\n builtin/init-db.c | 1 +\n config.c | 12 ++++++++++--\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\n-- \n2.1.1\n"},{"id":"251933","messageId":"1416036379-4994-2-git-send-email-mhagger@alum.mit.edu","threadId":"37974","inReplyTo":"1416036379-4994-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-15T07:26:18Z","receivedAt":"2014-11-15T07:26:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Since time immemorial, the test of whether to set \"core.filemode\" has\nbeen done by trying to toggle the u+x bit on $GIT_DIR/config and then\ntesting whether the change \"took\". It is somewhat odd to use the\nconfig file for this test, but whatever.\n\nThe test code didn't set the u+x bit back to its original state\nitself, instead relying on the subsequent call to git_config_set() to\nre-write the config file with correct permissions.\n\nBut ever since\n\n daa22c6f8d config: preserve config file permissions on edits (2014-05-06)\n\ngit_config_set() copies the permissions from the old config file to\nthe new one. This is a good change in and of itself, but it interacts\nbadly with create_default_files()'s sloppiness, causing \"git init\" to\nleave the executable bit set on $GIT_DIR/config.\n\nSo change create_default_files() to reset the permissions on\n$GIT_DIR/config after its test.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/init-db.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 56f85e2..95ca5e4 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)\n \t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n \t\t\t\t!lstat(path, &st2) &&\n \t\t\t\tst1.st_mode != st2.st_mode);\n+\t\tchmod(path, st1.st_mode);\n \t}\n \tgit_config_set(\"core.filemode\", filemode ? \"true\" : \"false\");\n \n-- \n2.1.1\n"},{"id":"251931","messageId":"1416036379-4994-3-git-send-email-mhagger@alum.mit.edu","threadId":"37974","inReplyTo":"1416036379-4994-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-15T07:26:19Z","receivedAt":"2014-11-15T07:26:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There is no reason for $GIT_DIR/config to be executable, plus this\nchange will help clean up repositories affected by the bug that was\nfixed by the previous commit.\n\nSigned-off-by: Michael Haggerty \n---\n config.c | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 9e42d38..0942e5f 100644\n--- a/config.c\n+++ b/config.c\n@@ -1653,7 +1653,15 @@ int git_config_set_multivar_in_file(const char *config_filename,\n \t\t\tMAP_PRIVATE, in_fd, 0);\n \t\tclose(in_fd);\n \n-\t\tif (chmod(lock->filename, st.st_mode & 07777) < 0) {\n+\t\t/*\n+\t\t * We make of the executable bits because (a) it\n+\t\t * doesn't make sense to have executable bits set on\n+\t\t * the config file, and (b) there was a bug in git 2.1\n+\t\t * which caused the config file to be created with u+x\n+\t\t * set, so this will help repair repositories created\n+\t\t * with that version.\n+\t\t */\n+\t\tif (chmod(lock->filename, st.st_mode & 07666) < 0) {\n \t\t\terror(\"chmod on %s failed: %s\",\n \t\t\t\tlock->filename, strerror(errno));\n \t\t\tret = CONFIG_NO_WRITE;\n@@ -1832,7 +1840,7 @@ int git_config_rename_section_in_file(const char *config_filename,\n \n \tfstat(fileno(config_file), &st);\n \n-\tif (chmod(lock->filename, st.st_mode & 07777) < 0) {\n+\tif (chmod(lock->filename, st.st_mode & 07666) < 0) {\n \t\tret = error(\"chmod on %s failed: %s\",\n \t\t\t\tlock->filename, strerror(errno));\n \t\tgoto out;\n-- \n2.1.1\n"},{"id":"251934","messageId":"5467018D.7000103@gmail.com","threadId":"37974","inReplyTo":"1416036379-4994-3-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-11-15T07:32:29Z","receivedAt":"2014-11-15T07:32:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 14.11.2014 23:26, Michael Haggerty wrote:\n> There is no reason for $GIT_DIR/config to be executable, plus this\n> change will help clean up repositories affected by the bug that was\n> fixed by the previous commit.\n> \n> Signed-off-by: Michael Haggerty \n> ---\n> config.c | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/config.c b/config.c\n> index 9e42d38..0942e5f 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1653,7 +1653,15 @@ int git_config_set_multivar_in_file(const char *config_filename,\n> \t\t\tMAP_PRIVATE, in_fd, 0);\n> \t\tclose(in_fd);\n> \n> -\t\tif (chmod(lock->filename, st.st_mode & 07777) < 0) {\n> +\t\t/*\n> +\t\t * We make of the executable bits because (a) it\n\nWe make *use* of\n\n> +\t\t * doesn't make sense to have executable bits set on\n> +\t\t * the config file, and (b) there was a bug in git 2.1\n> +\t\t * which caused the config file to be created with u+x\n> +\t\t * set, so this will help repair repositories created\n> +\t\t * with that version.\n> +\t\t */\n> +\t\tif (chmod(lock->filename, st.st_mode & 07666) < 0) {\n> \t\t\terror(\"chmod on %s failed: %s\",\n> \t\t\t\tlock->filename, strerror(errno));\n> \t\t\tret = CONFIG_NO_WRITE;\n> @@ -1832,7 +1840,7 @@ int git_config_rename_section_in_file(const char *config_filename,\n> \n> \tfstat(fileno(config_file), &st);\n> \n> -\tif (chmod(lock->filename, st.st_mode & 07777) < 0) {\n> +\tif (chmod(lock->filename, st.st_mode & 07666) < 0) {\n> \t\tret = error(\"chmod on %s failed: %s\",\n> \t\t\t\tlock->filename, strerror(errno));\n> \t\tgoto out;\n> \n"},{"id":"251935","messageId":"546703FD.9070507@alum.mit.edu","threadId":"37974","inReplyTo":"5467018D.7000103@gmail.com","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-15T07:42:53Z","receivedAt":"2014-11-15T07:42:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/15/2014 08:32 AM, Stefan Beller wrote:\n> On 14.11.2014 23:26, Michael Haggerty wrote:\n>> There is no reason for $GIT_DIR/config to be executable, plus this\n>> change will help clean up repositories affected by the bug that was\n>> fixed by the previous commit.\n>>\n>> Signed-off-by: Michael Haggerty \n>> ---\n>> config.c | 12 ++++++++++--\n>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/config.c b/config.c\n>> index 9e42d38..0942e5f 100644\n>> --- a/config.c\n>> +++ b/config.c\n>> @@ -1653,7 +1653,15 @@ int git_config_set_multivar_in_file(const char *config_filename,\n>> \t\t\tMAP_PRIVATE, in_fd, 0);\n>> \t\tclose(in_fd);\n>> \n>> -\t\tif (chmod(lock->filename, st.st_mode & 07777) < 0) {\n>> +\t\t/*\n>> +\t\t * We make of the executable bits because (a) it\n> \n> We make *use* of\n\nThanks for catching this. But it's even worse. I meant to type \"mask off\".\n\nJunio, would you mind fixing this if there is no other reason for a\nre-roll? Thanks.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"251936","messageId":"20141115075030.GA31043@dcvr.yhbt.net","threadId":"37974","inReplyTo":"1416036379-4994-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 0/2] Don't make $GIT_DIR executable","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-11-15T07:50:30Z","receivedAt":"2014-11-15T07:50:30Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael Haggerty wrote:\n> Michael Haggerty (2):\n> create_default_files(): don't set u+x bit on $GIT_DIR/config\n> config: clear the executable bits (if any) on $GIT_DIR/config\n\nThanks, I should've noticed this earlier :x\nTested-by: Eric Wong \n\nSince the damage is done, perhaps removing +x from $GIT_DIR/config on\n_any_ git operation would be appropriate.\n"},{"id":"251941","messageId":"546741AC.9030107@web.de","threadId":"37974","inReplyTo":"1416036379-4994-2-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-15T12:06:04Z","receivedAt":"2014-11-15T12:06:04Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-11-15 08.26, Michael Haggerty wrote:\nThe whole thing looks good to me, some minor comments below\n> git_config_set() copies the permissions from the old config file to\n> the new one. This is a good change in and of itself, but it interacts\n> badly with create_default_files()'s sloppiness, causing \"git init\" to\n> leave the executable bit set on $GIT_DIR/config.\n> \n> So change create_default_files() to reset the permissions on\ns/permissions/executable bit/ ?\n> $GIT_DIR/config after its test.\n> \n> Signed-off-by: Michael Haggerty \n> ---\n> builtin/init-db.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index 56f85e2..95ca5e4 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)\n> \t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> \t\t\t\t!lstat(path, &st2) &&\n> \t\t\t\tst1.st_mode != st2.st_mode);\n> +\t\tchmod(path, st1.st_mode);\nA \"blind\" chmod() is good, but I think checking the return code is better.\n\n filemode &= (!chmod(path, st1.st_mode));\n\n> \t}\n> \tgit_config_set(\"core.filemode\", filemode ? \"true\" : \"false\");\n> \n"},{"id":"251952","messageId":"546834ED.6050403@alum.mit.edu","threadId":"37974","inReplyTo":"546741AC.9030107@web.de","subject":"Re: [PATCH 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-16T05:23:57Z","receivedAt":"2014-11-16T05:23:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/15/2014 01:06 PM, Torsten Bögershausen wrote:\n> On 2014-11-15 08.26, Michael Haggerty wrote:\n> The whole thing looks good to me, some minor comments below\n>> git_config_set() copies the permissions from the old config file to\n>> the new one. This is a good change in and of itself, but it interacts\n>> badly with create_default_files()'s sloppiness, causing \"git init\" to\n>> leave the executable bit set on $GIT_DIR/config.\n>>\n>> So change create_default_files() to reset the permissions on\n> s/permissions/executable bit/ ?\n>> $GIT_DIR/config after its test.\n\nThe code literally resets all of the permissions to their values before\nthe test, so I think the existing text is more accurate (even though\nyour version would have been fine, too).\n\n>> Signed-off-by: Michael Haggerty \n>> ---\n>> builtin/init-db.c | 1 +\n>> 1 file changed, 1 insertion(+)\n>>\n>> diff --git a/builtin/init-db.c b/builtin/init-db.c\n>> index 56f85e2..95ca5e4 100644\n>> --- a/builtin/init-db.c\n>> +++ b/builtin/init-db.c\n>> @@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)\n>> \t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>> \t\t\t\t!lstat(path, &st2) &&\n>> \t\t\t\tst1.st_mode != st2.st_mode);\n>> +\t\tchmod(path, st1.st_mode);\n> A \"blind\" chmod() is good, but I think checking the return code is better.\n> \n> filemode &= (!chmod(path, st1.st_mode));\n\nI guess it is better to include this test, even though it is mostly\nredundant with what was already determined by the previous line. I\nsuppose the only alternative would be to\n\n die(\"Your chmod() support is bonkers\")\n\nI will change this in v2.\n\nThanks for your comments!\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"251953","messageId":"546840BD.2090507@alum.mit.edu","threadId":"37974","inReplyTo":"20141115075030.GA31043@dcvr.yhbt.net","subject":"Re: [PATCH 0/2] Don't make $GIT_DIR executable","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-16T06:14:21Z","receivedAt":"2014-11-16T06:14:21Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/15/2014 08:50 AM, Eric Wong wrote:\n> Michael Haggerty wrote:\n>> Michael Haggerty (2):\n>> create_default_files(): don't set u+x bit on $GIT_DIR/config\n>> config: clear the executable bits (if any) on $GIT_DIR/config\n> \n> Thanks, I should've noticed this earlier :x\n> Tested-by: Eric Wong \n> \n> Since the damage is done, perhaps removing +x from $GIT_DIR/config on\n> _any_ git operation would be appropriate.\n\nI believe that the u+x bit on the config file should be pretty harmless.\nNobody is likely to try to execute their config files, and even if they\ndo, the file contents are unlikely to resemble a script enough to do\nanything bad.\n\nSo my feeling is that it is unnecessary to make every git invocation try\nto remove the u+x bit on the file. Thus I'd rather skip the code clutter\nand extra system call, limiting cleanup attempts to when the config file\nis being rewritten anyway.\n\nBut if other people are more alarmed than I am about having executable\nconfig files, I could certainly be persuaded that the extra effort is\njustified.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"251966","messageId":"xmqqvbmfyo8w.fsf@gitster.dls.corp.google.com","threadId":"37974","inReplyTo":"1416036379-4994-3-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-16T18:49:35Z","receivedAt":"2014-11-16T18:49:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> There is no reason for $GIT_DIR/config to be executable, plus this\n> change will help clean up repositories affected by the bug that was\n> fixed by the previous commit.\n\nI do not think we want to do this.\n\nIt is a welcome bugfix to create $GIT_DIR/config without executable\nbit when and only when we create it. It is very much in line with\n\"There is no reason for $GIT_DIR/config to be executable\"---we do\nnot need to make it executable ourselves, so we shouldn't, but we\ndid which was a bug we want to fix in patch 1/2 you posted.\n\nBut with the \"preserve existing permissions\" fix we did earlier, the\nend users are now allowed to flip the executable bit on for their\nown purpose, and dropping it without knowing why they are doing so\nis simply rude. And honestly, Git do *not* even want to know why\nthe users want to flip the bit.\n\nSo I would suggest not to spend any cycle or any code complexity to\n\"repair\" existing repositories. Having that bit on does not hurt\nanybody. Those who found it curious can flip that bit off and then\nGit with \"preserve existing permissions\" fix will keep that bit off\nfrom then on.\n"},{"id":"251976","messageId":"5469B134.1050306@alum.mit.edu","threadId":"37974","inReplyTo":"xmqqvbmfyo8w.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-17T08:26:28Z","receivedAt":"2014-11-17T08:26:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/16/2014 07:49 PM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> \n>> There is no reason for $GIT_DIR/config to be executable, plus this\n>> change will help clean up repositories affected by the bug that was\n>> fixed by the previous commit.\n> \n> I do not think we want to do this.\n> \n> It is a welcome bugfix to create $GIT_DIR/config without executable\n> bit when and only when we create it. It is very much in line with\n> \"There is no reason for $GIT_DIR/config to be executable\"---we do\n> not need to make it executable ourselves, so we shouldn't, but we\n> did which was a bug we want to fix in patch 1/2 you posted.\n> \n> But with the \"preserve existing permissions\" fix we did earlier, the\n> end users are now allowed to flip the executable bit on for their\n> own purpose, and dropping it without knowing why they are doing so\n> is simply rude. And honestly, Git do *not* even want to know why\n> the users want to flip the bit.\n> \n> So I would suggest not to spend any cycle or any code complexity to\n> \"repair\" existing repositories. Having that bit on does not hurt\n> anybody. Those who found it curious can flip that bit off and then\n> Git with \"preserve existing permissions\" fix will keep that bit off\n> from then on.\n\nI disagree. The point of \"preserve existing permissions\" was to allow\npeople to make their config files more readable/writable than the\ndefault, with the main use case being to help users who want to hide\nsecret information in their config files.\n\nI think it is really far-fetched to imagine that anybody made his config\nfile executable on purpose. Whereas we are *sure* that we have created\nlots of repositories with config files that were set executable by accident.\n\nLet's redefine the \"feature\" that was added in 2.1 from \"preserve\nexisting permissions\" to \"preserve existing read/write permissions\" and\nthereby help people clean up the mess we made.\n\nThat being said, I still believe that executable config files are not a\nsignificant risk in practice, so I'm not going to lose sleep about it\neither way.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"251982","messageId":"xmqqfvdhzvsp.fsf@gitster.dls.corp.google.com","threadId":"37974","inReplyTo":"5469B134.1050306@alum.mit.edu","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-17T15:33:26Z","receivedAt":"2014-11-17T15:33:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 11/16/2014 07:49 PM, Junio C Hamano wrote:\n> ...\n>> So I would suggest not to spend any cycle or any code complexity to\n>> \"repair\" existing repositories. Having that bit on does not hurt\n>> anybody. Those who found it curious can flip that bit off and then\n>> Git with \"preserve existing permissions\" fix will keep that bit off\n>> from then on.\n>\n> I disagree. The point of \"preserve existing permissions\" was to allow\n> people to make their config files more readable/writable than the\n> default,...\n\ns/more/less/, I think, was the original motivation. Having to limit\nmore tightly than usual was what made the \"config\" unusual among\nfiles under $GIT_DIR. If it were to loosen, Eric's change should\nnot have been done in the first place. The sharedRepository setting\nto defeat the default umask is there for that kind of thing.\n\n> That being said, I still believe that executable config files are not a\n> significant risk ...\n\nIt is merely an annoyance, to the same degree of annoyance we find\nwhen we see all files appear executable on a FAT floppy mounted on\nLinux ;-) I do not think it is a risk at all, and I do not see a\npoint in going into people's repository and actively \"fixing\" it.\nPeople who notice can fix, and people who do not care do not care\nand are not harmed.\n"},{"id":"251984","messageId":"546A1B92.90900@alum.mit.edu","threadId":"37974","inReplyTo":"xmqqfvdhzvsp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-11-17T16:00:18Z","receivedAt":"2014-11-17T16:00:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/17/2014 04:33 PM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> \n>> On 11/16/2014 07:49 PM, Junio C Hamano wrote:\n>> ...\n>>> So I would suggest not to spend any cycle or any code complexity to\n>>> \"repair\" existing repositories. Having that bit on does not hurt\n>>> anybody. Those who found it curious can flip that bit off and then\n>>> Git with \"preserve existing permissions\" fix will keep that bit off\n>>> from then on.\n>>\n>> I disagree. The point of \"preserve existing permissions\" was to allow\n>> people to make their config files more readable/writable than the\n>> default,...\n> \n> s/more/less/, I think, was the original motivation. Having to limit\n> more tightly than usual was what made the \"config\" unusual among\n> files under $GIT_DIR. If it were to loosen, Eric's change should\n> not have been done in the first place. The sharedRepository setting\n> to defeat the default umask is there for that kind of thing.\n\nOops, you are right. I actually meant to type \"less or more\", but I see\nthat \"more\" would be pretty useless.\n\n>> That being said, I still believe that executable config files are not a\n>> significant risk ...\n> \n> It is merely an annoyance, to the same degree of annoyance we find\n> when we see all files appear executable on a FAT floppy mounted on\n> Linux ;-) I do not think it is a risk at all, and I do not see a\n> point in going into people's repository and actively \"fixing\" it.\n> People who notice can fix, and people who do not care do not care\n> and are not harmed.\n\nOK, then, I'll send a new copy of patch 1/2 and drop 2/2.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"}]}