Merge "Prevent running from a different Kayobe configuration repository"

kayobe/cli/commands.py

@@ -237,7 +237,15 @@ class HookDispatcher(CommandHook):
             self.logger.debug("Running hooks: %s" % hooks)
             self.command.run_kayobe_playbooks(parsed_args, hooks)
 
+    def _preflight_checks(self, parsed_args):
+        # NOTE(mgoddard): Currently all commands use KayobeAnsibleMixin, so
+        # should have a config_path attribute, but better to be defensive.
+        config_path = getattr(parsed_args, "config_path", None)
+        if config_path:
+            utils.validate_config_path(config_path)
+
     def before(self, parsed_args):
+        self._preflight_checks(parsed_args)
         self.run_hooks(parsed_args, "pre")
         return parsed_args
 

kayobe/tests/unit/test_utils.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import logging
 import os
 import subprocess
 import unittest
@@ -189,14 +190,12 @@ key3:
         mock_call.assert_called_once_with(["command", "to", "run"])
         self.assertIsNone(output)
 
-    @mock.patch("kayobe.utils.open")
     @mock.patch.object(subprocess, "check_call")
-    def test_run_command_quiet(self, mock_call, mock_open):
+    def test_run_command_quiet(self, mock_call):
-        mock_devnull = mock_open.return_value.__enter__.return_value
         output = utils.run_command(["command", "to", "run"], quiet=True)
         mock_call.assert_called_once_with(["command", "to", "run"],
-                                          stdout=mock_devnull,
+                                          stdout=subprocess.DEVNULL,
-                                          stderr=mock_devnull)
+                                          stderr=subprocess.DEVNULL)
         self.assertIsNone(output)
 
     @mock.patch.object(subprocess, "check_output")
@@ -332,3 +331,122 @@ key3:
         finder = utils.EnvironmentFinder('/etc/kayobe', None)
         self.assertEqual([], finder.ordered())
         self.assertEqual([], finder.ordered_paths())
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    def test_validate_config_path_kayobe(self, mock_readable, mock_run):
+        mock_run.return_value = b"/path/to/config"
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        self.assertFalse(mock_readable.called)
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    def test_validate_config_path_not_a_repo(self, mock_readable, mock_run):
+        mock_run.side_effect = subprocess.CalledProcessError(
+            "not a repo", "command")
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        self.assertFalse(mock_readable.called)
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    def test_validate_config_path_no_git(self, mock_readable, mock_run):
+        mock_run.side_effect = FileNotFoundError("No such file")
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        self.assertFalse(mock_readable.called)
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    @mock.patch.object(utils, "read_file")
+    def test_validate_config_path_gitreview(self, mock_read, mock_readable,
+                                            mock_run):
+        mock_run.return_value = b"/path/to/repo"
+        mock_readable.return_value = {"result": True}
+        mock_read.return_value = """
+[gerrit]
+project=openstack/kayobe-config.git
+"""
+        with self.assertLogs(level=logging.ERROR) as ctx:
+            self.assertRaises(SystemExit,
+                              utils.validate_config_path,
+                              "/path/to/config/etc/kayobe")
+            exp = ("Executing from within a different Kayobe configuration "
+                   "repository is not allowed")
+            assert any(exp in t for t in ctx.output)
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+        mock_read.assert_called_once_with("/path/to/repo/.gitreview")
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    def test_validate_config_path_no_gitreview(self, mock_readable, mock_run):
+        mock_run.return_value = b"/path/to/repo"
+        mock_readable.return_value = {"result": False}
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    @mock.patch.object(utils, "read_file")
+    def test_validate_config_path_gitreview_no_gerrit(self, mock_read,
+                                                      mock_readable, mock_run):
+        mock_run.return_value = b"/path/to/repo"
+        mock_readable.return_value = {"result": False}
+        mock_read.return_value = """
+[foo]
+bar=baz
+"""
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    @mock.patch.object(utils, "read_file")
+    def test_validate_config_path_gitreview_no_project(self, mock_read,
+                                                       mock_readable,
+                                                       mock_run):
+        mock_run.return_value = b"/path/to/repo"
+        mock_readable.return_value = {"result": False}
+        mock_read.return_value = """
+[gerrit]
+bar=baz
+"""
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        mock_readable.assert_called_once_with("/path/to/repo/.gitreview")
+
+    @mock.patch.object(utils, "run_command")
+    @mock.patch.object(utils, "is_readable_file")
+    @mock.patch.object(utils, "read_file")
+    def test_validate_config_path_gitreview_other_project(self, mock_read,
+                                                          mock_readable,
+                                                          mock_run):
+        mock_run.return_value = b"/path/to/repo"
+        mock_readable.return_value = {"result": False}
+        mock_read.return_value = """
+[gerrit]
+project=baz
+"""
+        utils.validate_config_path("/path/to/config/etc/kayobe")
+        mock_run.assert_called_once_with(
+            ["git", "rev-parse", "--show-toplevel"],
+            check_output=True, quiet=True)
+        mock_readable.assert_called_once_with("/path/to/repo/.gitreview")

kayobe/utils.py

@@ -14,6 +14,7 @@
 
 import base64
 from collections import defaultdict
+import configparser
 import glob
 import graphlib
 from importlib.metadata import Distribution
@@ -223,11 +224,10 @@ def run_command(cmd, quiet=False, check_output=False, **kwargs):
         cmd_string = " ".join(cmd)
     LOG.debug("Running command: %s", cmd_string)
     if quiet:
-        with open("/dev/null", "w") as devnull:
+        kwargs["stderr"] = subprocess.DEVNULL
-            kwargs["stdout"] = devnull
+        if not check_output:
-            kwargs["stderr"] = devnull
+            kwargs["stdout"] = subprocess.DEVNULL
-            subprocess.check_call(cmd, **kwargs)
+    if check_output:
-    elif check_output:
         return subprocess.check_output(cmd, **kwargs)
     else:
         subprocess.check_call(cmd, **kwargs)
@@ -409,3 +409,55 @@ class EnvironmentFinder(object):
             )
             result.append(full_path)
         return result
+
+
+def _gitreview_is_kayobe_config(gitreview_path):
+    """Return whether a .gitreview file is for kayobe-config."""
+    config = configparser.ConfigParser()
+    config_string = read_file(gitreview_path)
+    config.read_string(config_string)
+    gerrit_project = config.get('gerrit', 'project')
+    if not gerrit_project:
+        return False
+    gerrit_project = os.path.basename(gerrit_project)
+    gerrit_project = os.path.splitext(gerrit_project)[0]
+    if gerrit_project == 'kayobe-config':
+        return True
+
+
+def validate_config_path(config_path):
+    """Validate the Kayobe configuration path.
+
+    Check whether we are executing from inside a Kayobe configuration
+    repository, and if so, assert that matches the Kayobe configuration path
+    defined in CLI args or environment variables.
+
+    Exit 1 if validation fails.
+
+    :param config_path: Kayobe configuration path or None.
+    """
+    assert config_path
+
+    try:
+        cmd = ["git", "rev-parse", "--show-toplevel"]
+        repo_root = run_command(cmd, quiet=True, check_output=True)
+    except (FileNotFoundError, subprocess.CalledProcessError):
+        # FileNotFoundError: git probably not installed.
+        # CalledProcessError: probably not in a git repository.
+        return
+
+    repo_root = repo_root.decode().strip()
+    if config_path:
+        repo_config_path = os.path.join(repo_root, "etc", "kayobe")
+        if repo_config_path == os.path.realpath(config_path):
+            return
+
+    # Paths did not match. Check that repo_root does not look like a Kayobe
+    # configuration repo.
+    gitreview_path = os.path.join(repo_root, ".gitreview")
+    result = is_readable_file(gitreview_path)
+    if result["result"]:
+        if _gitreview_is_kayobe_config(gitreview_path):
+            LOG.error("Executing from within a different Kayobe configuration "
+                      "repository is not allowed")
+            sys.exit(1)

releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Adds validation to protect against executing Kayobe from within a different
+    Kayobe configuration repository than the one referred to by environment
+    variables (e.g. ``KAYOBE_CONFIG_PATH``) or CLI arguments (e.g.
+    ``--config-path``).