diff mbox series

[v3,06/21] builtin/config: check for writeability after source is set up

Message ID 6de9097fb2c339be72aab3810abbccbfc39bd480.1715755055.git.ps@pks.im (mailing list archive)
State Accepted
Commit e44b018c5299f2632fcbb079bead00a529546763
Headers show
Series builtin/config: remove global state | expand

Commit Message

Patrick Steinhardt May 15, 2024, 6:42 a.m. UTC
The `check_write()` function verifies that we do not try to write to a
config source that cannot be written to, like for example stdin. But
while the new subcommands do call this function, they do so before
calling `handle_config_location()`. Consequently, we only end up
checking the default config location for writeability, not the location
that was actually specified by the caller of git-config(1).

Fix this by calling `check_write()` after `handle_config_location()`. We
will further clarify the relationship between those two functions in a
subsequent commit where we remove the global state that both implicitly
rely on.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c  | 10 +++++-----
 t/t1300-config.sh |  6 ++++++
 2 files changed, 11 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/builtin/config.c b/builtin/config.c
index 0842e4f198..9866d1a055 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -843,7 +843,6 @@  static int cmd_config_set(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	check_write();
 	check_argc(argc, 2, 2);
 
 	if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -856,6 +855,7 @@  static int cmd_config_set(int argc, const char **argv, const char *prefix)
 	comment = git_config_prepare_comment_string(comment_arg);
 
 	handle_config_location(prefix);
+	check_write();
 
 	value = normalize_value(argv[0], argv[1], &default_kvi);
 
@@ -891,13 +891,13 @@  static int cmd_config_unset(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, opts, builtin_config_unset_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	check_write();
 	check_argc(argc, 1, 1);
 
 	if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
 		die(_("--fixed-value only applies with 'value-pattern'"));
 
 	handle_config_location(prefix);
+	check_write();
 
 	if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern)
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
@@ -918,10 +918,10 @@  static int cmd_config_rename_section(int argc, const char **argv, const char *pr
 
 	argc = parse_options(argc, argv, prefix, opts, builtin_config_rename_section_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	check_write();
 	check_argc(argc, 2, 2);
 
 	handle_config_location(prefix);
+	check_write();
 
 	ret = git_config_rename_section_in_file(given_config_source.file,
 						argv[0], argv[1]);
@@ -943,10 +943,10 @@  static int cmd_config_remove_section(int argc, const char **argv, const char *pr
 
 	argc = parse_options(argc, argv, prefix, opts, builtin_config_remove_section_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	check_write();
 	check_argc(argc, 1, 1);
 
 	handle_config_location(prefix);
+	check_write();
 
 	ret = git_config_rename_section_in_file(given_config_source.file,
 						argv[0], NULL);
@@ -997,10 +997,10 @@  static int cmd_config_edit(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, opts, builtin_config_edit_usage, 0);
-	check_write();
 	check_argc(argc, 0, 0);
 
 	handle_config_location(prefix);
+	check_write();
 
 	return show_editor();
 }
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index d90a69b29f..9de2d95f06 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2835,6 +2835,12 @@  test_expect_success 'specifying multiple modes causes failure' '
 	test_cmp expect err
 '
 
+test_expect_success 'writing to stdin is rejected' '
+	echo "fatal: writing to stdin is not supported" >expect &&
+	test_must_fail git config ${mode_set} --file - foo.bar baz 2>err &&
+	test_cmp expect err
+'
+
 done
 
 test_done