diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 43f4c36526e..73ca87de161 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -75,7 +75,8 @@ class BuildCommand(BuildCommandResultMixin): :py:class:`readthedocs.builds.models.BuildCommandResult` model. :param command: string or array of command parameters - :param cwd: current working path for the command + :param cwd: Absolute path used as the current working path for the command. + Defaults to ``RTD_DOCKER_WORKDIR``. :param shell: execute command in shell, default=False :param environment: environment variables to add to environment :type environment: dict @@ -102,7 +103,7 @@ def __init__( ): self.command = command self.shell = shell - self.cwd = cwd or '$HOME' + self.cwd = cwd or settings.RTD_DOCKER_WORKDIR self.user = user or settings.RTD_DOCKER_USER self.environment = environment.copy() if environment else {} if 'PATH' in self.environment: @@ -152,9 +153,7 @@ def run(self): proc = subprocess.Popen( command, shell=self.shell, - # This is done here for local builds, but not for docker, - # as we want docker to expand inside the container - cwd=os.path.expandvars(self.cwd), + cwd=self.cwd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -271,6 +270,11 @@ class DockerBuildCommand(BuildCommand): Build command to execute in docker container """ + bash_escape_re = re.compile( + r"([\t\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@" + r'\[\\\]\^\`\{\|\}\~])' + ) + def __init__(self, *args, escape_command=True, **kwargs): """ Override default to extend behavior. @@ -300,6 +304,7 @@ def run(self): cmd=self.get_wrapped_command(), environment=self.environment, user=self.user, + workdir=self.cwd, stdout=True, stderr=True, ) @@ -345,28 +350,28 @@ def get_wrapped_command(self): ``escape_command=True`` in the init method this escapes a good majority of those characters. """ - bash_escape_re = re.compile( - r"([\t\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@" - r'\[\\\]\^\`\{\|\}\~])', - ) prefix = '' if self.bin_path: - prefix += 'PATH={}:$PATH '.format(self.bin_path) + bin_path = self._escape_command(self.bin_path) + prefix += f'PATH={bin_path}:$PATH ' command = ( - ' '.join([ - bash_escape_re.sub(r'\\\1', part) if self.escape_command else part + ' '.join( + self._escape_command(part) if self.escape_command else part for part in self.command - ]) + ) ) return ( - "/bin/sh -c 'cd {cwd} && {prefix}{cmd}'".format( - cwd=self.cwd, + "/bin/sh -c '{prefix}{cmd}'".format( prefix=prefix, cmd=command, ) ) + def _escape_command(self, cmd): + r"""Escape the command by prefixing suspicious chars with `\`.""" + return self.bash_escape_re.sub(r'\\\1', cmd) + class BaseEnvironment: diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 196196a2e19..5a84f524f9a 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -323,7 +323,7 @@ def setup_base(self): # Don't use virtualenv bin that doesn't exist yet bin_path=None, # Don't use the project's root, some config files can interfere - cwd='$HOME', + cwd=None, ) def install_core_requirements(self): diff --git a/readthedocs/rtd_tests/tests/test_doc_building.py b/readthedocs/rtd_tests/tests/test_doc_building.py index c6938a95d00..5f8647d1f49 100644 --- a/readthedocs/rtd_tests/tests/test_doc_building.py +++ b/readthedocs/rtd_tests/tests/test_doc_building.py @@ -13,7 +13,7 @@ from unittest.mock import Mock, PropertyMock, mock_open, patch import pytest -from django.test import TestCase +from django.test import TestCase, override_settings from django_dynamic_fixture import get from docker.errors import APIError as DockerAPIError from docker.errors import DockerException @@ -356,6 +356,7 @@ def test_failing_execution_with_unexpected_exception(self): }) +@override_settings(RTD_DOCKER_WORKDIR='/tmp/') class TestDockerBuildEnvironment(TestCase): """Test docker build environment.""" @@ -699,7 +700,8 @@ def test_command_execution(self): self.mocks.docker_client.exec_create.assert_called_with( container='build-123-project-6-pip', - cmd="/bin/sh -c 'cd /tmp && echo\\ test'", + cmd="/bin/sh -c 'echo\\ test'", + workdir='/tmp', environment=mock.ANY, user='docs:docs', stderr=True, @@ -759,7 +761,8 @@ def test_command_not_recorded(self): self.mocks.docker_client.exec_create.assert_called_with( container='build-123-project-6-pip', - cmd="/bin/sh -c 'cd /tmp && echo\\ test'", + cmd="/bin/sh -c 'echo\\ test'", + workdir='/tmp', environment=mock.ANY, user='docs:docs', stderr=True, @@ -806,7 +809,8 @@ def test_record_command_as_success(self): self.mocks.docker_client.exec_create.assert_called_with( container='build-123-project-6-pip', - cmd="/bin/sh -c 'cd /tmp && echo\\ test'", + cmd="/bin/sh -c 'echo\\ test'", + workdir='/tmp', environment=mock.ANY, user='docs:docs', stderr=True, @@ -1010,6 +1014,7 @@ def test_container_timeout(self): }) +@override_settings(RTD_DOCKER_WORKDIR='/tmp/') class TestBuildCommand(TestCase): """Test build command creation.""" @@ -1090,6 +1095,7 @@ def test_unicode_output(self, mock_subprocess): ) +@override_settings(RTD_DOCKER_WORKDIR='/tmp/') class TestDockerBuildCommand(TestCase): """Test docker build commands.""" @@ -1109,7 +1115,7 @@ def test_wrapped_command(self): ) self.assertEqual( cmd.get_wrapped_command(), - "/bin/sh -c 'cd /tmp/foobar && pip install requests'", + "/bin/sh -c 'pip install requests'", ) cmd = DockerBuildCommand( ['python', '/tmp/foo/pip', 'install', 'Django>1.7'], @@ -1120,7 +1126,7 @@ def test_wrapped_command(self): cmd.get_wrapped_command(), ( '/bin/sh -c ' - "'cd /tmp/foobar && PATH=/tmp/foo:$PATH " + "'PATH=/tmp/foo:$PATH " r"python /tmp/foo/pip install Django\>1.7'" ), ) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 66069e26bde..b2b70680194 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -441,6 +441,7 @@ def TEMPLATES(self): # https://docs.docker.com/engine/reference/run/#user RTD_DOCKER_USER = 'docs:docs' RTD_DOCKER_SUPER_USER = 'root:root' + RTD_DOCKER_WORKDIR = '/home/docs/' RTD_DOCKER_COMPOSE = False