From 61c5bc82ff136c23aa3fad54912541f6694a9bc6 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 12:56:56 -0700 Subject: [PATCH 1/6] Add ownership checks for storage directories --- .../CHANGELOG.md | 1 + .../opentelemetry/exporter/_storage.py | 20 +- .../tests/test_storage.py | 282 +++++++++++++++--- 3 files changed, 263 insertions(+), 40 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md b/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md index 881c8931ee44..235f64b51290 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md @@ -3,6 +3,7 @@ ## 1.0.0b52 (Unreleased) ### Features Added +- Add ownership checks for storage directories - Add logger name to custom dimensions for Message, Exception and Event telemetry ([#46096](https://github.com/Azure/azure-sdk-for-python/pull/46096)) - Add support for populating SDK version from distro and Microsoft OpenTelemetry distro environment variables diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py index 0f2ac4ab8bf0..9176d37af1ff 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py @@ -66,7 +66,10 @@ def get(self) -> Optional[Tuple[Any, ...]]: def put(self, data: List[Any], lease_period: int = 0) -> Union[StorageExportResult, str]: try: fullpath = self.fullpath + ".tmp" - with open(fullpath, "w", encoding="utf-8") as file: + # Use O_CREAT | O_EXCL | O_WRONLY to atomically create the file + # and fail if it already exists, preventing race conditions. + fd = os.open(fullpath, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as file: for item in data: file.write(json.dumps(item)) # The official Python doc: Do not use os.linesep as a line @@ -255,6 +258,21 @@ def _check_and_set_folder_permissions(self) -> bool: return True # Unix else: + dir_stat = os.lstat(self._path) + owner_uid = dir_stat.st_uid + current_uid = os.getuid() # pylint: disable=no-member + if owner_uid not in (current_uid, 0): + logger.error( + "Storage directory %s is owned by uid %d, not the current user (%d) or admin (uid 0). " + "Refusing to use this directory.", + self._path, + owner_uid, + current_uid, + ) + set_local_storage_setup_state_exception( + f"Directory owned by uid {owner_uid}, expected {current_uid} or 0" + ) + return False os.chmod(self._path, 0o700) return True except OSError as error: diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py index 141bc672353f..f12bead85441 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py @@ -83,7 +83,7 @@ def test_put_file_write_error_returns_string(self): blob = LocalFileBlob(os.path.join(TEST_FOLDER, "write_error_blob")) test_input = [1, 2, 3] - with mock.patch("builtins.open", side_effect=PermissionError("Cannot write to file")): + with mock.patch("os.open", side_effect=PermissionError("Cannot write to file")): result = blob.put(test_input) self.assertIsInstance(result, str) self.assertIn("Cannot write to file", result) @@ -619,29 +619,34 @@ def test_check_and_set_folder_permissions_unix_chmod_exception_sets_exception_st # Clear any existing exception state (set to empty string, not None) set_local_storage_setup_state_exception("") + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 1000 + # Mock Unix environment and chmod failure with mock.patch("os.name", "posix"): # Unix with mock.patch("os.makedirs"): # Allow directory creation - with mock.patch("os.chmod", side_effect=OSError(test_error_message)): - stor = LocalFileStorage(os.path.join(TEST_FOLDER, "chmod_failure_test")) + with mock.patch("os.lstat", return_value=mock_stat_result): + with mock.patch("os.getuid", create=True, return_value=1000): + with mock.patch("os.chmod", side_effect=OSError(test_error_message)): + stor = LocalFileStorage(os.path.join(TEST_FOLDER, "chmod_failure_test")) - # Storage should be disabled due to chmod failure - self.assertFalse(stor._enabled) + # Storage should be disabled due to chmod failure + self.assertFalse(stor._enabled) - # Exception state should be set with the error message - exception_state = get_local_storage_setup_state_exception() - self.assertEqual(exception_state, test_error_message) + # Exception state should be set with the error message + exception_state = get_local_storage_setup_state_exception() + self.assertEqual(exception_state, test_error_message) - # When storage is disabled, put() behavior depends on readonly state - result = stor.put(test_input) - if get_local_storage_setup_state_readonly(): - # Readonly takes priority over exception state - self.assertEqual(result, StorageExportResult.CLIENT_READONLY) - else: - # If readonly not set, should return the exception message - self.assertEqual(result, test_error_message) + # When storage is disabled, put() behavior depends on readonly state + result = stor.put(test_input) + if get_local_storage_setup_state_readonly(): + # Readonly takes priority over exception state + self.assertEqual(result, StorageExportResult.CLIENT_READONLY) + else: + # If readonly not set, should return the exception message + self.assertEqual(result, test_error_message) - stor.close() + stor.close() # Clean up set_local_storage_setup_state_exception("") @@ -949,26 +954,31 @@ def mock_chmod(path, mode): def mock_makedirs(path, mode=0o777, exist_ok=False): makedirs_calls.append((path, oct(mode), exist_ok)) + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 1000 + with mock.patch(f"{STORAGE_MODULE}.os.makedirs", side_effect=mock_makedirs): - with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) - self.assertTrue(stor._enabled) + self.assertTrue(stor._enabled) - self.assertEqual( - makedirs_calls, - [(storage_abs_path, "0o777", True)], - f"Unexpected makedirs calls: {makedirs_calls}", - ) + self.assertEqual( + makedirs_calls, + [(storage_abs_path, "0o777", True)], + f"Unexpected makedirs calls: {makedirs_calls}", + ) - self.assertEqual( - {(storage_abs_path, "0o700")}, - set(chmod_calls), - f"Unexpected chmod calls: {chmod_calls}", - ) + self.assertEqual( + {(storage_abs_path, "0o700")}, + set(chmod_calls), + f"Unexpected chmod calls: {chmod_calls}", + ) - stor.close() + stor.close() # Clean up set_local_storage_setup_state_exception("") @@ -1024,17 +1034,211 @@ def mock_chmod(path, mode): raise PermissionError(test_error_message) raise OSError(f"Unexpected chmod call: {path}, {oct(mode)}") + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 1000 + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) - self.assertFalse(stor._enabled) + self.assertFalse(stor._enabled) - exception_state = get_local_storage_setup_state_exception() - self.assertEqual(exception_state, test_error_message) + exception_state = get_local_storage_setup_state_exception() + self.assertEqual(exception_state, test_error_message) - stor.close() + stor.close() # Clean up set_local_storage_setup_state_exception("") + + def test_check_and_set_folder_permissions_unix_ownership_by_current_user(self): + """Directory owned by the current user should pass ownership check.""" + from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( + set_local_storage_setup_state_exception, + ) + + set_local_storage_setup_state_exception("") + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 4242 + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + mock_chmod.assert_called_with(storage_abs_path, 0o700) + stor.close() + + set_local_storage_setup_state_exception("") + + def test_check_and_set_folder_permissions_unix_ownership_by_root(self): + """Directory owned by root (uid 0) should pass ownership check.""" + from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( + set_local_storage_setup_state_exception, + ) + + set_local_storage_setup_state_exception("") + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 0 # root + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + mock_chmod.assert_called_with(storage_abs_path, 0o700) + stor.close() + + set_local_storage_setup_state_exception("") + + def test_check_and_set_folder_permissions_unix_ownership_by_different_user(self): + """Directory owned by a different non-root user should fail ownership check.""" + from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( + get_local_storage_setup_state_exception, + set_local_storage_setup_state_exception, + ) + + set_local_storage_setup_state_exception("") + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 9999 # attacker + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertFalse(stor._enabled) + mock_chmod.assert_not_called() + exception_state = get_local_storage_setup_state_exception() + self.assertIn("owned by uid 9999", exception_state) + stor.close() + + set_local_storage_setup_state_exception("") + + def test_attack_scenario_precreated_directory_by_attacker(self): + """ + Simulates the attack described in the vulnerability report: + An unprivileged attacker pre-creates the storage directory in /tmp with + their own uid. When the legitimate process starts, it should detect the + ownership mismatch and refuse to use the directory, preventing data exfiltration. + """ + from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( + get_local_storage_setup_state_exception, + set_local_storage_setup_state_exception, + ) + + set_local_storage_setup_state_exception("") + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + # Attacker pre-created the directory — it is owned by attacker uid 5000 + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 5000 # attacker's uid + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): # dir already exists + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): # legitimate user + with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + + # Storage MUST be disabled — attacker owns the dir + self.assertFalse(stor._enabled) + # chmod must NOT be called — we refuse to use the directory + mock_chmod.assert_not_called() + # No data can be written + result = stor.put([{"telemetry": "sensitive_data"}]) + self.assertNotEqual(result, StorageExportResult.LOCAL_FILE_BLOB_SUCCESS) + # Exception state captures the ownership mismatch + exception_state = get_local_storage_setup_state_exception() + self.assertIn("owned by uid 5000", exception_state) + self.assertIn("expected 1000", exception_state) + + stor.close() + + set_local_storage_setup_state_exception("") + + def test_attack_scenario_symlink_to_attacker_controlled_path(self): + """ + Simulates a symlink attack: attacker creates a symlink at the expected + storage path pointing to a root-owned or attacker-controlled directory. + os.lstat checks the symlink itself (not the target), so if the symlink + is owned by the attacker, the check should fail. + """ + from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( + get_local_storage_setup_state_exception, + set_local_storage_setup_state_exception, + ) + + set_local_storage_setup_state_exception("") + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + # lstat on a symlink returns the symlink's own metadata, not the target. + # The symlink was created by attacker (uid 7777). + mock_symlink_stat = mock.MagicMock() + mock_symlink_stat.st_uid = 7777 # attacker created the symlink + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_symlink_stat): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + + # Storage MUST be disabled — symlink owned by attacker + self.assertFalse(stor._enabled) + mock_chmod.assert_not_called() + exception_state = get_local_storage_setup_state_exception() + self.assertIn("owned by uid 7777", exception_state) + + stor.close() + + set_local_storage_setup_state_exception("") + + def test_attack_scenario_file_race_condition_oexcl(self): + """ + Simulates O_EXCL protection: if an attacker manages to pre-create a + .tmp file at the exact path our blob will use, os.open with O_EXCL + must fail with FileExistsError, preventing data from being written + to an attacker-controlled file. + """ + storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) + + mock_stat_result = mock.MagicMock() + mock_stat_result.st_uid = 1000 + + with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): + with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): + with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.chmod"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + + # Now simulate the attack: attacker pre-created the .tmp file + with mock.patch("os.open", side_effect=FileExistsError("File exists")): + result = stor.put([{"secret": "credential_data"}]) + # put() must fail — data must NOT be written + self.assertIsInstance(result, str) + self.assertIn("File exists", result) + + stor.close() From e552e2aa01fc4c5731c2b316d11e606afb570f28 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 12:59:03 -0700 Subject: [PATCH 2/6] Update CHANGELOG --- sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md b/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md index 235f64b51290..69aed30dce94 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features Added - Add ownership checks for storage directories + ([#46725](https://github.com/Azure/azure-sdk-for-python/pull/46725)) - Add logger name to custom dimensions for Message, Exception and Event telemetry ([#46096](https://github.com/Azure/azure-sdk-for-python/pull/46096)) - Add support for populating SDK version from distro and Microsoft OpenTelemetry distro environment variables From b7aeddb26a3678d50051c55fe138cf96b3a43af8 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 13:43:52 -0700 Subject: [PATCH 3/6] Address comments --- .../opentelemetry/exporter/_storage.py | 44 ++- .../tests/test_storage.py | 293 +++++++++--------- 2 files changed, 182 insertions(+), 155 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py index 9176d37af1ff..4151994831e3 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py @@ -69,7 +69,12 @@ def put(self, data: List[Any], lease_period: int = 0) -> Union[StorageExportResu # Use O_CREAT | O_EXCL | O_WRONLY to atomically create the file # and fail if it already exists, preventing race conditions. fd = os.open(fullpath, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) - with os.fdopen(fd, "w", encoding="utf-8") as file: + try: + file = os.fdopen(fd, "w", encoding="utf-8") + except Exception: + os.close(fd) + raise + with file: for item in data: file.write(json.dumps(item)) # The official Python doc: Do not use os.linesep as a line @@ -258,22 +263,27 @@ def _check_and_set_folder_permissions(self) -> bool: return True # Unix else: - dir_stat = os.lstat(self._path) - owner_uid = dir_stat.st_uid - current_uid = os.getuid() # pylint: disable=no-member - if owner_uid not in (current_uid, 0): - logger.error( - "Storage directory %s is owned by uid %d, not the current user (%d) or admin (uid 0). " - "Refusing to use this directory.", - self._path, - owner_uid, - current_uid, - ) - set_local_storage_setup_state_exception( - f"Directory owned by uid {owner_uid}, expected {current_uid} or 0" - ) - return False - os.chmod(self._path, 0o700) + open_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW # pylint: disable=no-member + dir_fd = os.open(self._path, open_flags) + try: + dir_stat = os.fstat(dir_fd) + owner_uid = dir_stat.st_uid + current_uid = os.getuid() # pylint: disable=no-member + if owner_uid not in (current_uid, 0): + logger.error( + "Storage directory %s is owned by uid %d, not the current user (%d) or admin (uid 0). " + "Refusing to use this directory.", + self._path, + owner_uid, + current_uid, + ) + set_local_storage_setup_state_exception( + f"Directory owned by uid {owner_uid}, expected {current_uid} or 0" + ) + return False + os.fchmod(dir_fd, 0o700) # pylint: disable=no-member + finally: + os.close(dir_fd) return True except OSError as error: if getattr(error, "errno", None) == errno.EROFS: # cspell:disable-line diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py index f12bead85441..f0c25d794dee 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import errno import os import shutil import unittest @@ -21,6 +22,14 @@ TEST_USER = "multiuser-test" STORAGE_MODULE = "azure.monitor.opentelemetry.exporter._storage" +# Ensure Unix-only constants exist for cross-platform testing. +# These are used by the fd-based permission checks in the Unix code path; +# since os.open is mocked in those tests, the actual values don't matter. +if not hasattr(os, "O_DIRECTORY"): + os.O_DIRECTORY = 0o200000 +if not hasattr(os, "O_NOFOLLOW"): + os.O_NOFOLLOW = 0o400000 + def throw(exc_type, *args, **kwargs): def func(*_args, **_kwargs): @@ -83,7 +92,7 @@ def test_put_file_write_error_returns_string(self): blob = LocalFileBlob(os.path.join(TEST_FOLDER, "write_error_blob")) test_input = [1, 2, 3] - with mock.patch("os.open", side_effect=PermissionError("Cannot write to file")): + with mock.patch(f"{STORAGE_MODULE}.os.open", side_effect=PermissionError("Cannot write to file")): result = blob.put(test_input) self.assertIsInstance(result, str) self.assertIn("Cannot write to file", result) @@ -622,31 +631,33 @@ def test_check_and_set_folder_permissions_unix_chmod_exception_sets_exception_st mock_stat_result = mock.MagicMock() mock_stat_result.st_uid = 1000 - # Mock Unix environment and chmod failure + # Mock Unix environment and fchmod failure with mock.patch("os.name", "posix"): # Unix with mock.patch("os.makedirs"): # Allow directory creation - with mock.patch("os.lstat", return_value=mock_stat_result): - with mock.patch("os.getuid", create=True, return_value=1000): - with mock.patch("os.chmod", side_effect=OSError(test_error_message)): - stor = LocalFileStorage(os.path.join(TEST_FOLDER, "chmod_failure_test")) - - # Storage should be disabled due to chmod failure - self.assertFalse(stor._enabled) - - # Exception state should be set with the error message - exception_state = get_local_storage_setup_state_exception() - self.assertEqual(exception_state, test_error_message) - - # When storage is disabled, put() behavior depends on readonly state - result = stor.put(test_input) - if get_local_storage_setup_state_readonly(): - # Readonly takes priority over exception state - self.assertEqual(result, StorageExportResult.CLIENT_READONLY) - else: - # If readonly not set, should return the exception message - self.assertEqual(result, test_error_message) - - stor.close() + with mock.patch("os.open", return_value=99): + with mock.patch("os.fstat", return_value=mock_stat_result): + with mock.patch("os.getuid", create=True, return_value=1000): + with mock.patch("os.fchmod", create=True, side_effect=OSError(test_error_message)): + with mock.patch("os.close"): + stor = LocalFileStorage(os.path.join(TEST_FOLDER, "chmod_failure_test")) + + # Storage should be disabled due to fchmod failure + self.assertFalse(stor._enabled) + + # Exception state should be set with the error message + exception_state = get_local_storage_setup_state_exception() + self.assertEqual(exception_state, test_error_message) + + # When storage is disabled, put() behavior depends on readonly state + result = stor.put(test_input) + if get_local_storage_setup_state_readonly(): + # Readonly takes priority over exception state + self.assertEqual(result, StorageExportResult.CLIENT_READONLY) + else: + # If readonly not set, should return the exception message + self.assertEqual(result, test_error_message) + + stor.close() # Clean up set_local_storage_setup_state_exception("") @@ -945,11 +956,11 @@ def test_check_and_set_folder_permissions_unix_multiuser_scenario(self): storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): - chmod_calls = [] + fchmod_calls = [] makedirs_calls = [] - def mock_chmod(path, mode): - chmod_calls.append((path, oct(mode))) + def mock_fchmod(fd, mode): + fchmod_calls.append((fd, oct(mode))) def mock_makedirs(path, mode=0o777, exist_ok=False): makedirs_calls.append((path, oct(mode), exist_ok)) @@ -958,25 +969,27 @@ def mock_makedirs(path, mode=0o777, exist_ok=False): mock_stat_result.st_uid = 1000 with mock.patch(f"{STORAGE_MODULE}.os.makedirs", side_effect=mock_makedirs): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): - with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - - self.assertTrue(stor._enabled) - - self.assertEqual( - makedirs_calls, - [(storage_abs_path, "0o777", True)], - f"Unexpected makedirs calls: {makedirs_calls}", - ) - - self.assertEqual( - {(storage_abs_path, "0o700")}, - set(chmod_calls), - f"Unexpected chmod calls: {chmod_calls}", - ) + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True, side_effect=mock_fchmod): + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + + self.assertTrue(stor._enabled) + + self.assertEqual( + makedirs_calls, + [(storage_abs_path, "0o777", True)], + f"Unexpected makedirs calls: {makedirs_calls}", + ) + + self.assertEqual( + [(99, "0o700")], + fchmod_calls, + f"Unexpected fchmod calls: {fchmod_calls}", + ) stor.close() @@ -1029,27 +1042,27 @@ def test_check_and_set_folder_permissions_unix_multiuser_storage_permission_fail with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): - def mock_chmod(path, mode): - if mode == 0o700: - raise PermissionError(test_error_message) - raise OSError(f"Unexpected chmod call: {path}, {oct(mode)}") + def mock_fchmod(fd, mode): + raise PermissionError(test_error_message) mock_stat_result = mock.MagicMock() mock_stat_result.st_uid = 1000 with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): - with mock.patch(f"{STORAGE_MODULE}.os.chmod", side_effect=mock_chmod): - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True, side_effect=mock_fchmod): + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) - self.assertFalse(stor._enabled) + self.assertFalse(stor._enabled) - exception_state = get_local_storage_setup_state_exception() - self.assertEqual(exception_state, test_error_message) + exception_state = get_local_storage_setup_state_exception() + self.assertEqual(exception_state, test_error_message) - stor.close() + stor.close() # Clean up set_local_storage_setup_state_exception("") @@ -1068,14 +1081,16 @@ def test_check_and_set_folder_permissions_unix_ownership_by_current_user(self): with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): - with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - self.assertTrue(stor._enabled) - mock_chmod.assert_called_with(storage_abs_path, 0o700) - stor.close() + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True) as mock_fchmod: + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + mock_fchmod.assert_called_with(99, 0o700) + stor.close() set_local_storage_setup_state_exception("") @@ -1093,14 +1108,16 @@ def test_check_and_set_folder_permissions_unix_ownership_by_root(self): with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): - with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - self.assertTrue(stor._enabled) - mock_chmod.assert_called_with(storage_abs_path, 0o700) - stor.close() + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True) as mock_fchmod: + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + mock_fchmod.assert_called_with(99, 0o700) + stor.close() set_local_storage_setup_state_exception("") @@ -1119,16 +1136,18 @@ def test_check_and_set_folder_permissions_unix_ownership_by_different_user(self) with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): - with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - self.assertFalse(stor._enabled) - mock_chmod.assert_not_called() - exception_state = get_local_storage_setup_state_exception() - self.assertIn("owned by uid 9999", exception_state) - stor.close() + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=4242): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True) as mock_fchmod: + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertFalse(stor._enabled) + mock_fchmod.assert_not_called() + exception_state = get_local_storage_setup_state_exception() + self.assertIn("owned by uid 9999", exception_state) + stor.close() set_local_storage_setup_state_exception("") @@ -1153,34 +1172,35 @@ def test_attack_scenario_precreated_directory_by_attacker(self): with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): # dir already exists - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): # legitimate user - with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - - # Storage MUST be disabled — attacker owns the dir - self.assertFalse(stor._enabled) - # chmod must NOT be called — we refuse to use the directory - mock_chmod.assert_not_called() - # No data can be written - result = stor.put([{"telemetry": "sensitive_data"}]) - self.assertNotEqual(result, StorageExportResult.LOCAL_FILE_BLOB_SUCCESS) - # Exception state captures the ownership mismatch - exception_state = get_local_storage_setup_state_exception() - self.assertIn("owned by uid 5000", exception_state) - self.assertIn("expected 1000", exception_state) - - stor.close() + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): # legitimate user + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True) as mock_fchmod: + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + + # Storage MUST be disabled — attacker owns the dir + self.assertFalse(stor._enabled) + # fchmod must NOT be called — we refuse to use the directory + mock_fchmod.assert_not_called() + # No data can be written + result = stor.put([{"telemetry": "sensitive_data"}]) + self.assertNotEqual(result, StorageExportResult.LOCAL_FILE_BLOB_SUCCESS) + # Exception state captures the ownership mismatch + exception_state = get_local_storage_setup_state_exception() + self.assertIn("owned by uid 5000", exception_state) + self.assertIn("expected 1000", exception_state) + + stor.close() set_local_storage_setup_state_exception("") def test_attack_scenario_symlink_to_attacker_controlled_path(self): """ Simulates a symlink attack: attacker creates a symlink at the expected - storage path pointing to a root-owned or attacker-controlled directory. - os.lstat checks the symlink itself (not the target), so if the symlink - is owned by the attacker, the check should fail. + storage path. With O_NOFOLLOW, os.open() refuses to follow the symlink + and raises OSError, preventing the SDK from using the attacker's target. """ from azure.monitor.opentelemetry.exporter.statsbeat.customer._state import ( get_local_storage_setup_state_exception, @@ -1190,26 +1210,21 @@ def test_attack_scenario_symlink_to_attacker_controlled_path(self): set_local_storage_setup_state_exception("") storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) - # lstat on a symlink returns the symlink's own metadata, not the target. - # The symlink was created by attacker (uid 7777). - mock_symlink_stat = mock.MagicMock() - mock_symlink_stat.st_uid = 7777 # attacker created the symlink + # os.open with O_NOFOLLOW raises OSError (ELOOP) when path is a symlink + symlink_error = OSError(errno.ELOOP, "Too many levels of symbolic links") with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_symlink_stat): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): - with mock.patch(f"{STORAGE_MODULE}.os.chmod") as mock_chmod: - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - - # Storage MUST be disabled — symlink owned by attacker - self.assertFalse(stor._enabled) - mock_chmod.assert_not_called() - exception_state = get_local_storage_setup_state_exception() - self.assertIn("owned by uid 7777", exception_state) + with mock.patch(f"{STORAGE_MODULE}.os.open", side_effect=symlink_error): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) - stor.close() + # Storage MUST be disabled — symlink detected by O_NOFOLLOW + self.assertFalse(stor._enabled) + exception_state = get_local_storage_setup_state_exception() + self.assertIn("Too many levels of symbolic links", exception_state) + + stor.close() set_local_storage_setup_state_exception("") @@ -1227,18 +1242,20 @@ def test_attack_scenario_file_race_condition_oexcl(self): with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): - with mock.patch(f"{STORAGE_MODULE}.os.lstat", return_value=mock_stat_result): - with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): - with mock.patch(f"{STORAGE_MODULE}.os.chmod"): - with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): - stor = LocalFileStorage(storage_abs_path) - self.assertTrue(stor._enabled) - - # Now simulate the attack: attacker pre-created the .tmp file - with mock.patch("os.open", side_effect=FileExistsError("File exists")): - result = stor.put([{"secret": "credential_data"}]) - # put() must fail — data must NOT be written - self.assertIsInstance(result, str) - self.assertIn("File exists", result) - - stor.close() + with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): + with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): + with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True): + with mock.patch(f"{STORAGE_MODULE}.os.close"): + with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): + stor = LocalFileStorage(storage_abs_path) + self.assertTrue(stor._enabled) + + # Now simulate the attack: attacker pre-created the .tmp file + with mock.patch(f"{STORAGE_MODULE}.os.open", side_effect=FileExistsError("File exists")): + result = stor.put([{"secret": "credential_data"}]) + # put() must fail — data must NOT be written + self.assertIsInstance(result, str) + self.assertIn("File exists", result) + + stor.close() From f24133464638780ff40266047a44a53f13a40ac8 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 16:09:48 -0700 Subject: [PATCH 4/6] Fix cspell issue --- .../azure/monitor/opentelemetry/exporter/_storage.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py index 4151994831e3..d41caa3fbf60 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py @@ -66,9 +66,9 @@ def get(self) -> Optional[Tuple[Any, ...]]: def put(self, data: List[Any], lease_period: int = 0) -> Union[StorageExportResult, str]: try: fullpath = self.fullpath + ".tmp" - # Use O_CREAT | O_EXCL | O_WRONLY to atomically create the file + # Use O_CREAT | O_EXCL | O_WRONLY to atomically create the file # cspell:disable-line # and fail if it already exists, preventing race conditions. - fd = os.open(fullpath, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + fd = os.open(fullpath, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) # cspell:disable-line try: file = os.fdopen(fd, "w", encoding="utf-8") except Exception: @@ -263,7 +263,7 @@ def _check_and_set_folder_permissions(self) -> bool: return True # Unix else: - open_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW # pylint: disable=no-member + open_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW # pylint: disable=no-member # cspell:disable-line dir_fd = os.open(self._path, open_flags) try: dir_stat = os.fstat(dir_fd) From 9beb42036621fc5f8a7fd1146f3f59dd0ec6d503 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 16:49:33 -0700 Subject: [PATCH 5/6] Fix cspell --- .../monitor/opentelemetry/exporter/_storage.py | 2 +- .../tests/test_storage.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py index d41caa3fbf60..83a7a656146c 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py @@ -281,7 +281,7 @@ def _check_and_set_folder_permissions(self) -> bool: f"Directory owned by uid {owner_uid}, expected {current_uid} or 0" ) return False - os.fchmod(dir_fd, 0o700) # pylint: disable=no-member + os.fchmod(dir_fd, 0o700) # pylint: disable=no-member # cspell:disable-line finally: os.close(dir_fd) return True diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py index f0c25d794dee..c856a5d7cd56 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py @@ -631,17 +631,17 @@ def test_check_and_set_folder_permissions_unix_chmod_exception_sets_exception_st mock_stat_result = mock.MagicMock() mock_stat_result.st_uid = 1000 - # Mock Unix environment and fchmod failure + # Mock Unix environment and fchmod failure # cspell:disable-line with mock.patch("os.name", "posix"): # Unix with mock.patch("os.makedirs"): # Allow directory creation with mock.patch("os.open", return_value=99): with mock.patch("os.fstat", return_value=mock_stat_result): with mock.patch("os.getuid", create=True, return_value=1000): - with mock.patch("os.fchmod", create=True, side_effect=OSError(test_error_message)): + with mock.patch("os.fchmod", create=True, side_effect=OSError(test_error_message)): # cspell:disable-line with mock.patch("os.close"): stor = LocalFileStorage(os.path.join(TEST_FOLDER, "chmod_failure_test")) - # Storage should be disabled due to fchmod failure + # Storage should be disabled due to fchmod failure # cspell:disable-line self.assertFalse(stor._enabled) # Exception state should be set with the error message @@ -956,10 +956,10 @@ def test_check_and_set_folder_permissions_unix_multiuser_scenario(self): storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): - fchmod_calls = [] + fchmod_calls = [] # cspell:disable-line makedirs_calls = [] - def mock_fchmod(fd, mode): + def mock_fchmod(fd, mode): # cspell:disable-line fchmod_calls.append((fd, oct(mode))) def mock_makedirs(path, mode=0o777, exist_ok=False): @@ -1210,8 +1210,8 @@ def test_attack_scenario_symlink_to_attacker_controlled_path(self): set_local_storage_setup_state_exception("") storage_abs_path = _get_storage_directory(DUMMY_INSTRUMENTATION_KEY) - # os.open with O_NOFOLLOW raises OSError (ELOOP) when path is a symlink - symlink_error = OSError(errno.ELOOP, "Too many levels of symbolic links") + # os.open with O_NOFOLLOW raises OSError (ELOOP) when path is a symlink # cspell:disable-line + symlink_error = OSError(errno.ELOOP, "Too many levels of symbolic links") # cspell:disable-line with mock.patch(f"{STORAGE_MODULE}.os.name", "posix"): with mock.patch(f"{STORAGE_MODULE}.os.makedirs"): @@ -1228,7 +1228,7 @@ def test_attack_scenario_symlink_to_attacker_controlled_path(self): set_local_storage_setup_state_exception("") - def test_attack_scenario_file_race_condition_oexcl(self): + def test_attack_scenario_file_race_condition_oexcl(self): # cspell:disable-line """ Simulates O_EXCL protection: if an attacker manages to pre-create a .tmp file at the exact path our blob will use, os.open with O_EXCL From d53c1d4f619680ba65d2c8b4376811b44574e6d9 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 5 May 2026 17:39:16 -0700 Subject: [PATCH 6/6] Fix cspell --- .../tests/test_storage.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py index c856a5d7cd56..1d8c885a8e1e 100644 --- a/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py +++ b/sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py @@ -960,7 +960,7 @@ def test_check_and_set_folder_permissions_unix_multiuser_scenario(self): makedirs_calls = [] def mock_fchmod(fd, mode): # cspell:disable-line - fchmod_calls.append((fd, oct(mode))) + fchmod_calls.append((fd, oct(mode))) # cspell:disable-line def mock_makedirs(path, mode=0o777, exist_ok=False): makedirs_calls.append((path, oct(mode), exist_ok)) @@ -972,7 +972,7 @@ def mock_makedirs(path, mode=0o777, exist_ok=False): with mock.patch(f"{STORAGE_MODULE}.os.open", return_value=99): with mock.patch(f"{STORAGE_MODULE}.os.fstat", return_value=mock_stat_result): with mock.patch(f"{STORAGE_MODULE}.os.getuid", create=True, return_value=1000): - with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True, side_effect=mock_fchmod): + with mock.patch(f"{STORAGE_MODULE}.os.fchmod", create=True, side_effect=mock_fchmod): # cspell:disable-line with mock.patch(f"{STORAGE_MODULE}.os.close"): with mock.patch(f"{STORAGE_MODULE}.os.path.abspath", side_effect=lambda path: path): stor = LocalFileStorage(storage_abs_path) @@ -987,8 +987,8 @@ def mock_makedirs(path, mode=0o777, exist_ok=False): self.assertEqual( [(99, "0o700")], - fchmod_calls, - f"Unexpected fchmod calls: {fchmod_calls}", + fchmod_calls, # cspell:disable-line + f"Unexpected fchmod calls: {fchmod_calls}", # cspell:disable-line ) stor.close()