From 5e49695023f988ccc1645b3b5e47ddfc75f49b8a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 20 Feb 2024 21:38:52 +1300 Subject: [PATCH 1/4] Sanitise the model name before using it as a path component. --- pytest_operator/plugin.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pytest_operator/plugin.py b/pytest_operator/plugin.py index 286349a..35cb19e 100644 --- a/pytest_operator/plugin.py +++ b/pytest_operator/plugin.py @@ -510,18 +510,30 @@ def models(self) -> Mapping[str, ModelState]: """Returns the dict of managed models by this fixture.""" return {k: dataclasses.replace(v) for k, v in self._models.items()} + @staticmethod + def _model_name_to_filename(name: str): + """Sanitises a model name to one that is suitable for use as a component + of a filename.""" + # Model names can only contain lowercase letters, digits, hyphens, and + # slashes, so most of the normalisation is already done, but we do a + # little bit of duplicated sanitisation here just to be certain. + # "letters" currently means ASCII letters, but may not in the future. + return "".join((c if c.isalnum() or c == "-" else "_") for c in name) + @property def tmp_path(self) -> Path: tmp_path = self._global_tmp_path current_state = self.current_alias and self._models.get(self.current_alias) if current_state and current_state.tmp_path is None: - tmp_path = self._tmp_path_factory.mktemp(current_state.model_name) + tmp_path = self._tmp_path_factory.mktemp( + self._model_name_to_filename(current_state.model_name) + ) current_state.tmp_path = tmp_path elif current_state and current_state.tmp_path: tmp_path = current_state.tmp_path elif not tmp_path: tmp_path = self._global_tmp_path = self._tmp_path_factory.mktemp( - self.default_model_name + self._model_name_to_filename(self.default_model_name) ) log.info(f"Using tmp_path: {tmp_path}") return tmp_path From 047aa723e6306dd3aefdac6df21c5a200d0c22bf Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 20 Feb 2024 22:40:12 +1300 Subject: [PATCH 2/4] Test that the sanitisation works. --- tests/integration/test_pytest_operator.py | 8 ++++++++ tests/unit/test_pytest_operator.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/integration/test_pytest_operator.py b/tests/integration/test_pytest_operator.py index 9650d55..8b89e3f 100644 --- a/tests/integration/test_pytest_operator.py +++ b/tests/integration/test_pytest_operator.py @@ -115,6 +115,14 @@ def test_tmp_path(ops_test): assert ops_test.tmp_path.relative_to(tox_env_dir) +async def test_tmp_path_nonpath_chars(ops_test): + model_alias = f"user{os.sep}name" + new_model = await ops_test.track_model(model_alias) + with ops_test.model_context(model_alias): + assert os.sep not in ops_test.tmp_path + await ops_test.forget_model(model_alias) + + async def test_run(ops_test): assert await ops_test.run("/bin/true") == (0, "", "") assert await ops_test.run("/bin/false") == (1, "", "") diff --git a/tests/unit/test_pytest_operator.py b/tests/unit/test_pytest_operator.py index 0c65da9..3418116 100644 --- a/tests/unit/test_pytest_operator.py +++ b/tests/unit/test_pytest_operator.py @@ -60,6 +60,24 @@ async def test_without_tox(request, ops_test): result.assert_outcomes(passed=1) +@patch.object(plugin.OpsTest, "default_model_name", "user/model") +@patch.object(plugin.OpsTest, "_setup_model", AsyncMock()) +@patch.object(plugin.OpsTest, "_cleanup_models", AsyncMock()) +def test_tmp_path_with_nonpath_chars(pytester): + pytester.makepyfile( + f""" + import os + from pathlib import Path + + os.environ.update(**{ENV}) + async def test_with_tox(ops_test): + assert ops_test.tmp_path.name == "user_model0" + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1) + + async def test_destructive_mode(monkeypatch, tmp_path_factory): patch = monkeypatch.setattr patch(plugin.os, "getgroups", mock_getgroups := Mock(return_value=[])) From 9385dc63f5e70d213febe84fb6c643ed6ce4ca0d Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 26 Feb 2024 10:01:19 +1300 Subject: [PATCH 3/4] Remove unused name. --- tests/integration/test_pytest_operator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_pytest_operator.py b/tests/integration/test_pytest_operator.py index 8b89e3f..a38167e 100644 --- a/tests/integration/test_pytest_operator.py +++ b/tests/integration/test_pytest_operator.py @@ -117,7 +117,7 @@ def test_tmp_path(ops_test): async def test_tmp_path_nonpath_chars(ops_test): model_alias = f"user{os.sep}name" - new_model = await ops_test.track_model(model_alias) + await ops_test.track_model(model_alias) with ops_test.model_context(model_alias): assert os.sep not in ops_test.tmp_path await ops_test.forget_model(model_alias) From f64a5eb18b721a289d01843a160f359bb89bd1b2 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 26 Feb 2024 10:02:45 +1300 Subject: [PATCH 4/4] Fix type. --- tests/integration/test_pytest_operator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_pytest_operator.py b/tests/integration/test_pytest_operator.py index a38167e..bcaab38 100644 --- a/tests/integration/test_pytest_operator.py +++ b/tests/integration/test_pytest_operator.py @@ -119,7 +119,7 @@ async def test_tmp_path_nonpath_chars(ops_test): model_alias = f"user{os.sep}name" await ops_test.track_model(model_alias) with ops_test.model_context(model_alias): - assert os.sep not in ops_test.tmp_path + assert os.sep not in str(ops_test.tmp_path) await ops_test.forget_model(model_alias)