From 6edc4b227ecb14d3f3cf6b33c57af0b312f806f1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Mar 2022 11:58:37 +0200 Subject: [PATCH 1/2] Allow debugging unsafe directories' ownership When Git refuses to use an existing repository because it is owned by someone else than the current user, it can be a bit tricky on Windows to figure out what is going on. Let's help with that by offering some more information via the environment variable `GIT_TEST_DEBUG_UNSAFE_DIRECTORIES`. Signed-off-by: Johannes Schindelin --- Documentation/config/safe.txt | 6 ++++++ compat/mingw.c | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index 63597b2df8..9e57855f27 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -19,3 +19,9 @@ line option `-c safe.directory=`. The value of this setting is interpolated, i.e. `~/` expands to a path relative to the home directory and `%(prefix)/` expands to a path relative to Git's (runtime) prefix. ++ +Due to the permission model on Windows where ACLs are used instead of +Unix' simpler permission model, it can be a bit tricky to figure out why +a directory is considered unsafe. To help with this, Git will provide +more detailed information when the environment variable +`GIT_TEST_DEBUG_UNSAFE_DIRECTORIES` is set to `true`. diff --git a/compat/mingw.c b/compat/mingw.c index b10355a7ef..93f4f599b2 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1,6 +1,7 @@ #include "../git-compat-util.h" #include "win32.h" #include +#include #include #include #include @@ -3521,6 +3522,26 @@ int is_path_owned_by_current_sid(const char *path) IsValidSid(current_user_sid) && EqualSid(sid, current_user_sid)) result = 1; + else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { + LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; + + if (ConvertSidToStringSidA(sid, &str1)) + to_free1 = str1; + else + str1 = "(inconvertible)"; + + if (!current_user_sid) + str2 = "(none)"; + else if (!IsValidSid(current_user_sid)) + str2 = "(invalid)"; + else if (ConvertSidToStringSidA(current_user_sid, &str2)) + to_free2 = str2; + else + str2 = "(inconvertible)"; + warning("'%s' is owned by:\n\t'%s'\nbut the current user is:\n\t'%s'", path, str1, str2); + LocalFree(to_free1); + LocalFree(to_free2); + } } /* From 698ee939a4294678ffd70b4043703aff6b9b6d75 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Mar 2022 15:31:52 +0200 Subject: [PATCH 2/2] mingw: handle a file owned by the Administrators group correctly When an Administrator creates a file or directory, the created file/directory is owned not by the Administrator SID, but by the _Administrators Group_ SID. The reason is that users with administrator privileges usually run in unprivileged ("non-elevated") mode, and their user SID does not change when running in elevated mode. This is is relevant e.g. when running a GitHub workflow on a build agent, which runs in elevated mode: cloning a Git repository in a script step will cause the worktree to be owned by the Administrators Group SID, for example. Let's handle this case as following: if the current user is an administrator, Git should consider a worktree owned by the Administrators Group as if it were owned by said user. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index 93f4f599b2..ea34ec5f6d 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -3514,6 +3514,7 @@ int is_path_owned_by_current_sid(const char *path) else if (sid && IsValidSid(sid)) { /* Now, verify that the SID matches the current user's */ static PSID current_user_sid; + BOOL is_member; if (!current_user_sid) current_user_sid = get_current_user_sid(); @@ -3522,6 +3523,15 @@ int is_path_owned_by_current_sid(const char *path) IsValidSid(current_user_sid) && EqualSid(sid, current_user_sid)) result = 1; + else if (IsWellKnownSid(sid, WinBuiltinAdministratorsSid) && + CheckTokenMembership(NULL, sid, &is_member) && + is_member) + /* + * If owned by the Administrators group, and the + * current user is an administrator, we consider that + * okay, too. + */ + result = 1; else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;