From cfeb4f01707a15326fee5be02010508b7057d4ca Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 10 Jul 2026 12:31:38 +0200 Subject: [PATCH] fix(#2312): scrub model-authored result field before sentinel scan to stop false auth/prompt-too-long DoS --- hive-claude/src/classify.rs | 54 ++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/hive-claude/src/classify.rs b/hive-claude/src/classify.rs index b37511fc..826bacf3 100644 --- a/hive-claude/src/classify.rs +++ b/hive-claude/src/classify.rs @@ -75,7 +75,26 @@ impl Sentinels { if matches!(ty, Some("assistant" | "user")) { return; } - self.scan_failure_markers(raw); + // The terminal `result` event carries the model's final answer in its + // `result` field — model-authored text, same trust level as an + // assistant message. A marker quoted there (e.g. the model explaining + // *this* code, or echoing an error string back) would falsely trip a + // sentinel and needlessly kill/downgrade the turn. So for a `result` + // event, scan the control fields (`subtype` / `error` / `is_error`) + // but scrub the model-authored `result` field first; a genuine + // prompt-too-long/auth/session signal lives in those control fields + // (it's emitted *instead of* a successful model answer), never in the + // `result` text. Other control events (`error` / `system`) are + // claude-authored end to end, so they scan raw. + if ty == Some("result") { + let mut scrubbed = event.clone(); + if let Some(obj) = scrubbed.as_object_mut() { + obj.remove("result"); + } + self.scan_failure_markers(&scrubbed.to_string()); + } else { + self.scan_failure_markers(raw); + } // Rate-limit stays scoped to `error` events (unchanged): the only // control event that carries a rate-limit marker. if ty == Some("error") && RATE_LIMIT_MARKERS.iter().any(|m| raw.contains(m)) { @@ -145,6 +164,39 @@ mod tests { assert!(matches!(s.soft_error(), Some(Error::PromptTooLong))); } + #[test] + fn result_field_model_text_quoting_marker_is_ignored() { + // The terminal `result` event's `result` field is the model's final + // answer. A marker quoted there (the model discussing this very code, + // or echoing an API error string) must NOT trip a sentinel — that was + // a turn-kill DoS. Covers prompt-too-long, auth, and session markers. + for marker in [ + "Prompt is too long", + "Failed to authenticate. API Error: 401", + "does not match any session title", + ] { + let s = Sentinels::default(); + let raw = format!( + r#"{{"type":"result","subtype":"success","is_error":false,"result":"the harness scans stdout for {marker} — see classify.rs"}}"# + ); + s.scan_stdout_json(&json(&raw), &raw); + assert!( + s.soft_error().is_none(), + "marker {marker:?} in the model-authored result field must be ignored" + ); + } + } + + #[test] + fn result_control_field_still_trips_even_with_clean_result_text() { + // Scrubbing `result` must not blind us to a genuine signal in a + // control field of the same event. + let s = Sentinels::default(); + let raw = r#"{"type":"result","subtype":"error","is_error":true,"error":"Failed to authenticate. API Error: 401","result":"ok"}"#; + s.scan_stdout_json(&json(raw), raw); + assert!(matches!(s.soft_error(), Some(Error::AuthFailed))); + } + #[test] fn raw_non_json_marker_is_detected() { let s = Sentinels::default();