git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/3] safe.directory: normalize the configured path

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jul 25, 2024, 09:45 UTC
Message-ID
<5332f244-7476-492a-a797-2ef7ba73f490@gmail.com>
In-Reply-To
<20240723021900.388020-3-gitster@pobox.com>
Hi Junio
On 23/07/2024 03:18, Junio C Hamano wrote:
Show 10 quoted lines
> The pathname of a repository comes from getcwd() and it could be a
> path aliased via symbolic links, e.g., the real directory may be
> /home/u/repository but a symbolic link /home/u/repo may point at it,
> and the clone request may come as "git clone file:///home/u/repo/"
> 
> A request to check if /home/u/repository is safe would be rejected
> if the safe.directory configuration allows /home/u/repo/ but not its
> alias /home/u/repository/.  Normalize the paths configured for the
> safe.directory configuration variable before comparing them with the
> path being checked.

I think this is sensible if the key is an absolute path, if the key is a relative path I think we should ignore it as it is not clear which directory the user meant.

Best Wishes
Phillip
Show 104 quoted lines
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   setup.c                   | 12 +++++++++
>   t/t0033-safe-directory.sh | 57 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 69 insertions(+)
> 
> diff --git a/setup.c b/setup.c
> index 45bbbe329f..29304d7452 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1236,6 +1236,16 @@ static int safe_directory_cb(const char *key, const char *value,
>   
>   		if (!git_config_pathname(&allowed, key, value)) {
>   			const char *check = allowed ? allowed : value;
> +			char *to_free = real_pathdup(check, 0);
> +
> +			if (!to_free) {
> +				warning(_("safe.directory '%s' cannot be normalized"),
> +					check);
> +				goto next;
> +			} else {
> +				check = to_free;
> +			}
> +
>   			if (ends_with(check, "/*")) {
>   				size_t len = strlen(check);
>   				if (!fspathncmp(check, data->path, len - 1))
> @@ -1243,7 +1253,9 @@ static int safe_directory_cb(const char *key, const char *value,
>   			} else if (!fspathcmp(data->path, check)) {
>   				data->is_safe = 1;
>   			}
> +			free(to_free);
>   		}
> +	next:
>   		if (allowed != value)
>   			free(allowed);
>   	}
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> index 07ac0f9a01..ea74657255 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -176,4 +176,61 @@ test_expect_success SYMLINKS 'checked leading paths are normalized' '
>   	git -C repo/s/.git/ for-each-ref
>   '
>   
> +test_expect_success SYMLINKS 'configured paths are normalized' '
> +	test_when_finished "rm -rf repository; rm -f repo" &&
> +	(
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		git config --global --unset-all safe.directory
> +	) &&
> +	git init repository &&
> +	ln -s repository repo &&
> +	(
> +		cd repository &&
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		test_commit sample
> +	) &&
> +
> +	(
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		git config --global safe.directory "$(pwd)/repo"
> +	) &&
> +	git -C repository for-each-ref &&
> +	git -C repository/ for-each-ref &&
> +	git -C repo for-each-ref &&
> +	git -C repo/ for-each-ref &&
> +	test_must_fail git -C repository/.git for-each-ref &&
> +	test_must_fail git -C repository/.git/ for-each-ref &&
> +	test_must_fail git -C repo/.git for-each-ref &&
> +	test_must_fail git -C repo/.git/ for-each-ref
> +'
> +
> +test_expect_success SYMLINKS 'configured leading paths are normalized' '
> +	test_when_finished "rm -rf repository; rm -f repo" &&
> +	(
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		git config --global --unset-all safe.directory
> +	) &&
> +	mkdir -p repository &&
> +	git init repository/s &&
> +	ln -s repository repo &&
> +	(
> +		cd repository/s &&
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		test_commit sample
> +	) &&
> +
> +	(
> +		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> +		git config --global safe.directory "$(pwd)/repo/*"
> +	) &&
> +	git -C repository/s for-each-ref &&
> +	git -C repository/s/ for-each-ref &&
> +	git -C repository/s/.git for-each-ref &&
> +	git -C repository/s/.git/ for-each-ref &&
> +	git -C repo/s for-each-ref &&
> +	git -C repo/s/ for-each-ref &&
> +	git -C repo/s/.git for-each-ref &&
> +	git -C repo/s/.git/ for-each-ref
> +'
> +
>   test_done
Previous: Junio C HamanoNext: Junio C Hamano
Message 9 of 39 in “safe.directory clean-up”
  1. 0/2 safe.directory clean-upJunio C Hamano, Jul 20, 2024
  2. 1/2 safe.directory: normalize the checked pathJunio C Hamano, Jul 20, 2024
  3. 2/2 safe.directory: normalize the configured pathJunio C Hamano, Jul 20, 2024
  4. 3/2 setup: use a single return path in setup_git_directory*()Junio C Hamano, Jul 20, 2024
  5. 4/2 setup: cache normalized safe.directory configurationJunio C Hamano, Jul 20, 2024
  6. 0/3 safe.directory clean-upJunio C Hamano, Jul 23, 2024
  7. 1/3 safe.directory: normalize the checked pathJunio C Hamano, Jul 23, 2024
  8. 2/3 safe.directory: normalize the configured pathJunio C Hamano, Jul 23, 2024
  9. Phillip WoodJul 25, 2024
  10. Junio C HamanoJul 25, 2024
  11. Phillip WoodAug 14, 2024
  12. Junio C HamanoAug 14, 2024
  13. Phillip WoodAug 15, 2024
  14. Junio C HamanoAug 15, 2024
  15. Jeff KingJul 26, 2024
  16. Junio C HamanoJul 26, 2024
  17. Jeff KingJul 27, 2024
  18. 3/3 safe.directory: setting safe.directory="." allows the "current" directoryJunio C Hamano, Jul 23, 2024
  19. Phillip WoodJul 25, 2024
  20. Junio C HamanoJul 25, 2024
  21. Phillip WoodJul 25, 2024
  22. Junio C HamanoJul 25, 2024
  23. 0/3 safe.directory clean-upJunio C Hamano, Jul 30, 2024
  24. 1/3 safe.directory: normalize the checked pathJunio C Hamano, Jul 30, 2024
  25. 2/3 safe.directory: normalize the configured pathJunio C Hamano, Jul 30, 2024
  26. Jeff KingJul 30, 2024
  27. Junio C HamanoJul 30, 2024
  28. Jeff KingJul 30, 2024
  29. Jeff KingJul 30, 2024
  30. Junio C HamanoJul 30, 2024
  31. safe.directory: preliminary clean-upJunio C Hamano, Jul 30, 2024
  32. Jeff KingJul 30, 2024
  33. Jeff KingJul 30, 2024
  34. 3/3 safe.directory: setting safe.directory="." allows the "current" directoryJunio C Hamano, Jul 30, 2024
  35. 0/4 safe.directory clean-upJunio C Hamano, Jul 30, 2024
  36. 2/4 safe.directory: normalize the checked pathJunio C Hamano, Jul 30, 2024
  37. 1/4 safe.directory: preliminary clean-upJunio C Hamano, Jul 30, 2024
  38. 4/4 safe.directory: setting safe.directory="." allows the "current" directoryJunio C Hamano, Jul 30, 2024
  39. 3/4 safe.directory: normalize the configured pathJunio C Hamano, Jul 30, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.