| Message ID | 20240104192202.2124-2-soekkle@freenet.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Replace SID with domain/username on Windows | expand |
Sören Krecker <soekkle@freenet.de> writes: > Subject: Re: [PATCH v5 1/1] Adds domain/username to error message Looking at past commits that worked on the area this patch touches, namely, 7c83470e (mingw: be more informative when ownership check fails on FAT32, 2022-08-08) and e883e04b (mingw: provide details about unsafe directories' ownership, 2022-08-08), I would retitle the commit perhaps like so: Subject: [PATCH v5] mingw: give more details about unsafe directory's ownership if I were doing this patch. > Adds domain/username in error message, if owner sid of repository and "Adds" -> "Add". > user sid are not equal on windows systems. > > Old Prompted error message: > ''' > fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git' > 'C:/Users/test/source/repos/git' is owned by: > 'S-1-5-21-571067702-4104414259-3379520149-500' > but the current user is: > 'S-1-5-21-571067702-4104414259-3379520149-1001' > To add an exception for this directory, call: > > git config --global --add safe.directory C:/Users/test/source/repos/git > ''' > > New prompted error massage: "massage" -> "message". I probably would drop two "prompted" from the above, too, if I were doing this patch. Thanks for working on making this error message more readable. I'll queue it when I see an Ack from Dscho. > ''' > fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git' > 'C:/Users/test/source/repos/git' is owned by: > 'DESKTOP-L78JVA6/Administrator' (S-1-5-21-571067702-4104414259-3379520149-500) > but the current user is: > 'DESKTOP-L78JVA6/test' (S-1-5-21-571067702-4104414259-3379520149-1001) > To add an exception for this directory, call: > > git config --global --add safe.directory C:/Users/test/source/repos/git > ''' > > Signed-off-by: Sören Krecker <soekkle@freenet.de> > --- > compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 51 insertions(+), 13 deletions(-) > > diff --git a/compat/mingw.c b/compat/mingw.c > index 42053c1f65..6240387205 100644 > --- a/compat/mingw.c > +++ b/compat/mingw.c > @@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void) > return result; > } > > +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str) > +{ > + SID_NAME_USE pe_use; > + DWORD len_user = 0, len_domain = 0; > + BOOL translate_sid_to_user; > + > + /* returns only FALSE, because the string pointers are NULL*/ > + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain, > + &pe_use); > + /*Alloc needed space of the strings*/ > + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user); > + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user, > + *str, &len_domain, &pe_use); > + if (translate_sid_to_user == FALSE) > + FREE_AND_NULL(*str); > + else > + (*str)[len_domain] = '/'; > + return translate_sid_to_user; > +} > + > static int acls_supported(const char *path) > { > size_t offset = offset_1st_component(path); > @@ -2765,27 +2785,45 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) > 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; > + LPSTR str1, str2, str3, str4, to_free1 = NULL, to_free3 = NULL, to_local_free2=NULL, to_local_free4=NULL; > > - if (ConvertSidToStringSidA(sid, &str1)) > + if (user_sid_to_user_name(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; > + if (ConvertSidToStringSidA(sid, &str2)) > + to_local_free2 = str2; > else > str2 = "(inconvertible)"; > + > + if (!current_user_sid) { > + str3 = "(none)"; > + str4 = "(none)"; > + } > + else if (!IsValidSid(current_user_sid)) { > + str3 = "(invalid)"; > + str4 = "(invalid)"; > + } else { > + if (user_sid_to_user_name(current_user_sid, > + &str3)) > + to_free3 = str3; > + else > + str3 = "(inconvertible)"; > + if (ConvertSidToStringSidA(current_user_sid, > + &str4)) > + to_local_free4 = str4; > + else > + str4 = "(inconvertible)"; > + } > 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); > + "\t'%s' (%s)\nbut the current user is:\n" > + "\t'%s' (%s)\n", > + path, str1, str2, str3, str4); > + free(to_free1); > + LocalFree(to_local_free2); > + free(to_free3); > + LocalFree(to_local_free4); > } > }
diff --git a/compat/mingw.c b/compat/mingw.c index 42053c1f65..6240387205 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void) return result; } +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str) +{ + SID_NAME_USE pe_use; + DWORD len_user = 0, len_domain = 0; + BOOL translate_sid_to_user; + + /* returns only FALSE, because the string pointers are NULL*/ + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain, + &pe_use); + /*Alloc needed space of the strings*/ + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user); + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user, + *str, &len_domain, &pe_use); + if (translate_sid_to_user == FALSE) + FREE_AND_NULL(*str); + else + (*str)[len_domain] = '/'; + return translate_sid_to_user; +} + static int acls_supported(const char *path) { size_t offset = offset_1st_component(path); @@ -2765,27 +2785,45 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) 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; + LPSTR str1, str2, str3, str4, to_free1 = NULL, to_free3 = NULL, to_local_free2=NULL, to_local_free4=NULL; - if (ConvertSidToStringSidA(sid, &str1)) + if (user_sid_to_user_name(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; + if (ConvertSidToStringSidA(sid, &str2)) + to_local_free2 = str2; else str2 = "(inconvertible)"; + + if (!current_user_sid) { + str3 = "(none)"; + str4 = "(none)"; + } + else if (!IsValidSid(current_user_sid)) { + str3 = "(invalid)"; + str4 = "(invalid)"; + } else { + if (user_sid_to_user_name(current_user_sid, + &str3)) + to_free3 = str3; + else + str3 = "(inconvertible)"; + if (ConvertSidToStringSidA(current_user_sid, + &str4)) + to_local_free4 = str4; + else + str4 = "(inconvertible)"; + } 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); + "\t'%s' (%s)\nbut the current user is:\n" + "\t'%s' (%s)\n", + path, str1, str2, str3, str4); + free(to_free1); + LocalFree(to_local_free2); + free(to_free3); + LocalFree(to_local_free4); } }
Adds domain/username in error message, if owner sid of repository and user sid are not equal on windows systems. Old Prompted error message: ''' fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git' 'C:/Users/test/source/repos/git' is owned by: 'S-1-5-21-571067702-4104414259-3379520149-500' but the current user is: 'S-1-5-21-571067702-4104414259-3379520149-1001' To add an exception for this directory, call: git config --global --add safe.directory C:/Users/test/source/repos/git ''' New prompted error massage: ''' fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git' 'C:/Users/test/source/repos/git' is owned by: 'DESKTOP-L78JVA6/Administrator' (S-1-5-21-571067702-4104414259-3379520149-500) but the current user is: 'DESKTOP-L78JVA6/test' (S-1-5-21-571067702-4104414259-3379520149-1001) To add an exception for this directory, call: git config --global --add safe.directory C:/Users/test/source/repos/git ''' Signed-off-by: Sören Krecker <soekkle@freenet.de> --- compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 13 deletions(-)