From 2bbe9198db94ce2eb87adf8347ecdfb13053e9f3 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 25 Mar 2026 19:11:52 +0530 Subject: [PATCH 1/3] Strip stale auth fields from pycore_context after token acquisition bulkcopy() acquires a fresh Azure AD token and sets access_token in the pycore_context dict, but left authentication/user_name/password from the original connection string. py-core's validator rejects access_token combined with those fields (ODBC parity). Pop authentication, user_name, and password after setting access_token. --- mssql_python/cursor.py | 4 + tests/test_020_bulkcopy_auth_cleanup.py | 116 ++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tests/test_020_bulkcopy_auth_cleanup.py diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 52fcbfd2..7b56c012 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2713,6 +2713,10 @@ def bulkcopy( f"for auth_type '{self.connection._auth_type}': {e}" ) from e pycore_context["access_token"] = raw_token + # Token replaces credential fields — py-core's validator rejects + # access_token combined with authentication/user_name/password. + for key in ("authentication", "user_name", "password"): + pycore_context.pop(key, None) logger.debug( "Bulk copy: acquired fresh Azure AD token for auth_type=%s", self.connection._auth_type, diff --git a/tests/test_020_bulkcopy_auth_cleanup.py b/tests/test_020_bulkcopy_auth_cleanup.py new file mode 100644 index 00000000..7d80a457 --- /dev/null +++ b/tests/test_020_bulkcopy_auth_cleanup.py @@ -0,0 +1,116 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for bulkcopy auth field cleanup in cursor.py. + +When cursor.bulkcopy() acquires an Azure AD token, it must strip stale +authentication/user_name/password keys from the pycore_context dict before +passing it to mssql_py_core. The Rust validator rejects access_token +combined with those fields (ODBC parity). +""" + +import pytest +import secrets +from unittest.mock import MagicMock, patch, PropertyMock + +SAMPLE_TOKEN = secrets.token_hex(44) + + +def _make_cursor(connection_str, auth_type): + """Build a mock Cursor with just enough wiring for bulkcopy's auth path.""" + from mssql_python.cursor import Cursor + + mock_conn = MagicMock() + mock_conn.connection_str = connection_str + mock_conn._auth_type = auth_type + mock_conn._is_connected = True + + cursor = Cursor.__new__(Cursor) + cursor._connection = mock_conn + cursor.closed = False + cursor.hstmt = None + return cursor + + +class TestBulkcopyAuthCleanup: + """Verify cursor.bulkcopy strips stale auth fields after token acquisition.""" + + @patch("mssql_python.cursor.get_settings") + @patch("mssql_python.cursor.logger") + def test_token_replaces_auth_fields(self, mock_logger, mock_settings): + """access_token present ⇒ authentication, user_name, password removed.""" + mock_settings.return_value = MagicMock(logging=False) + mock_logger.is_debug_enabled = False + + cursor = _make_cursor( + "Server=tcp:test.database.windows.net;Database=testdb;" + "Authentication=ActiveDirectoryDefault;UID=user@test.com;PWD=secret", + "activedirectorydefault", + ) + + captured_context = {} + + mock_pycore_cursor = MagicMock() + mock_pycore_cursor.bulkcopy.return_value = { + "rows_copied": 1, + "batch_count": 1, + "elapsed_time": 0.1, + } + mock_pycore_conn = MagicMock() + mock_pycore_conn.cursor.return_value = mock_pycore_cursor + + def capture_context(ctx, **kwargs): + captured_context.update(ctx) + return mock_pycore_conn + + mock_pycore_module = MagicMock() + mock_pycore_module.PyCoreConnection = capture_context + + with ( + patch.dict("sys.modules", {"mssql_py_core": mock_pycore_module}), + patch("mssql_python.auth.AADAuth.get_raw_token", return_value=SAMPLE_TOKEN), + ): + cursor.bulkcopy("dbo.test_table", [(1, "row")], timeout=10) + + assert captured_context.get("access_token") == SAMPLE_TOKEN + assert "authentication" not in captured_context + assert "user_name" not in captured_context + assert "password" not in captured_context + + @patch("mssql_python.cursor.get_settings") + @patch("mssql_python.cursor.logger") + def test_no_auth_type_leaves_fields_intact(self, mock_logger, mock_settings): + """No _auth_type ⇒ credentials pass through unchanged (SQL auth path).""" + mock_settings.return_value = MagicMock(logging=False) + mock_logger.is_debug_enabled = False + + cursor = _make_cursor( + "Server=tcp:test.database.windows.net;Database=testdb;" + "UID=sa;PWD=password123", + None, # no AD auth + ) + + captured_context = {} + + mock_pycore_cursor = MagicMock() + mock_pycore_cursor.bulkcopy.return_value = { + "rows_copied": 1, + "batch_count": 1, + "elapsed_time": 0.1, + } + mock_pycore_conn = MagicMock() + mock_pycore_conn.cursor.return_value = mock_pycore_cursor + + def capture_context(ctx, **kwargs): + captured_context.update(ctx) + return mock_pycore_conn + + mock_pycore_module = MagicMock() + mock_pycore_module.PyCoreConnection = capture_context + + with patch.dict("sys.modules", {"mssql_py_core": mock_pycore_module}): + cursor.bulkcopy("dbo.test_table", [(1, "row")], timeout=10) + + assert "access_token" not in captured_context + assert captured_context.get("user_name") == "sa" + assert captured_context.get("password") == "password123" From f7e4628ea52d492f15a2007208c555bdf8a3f665 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 26 Mar 2026 15:37:05 +0530 Subject: [PATCH 2/3] linting fix --- tests/test_020_bulkcopy_auth_cleanup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_020_bulkcopy_auth_cleanup.py b/tests/test_020_bulkcopy_auth_cleanup.py index 7d80a457..c45b92a9 100644 --- a/tests/test_020_bulkcopy_auth_cleanup.py +++ b/tests/test_020_bulkcopy_auth_cleanup.py @@ -85,8 +85,7 @@ def test_no_auth_type_leaves_fields_intact(self, mock_logger, mock_settings): mock_logger.is_debug_enabled = False cursor = _make_cursor( - "Server=tcp:test.database.windows.net;Database=testdb;" - "UID=sa;PWD=password123", + "Server=tcp:test.database.windows.net;Database=testdb;" "UID=sa;PWD=password123", None, # no AD auth ) From 34524dc027cf5a38889a144bec727fa4dfe8d443 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 26 Mar 2026 16:29:40 +0530 Subject: [PATCH 3/3] Remove unused imports and unnecessary get_settings patching --- tests/test_020_bulkcopy_auth_cleanup.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/test_020_bulkcopy_auth_cleanup.py b/tests/test_020_bulkcopy_auth_cleanup.py index c45b92a9..9eb86d69 100644 --- a/tests/test_020_bulkcopy_auth_cleanup.py +++ b/tests/test_020_bulkcopy_auth_cleanup.py @@ -9,9 +9,8 @@ combined with those fields (ODBC parity). """ -import pytest import secrets -from unittest.mock import MagicMock, patch, PropertyMock +from unittest.mock import MagicMock, patch SAMPLE_TOKEN = secrets.token_hex(44) @@ -35,11 +34,9 @@ def _make_cursor(connection_str, auth_type): class TestBulkcopyAuthCleanup: """Verify cursor.bulkcopy strips stale auth fields after token acquisition.""" - @patch("mssql_python.cursor.get_settings") @patch("mssql_python.cursor.logger") - def test_token_replaces_auth_fields(self, mock_logger, mock_settings): + def test_token_replaces_auth_fields(self, mock_logger): """access_token present ⇒ authentication, user_name, password removed.""" - mock_settings.return_value = MagicMock(logging=False) mock_logger.is_debug_enabled = False cursor = _make_cursor( @@ -77,11 +74,9 @@ def capture_context(ctx, **kwargs): assert "user_name" not in captured_context assert "password" not in captured_context - @patch("mssql_python.cursor.get_settings") @patch("mssql_python.cursor.logger") - def test_no_auth_type_leaves_fields_intact(self, mock_logger, mock_settings): + def test_no_auth_type_leaves_fields_intact(self, mock_logger): """No _auth_type ⇒ credentials pass through unchanged (SQL auth path).""" - mock_settings.return_value = MagicMock(logging=False) mock_logger.is_debug_enabled = False cursor = _make_cursor(