From 9dad253fccee8374d63804e2c45b2dd7847f81cb Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Thu, 14 Oct 2021 09:53:50 +0300 Subject: [PATCH 01/16] Island: create data_dir.py where data dir setup logic will be held --- monkey/monkey_island/cc/server_setup.py | 2 +- monkey/monkey_island/cc/setup/config_setup.py | 16 ++++++++++++---- monkey/monkey_island/cc/setup/data_dir.py | 5 +++++ 3 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 monkey/monkey_island/cc/setup/data_dir.py diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index fdb94b67f..e3e8462a8 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -61,7 +61,7 @@ def run_monkey_island(): def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, str]: try: - return config_setup.setup_data_dir(island_args) + return config_setup.setup_server_config(island_args) except OSError as ex: print(f"Error opening server config file: {ex}") exit(1) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index 3d241b748..ec9a1637d 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -4,11 +4,11 @@ from common.utils.file_utils import expand_path from monkey_island.cc.arg_parser import IslandCmdArgs from monkey_island.cc.environment import server_config_handler from monkey_island.cc.server_utils.consts import DEFAULT_SERVER_CONFIG_PATH -from monkey_island.cc.server_utils.file_utils import create_secure_directory +from monkey_island.cc.setup.data_dir import setup_data_dir from monkey_island.cc.setup.island_config_options import IslandConfigOptions -def setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, str]: +def setup_server_config(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, str]: if island_args.server_config_path: return _setup_config_by_cmd_arg(island_args.server_config_path) @@ -18,7 +18,12 @@ def setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, str def _setup_config_by_cmd_arg(server_config_path) -> Tuple[IslandConfigOptions, str]: server_config_path = expand_path(server_config_path) config = server_config_handler.load_server_config_from_file(server_config_path) - create_secure_directory(str(config.data_dir)) + + # TODO refactor like in https://github.com/guardicore/monkey/pull/1528 because + # there's absolutely no reason to be exposed to IslandConfigOptions extraction logic + # if you want to modify data directory related code. + setup_data_dir(str(config.data_dir)) + return config, server_config_path @@ -26,7 +31,10 @@ def _setup_default_config() -> Tuple[IslandConfigOptions, str]: default_config = server_config_handler.load_server_config_from_file(DEFAULT_SERVER_CONFIG_PATH) default_data_dir = default_config.data_dir - create_secure_directory(str(default_data_dir)) + # TODO refactor like in https://github.com/guardicore/monkey/pull/1528 because + # there's absolutely no reason to be exposed to IslandConfigOptions extraction logic + # if you want to modify data directory related code. + setup_data_dir(str(default_data_dir)) server_config_path = server_config_handler.create_default_server_config_file(default_data_dir) config = server_config_handler.load_server_config_from_file(server_config_path) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py new file mode 100644 index 000000000..c95bc897d --- /dev/null +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -0,0 +1,5 @@ +from monkey_island.cc.server_utils.file_utils import create_secure_directory + + +def setup_data_dir(path: str): + create_secure_directory(path) From e77ed9769b7250ec7ecdbf9b5b02f284a09b2282 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Thu, 14 Oct 2021 17:00:26 +0300 Subject: [PATCH 02/16] Island: expand data directory setup process with workflow that drops a version file or cleans the directory if it's outdated Version file is needed in the data directory to know if data directory is from an earlier version --- monkey/monkey_island/cc/setup/data_dir.py | 43 ++++++++++++- .../cc/setup/version_file_setup.py | 21 +++++++ .../monkey_island/cc/setup/test_data_dir.py | 60 +++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 monkey/monkey_island/cc/setup/version_file_setup.py create mode 100644 monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index c95bc897d..d4d176ba7 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -1,5 +1,44 @@ +import logging +import shutil + +from common.version import get_version from monkey_island.cc.server_utils.file_utils import create_secure_directory +from monkey_island.cc.setup.version_file_setup import ( + get_version_from_dir, + is_version_greater, + write_version, +) + +logger = logging.getLogger(__name__) -def setup_data_dir(path: str): - create_secure_directory(path) +def setup_data_dir(data_dir_path: str): + logger.info("Setting up data directory.") + _reset_data_dir(data_dir_path) + create_secure_directory(data_dir_path) + write_version(data_dir_path) + logger.info("Data directory set up.") + + +def _reset_data_dir(data_dir_path: str): + try: + data_dir_version = get_version_from_dir(data_dir_path) + except FileNotFoundError: + logger.info("Version file not found on the data directory.") + _remove_data_dir(data_dir_path) + return + + island_version = get_version() + logger.info(f"Version found in the data directory: {data_dir_version}") + logger.info(f"Currently running version: {island_version}") + if is_version_greater(island_version, data_dir_version): + _remove_data_dir(data_dir_path) + + +def _remove_data_dir(data_dir_path: str): + logger.info("Attempting to remove data directory.") + try: + shutil.rmtree(data_dir_path) + logger.info("Data directory removed.") + except FileNotFoundError: + logger.info("Data directory not found, nothing to remove.") diff --git a/monkey/monkey_island/cc/setup/version_file_setup.py b/monkey/monkey_island/cc/setup/version_file_setup.py new file mode 100644 index 000000000..ac45c201b --- /dev/null +++ b/monkey/monkey_island/cc/setup/version_file_setup.py @@ -0,0 +1,21 @@ +from pathlib import Path + +from packaging import version + +from common.version import get_version + +_version_filename = "VERSION" + + +def get_version_from_dir(dir_path: str) -> str: + version_file_path = Path(dir_path, _version_filename) + return version_file_path.read_text() + + +def write_version(dir_path: str): + version_file_path = Path(dir_path, _version_filename) + version_file_path.write_text(get_version()) + + +def is_version_greater(version1: str, version2: str) -> bool: + return version.parse(version1) > version.parse(version2) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py new file mode 100644 index 000000000..81ed1f51d --- /dev/null +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -0,0 +1,60 @@ +from pathlib import Path + +import pytest + +from monkey_island.cc.setup.data_dir import setup_data_dir +from monkey_island.cc.setup.version_file_setup import _version_filename + +current_version = "1.1.1" +old_version = "1.1.0" + + +@pytest.fixture(autouse=True) +def mock_version(monkeypatch): + monkeypatch.setattr("monkey_island.cc.setup.data_dir.get_version", lambda: current_version) + monkeypatch.setattr( + "monkey_island.cc.setup.version_file_setup.get_version", lambda: current_version + ) + + +@pytest.fixture +def mocked_data_dir_path(tmpdir) -> Path: + return Path(tmpdir, "data_dir") + + +@pytest.fixture +def mocked_version_file_path(mocked_data_dir_path: Path) -> Path: + return mocked_data_dir_path.joinpath(_version_filename) + + +def test_setup_data_dir(mocked_data_dir_path, mocked_version_file_path): + data_dir_path = mocked_data_dir_path + setup_data_dir(str(data_dir_path)) + assert data_dir_path.is_dir() + + version_file_path = mocked_version_file_path + assert version_file_path.read_text() == current_version + + +def test_old_version_present(mocked_data_dir_path, mocked_version_file_path): + mocked_data_dir_path.mkdir() + mocked_version_file_path.write_text(old_version) + bogus_file_path = mocked_data_dir_path.joinpath("test.txt") + bogus_file_path.touch() + + # mock version + setup_data_dir(str(mocked_data_dir_path)) + + assert mocked_version_file_path.read_text() == current_version + assert not bogus_file_path.is_file() + + +def test_data_dir_setup_not_needed(mocked_data_dir_path, mocked_version_file_path): + mocked_data_dir_path.mkdir() + mocked_version_file_path.write_text(current_version) + bogus_file_path = mocked_data_dir_path.joinpath("test.txt") + bogus_file_path.touch() + + setup_data_dir(str(mocked_data_dir_path)) + assert mocked_version_file_path.read_text() == current_version + assert bogus_file_path.is_file() From 15949a9ed586a8e5a1ca22b2cba72d0450b2770f Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 15 Oct 2021 16:08:20 +0300 Subject: [PATCH 03/16] Monkey island: change the methods in data_dir.py and version_file_setup.py to handle Path rather than str. --- monkey/monkey_island/cc/setup/config_setup.py | 4 ++-- monkey/monkey_island/cc/setup/data_dir.py | 7 ++++--- monkey/monkey_island/cc/setup/version_file_setup.py | 8 ++++---- .../unit_tests/monkey_island/cc/setup/test_data_dir.py | 6 +++--- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/monkey/monkey_island/cc/setup/config_setup.py b/monkey/monkey_island/cc/setup/config_setup.py index ec9a1637d..b1e76b51d 100644 --- a/monkey/monkey_island/cc/setup/config_setup.py +++ b/monkey/monkey_island/cc/setup/config_setup.py @@ -22,7 +22,7 @@ def _setup_config_by_cmd_arg(server_config_path) -> Tuple[IslandConfigOptions, s # TODO refactor like in https://github.com/guardicore/monkey/pull/1528 because # there's absolutely no reason to be exposed to IslandConfigOptions extraction logic # if you want to modify data directory related code. - setup_data_dir(str(config.data_dir)) + setup_data_dir(config.data_dir) return config, server_config_path @@ -34,7 +34,7 @@ def _setup_default_config() -> Tuple[IslandConfigOptions, str]: # TODO refactor like in https://github.com/guardicore/monkey/pull/1528 because # there's absolutely no reason to be exposed to IslandConfigOptions extraction logic # if you want to modify data directory related code. - setup_data_dir(str(default_data_dir)) + setup_data_dir(default_data_dir) server_config_path = server_config_handler.create_default_server_config_file(default_data_dir) config = server_config_handler.load_server_config_from_file(server_config_path) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index d4d176ba7..bee9d0a05 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -1,5 +1,6 @@ import logging import shutil +from pathlib import Path from common.version import get_version from monkey_island.cc.server_utils.file_utils import create_secure_directory @@ -12,15 +13,15 @@ from monkey_island.cc.setup.version_file_setup import ( logger = logging.getLogger(__name__) -def setup_data_dir(data_dir_path: str): +def setup_data_dir(data_dir_path: Path): logger.info("Setting up data directory.") _reset_data_dir(data_dir_path) - create_secure_directory(data_dir_path) + create_secure_directory(str(data_dir_path)) write_version(data_dir_path) logger.info("Data directory set up.") -def _reset_data_dir(data_dir_path: str): +def _reset_data_dir(data_dir_path: Path): try: data_dir_version = get_version_from_dir(data_dir_path) except FileNotFoundError: diff --git a/monkey/monkey_island/cc/setup/version_file_setup.py b/monkey/monkey_island/cc/setup/version_file_setup.py index ac45c201b..52a71ce4f 100644 --- a/monkey/monkey_island/cc/setup/version_file_setup.py +++ b/monkey/monkey_island/cc/setup/version_file_setup.py @@ -7,13 +7,13 @@ from common.version import get_version _version_filename = "VERSION" -def get_version_from_dir(dir_path: str) -> str: - version_file_path = Path(dir_path, _version_filename) +def get_version_from_dir(dir_path: Path) -> str: + version_file_path = dir_path.joinpath(_version_filename) return version_file_path.read_text() -def write_version(dir_path: str): - version_file_path = Path(dir_path, _version_filename) +def write_version(dir_path: Path): + version_file_path = dir_path.joinpath(_version_filename) version_file_path.write_text(get_version()) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index 81ed1f51d..096c5b0f3 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -29,7 +29,7 @@ def mocked_version_file_path(mocked_data_dir_path: Path) -> Path: def test_setup_data_dir(mocked_data_dir_path, mocked_version_file_path): data_dir_path = mocked_data_dir_path - setup_data_dir(str(data_dir_path)) + setup_data_dir(data_dir_path) assert data_dir_path.is_dir() version_file_path = mocked_version_file_path @@ -43,7 +43,7 @@ def test_old_version_present(mocked_data_dir_path, mocked_version_file_path): bogus_file_path.touch() # mock version - setup_data_dir(str(mocked_data_dir_path)) + setup_data_dir(mocked_data_dir_path) assert mocked_version_file_path.read_text() == current_version assert not bogus_file_path.is_file() @@ -55,6 +55,6 @@ def test_data_dir_setup_not_needed(mocked_data_dir_path, mocked_version_file_pat bogus_file_path = mocked_data_dir_path.joinpath("test.txt") bogus_file_path.touch() - setup_data_dir(str(mocked_data_dir_path)) + setup_data_dir(mocked_data_dir_path) assert mocked_version_file_path.read_text() == current_version assert bogus_file_path.is_file() From 0efcffbe54513253ad02e33cfced19d740a575db Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 15 Oct 2021 17:11:43 +0300 Subject: [PATCH 04/16] Monkey island: instead of deleting the backup of data_dir, rename it to data_dir.old. If it already exists, delete it prior to renaming This change saves the users data in case version update destroyed some progress. We don't care about removing old backups because user is unlikely to do two updates at once --- monkey/monkey_island/cc/setup/data_dir.py | 23 ++++++++++++------ .../monkey_island/cc/setup/test_data_dir.py | 24 ++++++++++++++++--- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index bee9d0a05..c47aa1680 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -11,6 +11,7 @@ from monkey_island.cc.setup.version_file_setup import ( ) logger = logging.getLogger(__name__) +_data_dir_backup_suffix = ".old" def setup_data_dir(data_dir_path: Path): @@ -26,20 +27,28 @@ def _reset_data_dir(data_dir_path: Path): data_dir_version = get_version_from_dir(data_dir_path) except FileNotFoundError: logger.info("Version file not found on the data directory.") - _remove_data_dir(data_dir_path) + _backup_old_data_dir(data_dir_path) return island_version = get_version() logger.info(f"Version found in the data directory: {data_dir_version}") logger.info(f"Currently running version: {island_version}") if is_version_greater(island_version, data_dir_version): - _remove_data_dir(data_dir_path) + _backup_old_data_dir(data_dir_path) -def _remove_data_dir(data_dir_path: str): - logger.info("Attempting to remove data directory.") +def _backup_old_data_dir(data_dir_path: Path): + logger.info("Attempting to backup old data directory.") try: - shutil.rmtree(data_dir_path) - logger.info("Data directory removed.") + backup_path = _get_backup_path(data_dir_path) + if backup_path.is_dir(): + shutil.rmtree(backup_path) + Path(data_dir_path).replace(backup_path) + logger.info(f"Old data directory moved to {backup_path}.") except FileNotFoundError: - logger.info("Data directory not found, nothing to remove.") + logger.info("Old data directory not found, nothing to backup.") + + +def _get_backup_path(data_dir_path: Path) -> Path: + backup_dir_name = data_dir_path.name + _data_dir_backup_suffix + return Path(data_dir_path.parent, backup_dir_name) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index 096c5b0f3..dfabfa54e 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -2,8 +2,8 @@ from pathlib import Path import pytest -from monkey_island.cc.setup.data_dir import setup_data_dir -from monkey_island.cc.setup.version_file_setup import _version_filename +from monkey_island.cc.setup.data_dir import _get_backup_path, setup_data_dir +from monkey_island.cc.setup.version_file_setup import _version_filename, get_version_from_dir current_version = "1.1.1" old_version = "1.1.0" @@ -42,11 +42,29 @@ def test_old_version_present(mocked_data_dir_path, mocked_version_file_path): bogus_file_path = mocked_data_dir_path.joinpath("test.txt") bogus_file_path.touch() - # mock version setup_data_dir(mocked_data_dir_path) assert mocked_version_file_path.read_text() == current_version assert not bogus_file_path.is_file() + assert _get_backup_path(mocked_data_dir_path).joinpath("test.txt").is_file() + + +def test_old_version_and_backup_present(mocked_data_dir_path, mocked_version_file_path): + mocked_data_dir_path.mkdir() + mocked_version_file_path.write_text(old_version) + + old_backup_path = _get_backup_path(mocked_data_dir_path) + old_backup_path.mkdir() + bogus_file_path = old_backup_path.joinpath("test.txt") + bogus_file_path.touch() + + setup_data_dir(mocked_data_dir_path) + new_backup_path = old_backup_path + + # Make sure old backup got deleted and new backup took it's place + assert mocked_version_file_path.read_text() == current_version + assert get_version_from_dir(new_backup_path) == old_version + assert not _get_backup_path(mocked_data_dir_path).joinpath("test.txt").is_file() def test_data_dir_setup_not_needed(mocked_data_dir_path, mocked_version_file_path): From 93adbae2bf179c7430be2ce9c8d02dfedb25a364 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 18 Oct 2021 09:55:22 +0300 Subject: [PATCH 05/16] Island: change some less important log messages to debug level, log data directory path in data_dir.py --- monkey/monkey_island/cc/setup/data_dir.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index c47aa1680..63e609f8f 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -15,7 +15,7 @@ _data_dir_backup_suffix = ".old" def setup_data_dir(data_dir_path: Path): - logger.info("Setting up data directory.") + logger.info(f"Setting up data directory in {data_dir_path}.") _reset_data_dir(data_dir_path) create_secure_directory(str(data_dir_path)) write_version(data_dir_path) @@ -26,13 +26,13 @@ def _reset_data_dir(data_dir_path: Path): try: data_dir_version = get_version_from_dir(data_dir_path) except FileNotFoundError: - logger.info("Version file not found on the data directory.") + logger.debug("Version file not found.") _backup_old_data_dir(data_dir_path) return island_version = get_version() - logger.info(f"Version found in the data directory: {data_dir_version}") - logger.info(f"Currently running version: {island_version}") + logger.debug(f"Version found in the data directory: {data_dir_version}") + logger.debug(f"Currently running version: {island_version}") if is_version_greater(island_version, data_dir_version): _backup_old_data_dir(data_dir_path) @@ -46,7 +46,7 @@ def _backup_old_data_dir(data_dir_path: Path): Path(data_dir_path).replace(backup_path) logger.info(f"Old data directory moved to {backup_path}.") except FileNotFoundError: - logger.info("Old data directory not found, nothing to backup.") + logger.info("No data directory found to backup, this is likelly a first installation.") def _get_backup_path(data_dir_path: Path) -> Path: From c9335f90a4f3628aacc09954ac9ef8d94ecbdf6e Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 18 Oct 2021 10:37:38 +0300 Subject: [PATCH 06/16] Island UT's: rename methods that return directories from "mocked_..." notation to "temp_..." notation in test_data_dir.py --- .../monkey_island/cc/setup/test_data_dir.py | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index dfabfa54e..907e62741 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -18,61 +18,61 @@ def mock_version(monkeypatch): @pytest.fixture -def mocked_data_dir_path(tmpdir) -> Path: +def temp_data_dir_path(tmpdir) -> Path: return Path(tmpdir, "data_dir") @pytest.fixture -def mocked_version_file_path(mocked_data_dir_path: Path) -> Path: - return mocked_data_dir_path.joinpath(_version_filename) +def temp_version_file_path(temp_data_dir_path) -> Path: + return temp_data_dir_path.joinpath(_version_filename) -def test_setup_data_dir(mocked_data_dir_path, mocked_version_file_path): - data_dir_path = mocked_data_dir_path +def test_setup_data_dir(temp_data_dir_path, temp_version_file_path): + data_dir_path = temp_data_dir_path setup_data_dir(data_dir_path) assert data_dir_path.is_dir() - version_file_path = mocked_version_file_path + version_file_path = temp_version_file_path assert version_file_path.read_text() == current_version -def test_old_version_present(mocked_data_dir_path, mocked_version_file_path): - mocked_data_dir_path.mkdir() - mocked_version_file_path.write_text(old_version) - bogus_file_path = mocked_data_dir_path.joinpath("test.txt") +def test_old_version_present(temp_data_dir_path, temp_version_file_path): + temp_data_dir_path.mkdir() + temp_version_file_path.write_text(old_version) + bogus_file_path = temp_data_dir_path.joinpath("test.txt") bogus_file_path.touch() - setup_data_dir(mocked_data_dir_path) + setup_data_dir(temp_data_dir_path) - assert mocked_version_file_path.read_text() == current_version + assert temp_version_file_path.read_text() == current_version assert not bogus_file_path.is_file() - assert _get_backup_path(mocked_data_dir_path).joinpath("test.txt").is_file() + assert _get_backup_path(temp_data_dir_path).joinpath("test.txt").is_file() -def test_old_version_and_backup_present(mocked_data_dir_path, mocked_version_file_path): - mocked_data_dir_path.mkdir() - mocked_version_file_path.write_text(old_version) +def test_old_version_and_backup_present(temp_data_dir_path, temp_version_file_path): + temp_data_dir_path.mkdir() + temp_version_file_path.write_text(old_version) - old_backup_path = _get_backup_path(mocked_data_dir_path) + old_backup_path = _get_backup_path(temp_data_dir_path) old_backup_path.mkdir() bogus_file_path = old_backup_path.joinpath("test.txt") bogus_file_path.touch() - setup_data_dir(mocked_data_dir_path) + setup_data_dir(temp_data_dir_path) new_backup_path = old_backup_path # Make sure old backup got deleted and new backup took it's place - assert mocked_version_file_path.read_text() == current_version + assert temp_version_file_path.read_text() == current_version assert get_version_from_dir(new_backup_path) == old_version - assert not _get_backup_path(mocked_data_dir_path).joinpath("test.txt").is_file() + assert not _get_backup_path(temp_data_dir_path).joinpath("test.txt").is_file() -def test_data_dir_setup_not_needed(mocked_data_dir_path, mocked_version_file_path): - mocked_data_dir_path.mkdir() - mocked_version_file_path.write_text(current_version) - bogus_file_path = mocked_data_dir_path.joinpath("test.txt") +def test_data_dir_setup_not_needed(temp_data_dir_path, temp_version_file_path): + temp_data_dir_path.mkdir() + temp_version_file_path.write_text(current_version) + bogus_file_path = temp_data_dir_path.joinpath("test.txt") bogus_file_path.touch() - setup_data_dir(mocked_data_dir_path) - assert mocked_version_file_path.read_text() == current_version + setup_data_dir(temp_data_dir_path) + assert temp_version_file_path.read_text() == current_version assert bogus_file_path.is_file() From b0e96822dd409353ba96b10c4f75839667a07262 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 18 Oct 2021 11:42:12 +0300 Subject: [PATCH 07/16] Island: split up _reset_data_dir method into _backup_current_data_dir and _is_backup_needed in data_dir.py Change makes the code more readable because the functions have better names and the logic of finding out if the back up is needed / doing the actual back up is separated --- monkey/monkey_island/cc/setup/data_dir.py | 38 ++++++++++++----------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 63e609f8f..0ba011d65 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -16,37 +16,39 @@ _data_dir_backup_suffix = ".old" def setup_data_dir(data_dir_path: Path): logger.info(f"Setting up data directory in {data_dir_path}.") - _reset_data_dir(data_dir_path) + _backup_current_data_dir(data_dir_path) create_secure_directory(str(data_dir_path)) write_version(data_dir_path) logger.info("Data directory set up.") -def _reset_data_dir(data_dir_path: Path): +def _backup_current_data_dir(data_dir_path: Path): + if _is_backup_needed(data_dir_path): + logger.debug("Data directory backup needed.") + try: + return _rename_data_dir(data_dir_path) + except FileNotFoundError: + logger.debug("No data directory found to backup, this is likely a first installation.") + + +def _is_backup_needed(data_dir_path: Path) -> bool: try: data_dir_version = get_version_from_dir(data_dir_path) except FileNotFoundError: logger.debug("Version file not found.") - _backup_old_data_dir(data_dir_path) - return + return True island_version = get_version() - logger.debug(f"Version found in the data directory: {data_dir_version}") - logger.debug(f"Currently running version: {island_version}") - if is_version_greater(island_version, data_dir_version): - _backup_old_data_dir(data_dir_path) + + return is_version_greater(island_version, data_dir_version) -def _backup_old_data_dir(data_dir_path: Path): - logger.info("Attempting to backup old data directory.") - try: - backup_path = _get_backup_path(data_dir_path) - if backup_path.is_dir(): - shutil.rmtree(backup_path) - Path(data_dir_path).replace(backup_path) - logger.info(f"Old data directory moved to {backup_path}.") - except FileNotFoundError: - logger.info("No data directory found to backup, this is likelly a first installation.") +def _rename_data_dir(data_dir_path: Path): + backup_path = _get_backup_path(data_dir_path) + if backup_path.is_dir(): + shutil.rmtree(backup_path) + Path(data_dir_path).replace(backup_path) + logger.info(f"Old data directory renamed to {backup_path}.") def _get_backup_path(data_dir_path: Path) -> Path: From 988bdf047117657cad5066387c0963ab6f05d632 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 18 Oct 2021 11:54:06 +0300 Subject: [PATCH 08/16] Island: check if version from version file is the same as in island instead of checking if it's lower This change enables to clean the directory if an older version of monkey island is installed after removing the new one --- monkey/monkey_island/cc/setup/data_dir.py | 4 ++-- monkey/monkey_island/cc/setup/version_file_setup.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 0ba011d65..5ea3b49e9 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -6,7 +6,7 @@ from common.version import get_version from monkey_island.cc.server_utils.file_utils import create_secure_directory from monkey_island.cc.setup.version_file_setup import ( get_version_from_dir, - is_version_greater, + is_version_different, write_version, ) @@ -40,7 +40,7 @@ def _is_backup_needed(data_dir_path: Path) -> bool: island_version = get_version() - return is_version_greater(island_version, data_dir_version) + return is_version_different(island_version, data_dir_version) def _rename_data_dir(data_dir_path: Path): diff --git a/monkey/monkey_island/cc/setup/version_file_setup.py b/monkey/monkey_island/cc/setup/version_file_setup.py index 52a71ce4f..a1fb2a801 100644 --- a/monkey/monkey_island/cc/setup/version_file_setup.py +++ b/monkey/monkey_island/cc/setup/version_file_setup.py @@ -17,5 +17,5 @@ def write_version(dir_path: Path): version_file_path.write_text(get_version()) -def is_version_greater(version1: str, version2: str) -> bool: - return version.parse(version1) > version.parse(version2) +def is_version_different(version1: str, version2: str) -> bool: + return not version.parse(version1) == version.parse(version2) From c23a0721c58fcd0a4faf015ec2dbcfb425b37e22 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Mon, 18 Oct 2021 12:15:09 +0300 Subject: [PATCH 09/16] CHANGELOG.md entry about data dir backup based on version file --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bf579837..4f2b95f8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ Changelog](https://keepachangelog.com/en/1.0.0/). - ATT&CK report bug that said that the technique "`.bash_profile` and `.bashrc`" was not attempted when it actually was attempted but failed. #1511 - Bug that periodically cleared the telemetry table's filter. #1392 +- Updating the Infection Monkey will backup the old data +and will create a new data directory to prevent incompatibilities and crashes. #1114 + ### Security - Generate a random password when creating a new user for CommunicateAsNewUser From 27d04e4de662053076b509e6f92e462e9a03f005 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Tue, 19 Oct 2021 10:45:44 +0300 Subject: [PATCH 10/16] Monkey: simplify version comparison to string comparison instead of using a package to parse the version --- monkey/monkey_island/cc/setup/data_dir.py | 8 ++------ monkey/monkey_island/cc/setup/version_file_setup.py | 6 ------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 5ea3b49e9..590e966ba 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -4,11 +4,7 @@ from pathlib import Path from common.version import get_version from monkey_island.cc.server_utils.file_utils import create_secure_directory -from monkey_island.cc.setup.version_file_setup import ( - get_version_from_dir, - is_version_different, - write_version, -) +from monkey_island.cc.setup.version_file_setup import get_version_from_dir, write_version logger = logging.getLogger(__name__) _data_dir_backup_suffix = ".old" @@ -40,7 +36,7 @@ def _is_backup_needed(data_dir_path: Path) -> bool: island_version = get_version() - return is_version_different(island_version, data_dir_version) + return island_version != data_dir_version def _rename_data_dir(data_dir_path: Path): diff --git a/monkey/monkey_island/cc/setup/version_file_setup.py b/monkey/monkey_island/cc/setup/version_file_setup.py index a1fb2a801..380fcf4cc 100644 --- a/monkey/monkey_island/cc/setup/version_file_setup.py +++ b/monkey/monkey_island/cc/setup/version_file_setup.py @@ -1,7 +1,5 @@ from pathlib import Path -from packaging import version - from common.version import get_version _version_filename = "VERSION" @@ -15,7 +13,3 @@ def get_version_from_dir(dir_path: Path) -> str: def write_version(dir_path: Path): version_file_path = dir_path.joinpath(_version_filename) version_file_path.write_text(get_version()) - - -def is_version_different(version1: str, version2: str) -> bool: - return not version.parse(version1) == version.parse(version2) From dd480d1703bfd6d8542d73bb1b0f934efed52ab6 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Wed, 20 Oct 2021 16:10:01 +0530 Subject: [PATCH 11/16] island: Prompt user for old data dir's deletion during Island setup if old data dir's and Island's versions mismatch --- monkey/monkey_island/cc/server_setup.py | 20 ++++++++++ monkey/monkey_island/cc/setup/data_dir.py | 46 ++++++++++++----------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index e3e8462a8..38e7de5d8 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -1,6 +1,7 @@ import atexit import json import logging +import shutil import sys from pathlib import Path from threading import Thread @@ -32,6 +33,7 @@ from monkey_island.cc.services.initialize import initialize_services # noqa: E4 from monkey_island.cc.services.reporting.exporter_init import populate_exporter_list # noqa: E402 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 from monkey_island.cc.setup import island_config_options_validator # noqa: E402 +from monkey_island.cc.setup.data_dir import OldDataError # noqa: E402 from monkey_island.cc.setup.gevent_hub_error_handler import GeventHubErrorHandler # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 from monkey_island.cc.setup.mongo import mongo_setup # noqa: E402 @@ -68,6 +70,24 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st except json.JSONDecodeError as ex: print(f"Error loading server config: {ex}") exit(1) + except OldDataError as ex: + user_response = input( + f"\nExisting data directory ({ex.old_data_dir}) needs to be deleted." + " All data from previous runs will be lost. Proceed to delete? (y/n) " + ) + if user_response == "y": + shutil.rmtree(ex.old_data_dir) + print("\nOld data directory was deleted. Trying to set up again...\n") + return _setup_data_dir(island_args) + elif user_response == "n": + print( + "\nExiting. Please backup and delete the existing data directory. Then, try again." + "\nTo learn how to restore and use a backup, please refer to the documentation.\n" + ) + exit(1) + else: + print("\nExiting. Unrecognized response, please try again.\n") + exit(1) def _exit_on_invalid_config_options(config_options: IslandConfigOptions): diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 590e966ba..cc810f9da 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -1,5 +1,4 @@ import logging -import shutil from pathlib import Path from common.version import get_version @@ -7,27 +6,32 @@ from monkey_island.cc.server_utils.file_utils import create_secure_directory from monkey_island.cc.setup.version_file_setup import get_version_from_dir, write_version logger = logging.getLogger(__name__) + _data_dir_backup_suffix = ".old" -def setup_data_dir(data_dir_path: Path): - logger.info(f"Setting up data directory in {data_dir_path}.") - _backup_current_data_dir(data_dir_path) +class OldDataError(Exception): + def __init__(self, old_data_dir: Path) -> None: + self.old_data_dir = old_data_dir + + +def setup_data_dir(data_dir_path: Path) -> None: + logger.info(f"Setting up data directory at {data_dir_path}.") + if data_dir_path.exists(): + logger.info(f"Data directory already exists at {data_dir_path}.") + _check_current_data_dir(data_dir_path) create_secure_directory(str(data_dir_path)) write_version(data_dir_path) logger.info("Data directory set up.") -def _backup_current_data_dir(data_dir_path: Path): - if _is_backup_needed(data_dir_path): - logger.debug("Data directory backup needed.") - try: - return _rename_data_dir(data_dir_path) - except FileNotFoundError: - logger.debug("No data directory found to backup, this is likely a first installation.") +def _check_current_data_dir(data_dir_path: Path) -> None: + if _data_dir_version_mismatch_exists(data_dir_path): + logger.info("Version in data directory does not match the Island's version.") + raise OldDataError(data_dir_path) -def _is_backup_needed(data_dir_path: Path) -> bool: +def _data_dir_version_mismatch_exists(data_dir_path: Path) -> bool: try: data_dir_version = get_version_from_dir(data_dir_path) except FileNotFoundError: @@ -39,14 +43,14 @@ def _is_backup_needed(data_dir_path: Path) -> bool: return island_version != data_dir_version -def _rename_data_dir(data_dir_path: Path): - backup_path = _get_backup_path(data_dir_path) - if backup_path.is_dir(): - shutil.rmtree(backup_path) - Path(data_dir_path).replace(backup_path) - logger.info(f"Old data directory renamed to {backup_path}.") +# def _rename_data_dir(data_dir_path: Path): +# backup_path = _get_backup_path(data_dir_path) +# if backup_path.is_dir(): +# shutil.rmtree(backup_path) +# Path(data_dir_path).replace(backup_path) +# logger.info(f"Old data directory renamed to {backup_path}.") -def _get_backup_path(data_dir_path: Path) -> Path: - backup_dir_name = data_dir_path.name + _data_dir_backup_suffix - return Path(data_dir_path.parent, backup_dir_name) +# def _get_backup_path(data_dir_path: Path) -> Path: +# backup_dir_name = data_dir_path.name + _data_dir_backup_suffix +# return Path(data_dir_path.parent, backup_dir_name) From 4ec53cb1d12a70e3ac63e11d5de75f61c0103ee4 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Wed, 20 Oct 2021 16:30:52 +0530 Subject: [PATCH 12/16] island: Extract old data dir manipulation to a separate function --- monkey/monkey_island/cc/server_setup.py | 40 ++++++++++++++----------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 38e7de5d8..05a73280f 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -5,7 +5,7 @@ import shutil import sys from pathlib import Path from threading import Thread -from typing import Tuple +from typing import Optional, Tuple import gevent.hub from gevent.pywsgi import WSGIServer @@ -71,23 +71,29 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st print(f"Error loading server config: {ex}") exit(1) except OldDataError as ex: - user_response = input( - f"\nExisting data directory ({ex.old_data_dir}) needs to be deleted." - " All data from previous runs will be lost. Proceed to delete? (y/n) " + return _handle_existing_data_directory(ex, island_args) + + +def _handle_existing_data_directory( + exception: Exception, island_args: IslandCmdArgs +) -> Optional[Tuple[IslandConfigOptions, str]]: + user_response = input( + f"\nExisting data directory ({exception.old_data_dir}) needs to be deleted." + " All data from previous runs will be lost. Proceed to delete? (y/n) " + ) + if user_response == "y": + shutil.rmtree(exception.old_data_dir) + print("\nOld data directory was deleted. Trying to set up again...\n") + return _setup_data_dir(island_args) + elif user_response == "n": + print( + "\nExiting. Please backup and delete the existing data directory. Then, try again." + "\nTo learn how to restore and use a backup, please refer to the documentation.\n" ) - if user_response == "y": - shutil.rmtree(ex.old_data_dir) - print("\nOld data directory was deleted. Trying to set up again...\n") - return _setup_data_dir(island_args) - elif user_response == "n": - print( - "\nExiting. Please backup and delete the existing data directory. Then, try again." - "\nTo learn how to restore and use a backup, please refer to the documentation.\n" - ) - exit(1) - else: - print("\nExiting. Unrecognized response, please try again.\n") - exit(1) + exit(1) + else: + print("\nExiting. Unrecognized response, please try again.\n") + exit(1) def _exit_on_invalid_config_options(config_options: IslandConfigOptions): From b4c48e7cfb3ef0ffb41daf7ed5a740127d538a35 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Wed, 20 Oct 2021 16:49:52 +0530 Subject: [PATCH 13/16] island: Remove unneeded functions in monkey_island/cc/setup/data_dir.py --- monkey/monkey_island/cc/setup/data_dir.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index cc810f9da..245a4f14c 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -41,16 +41,3 @@ def _data_dir_version_mismatch_exists(data_dir_path: Path) -> bool: island_version = get_version() return island_version != data_dir_version - - -# def _rename_data_dir(data_dir_path: Path): -# backup_path = _get_backup_path(data_dir_path) -# if backup_path.is_dir(): -# shutil.rmtree(backup_path) -# Path(data_dir_path).replace(backup_path) -# logger.info(f"Old data directory renamed to {backup_path}.") - - -# def _get_backup_path(data_dir_path: Path) -> Path: -# backup_dir_name = data_dir_path.name + _data_dir_backup_suffix -# return Path(data_dir_path.parent, backup_dir_name) From d048f4e4cec89ea6d6d60ea44d9c3153316fc2fc Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Wed, 20 Oct 2021 20:42:49 +0530 Subject: [PATCH 14/16] Island: Move data directory deletion prompt to data_dir.py --- monkey/monkey_island/cc/server_setup.py | 29 ++----------- monkey/monkey_island/cc/setup/data_dir.py | 41 +++++++++++++------ .../monkey_island/cc/setup/test_data_dir.py | 31 +++++++------- 3 files changed, 48 insertions(+), 53 deletions(-) diff --git a/monkey/monkey_island/cc/server_setup.py b/monkey/monkey_island/cc/server_setup.py index 05a73280f..8b8e433ea 100644 --- a/monkey/monkey_island/cc/server_setup.py +++ b/monkey/monkey_island/cc/server_setup.py @@ -1,11 +1,10 @@ import atexit import json import logging -import shutil import sys from pathlib import Path from threading import Thread -from typing import Optional, Tuple +from typing import Tuple import gevent.hub from gevent.pywsgi import WSGIServer @@ -33,7 +32,7 @@ from monkey_island.cc.services.initialize import initialize_services # noqa: E4 from monkey_island.cc.services.reporting.exporter_init import populate_exporter_list # noqa: E402 from monkey_island.cc.services.utils.network_utils import local_ip_addresses # noqa: E402 from monkey_island.cc.setup import island_config_options_validator # noqa: E402 -from monkey_island.cc.setup.data_dir import OldDataError # noqa: E402 +from monkey_island.cc.setup.data_dir import IncompatibleDataDirectory # noqa: E402 from monkey_island.cc.setup.gevent_hub_error_handler import GeventHubErrorHandler # noqa: E402 from monkey_island.cc.setup.island_config_options import IslandConfigOptions # noqa: E402 from monkey_island.cc.setup.mongo import mongo_setup # noqa: E402 @@ -70,29 +69,7 @@ def _setup_data_dir(island_args: IslandCmdArgs) -> Tuple[IslandConfigOptions, st except json.JSONDecodeError as ex: print(f"Error loading server config: {ex}") exit(1) - except OldDataError as ex: - return _handle_existing_data_directory(ex, island_args) - - -def _handle_existing_data_directory( - exception: Exception, island_args: IslandCmdArgs -) -> Optional[Tuple[IslandConfigOptions, str]]: - user_response = input( - f"\nExisting data directory ({exception.old_data_dir}) needs to be deleted." - " All data from previous runs will be lost. Proceed to delete? (y/n) " - ) - if user_response == "y": - shutil.rmtree(exception.old_data_dir) - print("\nOld data directory was deleted. Trying to set up again...\n") - return _setup_data_dir(island_args) - elif user_response == "n": - print( - "\nExiting. Please backup and delete the existing data directory. Then, try again." - "\nTo learn how to restore and use a backup, please refer to the documentation.\n" - ) - exit(1) - else: - print("\nExiting. Unrecognized response, please try again.\n") + except IncompatibleDataDirectory: exit(1) diff --git a/monkey/monkey_island/cc/setup/data_dir.py b/monkey/monkey_island/cc/setup/data_dir.py index 245a4f14c..c728dca04 100644 --- a/monkey/monkey_island/cc/setup/data_dir.py +++ b/monkey/monkey_island/cc/setup/data_dir.py @@ -1,4 +1,5 @@ import logging +import shutil from pathlib import Path from common.version import get_version @@ -7,28 +8,44 @@ from monkey_island.cc.setup.version_file_setup import get_version_from_dir, writ logger = logging.getLogger(__name__) -_data_dir_backup_suffix = ".old" - -class OldDataError(Exception): - def __init__(self, old_data_dir: Path) -> None: - self.old_data_dir = old_data_dir +class IncompatibleDataDirectory(Exception): + pass def setup_data_dir(data_dir_path: Path) -> None: logger.info(f"Setting up data directory at {data_dir_path}.") - if data_dir_path.exists(): - logger.info(f"Data directory already exists at {data_dir_path}.") - _check_current_data_dir(data_dir_path) + if data_dir_path.exists() and _data_dir_version_mismatch_exists(data_dir_path): + logger.info("Version in data directory does not match the Island's version.") + _handle_old_data_directory(data_dir_path) create_secure_directory(str(data_dir_path)) write_version(data_dir_path) logger.info("Data directory set up.") -def _check_current_data_dir(data_dir_path: Path) -> None: - if _data_dir_version_mismatch_exists(data_dir_path): - logger.info("Version in data directory does not match the Island's version.") - raise OldDataError(data_dir_path) +def _handle_old_data_directory(data_dir_path: Path) -> None: + should_delete_data_directory = _prompt_user_to_delete_data_directory(data_dir_path) + if should_delete_data_directory: + shutil.rmtree(data_dir_path) + logger.info(f"{data_dir_path} was deleted.") + else: + logger.error( + "Unable to set up data directory. Please backup and delete the existing data directory" + f" ({data_dir_path}). Then, try again. To learn how to restore and use a backup, please" + " refer to the documentation." + ) + raise IncompatibleDataDirectory() + + +def _prompt_user_to_delete_data_directory(data_dir_path: Path) -> bool: + user_response = input( + f"\nExisting data directory ({data_dir_path}) needs to be deleted." + " All data from previous runs will be lost. Proceed to delete? (y/n) " + ) + print() + if user_response.lower() in {"y", "yes"}: + return True + return False def _data_dir_version_mismatch_exists(data_dir_path: Path) -> bool: diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index 907e62741..696b604dc 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -2,8 +2,8 @@ from pathlib import Path import pytest -from monkey_island.cc.setup.data_dir import _get_backup_path, setup_data_dir -from monkey_island.cc.setup.version_file_setup import _version_filename, get_version_from_dir +from monkey_island.cc.setup.data_dir import IncompatibleDataDirectory, setup_data_dir +from monkey_island.cc.setup.version_file_setup import _version_filename current_version = "1.1.1" old_version = "1.1.0" @@ -36,7 +36,9 @@ def test_setup_data_dir(temp_data_dir_path, temp_version_file_path): assert version_file_path.read_text() == current_version -def test_old_version_present(temp_data_dir_path, temp_version_file_path): +def test_old_version_removed(monkeypatch, temp_data_dir_path, temp_version_file_path): + monkeypatch.setattr("builtins.input", lambda _: "y") + temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) bogus_file_path = temp_data_dir_path.joinpath("test.txt") @@ -46,25 +48,24 @@ def test_old_version_present(temp_data_dir_path, temp_version_file_path): assert temp_version_file_path.read_text() == current_version assert not bogus_file_path.is_file() - assert _get_backup_path(temp_data_dir_path).joinpath("test.txt").is_file() -def test_old_version_and_backup_present(temp_data_dir_path, temp_version_file_path): +@pytest.mark.parametrize("input_value", ["n", "x"]) +def test_old_version_not_removed( + monkeypatch, temp_data_dir_path, temp_version_file_path, input_value +): + monkeypatch.setattr("builtins.input", lambda _: input_value) + temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) - - old_backup_path = _get_backup_path(temp_data_dir_path) - old_backup_path.mkdir() - bogus_file_path = old_backup_path.joinpath("test.txt") + bogus_file_path = temp_data_dir_path.joinpath("test.txt") bogus_file_path.touch() - setup_data_dir(temp_data_dir_path) - new_backup_path = old_backup_path + with pytest.raises(IncompatibleDataDirectory): + setup_data_dir(temp_data_dir_path) - # Make sure old backup got deleted and new backup took it's place - assert temp_version_file_path.read_text() == current_version - assert get_version_from_dir(new_backup_path) == old_version - assert not _get_backup_path(temp_data_dir_path).joinpath("test.txt").is_file() + assert temp_version_file_path.read_text() == old_version + assert bogus_file_path.is_file() def test_data_dir_setup_not_needed(temp_data_dir_path, temp_version_file_path): From 9b005255f121e1b47ba0cb95c1d402f744a18b54 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 20 Oct 2021 13:30:11 -0400 Subject: [PATCH 15/16] Changelog: Update changelog for issue #1114 --- CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f2b95f8c..682d2094b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,9 +56,8 @@ Changelog](https://keepachangelog.com/en/1.0.0/). - ATT&CK report bug that said that the technique "`.bash_profile` and `.bashrc`" was not attempted when it actually was attempted but failed. #1511 - Bug that periodically cleared the telemetry table's filter. #1392 -- Updating the Infection Monkey will backup the old data -and will create a new data directory to prevent incompatibilities and crashes. #1114 - +- Crashes, stack traces, and other malfunctions when data from older versions of Infection Monkey is + present in the data directory. #1114 ### Security - Generate a random password when creating a new user for CommunicateAsNewUser From f17183a76a0f90d441f8e39c4cb0f230dc247d26 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 20 Oct 2021 13:48:19 -0400 Subject: [PATCH 16/16] Island: Replace joinpath() with / --- monkey/monkey_island/cc/setup/version_file_setup.py | 4 ++-- .../monkey_island/cc/setup/test_data_dir.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/monkey/monkey_island/cc/setup/version_file_setup.py b/monkey/monkey_island/cc/setup/version_file_setup.py index 380fcf4cc..1489ca38d 100644 --- a/monkey/monkey_island/cc/setup/version_file_setup.py +++ b/monkey/monkey_island/cc/setup/version_file_setup.py @@ -6,10 +6,10 @@ _version_filename = "VERSION" def get_version_from_dir(dir_path: Path) -> str: - version_file_path = dir_path.joinpath(_version_filename) + version_file_path = dir_path / _version_filename return version_file_path.read_text() def write_version(dir_path: Path): - version_file_path = dir_path.joinpath(_version_filename) + version_file_path = dir_path / _version_filename version_file_path.write_text(get_version()) diff --git a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py index 696b604dc..fe60d227d 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py +++ b/monkey/tests/unit_tests/monkey_island/cc/setup/test_data_dir.py @@ -18,13 +18,13 @@ def mock_version(monkeypatch): @pytest.fixture -def temp_data_dir_path(tmpdir) -> Path: - return Path(tmpdir, "data_dir") +def temp_data_dir_path(tmp_path) -> Path: + return tmp_path / "data_dir" @pytest.fixture def temp_version_file_path(temp_data_dir_path) -> Path: - return temp_data_dir_path.joinpath(_version_filename) + return temp_data_dir_path / _version_filename def test_setup_data_dir(temp_data_dir_path, temp_version_file_path): @@ -41,7 +41,7 @@ def test_old_version_removed(monkeypatch, temp_data_dir_path, temp_version_file_ temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) - bogus_file_path = temp_data_dir_path.joinpath("test.txt") + bogus_file_path = temp_data_dir_path / "test.txt" bogus_file_path.touch() setup_data_dir(temp_data_dir_path) @@ -58,7 +58,7 @@ def test_old_version_not_removed( temp_data_dir_path.mkdir() temp_version_file_path.write_text(old_version) - bogus_file_path = temp_data_dir_path.joinpath("test.txt") + bogus_file_path = temp_data_dir_path / "test.txt" bogus_file_path.touch() with pytest.raises(IncompatibleDataDirectory): @@ -71,7 +71,7 @@ def test_old_version_not_removed( def test_data_dir_setup_not_needed(temp_data_dir_path, temp_version_file_path): temp_data_dir_path.mkdir() temp_version_file_path.write_text(current_version) - bogus_file_path = temp_data_dir_path.joinpath("test.txt") + bogus_file_path = temp_data_dir_path / "test.txt" bogus_file_path.touch() setup_data_dir(temp_data_dir_path)