fix(computer-use): sanitize env on the 4 remaining cua-driver spawn sites

tests/computer_use/test_cua_spawn_env_sanitization.py

@@ -0,0 +1,120 @@
+"""Regression tests: every remaining cua-driver spawn site must sanitize the
+subprocess environment.
+
+PR #58889 fixed the CLI-fallback transport; review of that fix found four
+sibling spawn sites still handing the third-party ``cua-driver`` binary the
+full parent environment (provider API keys included):
+
+- ``cua_backend._resolve_mcp_invocation`` (``cua-driver manifest``) — no
+  ``env=`` at all
+- ``cua_backend.cua_driver_update_check`` (``check-update --json``) —
+  telemetry env but no secret sanitization
+- ``doctor._drive_health_report`` (``<binary> mcp``) — telemetry env only
+- ``permissions._run`` (every permission probe) — telemetry env only
+"""
+
+import json
+from unittest.mock import MagicMock
+
+SECRET = "sk-super-secret-should-not-leak"
+
+
+def _fake_completed_process(stdout: str) -> MagicMock:
+    proc = MagicMock()
+    proc.stdout = stdout
+    proc.stderr = ""
+    proc.returncode = 0
+    return proc
+
+
+def _capture_run(captured, stdout=""):
+    def fake_run(cmd, **kwargs):
+        captured["cmd"] = cmd
+        captured["env"] = kwargs.get("env")
+        return _fake_completed_process(stdout)
+    return fake_run
+
+
+def _assert_sanitized(captured):
+    env = captured["env"]
+    assert env is not None, "subprocess must receive an explicit env="
+    assert "ANTHROPIC_API_KEY" not in env
+    # Sanitization filters secrets, not everything — ordinary vars survive.
+    assert env.get("PATH") == "/usr/bin:/bin"
+    # Confirms the telemetry helper still ran (default: telemetry disabled).
+    assert env.get("CUA_DRIVER_RS_TELEMETRY_ENABLED") == "0"
+
+
+def test_resolve_mcp_invocation_sanitizes_env(monkeypatch):
+    monkeypatch.setenv("ANTHROPIC_API_KEY", SECRET)
+    monkeypatch.setenv("PATH", "/usr/bin:/bin")
+    monkeypatch.delenv("HERMES_CUA_TELEMETRY", raising=False)
+
+    from tools.computer_use import cua_backend
+
+    captured = {}
+    manifest = json.dumps({"mcp_invocation": {"command": "cua-driver", "args": ["mcp"]}})
+    monkeypatch.setattr(
+        cua_backend.subprocess, "run", _capture_run(captured, stdout=manifest)
+    )
+
+    cmd, args = cua_backend._resolve_mcp_invocation("cua-driver")
+    assert cmd == "cua-driver"
+    _assert_sanitized(captured)
+
+
+def test_update_check_sanitizes_env(monkeypatch):
+    monkeypatch.setenv("ANTHROPIC_API_KEY", SECRET)
+    monkeypatch.setenv("PATH", "/usr/bin:/bin")
+    monkeypatch.delenv("HERMES_CUA_TELEMETRY", raising=False)
+
+    from tools.computer_use import cua_backend
+
+    captured = {}
+    payload = json.dumps({
+        "current_version": "1.0.0",
+        "latest_version": "1.0.0",
+        "update_available": False,
+    })
+    monkeypatch.setattr(
+        cua_backend.subprocess, "run", _capture_run(captured, stdout=payload)
+    )
+
+    cua_backend.cua_driver_update_check(timeout=1.0)
+    _assert_sanitized(captured)
+
+
+def test_permissions_run_sanitizes_env(monkeypatch):
+    monkeypatch.setenv("ANTHROPIC_API_KEY", SECRET)
+    monkeypatch.setenv("PATH", "/usr/bin:/bin")
+    monkeypatch.delenv("HERMES_CUA_TELEMETRY", raising=False)
+
+    from tools.computer_use import permissions
+
+    captured = {}
+    monkeypatch.setattr(
+        permissions.subprocess, "run", _capture_run(captured, stdout="{}")
+    )
+
+    permissions._run("cua-driver", "doctor", "--json", timeout=1.0)
+    _assert_sanitized(captured)
+
+
+def test_doctor_sanitized_env_helper(monkeypatch):
+    """_drive_health_report spawns via Popen; assert the env helper it uses
+    strips secrets (mocking the whole JSON-RPC handshake is not worth it)."""
+    monkeypatch.setenv("ANTHROPIC_API_KEY", SECRET)
+    monkeypatch.setenv("PATH", "/usr/bin:/bin")
+    monkeypatch.delenv("HERMES_CUA_TELEMETRY", raising=False)
+
+    from tools.computer_use import doctor
+    import inspect
+
+    env = doctor._sanitized_cua_env()
+    assert "ANTHROPIC_API_KEY" not in env
+    assert env.get("PATH") == "/usr/bin:/bin"
+    assert env.get("CUA_DRIVER_RS_TELEMETRY_ENABLED") == "0"
+
+    # The Popen spawn site must actually use the sanitized helper.
+    src = inspect.getsource(doctor._drive_health_report)
+    assert "_sanitized_cua_env()" in src

tools/computer_use/cua_backend.py

@@ -160,10 +160,15 @@ def _resolve_mcp_invocation(
     not refuse to start just because the discovery hop failed.
     """
     try:
+        from tools.environments.local import _sanitize_subprocess_env
         proc = subprocess.run(
             [driver_cmd, "manifest"],
             capture_output=True, text=True, timeout=timeout,
             stdin=subprocess.DEVNULL,
+            # cua-driver is a third-party binary — never hand it provider
+            # API keys via inherited env (same policy as the MCP and CLI
+            # fallback spawns below; #53503/#55709/#58889 lineage).
+            env=_sanitize_subprocess_env(cua_driver_child_env()),
         )
     except Exception:
         return driver_cmd, list(_CUA_DRIVER_ARGS)
@@ -246,14 +251,17 @@ def cua_driver_update_check(*, timeout: float = 8.0) -> Optional[Dict[str, Any]]
     raises.
     """
     try:
+        from tools.environments.local import _sanitize_subprocess_env
         proc = subprocess.run(
             [_CUA_DRIVER_CMD, "check-update", "--json"],
             capture_output=True, text=True, timeout=timeout,
             # Some older drivers don't have the verb and fall through to a
             # stdin-reading mode rather than erroring — DEVNULL gives them EOF
             # so they exit fast instead of blocking until the timeout.
             stdin=subprocess.DEVNULL,
-            env=cua_driver_child_env(),
+            # Sanitized like every other cua-driver spawn: third-party
+            # binary, no inherited provider keys (#53503/#55709/#58889).
+            env=_sanitize_subprocess_env(cua_driver_child_env()),
         )
     except Exception:
         return None

tools/computer_use/doctor.py

@@ -52,6 +52,23 @@ def _cua_child_env() -> Dict[str, str]:
         return dict(os.environ)
 
 
+def _sanitized_cua_env() -> Dict[str, str]:
+    """Telemetry-policy env with Hermes provider secrets stripped.
+
+    cua-driver is a third-party binary — it must never inherit provider
+    API keys (#53503/#55709/#58889 lineage). Falls back to the unsanitized
+    telemetry env if the sanitizer can't be imported, so doctor keeps
+    working in stripped-down environments.
+    """
+    env = _cua_child_env()
+    try:
+        from tools.environments.local import _sanitize_subprocess_env
+
+        return _sanitize_subprocess_env(env)
+    except Exception:
+        return env
+
+
 def _drive_health_report(
     binary: str,
     *,
@@ -87,7 +104,7 @@ def _drive_health_report(
         encoding="utf-8",
         errors="replace",
         bufsize=1,
-        env=_cua_child_env(),
+        env=_sanitized_cua_env(),
     )
     try:
         # 1. initialize

tools/computer_use/permissions.py

@@ -47,13 +47,24 @@ def _driver_cmd(override: Optional[str]) -> str:
 
 
 def _child_env() -> Dict[str, str]:
-    """cua-driver child env honoring the Hermes telemetry opt-in policy."""
+    """cua-driver child env: telemetry opt-in policy + secret sanitization.
+
+    cua-driver is a third-party binary — it must never inherit provider
+    API keys (#53503/#55709/#58889 lineage). Each layer degrades
+    gracefully so permission probes never break on a helper import error.
+    """
     try:
         from tools.computer_use.cua_backend import cua_driver_child_env
 
-        return cua_driver_child_env()
+        env = cua_driver_child_env()
+    except Exception:
+        env = dict(os.environ)
+    try:
+        from tools.environments.local import _sanitize_subprocess_env
+
+        return _sanitize_subprocess_env(env)
     except Exception:
-        return dict(os.environ)
+        return env
 
 
 def _run(binary: str, *args: str, timeout: float) -> subprocess.CompletedProcess: