diff --git a/crates/tinyagents-harness/src/providers/claude_code/driver.rs b/crates/tinyagents-harness/src/providers/claude_code/driver.rs index 151208556..7c0b4108e 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/driver.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/driver.rs @@ -68,6 +68,70 @@ fn push_bounded(acc: &mut String, chunk: &str, max_bytes: usize) { } } +/// Upper bound on the structured stdout error carried into a returned error. +const STDOUT_ERROR_CAP: usize = 2_048; + +/// Redact every `sk-` style key in `text`, wherever it sits (after `=`, `:`, +/// quotes, URL separators, ...). A match must start at a non-identifier +/// boundary (so `task-force` is untouched) and run over the key alphabet. +fn redact_sk_keys(text: &str) -> String { + let is_key_char = |c: char| c.is_ascii_alphanumeric() || c == '-' || c == '_'; + let mut out = String::with_capacity(text.len()); + let mut rest = text; + let mut prev: Option = None; + while let Some(pos) = rest.find("sk-") { + let before = rest[..pos].chars().next_back().or(prev); + let boundary = before.is_none_or(|c| !(c.is_ascii_alphanumeric() || c == '_')); + let tail = &rest[pos + 3..]; + let key_len = tail.find(|c: char| !is_key_char(c)).unwrap_or(tail.len()); + if boundary && key_len > 0 { + out.push_str(&rest[..pos]); + out.push_str("[redacted]"); + prev = Some(']'); + rest = &tail[key_len..]; + } else { + out.push_str(&rest[..pos + 3]); + prev = Some('-'); + rest = tail; + } + } + out.push_str(rest); + out +} + +/// Remove credentials from a CLI-reported message and bound its length. +/// Redacts the configured API key wherever it occurs (including inside larger +/// tokens) and any `sk-` style key, then caps the result. +fn sanitize_cli_message(raw: &str, api_key: Option<&str>) -> String { + let mut text = raw.to_string(); + if let Some(key) = api_key.filter(|key| !key.is_empty()) { + text = text.replace(key, "[redacted]"); + } + let redacted = redact_sk_keys(&text); + let mut out = String::new(); + push_bounded(&mut out, &redacted, STDOUT_ERROR_CAP); + out +} + +/// Error text for a nonzero CLI exit: the structured stdout error when the +/// CLI printed one (sanitized and bounded), otherwise the bounded stderr. +fn nonzero_exit_message( + code: Option, + stdout_error: Option<&str>, + stderr: &str, + api_key: Option<&str>, +) -> String { + match stdout_error.map(|e| sanitize_cli_message(e, api_key)) { + Some(message) if !message.is_empty() => { + format!("[claude-code][driver] exit {code:?}: {message}") + } + _ => format!( + "[claude-code][driver] exit {code:?} stderr={}", + sanitize_cli_message(stderr.trim(), api_key) + ), + } +} + use super::bridge::{ChatMessage, ChatResponse, ProviderDelta}; use super::event_mapper::EventMapper; use super::input_builder::build_stdin; @@ -612,14 +676,25 @@ pub(crate) async fn run_turn(ctx: TurnContext<'_>) -> anyhow::Result) -> anyhow::Result { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().expect("tempdir"); + let bin = dir.path().join("claude"); + std::fs::write(&bin, format!("#!/bin/sh\ncat >/dev/null\n{script_body}\n")).expect("script"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + let messages = [ChatMessage::user("hello")]; + run_turn(TurnContext { + bin_path: bin, + workspace_dir: dir.path().join("ws"), + project_dir: dir.path().join("project"), + thread_id: "t-5712".into(), + model: "sonnet".into(), + append_system_prompt: None, + messages: &messages, + session_store: Arc::new(SessionStore::open(&dir.path().join("ws"))), + stream: None, + anthropic_api_key: api_key.map(str::to_string), + persist_session: false, + mcp_provider: None, + }) + .await +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_surfaces_structured_stdout_error() { + let err = run_fake_claude( + r#"echo '{"type":"result","subtype":"success","is_error":true,"result":"Invalid API key - please run /login"}'; exit 1"#, + None, + ) + .await + .expect_err("nonzero exit must fail"); + let text = err.to_string(); + assert!( + text.contains("Invalid API key - please run /login"), + "{text}" + ); + assert!(text.contains("exit Some(1)"), "{text}"); +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_surfaces_error_event_message() { + let err = run_fake_claude( + r#"echo '{"type":"error","error":"model not found"}'; exit 2"#, + None, + ) + .await + .expect_err("nonzero exit must fail"); + assert!(err.to_string().contains("model not found"), "{err}"); +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_without_stdout_error_falls_back_to_stderr() { + let err = run_fake_claude(r#"echo 'boom on stderr' >&2; exit 1"#, None) + .await + .expect_err("nonzero exit must fail"); + let text = err.to_string(); + assert!(text.contains("stderr=boom on stderr"), "{text}"); +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_stdout_error_keeps_secrets_out() { + let err = run_fake_claude( + r#"echo '{"type":"result","is_error":true,"result":"auth failed for key sk-ant-live-key-1234 and my-configured-secret"}'; exit 1"#, + Some("my-configured-secret"), + ) + .await + .expect_err("nonzero exit must fail"); + let text = err.to_string(); + assert!(text.contains("auth failed"), "{text}"); + assert!(!text.contains("sk-ant-live-key-1234"), "{text}"); + assert!(!text.contains("my-configured-secret"), "{text}"); +} + +#[test] +fn stdout_error_is_bounded() { + let long = "e".repeat(STDOUT_ERROR_CAP * 3); + let message = nonzero_exit_message(Some(1), Some(&long), "", None); + assert!(message.len() < STDOUT_ERROR_CAP + 128, "{}", message.len()); +} + +#[test] +fn sk_keys_are_redacted_after_separators() { + for raw in [ + "key:sk-ant-abc123 failed", + "token=sk-ant-abc123", + "https://x.test/?k=sk-ant-abc123&y=1", + "\"sk-ant-abc123\"", + "ANTHROPIC_API_KEY=sk-ant-abc123,", + ] { + let out = sanitize_cli_message(raw, None); + assert!(!out.contains("abc123"), "{raw} -> {out}"); + assert!(out.contains("[redacted]"), "{raw} -> {out}"); + } + assert_eq!( + sanitize_cli_message("task-force ask-me sk-", None), + "task-force ask-me sk-" + ); +} + +#[test] +fn configured_key_embedded_in_larger_token_is_redacted() { + let out = sanitize_cli_message("bad my-secret_suffix and pre-my-secret.", Some("my-secret")); + assert!(!out.contains("my-secret"), "{out}"); +} + +#[cfg(unix)] +#[tokio::test] +async fn zero_exit_is_error_result_is_sanitized_and_bounded() { + let big = "x".repeat(5_000); + let script = format!( + r#"echo '{{"type":"result","subtype":"success","is_error":true,"result":"bad key topsecret99 sk-ant-zzz {big}"}}'; exit 0"# + ); + let err = run_fake_claude(&script, Some("topsecret99")) + .await + .expect_err("is_error must fail"); + let text = err.to_string(); + assert!( + !text.contains("topsecret99") && !text.contains("sk-ant-zzz"), + "{text}" + ); + assert!(text.len() < 2_300, "len {}", text.len()); +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_stderr_fallback_is_redacted_and_bounded() { + let script = r#"echo "auth failed key=topsecret99 sk-ant-live-1234" >&2; head -c 5000 /dev/zero | tr '\0' x >&2; exit 1"#; + let err = run_fake_claude(script, Some("topsecret99")) + .await + .expect_err("nonzero exit must fail"); + let text = err.to_string(); + assert!( + !text.contains("topsecret99") && !text.contains("sk-ant-live-1234"), + "{text}" + ); + assert!(text.len() < 2_300, "len {}", text.len()); +} + +#[cfg(unix)] +#[tokio::test] +async fn nonzero_exit_surfaces_object_error_message_sanitized() { + let err = run_fake_claude( + r#"echo '{"type":"error","error":{"message":"denied for topsecret99 sk-ant-xyz"}}'; exit 2"#, + Some("topsecret99"), + ) + .await + .expect_err("nonzero exit must fail"); + let text = err.to_string(); + assert!(text.contains("denied for"), "{text}"); + assert!( + !text.contains("topsecret99") && !text.contains("sk-ant-xyz"), + "{text}" + ); +} diff --git a/crates/tinyagents-harness/src/providers/claude_code/event_mapper.rs b/crates/tinyagents-harness/src/providers/claude_code/event_mapper.rs index 29648c0c6..2d4f3bf65 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/event_mapper.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/event_mapper.rs @@ -108,7 +108,7 @@ impl EventMapper { subtype, usage, total_cost_usd, - .. + raw, } => { let mut parsed = usage.as_ref().map(parse_usage); // CC stream emits `total_cost_usd` on the terminal `result` @@ -120,8 +120,20 @@ impl EventMapper { usage.charged_amount_usd = cost; } self.usage = parsed; - if subtype.as_deref() == Some("error") && self.error.is_none() { - self.error = Some("claude reported `result.subtype=error`".into()); + // A failed turn is reported as `subtype=error` or as + // `is_error=true` with the human-readable reason in `result`. + let failed = subtype.as_deref() == Some("error") + || raw.get("is_error").and_then(Value::as_bool) == Some(true); + if failed && self.error.is_none() { + let reason = raw + .get("result") + .and_then(Value::as_str) + .map(str::trim) + .filter(|reason| !reason.is_empty()); + self.error = Some(match reason { + Some(reason) => reason.to_string(), + None => "claude reported `result.subtype=error`".into(), + }); } self.finished = true; Vec::new() diff --git a/crates/tinyagents-harness/src/providers/claude_code/event_mapper_tests.rs b/crates/tinyagents-harness/src/providers/claude_code/event_mapper_tests.rs index 17d60f4de..aeb673e42 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/event_mapper_tests.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/event_mapper_tests.rs @@ -160,3 +160,15 @@ fn final_assistant_message_is_skipped() { }); assert!(deltas.is_empty()); } + +#[test] +fn is_error_result_carries_its_reason_as_the_error() { + let mut m = EventMapper::new(); + m.handle(ClaudeCodeEvent::Result { + subtype: Some("success".into()), + usage: None, + total_cost_usd: None, + raw: json!({"type":"result","subtype":"success","is_error":true,"result":"Invalid API key"}), + }); + assert_eq!(m.error.as_deref(), Some("Invalid API key")); +} diff --git a/crates/tinyagents-harness/src/providers/claude_code/stream_parser.rs b/crates/tinyagents-harness/src/providers/claude_code/stream_parser.rs index 2bd8ad508..32c8372c2 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/stream_parser.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/stream_parser.rs @@ -219,7 +219,11 @@ impl StreamJsonParser { "error" => ClaudeCodeEvent::Error { message: v .get("error") - .and_then(Value::as_str) + .and_then(|e| { + e.as_str() + .or_else(|| e.get("message").and_then(Value::as_str)) + }) + .or_else(|| v.get("message").and_then(Value::as_str)) .unwrap_or("claude-code error") .to_string(), }, diff --git a/crates/tinyagents-harness/src/providers/claude_code/stream_parser_tests.rs b/crates/tinyagents-harness/src/providers/claude_code/stream_parser_tests.rs index 1e97518b1..3b6078f7d 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/stream_parser_tests.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/stream_parser_tests.rs @@ -243,3 +243,13 @@ fn feed_bytes_still_replaces_genuinely_invalid_bytes() { "0xFF must decode to the replacement character, not vanish" ); } + +#[test] +fn error_event_with_object_payload_keeps_its_message() { + let mut parser = StreamJsonParser::new(); + let events = parser.feed("{\"type\":\"error\",\"error\":{\"message\":\"rate limited\"}}\n"); + assert!(matches!( + events.as_slice(), + [ClaudeCodeEvent::Error { message }] if message == "rate limited" + )); +} diff --git a/crates/tinyagents-integration-tests/tests/dependency_boundary.rs b/crates/tinyagents-integration-tests/tests/dependency_boundary.rs index e9d653e75..c4f0705dd 100644 --- a/crates/tinyagents-integration-tests/tests/dependency_boundary.rs +++ b/crates/tinyagents-integration-tests/tests/dependency_boundary.rs @@ -43,11 +43,15 @@ const KNOWN_GENERIC_CLAUDE_CODE_CHAT_MESSAGE_DEBT: &[(&str, usize)] = &[ ), ( "crates/tinyagents-harness/src/providers/claude_code/driver.rs", - 71, + 135, ), ( "crates/tinyagents-harness/src/providers/claude_code/driver.rs", - 240, + 304, + ), + ( + "crates/tinyagents-harness/src/providers/claude_code/driver_tests.rs", + 363, ), ( "crates/tinyagents-harness/src/providers/claude_code/input_builder.rs",