Due to the semantics being substantially different from Unix, the safe.directory feature presents its own set of problems on Windows. One particular issue would have prevented it from working in GitHub Actions' build agents, which we definitely rely on in the Git project itself. This was addressed via the fifth patch, which had made it (in a slightly different form) already into Git for Windows v2.35.2, and they are ready to be applied to core Git, too. The FAT32 patch came in later, and was released as part of Git for Windows v2.37.0, so I also have confidence that it is stable and ready to be integrated into core Git, too. Changes since v1: * Restructured the patch series. * Instead of an environment variable to turn on debugging, we now always show the platform-dependent information together with the error message about the dubious ownership (iff it is shown, that is), based on an idea by Junio. * Rebased onto gc/bare-repo-discovery to avoid a merge conflict. Johannes Schindelin (5): setup: fix some formatting Prepare for more detailed "dubious ownership" messages mingw: provide details about unsafe directories' ownership mingw: be more informative when ownership check fails on FAT32 mingw: handle a file owned by the Administrators group correctly compat/mingw.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++- compat/mingw.h | 2 +- git-compat-util.h | 5 +++- setup.c | 30 ++++++++++++++---------- 4 files changed, 81 insertions(+), 15 deletions(-) base-commit: 776f184893d2861a729aa4b91d69931036e03e4b Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1286%2Fdscho%2Fsafe.directory-and-windows-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1286/dscho/safe.directory-and-windows-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1286 Range-diff vs v1: -: ----------- > 1: 301d94f18f5 setup: fix some formatting -: ----------- > 2: 8cc45e4922a Prepare for more detailed "dubious ownership" messages 1: 3480381b8b9 ! 3: 63494818105 Allow debugging unsafe directories' ownership @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx> ## Commit message ## - Allow debugging unsafe directories' ownership + mingw: provide details about 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`. + Let's help with that by providing more detailed information. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> - ## Documentation/config/safe.txt ## -@@ Documentation/config/safe.txt: which id the original user has. - If that is not what you would prefer and want git to only trust - repositories that are owned by root instead, then you can remove - the `SUDO_UID` variable from root's environment before invoking git. -++ -+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`. - ## compat/mingw.c ## @@ #include "../git-compat-util.h" @@ compat/mingw.c #include <conio.h> #include <wchar.h> #include "../strbuf.h" -@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) +@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path, struct strbuf *report) IsValidSid(current_user_sid) && EqualSid(sid, current_user_sid)) result = 1; -+ else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { ++ else if (report) { + LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; + + if (ConvertSidToStringSidA(sid, &str1)) @@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) + 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); ++ strbuf_addf(report, ++ "'%s' is owned by:\n" ++ "\t'%s'\nbut the current user is:\n" ++ "\t'%s'\n", path, str1, str2); + LocalFree(to_free1); + LocalFree(to_free2); + } } /* - - ## setup.c ## -@@ setup.c: const char *setup_git_directory_gently(int *nongit_ok) - case GIT_DIR_INVALID_OWNERSHIP: - if (!nongit_ok) { - struct strbuf quoted = STRBUF_INIT; -+ struct strbuf hint = STRBUF_INIT; -+ -+#ifdef __MINGW32__ -+ if (!git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) -+ strbuf_addstr(&hint, -+ _("\n\nSet the environment variable " -+ "GIT_TEST_DEBUG_UNSAFE_DIRECTORIES=true " -+ "and run\n" -+ "again for more information.")); -+#endif - - sq_quote_buf_pretty("ed, dir.buf); - die(_("detected dubious ownership in repository at '%s'\n" - "To add an exception for this directory, call:\n" - "\n" -- "\tgit config --global --add safe.directory %s"), -- dir.buf, quoted.buf); -+ "\tgit config --global --add safe.directory %s%s"), -+ dir.buf, quoted.buf, hint.buf); - } - *nongit_ok = 1; - break; 3: dae03f1b204 ! 4: 7aaa6248dfe mingw: be more informative when ownership check fails on FAT32 @@ Commit message any ownership information anyway, and the `GetNamedSecurityInfoW()` call pretends that everything is owned "by the world". - Let's special-case that scenario and tell the user what's going on, at - least when they set `GIT_TEST_DEBUG_UNSAFE_DIRECTORIES`. + Let's special-case that scenario and tell the user what's going on. This addresses https://github.com/git-for-windows/git/issues/3886 @@ compat/mingw.c: static PSID get_current_user_sid(void) + return 0; +} + - int is_path_owned_by_current_sid(const char *path) + int is_path_owned_by_current_sid(const char *path, struct strbuf *report) { WCHAR wpath[MAX_PATH]; -@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) - * okay, too. - */ +@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path, struct strbuf *report) + IsValidSid(current_user_sid) && + EqualSid(sid, current_user_sid)) result = 1; -- else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { -+ else if (IsWellKnownSid(sid, WinWorldSid) && -+ git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0) && +- else if (report) { ++ else if (report && ++ IsWellKnownSid(sid, WinWorldSid) && + !acls_supported(path)) { + /* + * On FAT32 volumes, ownership is not actually recorded. + */ -+ warning("'%s' is on a file system that does not record ownership", path); -+ } else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { ++ strbuf_addf(report, "'%s' is on a file system that does" ++ "not record ownership\n", path); ++ } else if (report) { LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; if (ConvertSidToStringSidA(sid, &str1)) 2: be06d711a13 ! 5: fbfaff2ec21 mingw: handle a file owned by the Administrators group correctly @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> ## compat/mingw.c ## -@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) +@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path, struct strbuf *report) else if (sid && IsValidSid(sid)) { /* Now, verify that the SID matches the current user's */ static PSID current_user_sid; @@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) if (!current_user_sid) current_user_sid = get_current_user_sid(); -@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) +@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path, struct strbuf *report) IsValidSid(current_user_sid) && EqualSid(sid, current_user_sid)) result = 1; @@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path) + * okay, too. + */ + result = 1; - else if (git_env_bool("GIT_TEST_DEBUG_UNSAFE_DIRECTORIES", 0)) { - LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; - + else if (report && + IsWellKnownSid(sid, WinWorldSid) && + !acls_supported(path)) { -- gitgitgadget
