From c97cd55c31469ac3130cdbbef2edf9b99b70ab61 Mon Sep 17 00:00:00 2001 From: Patryk Czajka Date: Tue, 27 Aug 2024 14:50:52 +0200 Subject: [PATCH] config is created with strict permissions --- src/snowflake/cli/api/secure_path.py | 20 ++---------------- src/snowflake/cli/api/secure_utils.py | 30 +++++++++++++++++++++++++++ tests/test_config.py | 1 - 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/snowflake/cli/api/secure_path.py b/src/snowflake/cli/api/secure_path.py index 1edceebde3..97ed53e1ea 100644 --- a/src/snowflake/cli/api/secure_path.py +++ b/src/snowflake/cli/api/secure_path.py @@ -24,6 +24,7 @@ from typing import Optional, Union from snowflake.cli.api.exceptions import DirectoryIsNotEmptyError, FileTooLargeError +from snowflake.cli.api.secure_utils import restrict_file_permissions log = logging.getLogger(__name__) @@ -97,28 +98,11 @@ def name(self) -> str: """A string representing the final path component.""" return self._path.name - def chmod(self, permissions_mask: int) -> None: - """ - Change the file mode and permissions, like os.chmod(). - """ - log.info( - "Update permissions of file %s to %s", self._path, oct(permissions_mask) - ) - self._path.chmod(permissions_mask) - def restrict_permissions(self) -> None: """ Restrict file/directory permissions to owner-only. """ - import stat - - owner_permissions = ( - # https://docs.python.org/3/library/stat.html - stat.S_IRUSR # readable by owner - | stat.S_IWUSR # writeable by owner - | stat.S_IXUSR # executable by owner - ) - self.chmod(self._path.stat().st_mode & owner_permissions) + restrict_file_permissions(self._path) def touch(self, permissions_mask: int = 0o600, exist_ok: bool = True) -> None: """ diff --git a/src/snowflake/cli/api/secure_utils.py b/src/snowflake/cli/api/secure_utils.py index 4cd3ced202..4e93ce3fd6 100644 --- a/src/snowflake/cli/api/secure_utils.py +++ b/src/snowflake/cli/api/secure_utils.py @@ -12,12 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging import stat from pathlib import Path from typing import List from snowflake.connector.compat import IS_WINDOWS +log = logging.getLogger(__name__) + def _get_windows_whitelisted_users(): # whitelisted users list obtained in consultation with prodsec: CASEC-9627 @@ -81,3 +84,30 @@ def file_permissions_are_strict(file_path: Path) -> bool: if IS_WINDOWS: return _windows_file_permissions_are_strict(file_path) return _unix_file_permissions_are_strict(file_path) + + +def _unix_restrict_file_permissions(path: Path) -> None: + owner_permissions = ( + # https://docs.python.org/3/library/stat.html + stat.S_IRUSR # readable by owner + | stat.S_IWUSR # writeable by owner + | stat.S_IXUSR # executable by owner + ) + target_permissions = path.stat().st_mode & owner_permissions + log.info("Update permissions of file %s to %s", path, oct(target_permissions)) + path.chmod(target_permissions) + + +def _windows_restrict_file_permissions(path: Path) -> None: + import subprocess + + for user in windows_get_not_whitelisted_users_with_access(path): + log.info("Removing permissions of user %s from file %s", user, path) + subprocess.run(["icacls", str(path), "/DENY", f"{user}:F"]) + + +def restrict_file_permissions(file_path: Path) -> None: + if IS_WINDOWS: + _windows_restrict_file_permissions(file_path) + else: + _unix_restrict_file_permissions(file_path) diff --git a/tests/test_config.py b/tests/test_config.py index 0e1390ed42..65da5e2c27 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -31,7 +31,6 @@ from tests_common import IS_WINDOWS -@pytest.mark.skipif(condition=IS_WINDOWS, reason="Does not work on Windows") def test_empty_config_file_is_created_if_not_present(): with TemporaryDirectory() as tmp_dir: config_file = Path(tmp_dir) / "sub" / "config.toml"