From f22937a858c65c33ae0766789e89734214dd667e Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 29 Jul 2026 11:04:52 -0400 Subject: [PATCH] Add ownership check for loading project config files Signed-off-by: Timothy Johnson --- CHANGELOG.md | 1 + .../zowe/core_for_zowe_sdk/config_file.py | 87 ++++++++++++++++++- tests/unit/core/test_profile_manager.py | 38 ++++++-- 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d3c2d5..1a139b60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to the Zowe Client Python SDK will be documented in this fil ### Enhancements - Allowed the profile manager to only validate schemas at the project level with the new `validate_only_project_config` parameter. [#393](https://github.com/zowe/zowe-client-python-sdk/pull/393) +- Updated the `ConfigFile.autodiscover_config_dir` method to refuse loading a potentially untrusted config file whose parent directory is not owned by the current user. This check can be disabled by calling `trust_all_directories(True)`. ### Bug Fixes diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 82bc14d4..fd106fcb 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -10,9 +10,12 @@ Copyright Contributors to the Zowe Project. """ +import getpass import json import os.path import re +import subprocess +import sys import warnings from copy import deepcopy from dataclasses import dataclass, field @@ -74,7 +77,8 @@ class ConfigFile: jsonc: Optional[dict[str, Any]] = None _missing_secure_props: list[str] = field(default_factory=list) - __suppress_config_file_warnings: Optional[bool] = True + __suppress_config_file_warnings: bool = True + __trust_all_directories: bool = False __logger = Log.register_logger(__name__) @property @@ -277,16 +281,78 @@ def get_profile( return Profile(props, profile_name, self._missing_secure_props) + @staticmethod + def __is_owned_by_current_user(path: str) -> bool: + """ + Check whether the given file or directory is owned by the current user. + + This mirrors the ownership check behind Git's `safe.directory` protection + (CVE-2022-24765): only ownership is verified, not permission bits, since an + owner can freely change permissions at will, so mode bits say nothing about + whether a different, non-owning user could have tampered with the path. + + Parameters + ---------- + path: str + The file or directory to check + + Returns + ------- + bool + True if the path is owned by the current user + """ + if not os.path.exists(path): + return False + + try: + if sys.platform == "win32": + # Passing path as its own argv element (rather than interpolating it into + # the -Command string) keeps it out of PowerShell's parser entirely, so paths + # containing quotes or other special characters can't affect the script. + # -Command already ignores the local execution policy (it only restricts + # running saved .ps1 script files, not inline commands), so no + # -ExecutionPolicy override is needed here. + result = subprocess.run( + [ + "powershell", + "-NoProfile", + "-Command", + "& {param($p) (Get-Acl -LiteralPath $p).Owner}", + path, + ], + capture_output=True, + text=True, + check=False, + ) + identity = result.stdout.strip().lower() + if not identity: + return False + + username = getpass.getuser().lower() + return identity == username or identity.endswith(f"\\{username}") + else: + return os.stat(path).st_uid == os.getuid() + except OSError: + return False + def autodiscover_config_dir(self) -> None: """ Autodiscover Zowe z/OSMF Team Config files by going up the path from current working directory. Sets path if it finds the config directory, Else, it raises an Exception. + To prevent loading a config file placed by another user in a shared directory + (e.g. /tmp) that happens to be an ancestor of the current working directory, the directory + containing the config file must be owned by the current user. This check can be disabled + by calling `trust_all_directories(True)`. + Raises ------ FileNotFoundError Cannot find file in directory. + PermissionError + Found a config file, but its directory is not owned by the current user + and directory trust has not been enabled via `trust_all_directories`. """ current_dir = CURRENT_DIR @@ -294,6 +360,14 @@ def autodiscover_config_dir(self) -> None: path = os.path.join(current_dir, self.filename) if os.path.isfile(path): + if not self.__trust_all_directories and not self.__is_owned_by_current_user(current_dir): + self.__logger.error( + f"Refusing to load config file {path}: {current_dir} is not owned by the current user" + ) + raise PermissionError( + f"Found config file at {path}, but the directory {current_dir} is not owned by " + "the current user. Call trust_all_directories(True) to bypass this check." + ) self.location = current_dir return @@ -673,3 +747,14 @@ def suppress_config_warnings(self, value: bool) -> None: Warnings are shown or not """ self.__suppress_config_file_warnings = value + + def trust_all_directories(self, value: bool) -> None: + """ + Disable (or re-enable) the directory ownership check performed by autodiscover_config_dir. + + Parameters + ---------- + value: bool + If True, a discovered config file is loaded regardless of who owns its directory + """ + self.__trust_all_directories = value diff --git a/tests/unit/core/test_profile_manager.py b/tests/unit/core/test_profile_manager.py index 1e797a43..2b1da259 100644 --- a/tests/unit/core/test_profile_manager.py +++ b/tests/unit/core/test_profile_manager.py @@ -113,6 +113,30 @@ def test_autodiscovery_and_base_profile_loading(self, get_pass_func): } self.assertEqual(props, expected_props) + def test_autodiscover_config_dir_untrusted_directory(self): + """Test that autodiscover_config_dir raises PermissionError when the directory is not owned by the current user.""" + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + shutil.copy(self.original_file_path, cwd_up_file_path) + + config_file = ConfigFile(type="team_config", name="zowe") + with mock.patch("os.getuid", return_value=os.getuid() + 1): + with self.assertRaises(PermissionError): + config_file.autodiscover_config_dir() + + def test_autodiscover_config_dir_untrusted_directory_bypassed(self): + """Test that trust_all_directories(True) allows loading a config from a directory not owned by the current user.""" + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + shutil.copy(self.original_file_path, cwd_up_file_path) + + config_file = ConfigFile(type="team_config", name="zowe") + config_file.trust_all_directories(True) + with mock.patch("os.getuid", return_value=os.getuid() + 1): + config_file.autodiscover_config_dir() + + self.assertEqual(config_file.location, cwd_up_dir_path) + @mock.patch("zowe.secrets_for_zowe_sdk.keyring.get_password", side_effect=keyring_get_password) def test_custom_file_and_custom_profile_loading(self, get_pass_func): """ @@ -825,19 +849,17 @@ def test_config_file_save(self, mock_save_secure_props): ["port"], list(config_file.jsonc["profiles"]["lpar1"]["profiles"]["zosmf"]["properties"].keys()) ) + def test_find_profile_with_non_dict_value(): """Test what happens when a non-dict value is passed to find_profile.""" config_file = ConfigFile(type="Team Config", name="test") - - profiles = { - "my_profile": False, - "valid_profile": {"type": "zosmf", "properties": {"host": "example.com"}} - } - + + profiles = {"my_profile": False, "valid_profile": {"type": "zosmf", "properties": {"host": "example.com"}}} + result = config_file.find_profile("my_profile", profiles) - + assert result is None - + valid_result = config_file.find_profile("valid_profile", profiles) assert valid_result is not None assert valid_result["type"] == "zosmf"