fix: enforce privacy filter precedence for sentry init overrides

Author: abrichr

README.md

@@ -160,16 +160,17 @@ export OPENADAPT_TELEMETRY_ENABLED=false
 - File paths have usernames replaced with `<user>`
 - Sensitive fields (password, token, api_key, etc.) are redacted
 - Email addresses and phone numbers are scrubbed from messages
+- User IDs are hashed before upload (`anon:<hash>`)
+- `send_default_pii` is enforced to `false` by the client
 
 ## Internal Usage Tagging
 
 Internal/developer usage is automatically detected via:
 
 1. `OPENADAPT_INTERNAL=true` environment variable
 2. `OPENADAPT_DEV=true` environment variable
-3. Running from source (not frozen executable)
-4. Git repository present in working directory
-5. CI environment detected (GitHub Actions, GitLab CI, etc.)
+3. Git repository present in working directory
+4. CI environment detected (GitHub Actions, GitLab CI, etc.)
 
 Filter in GlitchTip:
 ```

src/openadapt_telemetry/client.py

@@ -9,14 +9,18 @@
 import os
 import platform
 import sys
+import warnings
 from pathlib import Path
-from typing import Any, Dict, Optional
+from typing import Any, Callable, Dict, Optional
 
 import sentry_sdk
+from sentry_sdk.types import Event, Hint
 
 from .config import TelemetryConfig, load_config
 from .privacy import anonymize_identifier, create_before_send_filter
 
+BeforeSendFn = Callable[[Event, Hint], Optional[Event]]
+
 
 def is_running_from_executable() -> bool:
     """Check if running from a frozen/bundled executable.
@@ -84,6 +88,18 @@ def is_internal_user() -> bool:
     return False
 
 
+def _compose_before_send(base: BeforeSendFn, extra: BeforeSendFn) -> BeforeSendFn:
+    """Compose custom before_send after privacy filtering."""
+
+    def composed(event: Event, hint: Hint) -> Optional[Event]:
+        sanitized = base(event, hint)
+        if sanitized is None:
+            return None
+        return extra(sanitized, hint)
+
+    return composed
+
+
 class TelemetryClient:
     """Unified telemetry client for all OpenAdapt packages.
 
@@ -175,7 +191,8 @@ def initialize(
         Returns:
             True if initialization succeeded, False if disabled or already initialized.
         """
-        if self._initialized and not kwargs.get("force", False):
+        force = bool(kwargs.pop("force", False))
+        if self._initialized and not force:
             return True
 
         # Load configuration
@@ -195,23 +212,40 @@ def initialize(
         if not self._config.dsn:
             return False
 
-        # Create privacy filter
-        before_send = create_before_send_filter()
+        # Always enforce privacy scrubber first; optional custom filter can run afterward.
+        base_before_send = create_before_send_filter()
+        custom_before_send = kwargs.pop("before_send", None)
+        if custom_before_send is not None:
+            if not callable(custom_before_send):
+                raise TypeError("before_send must be callable")
+            warnings.warn(
+                "Custom before_send is composed after OpenAdapt privacy filtering and cannot bypass scrubbing.",
+                stacklevel=2,
+            )
+            before_send = _compose_before_send(base_before_send, custom_before_send)
+        else:
+            before_send = base_before_send
+
+        if "send_default_pii" in kwargs:
+            kwargs.pop("send_default_pii")
+            warnings.warn(
+                "Ignoring sentry init override for send_default_pii; OpenAdapt telemetry enforces send_default_pii=False.",
+                stacklevel=2,
+            )
 
         # Initialize Sentry SDK
         sentry_kwargs = {
             "dsn": self._config.dsn,
             "environment": self._config.environment,
             "sample_rate": self._config.sample_rate,
             "traces_sample_rate": self._config.traces_sample_rate,
-            "send_default_pii": self._config.send_default_pii,
+            # Enforced for privacy safety across all callers/configs.
+            "send_default_pii": False,
             "before_send": before_send,
         }
 
         # Merge in any additional kwargs
         sentry_kwargs.update(kwargs)
-        # Remove our internal kwargs
-        sentry_kwargs.pop("force", None)
 
         sentry_sdk.init(**sentry_kwargs)
 

tests/test_client.py

@@ -3,6 +3,8 @@
 import os
 from unittest.mock import patch
 
+import pytest
+
 from openadapt_telemetry.client import (
     TelemetryClient,
     get_telemetry,
@@ -188,6 +190,8 @@ def test_initialize_with_dsn(self, mock_sentry):
             assert result is True
             assert client.initialized is True
             mock_sentry.init.assert_called_once()
+            init_kwargs = mock_sentry.init.call_args.kwargs
+            assert init_kwargs["send_default_pii"] is False
 
     @patch("openadapt_telemetry.client.sentry_sdk")
     def test_capture_exception_when_enabled(self, mock_sentry):
@@ -254,6 +258,54 @@ def test_set_user_hashes_identifier_and_drops_extra_fields(self, mock_sentry):
             assert payload["id"].startswith("anon:")
             assert payload["id"] != "user@example.com"
 
+    @patch("openadapt_telemetry.client.sentry_sdk")
+    def test_custom_before_send_is_composed_after_privacy_filter(self, mock_sentry):
+        """Custom before_send should run after built-in scrubbing."""
+
+        def custom_before_send(event, hint):
+            event.setdefault("tags", {})["custom"] = "true"
+            return event
+
+        with patch.dict(os.environ, {"DO_NOT_TRACK": ""}, clear=False):
+            TelemetryClient.reset_instance()
+            client = TelemetryClient.get_instance()
+            with pytest.warns(UserWarning, match="Custom before_send is composed after OpenAdapt privacy filtering"):
+                client.initialize(
+                    dsn="https://test@example.com/1",
+                    before_send=custom_before_send,
+                )
+
+            before_send = mock_sentry.init.call_args.kwargs["before_send"]
+            event = {"user": {"id": "user@example.com", "email": "user@example.com"}}
+            output = before_send(event, hint={})
+            assert output is not None
+            assert output["user"]["id"].startswith("anon:")
+            assert "email" not in output["user"]
+            assert output["tags"]["custom"] == "true"
+
+    @patch("openadapt_telemetry.client.sentry_sdk")
+    def test_send_default_pii_override_is_ignored(self, mock_sentry):
+        """send_default_pii should always be enforced to False."""
+        with patch.dict(os.environ, {"DO_NOT_TRACK": ""}, clear=False):
+            TelemetryClient.reset_instance()
+            client = TelemetryClient.get_instance()
+            with pytest.warns(UserWarning, match="Ignoring sentry init override for send_default_pii"):
+                client.initialize(
+                    dsn="https://test@example.com/1",
+                    send_default_pii=True,
+                )
+
+            assert mock_sentry.init.call_args.kwargs["send_default_pii"] is False
+
+    def test_initialize_rejects_non_callable_before_send(self):
+        """Non-callable before_send should raise TypeError."""
+        with patch.dict(os.environ, {"DO_NOT_TRACK": ""}, clear=False):
+            TelemetryClient.reset_instance()
+            client = TelemetryClient.get_instance()
+            with patch("openadapt_telemetry.client.sentry_sdk"):
+                with pytest.raises(TypeError, match="before_send must be callable"):
+                    client.initialize(dsn="https://test@example.com/1", before_send="invalid")
+
     @patch("openadapt_telemetry.client.sentry_sdk")
     def test_add_breadcrumb(self, mock_sentry):
         """add_breadcrumb should call sentry when enabled."""