From a50620a5460ec177c10797251a427bed5c11d513 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Thu, 9 Apr 2026 15:40:50 +1000 Subject: [PATCH 1/2] fix --- .../azure/cli/command_modules/acs/custom.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 9854f9620de..4039aa0a9ca 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2050,10 +2050,26 @@ def merge_kubernetes_configurations(existing_file, addition_file, replace, conte existing_file_perms, ) + # Refuse to write through a symlink + if os.path.islink(existing_file): + raise CLIError( + 'Kubeconfig path "{}" is a symbolic link. ' + 'Refusing to write to prevent symlink-following attacks.'.format(existing_file) + ) + + # Atomic write: write to a temp file in the same directory, then replace + parent_dir = os.path.dirname(existing_file) or '.' + tmp_fd, tmp_path = tempfile.mkstemp(dir=parent_dir) try: - with open(existing_file, 'w+') as stream: + with os.fdopen(tmp_fd, 'w') as stream: yaml.safe_dump(existing, stream, default_flow_style=False) + os.chmod(tmp_path, 0o600) + os.replace(tmp_path, existing_file) except OSError as ex: + try: + os.unlink(tmp_path) + except OSError: + pass if getattr(ex, 'errno', 0) in (errno.EACCES, errno.EPERM, errno.EROFS): raise FileOperationError( 'Permission denied when trying to write to {}. ' From 71e171dcc8e47a8fb7b6fdf3dcb7bed58ab82f43 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Thu, 9 Apr 2026 15:56:56 +1000 Subject: [PATCH 2/2] fix --- .../azure/cli/command_modules/acs/custom.py | 11 ++++-- .../acs/tests/latest/test_custom.py | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 4039aa0a9ca..3700a77786a 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2063,14 +2063,19 @@ def merge_kubernetes_configurations(existing_file, addition_file, replace, conte try: with os.fdopen(tmp_fd, 'w') as stream: yaml.safe_dump(existing, stream, default_flow_style=False) - os.chmod(tmp_path, 0o600) + # Preserve existing file permissions if available, otherwise default to 0600 + if os.path.exists(existing_file): + existing_mode = stat.S_IMODE(os.stat(existing_file).st_mode) + os.chmod(tmp_path, existing_mode) + else: + os.chmod(tmp_path, 0o600) os.replace(tmp_path, existing_file) - except OSError as ex: + except Exception as ex: # pylint: disable=broad-except try: os.unlink(tmp_path) except OSError: pass - if getattr(ex, 'errno', 0) in (errno.EACCES, errno.EPERM, errno.EROFS): + if isinstance(ex, OSError) and getattr(ex, 'errno', 0) in (errno.EACCES, errno.EPERM, errno.EROFS): raise FileOperationError( 'Permission denied when trying to write to {}. ' 'Please ensure you have write access to this file, or specify a different file path ' diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 4844bb7ca3c..fe40396e062 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -484,6 +484,41 @@ def test_merge_credentials_already_present(self): self.assertEqual(merged['users'], expected_users) self.assertEqual(merged['current-context'], obj2['current-context']) + @unittest.skipIf(os.name == 'nt', 'Symlink test not applicable on Windows') + def test_merge_credentials_rejects_symlink(self): + # Create a real kubeconfig file and a symlink pointing to it + target = tempfile.NamedTemporaryFile(delete=False, suffix='.kubeconfig') + target.close() + with open(target.name, 'w') as f: + yaml.safe_dump({'clusters': [], 'contexts': [], 'users': [], + 'current-context': '', 'kind': 'Config'}, f) + self.addCleanup(os.remove, target.name) + + symlink_path = target.name + '.link' + os.symlink(target.name, symlink_path) + self.addCleanup(lambda: os.remove(symlink_path) if os.path.islink(symlink_path) else None) + + addition = tempfile.NamedTemporaryFile(delete=False) + addition.close() + obj = { + 'clusters': [{'cluster': {'server': 'https://test'}, 'name': 'c1'}], + 'contexts': [{'context': {'cluster': 'c1', 'user': 'u1'}, 'name': 'ctx1'}], + 'users': [{'name': 'u1', 'user': {'token': 'tok'}}], + 'current-context': 'ctx1', + } + with open(addition.name, 'w') as f: + yaml.safe_dump(obj, f) + self.addCleanup(os.remove, addition.name) + + # Should raise CLIError when existing_file is a symlink + with self.assertRaises(CLIError): + merge_kubernetes_configurations(symlink_path, addition.name, False) + + # Verify the symlink target was not modified + with open(target.name, 'r') as f: + content = yaml.safe_load(f) + self.assertEqual(content['clusters'], []) + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True) @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resources_client', autospec=True)