diff --git a/kayobe/cli/commands.py b/kayobe/cli/commands.py index 170a5698f..5231a0b04 100644 --- a/kayobe/cli/commands.py +++ b/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 diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py index 88fb96da3..0d31f39dc 100644 --- a/kayobe/tests/unit/test_utils.py +++ b/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): - mock_devnull = mock_open.return_value.__enter__.return_value + def test_run_command_quiet(self, mock_call): output = utils.run_command(["command", "to", "run"], quiet=True) mock_call.assert_called_once_with(["command", "to", "run"], - stdout=mock_devnull, - stderr=mock_devnull) + stdout=subprocess.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") diff --git a/kayobe/utils.py b/kayobe/utils.py index 4d10e924c..f53d8e2b6 100644 --- a/kayobe/utils.py +++ b/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["stdout"] = devnull - kwargs["stderr"] = devnull - subprocess.check_call(cmd, **kwargs) - elif check_output: + kwargs["stderr"] = subprocess.DEVNULL + if not check_output: + kwargs["stdout"] = subprocess.DEVNULL + if 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) diff --git a/releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml b/releasenotes/notes/validate-config-path-f26550903c1eb82a.yaml new file mode 100644 index 000000000..ed99e10b5 --- /dev/null +++ b/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``).