diff --git a/mssql_python/logging.py b/mssql_python/logging.py index a16bf773..2130f3a8 100644 --- a/mssql_python/logging.py +++ b/mssql_python/logging.py @@ -273,17 +273,44 @@ def _cleanup_handlers(self): except: pass - def _validate_log_file_extension(self, file_path: str) -> None: + def _validate_log_file_path(self, file_path: str) -> str: """ - Validate that the log file has an allowed extension. + Validate and sanitize the log file path. + + Resolves the path to its canonical form, checks for path traversal + outside the current working directory (for relative paths), and + validates the file extension. Args: file_path: Path to the log file + Returns: + The resolved canonical path. + Raises: - ValueError: If the file extension is not allowed + ValueError: If the path contains traversal outside the allowed + base directory, or if the file extension is not allowed """ - _, ext = os.path.splitext(file_path) + resolved = os.path.realpath(os.path.abspath(file_path)) + + # For relative paths, ensure the resolved path stays under cwd + if not os.path.isabs(file_path): + base = os.path.realpath(os.path.abspath(os.getcwd())) + # os.path.commonpath raises ValueError if paths are on different drives + try: + common = os.path.commonpath([base, resolved]) + except ValueError: + raise ValueError( + "log_file_path resolves outside the current working directory. " + "Path traversal is not permitted for relative paths." + ) + if common != base: + raise ValueError( + "log_file_path resolves outside the current working directory. " + "Path traversal is not permitted for relative paths." + ) + + _, ext = os.path.splitext(resolved) ext_lower = ext.lower() if ext_lower not in ALLOWED_LOG_EXTENSIONS: @@ -292,6 +319,8 @@ def _validate_log_file_extension(self, file_path: str) -> None: f"Invalid log file extension '{ext}'. " f"Allowed extensions: {allowed}" ) + return resolved + def _write_log_header(self): """ Write CSV header and metadata to the log file. @@ -483,10 +512,9 @@ def _setLevel( ) self._output_mode = output - # Store custom log file path if provided + # Store custom log file path if provided (sanitized) if log_file_path is not None: - self._validate_log_file_extension(log_file_path) - self._custom_log_path = log_file_path + self._custom_log_path = self._validate_log_file_path(log_file_path) # Setup handlers if not yet initialized or if output mode/path changed # Handler setup is protected by _handler_lock inside _setup_handlers() diff --git a/tests/test_007_logging.py b/tests/test_007_logging.py index daafc6fc..cb6eefd5 100644 --- a/tests/test_007_logging.py +++ b/tests/test_007_logging.py @@ -150,8 +150,9 @@ def test_custom_log_file_path(self, cleanup_logger, temp_log_dir): setup_logging(log_file_path=custom_path) logger.debug("Test message") - assert logger.log_file == custom_path - assert os.path.exists(custom_path) + resolved = os.path.realpath(os.path.abspath(custom_path)) + assert logger.log_file == resolved + assert os.path.exists(resolved) def test_custom_log_file_path_creates_directory(self, cleanup_logger, temp_log_dir): """Custom log file path should create parent directories""" @@ -179,6 +180,36 @@ def test_log_file_extension_validation_invalid(self, cleanup_logger, temp_log_di with pytest.raises(ValueError, match="Invalid log file extension"): setup_logging(log_file_path=custom_path) + def test_path_traversal_rejected(self, cleanup_logger, monkeypatch, tmp_path): + """Relative paths with traversal outside a controlled cwd should be rejected""" + monkeypatch.chdir(tmp_path) + with pytest.raises(ValueError, match="Path traversal is not permitted"): + setup_logging(log_file_path="../evil.log") + + def test_path_traversal_dot_dot_rejected(self, cleanup_logger, monkeypatch, tmp_path): + """Relative paths using ../ to escape a controlled cwd should be rejected""" + monkeypatch.chdir(tmp_path) + with pytest.raises(ValueError, match="Path traversal is not permitted"): + setup_logging(log_file_path="../nested/evil.log") + + def test_path_traversal_different_drive(self, cleanup_logger, monkeypatch, tmp_path): + """commonpath ValueError (e.g. different drives on Windows) should be rejected""" + monkeypatch.chdir(tmp_path) + + def mock_commonpath(paths): + raise ValueError("Can't mix absolute and relative paths") + + monkeypatch.setattr(os.path, "commonpath", mock_commonpath) + with pytest.raises(ValueError, match="Path traversal is not permitted"): + setup_logging(log_file_path="subdir/test.log") + + def test_absolute_path_allowed(self, cleanup_logger, temp_log_dir): + """Absolute paths should be allowed (developer-controlled)""" + custom_path = os.path.join(temp_log_dir, "absolute_test.log") + setup_logging(log_file_path=custom_path) + logger.debug("Test message") + assert os.path.exists(os.path.realpath(os.path.abspath(custom_path))) + class TestCSVFormat: """Test CSV output format"""