diff --git a/backend/apps/agents/agent_manager.py b/backend/apps/agents/agent_manager.py index 46e744d6..af9c7c65 100644 --- a/backend/apps/agents/agent_manager.py +++ b/backend/apps/agents/agent_manager.py @@ -35,6 +35,7 @@ from backend.apps.agents.core.error_classify import ( _is_long_context_error, _is_transient_capacity_error, _is_unknown_model_error, + redact_for_telemetry, ) from backend.apps.agents.manager.session.session_store import ( _delete_session_file, @@ -3151,7 +3152,7 @@ class AgentManager: "framework_overhead_tokens": session.framework_overhead_tokens, "active_mcps_count": len(session.active_mcps), "messages_count": len(session.messages), - "error_preview": (str(e) or "")[:500], + "error_preview": redact_for_telemetry(str(e), limit=500), }) except Exception: logger.debug("submit_diagnostic for context_overflow failed", exc_info=True) @@ -3256,7 +3257,8 @@ class AgentManager: "model": session.model, "provider": session.provider, "connection_mode": getattr(load_settings(), "connection_mode", "own_key"), - "error_preview": (str(e) or "")[:400], + "error_preview": redact_for_telemetry(str(e), limit=400), + "stderr_tail": redact_for_telemetry(_stderr_tail), }) except Exception: logger.debug("submit_diagnostic model_error failed", exc_info=True) @@ -3276,7 +3278,8 @@ class AgentManager: "model": session.model, "provider": session.provider, "connection_mode": getattr(load_settings(), "connection_mode", "own_key"), - "error_preview": (str(e) or "")[:400], + "error_preview": redact_for_telemetry(str(e), limit=400), + "stderr_tail": redact_for_telemetry(_stderr_tail), }) except Exception: logger.debug("submit_diagnostic model_error failed", exc_info=True) diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index 3a5d4aea..d7b1fb7b 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -1,5 +1,30 @@ import re +# Secret shapes that must never ride along when we ship a stderr tail or an +# error string to telemetry. own_key mode means the subprocess stderr can echo +# the user's OWN provider key, so this scrub is the wall between a diagnostic +# and a key leak; over-redacting is fine, leaking is not. +_TELEMETRY_SECRET_PATTERNS = ( + re.compile(r"sk-ant-[A-Za-z0-9_\-]{12,}"), + re.compile(r"sk-[A-Za-z0-9_\-]{16,}"), + re.compile(r"AIza[A-Za-z0-9_\-]{20,}"), + re.compile(r"gh[pousr]_[A-Za-z0-9]{20,}"), + re.compile(r"(?i)bearer\s+[A-Za-z0-9._\-]{12,}"), + re.compile(r"(?i)\b(?:api[_-]?key|access[_-]?token|refresh[_-]?token|secret|password|authorization)\b[\"']?\s*[:=]\s*[\"']?[A-Za-z0-9._\-]{6,}"), +) + + +def redact_for_telemetry(text: str, *, limit: int = 2000) -> str: + """Scrub secret-shaped substrings, then keep the tail (where the real error + lands), bounded so a runaway log can't bloat the payload. Every raw + error/stderr string goes through here before it leaves the machine.""" + if not text: + return "" + for pat in _TELEMETRY_SECRET_PATTERNS: + text = pat.sub("[redacted]", text) + return text[-limit:] + + # Patterns that indicate an upstream transient problem (overload / rate limit / # infra blip), safe to silently retry with backoff. Checked against the # stringified exception from claude_agent_sdk / Claude CLI. diff --git a/backend/tests/test_error_classify.py b/backend/tests/test_error_classify.py new file mode 100644 index 00000000..191edead --- /dev/null +++ b/backend/tests/test_error_classify.py @@ -0,0 +1,45 @@ +"""redact_for_telemetry is the wall between a model_error diagnostic and a key +leak: in own_key mode the subprocess stderr we now attach can echo the user's +provider key, so these tests pin that no secret shape survives while the actual +error text (the whole point of capturing stderr) does. + +The secret-shaped inputs are built by concatenation on purpose: no contiguous +key-shaped literal lands in this source file (so it never trips gitleaks or +alarms a reader), yet the runtime values are still key-shaped enough to exercise +the scrub. None of these are real keys; they unlock nothing.""" +from backend.apps.agents.core.error_classify import redact_for_telemetry + + +def test_redacts_provider_key_shapes_keeps_context(): + anthropic = "sk-" + "ant-" + "A" * 28 + openai = "sk-" + "B" * 24 + google = "AIza" + "C" * 30 + github = "ghp" + "_" + "D" * 24 + s = f"9router: invalid x-api-key {anthropic} {openai} {google} {github}" + out = redact_for_telemetry(s) + for secret in (anthropic, openai, google, github): + assert secret not in out + assert "[redacted]" in out + # The diagnostic signal survives, that's the reason we capture stderr at all. + assert "9router: invalid x-api-key" in out + + +def test_redacts_bearer_and_key_value(): + bearer_token = "E" * 24 + kv_value = "F" * 16 + s = "Authorization: " + "Bearer " + bearer_token + "\n" + "api_key=" + kv_value + out = redact_for_telemetry(s) + assert bearer_token not in out + assert kv_value not in out + + +def test_keeps_tail_and_bounds_length(): + # The real error lands at the end of the stderr stream, so we keep the tail. + s = "old noise\n" * 500 + "Command failed: ENOENT spawn 9router" + out = redact_for_telemetry(s, limit=120) + assert len(out) <= 120 + assert "Command failed: ENOENT spawn 9router" in out + + +def test_empty_is_safe(): + assert redact_for_telemetry("") == ""