Fairuzan Roslan [off-list ref] writes:
$ git clone https://github.com/robbyrussell/oh-my-zsh.git
Cloning into 'oh-my-zsh'...
remote: Counting objects: 11830, done.
remote: Total 11830 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.
Resolving deltas: 100% (6510/6510), done.
warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
This should be fixable from Git itself, by replacing the calls to
"unlink" with something like
int unlink_or_chmod(...) {
if (unlink(...)) {
chmod(...); // give user write permission
return unlink(...);
}
}
This does not add extra cost in the normal case, and would fix this
particular issue for afp shares. So, I think that would fix the biggest
problem for afp-share users without disturbing others. It seems
reasonable to me to do that unconditionnally.
$ rm -rf oh-my-zsh/.git/objects/pack/tmp_*
rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted
rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
What happens if you do "rm -fr oh-my-zsh/.git/objects/pack/" (i.e.
remove the directory, not the files)?
If you can still remove the directory, then I'd say the solution above
could be sufficient: the user isn't supposed to interfer with the
content of .git/objects other than by using Git, and if he or she does,
then asking a chmod prior to an rm seems reasonable.
If you can't, then it's another problematic use-case (basically, you
can't just "rm -fr" a whole clone), and then it deserves at least an
opt-in configuration to get writable pack files.
(Unfortunately, I suspect we're in the later case)
If you insist on setting the tmp idx & pack file permission to 0444 at
least give it a u+w permission whenever you try to unlink and rename
it so it won’t fail.
Yes. In case you hadn't guessed, this is precisely what I had in mind
when I asked "Is it a problem when using Git [...] or when trying to
remove files outside Git?".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
On Feb 17, 2015, at 4:51 PM, Matthieu Moy [off-list ref] wrote:
Fairuzan Roslan [off-list ref] writes:
quoted
$ git clone https://github.com/robbyrussell/oh-my-zsh.git
Cloning into 'oh-my-zsh'...
remote: Counting objects: 11830, done.
remote: Total 11830 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.
Resolving deltas: 100% (6510/6510), done.
warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
This should be fixable from Git itself, by replacing the calls to
"unlink" with something like
int unlink_or_chmod(...) {
if (unlink(...)) {
chmod(...); // give user write permission
return unlink(...);
}
}
This does not add extra cost in the normal case, and would fix this
particular issue for afp shares. So, I think that would fix the biggest
problem for afp-share users without disturbing others. It seems
reasonable to me to do that unconditionnally.
quoted
$ rm -rf oh-my-zsh/.git/objects/pack/tmp_*
rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted
rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
What happens if you do "rm -fr oh-my-zsh/.git/objects/pack/" (i.e.
remove the directory, not the files)?
If you can still remove the directory, then I'd say the solution above
could be sufficient: the user isn't supposed to interfer with the
content of .git/objects other than by using Git, and if he or she does,
then asking a chmod prior to an rm seems reasonable.
If you can't, then it's another problematic use-case (basically, you
can't just "rm -fr" a whole clone), and then it deserves at least an
opt-in configuration to get writable pack files.
(Unfortunately, I suspect we're in the later case)
quoted
If you insist on setting the tmp idx & pack file permission to 0444 at
least give it a u+w permission whenever you try to unlink and rename
it so it won’t fail.
Yes. In case you hadn't guessed, this is precisely what I had in mind
when I asked "Is it a problem when using Git [...] or when trying to
remove files outside Git?".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Yes. It’s a problem when using Git where it fails to unlink and rename the tmp idx and pack files.
The reason I tries to rm -rf the tmp_idx_XXXXXX and tmp_pack_XXXXXX is to proof a point why Git fails
Perhaps my explanation wasn’t clear enough. Maybe it’s hard for you to understand without having to test it yourself on a AFP filesystem.
Let me explain why AFP filesystem is more strict and different from your typical filesystem like ext4,hfs+,etc.
$ mkdir testdir; chmod 0755 testdir; touch testdir/testfile; chmod 0444 testdir/testfile; ls -la testdir
total 0
drwxr-xr-x 1 user staff 264 Feb 18 00:26 .
drwx------ 1 user staff 264 Feb 18 00:26 ..
-r--r--r-- 1 user staff 0 Feb 18 00:26 testfile
$ rm -rf testdir
rm: testdir/testfile: Operation not permitted
rm: testdir: Directory not empty
$ chmod +w testdir/testfile; ls -la testdir
total 0
drwxr-xr-x 1 riaf staff 264 Feb 18 00:26 .
drwx------ 1 riaf staff 264 Feb 18 00:26 ..
-rw-r—r-- 1 riaf staff 0 Feb 18 00:26 testfile
$ rm -rf testdir <—— No error message
This show that you cannot delete a directory or a file without a write permission in AFP filesystem.
The problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp.
Try google for “Git AFP” and you will see a lot people are facing with the same problem.
Regarding your suggestion, yes I think it would work but you have to take care (chmod) every calls that rename or unlink or delete files with 0444 permission.
Regards,
Fairuzan
On 17/02/15 17:58, Fairuzan Roslan wrote:
quoted
On Feb 17, 2015, at 4:51 PM, Matthieu Moy [off-list ref] wrote:
Fairuzan Roslan [off-list ref] writes:
quoted
$ git clone https://github.com/robbyrussell/oh-my-zsh.git
Cloning into 'oh-my-zsh'...
remote: Counting objects: 11830, done.
remote: Total 11830 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.
Resolving deltas: 100% (6510/6510), done.
warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
This should be fixable from Git itself, by replacing the calls to
"unlink" with something like
int unlink_or_chmod(...) {
if (unlink(...)) {
chmod(...); // give user write permission
return unlink(...);
}
}
This does not add extra cost in the normal case, and would fix this
particular issue for afp shares. So, I think that would fix the biggest
problem for afp-share users without disturbing others. It seems
reasonable to me to do that unconditionnally.
quoted
$ rm -rf oh-my-zsh/.git/objects/pack/tmp_*
rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted
rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
What happens if you do "rm -fr oh-my-zsh/.git/objects/pack/" (i.e.
remove the directory, not the files)?
If you can still remove the directory, then I'd say the solution above
could be sufficient: the user isn't supposed to interfer with the
content of .git/objects other than by using Git, and if he or she does,
then asking a chmod prior to an rm seems reasonable.
If you can't, then it's another problematic use-case (basically, you
can't just "rm -fr" a whole clone), and then it deserves at least an
opt-in configuration to get writable pack files.
(Unfortunately, I suspect we're in the later case)
quoted
If you insist on setting the tmp idx & pack file permission to 0444 at
least give it a u+w permission whenever you try to unlink and rename
it so it won’t fail.
Yes. In case you hadn't guessed, this is precisely what I had in mind
when I asked "Is it a problem when using Git [...] or when trying to
remove files outside Git?".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Yes. It’s a problem when using Git where it fails to unlink and rename the tmp idx and pack files.
The reason I tries to rm -rf the tmp_idx_XXXXXX and tmp_pack_XXXXXX is to proof a point why Git fails
Perhaps my explanation wasn’t clear enough. Maybe it’s hard for you to understand without having to test it yourself on a AFP filesystem.
Let me explain why AFP filesystem is more strict and different from your typical filesystem like ext4,hfs+,etc.
$ mkdir testdir; chmod 0755 testdir; touch testdir/testfile; chmod 0444 testdir/testfile; ls -la testdir
total 0
drwxr-xr-x 1 user staff 264 Feb 18 00:26 .
drwx------ 1 user staff 264 Feb 18 00:26 ..
-r--r--r-- 1 user staff 0 Feb 18 00:26 testfile
$ rm -rf testdir
rm: testdir/testfile: Operation not permitted
rm: testdir: Directory not empty
This works on my system (Mac OS 10.9 as server and client)
$ chmod +w testdir/testfile; ls -la testdir
total 0
drwxr-xr-x 1 riaf staff 264 Feb 18 00:26 .
drwx------ 1 riaf staff 264 Feb 18 00:26 ..
-rw-r—r-- 1 riaf staff 0 Feb 18 00:26 testfile
$ rm -rf testdir <—— No error message
This show that you cannot delete a directory or a file without a write permission in AFP filesystem.
The problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp.
Try google for “Git AFP” and you will see a lot people are facing with the same problem.
Yes, (at least to my knowledge) you seem to be one of the first to report it here, thanks for that.
Regarding your suggestion, yes I think it would work but you have to take care (chmod) every calls that rename or unlink or delete files with 0444 permission.
Regards,
Fairuzan
The "right" solution is to make a wrapper function, and to re-define unlink() and rename() with help
of the preprocessor.
git-compat-util.h has an example for fopen(), so that can be used for a patch.
And no, as I can not reproduce it here, I can only help with reviews or so.
On Tue, Feb 17, 2015 at 09:51:38AM +0100, Matthieu Moy wrote:
This should be fixable from Git itself, by replacing the calls to
"unlink" with something like
int unlink_or_chmod(...) {
if (unlink(...)) {
chmod(...); // give user write permission
return unlink(...);
}
}
This does not add extra cost in the normal case, and would fix this
particular issue for afp shares. So, I think that would fix the biggest
problem for afp-share users without disturbing others. It seems
reasonable to me to do that unconditionnally.
This can have security issues if you're trying to unlink a symlink, as
chmod will dereference the symlink but unlink will not. Giving the file
owner write permission may not be sufficient, as the user may be a
member of a group with write access to the repo. A malicious user who
also has access to the repo could force the current user to chmod an
arbitrary file such that it had looser permissions.
I've seen a case where Perl's ExtUtils::MakeMaker chmoded
/etc/mime.types 0666 as a result of this.
I don't think there's a secure way to implement this unless you're on an
OS with lchmod or fchmodat that supports AT_SYMLINK_NOFOLLOW. Linux is
not one of those systems.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187