From 9f733a0804f1b4f0b34ed71c217201596c7e1d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 7 Jul 2020 14:07:04 +0200 Subject: [PATCH 1/2] Fix permission handling on project generation from template files --- scrapy/commands/startproject.py | 35 ++----- tests/test_commands.py | 170 ++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 25 deletions(-) diff --git a/scrapy/commands/startproject.py b/scrapy/commands/startproject.py index 852281959..eccc2a3e1 100644 --- a/scrapy/commands/startproject.py +++ b/scrapy/commands/startproject.py @@ -1,10 +1,10 @@ import re import os -import stat import string from importlib import import_module from os.path import join, exists, abspath from shutil import ignore_patterns, move, copy2, copystat +from stat import S_IWUSR as OWNER_WRITE_PERMISSION import scrapy from scrapy.commands import ScrapyCommand @@ -20,7 +20,12 @@ TEMPLATES_TO_RENDER = ( ('${project_name}', 'middlewares.py.tmpl'), ) -IGNORE = ignore_patterns('*.pyc', '.svn') +IGNORE = ignore_patterns('*.pyc', '__pycache__', '.svn') + + +def _make_writable(path): + current_permissions = os.stat(path).st_mode + os.chmod(path, current_permissions | OWNER_WRITE_PERMISSION) class Command(ScrapyCommand): @@ -78,30 +83,10 @@ class Command(ScrapyCommand): self._copytree(srcname, dstname) else: copy2(srcname, dstname) + _make_writable(dstname) + copystat(src, dst) - self._set_rw_permissions(dst) - - def _set_rw_permissions(self, path): - """ - Sets permissions of a directory tree to +rw and +rwx for folders. - This is necessary if the start template files come without write - permissions. - """ - mode_rw = (stat.S_IRUSR - | stat.S_IWUSR - | stat.S_IRGRP - | stat.S_IROTH) - - mode_x = (stat.S_IXUSR - | stat.S_IXGRP - | stat.S_IXOTH) - - os.chmod(path, mode_rw | mode_x) - for root, dirs, files in os.walk(path): - for dir in dirs: - os.chmod(join(root, dir), mode_rw | mode_x) - for file in files: - os.chmod(join(root, file), mode_rw) + _make_writable(dst) def run(self, args, opts): if len(args) not in (1, 2): diff --git a/tests/test_commands.py b/tests/test_commands.py index 24a341759..002237824 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,11 +2,14 @@ import inspect import json import optparse import os +from stat import S_IWRITE as ANYONE_WRITE_PERMISSION import subprocess import sys import tempfile from contextlib import contextmanager +from itertools import chain from os.path import exists, join, abspath +from pathlib import Path from shutil import rmtree, copytree from tempfile import mkdtemp from threading import Timer @@ -15,6 +18,7 @@ from twisted.trial import unittest import scrapy from scrapy.commands import ScrapyCommand +from scrapy.commands.startproject import IGNORE from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import get_testenv @@ -119,6 +123,29 @@ class StartprojectTest(ProjectTest): self.assertEqual(2, self.call('startproject', self.project_name, project_dir, 'another_params')) +def get_permissions_dict(path, renamings=None, ignore=None): + renamings = renamings or tuple() + permissions_dict = { + '.': os.stat(path).st_mode, + } + for root, dirs, files in os.walk(path): + nodes = list(chain(dirs, files)) + if ignore: + ignored_names = ignore(root, nodes) + nodes = [node for node in nodes if node not in ignored_names] + for node in nodes: + absolute_path = os.path.join(root, node) + relative_path = os.path.relpath(absolute_path, path) + for search_string, replacement in renamings: + relative_path = relative_path.replace( + search_string, + replacement + ) + permissions = os.stat(absolute_path).st_mode + permissions_dict[relative_path] = permissions + return permissions_dict + + class StartprojectTemplatesTest(ProjectTest): def setUp(self): @@ -139,6 +166,149 @@ class StartprojectTemplatesTest(ProjectTest): self.assertIn(self.tmpl_proj, out) assert exists(join(self.proj_path, 'root_template')) + def test_startproject_permissions_from_writable(self): + """Check that generated files have the right permissions when the + template folder has the same permissions as in the project, i.e. + everything is writable.""" + scrapy_path = scrapy.__path__[0] + project_template = os.path.join(scrapy_path, 'templates', 'project') + project_name = 'startproject1' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + project_template, + renamings, + IGNORE, + ) + + destination = mkdtemp() + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + ), + cwd=destination, + env=self.env, + ) + process.wait() + + project_dir = os.path.join(destination, project_name) + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + + def test_startproject_permissions_from_read_only(self): + """Check that generated files have the right permissions when the + template folder has been made read-only, which is something that some + systems do. + + See https://github.com/scrapy/scrapy/pull/4604 + """ + scrapy_path = scrapy.__path__[0] + templates_dir = os.path.join(scrapy_path, 'templates') + project_template = os.path.join(templates_dir, 'project') + project_name = 'startproject2' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + project_template, + renamings, + IGNORE, + ) + + def _make_read_only(path): + current_permissions = os.stat(path).st_mode + os.chmod(path, current_permissions & ~ANYONE_WRITE_PERMISSION) + + read_only_templates_dir = str(Path(mkdtemp()) / 'templates') + copytree(templates_dir, read_only_templates_dir) + + for root, dirs, files in os.walk(read_only_templates_dir): + for node in chain(dirs, files): + _make_read_only(os.path.join(root, node)) + + destination = mkdtemp() + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + '--set', + 'TEMPLATES_DIR={}'.format(read_only_templates_dir), + ), + cwd=destination, + env=self.env, + ) + process.wait() + + project_dir = os.path.join(destination, project_name) + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + + def test_startproject_permissions_unchanged_in_destination(self): + """Check that pre-existing folders and files in the destination folder + do not see their permissions modified.""" + scrapy_path = scrapy.__path__[0] + project_template = os.path.join(scrapy_path, 'templates', 'project') + project_name = 'startproject3' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + project_template, + renamings, + IGNORE, + ) + + destination = mkdtemp() + project_dir = os.path.join(destination, project_name) + + existing_nodes = { + oct(permissions)[2:] + extension: permissions + for extension in ('', '.d') + for permissions in ( + 0o444, 0o555, 0o644, 0o666, 0o755, 0o777, + ) + } + os.mkdir(project_dir) + project_dir_path = Path(project_dir) + for node, permissions in existing_nodes.items(): + path = project_dir_path / node + if node.endswith('.d'): + path.mkdir(mode=permissions) + else: + path.touch(mode=permissions) + expected_permissions[node] = path.stat().st_mode + + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + '.', + ), + cwd=project_dir, + env=self.env, + ) + process.wait() + + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + class CommandTest(ProjectTest): From c3bbf2bd23f25b4916f8899b67d3abd5a3199f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jul 2020 12:15:27 +0200 Subject: [PATCH 2/2] Cover 2.2.1 in the release notes --- docs/news.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index 80d130e4a..fd507c3cc 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,16 @@ Release notes ============= +.. _release-2.2.1: + +Scrapy 2.2.1 (2020-07-17) +------------------------- + +* The :command:`startproject` command no longer makes unintended changes to + the permissions of files in the destination folder, such as removing + execution permissions (:issue:`4662`, :issue:`4666`) + + .. _release-2.2.0: Scrapy 2.2.0 (2020-06-24)