Hi Brian,
On Fri, 24 Jan 2014, brian m. carlson wrote:
> On Fri, Jan 24, 2014 at 10:05:14PM +0100, Torsten Bögershausen wrote:
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index ba66c6e..033b4c2 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -324,6 +324,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> > statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
> > chmod(fname_old, statbuffer.st_mode);
> > }
> > + if (!stat(fname, &statbuffer)) {
> > + statbuffer.st_mode |= (S_IWUSR | S_IWGRP | S_IWOTH);
> > + chmod(fname, statbuffer.st_mode);
> > + }
>
> Are we sure that the file in question can never be a symlink?
On Windows: yes. Otherwise, no.
Having said that, the files in question are files generated by Git, so you
really would have to tamper with things in order to get into trouble.
> Because if it is, we'll end up changing the permissions on the wrong
> file.
In any case, I'd rather change the permissions only when the rename
failed. *And* I feel uncomfortable ignoring the return value...
> In general, I'm wary of changing permissions on a file to suit Windows's
> rename because of the symlink issue and the security issues that can
> result.
I agree on the Windows issue.
> Hard links probably have the same issue.
No, hard links have their own permissions. If you change the permissions
on a hardlink, any other hardlinks to the same content are unaffected.
Ciao,
Johannes