refactor: make operation_context a structured object with PII validation

Address PR comments:
- Replace raw string with OperationContext dataclass (key=value format)
- Add regex validation to reject PII (emails, free-form text, passwords)
- Rename client kwarg from operation_context to context
- Validation happens at OperationContext creation time (fail-fast)
- 20 unit tests covering valid formats, PII rejection, and User-Agent header

Author: arorashivam96

src/PowerPlatform/Dataverse/client.py

@@ -12,7 +12,7 @@
 from azure.core.credentials import TokenCredential
 
 from .core._auth import _AuthManager
-from .core.config import DataverseConfig
+from .core.config import DataverseConfig, OperationContext
 from .data._odata import _ODataClient
 from .operations.dataframe import DataFrameOperations
 from .operations.records import RecordOperations
@@ -44,14 +44,14 @@ class DataverseClient:
     :param config: Optional configuration for language, timeouts, and retries.
         If not provided, defaults are loaded from :meth:`~PowerPlatform.Dataverse.core.config.DataverseConfig.from_env`.
     :type config: ~PowerPlatform.Dataverse.core.config.DataverseConfig or None
-    :param operation_context: Optional caller-defined context string appended to the
-        outbound ``User-Agent`` header as a parenthesized comment. Cannot be used
+    :param context: Optional caller-defined context object appended to the
+        outbound ``User-Agent`` header for plugin/tool attribution. Cannot be used
         together with ``config`` -- pass the context via
         :class:`~PowerPlatform.Dataverse.core.config.DataverseConfig` instead.
-    :type operation_context: :class:`str` or None
+    :type context: ~PowerPlatform.Dataverse.core.config.OperationContext or None
 
     :raises ValueError: If ``base_url`` is missing or empty after trimming.
-    :raises ValueError: If both ``config`` and ``operation_context`` are provided.
+    :raises ValueError: If both ``config`` and ``context`` are provided.
 
     .. note::
         The client lazily initializes its internal OData client on first use, allowing lightweight construction without immediate network calls.
@@ -102,21 +102,20 @@ def __init__(
         credential: TokenCredential,
         config: Optional[DataverseConfig] = None,
         *,
-        operation_context: Optional[str] = None,
+        context: Optional[OperationContext] = None,
     ) -> None:
-        if config is not None and operation_context is not None:
+        if config is not None and context is not None:
             raise ValueError(
-                "Cannot specify both 'config' and 'operation_context'. "
-                "Pass operation_context via DataverseConfig instead."
+                "Cannot specify both 'config' and 'context'. " "Pass operation_context via DataverseConfig instead."
             )
         self.auth = _AuthManager(credential)
         self._base_url = (base_url or "").rstrip("/")
         if not self._base_url:
             raise ValueError("base_url is required.")
         if config is not None:
             self._config = config
-        elif operation_context is not None:
-            self._config = DataverseConfig(operation_context=operation_context)
+        elif context is not None:
+            self._config = DataverseConfig(operation_context=context)
         else:
             self._config = DataverseConfig.from_env()
         self._odata: Optional[_ODataClient] = None

src/PowerPlatform/Dataverse/core/config.py

@@ -11,12 +11,50 @@
 
 from __future__ import annotations
 
+import re
 from dataclasses import dataclass
 from typing import TYPE_CHECKING, Optional
 
 if TYPE_CHECKING:
     from .log_config import LogConfig
 
+# key=value pairs separated by semicolons.
+# Keys: alphanumeric, hyphens, underscores.
+# Values: alphanumeric, hyphens, underscores, dots, slashes.
+_CONTEXT_PATTERN = re.compile(r"^[a-zA-Z0-9_-]+=[a-zA-Z0-9_./-]+(;[a-zA-Z0-9_-]+=[a-zA-Z0-9_./-]+)*$")
+
+
+@dataclass(frozen=True)
+class OperationContext:
+    """Caller-defined context appended to outbound ``User-Agent`` headers.
+
+    The context string is validated to be semicolon-separated ``key=value`` pairs
+    (e.g. ``"app=myapp/1.0;agent=claude-code"``).  Free-form text, email
+    addresses, and other potentially sensitive strings are rejected.
+
+    :param operation_context: Attribution string in ``key=value;key=value`` format.
+    :type operation_context: :class:`str`
+
+    :raises ValueError: If the string is empty, contains control characters, or
+        does not match the required ``key=value`` format.
+    """
+
+    operation_context: str
+
+    def __post_init__(self) -> None:
+        val = self.operation_context
+        if not val:
+            raise ValueError("operation_context must not be empty.")
+        if any(c in val for c in "\r\n\x00"):
+            raise ValueError("operation_context must not contain CR, LF, or NUL characters.")
+        if not _CONTEXT_PATTERN.match(val):
+            raise ValueError(
+                "operation_context must be semicolon-separated key=value pairs "
+                "(e.g. 'app=myapp/1.0;agent=claude-code'). "
+                "Keys and values may contain alphanumerics, hyphens, underscores, "
+                "dots, and slashes."
+            )
+
 
 @dataclass(frozen=True)
 class DataverseConfig:
@@ -35,10 +73,10 @@ class DataverseConfig:
         When provided, all HTTP requests and responses are logged to timestamped
         ``.log`` files with automatic redaction of sensitive headers.
     :type log_config: ~PowerPlatform.Dataverse.core.log_config.LogConfig or None
-    :param operation_context: Optional caller-defined context string appended to the
+    :param operation_context: Optional caller-defined context object appended to the
         outbound ``User-Agent`` header as a parenthesized comment. Intended for
-        plugin/tool attribution (e.g. ``"app=dataverse-skills/1.2.1;skill=dv-data;agent=claude-code"``).
-    :type operation_context: :class:`str` or None
+        plugin/tool attribution.
+    :type operation_context: ~PowerPlatform.Dataverse.core.config.OperationContext or None
     """
 
     language_code: int = 1033
@@ -50,7 +88,7 @@ class DataverseConfig:
 
     log_config: Optional["LogConfig"] = None
 
-    operation_context: Optional[str] = None
+    operation_context: Optional[OperationContext] = None
 
     @classmethod
     def from_env(cls) -> "DataverseConfig":

src/PowerPlatform/Dataverse/data/_odata.py

@@ -206,10 +206,8 @@ def __init__(
             session=session,
             logger=self._http_logger,
         )
-        ctx = self.config.operation_context
-        if ctx and any(c in ctx for c in "\r\n\x00"):
-            raise ValueError("operation_context must not contain CR, LF, or NUL characters.")
-        self._operation_context = ctx
+        ctx_obj = self.config.operation_context
+        self._operation_context = ctx_obj.operation_context if ctx_obj else None
         self._logical_to_entityset_cache: dict[str, str] = {}
         # Cache: normalized table_schema_name (lowercase) -> primary id attribute (e.g. accountid)
         self._logical_primaryid_cache: dict[str, str] = {}

tests/unit/data/test_enum_optionset_payload.py

@@ -25,7 +25,7 @@ def __init__(self, language_code=1033):
         self.http_backoff = 0
         self.http_timeout = 5
         self.log_config = None
-        self.operation_context = None
+        self.operation_context = None  # None or OperationContext object
 
 
 def _make_client(lang=1033):

tests/unit/test_operation_context.py

@@ -9,10 +9,51 @@
 from azure.core.credentials import TokenCredential
 
 from PowerPlatform.Dataverse.client import DataverseClient
-from PowerPlatform.Dataverse.core.config import DataverseConfig
+from PowerPlatform.Dataverse.core.config import DataverseConfig, OperationContext
 from PowerPlatform.Dataverse.data._odata import _ODataClient, _USER_AGENT
 
 
+class TestOperationContextValidation(unittest.TestCase):
+    """Tests for OperationContext format validation and PII rejection."""
+
+    def test_valid_single_pair(self):
+        ctx = OperationContext(operation_context="app=test/1.0")
+        self.assertEqual(ctx.operation_context, "app=test/1.0")
+
+    def test_valid_multiple_pairs(self):
+        ctx = OperationContext(operation_context="app=test/1.0;skill=dv-data;agent=claude-code")
+        self.assertEqual(ctx.operation_context, "app=test/1.0;skill=dv-data;agent=claude-code")
+
+    def test_valid_with_dots_slashes_hyphens(self):
+        ctx = OperationContext(operation_context="app=dataverse-skills/1.2.1")
+        self.assertEqual(ctx.operation_context, "app=dataverse-skills/1.2.1")
+
+    def test_reject_empty(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="")
+
+    def test_reject_email(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="myname@email.com")
+
+    def test_reject_freeform_text(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="my bank password is 1234")
+
+    def test_reject_control_chars(self):
+        for bad in ["has\rnewline", "has\nnewline", "has\x00null"]:
+            with self.assertRaises(ValueError):
+                OperationContext(operation_context=bad)
+
+    def test_reject_spaces(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="app=my app")
+
+    def test_reject_no_equals(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="justaplainstring")
+
+
 class TestOperationContextConfig(unittest.TestCase):
     """Tests for operation_context on DataverseConfig."""
 
@@ -21,47 +62,57 @@ def test_default_is_none(self):
         self.assertIsNone(config.operation_context)
 
     def test_explicit_value(self):
-        config = DataverseConfig(operation_context="app=test/1.0;agent=claude-code")
-        self.assertEqual(config.operation_context, "app=test/1.0;agent=claude-code")
+        ctx = OperationContext(operation_context="app=test/1.0;agent=claude-code")
+        config = DataverseConfig(operation_context=ctx)
+        self.assertEqual(config.operation_context.operation_context, "app=test/1.0;agent=claude-code")
 
     def test_default_constructor_is_none(self):
         config = DataverseConfig()
         self.assertIsNone(config.operation_context)
 
 
 class TestOperationContextClient(unittest.TestCase):
-    """Tests for operation_context kwarg on DataverseClient."""
+    """Tests for context kwarg on DataverseClient."""
 
     def setUp(self):
         self.mock_credential = MagicMock(spec=TokenCredential)
         self.base_url = "https://example.crm.dynamics.com"
 
     def test_kwarg_sets_config(self):
+        ctx = OperationContext(operation_context="app=test/1.0;skill=dv-data;agent=claude-code")
         client = DataverseClient(
             self.base_url,
             self.mock_credential,
-            operation_context="app=test/1.0;skill=dv-data;agent=claude-code",
+            context=ctx,
+        )
+        self.assertEqual(
+            client._config.operation_context.operation_context,
+            "app=test/1.0;skill=dv-data;agent=claude-code",
         )
-        self.assertEqual(client._config.operation_context, "app=test/1.0;skill=dv-data;agent=claude-code")
 
     def test_no_kwarg_leaves_config_default(self):
         client = DataverseClient(self.base_url, self.mock_credential)
         self.assertIsNone(client._config.operation_context)
 
-    def test_config_and_kwarg_raises(self):
-        config = DataverseConfig(operation_context="app=test/1.0")
+    def test_config_and_context_raises(self):
+        ctx = OperationContext(operation_context="app=test/1.0")
+        config = DataverseConfig(operation_context=ctx)
         with self.assertRaises(ValueError):
             DataverseClient(
                 self.base_url,
                 self.mock_credential,
                 config=config,
-                operation_context="app=other/2.0",
+                context=OperationContext(operation_context="app=other/2.0"),
             )
 
     def test_config_alone_works(self):
-        config = DataverseConfig(operation_context="app=test/1.0;agent=copilot")
+        ctx = OperationContext(operation_context="app=test/1.0;agent=copilot")
+        config = DataverseConfig(operation_context=ctx)
         client = DataverseClient(self.base_url, self.mock_credential, config=config)
-        self.assertEqual(client._config.operation_context, "app=test/1.0;agent=copilot")
+        self.assertEqual(
+            client._config.operation_context.operation_context,
+            "app=test/1.0;agent=copilot",
+        )
 
 
 class TestOperationContextUserAgent(unittest.TestCase):
@@ -80,26 +131,19 @@ def test_default_user_agent_unchanged(self):
         self.assertEqual(headers["User-Agent"], _USER_AGENT)
 
     def test_operation_context_appended(self):
-        ctx = "app=dataverse-skills/1.2.1;skill=dv-data;agent=claude-code"
+        ctx_str = "app=dataverse-skills/1.2.1;skill=dv-data;agent=claude-code"
+        ctx = OperationContext(operation_context=ctx_str)
         config = DataverseConfig(operation_context=ctx)
         odata = _ODataClient(self.dummy_auth, self.base_url, config=config)
         headers = odata._headers()
-        self.assertEqual(headers["User-Agent"], f"{_USER_AGENT} ({ctx})")
+        self.assertEqual(headers["User-Agent"], f"{_USER_AGENT} ({ctx_str})")
 
     def test_none_context_no_parentheses(self):
         config = DataverseConfig(operation_context=None)
         odata = _ODataClient(self.dummy_auth, self.base_url, config=config)
         headers = odata._headers()
         self.assertNotIn("(", headers["User-Agent"])
 
-    def test_empty_string_context_no_parentheses(self):
-        config = DataverseConfig(operation_context="")
-        odata = _ODataClient(self.dummy_auth, self.base_url, config=config)
-        headers = odata._headers()
-        self.assertNotIn("(", headers["User-Agent"])
-
-    def test_control_chars_rejected(self):
-        for bad in ["has\rnewline", "has\nnewline", "has\x00null"]:
-            config = DataverseConfig(operation_context=bad)
-            with self.assertRaises(ValueError):
-                _ODataClient(self.dummy_auth, self.base_url, config=config)
+    def test_empty_string_rejected_at_creation(self):
+        with self.assertRaises(ValueError):
+            OperationContext(operation_context="")