Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Production installer fails with umask 077 #2373

Closed
andersk opened this issue Nov 20, 2016 · 5 comments
Closed

Production installer fails with umask 077 #2373

andersk opened this issue Nov 20, 2016 · 5 comments

Comments

@andersk
Copy link
Member

andersk commented Nov 20, 2016

Running the production installer under umask 077 fails with permission errors, because, e.g., /srv/zulip-venv-cache becomes readable only by root. It should probably umask 022 near the top.

@timabbott
Copy link
Sponsor Member

Sounds good; I think we just need to add that line near the top of scripts/lib/install and then test it to make sure we did it right :)

@Shashankagarwal181996
Copy link
Collaborator

@timabbott I would like to work on this.

@timabbott
Copy link
Sponsor Member

Cool. Basically you'll just want to do a series of production installs on a VM, using build-release-tarball to generate the releases to install. First reproduce the failure; then add umask 022 at the top of scripts/lib/install, and see if that makes the installation work. That sort of careful testing procedure is essential, since this is the kind of change that is likely to fail in some subtle way the first time :)

@feorlen
Copy link
Collaborator

feorlen commented Mar 23, 2017

A quick test looks like that solves the problem. I'll do a more thorough one creating tarballs to confirm.

Without any changes, using umask 077 I get this error on a new install:

root@minmi:~# su zulip -c /home/zulip/deployments/current/scripts/setup/initialize-database
++ dirname /home/zulip/deployments/current/scripts/setup/initialize-database
+ cd /home/zulip/deployments/current/scripts/setup/../..
+ ./manage.py checkconfig
Traceback (most recent call last):
  File "./manage.py", line 18, in <module>
    from django.conf import settings
ImportError: No module named django.conf

@timabbott
Copy link
Sponsor Member

Cool! Just submit a PR with the change once you've confirmed. Yay, it'll be nice to close this out.

feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 18, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading.

From experimentation, it seems `create-production-venv` is the only part that needs
this. If that's not correct, then any other `subprocess.check_call` can have the same
preexec_fn added. The permissions do propagate down child subprocesses, but in this
case we can't take advantage of that and use it in `lib/upgrade-zulip`, because that
isn't updated before being used like `upgrade-zulip-stage-2` is.

If in the future it matters, preexec_fn doesn't work for Windows. Also, the lambda is
because normally it won't accept a function with arguments.

I've tested this by starting from a clean install, deleting /srv/* so new files are
downloaded, and then doing an upgrade. It would be nice if someone else can also test
this in a production environment to verify I've not missed anything.
feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 18, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading.

From experimentation, it seems `create-production-venv` is the only part that
needs this. If that's not correct, then any other `subprocess.check_call` can have
the same preexec_fn added. The permissions do propagate down child
subprocesses, but in this case we can't take advantage of that and use it in
`lib/upgrade-zulip`, because that isn't updated before being used like
`upgrade-zulip-stage-2` is.

If in the future it matters, preexec_fn doesn't work for Windows. Also, the lambda
is because normally it won't accept a function with arguments.

I've tested this by starting from a clean install, deleting /srv/* so new files are
downloaded, and then doing an upgrade. It would be nice if someone else can
also test this in a production environment to verify I've not missed anything.
feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 18, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading.

From experimentation, it seems `create-production-venv` is the only part that
needs this. If that's not correct, then any other `subprocess.check_call` can
have the same preexec_fn added. The permissions do propagate down child
subprocesses, but in this case we can't take advantage of that and use it in
`lib/upgrade-zulip`, because that isn't updated before being used like
`upgrade-zulip-stage-2` is.

If in the future it matters, preexec_fn doesn't work for Windows. Also, the
lambda is because normally it won't accept a function with arguments.

I've tested this by starting from a clean install, deleting /srv/* so new files
are downloaded, and then doing an upgrade. It would be nice if someone else can
also test this in a production environment to verify I've not missed anything.
feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 18, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading.

From experimentation, it seems create-production-venv is the only part that
needs this. If that's not correct, then any other subprocess.check_call can
have the same preexec_fn added. The permissions do propagate down child
subprocesses, but in this case we can't take advantage of that and use it in
lib/upgrade-zulip, because that isn't updated before being used like
upgrade-zulip-stage-2 is.

If in the future it matters, preexec_fn doesn't work for Windows. Also, the
lambda is because normally it won't accept a function with arguments.

I've tested this by starting from a clean install, deleting /srv/* so new files
are downloaded, and then doing an upgrade. It would be nice if someone else can
also test this in a production environment to verify I've not missed anything.
feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 19, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading.

From experimentation, it seems create-production-venv is the only part that
needs this. If that's not correct, then any other subprocess.check_call can
have the same preexec_fn added. The permissions do propagate down child
subprocesses, but in this case we can't take advantage of that and use it in
lib/upgrade-zulip, because that isn't updated before being used like
upgrade-zulip-stage-2 is.

If in the future it matters, preexec_fn doesn't work for Windows. Also, the
lambda is because normally it won't accept a function with arguments.

I've tested this by starting from a clean install, deleting /srv/* so new files
are downloaded, and then doing an upgrade. It would be nice if someone else can
also test this in a production environment to verify I've not missed anything.
feorlen pushed a commit to feorlen/zulip that referenced this issue Apr 19, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading so files have appropriate permissions.

I've tested this by starting from a clean install, deleting /srv/* so new
files are downloaded, and then doing an upgrade. It worked starting with both
a current version from master and an older release installed with a less
restrictive umask and then the umask changed.

Fixes zulip#2373
brockwhittaker pushed a commit to brockwhittaker/zulip that referenced this issue May 1, 2017
Follow-on from zulip#2373/ PR zulip#4316, to set an
appropriate umask also when upgrading so files have appropriate permissions.

I've tested this by starting from a clean install, deleting /srv/* so new
files are downloaded, and then doing an upgrade. It worked starting with both
a current version from master and an older release installed with a less
restrictive umask and then the umask changed.

Fixes zulip#2373.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants