Merge pull request #4 from OpenAdaptAI/feat/telemetry-hardening-phase2

feat: harden telemetry privacy filters and anon ID policy

Author: abrichr

README.md

@@ -15,7 +15,7 @@ Unified telemetry and error tracking for OpenAdapt packages.
 - **Unified Error Tracking**: Consistent error reporting across all OpenAdapt packages
 - **Privacy-First Design**: Automatic PII scrubbing and path sanitization
 - **Configurable Opt-Out**: Respects `DO_NOT_TRACK` and custom environment variables
-- **CI/Dev Mode Detection**: Automatically tags internal usage for filtering
+- **Internal Usage Tagging**: Explicit flags + CI detection with optional git heuristic
 - **GlitchTip/Sentry Compatible**: Uses the Sentry SDK for maximum compatibility
 
 ## Installation
@@ -98,10 +98,12 @@ with TelemetrySpan("indexing", "build_faiss_index") as span:
 | `OPENADAPT_TELEMETRY_ENABLED` | `true` | Enable/disable telemetry |
 | `OPENADAPT_INTERNAL` | `false` | Tag as internal usage |
 | `OPENADAPT_DEV` | `false` | Development mode |
+| `OPENADAPT_INTERNAL_FROM_GIT` | `false` | Optional: tag as internal when running from a git checkout |
 | `OPENADAPT_TELEMETRY_DSN` | - | GlitchTip/Sentry DSN |
 | `OPENADAPT_TELEMETRY_ENVIRONMENT` | `production` | Environment name |
 | `OPENADAPT_TELEMETRY_SAMPLE_RATE` | `1.0` | Error sampling rate (0.0-1.0) |
 | `OPENADAPT_TELEMETRY_TRACES_SAMPLE_RATE` | `0.01` | Performance sampling rate |
+| `OPENADAPT_TELEMETRY_ANON_SALT` | generated | Optional anonymization salt override (advanced use only) |
 
 ### Configuration File
 
@@ -160,7 +162,9 @@ 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>`)
+- Top-level event messages/logentry strings are scrubbed
+- Tag keys are validated, sensitive/invalid keys are dropped, and values are scrubbed before upload
+- User IDs are HMAC-anonymized before upload (`anon:v2:<hash>`)
 - `send_default_pii` is enforced to `false` by the client
 
 ## Internal Usage Tagging
@@ -169,8 +173,8 @@ Internal/developer usage is automatically detected via:
 
 1. `OPENADAPT_INTERNAL=true` environment variable
 2. `OPENADAPT_DEV=true` environment variable
-3. Git repository present in working directory
-4. CI environment detected (GitHub Actions, GitLab CI, etc.)
+3. CI environment detected (GitHub Actions, GitLab CI, etc.)
+4. Optional git repository heuristic when `OPENADAPT_INTERNAL_FROM_GIT=true`
 
 Filter in GlitchTip:
 ```

src/openadapt_telemetry/client.py

@@ -60,11 +60,11 @@ def is_ci_environment() -> bool:
 def is_internal_user() -> bool:
     """Determine if current usage is from internal team.
 
-    Uses multiple heuristics to detect internal/developer usage:
+    Uses multiple signals to detect internal/developer usage:
     1. Explicit OPENADAPT_INTERNAL environment variable
     2. OPENADAPT_DEV environment variable
-    3. Git repository present in current directory
-    4. CI environment detected
+    3. CI environment detected
+    4. Optional git repository heuristic when OPENADAPT_INTERNAL_FROM_GIT=true
 
     Returns:
         True if this appears to be internal usage.
@@ -77,25 +77,26 @@ def is_internal_user() -> bool:
     if os.getenv("OPENADAPT_DEV", "").lower() in ("true", "1", "yes"):
         return True
 
-    # Method 3: Git repository present (development checkout)
-    if Path(".git").exists() or Path("../.git").exists():
-        return True
-
-    # Method 4: CI/CD environment
+    # Method 3: CI/CD environment
     if is_ci_environment():
         return True
 
+    # Method 4: optional git heuristic
+    if os.getenv("OPENADAPT_INTERNAL_FROM_GIT", "").lower() in ("true", "1", "yes"):
+        if Path(".git").exists() or Path("../.git").exists():
+            return True
+
     return False
 
 
 def _compose_before_send(base: BeforeSendFn, extra: BeforeSendFn) -> BeforeSendFn:
-    """Compose custom before_send after privacy filtering."""
+    """Compose custom before_send before final privacy filtering."""
 
     def composed(event: Event, hint: Hint) -> Optional[Event]:
-        sanitized = base(event, hint)
-        if sanitized is None:
+        modified = extra(event, hint)
+        if modified is None:
             return None
-        return extra(sanitized, hint)
+        return base(modified, hint)
 
     return composed
 
@@ -219,7 +220,7 @@ def initialize(
             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.",
+                "Custom before_send runs before OpenAdapt privacy filtering; final payload is always scrubbed.",
                 stacklevel=2,
             )
             before_send = _compose_before_send(base_before_send, custom_before_send)

src/openadapt_telemetry/config.py

@@ -11,6 +11,8 @@
 
 import json
 import os
+import secrets
+import warnings
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Any, Optional
@@ -27,11 +29,13 @@
     "performance_tracking": True,
     "feature_usage": True,
     "send_default_pii": False,
+    "anon_salt": None,
 }
 
 # Config file location
 CONFIG_DIR = Path.home() / ".config" / "openadapt"
 CONFIG_FILE = CONFIG_DIR / "telemetry.json"
+_INVALID_ANON_SALT_WARNED = False
 
 
 @dataclass
@@ -48,6 +52,7 @@ class TelemetryConfig:
     performance_tracking: bool = True
     feature_usage: bool = True
     send_default_pii: bool = False
+    anon_salt: Optional[str] = None
 
     _loaded: bool = field(default=False, repr=False)
 
@@ -73,7 +78,8 @@ def _load_config_file() -> dict[str, Any]:
 
     try:
         with open(CONFIG_FILE) as f:
-            return json.load(f)
+            data = json.load(f)
+            return data if isinstance(data, dict) else {}
     except (json.JSONDecodeError, OSError):
         return {}
 
@@ -122,9 +128,72 @@ def _get_env_config() -> dict[str, Any]:
         except ValueError:
             pass
 
+    # Optional override for deterministic anonymization in controlled environments.
+    anon_salt = os.getenv("OPENADAPT_TELEMETRY_ANON_SALT")
+    if anon_salt:
+        if _is_valid_anon_salt(anon_salt):
+            config["anon_salt"] = anon_salt.strip()
+        else:
+            _warn_invalid_anon_salt_once()
+
     return config
 
 
+def _is_valid_anon_salt(value: Any) -> bool:
+    """Check whether a salt value is valid for HMAC anonymization."""
+    return isinstance(value, str) and len(value.strip()) >= 32
+
+
+def _warn_invalid_anon_salt_once() -> None:
+    """Warn once per process when OPENADAPT_TELEMETRY_ANON_SALT is invalid."""
+    global _INVALID_ANON_SALT_WARNED
+    if _INVALID_ANON_SALT_WARNED:
+        return
+    warnings.warn(
+        "Ignoring invalid OPENADAPT_TELEMETRY_ANON_SALT; must be >= 32 chars.",
+        stacklevel=2,
+    )
+    _INVALID_ANON_SALT_WARNED = True
+
+
+def _generate_anon_salt() -> str:
+    """Generate a high-entropy random salt."""
+    return secrets.token_hex(32)
+
+
+def get_or_create_anon_salt() -> str:
+    """Get anonymization salt from env/config, creating one if missing.
+
+    Priority:
+    1. OPENADAPT_TELEMETRY_ANON_SALT (if valid)
+    2. telemetry config file `anon_salt` (if valid)
+    3. generated and persisted random salt
+    """
+    env_salt = os.getenv("OPENADAPT_TELEMETRY_ANON_SALT")
+    if env_salt:
+        if _is_valid_anon_salt(env_salt):
+            return env_salt.strip()
+        _warn_invalid_anon_salt_once()
+
+    config_data = _load_config_file()
+    file_salt = config_data.get("anon_salt")
+    if _is_valid_anon_salt(file_salt):
+        return str(file_salt).strip()
+
+    generated = _generate_anon_salt()
+    config_data["anon_salt"] = generated
+    try:
+        CONFIG_DIR.mkdir(parents=True, exist_ok=True)
+        with open(CONFIG_FILE, "w") as f:
+            json.dump(config_data, f, indent=2)
+    except OSError:
+        warnings.warn(
+            "Failed to persist telemetry anonymization salt; using ephemeral salt for this process.",
+            stacklevel=2,
+        )
+    return generated
+
+
 def load_config() -> TelemetryConfig:
     """Load telemetry configuration from all sources.
 
@@ -148,7 +217,7 @@ def load_config() -> TelemetryConfig:
     merged.update(env_config)
 
     # Remove None values for fields that should use defaults
-    config_dict = {k: v for k, v in merged.items() if v is not None or k == "dsn"}
+    config_dict = {k: v for k, v in merged.items() if v is not None or k in {"dsn", "anon_salt"}}
 
     return TelemetryConfig(**config_dict, _loaded=True)
 
@@ -172,6 +241,7 @@ def save_config(config: TelemetryConfig) -> None:
         "performance_tracking": config.performance_tracking,
         "feature_usage": config.feature_usage,
         "send_default_pii": config.send_default_pii,
+        "anon_salt": config.anon_salt,
     }
 
     with open(CONFIG_FILE, "w") as f:

src/openadapt_telemetry/decorators.py

@@ -10,7 +10,7 @@
 
 import functools
 import time
-from typing import Any, Callable, Optional, TypeVar, Union
+from typing import Any, Callable, Optional, TypeVar
 
 import sentry_sdk
 
@@ -57,7 +57,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
                     result = func(*args, **kwargs)
                     transaction.set_status("ok")
                     return result
-                except Exception as e:
+                except Exception:
                     transaction.set_status("internal_error")
                     raise
                 finally: