{"thread":{"id":"66585","subject":"[RFC/PATCH] config: honor file permission bits","startedAt":"2026-10-19T14:27:28Z","lastAt":"2026-11-03T19:11:18Z","messageCount":11,"participants":["Junio C Hamano","brian m. carlson","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"555421","messageId":"xmqqbj8p7okv.fsf@gitster.g","threadId":"66585","inReplyTo":null,"subject":"[RFC/PATCH] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-19T14:27:28Z","receivedAt":"2026-10-19T14:27:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Because our updates to an existing file almost never makes\nmodifications in place, and instead create a new \"lockfile\",\npropagate the permission bits from the original, copy the contents\nfrom the original file while tweaking the contents as needed, and\nthen finally rename the lockfile to replace the original file, you\ncannot \"protect\" your .global configuration file from accidental\nclobbering by doing something like this:\n\n\t$ chmod a-w $HOME/.gitconfig\n\nOf course, if you make your $HOME directory unwritable, it may\nprotect your .gitconfig from getting updated, but that goes totally\nagainst the intuition and is not how you would protect a regular\nfile from getting overwritten.\n\nTeach the configuration file writing machinery to inspect the\npermission bits of the original with access(2) and refrain from\ndoing the usual \"write a lockfile and replace the original with it\"\ndance ourselves when we shouldn't be able to write it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n config.c          | 22 ++++++++++++++++++++++\n t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+)\n\ndiff --git c/config.c w/config.c\nindex e0bb29b53d..2b84fb215c 100644\n--- c/config.c\n+++ w/config.c\n@@ -3065,8 +3065,30 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,\n \n \tif (!config_filename)\n \t\tconfig_filename = filename_buf = repo_git_path(r, \"config\");\n \n+\t/*\n+\t * Do not allow updating a read-only configuration file; it is\n+\t * annoying and alarming that \"chmod a-w $HOME/.gitconfig\" does not\n+\t * prevent the global configuration file from getting clobbered by\n+\t * mistake with the usual \"create a new lockfile, copy the original\n+\t * while setting a new value, propagate the permission bits to the\n+\t * lockfile, and then rename it to replace the original\" dance.\n+\t */\n+\tif (!access(config_filename, W_OK)) {\n+\t\t; /* happy: we can write to it */\n+\t} else if (errno == ENOENT) {\n+\t\t; /* happy: we are creating this file anew */\n+\t} else {\n+\t\t/*\n+\t\t * most likely EACCES but other errors also indicate\n+\t\t * that we cannot write to it anyway.\n+\t\t */\n+\t\terror_errno(_(\"file %s unwritable\"), config_filename);\n+\t\tret = CONFIG_NO_WRITE;\n+\t\tgoto out_free;\n+\t}\n+\n \t/*\n \t * The lock serves a purpose in addition to locking: the new\n \t * contents of .git/config will be written into it.\n \t */\ndiff --git c/t/t1300-config.sh w/t/t1300-config.sh\nindex e3f8064889..876d376cc7 100755\n--- c/t/t1300-config.sh\n+++ w/t/t1300-config.sh\n@@ -3018,5 +3018,35 @@ test_expect_success 'writing config retries until lock is released' '\n \tgit -c core.configLockTimeout=5000 config retried.key value &&\n \ttest \"$(git config retried.key)\" = value\n '\n \n+test_expect_success POSIXPERM 'refuse to \"overwrite\" a read-only file' '\n+\tgconf=custom-global-config &&\n+\ttest_when_finished \"chmod +w $gconf\" &&\n+\t(\n+\t\tGIT_CONFIG_GLOBAL=$gconf &&\n+\t\texport GIT_CONFIG_GLOBAL &&\n+\t\tgit config set --global some.variable some-value &&\n+\t\tgit config list --global >expect &&\n+\n+\t\tchmod a-w $gconf &&\n+\t\t# \"get\" from read-only file should just work fine.\n+\t\tgit config list --global >actual &&\n+\n+\t\t# \"set\" to read-only file should be refused.\n+\t\ttest_must_fail git config set --global some.variable other-value &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# traditional \"config var val\" syntax to set\n+\t\ttest_must_fail git config --global some.variable third-value &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# \"unset\" to read-only file should be refused.\n+\t\ttest_must_fail git config unset --global some.variable &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n\n"},{"id":"555910","messageId":"xmqqpkx1e9iq.fsf@gitster.g","threadId":"66585","inReplyTo":"xmqqbj8p7okv.fsf@gitster.g","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-22T20:59:57Z","receivedAt":"2026-10-22T20:59:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Because our updates to an existing file almost never makes\n> modifications in place, and instead create a new \"lockfile\",\n> propagate the permission bits from the original, copy the contents\n> from the original file while tweaking the contents as needed, and\n> then finally rename the lockfile to replace the original file, you\n> cannot \"protect\" your .global configuration file from accidental\n> clobbering by doing something like this:\n>\n> \t$ chmod a-w $HOME/.gitconfig\n>\n> Of course, if you make your $HOME directory unwritable, it may\n> protect your .gitconfig from getting updated, but that goes totally\n> against the intuition and is not how you would protect a regular\n> file from getting overwritten.\n>\n> Teach the configuration file writing machinery to inspect the\n> permission bits of the original with access(2) and refrain from\n> doing the usual \"write a lockfile and replace the original with it\"\n> dance ourselves when we shouldn't be able to write it.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  config.c          | 22 ++++++++++++++++++++++\n>  t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++\n>  2 files changed, 52 insertions(+)\n\nHmph, no responses to this RFC.  I somehow find it hard to believe\nthat I was the only person who got annoyed by this behaviour [*].\n\n[Footnote]\n\n * I was very tempted to write \"unexpected behaviour\", but I know\n   what happens in the code path, so it is not unexpected at least\n   to me.\n\n\n"},{"id":"555912","messageId":"atp7Pvtp1rdL8Uj_@fruit.crustytoothpaste.net","threadId":"66585","inReplyTo":"xmqqpkx1e9iq.fsf@gitster.g","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-10-22T21:08:15Z","receivedAt":"2026-10-22T21:08:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2026-10-22 at 20:59:57, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Because our updates to an existing file almost never makes\n> > modifications in place, and instead create a new \"lockfile\",\n> > propagate the permission bits from the original, copy the contents\n> > from the original file while tweaking the contents as needed, and\n> > then finally rename the lockfile to replace the original file, you\n> > cannot \"protect\" your .global configuration file from accidental\n> > clobbering by doing something like this:\n> >\n> > \t$ chmod a-w $HOME/.gitconfig\n> >\n> > Of course, if you make your $HOME directory unwritable, it may\n> > protect your .gitconfig from getting updated, but that goes totally\n> > against the intuition and is not how you would protect a regular\n> > file from getting overwritten.\n> >\n> > Teach the configuration file writing machinery to inspect the\n> > permission bits of the original with access(2) and refrain from\n> > doing the usual \"write a lockfile and replace the original with it\"\n> > dance ourselves when we shouldn't be able to write it.\n> >\n> > Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> > ---\n> >  config.c          | 22 ++++++++++++++++++++++\n> >  t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++\n> >  2 files changed, 52 insertions(+)\n> \n> Hmph, no responses to this RFC.  I somehow find it hard to believe\n> that I was the only person who got annoyed by this behaviour [*].\n\nSorry, I looked but forgot to respond.  It seems sensible to me.\n\nI don't personally make that file read-only, so it doesn't affect me at\nall; instead, I just overwrite it with my personal configuration every\ntime I deploy my dotfiles.  I still think it's a reasonable thing to do\nto help those who need it, though.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"555936","messageId":"xmqqa4o5co1d.fsf@gitster.g","threadId":"66585","inReplyTo":"atp7Pvtp1rdL8Uj_@fruit.crustytoothpaste.net","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-22T23:29:18Z","receivedAt":"2026-10-22T23:29:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I don't personally make that file read-only, so it doesn't affect me at\n> all; instead, I just overwrite it with my personal configuration every\n> time I deploy my dotfiles.\n\n> I don't personally make that file read-only, so it doesn't affect me at\n> all; instead, I just overwrite it with my personal configuration every\n> time I deploy my dotfiles.\n\nWell, I deploy my dotfiles from the freshly built one (the sources\nof dotfiles are kept elsewhere under version control), too, but when\n\"installing\", I make them read-only, just like all other \"installed\"\nfiles like the contents of my ~/bin/ directory.  My personal\nmakefiles have \"dry-run\" support to tell me what the changes are in\nthe version I am about to install in 'patch' form, and that is how I\nnoticed that \"git config\" allowed to make such a mistake, even though\nthe file is read-only, to some failed experiment I made earlier.\n\n\n"},{"id":"555977","messageId":"atsZX6exgGJkNQQQ@pks.im","threadId":"66585","inReplyTo":"xmqqbj8p7okv.fsf@gitster.g","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-23T08:23:05Z","receivedAt":"2026-10-23T08:23:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 19, 2026 at 07:27:28AM -0700, Junio C Hamano wrote:\n> Because our updates to an existing file almost never makes\n> modifications in place, and instead create a new \"lockfile\",\n> propagate the permission bits from the original, copy the contents\n> from the original file while tweaking the contents as needed, and\n> then finally rename the lockfile to replace the original file, you\n> cannot \"protect\" your .global configuration file from accidental\n> clobbering by doing something like this:\n> \n> \t$ chmod a-w $HOME/.gitconfig\n> \n> Of course, if you make your $HOME directory unwritable, it may\n> protect your .gitconfig from getting updated, but that goes totally\n> against the intuition and is not how you would protect a regular\n> file from getting overwritten.\n> \n> Teach the configuration file writing machinery to inspect the\n> permission bits of the original with access(2) and refrain from\n> doing the usual \"write a lockfile and replace the original with it\"\n> dance ourselves when we shouldn't be able to write it.\n\nWhat I wonder is what makes our config file so special compared to\neverything else that uses lockfiles. Or put differently: if we think\nit's important to honor permissions bits (which I think is sensible),\nshouldn't we do that in a central location like in \"lockfile.c\" so that\nall users of this interface behave the same?\n\nPatrick\n\n"},{"id":"556050","messageId":"xmqq8q3obh7u.fsf@gitster.g","threadId":"66585","inReplyTo":"atsZX6exgGJkNQQQ@pks.im","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-23T14:54:13Z","receivedAt":"2026-10-23T14:54:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> What I wonder is what makes our config file so special compared to\n> everything else that uses lockfiles. Or put differently: if we think\n> it's important to honor permissions bits (which I think is sensible),\n> shouldn't we do that in a central location like in \"lockfile.c\" so that\n> all users of this interface behave the same?\n\nThe $HOME/.gitconfig file and its XDG counterparts are the only\nout-of-tree files that suffer from this issue, and being outside the\nrepository is what makes them special.  Theoretically, /etc/gitconfig\non a multi-user system could have the same issue, but the containing\ndirectory would typically be read-only for regular users, which makes\nthe issue irrelevant there.\n\nI do not see any sensible reason why a user would want to \"protect\"\nin-tree files like .git/index from Git, either by running \"chmod -w\"\non them or by any other means.  We make files under the .git/objects/\nhierarchy read-only, but that is to prevent accidental overwriting\nby tools other than Git, so it does not serve as an example of Git\nneeding to respect read-only bits in the general lockfile machinery.\n\nHaving said all that, the patch was intentionally isolated to the\ncaller's side of the API, primarily because auditing all code paths\nthat use the lockfile API was far beyond my available time budget.\n\n"},{"id":"556282","messageId":"at8R_QThIEedVbO8@pks.im","threadId":"66585","inReplyTo":"xmqq8q3obh7u.fsf@gitster.g","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-26T08:40:29Z","receivedAt":"2026-10-26T08:40:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 23, 2026 at 07:54:13AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > What I wonder is what makes our config file so special compared to\n> > everything else that uses lockfiles. Or put differently: if we think\n> > it's important to honor permissions bits (which I think is sensible),\n> > shouldn't we do that in a central location like in \"lockfile.c\" so that\n> > all users of this interface behave the same?\n> \n> The $HOME/.gitconfig file and its XDG counterparts are the only\n> out-of-tree files that suffer from this issue, and being outside the\n> repository is what makes them special.  Theoretically, /etc/gitconfig\n> on a multi-user system could have the same issue, but the containing\n> directory would typically be read-only for regular users, which makes\n> the issue irrelevant there.\n\nThat explanation makes sense to me. Probably something to put into the\ncommit message :)\n\nThanks!\n\nPatrick\n\n"},{"id":"556354","messageId":"xmqqa4o0v76o.fsf@gitster.g","threadId":"66585","inReplyTo":"xmqqbj8p7okv.fsf@gitster.g","subject":"[PATCH v2] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-26T14:59:43Z","receivedAt":"2026-10-26T14:59:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When dotfile sources are kept under version control, you might\nprotect installed files like ~/.gitconfig from accidental changes by\nmaking them read-only.  This fails because updating a file writes a\nnew lockfile, copies and tweaks the content, and renames the\nlockfile over the original.  The original file's permission bits are\nignored in this process.\n\nTeach the configuration writing machinery to inspect the original\nfile's permissions with access(2) and avoid replacing it when the\nfile should not be writable.\n\nThis implementation targets configuration files rather than the\nlockfile API layer.  While in-tree files like .git/index do not need\nsuch protection, out-of-tree files like ~/.gitconfig and its XDG\ncounterparts are unique in this regard.  System-wide configuration\nlike /etc/gitconfig is typically in a directory already read-only to\nnormal users, making the issue irrelevant there.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * Only the commit message has been updated to mention that this is\n   deliberately done at the configuration API, not at the lockfile\n   layer.\n\n config.c          | 22 ++++++++++++++++++++++\n t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex e0bb29b53d..2b84fb215c 100644\n--- a/config.c\n+++ b/config.c\n@@ -3066,6 +3066,28 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,\n \tif (!config_filename)\n \t\tconfig_filename = filename_buf = repo_git_path(r, \"config\");\n \n+\t/*\n+\t * Do not allow updating a read-only configuration file; it is\n+\t * annoying and alarming that \"chmod a-w $HOME/.gitconfig\" does not\n+\t * prevent the global configuration file from getting clobbered by\n+\t * mistake with the usual \"create a new lockfile, copy the original\n+\t * while setting a new value, propagate the permission bits to the\n+\t * lockfile, and then rename it to replace the original\" dance.\n+\t */\n+\tif (!access(config_filename, W_OK)) {\n+\t\t; /* happy: we can write to it */\n+\t} else if (errno == ENOENT) {\n+\t\t; /* happy: we are creating this file anew */\n+\t} else {\n+\t\t/*\n+\t\t * most likely EACCES but other errors also indicate\n+\t\t * that we cannot write to it anyway.\n+\t\t */\n+\t\terror_errno(_(\"file %s unwritable\"), config_filename);\n+\t\tret = CONFIG_NO_WRITE;\n+\t\tgoto out_free;\n+\t}\n+\n \t/*\n \t * The lock serves a purpose in addition to locking: the new\n \t * contents of .git/config will be written into it.\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e3f8064889..876d376cc7 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -3019,4 +3019,34 @@ test_expect_success 'writing config retries until lock is released' '\n \ttest \"$(git config retried.key)\" = value\n '\n \n+test_expect_success POSIXPERM 'refuse to \"overwrite\" a read-only file' '\n+\tgconf=custom-global-config &&\n+\ttest_when_finished \"chmod +w $gconf\" &&\n+\t(\n+\t\tGIT_CONFIG_GLOBAL=$gconf &&\n+\t\texport GIT_CONFIG_GLOBAL &&\n+\t\tgit config set --global some.variable some-value &&\n+\t\tgit config list --global >expect &&\n+\n+\t\tchmod a-w $gconf &&\n+\t\t# \"get\" from read-only file should just work fine.\n+\t\tgit config list --global >actual &&\n+\n+\t\t# \"set\" to read-only file should be refused.\n+\t\ttest_must_fail git config set --global some.variable other-value &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# traditional \"config var val\" syntax to set\n+\t\ttest_must_fail git config --global some.variable third-value &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# \"unset\" to read-only file should be refused.\n+\t\ttest_must_fail git config unset --global some.variable &&\n+\t\tgit config list --global >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.56.0-369-gb8347ef8b9\n\n\n"},{"id":"556355","messageId":"xmqq5wyov738.fsf@gitster.g","threadId":"66585","inReplyTo":"at8R_QThIEedVbO8@pks.im","subject":"Re: [RFC/PATCH] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-26T15:01:47Z","receivedAt":"2026-10-26T15:01:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Oct 23, 2026 at 07:54:13AM -0700, Junio C Hamano wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> \n>> > What I wonder is what makes our config file so special compared to\n>> > everything else that uses lockfiles. Or put differently: if we think\n>> > it's important to honor permissions bits (which I think is sensible),\n>> > shouldn't we do that in a central location like in \"lockfile.c\" so that\n>> > all users of this interface behave the same?\n>> \n>> The $HOME/.gitconfig file and its XDG counterparts are the only\n>> out-of-tree files that suffer from this issue, and being outside the\n>> repository is what makes them special.  Theoretically, /etc/gitconfig\n>> on a multi-user system could have the same issue, but the containing\n>> directory would typically be read-only for regular users, which makes\n>> the issue irrelevant there.\n>\n> That explanation makes sense to me. Probably something to put into the\n> commit message :)\n\nI ended up rewriting the whole thing from scratch to follow the\nrecommended flow more closely, starting from an observation of an\nend-user perceived problem, analysis of the issue, and the solution.\nThe above naturally became a sidenote for the solution description.\n\nThanks.\n\n\n\n"},{"id":"557292","messageId":"aum4pZp5P0ZRC-cf@pks.im","threadId":"66585","inReplyTo":"xmqqa4o0v76o.fsf@gitster.g","subject":"Re: [PATCH v2] config: honor file permission bits","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:34:13Z","receivedAt":"2026-11-03T10:34:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 26, 2026 at 07:59:43AM -0700, Junio C Hamano wrote:\n> When dotfile sources are kept under version control, you might\n> protect installed files like ~/.gitconfig from accidental changes by\n> making them read-only.  This fails because updating a file writes a\n> new lockfile, copies and tweaks the content, and renames the\n> lockfile over the original.  The original file's permission bits are\n> ignored in this process.\n> \n> Teach the configuration writing machinery to inspect the original\n> file's permissions with access(2) and avoid replacing it when the\n> file should not be writable.\n> \n> This implementation targets configuration files rather than the\n> lockfile API layer.  While in-tree files like .git/index do not need\n> such protection, out-of-tree files like ~/.gitconfig and its XDG\n\nNit: a curious reader might wonder why such in-tree files don't need\nsuch protection.\n\n> counterparts are unique in this regard.  System-wide configuration\n> like /etc/gitconfig is typically in a directory already read-only to\n> normal users, making the issue irrelevant there.\n\nThanks for adding the motivation.\n\n> diff --git a/config.c b/config.c\n> index e0bb29b53d..2b84fb215c 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -3066,6 +3066,28 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,\n>  \tif (!config_filename)\n>  \t\tconfig_filename = filename_buf = repo_git_path(r, \"config\");\n>  \n> +\t/*\n> +\t * Do not allow updating a read-only configuration file; it is\n> +\t * annoying and alarming that \"chmod a-w $HOME/.gitconfig\" does not\n> +\t * prevent the global configuration file from getting clobbered by\n> +\t * mistake with the usual \"create a new lockfile, copy the original\n> +\t * while setting a new value, propagate the permission bits to the\n> +\t * lockfile, and then rename it to replace the original\" dance.\n> +\t */\n> +\tif (!access(config_filename, W_OK)) {\n> +\t\t; /* happy: we can write to it */\n> +\t} else if (errno == ENOENT) {\n> +\t\t; /* happy: we are creating this file anew */\n> +\t} else {\n> +\t\t/*\n> +\t\t * most likely EACCES but other errors also indicate\n> +\t\t * that we cannot write to it anyway.\n> +\t\t */\n> +\t\terror_errno(_(\"file %s unwritable\"), config_filename);\n\nShould we quote '%s' here? The function is quite inconsistent, where it\nsometimes quotes and sometimes doesn't. I'd say that quoting is the\nbetter choice though.\n\nOther than that this patch looks good to me, thanks!\n\nPatrick\n\n"},{"id":"557361","messageId":"xmqqld797msp.fsf@gitster.g","threadId":"66585","inReplyTo":"aum4pZp5P0ZRC-cf@pks.im","subject":"Re: [PATCH v2] config: honor file permission bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-03T19:11:18Z","receivedAt":"2026-11-03T19:11:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Nit: a curious reader might wonder why such in-tree files don't need\n> such protection.\n\nBut isn't it self evident that repository contents are supposed to\nbe mutable, and you are not even supposed to muck with filesystem\nlevel tools like chmod or mv to begin with?\n\n>> +\t\terror_errno(_(\"file %s unwritable\"), config_filename);\n>\n> Should we quote '%s' here? The function is quite inconsistent, where it\n> sometimes quotes and sometimes doesn't. I'd say that quoting is the\n> better choice though.\n\nWhile it probably makes sense, many of the existing messages in the\nsame function also lack the quote.  I would rather leave it for\nothers to clean up the subsystem as a whole, as I expect there would\nprobably be a huge fallout in expected test output ;-).\n\n"}]}