Drop brace-aware splitter; keep MSI extraction at parity with rest of parser

Reverts 825d902's _split_top_level_params helper and the
test_msi_braced_value_cannot_forge_uid test. The brace-aware fix
prevents a Database value containing ';uid=<guid>;' from spoofing
a top-level UID, but:

- The same naive split(';') exists in process_connection_string and
  extract_auth_type for ALL Entra ID auth types (Default, DeviceCode,
  Interactive, MSI). Hardening only the MSI helper is inconsistent.
- A Database name with embedded semicolons + a syntactically valid
  GUID after ';uid=' is vanishingly unlikely in practice.
- The fix belongs in its own PR scoped to the connection-string
  parser, not in the MSI feature PR.

What stays:

- H1 fix: persist _credential_kwargs on Connection so cursor.bulkcopy()
  acquires its fresh token with the right user-assigned client_id
- Real cursor.bulkcopy() regression test (mocks mssql_py_core, captures
  AADAuth.get_raw_token args)
- Connection.__init__ persistence tests
- Black formatting

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: bewithgaurav

mssql_python/auth.py

@@ -150,65 +150,13 @@ def _acquire_token(
             raise RuntimeError(f"Failed to create {credential_class.__name__}: {e}") from e
 
 
-def _split_top_level_params(connection_string: str) -> List[str]:
-    """Split an ODBC connection string on top-level ``;`` separators.
-
-    Honors braced values: ``;`` inside ``{...}`` is part of the value, not a
-    separator. ``}}`` inside braces is the ODBC escape for a literal ``}``
-    and does not close the brace.
-
-    This exists so that auth-related extraction (Authentication, UID for MSI)
-    cannot be forged by a semicolon inside a legitimate braced value such as
-    ``Database={foo;uid=<guid>;bar}``. Naive ``split(';')`` would let that
-    value spoof a top-level ``UID=`` and silently switch the bulkcopy path
-    to a wrong identity.
-
-    Returns chunks with surrounding whitespace preserved (callers strip).
-    """
-    parts: List[str] = []
-    current: List[str] = []
-    depth = 0
-    i = 0
-    n = len(connection_string)
-    while i < n:
-        c = connection_string[i]
-        if c == "{":
-            depth += 1
-            current.append(c)
-        elif c == "}":
-            # ``}}`` inside braces is an escaped literal ``}``.
-            if depth > 0 and i + 1 < n and connection_string[i + 1] == "}":
-                current.append("}}")
-                i += 2
-                continue
-            if depth > 0:
-                depth -= 1
-            current.append(c)
-        elif c == ";" and depth == 0:
-            parts.append("".join(current))
-            current = []
-        else:
-            current.append(c)
-        i += 1
-    parts.append("".join(current))
-    return parts
-
-
-def _extract_msi_client_id(connection_string: str) -> Optional[str]:
-    """Pull UID out of a connection string for user-assigned MSI.
+def _extract_msi_client_id(parameters: List[str]) -> Optional[str]:
+    """Pull UID out of connection parameters for user-assigned MSI.
 
     For ActiveDirectoryMSI, UID (when present) carries the user-assigned
-    identity's ``client_id``. Returns None for system-assigned MSI.
-
-    Uses brace-aware splitting so a semicolon inside a legitimate braced
-    ODBC value (e.g. ``Database={foo;uid=<guid>;bar}``) cannot spoof a
-    top-level ``UID=`` and silently switch the bulkcopy path to a wrong
-    identity. The pre-existing naive ``split(';')`` in
-    ``process_connection_string`` / ``extract_auth_type`` has the same
-    class of issue for ``Authentication=`` detection across all auth
-    types; that is out of scope for this PR and tracked separately.
+    identity's client_id. Returns None for system-assigned MSI.
     """
-    for param in _split_top_level_params(connection_string):
+    for param in parameters:
         key, _, value = param.strip().partition("=")
         if key.strip().lower() == "uid":
             value = value.strip()
@@ -400,13 +348,10 @@ def process_connection_string(
     modified_parameters, auth_type = process_auth_parameters(parameters)
 
     # Capture credential kwargs (e.g. user-assigned MSI client_id) before
-    # remove_sensitive_params strips UID from the parameter list. Pass the
-    # original connection_string (not modified_parameters) so the helper can
-    # do its own brace-aware split — a Database value containing ';uid=...'
-    # in braces must not spoof a top-level UID.
+    # remove_sensitive_params strips UID from the parameter list.
     credential_kwargs: Dict[str, str] = {}
     if auth_type == "msi":
-        client_id = _extract_msi_client_id(connection_string)
+        client_id = _extract_msi_client_id(modified_parameters)
         if client_id:
             credential_kwargs["client_id"] = client_id
 

tests/test_008_auth.py

@@ -573,31 +573,6 @@ def fake_get_raw_token(auth_type, credential_kwargs=None):
             f"the cursor likely re-parses the (UID-stripped) connection_str."
         )
 
-    def test_msi_braced_value_cannot_forge_uid(self):
-        """A semicolon inside a legitimate braced ODBC value (e.g. a Database
-        name) must not spoof a top-level UID= and silently switch the bulkcopy
-        path to a wrong identity. The MSI client_id extraction is brace-aware
-        for this reason."""
-        from mssql_python.connection_string_parser import _ConnectionStringParser
-        from mssql_python.connection_string_builder import _ConnectionStringBuilder
-
-        attacker_id = "00000000-0000-0000-0000-000000000bad"
-        raw = (
-            "Server=test;Authentication=ActiveDirectoryMSI;"
-            f"Database={{foo;uid={attacker_id};bar}}"
-        )
-        parsed = _ConnectionStringParser(validate_keywords=True)._parse(raw)
-        normalized = _ConnectionStringParser._normalize_params(parsed, warn_rejected=False)
-        built = _ConnectionStringBuilder(normalized).build()
-
-        _, _, auth_type, credential_kwargs = process_connection_string(built)
-
-        assert auth_type == "msi"  # legit Authentication= was top-level
-        assert credential_kwargs is None, (
-            f"braced Database value forged UID and produced credential_kwargs={credential_kwargs!r}; "
-            "_extract_msi_client_id is no longer brace-aware"
-        )
-
 
 class TestCredentialInstanceCache:
     """Tests for the credential instance caching behavior."""