startproject should honor umask

[talex5+django@…]

Ticket #1651 fixed the problem of manage.py not being made executable by copying *all* the permission bits (not just the executable flags). This means that the user's umask doesn't work, e.g.:

$ umask 077
$ touch foo
$ ls -l foo
-rw------- 1 talex talex 0 2007-05-12 13:27 foo

$ PYTHONPATH=trunk ./trunk/django/bin/django-admin.py startproject mysite
$ ls -l mysite/settings.py 
-rw-r--r-- 1 talex talex 2804 2007-05-12 13:28 mysite/settings.py

I discovered this whilst trying to make a Zero Install package for Django. Everything in the Zero Install cache is read-only, so startproject fails with:

  File "/var/cache/0install.net/implementations/sha1new=262c95b5a7cc34f525408b675106e4e4ae3494cc/django/core/management.py", line 799, in startproject
    fp = open(main_settings_file, 'w')
IOError: [Errno 13] Permission denied: '.../site/settings.py'

Thanks,

[Adrian Holovaty]

Seems like a good improvement to me. A patch would be great.

[talex5+django@…]

Fix.

[anonymous]

Any comments on this patch? Is it OK?

[Fredrik Lundh <fredrik@…>]

The patch doesn't apply any more (the relevant code is now in django/core/management/base.py). I've adapted the patch, but I'm not sure how this should interact with the _make_writeable call. I'll upload the updated patch anyway, just in case.

[Fredrik Lundh <fredrik@…>]

Updated patch, against revision 6281.

[Adam Nelson]

Patch needs to be updated again.

[Peter Baumgartner]

Confirmed issue still exists in r16741. This patch applies cleanly and fixes the issue.

[merb]

I uploaded & updated the pull request. ​https://github.com/django/django/pull/2061
would be great if somebody will making suggestions.

[Claude Paroz]

Comment added on the pull request.

[Ad Timmering]

Browsing some ancient tickets, wondering if this ticket can perhaps be closed as obsolete? If not - what is still missing?

Ticket isn't very clear on what the issue is and how to reproduce is, but a lot has changed on permission handling between the time of the ticket and #17042, #26494, #27628.

[Ad Timmering]

(de-assigning as not touched in 8 years)

[Carlton Gibson]

Hey Adam — thanks for looking!

Seems like the behaviour is unchanged (to 4.0b1):

% mkdir issue4282
% cd issue4282 
issue4282 % umask 077
issue4282 % touch foo 
issue4282 % ls -l foo
-rw-------  1 carlton  staff  0 Nov 16 10:06 foo
issue4282 % django-admin startproject mysite
issue4282 % ls -l mysite/mysite/settings.py 
-rw-r--r--  1 carlton  staff  3221 Nov 16 10:06 mysite/mysite/settings.py
issue4282 % django-admin --version
4.0b1

Expected behaviour is that the created files have the restricted permissions, I think.

The original patch didn't look too complex… 🤔 (We do similar in ​storage.py — I didn't look into Claude's comments on the PR there.)

It may be that if we're not going to fix this, we can mark it wontfix. (Is this something Django needs to handle that a find ... | xargs chmod wouldn't be more appropriate for?)

[Ad Timmering]

Thanks for context Claude.

I have updated the original PR - however got stuck on the fact that subprocess.run() does not support umask (at least not prior to 3.9), and just setting umask has no effect - which renders this untestable. If you (or anyone reading this) has any suggestions please lmk.

PR that does not passes the tests (because of umask):
​https://github.com/django/django/pull/15102

[Ad Timmering]

Still need to fix the PR for Windows (checking hasattr(os, 'umask') doesn't cut it), but keen to know if PR is directionally fine.

[Ad Timmering]

Modified and tests pass- but as said in previous comment, tests are skipped on Python 3.8 as it does not support setting umask in subprocess.run() so we can't test.

[Ad Timmering]

Thanks for the review Mariusz, learning from your comments. Applied the changes.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 59f47969:

Fixed #4282 -- Made startapp/startproject management commands honor umask.

Co-authored-by: Christian Schmitt <c.schmitt@…>