@matheusbsbs / matheus-bagliano / commits / e44b018c52

builtin/config: check for writeability after source is set up

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 15, 2024 at 08:42 UTC e44b018c5299f2632fcbb079bead00a529546763

2 files changed +11 -5

builtin/config.c

+5 -5

@@ -843,7 +843,6 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix)

843

844 argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,

845 PARSE_OPT_STOP_AT_NON_OPTION);

846 - check_write();

846 check_argc(argc, 2, 2);

847

848 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)

855 comment = git_config_prepare_comment_string(comment_arg);

856

857 handle_config_location(prefix);

858 + check_write();

859

860 value = normalize_value(argv[0], argv[1], &default_kvi);

861

@@ -891,13 +891,13 @@ static int cmd_config_unset(int argc, const char **argv, const char *prefix)

891

892 argc = parse_options(argc, argv, prefix, opts, builtin_config_unset_usage,

893 PARSE_OPT_STOP_AT_NON_OPTION);

894 - check_write();

894 check_argc(argc, 1, 1);

895

896 if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)

897 die(_("--fixed-value only applies with 'value-pattern'"));

898

899 handle_config_location(prefix);

900 + check_write();

901

902 if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern)

903 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

918

919 argc = parse_options(argc, argv, prefix, opts, builtin_config_rename_section_usage,

920 PARSE_OPT_STOP_AT_NON_OPTION);

921 - check_write();

921 check_argc(argc, 2, 2);

922

923 handle_config_location(prefix);

924 + check_write();

925

926 ret = git_config_rename_section_in_file(given_config_source.file,

927 argv[0], argv[1]);

@@ -943,10 +943,10 @@ static int cmd_config_remove_section(int argc, const char **argv, const char *pr

943

944 argc = parse_options(argc, argv, prefix, opts, builtin_config_remove_section_usage,

945 PARSE_OPT_STOP_AT_NON_OPTION);

946 - check_write();

946 check_argc(argc, 1, 1);

947

948 handle_config_location(prefix);

949 + check_write();

950

951 ret = git_config_rename_section_in_file(given_config_source.file,

952 argv[0], NULL);

@@ -997,10 +997,10 @@ static int cmd_config_edit(int argc, const char **argv, const char *prefix)

997 };

998

999 argc = parse_options(argc, argv, prefix, opts, builtin_config_edit_usage, 0);

1000 - check_write();

1000 check_argc(argc, 0, 0);

1001

1002 handle_config_location(prefix);

1003 + check_write();

1004

1005 return show_editor();

1006 }

t/t1300-config.sh

@@ -2835,6 +2835,12 @@ test_expect_success 'specifying multiple modes causes failure' '

2835 test_cmp expect err

2836 '

2837

2838 +test_expect_success 'writing to stdin is rejected' '

2839 + echo "fatal: writing to stdin is not supported" >expect &&

2840 + test_must_fail git config ${mode_set} --file - foo.bar baz 2>err &&

2841 + test_cmp expect err

2842 +'

2843 +

2844 done

2845

2846 test_done