Distributables are created with wrong file and directory permissions

[raamdev]

The distributables built by Phing for both the Pro and the Lite versions have directory permissions set to 777 (should be 755) and files set to 666 (should be 644).

Reported in wpsharks/comet-cache#773.

[jaswrks]

I reviewed this issue and the available Phing options and I'm not seeing a solution to this problem at the moment. Actually, I wasn't aware that a ZIP could save permissions at all until I researched this. I thought that was a TAR-only feature. It does seem to be possible though, but unsupported by Phing.

We could move to using <exec command="zip... and <exec command="tar ... to try and improve this, but that might be more work than it's worth, because the current packaging routines are depending on Phing pattern-sets to include/exclude things intelligently. If we move back to CLI tools alone, we'd need to formulate a new set of patterns in addition to tweaking permissions.

Also, I don't think the zip and unzip command-line tools are readily available on OS X and many other systems. Most Nix systems are mainly focused on TAR. So using <exec command="zip... seems like it would add a new potentially problematic dependency.

[raamdev]

@jaswsinc writes...

Also, I don't think the zip and unzip command-line tools are readily available on OS X and many other systems. Most Nix systems are mainly focused on TAR. So using <exec command="zip... seems like it would add a new potentially problematic dependency.

That's not true; OS X has had those command line utilities for a long time (going back at least 5 years, AFAIK).

@jaswsinc writes...

I reviewed this issue and the available Phing options and I'm not seeing a solution to this problem at the moment.

I just did a bunch of research on this as well. Let me share a bit of what I found:

---

There's a note in the Phing API docs for ZipFileSet (which the Phing ZipTask uses):

Permissions are currently not implemented by PEAR Archive_Tar, but hopefully they will be in the future.

This StackOverflow comment says:

Zip can store Unix file permissions by using the "external attributes" field inside the ZIP header. If it is uncompressed with the right tool on a unix system, the files will keep the permissions. If it is unpacked on windows, the files will lose the permissions.

Another answer on StackOverflow explains further:

This is possible using the Info-Zip open-source Zip utilities.

Info-Zip is what is used on OS X:

$ unzip --version
UnZip 5.52 of 28 February 2005, by Info-ZIP.  Maintained by C. Spieler.

The Apache Ant ZipFileSet does support file permissions, and also has a note on the zip task page about Info-Zip:

Starting with Ant 1.5.2, <zip> can store Unix permissions inside the archive (see description of the filemode and dirmode attributes for <zipfileset>). Unfortunately there is no portable way to store these permissions. Ant uses the algorithm used by Info-Zip's implementation of the zip and unzip commands - these are the default versions of zip and unzip for many Unix and Unix-like systems.

This StackOverflow answer suggests that the solution to this problem is to use an Installer for your target OS, so that you can have the installer set the permissions appropriately after extracting the zip file.

Of course in our case, an Installer doesn't make sense, as we're simply distributing a zip file that WordPress then "installs".

---

So long story short, this does appear to be a limitation of Phing, and I suspect it is likely to remain a limitation since support for storing file permissions inside a zip file (and those permissions then being retained when the zip file is extracted) largely depends on the algorithm used by whatever unzip utility is used.

Why should we bother with fixing this?

Because file permissions are a big deal, and right now Phing generates a zip file with dangerous permissions (777 on directories and 666 on files, which allows Other and Group write access). This also makes us look bad and makes it look like we don't know what we're doing.

I received a report almost immediately after releasing Comet Cache v160521 that the permissions were wrong.

How I suggest we solve this

Since Phing can't do this for us, and since we don't want to revert to running <exec command="zip..., which complicates the Phing scripts, I suggest that we let Phing generate the zip file as it does now, but then when it's done we unzip, set file permissions, and run a single <exec command="zip... to repackage the zip file with the proper permissions. That way we're leaving the complicated file exclusion/inclusion portion to Phing, but we handle fixing the file permissions issue by running three extra commands (extract, change file permissions, zip).

[jaswrks]

That's not true; OS X has had those command line utilities for a long time (going back at least 5 years, AFAIK)

Ah. Cool. I see you are right and I was wrong. Good to know.

[jaswrks]

This StackOverflow answer suggests that the solution to this problem is to use an Installer for your target OS, so that you can have the installer set the permissions appropriately after extracting the zip file.

Yeah, that would seem logical for any web-based application that is importing plugin ZIP files. It seems like WordPress should be dealing with permissions after extraction. I'm going to run some tests just to see how it handles this. I haven't noticed this being a problem before.

Because file permissions are a big deal, and right now Phing generates a zip file with dangerous permissions (777 on directories and 666 on files, which allows Other and Group write access). This also makes us look bad and makes it look like we don't know what we're doing.

Agreed.

[jaswrks]

The Apache Ant ZipFileSet does support file permissions, and also has a note on the zip task page about Info-Zip:

I noticed that too. Too bad it's not supported in Phing.

[jaswrks]

I suggest that we let Phing generate the zip file as it does now, but then when it's done we unzip, set file permissions

Excellent idea! Love that solution.

[jaswrks]

Next Release Changelog:

• Refactored ZIP/TGZ generation. Now calling zip, unzip, and tar directly, but still using <copy toDir=""> in Phing, along with the same include/exclude patterns as before.

  The goal here was to remove our dependency on the built-in ZIP/TAR tasks in Phing, because they do not support some advanced features, such as the ability to preserve permissions in ZIP files. Fixed in this release. Props @raamdev for his research assistance and for reposting a report in Distributables are created with wrong file and directory permissions #97

  Note also that a new directory is added to .~build/.~/pkg-dirs/for-zip-tgz for review. The .zip and .tar.gz distros are based on this directory, and this directory is created using our existing ZIP/TGZ pattern sets for inclusion/exclusion.

[raamdev]

Woohoo! Nice! I confirmed this issue has been fixed as of Phings v160530. 👍