Git: don't expand envvars in Gitpython (#8263)

- Gitpython expands envvars by default
  We execute gitpython outside the containers.
  Currently there isn't a way to exploit this vulnerability
  (looks like gitpython is aware of this and may change the default
  to false any time https://github.com/gitpython-developers/GitPython/blob/617c09e70bfd54af1c88b4d2c892b8d287747542/git/repo/base.py#L142-L143)
- In some places we were executing git commands outside the container
  (to get the commit), let's always use the current env (docker in
  production).

Author: stsewd

readthedocs/doc_builder/backends/mkdocs.py

@@ -4,7 +4,6 @@
 .. _MkDocs: http://www.mkdocs.org/
 """
 
-import json
 import logging
 import os
 
@@ -17,7 +16,6 @@
 from readthedocs.projects.constants import MKDOCS, MKDOCS_HTML
 from readthedocs.projects.models import Feature
 
-
 log = logging.getLogger(__name__)
 
 
@@ -235,6 +233,14 @@ def generate_rtd_data(self, docs_dir, mkdocs_config):
             # http://www.mkdocs.org/user-guide/configuration/#google_analytics
             analytics_code = mkdocs_config['google_analytics'][0]
 
+        commit = (
+            self.version.project.vcs_repo(
+                version=self.version.slug,
+                environment=self.build_env,
+            )
+            .commit,
+        )
+
         # Will be available in the JavaScript as READTHEDOCS_DATA.
         readthedocs_data = {
             'project': self.version.project.slug,
@@ -248,7 +254,7 @@ def generate_rtd_data(self, docs_dir, mkdocs_config):
             'source_suffix': '.md',
             'api_host': settings.PUBLIC_API_URL,
             'ad_free': not self.project.show_advertising,
-            'commit': self.version.project.vcs_repo(self.version.slug).commit,
+            'commit': commit,
             'global_analytics_code': (
                 None if self.project.analytics_disabled else settings.GLOBAL_ANALYTICS_CODE
             ),

readthedocs/doc_builder/backends/sphinx.py

@@ -160,6 +160,14 @@ def get_config_params(self):
         if self.version.is_external:
             vcs_url = self.version.vcs_url
 
+        commit = (
+            self.project.vcs_repo(
+                version=self.version.slug,
+                environment=self.build_env,
+            )
+            .commit
+        )
+
         data = {
             'html_theme': 'sphinx_rtd_theme',
             'html_theme_import': 'sphinx_rtd_theme',
@@ -169,7 +177,7 @@ def get_config_params(self):
             'settings': settings,
             'conf_py_path': conf_py_path,
             'api_host': settings.PUBLIC_API_URL,
-            'commit': self.project.vcs_repo(self.version.slug).commit,
+            'commit': commit,
             'versions': versions,
             'downloads': downloads,
             'subproject_urls': subproject_urls,

readthedocs/projects/tasks.py

@@ -927,7 +927,7 @@ def setup_vcs(self, environment):
             # Re raise the exception to stop the build at this point
             raise
 
-        commit = self.commit or self.project.vcs_repo(self.version.slug).commit
+        commit = self.commit or self.get_vcs_repo(environment).commit
         if commit:
             self.build['commit'] = commit
 

readthedocs/rtd_tests/tests/test_doc_builder.py

@@ -1,6 +1,5 @@
 import os
 import tempfile
-from collections import namedtuple
 from unittest import mock
 from unittest.mock import patch
 
@@ -25,6 +24,7 @@
     SingleHtmlBuilder,
 )
 from readthedocs.doc_builder.config import load_yaml_config
+from readthedocs.doc_builder.environments import LocalBuildEnvironment
 from readthedocs.doc_builder.exceptions import MkDocsYAMLParseError
 from readthedocs.doc_builder.python_environments import Virtualenv
 from readthedocs.projects.constants import PRIVATE, PUBLIC
@@ -321,7 +321,7 @@ def setUp(self):
         self.project = get(Project, documentation_type='mkdocs', name='mkdocs')
         self.version = get(Version, project=self.project)
 
-        self.build_env = namedtuple('project', 'version')
+        self.build_env = LocalBuildEnvironment(record=False)
         self.build_env.project = self.project
         self.build_env.version = self.version
 

readthedocs/vcs_support/backends/git.py

@@ -72,11 +72,16 @@ def update(self):
 
     def repo_exists(self):
         try:
-            git.Repo(self.working_dir)
+            self._repo
         except (InvalidGitRepositoryError, NoSuchPathError):
             return False
         return True
 
+    @property
+    def _repo(self):
+        """Get a `git.Repo` instance from the current `self.working_dir`."""
+        return git.Repo(self.working_dir, expand_vars=False)
+
     def are_submodules_available(self, config):
         """Test whether git submodule checkout step should be performed."""
         submodules_in_config = (
@@ -232,7 +237,7 @@ def lsremote(self):
     @property
     def tags(self):
         versions = []
-        repo = git.Repo(self.working_dir)
+        repo = self._repo
 
         # Build a cache of tag -> commit
         # GitPython is not very optimized for reading large numbers of tags
@@ -265,7 +270,7 @@ def tags(self):
 
     @property
     def branches(self):
-        repo = git.Repo(self.working_dir)
+        repo = self._repo
         versions = []
         branches = []
 
@@ -291,8 +296,7 @@ def commit(self):
 
     @property
     def submodules(self):
-        repo = git.Repo(self.working_dir)
-        return list(repo.submodules)
+        return list(self._repo.submodules)
 
     def checkout(self, identifier=None):
         """Checkout to identifier or latest."""
@@ -350,8 +354,7 @@ def find_ref(self, ref):
 
     def ref_exists(self, ref):
         try:
-            r = git.Repo(self.working_dir)
-            if r.commit(ref):
+            if self._repo.commit(ref):
                 return True
         except (BadName, ValueError):
             return False