[RFC/PATCH] config: honor file permission bits

[RFC/PATCH] config: honor file permission bits

[Junio C Hamano]

Because our updates to an existing file almost never makes
modifications in place, and instead create a new "lockfile",
propagate the permission bits from the original, copy the contents
from the original file while tweaking the contents as needed, and
then finally rename the lockfile to replace the original file, you
cannot "protect" your .global configuration file from accidental
clobbering by doing something like this:

	$ chmod a-w $HOME/.gitconfig

Of course, if you make your $HOME directory unwritable, it may
protect your .gitconfig from getting updated, but that goes totally
against the intuition and is not how you would protect a regular
file from getting overwritten.

Teach the configuration file writing machinery to inspect the
permission bits of the original with access(2) and refrain from
doing the usual "write a lockfile and replace the original with it"
dance ourselves when we shouldn't be able to write it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 config.c          | 22 ++++++++++++++++++++++
 t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git c/config.c w/config.c
index e0bb29b53d..2b84fb215c 100644
--- c/config.c
+++ w/config.c
@@ -3065,8 +3065,30 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
 
 	if (!config_filename)
 		config_filename = filename_buf = repo_git_path(r, "config");
 
+	/*
+	 * Do not allow updating a read-only configuration file; it is
+	 * annoying and alarming that "chmod a-w $HOME/.gitconfig" does not
+	 * prevent the global configuration file from getting clobbered by
+	 * mistake with the usual "create a new lockfile, copy the original
+	 * while setting a new value, propagate the permission bits to the
+	 * lockfile, and then rename it to replace the original" dance.
+	 */
+	if (!access(config_filename, W_OK)) {
+		; /* happy: we can write to it */
+	} else if (errno == ENOENT) {
+		; /* happy: we are creating this file anew */
+	} else {
+		/*
+		 * most likely EACCES but other errors also indicate
+		 * that we cannot write to it anyway.
+		 */
+		error_errno(_("file %s unwritable"), config_filename);
+		ret = CONFIG_NO_WRITE;
+		goto out_free;
+	}
+
 	/*
 	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
 	 */
diff --git c/t/t1300-config.sh w/t/t1300-config.sh
index e3f8064889..876d376cc7 100755
--- c/t/t1300-config.sh
+++ w/t/t1300-config.sh
@@ -3018,5 +3018,35 @@ test_expect_success 'writing config retries until lock is released' '
 	git -c core.configLockTimeout=5000 config retried.key value &&
 	test "$(git config retried.key)" = value
 '
 
+test_expect_success POSIXPERM 'refuse to "overwrite" a read-only file' '
+	gconf=custom-global-config &&
+	test_when_finished "chmod +w $gconf" &&
+	(
+		GIT_CONFIG_GLOBAL=$gconf &&
+		export GIT_CONFIG_GLOBAL &&
+		git config set --global some.variable some-value &&
+		git config list --global >expect &&
+
+		chmod a-w $gconf &&
+		# "get" from read-only file should just work fine.
+		git config list --global >actual &&
+
+		# "set" to read-only file should be refused.
+		test_must_fail git config set --global some.variable other-value &&
+		git config list --global >actual &&
+		test_cmp expect actual &&
+
+		# traditional "config var val" syntax to set
+		test_must_fail git config --global some.variable third-value &&
+		git config list --global >actual &&
+		test_cmp expect actual &&
+
+		# "unset" to read-only file should be refused.
+		test_must_fail git config unset --global some.variable &&
+		git config list --global >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done

Re: [RFC/PATCH] config: honor file permission bits

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Because our updates to an existing file almost never makes
> modifications in place, and instead create a new "lockfile",
> propagate the permission bits from the original, copy the contents
> from the original file while tweaking the contents as needed, and
> then finally rename the lockfile to replace the original file, you
> cannot "protect" your .global configuration file from accidental
> clobbering by doing something like this:
>
> 	$ chmod a-w $HOME/.gitconfig
>
> Of course, if you make your $HOME directory unwritable, it may
> protect your .gitconfig from getting updated, but that goes totally
> against the intuition and is not how you would protect a regular
> file from getting overwritten.
>
> Teach the configuration file writing machinery to inspect the
> permission bits of the original with access(2) and refrain from
> doing the usual "write a lockfile and replace the original with it"
> dance ourselves when we shouldn't be able to write it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  config.c          | 22 ++++++++++++++++++++++
>  t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)

Hmph, no responses to this RFC.  I somehow find it hard to believe
that I was the only person who got annoyed by this behaviour [*].

[Footnote]

 * I was very tempted to write "unexpected behaviour", but I know
   what happens in the code path, so it is not unexpected at least
   to me.

Re: [RFC/PATCH] config: honor file permission bits

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1872 bytes --]

On 2026-10-22 at 20:59:57, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Because our updates to an existing file almost never makes
> > modifications in place, and instead create a new "lockfile",
> > propagate the permission bits from the original, copy the contents
> > from the original file while tweaking the contents as needed, and
> > then finally rename the lockfile to replace the original file, you
> > cannot "protect" your .global configuration file from accidental
> > clobbering by doing something like this:
> >
> > 	$ chmod a-w $HOME/.gitconfig
> >
> > Of course, if you make your $HOME directory unwritable, it may
> > protect your .gitconfig from getting updated, but that goes totally
> > against the intuition and is not how you would protect a regular
> > file from getting overwritten.
> >
> > Teach the configuration file writing machinery to inspect the
> > permission bits of the original with access(2) and refrain from
> > doing the usual "write a lockfile and replace the original with it"
> > dance ourselves when we shouldn't be able to write it.
> >
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >  config.c          | 22 ++++++++++++++++++++++
> >  t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++
> >  2 files changed, 52 insertions(+)
> 
> Hmph, no responses to this RFC.  I somehow find it hard to believe
> that I was the only person who got annoyed by this behaviour [*].

Sorry, I looked but forgot to respond.  It seems sensible to me.

I don't personally make that file read-only, so it doesn't affect me at
all; instead, I just overwrite it with my personal configuration every
time I deploy my dotfiles.  I still think it's a reasonable thing to do
to help those who need it, though.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [RFC/PATCH] config: honor file permission bits

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I don't personally make that file read-only, so it doesn't affect me at
> all; instead, I just overwrite it with my personal configuration every
> time I deploy my dotfiles.

> I don't personally make that file read-only, so it doesn't affect me at
> all; instead, I just overwrite it with my personal configuration every
> time I deploy my dotfiles.

Well, I deploy my dotfiles from the freshly built one (the sources
of dotfiles are kept elsewhere under version control), too, but when
"installing", I make them read-only, just like all other "installed"
files like the contents of my ~/bin/ directory.  My personal
makefiles have "dry-run" support to tell me what the changes are in
the version I am about to install in 'patch' form, and that is how I
noticed that "git config" allowed to make such a mistake, even though
the file is read-only, to some failed experiment I made earlier.

Re: [RFC/PATCH] config: honor file permission bits

[Patrick Steinhardt]

On Mon, Oct 19, 2026 at 07:27:28AM -0700, Junio C Hamano wrote:
> Because our updates to an existing file almost never makes
> modifications in place, and instead create a new "lockfile",
> propagate the permission bits from the original, copy the contents
> from the original file while tweaking the contents as needed, and
> then finally rename the lockfile to replace the original file, you
> cannot "protect" your .global configuration file from accidental
> clobbering by doing something like this:
> 
> 	$ chmod a-w $HOME/.gitconfig
> 
> Of course, if you make your $HOME directory unwritable, it may
> protect your .gitconfig from getting updated, but that goes totally
> against the intuition and is not how you would protect a regular
> file from getting overwritten.
> 
> Teach the configuration file writing machinery to inspect the
> permission bits of the original with access(2) and refrain from
> doing the usual "write a lockfile and replace the original with it"
> dance ourselves when we shouldn't be able to write it.

What I wonder is what makes our config file so special compared to
everything else that uses lockfiles. Or put differently: if we think
it's important to honor permissions bits (which I think is sensible),
shouldn't we do that in a central location like in "lockfile.c" so that
all users of this interface behave the same?

Patrick

Re: [RFC/PATCH] config: honor file permission bits

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> What I wonder is what makes our config file so special compared to
> everything else that uses lockfiles. Or put differently: if we think
> it's important to honor permissions bits (which I think is sensible),
> shouldn't we do that in a central location like in "lockfile.c" so that
> all users of this interface behave the same?

The $HOME/.gitconfig file and its XDG counterparts are the only
out-of-tree files that suffer from this issue, and being outside the
repository is what makes them special.  Theoretically, /etc/gitconfig
on a multi-user system could have the same issue, but the containing
directory would typically be read-only for regular users, which makes
the issue irrelevant there.

I do not see any sensible reason why a user would want to "protect"
in-tree files like .git/index from Git, either by running "chmod -w"
on them or by any other means.  We make files under the .git/objects/
hierarchy read-only, but that is to prevent accidental overwriting
by tools other than Git, so it does not serve as an example of Git
needing to respect read-only bits in the general lockfile machinery.

Having said all that, the patch was intentionally isolated to the
caller's side of the API, primarily because auditing all code paths
that use the lockfile API was far beyond my available time budget.

Re: [RFC/PATCH] config: honor file permission bits

[Patrick Steinhardt]

On Fri, Oct 23, 2026 at 07:54:13AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > What I wonder is what makes our config file so special compared to
> > everything else that uses lockfiles. Or put differently: if we think
> > it's important to honor permissions bits (which I think is sensible),
> > shouldn't we do that in a central location like in "lockfile.c" so that
> > all users of this interface behave the same?
> 
> The $HOME/.gitconfig file and its XDG counterparts are the only
> out-of-tree files that suffer from this issue, and being outside the
> repository is what makes them special.  Theoretically, /etc/gitconfig
> on a multi-user system could have the same issue, but the containing
> directory would typically be read-only for regular users, which makes
> the issue irrelevant there.

That explanation makes sense to me. Probably something to put into the
commit message :)

Thanks!

Patrick

[PATCH v2] config: honor file permission bits

[Junio C Hamano]

When dotfile sources are kept under version control, you might
protect installed files like ~/.gitconfig from accidental changes by
making them read-only.  This fails because updating a file writes a
new lockfile, copies and tweaks the content, and renames the
lockfile over the original.  The original file's permission bits are
ignored in this process.

Teach the configuration writing machinery to inspect the original
file's permissions with access(2) and avoid replacing it when the
file should not be writable.

This implementation targets configuration files rather than the
lockfile API layer.  While in-tree files like .git/index do not need
such protection, out-of-tree files like ~/.gitconfig and its XDG
counterparts are unique in this regard.  System-wide configuration
like /etc/gitconfig is typically in a directory already read-only to
normal users, making the issue irrelevant there.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Only the commit message has been updated to mention that this is
   deliberately done at the configuration API, not at the lockfile
   layer.

 config.c          | 22 ++++++++++++++++++++++
 t/t1300-config.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/config.c b/config.c
index e0bb29b53d..2b84fb215c 100644
--- a/config.c
+++ b/config.c
@@ -3066,6 +3066,28 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
 	if (!config_filename)
 		config_filename = filename_buf = repo_git_path(r, "config");
 
+	/*
+	 * Do not allow updating a read-only configuration file; it is
+	 * annoying and alarming that "chmod a-w $HOME/.gitconfig" does not
+	 * prevent the global configuration file from getting clobbered by
+	 * mistake with the usual "create a new lockfile, copy the original
+	 * while setting a new value, propagate the permission bits to the
+	 * lockfile, and then rename it to replace the original" dance.
+	 */
+	if (!access(config_filename, W_OK)) {
+		; /* happy: we can write to it */
+	} else if (errno == ENOENT) {
+		; /* happy: we are creating this file anew */
+	} else {
+		/*
+		 * most likely EACCES but other errors also indicate
+		 * that we cannot write to it anyway.
+		 */
+		error_errno(_("file %s unwritable"), config_filename);
+		ret = CONFIG_NO_WRITE;
+		goto out_free;
+	}
+
 	/*
 	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e3f8064889..876d376cc7 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -3019,4 +3019,34 @@ test_expect_success 'writing config retries until lock is released' '
 	test "$(git config retried.key)" = value
 '
 
+test_expect_success POSIXPERM 'refuse to "overwrite" a read-only file' '
+	gconf=custom-global-config &&
+	test_when_finished "chmod +w $gconf" &&
+	(
+		GIT_CONFIG_GLOBAL=$gconf &&
+		export GIT_CONFIG_GLOBAL &&
+		git config set --global some.variable some-value &&
+		git config list --global >expect &&
+
+		chmod a-w $gconf &&
+		# "get" from read-only file should just work fine.
+		git config list --global >actual &&
+
+		# "set" to read-only file should be refused.
+		test_must_fail git config set --global some.variable other-value &&
+		git config list --global >actual &&
+		test_cmp expect actual &&
+
+		# traditional "config var val" syntax to set
+		test_must_fail git config --global some.variable third-value &&
+		git config list --global >actual &&
+		test_cmp expect actual &&
+
+		# "unset" to read-only file should be refused.
+		test_must_fail git config unset --global some.variable &&
+		git config list --global >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.56.0-369-gb8347ef8b9

Re: [RFC/PATCH] config: honor file permission bits

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 23, 2026 at 07:54:13AM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> 
>> > What I wonder is what makes our config file so special compared to
>> > everything else that uses lockfiles. Or put differently: if we think
>> > it's important to honor permissions bits (which I think is sensible),
>> > shouldn't we do that in a central location like in "lockfile.c" so that
>> > all users of this interface behave the same?
>> 
>> The $HOME/.gitconfig file and its XDG counterparts are the only
>> out-of-tree files that suffer from this issue, and being outside the
>> repository is what makes them special.  Theoretically, /etc/gitconfig
>> on a multi-user system could have the same issue, but the containing
>> directory would typically be read-only for regular users, which makes
>> the issue irrelevant there.
>
> That explanation makes sense to me. Probably something to put into the
> commit message :)

I ended up rewriting the whole thing from scratch to follow the
recommended flow more closely, starting from an observation of an
end-user perceived problem, analysis of the issue, and the solution.
The above naturally became a sidenote for the solution description.

Thanks.