[Django] #32223: Path.resolve(strict=True) can raise a PermissionError in autoreloader.
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz | Owner: nobody
Felisiak |
Type: Bug | Status: new
Component: Core | Version: 3.1
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Using `Path.resolve(strict=True)` can cause permission errors when users
don't have permissions to all intermediate directories in a Django
installation path, see:
{{{
Traceback (most recent call last):
File "manage.py", line 21, in <module>
main()
File "manage.py", line 17, in main
execute_from_command_line(sys.argv)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/__init__.py", line 401, in
execute_from_command_line
utility.execute()
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/__init__.py", line 395, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/base.py", line 328, in run_from_argv
self.execute(*args, **cmd_options)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/commands/runserver.py", line 60, in
execute
super().execute(*args, **options)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/base.py", line 369, in execute
output = self.handle(*args, **options)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/commands/runserver.py", line 95, in handle
self.run(**options)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/core/management/commands/runserver.py", line 102, in run
autoreload.run_with_reloader(self.inner_run, **options)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 599, in run_with_reloader
start_django(reloader, main_func, *args, **kwargs)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 584, in start_django
reloader.run(django_main_thread)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 299, in run
self.run_loop()
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 305, in run_loop
next(ticker)
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 345, in tick
for filepath, mtime in self.snapshot_files():
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 361, in snapshot_files
for file in self.watched_files():
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 260, in watched_files
yield from iter_all_python_module_files()
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 105, in
iter_all_python_module_files
return iter_modules_and_files(modules, frozenset(_error_files))
File "/usr/home/XXX/.virtualenvs/go/lib/python3.8/site-
packages/django/utils/autoreload.py", line 141, in iter_modules_and_files
resolved_path = path.resolve(strict=True).absolute()
File "/usr/local/lib/python3.8/pathlib.py", line 1180, in resolve
s = self._flavour.resolve(self, strict=strict)
File "/usr/local/lib/python3.8/pathlib.py", line 362, in resolve
return _resolve(base, str(path)) or sep
File "/usr/local/lib/python3.8/pathlib.py", line 346, in _resolve
target = accessor.readlink(newpath)
File "/usr/local/lib/python3.8/pathlib.py", line 451, in readlink
return os.readlink(path)
PermissionError: [Errno 13] Permission denied: '/usr'
}}}
Regression in e28671187903e6aca2428374fdd504fca3032aee (Django 3.0).
Thanks Jakub Szafrański for the
[https://code.djangoproject.com/ticket/31912#comment:15 report].
--
Ticket URL: <https://code.djangoproject.com/ticket/32223>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Mariusz Felisiak
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13713 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32223#comment:1>
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
I think we need more clarity on what is going on here. The report in the
other ticket says:
> Just to explain a little, this configuration is pretty common in shared-
system environments - it is intentional and prevents users from
enumerating local users on the system and accessing their home directories
in case of miss-configured permissions.
But to see `PermissionError` like this it implies that `/usr` is not
executable which ought to mean it cannot be traversed in which case there
are surely larger problems? Also `/usr/home/…` is highly irregular.
Is this using `ugidfw` again? If it breaks the expectations around how as
permissions work, or is configured incorrectly, then something needs
fixing elsewhere, not in Django. I noticed this too:
https://bugs.python.org/issue41593
I also see that we're using the undocumented `pathlib.Path.absolute()`
which is not recommended and may even go away.
`pathlib.Path.resolve(strict=True) is the documented and recommended way
to do this.
--
Ticket URL: <https://code.djangoproject.com/ticket/32223#comment:2>
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
With the following folder structure and permissions:
{{{
drwxr-x--x root root /broken
drwxr-x--x root root /broken/home
drwx------ user users /broken/home/user
drwx------ user users /broken/home/user/package
drwx------ user users /broken/home/user/package/thing
}}}
Running the following works fine:
{{{#!python
>>> import os
>>> import pathlib
>>> os.chdir('/broken/home/user')
>>> pathlib.Path('package/thing')
PosixPath('package/thing')
>>> pathlib.Path('package/thing').resolve(strict=True)
PosixPath('/broken/home/user/package/thing')
}}}
If I then do `sudo chmod o-x /broken` and run the following in my open
REPL:
{{{#!python
>>> pathlib.Path('package/thing').resolve(strict=True)
...
PermissionError: [Errno 13] Permission denied: '/broken/home/user/package'
}}}
But then the following would also be a problem because without the
executable bit the directory cannot be traversed:
{{{#!python
>>> os.chdir('/broken/home/user')
...
PermissionError: [Errno 13] Permission denied: '/broken/home/user'
}}}
So to summarise:
- As far as I can tell, the issue must be with `ugidfw` -- mentioned in
the original report -- which looks to be FreeBSD-specific.
- There is implication that it is difficult to configure with few
examples, e.g.
[https://twitter.com/thedarktangent/status/1179976664018632704 here].
- We don't explicitly test Django on FreeBSD. Is it considered supported?
(I know this is a contentious point, but it should be asked.)
- It won't stop here. We'll have to remove all uses of
`.resolve(strict=True)` which may mean removing all use of `pathlib`.
Some other quick fire questions:
- What hosting platform is the original bug reporter using?
- Do they work for that company?
- If so, have they configured `ugidfw` properly? (Not trying to be
obnoxious, but this is a serious point. Many more things will be broken if
`readlink` doesn't work.)
I believe this is a wontfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/32223#comment:3>
Django
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32223#comment:4>
Django
-------------------------------------+-------------------------------------
| Felisiak
Component: Core (Management | Version: 3.1
commands) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"aade2b461acafd16cfc82449b3df88a0f1c1c197" aade2b46]:
{{{
#!CommitTicketReference repository=""
revision="aade2b461acafd16cfc82449b3df88a0f1c1c197"
Fixed #32223 -- Removed strict=True in Path.resolve() in autoreloader.
This reverts commit e28671187903e6aca2428374fdd504fca3032aee which
caused permission errors when users didn't have permissions to all
intermediate directories in a Django installation path.
Thanks Jakub Szafrański for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32223#comment:5>