Updates to the safe.directory config option #1215
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is difficult to change the ownership on a directory in our test suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment variable to trick Git into thinking we are in a differently-owned directory. This allows us to test that the config is parsed correctly. Signed-off-by: Derrick Stolee <[email protected]>
It seems that nothing is ever checking to make sure the safe directories in the configs actually have the key safe.directory, so some unrelated config that has a value with a certain directory would also make it a safe directory. Signed-off-by: Matheus Valadares <[email protected]> Signed-off-by: Derrick Stolee <[email protected]>
With the addition of the safe.directory in 8959555 (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02) released in v2.35.2, we are receiving feedback from a variety of users about the feature. Some users have a very large list of shared repositories and find it cumbersome to add this config for every one of them. In a more difficult case, certain workflows involve running Git commands within containers. The container boundary prevents any global or system config from communicating `safe.directory` values from the host into the container. Further, the container almost always runs as a different user than the owner of the directory in the host. To simplify the reactions necessary for these users, extend the definition of the safe.directory config value to include a possible '*' value. This value implies that all directories are safe, providing a single setting to opt-out of this protection. Note that an empty assignment of safe.directory clears all previous values, and this is already the case with the "if (!value || !*value)" condition. Signed-off-by: Derrick Stolee <[email protected]>
1 task
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -1119,7 +1119,8 @@ static int ensure_valid_ownership(const char *path) | |||
| { | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Derrick Stolee via GitGitGadget" <[email protected]> writes:
> From: Derrick Stolee <[email protected]>
>
> It is difficult to change the ownership on a directory in our test
> suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment
> variable to trick Git into thinking we are in a differently-owned
> directory. This allows us to test that the config is parsed correctly.
OK.
> - if (is_path_owned_by_current_user(path))
> + if (is_path_owned_by_current_user(path) &&
> + !git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0))
> return 1;
Shouldn't the overriding "GIT_TEST_BLAH" be checked before the
real logic kicks in, I wonder?
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> new file mode 100755
> index 00000000000..9380ff3d017
> --- /dev/null
> +++ b/t/t0033-safe-directory.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='verify safe.directory checks'
> +
> +. ./test-lib.sh
> +
> +GIT_TEST_ASSUME_DIFFERENT_OWNER=1
> +export GIT_TEST_ASSUME_DIFFERENT_OWNER
> +
> +expect_rejected_dir () {
> + test_must_fail git status 2>err &&
> + grep "safe.directory" err
> +}
> +...
> +test_expect_success 'safe.directory matches' '
> + git config --global --add safe.directory "$(pwd)" &&
> + git status
> +'
Just double checking, as I know you are much closer to the affected
platform than I'd ever be ;-) but is the use of $(pwd) safe and
correct here?
I always get confused between $(pwd) and $PWD, which does not make
any difference on platforms I have access to, but makes difference
to hurt Windows users.
> +test_expect_success 'safe.directory matches, but is reset' '
> + git config --global --add safe.directory "" &&
> + expect_rejected_dir
> +'
> +
> +test_done
Thanks. This step should apply to maint-2.30 cleanly, I would
think.
There was a problem hiding this comment.
On the Git mailing list, Derrick Stolee wrote (reply to this):
On 4/13/2022 12:24 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[email protected]> writes:
>
>> From: Derrick Stolee <[email protected]>
>>
>> It is difficult to change the ownership on a directory in our test
>> suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment
>> variable to trick Git into thinking we are in a differently-owned
>> directory. This allows us to test that the config is parsed correctly.
>
> OK.
>
>> - if (is_path_owned_by_current_user(path))
>> + if (is_path_owned_by_current_user(path) &&
>> + !git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0))
>> return 1;
>
> Shouldn't the overriding "GIT_TEST_BLAH" be checked before the
> real logic kicks in, I wonder?
Either order would work. I bet that checking the environment is
faster than checking the disk, so swapping the order would be prudent
here.
>> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
>> new file mode 100755
>> index 00000000000..9380ff3d017
>> --- /dev/null
>> +++ b/t/t0033-safe-directory.sh
>> @@ -0,0 +1,34 @@
>> +#!/bin/sh
>> +
>> +test_description='verify safe.directory checks'
>> +
>> +. ./test-lib.sh
>> +
>> +GIT_TEST_ASSUME_DIFFERENT_OWNER=1
>> +export GIT_TEST_ASSUME_DIFFERENT_OWNER
>> +
>> +expect_rejected_dir () {
>> + test_must_fail git status 2>err &&
>> + grep "safe.directory" err
>> +}
>> +...
>> +test_expect_success 'safe.directory matches' '
>> + git config --global --add safe.directory "$(pwd)" &&
>> + git status
>> +'
>
> Just double checking, as I know you are much closer to the affected
> platform than I'd ever be ;-) but is the use of $(pwd) safe and
> correct here?
>
> I always get confused between $(pwd) and $PWD, which does not make
> any difference on platforms I have access to, but makes difference
> to hurt Windows users.
These tests pass CI on Windows. I've had issues before using $PWD,
thinking back to tests in t7900-maintenance.sh that use $(pwd)
instead.
Thanks,
-Stolee
|
On the Git mailing list, Derrick Stolee wrote (reply to this): |
| @@ -1100,6 +1100,9 @@ static int safe_directory_cb(const char *key, const char *value, void *d) | |||
| { | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matheus Valadares via GitGitGadget" <[email protected]>
writes:
> From: Matheus Valadares <[email protected]>
>
> It seems that nothing is ever checking to make sure the safe directories
> in the configs actually have the key safe.directory, so some unrelated
> config that has a value with a certain directory would also make it a
> safe directory.
Good finding, and the fix is straight-forward and obviously correct.
Thanks.
> Signed-off-by: Matheus Valadares <[email protected]>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
> setup.c | 3 +++
> t/t0033-safe-directory.sh | 5 +++++
> 2 files changed, 8 insertions(+)
> diff --git a/setup.c b/setup.c
> index f54f449008a..a995c359c32 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1100,6 +1100,9 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
> {
> struct safe_directory_data *data = d;
>
> + if (strcmp(key, "safe.directory"))
> + return 0;
> +
> if (!value || !*value)
> data->is_safe = 0;
> else {
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> index 9380ff3d017..6f33c0dfefa 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -21,6 +21,11 @@ test_expect_success 'safe.directory does not match' '
> expect_rejected_dir
> '
>
> +test_expect_success 'path exist as different key' '
> + git config --global foo.bar "$(pwd)" &&
> + expect_rejected_dir
> +'
> +
> test_expect_success 'safe.directory matches' '
> git config --global --add safe.directory "$(pwd)" &&
> git status
| @@ -19,3 +19,10 @@ line option `-c safe.directory=<path>`. | |||
| The value of this setting is interpolated, i.e. `~/<path>` expands to a | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Derrick Stolee via GitGitGadget" <[email protected]> writes:
> To simplify the reactions necessary for these users, extend the
> definition of the safe.directory config value to include a possible '*'
> value. This value implies that all directories are safe, providing a
> single setting to opt-out of this protection.
OK. During the development of the original fix, we discussed if a
more flexible mechanism, like allowing globs, but ended up with the
simplest and easiest to explain option, with the expectation that we
may want to loosen it later as necessary. And this is certainly
what we would have expected to add.
> Note that an empty assignment of safe.directory clears all previous
> values, and this is already the case with the "if (!value || !*value)"
> condition.
OK.
> if (strcmp(key, "safe.directory"))
> return 0;
>
> - if (!value || !*value)
> + if (!value || !*value) {
> data->is_safe = 0;
> - else {
> + } else if (!strcmp(value, "*")) {
> + data->is_safe = 1;
> + } else {
> const char *interpolated = NULL;
>
> if (!git_config_pathname(&interpolated, key, value) &&
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
OK.
> index 6f33c0dfefa..239d93f4d21 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -36,4 +36,14 @@ test_expect_success 'safe.directory matches, but is reset' '
> expect_rejected_dir
> '
>
> +test_expect_success 'safe.directory=*' '
> + git config --global --add safe.directory "*" &&
> + git status
> +'
> +
> +test_expect_success 'safe.directory=*, but is reset' '
> + git config --global --add safe.directory "" &&
> + expect_rejected_dir
> +'
Thanks.
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
| @@ -1119,7 +1119,8 @@ static int ensure_valid_ownership(const char *path) | |||
| { | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Wed, Apr 13 2022, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email protected]>
>
> It is difficult to change the ownership on a directory in our test
> suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment
> variable to trick Git into thinking we are in a differently-owned
> directory. This allows us to test that the config is parsed correctly.
I think this is a good trade-off, but FWIW I'd think we could test also
without the git_env_bool() by having the test depend on !NOT_ROOT, then
check the owner of t/test-lib.sh, and chown to that user (i.e. the
"real" user).
But that's all sorts of more fragile than just this test variable..
> +test_description='verify safe.directory checks'
> +
> +. ./test-lib.sh
> +
> +GIT_TEST_ASSUME_DIFFERENT_OWNER=1
> +export GIT_TEST_ASSUME_DIFFERENT_OWNER
Instead of this "export" perhaps just add it in front of the "git
status"?
These tests also pass with SANITIZE=leak, so please add
TEST_PASSES_SANITIZE_LEAK=true at the top.
> +expect_rejected_dir () {
> + test_must_fail git status 2>err &&
> + grep "safe.directory" err
> +}
> +
> +test_expect_success 'safe.directory is not set' '
> + expect_rejected_dir
> +'
> +
> +test_expect_success 'safe.directory does not match' '
> + git config --global safe.directory bogus &&
> + expect_rejected_dir
> +'
> +
> +test_expect_success 'safe.directory matches' '
> + git config --global --add safe.directory "$(pwd)" &&
nit: $PWD instead of $(pwd)
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Ævar Arnfjörð Bjarmason <[email protected]> writes:
(just this part)
> These tests also pass with SANITIZE=leak, so please add
> TEST_PASSES_SANITIZE_LEAK=true at the top.
Derrick, please ignore the above. It is totally outside the scope
of these patches, and they are meant to be applied on top of the
2.30 maintenance track, where TEST_PASSES_SANITIZE_LEAK=true was
irrelevant.
I do not mind adding such after the dust settles on top of 'master'
(or possibly 'maint'), but not as part of these "let's fix the screw
up in 2.30.3 and its friends" effort.
Thanks.
There was a problem hiding this comment.
On the Git mailing list, Derrick Stolee wrote (reply to this):
On 4/13/2022 3:16 PM, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Apr 13 2022, Derrick Stolee via GitGitGadget wrote:
>
>> From: Derrick Stolee <[email protected]>
>>
>> It is difficult to change the ownership on a directory in our test
>> suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment
>> variable to trick Git into thinking we are in a differently-owned
>> directory. This allows us to test that the config is parsed correctly.
>
> I think this is a good trade-off, but FWIW I'd think we could test also
> without the git_env_bool() by having the test depend on !NOT_ROOT, then
> check the owner of t/test-lib.sh, and chown to that user (i.e. the
> "real" user).
>
> But that's all sorts of more fragile than just this test variable..
>> +test_description='verify safe.directory checks'
>> +
>> +. ./test-lib.sh
>> +
>> +GIT_TEST_ASSUME_DIFFERENT_OWNER=1
>> +export GIT_TEST_ASSUME_DIFFERENT_OWNER
>
> Instead of this "export" perhaps just add it in front of the "git
> status"?
If the only runs were in this helper below, then yes.
>> +expect_rejected_dir () {
>> + test_must_fail git status 2>err &&
>> + grep "safe.directory" err
>> +}
Later patches add more success cases that run 'git status'
as its verification that the match works. I didn't think it
was good to have this environment variable set for each of
those invocations.
This script has one purpose, and this environment variable
is required to make any of the checks work. Setting it
globally seems the best way to do that.
>> +test_expect_success 'safe.directory matches' '
>> + git config --global --add safe.directory "$(pwd)" &&
>
> nit: $PWD instead of $(pwd)
Historically, $PWD doesn't work properly across platforms,
so I have used $(pwd) consistently across many contributions.
Thanks,
-Stolee
|
User |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
On the Git mailing list, Taylor Blau wrote (reply to this): |
derrickstolee
added a commit
to git-for-windows/git
that referenced
this pull request
Apr 13, 2022
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
This patch series was integrated into maint via git@d516b2d. |
|
This patch series was integrated into seen via git@1ac7422. |
|
This patch series was integrated into master via git@1ac7422. |
|
This patch series was integrated into next via git@5071ed8. |
dscho
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 22, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 22, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 22, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 22, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 25, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 25, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 25, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 25, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Nov 25, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Nov 25, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Dec 16, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Dec 16, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Dec 17, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Dec 17, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Dec 19, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Dec 30, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
dscho
pushed a commit
to dscho/git
that referenced
this pull request
Dec 30, 2024
…irectory` The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Dec 31, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Dec 31, 2024
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 1, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 1, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 1, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 1, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 1, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 2, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 2, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 2, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 3, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 6, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
git-for-windows-ci
pushed a commit
to git-for-windows/git
that referenced
this pull request
Jan 6, 2025
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here is a very fast response to the security release yesterday.
The second patch here is an adaptation from a contributor who created a pull request against git/git [1]. I augmented the patch with a test (the test infrastructure is added in patch 1).
The third patch is a change to the
safe.directoryconfig option to include a possible "*" value to completely opt-out of the check. This will be particularly helpful for cases where users run Git commands within a container. This container workflow always runs as a different user than the host, but also the container does not have access to the host's system or global config files. It's also helpful for users who don't want to set the config for a large number of shared repositories [2].Thanks,
-Stolee
[1] git#1235
[2] git-for-windows#3787
[3] desktop/desktop#14336
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: Ævar Arnfjörð Bjarmason [email protected]
cc: SZEDER Gábor [email protected]
cc: Carlo Marcelo Arenas Belón [email protected]
cc: Eric Sunshine [email protected]