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 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 started a PR based on the original.

What has changed since the original is that django-admin is now called with subprocess.run(), where setting umask is only possibly since 3.9. (Discussion on introduction of umask and [lack of] alternatives at ​https://bugs.python.org/issue38417).
Would it be OK to write the tests so they are only run on >=3.9 ?

I'll try and fix the PR to reflect that first and flag it ready for review once done.

[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@…>