[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