[PATCH v2] config: honor file permission bits

[Junio C Hamano <gitster@pobox.com>]

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