safe.directory: use git_protected_config()

Use git_protected_config() to read `safe.directory` instead of
read_very_early_config(), making it 'protected configuration only'.

As a result, `safe.directory` now respects "-c", so update the tests and
docs accordingly. It used to ignore "-c" due to how it was implemented,
not because of security or correctness concerns [1].

[1] https://lore.kernel.org/git/xmqqlevabcsu.fsf@gitster.g/

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
index fa02f3ccc5490d2c4b72685b8fbb15e4fd3d0668..f72b4408798e27549146c99436b955f905de4580 100644 (file)
--- a/Documentation/config/safe.txt
+++ b/Documentation/config/safe.txt
@@ -12,9 +12,9 @@ via `git config --add`. To reset the list of safe directories (e.g. to
 override any such directories specified in the system config), add a
 `safe.directory` entry with an empty value.
 +
-This config setting is only respected when specified in a system or global
-config, not when it is specified in a repository config, via the command
-line option `-c safe.directory=<path>`, or in environment variables.
+This config setting is only respected in protected configuration (see
+<<SCOPES>>). This prevents the untrusted repository from tampering with this
+value.
 +
 The value of this setting is interpolated, i.e. `~/<path>` expands to a
 path relative to the home directory and `%(prefix)/<path>` expands to a

diff --git a/setup.c b/setup.c
index 09b6549ba9e211f4038025b338f5d42c086f3729..ec5b9139e32c117252f734e9e6c2ffe621bd8749 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1155,7 +1155,7 @@ static int ensure_valid_ownership(const char *gitfile,
         * constant regardless of what failed above. data.is_safe should be
         * initialized to false, and might be changed by the callback.
         */
-       read_very_early_config(safe_directory_cb, &data);
+       git_protected_config(safe_directory_cb, &data);
 
        return data.is_safe;
 }

diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
index 3908597d42d705161b5b8808e55c8125f8edffe1..f4d737dadd04a1b582ed765acdddd5ddad14d5de 100755 (executable)
--- a/t/t0033-safe-directory.sh
+++ b/t/t0033-safe-directory.sh
@@ -16,24 +16,20 @@ test_expect_success 'safe.directory is not set' '
        expect_rejected_dir
 '
 
-test_expect_success 'ignoring safe.directory on the command line' '
-       test_must_fail git -c safe.directory="$(pwd)" status 2>err &&
-       grep "dubious ownership" err
+test_expect_success 'safe.directory on the command line' '
+       git -c safe.directory="$(pwd)" status
 '
 
-test_expect_success 'ignoring safe.directory in the environment' '
-       test_must_fail env GIT_CONFIG_COUNT=1 \
-               GIT_CONFIG_KEY_0="safe.directory" \
-               GIT_CONFIG_VALUE_0="$(pwd)" \
-               git status 2>err &&
-       grep "dubious ownership" err
+test_expect_success 'safe.directory in the environment' '
+       env GIT_CONFIG_COUNT=1 \
+           GIT_CONFIG_KEY_0="safe.directory" \
+           GIT_CONFIG_VALUE_0="$(pwd)" \
+           git status
 '
 
-test_expect_success 'ignoring safe.directory in GIT_CONFIG_PARAMETERS' '
-       test_must_fail env \
-               GIT_CONFIG_PARAMETERS="${SQ}safe.directory${SQ}=${SQ}$(pwd)${SQ}" \
-               git status 2>err &&
-       grep "dubious ownership" err
+test_expect_success 'safe.directory in GIT_CONFIG_PARAMETERS' '
+       env GIT_CONFIG_PARAMETERS="${SQ}safe.directory${SQ}=${SQ}$(pwd)${SQ}" \
+           git status
 '
 
 test_expect_success 'ignoring safe.directory in repo config' '