On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt <j.sixt@xxxxxxxxxxxxx> wrote: > Am 5/6/2014 2:17, schrieb Eric Wong: >> Users may already store sensitive data such as imap.pass in >> ..git/config; making the file world-readable when "git config" >> is called to edit means their password would be compromised >> on a shared system. >> >> [v2: updated for section renames, as noted by Junio] >> >> Signed-off-by: Eric Wong <normalperson@xxxxxxxx> >> --- >> config.c | 16 ++++++++++++++++ >> t/t1300-repo-config.sh | 10 ++++++++++ >> 2 files changed, 26 insertions(+) >> >> diff --git a/config.c b/config.c >> index a30cb5c..c227aa8 100644 >> --- a/config.c >> +++ b/config.c >> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char *config_filename, >> MAP_PRIVATE, in_fd, 0); >> close(in_fd); >> >> + if (fchmod(fd, st.st_mode & 07777) < 0) { >> + error("fchmod on %s failed: %s", >> + lock->filename, strerror(errno)); >> + ret = CONFIG_NO_WRITE; >> + goto out_free; >> + } > > This introduces a severe failure in the Windows port because we do not > implement fchmod. Existing fchmod invocations do not check the return > value, and they are only interested in removing the write bits, and we > generally don't care a lot if files remain writable. > > I'm not proficient enough to add any ACL fiddling to fchmod that would be > required by the above change, whose purpose is to be strict about > permissions. Nor am I interested (who the heck is sharing a Windows box > anyway? ;-) > > Therefore, here's just a work-around patch to keep things going on > Windows. Any opinions from the Windows corner? > Since we use MSVCRT's chmod implementation directly which only checks for S_IWRITE,and Get/SetFileAttributes to simply set or clear the FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except using Get/SetFileInformationByHandle instead? That takes us in a better direction, IMO. Adding full ACL support seems like a bigger project. If there's no objection, I'll sketch up a patch for that... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html