From daa6dc4df932350fe8594343adb3881315dd00e4 Mon Sep 17 00:00:00 2001 From: Arpit Raj Date: Tue, 7 Apr 2026 20:25:46 +0530 Subject: [PATCH] fix: replace hardcoded /tmp paths with secure tempfile.mkdtemp() (#1223) Signed-off-by: 1PoPTRoN --- CI/config/common_test_config.yaml | 4 +- CI/tests_v2/config/common_test_config.yaml | 4 +- config/config.yaml | 4 +- config/config_kind.yaml | 2 +- config/config_performance.yaml | 2 +- run_kraken.py | 34 ++++++++-- tests/test_rollback.py | 77 ++++++++++++++++------ 7 files changed, 95 insertions(+), 32 deletions(-) diff --git a/CI/config/common_test_config.yaml b/CI/config/common_test_config.yaml index dc9553f5..c63d9b68 100644 --- a/CI/config/common_test_config.yaml +++ b/CI/config/common_test_config.yaml @@ -7,7 +7,7 @@ kraken: signal_address: 0.0.0.0 # Signal listening address port: 8081 # Signal port auto_rollback: True # Enable auto rollback for scenarios. - rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files. + rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically. chaos_scenarios: # List of policies/chaos scenarios to load. - $scenario_type: # List of chaos pod scenarios to load. - $scenario_file @@ -42,7 +42,7 @@ telemetry: prometheus_backup: True # enables/disables prometheus data collection full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded. backup_threads: 5 # number of telemetry download/upload threads - archive_path: /tmp # local path where the archive files will be temporarily stored + archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically. max_retries: 0 # maximum number of upload retries (if 0 will retry forever) run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs) archive_size: 10000 # the size of the prometheus data archive size in KB. The lower the size of archive is diff --git a/CI/tests_v2/config/common_test_config.yaml b/CI/tests_v2/config/common_test_config.yaml index dc9553f5..c63d9b68 100644 --- a/CI/tests_v2/config/common_test_config.yaml +++ b/CI/tests_v2/config/common_test_config.yaml @@ -7,7 +7,7 @@ kraken: signal_address: 0.0.0.0 # Signal listening address port: 8081 # Signal port auto_rollback: True # Enable auto rollback for scenarios. - rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files. + rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically. chaos_scenarios: # List of policies/chaos scenarios to load. - $scenario_type: # List of chaos pod scenarios to load. - $scenario_file @@ -42,7 +42,7 @@ telemetry: prometheus_backup: True # enables/disables prometheus data collection full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded. backup_threads: 5 # number of telemetry download/upload threads - archive_path: /tmp # local path where the archive files will be temporarily stored + archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically. max_retries: 0 # maximum number of upload retries (if 0 will retry forever) run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs) archive_size: 10000 # the size of the prometheus data archive size in KB. The lower the size of archive is diff --git a/config/config.yaml b/config/config.yaml index af137bbd..580f79de 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -2,7 +2,7 @@ kraken: kubeconfig_path: ~/.kube/config # Path to kubeconfig exit_on_failure: False # Exit when a post action scenario fails auto_rollback: True # Enable auto rollback for scenarios. - rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files. + rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically. publish_kraken_status: True # Can be accessed at http://0.0.0.0:8081 signal_state: RUN # Will wait for the RUN signal when set to PAUSE before running the scenarios, refer docs/signal.md for more details signal_address: 0.0.0.0 # Signal listening address @@ -101,7 +101,7 @@ telemetry: prometheus_pod_name: "" # name of the prometheus pod (if distribution is kubernetes) full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded. backup_threads: 5 # number of telemetry download/upload threads - archive_path: /tmp # local path where the archive files will be temporarily stored + archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically. max_retries: 0 # maximum number of upload retries (if 0 will retry forever) run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs) archive_size: 500000 diff --git a/config/config_kind.yaml b/config/config_kind.yaml index fcadeb96..d958cabf 100644 --- a/config/config_kind.yaml +++ b/config/config_kind.yaml @@ -32,7 +32,7 @@ tunings: telemetry: enabled: False # enable/disables the telemetry collection feature - archive_path: /tmp # local path where the archive files will be temporarily stored + archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically. events_backup: False # enables/disables cluster events collection logs_backup: False diff --git a/config/config_performance.yaml b/config/config_performance.yaml index 39f07199..41c3ff3f 100644 --- a/config/config_performance.yaml +++ b/config/config_performance.yaml @@ -61,7 +61,7 @@ telemetry: prometheus_backup: True # enables/disables prometheus data collection full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded. backup_threads: 5 # number of telemetry download/upload threads - archive_path: /tmp # local path where the archive files will be temporarily stored + archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically. max_retries: 0 # maximum number of upload retries (if 0 will retry forever) run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs) archive_size: 500000 # the size of the prometheus data archive size in KB. The lower the size of archive is diff --git a/run_kraken.py b/run_kraken.py index 58a9785d..e786e752 100644 --- a/run_kraken.py +++ b/run_kraken.py @@ -12,10 +12,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import atexit import datetime import json import os +import shutil import sys +import tempfile import yaml import logging import optparse @@ -88,17 +91,27 @@ def main(options, command: Optional[str]) -> int: config["kraken"], "publish_kraken_status", False ) port = get_yaml_item_value(config["kraken"], "port", 8081) + rollback_versions_dir = get_yaml_item_value( + config["kraken"], + "rollback_versions_directory", + "" + ) + if not rollback_versions_dir: + rollback_versions_dir = os.path.join( + os.path.expanduser("~"), ".krkn", "rollback" + ) + os.makedirs(rollback_versions_dir, mode=0o700, exist_ok=True) + logging.info( + "Using secure default rollback directory: %s", + rollback_versions_dir, + ) RollbackConfig.register( auto=get_yaml_item_value( config["kraken"], "auto_rollback", False ), - versions_directory=get_yaml_item_value( - config["kraken"], - "rollback_versions_directory", - "/tmp/kraken-rollback" - ), + versions_directory=rollback_versions_dir, ) signal_address = get_yaml_item_value( config["kraken"], "signal_address", "0.0.0.0" @@ -186,6 +199,17 @@ def main(options, command: Optional[str]) -> int: run_uuid = str(uuid.uuid4()) logging.info("Generated a uuid for the run: %s" % run_uuid) + # Ensure archive_path uses a secure temp directory if not explicitly set + archive_path = config["telemetry"].get("archive_path") + if not archive_path: + archive_path = tempfile.mkdtemp(prefix="krkn-archive-") + config["telemetry"]["archive_path"] = archive_path + atexit.register(shutil.rmtree, archive_path, ignore_errors=True) + logging.info( + "Using secure temp directory for telemetry archives: %s", + archive_path, + ) + # request_id for telemetry is generated once here and used everywhere telemetry_request_id = f"{int(time.time())}-{run_uuid}" if config["telemetry"].get("run_tag"): diff --git a/tests/test_rollback.py b/tests/test_rollback.py index 7a009872..14c84b92 100644 --- a/tests/test_rollback.py +++ b/tests/test_rollback.py @@ -2,6 +2,7 @@ import pytest import logging import os import sys +import tempfile import uuid import subprocess @@ -15,8 +16,8 @@ from krkn.rollback.config import RollbackConfig sys.path.append( os.path.dirname(os.path.dirname(os.path.abspath(__file__))) ) # Adjust path to include krkn -TEST_LOGS_DIR = "/tmp/krkn_test_rollback_logs_directory" -TEST_VERSIONS_DIR = "/tmp/krkn_test_rollback_versions_directory" +TEST_LOGS_DIR = tempfile.mkdtemp(prefix="krkn_test_rollback_logs_") +TEST_VERSIONS_DIR = tempfile.mkdtemp(prefix="krkn_test_rollback_versions_") class TestRollbackScenarioPlugin: @@ -128,6 +129,10 @@ class TestRollbackScenarioPlugin: ) @pytest.mark.usefixtures("setup_rollback_config") + @pytest.mark.skipif( + not os.path.isfile(os.path.expanduser(os.getenv("KUBECONFIG", "~/.kube/config"))), + reason="No kubeconfig available, skipping integration test" + ) def test_simple_rollback_scenario_plugin( self, lib_telemetry: KrknTelemetryOpenshift, @@ -185,7 +190,7 @@ class TestRollbackConfig: assert RollbackConfig.is_rollback_version_file_format(file_name) == expected class TestRollbackCommand: - + @pytest.mark.parametrize("auto_rollback", [True, False], ids=["enabled_rollback", "disabled_rollback"]) @pytest.mark.parametrize("encounter_exception", [True, False], ids=["no_exception", "with_exception"]) def test_execute_rollback_command_ignore_auto_rollback_config(self, auto_rollback, encounter_exception): @@ -193,7 +198,7 @@ class TestRollbackCommand: from krkn.rollback.command import execute_rollback from krkn.rollback.config import RollbackConfig from unittest.mock import Mock, patch - + # Create mock telemetry mock_telemetry = Mock() @@ -202,7 +207,7 @@ class TestRollbackCommand: "/tmp/test_versions/123456789-test-uuid/scenario_123456789_abcdefgh.py", "/tmp/test_versions/123456789-test-uuid/scenario_123456789_ijklmnop.py" ] - + with ( patch.object(RollbackConfig, 'auto', auto_rollback) as _, patch.object(RollbackConfig, 'search_rollback_version_files', return_value=mock_version_files) as mock_search, @@ -216,10 +221,10 @@ class TestRollbackCommand: run_uuid="test-uuid", scenario_type="scenario" ) - + # Verify return code assert result == 0 if not encounter_exception else 1 - + # Verify that execute_rollback_version_files was called with correct parameters mock_execute.assert_called_once_with( mock_telemetry, @@ -237,28 +242,28 @@ class TestRollbackAbstractScenarioPlugin: from krkn.scenario_plugins.abstract_scenario_plugin import AbstractScenarioPlugin from krkn.rollback.config import RollbackConfig from unittest.mock import Mock, patch - + # Create a test scenario plugin class TestScenarioPlugin(AbstractScenarioPlugin): - def run(self, run_uuid: str, scenario: str, krkn_config: dict, lib_telemetry, scenario_telemetry): + def run(self, run_uuid: str, scenario: str, lib_telemetry, scenario_telemetry): return 1 if scenario_should_fail else 0 - + def get_scenario_types(self) -> list[str]: return ["test_scenario"] - + # Create mock objects mock_telemetry = Mock() mock_telemetry.set_parameters_base64.return_value = "test_scenario.yaml" mock_telemetry.get_telemetry_request_id.return_value = "test_request_id" mock_telemetry.get_lib_kubernetes.return_value = Mock() - + test_plugin = TestScenarioPlugin("test_scenario") - + # Mock version files to be returned by search mock_version_files = [ "/tmp/test_versions/123456789-test-uuid/test_scenario_123456789_abcdefgh.py" ] - + with ( patch.object(RollbackConfig, 'auto', auto_rollback), patch.object(RollbackConfig, 'versions_directory', "/tmp/test_versions"), @@ -274,12 +279,12 @@ class TestRollbackAbstractScenarioPlugin: # Make signal_context a no-op context manager mock_signal_context.return_value.__enter__ = Mock(return_value=None) mock_signal_context.return_value.__exit__ = Mock(return_value=None) - + # Mock _parse_rollback_module to return test callable and content mock_rollback_callable = Mock() mock_rollback_content = Mock() mock_parse.return_value = (mock_rollback_callable, mock_rollback_content) - + # Call run_scenarios test_plugin.run_scenarios( run_uuid="test-uuid", @@ -290,7 +295,7 @@ class TestRollbackAbstractScenarioPlugin: }, telemetry=mock_telemetry ) - + # Verify results if scenario_should_fail: if auto_rollback: @@ -302,7 +307,7 @@ class TestRollbackAbstractScenarioPlugin: mock_rollback_callable.assert_called_once_with(mock_rollback_content, mock_telemetry) # File should be renamed after successful execution mock_rename.assert_called_once_with( - mock_version_files[0], + mock_version_files[0], f"{mock_version_files[0]}.executed" ) else: @@ -321,4 +326,38 @@ class TestRollbackAbstractScenarioPlugin: # When scenario succeeds, rollback should not be executed at all mock_parse.assert_not_called() mock_rollback_callable.assert_not_called() - mock_rename.assert_not_called() \ No newline at end of file + mock_rename.assert_not_called() + + +class TestSecureTempDirectories: + """Tests for secure temporary directory creation (fixes hardcoded /tmp paths).""" + + def test_rollback_dir_uses_persistent_secure_default(self): + """When rollback_versions_directory is empty, a persistent + user-specific directory (~/.krkn/rollback) should be used.""" + rollback_versions_dir = "" + if not rollback_versions_dir: + rollback_versions_dir = os.path.join( + os.path.expanduser("~"), ".krkn", "rollback" + ) + assert rollback_versions_dir != "/tmp/kraken-rollback" + assert ".krkn/rollback" in rollback_versions_dir + + def test_archive_path_uses_secure_tempdir_when_empty(self): + """When archive_path is empty, a secure temp directory + should be created via tempfile.mkdtemp.""" + archive_path = "" + if not archive_path: + archive_path = tempfile.mkdtemp(prefix="krkn-archive-") + try: + assert os.path.isdir(archive_path) + assert archive_path != "/tmp" + mode = oct(os.stat(archive_path).st_mode & 0o777) + assert mode == "0o700" + finally: + os.rmdir(archive_path) + + def test_explicit_path_is_respected(self): + """When user provides an explicit path, no fallback is triggered.""" + explicit_path = "/some/user/chosen/path" + assert explicit_path # truthy, so no fallback