remove write permission after installation

[sbailey]

This PR updates desiInstall so that it removes write permission after an installation to protect against accidental changes later. It purposefully removes write permission even from the installing user (e.g. desi).

Previously desiInstall would run fix_permissions.sh which leaves write-permission alone, leaving us in a state where any desi collaborator could accidentally delete a code installation.

I'd like to get this (or equivalent) into a new tag to include in the 18.9 release so that future installations will be safe from accidental overwrite.

changes.rst still needs to be updated, after this has a PR number and @weaverba137 approves of the approach implemented here.

[weaverba137]

I haven't looked closely at this yet, but I had a thought. Don't at least some of our tests at NERSC run the test suite of various packages, e.g., before running a full integration test? Some of our unit tests write to the install directory, so if the owner doesn't have write permission, how will that work?

[sbailey]

The nightly unit tests are run on git checkouts that were not installed by desiInstall. We haven't been running unit tests on installed packages, but you are correct that that wouldn't work if we don't allow even the desi user to have write access. I believe it would work if we allowed desi-user write access at the top level directory but not the bin and lib subdirectories.

The most important change here is to remove group write access. I'd prefer to also remove desi-user write access, but can be flexible on that if it causes more problems than it fixes (we've had several cases of users accidentally changing an installation; I'm not aware of any cases of the desi-user doing that).

[weaverba137]

The code looks fine as is, although we should do a full test at NERSC before merging.

[weaverba137]

I have a suggestion: I think the default practice has been to install git checkouts by hand, but desiInstall is also capable of installing git checkouts. Let's only remove user write access for tag installs, not master/trunk/branch installs.

Also, have you already manually changed permissions on existing installations?

[sbailey]

I have a suggestion: I think the default practice has been to install git checkouts by hand, but desiInstall is also capable of installing git checkouts. Let's only remove user write access for tag installs, not master/trunk/branch installs.

Please go ahead and make that change; I'm not familiar enough with desiInstall to know how to tell the difference at the time the files are being written.

Also, have you already manually changed permissions on existing installations?

I manually removed group write permission from the master git checkouts a few weeks ago and I think I also did that for the tagged installations too.

/global/common/software/desi/cori/desiconda/20180709-1.2.6-spec/code/desisurvey/0.10.3 is an example of a product that was installed after that and still has group write. /global/common/software/desi/cori/desiconda/20180709-1.2.6-spec/code/desisurvey/0.10.4 is an example of an installation from yesterday using this branch to demonstrate the updated permissions without write access.

[weaverba137]

I've updated the code to allow user-write on master installs. I can go ahead and update the permissions on existing installs as needed. I assume it's sufficient to just fix software installed in desiconda/20180709-1.2.6-spec, correct?

I will also check the permissions on the module files.

[weaverba137]

I just ran my own tests of desiInstall. I also went ahead and removed write permission on all desiInstalled code in desiconda/20180709-1.2.6-spec on both cori and edison. I think this is ready to merge unless you have some final comments.